fix handling of static/volume dir

The processing and setting of the static and volume directories was
scattered across the code base (including c/common) leading to subtle
errors that surfaced in #19938.

There were multiple issues that I try to summarize below:

 - c/common loaded the graphroot from c/storage to set the defaults for
   static and volume dir.  That ignored Podman's --root flag and
   surfaced in #19938 and other bugs.  c/common does not set the
   defaults anymore which gives Podman the ability to detect when the
   user/admin configured a custom directory (not empty value).

 - When parsing the CLI, Podman (ab)uses containers.conf structures to
   set the defaults but also to override them in case the user specified
   a flag.  The --root flag overrode the static dir which is wrong and
   broke a couple of use cases.  Now there is a dedicated field for in
   the "PodmanConfig" which also includes a containers.conf struct.

 - The defaults for static and volume dir and now being set correctly
   and adhere to --root.

Overall I find that the code and logic is scattered and hard to
understand and follow.  I refrained from larger refactorings as I really
just want to get #19938 fixed and then go back to other priorities.

Fixes: #19938
Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>

cmd/podman/root.go

@@ -540,7 +540,7 @@ func rootFlags(cmd *cobra.Command, podmanConfig *entities.PodmanConfig) {
 		_ = pFlags.MarkHidden(networkBackendFlagName)
 
 		rootFlagName := "root"
-		pFlags.StringVar(&podmanConfig.ContainersConf.Engine.StaticDir, rootFlagName, podmanConfig.ContainersConfDefaultsRO.Engine.StaticDir, "Path to the root directory in which data, including images, is stored")
+		pFlags.StringVar(&podmanConfig.GraphRoot, rootFlagName, "", "Path to the graph root directory where images, containers, etc. are stored")
 		_ = cmd.RegisterFlagCompletionFunc(rootFlagName, completion.AutocompleteDefault)
 
 		pFlags.StringVar(&podmanConfig.RegistriesConf, "registries-conf", "", "Path to a registries.conf to use for image processing")

go.mod

@@ -201,3 +201,5 @@ require (
 )
 
 replace github.com/opencontainers/runc => github.com/opencontainers/runc v1.1.1-0.20230904132852-a0466dd76f23
+
+replace github.com/containers/common => github.com/vrothberg/common v0.0.3-0.20230921092713-5e6ac49ab839

go.sum

@@ -249,8 +249,6 @@ github.com/containernetworking/plugins v1.3.0 h1:QVNXMT6XloyMUoO2wUOqWTC1hWFV62Q
 github.com/containernetworking/plugins v1.3.0/go.mod h1:Pc2wcedTQQCVuROOOaLBPPxrEXqqXBFt3cZ+/yVg6l0=
 github.com/containers/buildah v1.32.0 h1:uz5Rcf7lGeStj7iPTBgO4UdhQYZqMMzyt9suDf16k1k=
 github.com/containers/buildah v1.32.0/go.mod h1:sN3rA3DbnqekNz3bNdkqWduuirYDuMs54LUCOZOomBE=
-github.com/containers/common v0.56.1-0.20230920110729-eb4ad859f309 h1:ZuP6m9Ps9bBR/3TlR4AqzobAvtYstKkoYmKIg3R7O84=
-github.com/containers/common v0.56.1-0.20230920110729-eb4ad859f309/go.mod h1:ABFEglmyt48WWWQv80kGhitfbVfR1Br35wk3gBQdrIk=
 github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg=
 github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I=
 github.com/containers/gvisor-tap-vsock v0.7.1-0.20230921084021-9298405740ad h1:bJu6ieGZqM6C1SyDkpw2x7D3T5VqajoyvudLmdS50e0=
@@ -1034,6 +1032,8 @@ github.com/vishvananda/netns v0.0.4 h1:Oeaw1EM2JMxD51g9uhtC0D7erkIjgmj8+JZc26m1Y
 github.com/vishvananda/netns v0.0.4/go.mod h1:SpkAiCQRtJ6TvvxPnOSyH3BMl6unz3xZlaprSwhNNJM=
 github.com/vmihailenco/msgpack/v5 v5.3.5 h1:5gO0H1iULLWGhs2H5tbAHIZTV8/cYafcFOr9znI5mJU=
 github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g=
+github.com/vrothberg/common v0.0.3-0.20230921092713-5e6ac49ab839 h1:rXUz05qE7VgDMyvxiiL0lauqhvgfRmkvQZaC3fwYolI=
+github.com/vrothberg/common v0.0.3-0.20230921092713-5e6ac49ab839/go.mod h1:ABFEglmyt48WWWQv80kGhitfbVfR1Br35wk3gBQdrIk=
 github.com/willf/bitset v1.1.11-0.20200630133818-d5bec3311243/go.mod h1:RjeCKbqT1RxIR/KWY6phxZiaY1IyutSBfGjNPySAYV4=
 github.com/willf/bitset v1.1.11/go.mod h1:83CECat5yLh5zVOf4P1ErAgKA5UDvKtgyUABdr3+MjI=
 github.com/xdg-go/pbkdf2 v1.0.0/go.mod h1:jrpuAogTd400dnrH08LKmI/xc1MbPOebTwRqcT5RDeI=

libpod/options.go

@@ -52,17 +52,22 @@ func WithStorageConfig(config storage.StoreOptions) RuntimeOption {
 		if config.GraphRoot != "" {
 			rt.storageConfig.GraphRoot = config.GraphRoot
 			rt.storageSet.GraphRootSet = true
+			setField = true
+		}
 
+		if rt.config.Engine.StaticDir != "" {
 			// Also set libpod static dir, so we are a subdirectory
 			// of the c/storage store by default
-			rt.config.Engine.StaticDir = filepath.Join(config.GraphRoot, "libpod")
+			rt.config.Engine.StaticDir = filepath.Join(rt.storageConfig.GraphRoot, "libpod")
 			rt.storageSet.StaticDirSet = true
+			setField = true
+		}
 
+		if rt.config.Engine.VolumePath != "" {
 			// Also set libpod volume path, so we are a subdirectory
 			// of the c/storage store by default
-			rt.config.Engine.VolumePath = filepath.Join(config.GraphRoot, "volumes")
+			rt.config.Engine.VolumePath = filepath.Join(rt.storageConfig.GraphRoot, "volumes")
 			rt.storageSet.VolumePathSet = true
-
 			setField = true
 		}
 
@@ -270,7 +275,7 @@ func WithStaticDir(dir string) RuntimeOption {
 		}
 
 		rt.config.Engine.StaticDir = dir
-		rt.config.Engine.StaticDirSet = true
+		rt.storageSet.StaticDirSet = true
 
 		return nil
 	}
@@ -372,7 +377,7 @@ func WithTmpDir(dir string) RuntimeOption {
 			return define.ErrRuntimeFinalized
 		}
 		rt.config.Engine.TmpDir = dir
-		rt.config.Engine.TmpDirSet = true
+		rt.storageSet.TmpDirSet = true
 
 		return nil
 	}
@@ -457,7 +462,7 @@ func WithVolumePath(volPath string) RuntimeOption {
 		}
 
 		rt.config.Engine.VolumePath = volPath
-		rt.config.Engine.VolumePathSet = true
+		rt.storageSet.VolumePathSet = true
 
 		return nil
 	}

pkg/domain/entities/engine.go

@@ -58,4 +58,5 @@ type PodmanConfig struct {
 	SSHMode        string
 	MachineMode    bool
 	TransientStore bool
+	GraphRoot      string
 }

pkg/domain/infra/runtime_libpod.go

@@ -123,7 +123,7 @@ func getRuntime(ctx context.Context, fs *flag.FlagSet, opts *engineOpts) (*libpo
 	storageOpts := types.StoreOptions{}
 	cfg := opts.config
 
-	storageSet := false
+	storageSet := cfg.ContainersConfDefaultsRO.Engine.StaticDir != ""
 
 	uidmapFlag := fs.Lookup("uidmap")
 	gidmapFlag := fs.Lookup("gidmap")
@@ -148,7 +148,7 @@ func getRuntime(ctx context.Context, fs *flag.FlagSet, opts *engineOpts) (*libpo
 
 	if fs.Changed("root") {
 		storageSet = true
-		storageOpts.GraphRoot = cfg.ContainersConf.Engine.StaticDir
+		storageOpts.GraphRoot = cfg.GraphRoot
 		storageOpts.GraphDriverOptions = []string{}
 	}
 	if fs.Changed("runroot") {

test/system/030-run.bats

@@ -1212,6 +1212,26 @@ EOF
     run_podman rm -f -t0 $ctr
 }
 
+@test "podman run - custom static_dir" {
+    # regression test for #19938 to make sure the cleanup process uses the same
+    # static_dir and writes the exit code.  If not, podman-run will run into
+    # it's 20 sec timeout waiting for the exit code to be written.
+
+    skip_if_remote "CONTAINERS_CONF_OVERRIDE redirect does not work on remote"
+    containersconf=$PODMAN_TMPDIR/containers.conf
+    static_dir=$PODMAN_TMPDIR/static_dir
+cat >$containersconf <<EOF
+[containers]
+static_dir="$static_dir"
+EOF
+    ctr=$(random_string)
+    CONTAINERS_CONF_OVERRIDE=$containersconf PODMAN_TIMEOUT=20 run_podman run $IMAGE true
+    # Since the container has been run with custom static_dir (where the libpod
+    # DB is stored), the default podman should not see it.
+    run_podman 1 container exists $ctr
+    CONTAINERS_CONF_OVERRIDE=$containersconf run_podman rm -f -t0 $ctr
+}
+
 @test "podman --authfile=nonexistent-path" {
     # List of commands to be tested. These all share a common authfile check.
     #

vendor/modules.txt

@@ -164,7 +164,7 @@ github.com/containers/buildah/pkg/sshagent
 github.com/containers/buildah/pkg/util
 github.com/containers/buildah/pkg/volumes
 github.com/containers/buildah/util
-# github.com/containers/common v0.56.1-0.20230920110729-eb4ad859f309
+# github.com/containers/common v0.56.1-0.20230920110729-eb4ad859f309 => github.com/vrothberg/common v0.0.3-0.20230921092713-5e6ac49ab839
 ## explicit; go 1.18
 github.com/containers/common/libimage
 github.com/containers/common/libimage/define
@@ -1294,3 +1294,4 @@ k8s.io/kubernetes/third_party/forked/golang/expansion
 ## explicit; go 1.12
 sigs.k8s.io/yaml
 # github.com/opencontainers/runc => github.com/opencontainers/runc v1.1.1-0.20230904132852-a0466dd76f23
+# github.com/containers/common => github.com/vrothberg/common v0.0.3-0.20230921092713-5e6ac49ab839