Add ability to rewrite cached paths in the database

This is accomplished by a new flag, `--rewrite-config`, which
instructs the database to replace cached paths. Before this, the
only real way to change paths like tmpdir, runroot, graphroot,
etc was to do a `podman system reset` then change the config
files before running another Podman command. Now, changing the
config files and running any Podman command with
`--rewrite-config` should be sufficient to pick up the new paths.

Please note that this can only be done with no containers, pods,
and volumes present. Otherwise, we risk the breakages that
caching paths was supposed to prevent in the first place.

This is SQLite only, given the deprecation and impending removal
of BoltDB.

Signed-off-by: Matt Heon <[email protected]>

Author: mheon

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{}