Add ability to rewrite cached paths in the database

cmd/podman/root.go

@@ -595,6 +595,8 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
 		_ = cmd.RegisterFlagCompletionFunc(networkBackendFlagName, common.AutocompleteNetworkBackend)
 		_ = pFlags.MarkHidden(networkBackendFlagName)
 
+		pFlags.BoolVar(&podmanConfig.IsRewrite, "rewrite-config", false, "Rewrite cached database configuration")
+
 		rootFlagName := "root"
 		pFlags.StringVar(&podmanConfig.GraphRoot, rootFlagName, "", "Path to the graph root directory where images, containers, etc. are stored")
 		_ = cmd.RegisterFlagCompletionFunc(rootFlagName, completion.AutocompleteDefault)

docs/source/markdown/podman.1.md

@@ -124,6 +124,14 @@ When true, access to the Podman service is remote. Defaults to false.
 Settings can be modified in the containers.conf file. If the CONTAINER_HOST
 environment variable is set, the **--remote** option defaults to true.
 
+#### **--rewrite-config**
+When true, cached configuration values in the database will be rewritten.
+Normally, changes to certain configuration values - graphDriver, graphRoot, and runRoot in storage.conf, as well as static_dir, tmp_dir, and volume_path in containers.conf - will be ignored until a `podman system reset`, as old values cached in the database will be used.
+This is done to ensure that configuration changes do not break existing pods, containers, and volumes present in the database.
+This option rewrites the cached values in the database, replacing them with the current configuration.
+This can only be done if no containers, pods, and volumes are present, to prevent the breakage described earlier.
+If any containers, pods, or volumes are present, an error will be returned.
+
 #### **--root**=*value*
 
 Storage root dir in which data, including images, is stored (default: "/var/lib/containers/storage" for UID 0, "$HOME/.local/share/containers/storage" for other users).

libpod/boltdb_state.go

@@ -493,11 +493,15 @@ func (s *BoltState) GetDBConfig() (*DBConfig, error) {
 }
 
 // ValidateDBConfig validates paths in the given runtime against the database
-func (s *BoltState) ValidateDBConfig(runtime *Runtime) error {
+func (s *BoltState) ValidateDBConfig(runtime *Runtime, performRewrite bool) error {
 	if !s.valid {
 		return define.ErrDBClosed
 	}
 
+	if performRewrite {
+		return fmt.Errorf("rewriting database config is not allowed with the boltdb database backend: %w", define.ErrNotSupported)
+	}
+
 	db, err := s.getDBCon()
 	if err != nil {
 		return err

libpod/define/errors.go

@@ -148,6 +148,8 @@ var (
 	// yet present
 	ErrNotImplemented = errors.New("not yet implemented")
 
+	// ErrNotSupported indicates this function is not supported.
+	ErrNotSupported = errors.New("not supported")
 	// ErrOSNotSupported indicates the function is not available on the particular
 	// OS.
 	ErrOSNotSupported = errors.New("no support for this OS yet")

libpod/options.go

@@ -430,6 +430,23 @@ func WithRenumber() RuntimeOption {
 	}
 }
 
+// WithRewrite tells Libpod that the runtime should rewrite cached configuration
+// paths in the database.
+// This must be used to make configuration changes to certain paths take effect.
+// This option can only be used if no containers, pods, and volumes exist.
+// The runtime is fully usable after being returned.
+func WithRewrite() RuntimeOption {
+	return func(rt *Runtime) error {
+		if rt.valid {
+			return define.ErrRuntimeFinalized
+		}
+
+		rt.doRewrite = true
+
+		return nil
+	}
+}
+
 // WithEventsLogger sets the events backend to use.
 // Currently supported values are "file" for file backend and "journald" for
 // journald backend.

libpod/runtime.go

@@ -110,6 +110,15 @@ type Runtime struct {
 	// errors related to lock initialization so a renumber can be performed
 	// if something has gone wrong.
 	doRenumber bool
+	// doRewrite indicates that the runtime will overwrite cached values in
+	// the database for a number of paths in the configuration (e.g.
+	// graphroot, runroot, tmpdir). These paths will otherwise be overridden
+	// by cached versions when changed by the user, requiring a wipe of the
+	// database to change.
+	// If doRewrite is set, the returned runtime is fully usable.
+	// If doRewrite is set and any containers, pods, or volumes are present
+	// in the database, an error will be returned during runtime init.
+	doRewrite bool
 
 	// valid indicates whether the runtime is ready to use.
 	// valid is set to true when a runtime is returned from GetRuntime(),
@@ -394,7 +403,11 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
 		return fmt.Errorf("retrieving runtime configuration from database: %w", err)
 	}
 
-	runtime.mergeDBConfig(dbConfig)
+	if !runtime.doRewrite {
+		runtime.mergeDBConfig(dbConfig)
+	} else {
+		logrus.Debugf("Going to rewrite cached paths in database. Values below will be used for new cached configuration.")
+	}
 
 	checkCgroups2UnifiedMode(runtime)
 
@@ -408,7 +421,7 @@ func makeRuntime(ctx context.Context, runtime *Runtime) (retErr error) {
 
 	// Validate our config against the database, now that we've set our
 	// final storage configuration
-	if err := runtime.state.ValidateDBConfig(runtime); err != nil {
+	if err := runtime.state.ValidateDBConfig(runtime, runtime.doRewrite); err != nil {
 		// If we are performing a storage reset: continue on with a
 		// warning. Otherwise we can't `system reset` after a change to
 		// the core paths.

libpod/sqlite_state.go

@@ -306,7 +306,7 @@ func (s *SQLiteState) GetDBConfig() (*DBConfig, error) {
 }
 
 // ValidateDBConfig validates paths in the given runtime against the database
-func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
+func (s *SQLiteState) ValidateDBConfig(runtime *Runtime, performRewrite bool) (defErr error) {
 	if !s.valid {
 		return define.ErrDBClosed
 	}
@@ -322,6 +322,20 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
                 ?, ?, ?,
                 ?, ?, ?
         );`
+	const getNumObject = `
+        SELECT
+                (SELECT COUNT(*) FROM ContainerConfig) AS container_count,
+                (SELECT COUNT(*) FROM PodConfig)       AS pod_count,
+                (SELECT COUNT(*) FROM VolumeConfig)    AS volume_count;`
+	const updateRow = `
+        UPDATE DBConfig SET
+                OS=?,
+                StaticDir=?,
+                TmpDir=?,
+                GraphRoot=?,
+                RunRoot=?,
+                GraphDriver=?,
+                VolumeDir=?;`
 
 	var (
 		dbOS, staticDir, tmpDir, graphRoot, runRoot, graphDriver, volumePath string
@@ -335,7 +349,9 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
 	)
 
 	// Some fields may be empty, indicating they are set to the default.
-	// If so, grab the default from c/storage for them.
+	// If so, grab the values from c/storage for them.
+	// TODO: Validate the c/storage ones are not empty string,
+	// grab default values if they are.
 	if runtimeGraphRoot == "" {
 		runtimeGraphRoot = storeOpts.GraphRoot
 	}
@@ -386,6 +402,37 @@ func (s *SQLiteState) ValidateDBConfig(runtime *Runtime) (defErr error) {
 		return fmt.Errorf("retrieving DB config: %w", err)
 	}
 
+	if performRewrite {
+		// If a rewrite of database configuration is requested:
+		// First ensure no containers, pods, volumes are present.
+		// If clear to proceed, update the row and return. Do not
+		// perform any checks; use current configuration without
+		// question, as we would on DB init.
+		var numCtrs, numPods, numVols int
+		countRow := tx.QueryRow(getNumObject)
+		if err := countRow.Scan(&numCtrs, &numPods, &numVols); err != nil {
+			return fmt.Errorf("querying number of objects in database: %w", err)
+		}
+		if numCtrs+numPods+numVols != 0 {
+			return fmt.Errorf("refusing to rewrite database cached configuration as containers, pods, or volumes are present: %w", define.ErrInternal)
+		}
+		result, err := tx.Exec(updateRow, runtimeOS, runtimeStaticDir, runtimeTmpDir, runtimeGraphRoot, runtimeRunRoot, runtimeGraphDriver, runtimeVolumePath)
+		if err != nil {
+			return fmt.Errorf("updating database cached configuration: %w", err)
+		}
+		rows, err := result.RowsAffected()
+		if err != nil {
+			return fmt.Errorf("counting rows affected by DB configuration update: %w", err)
+		}
+		if rows != 1 {
+			return fmt.Errorf("updated %d rows when changing DB configuration, expected 1: %w", rows, define.ErrInternal)
+		}
+		if err := tx.Commit(); err != nil {
+			return fmt.Errorf("committing write of database validation row: %w", err)
+		}
+		return nil
+	}
+
 	// 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.

libpod/state.go

@@ -39,7 +39,11 @@ type State interface { //nolint:interfacebloat
 	// This is not implemented by the in-memory state, as it has no need to
 	// validate runtime configuration that may change over multiple runs of
 	// the program.
-	ValidateDBConfig(runtime *Runtime) error
+	// If performRewrite is set, the current configuration values will be
+	// overwritten by the values given in the current runtime struct.
+	// This occurs if and only if no containers, pods, and volumes are
+	// present.
+	ValidateDBConfig(runtime *Runtime, performRewrite bool) error
 
 	// Resolve an ID to a Container Name.
 	GetContainerName(id string) (string, error)
@@ -164,7 +168,7 @@ type State interface { //nolint:interfacebloat
 	// There are a lot of capital letters and conditions here, but the short
 	// answer is this: use this only very sparingly, and only if you really
 	// know what you're doing.
-	// TODO: Once BoltDB is removed, RewriteContainerConfig and
+	// TODO 6.0: Once BoltDB is removed, RewriteContainerConfig and
 	// SafeRewriteContainerConfig can be merged.
 	RewriteContainerConfig(ctr *Container, newCfg *ContainerConfig) error
 	// This is a more limited version of RewriteContainerConfig, though it

pkg/domain/entities/engine.go

@@ -39,6 +39,7 @@ type PodmanConfig struct {
 	Identity                 string   // ssh identity for connecting to server
 	IsRenumber               bool     // Is this a system renumber command? If so, a number of checks will be relaxed
 	IsReset                  bool     // Is this a system reset command? If so, a number of checks will be skipped/omitted
+	IsRewrite                bool     // Rewrite cached database configuration.
 	MaxWorks                 int      // maximum number of parallel threads
 	MemoryProfile            string   // Hidden: Should memory profile be taken
 	RegistriesConf           string   // allows for specifying a custom registries.conf

pkg/domain/infra/runtime_libpod.go

@@ -134,6 +134,9 @@ func getRuntime(ctx context.Context, fs *flag.FlagSet, opts *engineOpts) (*libpo
 	if opts.renumber {
 		options = append(options, libpod.WithRenumber())
 	}
+	if opts.config.IsRewrite {
+		options = append(options, libpod.WithRewrite())
+	}
 
 	if len(cfg.RuntimeFlags) > 0 {
 		runtimeFlags := []string{}

test/system/005-info.bats

@@ -328,4 +328,69 @@ EOF
     CONTAINERS_STORAGE_CONF=$PODMAN_TMPDIR/storage.conf run_podman $safe_opts info
 }
 
+@test "podman info --rewrite-config respects new runRoot" {
+    skip_if_remote "Test uses nonstandard paths for c/storage directories"
+    skip_if_boltdb "Config rewrite only implemented for SQLite"
+
+    # Create temporary storage directories
+    GRAPHROOT=$PODMAN_TMPDIR/graphroot
+    RUNROOT_A=$PODMAN_TMPDIR/runroota
+    RUNROOT_B=$PODMAN_TMPDIR/runrootb
+
+    STORAGE_CONF1=$PODMAN_TMPDIR/storage1.conf
+    STORAGE_CONF2=$PODMAN_TMPDIR/storage2.conf
+
+    cat >$STORAGE_CONF1 <<EOF
+[storage]
+driver="$(podman_storage_driver)"
+graphroot = "$GRAPHROOT"
+runroot = "$RUNROOT_A"
+[storage.options]
+EOF
+
+    cat >$STORAGE_CONF2 <<EOF
+[storage]
+driver="$(podman_storage_driver)"
+graphroot = "$GRAPHROOT"
+runroot = "$RUNROOT_B"
+[storage.options]
+EOF
+
+    # First, verify original runRoot is used with first config
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF1 run_podman info
+    assert "$output" =~ "runRoot: $RUNROOT_A" "runRoot not properly set on first Podman call"
+
+    # Second config should be ignored; still original runRoot
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF2 run_podman info
+    assert "$output" =~ "runRoot: $RUNROOT_A" "runRoot overwritten by subsequent Podman call"
+
+    # Rewrite with a container should fail
+    randomctrname=c_$(safename)
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF1 run_podman create --name $randomctrname $IMAGE sh
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF2 run_podman 125 --rewrite-config info
+    assert "$output" = "Error: refusing to rewrite database cached configuration as containers, pods, or volumes are present: internal libpod error"
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF1 run_podman rm $randomctrname
+
+    # Rewrite with a pod should fail
+    randompodname=p_$(safename)
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF1 run_podman pod create $randompodname
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF2 run_podman 125 --rewrite-config info
+    assert "$output" = "Error: refusing to rewrite database cached configuration as containers, pods, or volumes are present: internal libpod error"
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF1 run_podman pod rm $randompodname
+
+    # Rewrite with a volume should fail
+    randomvolname=v_$(safename)
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF1 run_podman volume create $randomvolname
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF2 run_podman 125 --rewrite-config info
+    assert "$output" = "Error: refusing to rewrite database cached configuration as containers, pods, or volumes are present: internal libpod error"
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF1 run_podman volume rm $randomvolname
+
+    # With rewrite-config flag, new runRoot should be used
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF2 run_podman --rewrite-config info
+    assert "$output" =~ "runRoot: $RUNROOT_B" "runRoot not overwritten by --rewrite-config"
+
+    # Since we ran a container, there'll be leftover files in GRAPHROOT to clean up
+    CONTAINERS_STORAGE_CONF=$STORAGE_CONF2 run_podman system reset --force
+}
+
 # vim: filetype=sh

test/system/helpers.bash

@@ -935,6 +935,15 @@ function skip_if_aarch64 {
     fi
 }
 
+##################################
+#  skip_if_boltdb  #  ...with an optional message
+##################################
+function skip_if_boltdb {
+    if [[ "$CI_DESIRED_DATABASE" = "boltdb" ]]; then
+        skip "${1:-not supported with boltdb database backend}"
+    fi
+}
+
 #########
 #  die  #  Abort with helpful message
 #########