From b8e6ef618bd9233796ef6dbff187052a499ea022 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 26 Feb 2019 15:32:53 -0500 Subject: Move all storage configuration defaults into libpod Instead of passing in defaults via WithStorageConfig after computing them in cmd/podman/libpodruntime, do all defaults in libpod itself. This can alleviate ordering issues which caused settings in the libpod config (most notably, volume path) to be ignored. Signed-off-by: Matthew Heon --- cmd/podman/libpodruntime/runtime.go | 18 ++++++++++++------ libpod/runtime.go | 15 ++++++--------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/cmd/podman/libpodruntime/runtime.go b/cmd/podman/libpodruntime/runtime.go index f4ddf3521..2b96f0c20 100644 --- a/cmd/podman/libpodruntime/runtime.go +++ b/cmd/podman/libpodruntime/runtime.go @@ -5,6 +5,7 @@ import ( "github.com/containers/libpod/libpod" "github.com/containers/libpod/pkg/rootless" "github.com/containers/libpod/pkg/util" + "github.com/containers/storage" "github.com/pkg/errors" ) @@ -20,11 +21,8 @@ func GetRuntime(c *cliconfig.PodmanCommand) (*libpod.Runtime, error) { func getRuntime(c *cliconfig.PodmanCommand, renumber bool) (*libpod.Runtime, error) { options := []libpod.RuntimeOption{} - - storageOpts, _, err := util.GetDefaultStoreOptions() - if err != nil { - return nil, err - } + storageOpts := storage.StoreOptions{} + storageSet := false uidmapFlag := c.Flags().Lookup("uidmap") gidmapFlag := c.Flags().Lookup("gidmap") @@ -43,25 +41,33 @@ func getRuntime(c *cliconfig.PodmanCommand, renumber bool) (*libpod.Runtime, err storageOpts.UIDMap = mappings.UIDMap storageOpts.GIDMap = mappings.GIDMap + storageSet = true } if c.Flags().Changed("root") { + storageSet = true storageOpts.GraphRoot = c.GlobalFlags.Root } if c.Flags().Changed("runroot") { + storageSet = true storageOpts.RunRoot = c.GlobalFlags.Runroot } if len(storageOpts.RunRoot) > 50 { return nil, errors.New("the specified runroot is longer than 50 characters") } if c.Flags().Changed("storage-driver") { + storageSet = true storageOpts.GraphDriverName = c.GlobalFlags.StorageDriver } if len(c.GlobalFlags.StorageOpts) > 0 { + storageSet = true storageOpts.GraphDriverOptions = c.GlobalFlags.StorageOpts } - options = append(options, libpod.WithStorageConfig(storageOpts)) + // Only set this if the user changes storage config on the command line + if storageSet { + options = append(options, libpod.WithStorageConfig(storageOpts)) + } // TODO CLI flags for image config? // TODO CLI flag for signature policy? diff --git a/libpod/runtime.go b/libpod/runtime.go index 827c22f5b..635783790 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -331,16 +331,13 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { deepcopier.Copy(defaultRuntimeConfig).To(runtime.config) runtime.config.TmpDir = tmpDir - if rootless.IsRootless() { - // If we're rootless, override the default storage config - storageConf, volumePath, err := util.GetDefaultStoreOptions() - if err != nil { - return nil, errors.Wrapf(err, "error retrieving rootless storage config") - } - runtime.config.StorageConfig = storageConf - runtime.config.StaticDir = filepath.Join(storageConf.GraphRoot, "libpod") - runtime.config.VolumePath = volumePath + storageConf, volumePath, err := util.GetDefaultStoreOptions() + if err != nil { + return nil, errors.Wrapf(err, "error retrieving rootless storage config") } + runtime.config.StorageConfig = storageConf + runtime.config.StaticDir = filepath.Join(storageConf.GraphRoot, "libpod") + runtime.config.VolumePath = volumePath configPath := ConfigPath foundConfig := true -- cgit v1.2.3-54-g00ecf From 5511cdc4879925c14243be915c51fd7e02d7a2f7 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 26 Feb 2019 15:39:13 -0500 Subject: Ensure volume path is set appropriately by default There are some cases where we might not be properly adjusting the volume path after setting the storage graph root. Ensure that we always set volume path to be a child of graph root. Signed-off-by: Matthew Heon --- pkg/util/utils.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pkg/util/utils.go b/pkg/util/utils.go index db8a3d5bb..2468d525a 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -325,6 +325,7 @@ func GetDefaultStoreOptions() (storage.StoreOptions, string, error) { defaultRootlessGraphRoot = storageOpts.GraphRoot storageOpts = storage.StoreOptions{} storage.ReloadConfigurationFile(storageConf, &storageOpts) + volumePath = filepath.Join(storageOpts.GraphRoot, "volumes") } if rootless.IsRootless() { @@ -351,6 +352,7 @@ func GetDefaultStoreOptions() (storage.StoreOptions, string, error) { if storageOpts.GraphRoot == "" { storageOpts.GraphRoot = defaultRootlessGraphRoot } + volumePath = filepath.Join(storageOpts.GraphRoot, "volumes") } } return storageOpts, volumePath, nil -- cgit v1.2.3-54-g00ecf From f68a243f8ee4d36990a50fa490dc427520dc8a14 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 26 Feb 2019 15:44:46 -0500 Subject: Centralize setting default volume path No reason to do it in util/ anymore. It's always going to be a subdirectory of c/storage graph root by default, so we can just set it after the return. Signed-off-by: Matthew Heon --- libpod/runtime.go | 4 ++-- pkg/util/utils.go | 28 +++++----------------------- 2 files changed, 7 insertions(+), 25 deletions(-) diff --git a/libpod/runtime.go b/libpod/runtime.go index 635783790..f53cdd8b8 100644 --- a/libpod/runtime.go +++ b/libpod/runtime.go @@ -331,13 +331,13 @@ func NewRuntime(options ...RuntimeOption) (runtime *Runtime, err error) { deepcopier.Copy(defaultRuntimeConfig).To(runtime.config) runtime.config.TmpDir = tmpDir - storageConf, volumePath, err := util.GetDefaultStoreOptions() + storageConf, err := util.GetDefaultStoreOptions() if err != nil { return nil, errors.Wrapf(err, "error retrieving rootless storage config") } runtime.config.StorageConfig = storageConf runtime.config.StaticDir = filepath.Join(storageConf.GraphRoot, "libpod") - runtime.config.VolumePath = volumePath + runtime.config.VolumePath = filepath.Join(storageConf.GraphRoot, "volumes") configPath := ConfigPath foundConfig := true diff --git a/pkg/util/utils.go b/pkg/util/utils.go index 2468d525a..a4576191b 100644 --- a/pkg/util/utils.go +++ b/pkg/util/utils.go @@ -259,15 +259,6 @@ func GetRootlessStorageOpts() (storage.StoreOptions, error) { return opts, nil } -// GetRootlessVolumePath returns where all the name volumes will be created in rootless mode -func GetRootlessVolumePath() (string, error) { - dataDir, _, err := GetRootlessDirInfo() - if err != nil { - return "", err - } - return filepath.Join(dataDir, "containers", "storage", "volumes"), nil -} - type tomlOptionsConfig struct { MountProgram string `toml:"mount_program"` } @@ -297,25 +288,18 @@ func getTomlStorage(storeOptions *storage.StoreOptions) *tomlConfig { return config } -// GetDefaultStoreOptions returns the storage ops for containers and the volume path -// for the volume API -// It also returns the path where all named volumes will be created using the volume API -func GetDefaultStoreOptions() (storage.StoreOptions, string, error) { +// GetDefaultStoreOptions returns the default storage ops for containers +func GetDefaultStoreOptions() (storage.StoreOptions, error) { var ( defaultRootlessRunRoot string defaultRootlessGraphRoot string err error ) storageOpts := storage.DefaultStoreOptions - volumePath := filepath.Join(storageOpts.GraphRoot, "volumes") if rootless.IsRootless() { storageOpts, err = GetRootlessStorageOpts() if err != nil { - return storageOpts, volumePath, err - } - volumePath, err = GetRootlessVolumePath() - if err != nil { - return storageOpts, volumePath, err + return storageOpts, err } } @@ -325,7 +309,6 @@ func GetDefaultStoreOptions() (storage.StoreOptions, string, error) { defaultRootlessGraphRoot = storageOpts.GraphRoot storageOpts = storage.StoreOptions{} storage.ReloadConfigurationFile(storageConf, &storageOpts) - volumePath = filepath.Join(storageOpts.GraphRoot, "volumes") } if rootless.IsRootless() { @@ -333,7 +316,7 @@ func GetDefaultStoreOptions() (storage.StoreOptions, string, error) { os.MkdirAll(filepath.Dir(storageConf), 0755) file, err := os.OpenFile(storageConf, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0666) if err != nil { - return storageOpts, volumePath, errors.Wrapf(err, "cannot open %s", storageConf) + return storageOpts, errors.Wrapf(err, "cannot open %s", storageConf) } tomlConfiguration := getTomlStorage(&storageOpts) @@ -352,10 +335,9 @@ func GetDefaultStoreOptions() (storage.StoreOptions, string, error) { if storageOpts.GraphRoot == "" { storageOpts.GraphRoot = defaultRootlessGraphRoot } - volumePath = filepath.Join(storageOpts.GraphRoot, "volumes") } } - return storageOpts, volumePath, nil + return storageOpts, nil } // StorageConfigFile returns the path to the storage config file used -- cgit v1.2.3-54-g00ecf