From 17c2812ac4de130f6adb1b30bcbe063eb2c9ef4d Mon Sep 17 00:00:00 2001 From: Daniel J Walsh Date: Thu, 5 Dec 2019 08:07:03 -0500 Subject: [PATCH] Cleanup handling of default driver name If the user specifies drivername="" in the storage.conf then we default to the first driver name based on priority list, usually overla. The problem with this, is now the storage.conf file has options that are driver specific. The code does not catch this so ends up looking for a driver name of "", and finds no matching options. This code will set the drivername that the storage layer would choose earlies, so that storage will get the correct options out of the config file. Signed-off-by: Daniel J Walsh --- drivers/aufs/aufs.go | 14 ++++----- drivers/driver.go | 69 +++++++++++++++++++++++++++++--------------- store.go | 24 +++++++++++---- tests/helpers.bash | 5 ++++ 4 files changed, 76 insertions(+), 36 deletions(-) diff --git a/drivers/aufs/aufs.go b/drivers/aufs/aufs.go index 4430670a20..74c8ec1f9f 100644 --- a/drivers/aufs/aufs.go +++ b/drivers/aufs/aufs.go @@ -93,7 +93,7 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) fsMagic, err := graphdriver.GetFSMagic(home) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "aufs: failed to getfsmagic %s", home) } if fsName, ok := graphdriver.FsNames[fsMagic]; ok { backingFs = fsName @@ -109,7 +109,7 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) for _, option := range options.DriverOptions { key, val, err := parsers.ParseKeyValueOpt(option) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "aufs: ParseKeyValueOpt %s", option) } key = strings.ToLower(key) switch key { @@ -137,7 +137,7 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) rootUID, rootGID, err := idtools.GetRootUIDGID(options.UIDMaps, options.GIDMaps) if err != nil { - return nil, err + return nil, errors.Wrapf(err, "aufs: failed to GetRootUIDGID") } // Create the root aufs driver dir and return // if it already exists @@ -146,17 +146,17 @@ func Init(home string, options graphdriver.Options) (graphdriver.Driver, error) if os.IsExist(err) { return a, nil } - return nil, err + return nil, errors.Wrapf(err, "aufs: failed to mkdirallas: %s", home) } if err := mountpk.MakePrivate(home); err != nil { - return nil, err + return nil, errors.Wrapf(err, "aufs: failed to MakePrivate: %s", home) } // Populate the dir structure for _, p := range paths { if err := idtools.MkdirAllAs(path.Join(home, p), 0700, rootUID, rootGID); err != nil { - return nil, err + return nil, errors.Wrapf(err, "aufs: failed to MakePrivate: %s", path.Join(home, p)) } } logger := logrus.WithFields(logrus.Fields{ @@ -326,7 +326,7 @@ func (a *Driver) createDirsFor(id, parent string) error { rootPair.GID = int(st.GID()) } if err := idtools.MkdirAllAndChownNew(path.Join(a.rootPath(), p, id), os.FileMode(0755), rootPair); err != nil { - return err + return errors.Wrapf(err, "aufs: failed to makedir %s", path.Join(a.rootPath(), p)) } } return nil diff --git a/drivers/driver.go b/drivers/driver.go index 8d6b2a5dcc..5e072687d9 100644 --- a/drivers/driver.go +++ b/drivers/driver.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" "strings" + "sync" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -24,6 +25,9 @@ const ( ) var ( + once sync.Once + defaultDriverName string + defaultDriverNameErr error // All registered drivers drivers map[string]InitFunc @@ -209,6 +213,46 @@ func Register(name string, initFunc InitFunc) error { return nil } +// GetDefaultDriverName returns the driver name +func GetDefaultDriverName(config Options) (string, error) { + once.Do(func() { + driver, err := getDefaultDriver(config) + if err != nil { + defaultDriverNameErr = err + } else { + defaultDriverName = driver.String() + } + }) + return defaultDriverName, defaultDriverNameErr +} + +// GetDefaultDriver initializes and returns the registered driver +func getDefaultDriver(config Options) (Driver, error) { + // Check for priority drivers first + for _, name := range priority { + driver, err := getBuiltinDriver(name, config.Root, config) + if err != nil { + if isDriverNotSupported(err) { + continue + } + return nil, err + } + return driver, nil + } + // Check all registered drivers if no priority driver is found + for name, initFunc := range drivers { + driver, err := initFunc(filepath.Join(config.Root, name), config) + if err != nil { + if isDriverNotSupported(err) { + continue + } + return nil, err + } + return driver, nil + } + return nil, fmt.Errorf("No supported storage backend found") +} + // GetDriver initializes and returns the registered driver func GetDriver(name string, config Options) (Driver, error) { if initFunc, exists := drivers[name]; exists { @@ -281,30 +325,7 @@ func New(name string, config Options) (Driver, error) { } } - // Check for priority drivers first - for _, name := range priority { - driver, err := getBuiltinDriver(name, config.Root, config) - if err != nil { - if isDriverNotSupported(err) { - continue - } - return nil, err - } - return driver, nil - } - - // Check all registered drivers if no priority driver is found - for name, initFunc := range drivers { - driver, err := initFunc(filepath.Join(config.Root, name), config) - if err != nil { - if isDriverNotSupported(err) { - continue - } - return nil, err - } - return driver, nil - } - return nil, fmt.Errorf("No supported storage backend found") + return getDefaultDriver(config) } // isDriverNotSupported returns true if the error initializing diff --git a/store.go b/store.go index 65808b8a0b..a51fa85926 100644 --- a/store.go +++ b/store.go @@ -3278,7 +3278,7 @@ func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) { data, err := ioutil.ReadFile(configFile) if err != nil { if !os.IsNotExist(err) { - fmt.Printf("Failed to read %s %v\n", configFile, err.Error()) + fmt.Fprintf(os.Stderr, "Failed to read %s %v\n", configFile, err.Error()) return } } @@ -3286,7 +3286,7 @@ func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) { config := new(tomlConfig) if _, err := toml.Decode(string(data), config); err != nil { - fmt.Printf("Failed to parse %s %v\n", configFile, err.Error()) + fmt.Fprintf(os.Stderr, "Failed to parse %s %v\n", configFile, err.Error()) return } if config.Storage.Driver != "" { @@ -3322,7 +3322,7 @@ func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) { if config.Storage.Options.RemapUser != "" && config.Storage.Options.RemapGroup != "" { mappings, err := idtools.NewIDMappings(config.Storage.Options.RemapUser, config.Storage.Options.RemapGroup) if err != nil { - fmt.Printf("Error initializing ID mappings for %s:%s %v\n", config.Storage.Options.RemapUser, config.Storage.Options.RemapGroup, err) + fmt.Fprintf(os.Stderr, "Error initializing ID mappings for %s:%s %v\n", config.Storage.Options.RemapUser, config.Storage.Options.RemapGroup, err) return } storeOptions.UIDMap = mappings.UIDs() @@ -3331,13 +3331,13 @@ func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) { uidmap, err := idtools.ParseIDMap([]string{config.Storage.Options.RemapUIDs}, "remap-uids") if err != nil { - fmt.Print(err) + fmt.Fprint(os.Stderr, err) } else { storeOptions.UIDMap = append(storeOptions.UIDMap, uidmap...) } gidmap, err := idtools.ParseIDMap([]string{config.Storage.Options.RemapGIDs}, "remap-gids") if err != nil { - fmt.Print(err) + fmt.Fprint(os.Stderr, err) } else { storeOptions.GIDMap = append(storeOptions.GIDMap, gidmap...) } @@ -3345,6 +3345,20 @@ func ReloadConfigurationFile(configFile string, storeOptions *StoreOptions) { storeOptions.GraphDriverName = os.Getenv("STORAGE_DRIVER") } + if storeOptions.GraphDriverName == "" { + options := drivers.Options{ + Root: storeOptions.GraphRoot, + RunRoot: storeOptions.RunRoot, + UIDMaps: storeOptions.UIDMap, + GIDMaps: storeOptions.GIDMap, + } + name, err := drivers.GetDefaultDriverName(options) + if err != nil { + fmt.Fprintf(os.Stderr, "Error getting default driver name: %v\n", err) + } else { + storeOptions.GraphDriverName = name + } + } storeOptions.GraphDriverOptions = cfg.GetGraphDriverOptions(storeOptions.GraphDriverName, config.Storage.Options) if os.Getenv("STORAGE_OPTS") != "" { diff --git a/tests/helpers.bash b/tests/helpers.bash index f4cf37e5a6..000279243c 100755 --- a/tests/helpers.bash +++ b/tests/helpers.bash @@ -54,6 +54,7 @@ populate() { lowerlayer="$output" # Mount the layer. run storage --debug=false mount $lowerlayer + echo $output [ "$status" -eq 0 ] [ "$output" != "" ] local lowermount="$output" @@ -81,11 +82,13 @@ populate() { # Create a second layer based on the first. run storage --debug=false create-layer "$lowerlayer" + echo $output [ "$status" -eq 0 ] [ "$output" != "" ] midlayer="$output" # Mount the second layer. run storage --debug=false mount $midlayer + echo $output [ "$status" -eq 0 ] [ "$output" != "" ] local midmount="$output" @@ -133,11 +136,13 @@ populate() { # Create a third layer based on the second. run storage --debug=false create-layer "$midlayer" + echo $output [ "$status" -eq 0 ] [ "$output" != "" ] upperlayer="$output" # Mount the third layer. run storage --debug=false mount $upperlayer + echo $output [ "$status" -eq 0 ] [ "$output" != "" ] local uppermount="$output"