From 020fd68326303e8e7996cddfe705ae9084ae216b Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 14 Mar 2022 16:42:05 +0100 Subject: [PATCH 1/2] daemon: graphdriver: some minor cleanup - use pkg/errors for errors and fix error-capitalisation - remove one redundant call to logDeprecatedWarning() (we're already skipping deprecated drivers in that loop). - rename `list` to `priorityList` for readability. - remove redundant "skip" for the vfs storage driver, as it's already excluded by `scanPriorDrivers()` - change one debug log to an "info", so that the daemon logs contain the driver that was configured, and include "multiple prior states found" error in the daemon logs, to assist in debugging failed daemon starts. Signed-off-by: Sebastiaan van Stijn --- daemon/graphdriver/driver.go | 39 ++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 20 deletions(-) diff --git a/daemon/graphdriver/driver.go b/daemon/graphdriver/driver.go index f73f66a677..e9d2f6afc1 100644 --- a/daemon/graphdriver/driver.go +++ b/daemon/graphdriver/driver.go @@ -1,19 +1,18 @@ package graphdriver // import "github.com/docker/docker/daemon/graphdriver" import ( - "fmt" "io" "os" "path/filepath" "strings" - "github.com/sirupsen/logrus" - "github.com/vbatts/tar-split/tar/storage" - "github.com/docker/docker/pkg/archive" "github.com/docker/docker/pkg/containerfs" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/plugingetter" + "github.com/pkg/errors" + "github.com/sirupsen/logrus" + "github.com/vbatts/tar-split/tar/storage" ) // FsMagic unsigned id of the filesystem in use. @@ -152,7 +151,7 @@ func init() { // Register registers an InitFunc for the driver. func Register(name string, initFunc InitFunc) error { if _, exists := drivers[name]; exists { - return fmt.Errorf("Name already registered %s", name) + return errors.Errorf("name already registered %s", name) } drivers[name] = initFunc @@ -193,20 +192,16 @@ type Options struct { // New creates the driver and initializes it at the specified root. func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, error) { if name != "" { - logrus.Debugf("[graphdriver] trying provided driver: %s", name) // so the logs show specified driver + logrus.Infof("[graphdriver] trying configured driver: %s", name) logDeprecatedWarning(name) return GetDriver(name, pg, config) } // Guess for prior driver driversMap := scanPriorDrivers(config.Root) - list := strings.Split(priority, ",") - logrus.Debugf("[graphdriver] priority list: %v", list) - for _, name := range list { - if name == "vfs" { - // don't use vfs even if there is state present. - continue - } + priorityList := strings.Split(priority, ",") + logrus.Debugf("[graphdriver] priority list: %v", priorityList) + for _, name := range priorityList { if _, prior := driversMap[name]; prior { // of the state found from prior drivers, check in order of our priority // which we would prefer @@ -222,13 +217,15 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err // abort starting when there are other prior configured drivers // to ensure the user explicitly selects the driver to load - if len(driversMap)-1 > 0 { + if len(driversMap) > 1 { var driversSlice []string for name := range driversMap { driversSlice = append(driversSlice, name) } - return nil, fmt.Errorf("%s contains several valid graphdrivers: %s; Please cleanup or explicitly choose storage driver (-s )", config.Root, strings.Join(driversSlice, ", ")) + err = errors.Errorf("%s contains several valid graphdrivers: %s; cleanup or explicitly choose storage driver (-s )", config.Root, strings.Join(driversSlice, ", ")) + logrus.Errorf("[graphdriver] %v", err) + return nil, err } logrus.Infof("[graphdriver] using prior storage driver: %s", name) @@ -237,8 +234,9 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err } } - // Check for priority drivers first - for _, name := range list { + // If no prior state was found, continue with automatic selection, and pick + // the first supported storage driver (in order of priorityList). + for _, name := range priorityList { driver, err := getBuiltinDriver(name, config.Root, config.DriverOptions, config.IDMap) if err != nil { if IsDriverNotSupported(err) { @@ -264,13 +262,14 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err } return nil, err } - logDeprecatedWarning(name) return driver, nil } - return nil, fmt.Errorf("No supported storage backend found") + + return nil, errors.Errorf("no supported storage driver found") } -// scanPriorDrivers returns an un-ordered scan of directories of prior storage drivers +// scanPriorDrivers returns an un-ordered scan of directories of prior storage +// drivers. The 'vfs' storage driver is not taken into account, and ignored. func scanPriorDrivers(root string) map[string]bool { driversMap := make(map[string]bool) From 3853eb59d16431793a1ac64b3736931b2a078abd Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 11 Mar 2022 17:51:12 +0100 Subject: [PATCH 2/2] daemon: require storage-driver to be set if the driver is deprecated Previously, we only printed a warning if a storage driver was deprecated. The intent was to continue supporting these drivers, to allow users to migrate to a different storage driver. This patch changes the behavior; if the user has no storage driver specified in the daemon configuration (so if we try to detect the previous storage driver based on what's present in /var/lib/docker), we now produce an error, informing the user that the storage driver is deprecated (and to be removed), as well as instructing them to change the daemon configuration to explicitly select the storage driver (to allow them to migrate). This should make the deprecation more visible; this will be disruptive, but it's better to have the failure happening *now* (while the drivers are still there), than for users to discover the storage driver is no longer there (which would require them to *downgrade* the daemon in order to migrate to a different driver). With this change, `docker info` includes a link in the warnings that: / # docker info Client: Context: default Debug Mode: false Server: ... Live Restore Enabled: false WARNING: The overlay storage-driver is deprecated, and will be removed in a future release. Refer to the documentation for more information: https://docs.docker.com/go/storage-driver/ When starting the daemon without a storage driver configured explicitly, but previous state was using a deprecated driver, the error is both logged and printed: ... ERRO[2022-03-25T14:14:06.032014013Z] [graphdriver] prior storage driver overlay is deprecated and will be removed in a future release; update the the daemon configuration and explicitly choose this storage driver to continue using it; visit https://docs.docker.com/go/storage-driver/ for more information ... failed to start daemon: error initializing graphdriver: prior storage driver overlay is deprecated and will be removed in a future release; update the the daemon configuration and explicitly choose this storage driver to continue using it; visit https://docs.docker.com/go/storage-driver/ for more information When starting the daemon and explicitly configuring it with a deprecated storage driver: WARN[2022-03-25T14:15:59.042335412Z] [graphdriver] WARNING: the overlay storage-driver is deprecated and will be removed in a future release; visit https://docs.docker.com/go/storage-driver/ for more information Signed-off-by: Sebastiaan van Stijn --- daemon/graphdriver/driver.go | 25 ++++++++++++++----------- daemon/info.go | 6 +++++- 2 files changed, 19 insertions(+), 12 deletions(-) diff --git a/daemon/graphdriver/driver.go b/daemon/graphdriver/driver.go index e9d2f6afc1..3372b1358e 100644 --- a/daemon/graphdriver/driver.go +++ b/daemon/graphdriver/driver.go @@ -193,7 +193,9 @@ type Options struct { func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, error) { if name != "" { logrus.Infof("[graphdriver] trying configured driver: %s", name) - logDeprecatedWarning(name) + if isDeprecated(name) { + logrus.Warnf("[graphdriver] WARNING: the %s storage-driver is deprecated and will be removed in a future release; visit https://docs.docker.com/go/storage-driver/ for more information", name) + } return GetDriver(name, pg, config) } @@ -214,6 +216,11 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err logrus.Errorf("[graphdriver] prior storage driver %s failed: %s", name, err) return nil, err } + if isDeprecated(name) { + err = errors.Errorf("prior storage driver %s is deprecated and will be removed in a future release; update the the daemon configuration and explicitly choose this storage driver to continue using it; visit https://docs.docker.com/go/storage-driver/ for more information", name) + logrus.Errorf("[graphdriver] %v", err) + return nil, err + } // abort starting when there are other prior configured drivers // to ensure the user explicitly selects the driver to load @@ -229,14 +236,18 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err } logrus.Infof("[graphdriver] using prior storage driver: %s", name) - logDeprecatedWarning(name) return driver, nil } } // If no prior state was found, continue with automatic selection, and pick - // the first supported storage driver (in order of priorityList). + // the first supported, non-deprecated, storage driver (in order of priorityList). for _, name := range priorityList { + if isDeprecated(name) { + // Deprecated storage-drivers are skipped in automatic selection, but + // can be selected through configuration. + continue + } driver, err := getBuiltinDriver(name, config.Root, config.DriverOptions, config.IDMap) if err != nil { if IsDriverNotSupported(err) { @@ -244,7 +255,6 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err } return nil, err } - logDeprecatedWarning(name) return driver, nil } @@ -322,10 +332,3 @@ func isDeprecated(name string) bool { } return false } - -// logDeprecatedWarning logs a warning if the given storage-driver is marked "deprecated" -func logDeprecatedWarning(name string) { - if isDeprecated(name) { - logrus.Warnf("[graphdriver] WARNING: the %s storage-driver is deprecated, and will be removed in a future release", name) - } -} diff --git a/daemon/info.go b/daemon/info.go index 367e670055..93a883cadb 100644 --- a/daemon/info.go +++ b/daemon/info.go @@ -124,9 +124,13 @@ func (daemon *Daemon) SystemVersion() types.Version { } func (daemon *Daemon) fillDriverInfo(v *types.Info) { + const warnMsg = ` +WARNING: The %s storage-driver is deprecated, and will be removed in a future release. + Refer to the documentation for more information: https://docs.docker.com/go/storage-driver/` + switch daemon.graphDriver { case "aufs", "devicemapper", "overlay": - v.Warnings = append(v.Warnings, fmt.Sprintf("WARNING: the %s storage-driver is deprecated, and will be removed in a future release.", daemon.graphDriver)) + v.Warnings = append(v.Warnings, fmt.Sprintf(warnMsg, daemon.graphDriver)) } v.Driver = daemon.graphDriver