From 403fcf5047d6d6afc91f0731455c0ea9f06a1ef9 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 4 Dec 2017 14:45:26 -0800 Subject: [PATCH 1/3] Perform fsmagic detection on driver's home-dir if it exists The fsmagic check was always performed on "data-root" (`/var/lib/docker`), not on the storage-driver's home directory (e.g. `/var/lib/docker/`). This caused detection to be done on the wrong filesystem in situations where `/var/lib/docker/` was a mount, and a different filesystem than `/var/lib/docker` itself. This patch checks if the storage-driver's home directory exists, and only falls back to `/var/lib/docker` if it doesn't exist. Signed-off-by: Sebastiaan van Stijn Upstream-commit: f9c8fa305e1501d8056f8744cb193a720aab0e13 Component: engine --- components/engine/daemon/graphdriver/aufs/aufs.go | 11 ++++++++++- components/engine/daemon/graphdriver/btrfs/btrfs.go | 11 ++++++++++- components/engine/daemon/graphdriver/driver_linux.go | 4 +--- .../engine/daemon/graphdriver/overlay/overlay.go | 12 +++++++++++- .../engine/daemon/graphdriver/overlay2/overlay.go | 11 ++++++++++- 5 files changed, 42 insertions(+), 7 deletions(-) diff --git a/components/engine/daemon/graphdriver/aufs/aufs.go b/components/engine/daemon/graphdriver/aufs/aufs.go index 11e763d025..248b8bf88d 100644 --- a/components/engine/daemon/graphdriver/aufs/aufs.go +++ b/components/engine/daemon/graphdriver/aufs/aufs.go @@ -89,7 +89,16 @@ func Init(root string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, graphdriver.ErrNotSupported } - fsMagic, err := graphdriver.GetFSMagic(root) + // Perform feature detection on /var/lib/docker/aufs if it's an existing directory. + // This covers situations where /var/lib/docker/aufs is a mount, and on a different + // filesystem than /var/lib/docker. + // If the path does not exist, fall back to using /var/lib/docker for feature detection. + testdir := root + if _, err := os.Stat(testdir); os.IsNotExist(err) { + testdir = filepath.Dir(testdir) + } + + fsMagic, err := graphdriver.GetFSMagic(testdir) if err != nil { return nil, err } diff --git a/components/engine/daemon/graphdriver/btrfs/btrfs.go b/components/engine/daemon/graphdriver/btrfs/btrfs.go index 0dabf711dd..57313c94c6 100644 --- a/components/engine/daemon/graphdriver/btrfs/btrfs.go +++ b/components/engine/daemon/graphdriver/btrfs/btrfs.go @@ -51,7 +51,16 @@ type btrfsOptions struct { // An error is returned if BTRFS is not supported. func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (graphdriver.Driver, error) { - fsMagic, err := graphdriver.GetFSMagic(home) + // Perform feature detection on /var/lib/docker/btrfs if it's an existing directory. + // This covers situations where /var/lib/docker/btrfs is a mount, and on a different + // filesystem than /var/lib/docker. + // If the path does not exist, fall back to using /var/lib/docker for feature detection. + testdir := home + if _, err := os.Stat(testdir); os.IsNotExist(err) { + testdir = filepath.Dir(testdir) + } + + fsMagic, err := graphdriver.GetFSMagic(testdir) if err != nil { return nil, err } diff --git a/components/engine/daemon/graphdriver/driver_linux.go b/components/engine/daemon/graphdriver/driver_linux.go index aa3cfc9f79..f59862db85 100644 --- a/components/engine/daemon/graphdriver/driver_linux.go +++ b/components/engine/daemon/graphdriver/driver_linux.go @@ -3,8 +3,6 @@ package graphdriver import ( - "path/filepath" - "github.com/docker/docker/pkg/mount" "golang.org/x/sys/unix" ) @@ -82,7 +80,7 @@ var ( // GetFSMagic returns the filesystem id given the path. func GetFSMagic(rootpath string) (FsMagic, error) { var buf unix.Statfs_t - if err := unix.Statfs(filepath.Dir(rootpath), &buf); err != nil { + if err := unix.Statfs(rootpath, &buf); err != nil { return 0, err } return FsMagic(buf.Type), nil diff --git a/components/engine/daemon/graphdriver/overlay/overlay.go b/components/engine/daemon/graphdriver/overlay/overlay.go index 83a1677fe2..5c21068c0f 100644 --- a/components/engine/daemon/graphdriver/overlay/overlay.go +++ b/components/engine/daemon/graphdriver/overlay/overlay.go @@ -10,6 +10,7 @@ import ( "os" "os/exec" "path" + "path/filepath" "strconv" "github.com/docker/docker/daemon/graphdriver" @@ -119,7 +120,16 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, graphdriver.ErrNotSupported } - fsMagic, err := graphdriver.GetFSMagic(home) + // Perform feature detection on /var/lib/docker/overlay if it's an existing directory. + // This covers situations where /var/lib/docker/overlay is a mount, and on a different + // filesystem than /var/lib/docker. + // If the path does not exist, fall back to using /var/lib/docker for feature detection. + testdir := home + if _, err := os.Stat(testdir); os.IsNotExist(err) { + testdir = filepath.Dir(testdir) + } + + fsMagic, err := graphdriver.GetFSMagic(testdir) if err != nil { return nil, err } diff --git a/components/engine/daemon/graphdriver/overlay2/overlay.go b/components/engine/daemon/graphdriver/overlay2/overlay.go index e660d8058a..1313a89c69 100644 --- a/components/engine/daemon/graphdriver/overlay2/overlay.go +++ b/components/engine/daemon/graphdriver/overlay2/overlay.go @@ -136,7 +136,16 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, err } - fsMagic, err := graphdriver.GetFSMagic(home) + // Perform feature detection on /var/lib/docker/overlay2 if it's an existing directory. + // This covers situations where /var/lib/docker/overlay2 is a mount, and on a different + // filesystem than /var/lib/docker. + // If the path does not exist, fall back to using /var/lib/docker for feature detection. + testdir := home + if _, err := os.Stat(testdir); os.IsNotExist(err) { + testdir = filepath.Dir(testdir) + } + + fsMagic, err := graphdriver.GetFSMagic(testdir) if err != nil { return nil, err } From e4dde678751286b751cc7739e87205e5170efa99 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 16 Nov 2017 01:48:43 +0100 Subject: [PATCH 2/3] Remove support for overlay/overlay2 without d_type Support for running overlay/overlay2 on a backing filesystem without d_type support (most likely: xfs, as ext4 supports this by default), was deprecated for some time. Running without d_type support is problematic, and can lead to difficult to debug issues ("invalid argument" errors, or unable to remove files from the container's filesystem). This patch turns the warning that was previously printed into an "unsupported" error, so that the overlay/overlay2 drivers are not automatically selected when detecting supported storage drivers. Signed-off-by: Sebastiaan van Stijn Upstream-commit: 0abb8dec3f730f3ad2cc9a161c97968a6bfd0631 Component: engine --- .../engine/daemon/graphdriver/driver.go | 18 ++-------- .../engine/daemon/graphdriver/errors.go | 36 +++++++++++++++++++ .../graphdriver/graphtest/graphtest_unix.go | 2 +- .../daemon/graphdriver/overlay/overlay.go | 19 +++++----- .../daemon/graphdriver/overlay2/overlay.go | 21 +++++------ .../graphdriver/overlayutils/overlayutils.go | 8 +++-- 6 files changed, 62 insertions(+), 42 deletions(-) create mode 100644 components/engine/daemon/graphdriver/errors.go diff --git a/components/engine/daemon/graphdriver/driver.go b/components/engine/daemon/graphdriver/driver.go index d08c6dc5b7..721f6609e9 100644 --- a/components/engine/daemon/graphdriver/driver.go +++ b/components/engine/daemon/graphdriver/driver.go @@ -1,7 +1,6 @@ package graphdriver import ( - "errors" "fmt" "io" "os" @@ -28,13 +27,6 @@ const ( var ( // All registered drivers drivers map[string]InitFunc - - // ErrNotSupported returned when driver is not supported. - ErrNotSupported = errors.New("driver not supported") - // ErrPrerequisites returned when driver does not meet prerequisites. - ErrPrerequisites = errors.New("prerequisites for driver not satisfied (wrong filesystem?)") - // ErrIncompatibleFS returned when file system is not supported. - ErrIncompatibleFS = fmt.Errorf("backing file system is unsupported for this graph driver") ) //CreateOpts contains optional arguments for Create() and CreateReadWrite() @@ -248,7 +240,7 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err for _, name := range list { driver, err := getBuiltinDriver(name, config.Root, config.DriverOptions, config.UIDMaps, config.GIDMaps) if err != nil { - if isDriverNotSupported(err) { + if IsDriverNotSupported(err) { continue } return nil, err @@ -260,7 +252,7 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err for name, initFunc := range drivers { driver, err := initFunc(filepath.Join(config.Root, name), config.DriverOptions, config.UIDMaps, config.GIDMaps) if err != nil { - if isDriverNotSupported(err) { + if IsDriverNotSupported(err) { continue } return nil, err @@ -270,12 +262,6 @@ func New(name string, pg plugingetter.PluginGetter, config Options) (Driver, err return nil, fmt.Errorf("No supported storage backend found") } -// isDriverNotSupported returns true if the error initializing -// the graph driver is a non-supported error. -func isDriverNotSupported(err error) bool { - return err == ErrNotSupported || err == ErrPrerequisites || err == ErrIncompatibleFS -} - // scanPriorDrivers returns an un-ordered scan of directories of prior storage drivers func scanPriorDrivers(root string) map[string]bool { driversMap := make(map[string]bool) diff --git a/components/engine/daemon/graphdriver/errors.go b/components/engine/daemon/graphdriver/errors.go new file mode 100644 index 0000000000..dd52ee4ada --- /dev/null +++ b/components/engine/daemon/graphdriver/errors.go @@ -0,0 +1,36 @@ +package graphdriver + +const ( + // ErrNotSupported returned when driver is not supported. + ErrNotSupported NotSupportedError = "driver not supported" + // ErrPrerequisites returned when driver does not meet prerequisites. + ErrPrerequisites NotSupportedError = "prerequisites for driver not satisfied (wrong filesystem?)" + // ErrIncompatibleFS returned when file system is not supported. + ErrIncompatibleFS NotSupportedError = "backing file system is unsupported for this graph driver" +) + +// ErrUnSupported signals that the graph-driver is not supported on the current configuration +type ErrUnSupported interface { + NotSupported() +} + +// NotSupportedError signals that the graph-driver is not supported on the current configuration +type NotSupportedError string + +func (e NotSupportedError) Error() string { + return string(e) +} + +// NotSupported signals that a graph-driver is not supported. +func (e NotSupportedError) NotSupported() {} + +// IsDriverNotSupported returns true if the error initializing +// the graph driver is a non-supported error. +func IsDriverNotSupported(err error) bool { + switch err.(type) { + case ErrUnSupported: + return true + default: + return false + } +} diff --git a/components/engine/daemon/graphdriver/graphtest/graphtest_unix.go b/components/engine/daemon/graphdriver/graphtest/graphtest_unix.go index c25d4826fc..da9443e098 100644 --- a/components/engine/daemon/graphdriver/graphtest/graphtest_unix.go +++ b/components/engine/daemon/graphdriver/graphtest/graphtest_unix.go @@ -42,7 +42,7 @@ func newDriver(t testing.TB, name string, options []string) *Driver { d, err := graphdriver.GetDriver(name, nil, graphdriver.Options{DriverOptions: options, Root: root}) if err != nil { t.Logf("graphdriver: %v\n", err) - if err == graphdriver.ErrNotSupported || err == graphdriver.ErrPrerequisites || err == graphdriver.ErrIncompatibleFS { + if graphdriver.IsDriverNotSupported(err) { t.Skipf("Driver %s not supported", name) } t.Fatal(err) diff --git a/components/engine/daemon/graphdriver/overlay/overlay.go b/components/engine/daemon/graphdriver/overlay/overlay.go index 5c21068c0f..b148431863 100644 --- a/components/engine/daemon/graphdriver/overlay/overlay.go +++ b/components/engine/daemon/graphdriver/overlay/overlay.go @@ -138,11 +138,19 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap } switch fsMagic { - case graphdriver.FsMagicAufs, graphdriver.FsMagicBtrfs, graphdriver.FsMagicOverlay, graphdriver.FsMagicZfs, graphdriver.FsMagicEcryptfs, graphdriver.FsMagicNfsFs: + case graphdriver.FsMagicAufs, graphdriver.FsMagicBtrfs, graphdriver.FsMagicEcryptfs, graphdriver.FsMagicNfsFs, graphdriver.FsMagicOverlay, graphdriver.FsMagicZfs: logrus.Errorf("'overlay' is not supported over %s", backingFs) return nil, graphdriver.ErrIncompatibleFS } + supportsDType, err := fsutils.SupportsDType(testdir) + if err != nil { + return nil, err + } + if !supportsDType { + return nil, overlayutils.ErrDTypeNotSupported("overlay", backingFs) + } + rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps) if err != nil { return nil, err @@ -156,15 +164,6 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, err } - supportsDType, err := fsutils.SupportsDType(home) - if err != nil { - return nil, err - } - if !supportsDType { - // not a fatal error until v17.12 (#27443) - logrus.Warn(overlayutils.ErrDTypeNotSupported("overlay", backingFs)) - } - d := &Driver{ home: home, uidMaps: uidMaps, diff --git a/components/engine/daemon/graphdriver/overlay2/overlay.go b/components/engine/daemon/graphdriver/overlay2/overlay.go index 1313a89c69..ea707a0af6 100644 --- a/components/engine/daemon/graphdriver/overlay2/overlay.go +++ b/components/engine/daemon/graphdriver/overlay2/overlay.go @@ -153,9 +153,8 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap backingFs = fsName } - // check if they are running over btrfs, aufs, zfs, overlay, or ecryptfs switch fsMagic { - case graphdriver.FsMagicAufs, graphdriver.FsMagicZfs, graphdriver.FsMagicOverlay, graphdriver.FsMagicEcryptfs, graphdriver.FsMagicNfsFs: + case graphdriver.FsMagicAufs, graphdriver.FsMagicEcryptfs, graphdriver.FsMagicNfsFs, graphdriver.FsMagicOverlay, graphdriver.FsMagicZfs: logrus.Errorf("'overlay2' is not supported over %s", backingFs) return nil, graphdriver.ErrIncompatibleFS case graphdriver.FsMagicBtrfs: @@ -174,12 +173,19 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap if opts.overrideKernelCheck { logrus.Warn("Using pre-4.0.0 kernel for overlay2, mount failures may require kernel update") } else { - if err := supportsMultipleLowerDir(filepath.Dir(home)); err != nil { + if err := supportsMultipleLowerDir(testdir); err != nil { logrus.Debugf("Multiple lower dirs not supported: %v", err) return nil, graphdriver.ErrNotSupported } } } + supportsDType, err := fsutils.SupportsDType(testdir) + if err != nil { + return nil, err + } + if !supportsDType { + return nil, overlayutils.ErrDTypeNotSupported("overlay2", backingFs) + } rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps) if err != nil { @@ -194,15 +200,6 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, err } - supportsDType, err := fsutils.SupportsDType(home) - if err != nil { - return nil, err - } - if !supportsDType { - // not a fatal error until v17.12 (#27443) - logrus.Warn(overlayutils.ErrDTypeNotSupported("overlay2", backingFs)) - } - d := &Driver{ home: home, uidMaps: uidMaps, diff --git a/components/engine/daemon/graphdriver/overlayutils/overlayutils.go b/components/engine/daemon/graphdriver/overlayutils/overlayutils.go index 7491c3457c..9f71c60d51 100644 --- a/components/engine/daemon/graphdriver/overlayutils/overlayutils.go +++ b/components/engine/daemon/graphdriver/overlayutils/overlayutils.go @@ -3,8 +3,9 @@ package overlayutils import ( - "errors" "fmt" + + "github.com/docker/docker/daemon/graphdriver" ) // ErrDTypeNotSupported denotes that the backing filesystem doesn't support d_type. @@ -13,6 +14,7 @@ func ErrDTypeNotSupported(driver, backingFs string) error { if backingFs == "xfs" { msg += " Reformat the filesystem with ftype=1 to enable d_type support." } - msg += " Running without d_type support will no longer be supported in Docker 17.12." - return errors.New(msg) + msg += " Backing filesystems without d_type support are not supported." + + return graphdriver.NotSupportedError(msg) } From 0c8a47d0194c93b2de4c1be39d0e50e0716114a0 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Mon, 4 Dec 2017 17:02:52 -0800 Subject: [PATCH 3/3] Allow existing setups to continue using d_type Even though it's highly discouraged, there are existing installs that are running overlay/overlay2 on filesystems without d_type support. This patch allows the daemon to start in such cases, instead of refusing to start without an option to override. For fresh installs, backing filesystems without d_type support will still cause the overlay/overlay2 drivers to be marked as "unsupported", and skipped during the automatic selection. This feature is only to keep backward compatibility, but will be removed at some point. Signed-off-by: Sebastiaan van Stijn Upstream-commit: 0a4e793a3da9ba6d20bccfb83f7c48e20a76d895 Component: engine --- components/engine/daemon/graphdriver/driver.go | 12 ++++++++++++ .../engine/daemon/graphdriver/overlay/overlay.go | 6 +++++- .../engine/daemon/graphdriver/overlay2/overlay.go | 6 +++++- 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/components/engine/daemon/graphdriver/driver.go b/components/engine/daemon/graphdriver/driver.go index 721f6609e9..ceb49b9818 100644 --- a/components/engine/daemon/graphdriver/driver.go +++ b/components/engine/daemon/graphdriver/driver.go @@ -277,6 +277,18 @@ func scanPriorDrivers(root string) map[string]bool { return driversMap } +// IsInitialized checks if the driver's home-directory exists and is non-empty. +func IsInitialized(driverHome string) bool { + _, err := os.Stat(driverHome) + if os.IsNotExist(err) { + return false + } + if err != nil { + logrus.Warnf("graphdriver.IsInitialized: stat failed: %v", err) + } + return !isEmptyDir(driverHome) +} + // isEmptyDir checks if a directory is empty. It is used to check if prior // storage-driver directories exist. If an error occurs, it also assumes the // directory is not empty (which preserves the behavior _before_ this check diff --git a/components/engine/daemon/graphdriver/overlay/overlay.go b/components/engine/daemon/graphdriver/overlay/overlay.go index b148431863..fce775ea6b 100644 --- a/components/engine/daemon/graphdriver/overlay/overlay.go +++ b/components/engine/daemon/graphdriver/overlay/overlay.go @@ -148,7 +148,11 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, err } if !supportsDType { - return nil, overlayutils.ErrDTypeNotSupported("overlay", backingFs) + if !graphdriver.IsInitialized(home) { + return nil, overlayutils.ErrDTypeNotSupported("overlay", backingFs) + } + // allow running without d_type only for existing setups (#27443) + logrus.Warn(overlayutils.ErrDTypeNotSupported("overlay", backingFs)) } rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps) diff --git a/components/engine/daemon/graphdriver/overlay2/overlay.go b/components/engine/daemon/graphdriver/overlay2/overlay.go index ea707a0af6..f1731ea935 100644 --- a/components/engine/daemon/graphdriver/overlay2/overlay.go +++ b/components/engine/daemon/graphdriver/overlay2/overlay.go @@ -184,7 +184,11 @@ func Init(home string, options []string, uidMaps, gidMaps []idtools.IDMap) (grap return nil, err } if !supportsDType { - return nil, overlayutils.ErrDTypeNotSupported("overlay2", backingFs) + if !graphdriver.IsInitialized(home) { + return nil, overlayutils.ErrDTypeNotSupported("overlay2", backingFs) + } + // allow running without d_type only for existing setups (#27443) + logrus.Warn(overlayutils.ErrDTypeNotSupported("overlay2", backingFs)) } rootUID, rootGID, err := idtools.GetRootUIDGID(uidMaps, gidMaps)