Merge pull request #24775 from mheon/fix_24738

In SQLite state, use defaults for empty-string checks

Author: openshift-merge-bot[bot]

libpod/sqlite_state.go

@@ -383,6 +383,30 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
 		return fmt.Errorf("retrieving DB config: %w", err)
 	}
 
+	// Sometimes, for as-yet unclear reasons, the database value ends up set
+	// to the empty string. If it does, this evaluation is always going to
+	// fail, and libpod will be unusable.
+	// At this point, the check is effectively meaningless - we don't
+	// actually know the settings we should be checking against. The best
+	// thing we can do (and what BoltDB did in this case) is to compare
+	// against the default, on the assumption that is what was in use.
+	// TODO: We can't remove this code without breaking existing SQLite DBs
+	// that already have incorrect values in the database, but we should
+	// investigate why this is happening and try and prevent the creation of
+	// new databases with these garbage checks.
+	if graphRoot == "" {
+		logrus.Debugf("Database uses empty-string graph root, substituting default %q", storeOpts.GraphRoot)
+		graphRoot = storeOpts.GraphRoot
+	}
+	if runRoot == "" {
+		logrus.Debugf("Database uses empty-string run root, substituting default %q", storeOpts.RunRoot)
+		runRoot = storeOpts.RunRoot
+	}
+	if graphDriver == "" {
+		logrus.Debugf("Database uses empty-string graph driver, substituting default %q", storeOpts.GraphDriverName)
+		graphDriver = storeOpts.GraphDriverName
+	}
+
 	checkField := func(fieldName, dbVal, ourVal string, isPath bool) error {
 		if isPath {
 			// Tolerate symlinks when possible - most relevant for OStree systems

test/system/005-info.bats

@@ -305,4 +305,27 @@ EOF
     run_podman $safe_opts system reset --force
 }
 
+@test "podman - empty string defaults for certain values" {
+    skip_if_remote "Test uses nonstandard paths for c/storage directories"
+
+    # We just want this to be empty - so graph driver will be set to the empty string
+    touch $PODMAN_TMPDIR/storage.conf
+
+    safe_opts=$(podman_isolation_opts ${PODMAN_TMPDIR})
+
+    # Force all custom directories so we don't pick up an existing database
+    CONTAINERS_STORAGE_CONF=$PODMAN_TMPDIR/storage.conf run_podman 0+w $safe_opts info
+    require_warning "The storage 'driver' option should be set" \
+       	            "c/storage should warn on empty storage driver"
+
+    # Now add a valid graph driver to storage.conf
+    cat >$PODMAN_TMPDIR/storage.conf <<EOF
+[storage]
+driver="$(podman_storage_driver)"
+EOF
+
+    # Second run of Podman should still succeed after editing the graph driver.
+    CONTAINERS_STORAGE_CONF=$PODMAN_TMPDIR/storage.conf run_podman $safe_opts info
+}
+
 # vim: filetype=sh