diff --git a/staging/src/k8s.io/mount-utils/fake_mounter.go b/staging/src/k8s.io/mount-utils/fake_mounter.go index 55ea5e2986b..d367a70c2e1 100644 --- a/staging/src/k8s.io/mount-utils/fake_mounter.go +++ b/staging/src/k8s.io/mount-utils/fake_mounter.go @@ -212,6 +212,11 @@ func (f *FakeMounter) IsLikelyNotMountPoint(file string) (bool, error) { return true, nil } +// CanSafelySkipMountPointCheck always returns false for FakeMounter. +func (f *FakeMounter) CanSafelySkipMountPointCheck() bool { + return false +} + // GetMountRefs finds all mount references to the path, returns a // list of paths. func (f *FakeMounter) GetMountRefs(pathname string) ([]string, error) { diff --git a/staging/src/k8s.io/mount-utils/mount.go b/staging/src/k8s.io/mount-utils/mount.go index a882fcc7399..a6c20155a3d 100644 --- a/staging/src/k8s.io/mount-utils/mount.go +++ b/staging/src/k8s.io/mount-utils/mount.go @@ -66,6 +66,10 @@ type Interface interface { // care about such situations, this is a faster alternative to calling List() // and scanning that output. IsLikelyNotMountPoint(file string) (bool, error) + // CanSafelySkipMountPointCheck indicates whether this mounter returns errors on + // operations for targets that are not mount points. If this returns true, no such + // errors will be returned. + CanSafelySkipMountPointCheck() bool // GetMountRefs finds all mount references to pathname, returning a slice of // paths. Pathname can be a mountpoint path or a normal directory // (for bind mount). On Linux, pathname is excluded from the slice. diff --git a/staging/src/k8s.io/mount-utils/mount_helper_common.go b/staging/src/k8s.io/mount-utils/mount_helper_common.go index 196ae30ad1a..0183f6e71c7 100644 --- a/staging/src/k8s.io/mount-utils/mount_helper_common.go +++ b/staging/src/k8s.io/mount-utils/mount_helper_common.go @@ -53,7 +53,7 @@ func CleanupMountWithForce(mountPath string, mounter MounterForceUnmounter, exte } var notMnt bool var err error - if !corruptedMnt { + if !mounter.CanSafelySkipMountPointCheck() && !corruptedMnt { notMnt, err = removePathIfNotMountPoint(mountPath, mounter, extensiveMountPointCheck) // if mountPath was not a mount point - we would have attempted to remove mountPath // and hence return errors if any. @@ -68,6 +68,10 @@ func CleanupMountWithForce(mountPath string, mounter MounterForceUnmounter, exte return err } + if mounter.CanSafelySkipMountPointCheck() { + return removePath(mountPath) + } + notMnt, err = removePathIfNotMountPoint(mountPath, mounter, extensiveMountPointCheck) // mountPath is not a mount point we should return whatever error we saw if notMnt { @@ -82,11 +86,11 @@ func CleanupMountWithForce(mountPath string, mounter MounterForceUnmounter, exte // IsNotMountPoint will be called instead of IsLikelyNotMountPoint. // IsNotMountPoint is more expensive but properly handles bind mounts within the same fs. // if corruptedMnt is true, it means that the mountPath is a corrupted mountpoint, and the mount point check -// will be skipped +// will be skipped. The mount point check will also be skipped if the mounter supports it. func doCleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool, corruptedMnt bool) error { var notMnt bool var err error - if !corruptedMnt { + if !mounter.CanSafelySkipMountPointCheck() && !corruptedMnt { notMnt, err = removePathIfNotMountPoint(mountPath, mounter, extensiveMountPointCheck) // if mountPath was not a mount point - we would have attempted to remove mountPath // and hence return errors if any. @@ -101,6 +105,10 @@ func doCleanupMountPoint(mountPath string, mounter Interface, extensiveMountPoin return err } + if mounter.CanSafelySkipMountPointCheck() { + return removePath(mountPath) + } + notMnt, err = removePathIfNotMountPoint(mountPath, mounter, extensiveMountPointCheck) // mountPath is not a mount point we should return whatever error we saw if notMnt { @@ -135,3 +143,14 @@ func removePathIfNotMountPoint(mountPath string, mounter Interface, extensiveMou } return notMnt, nil } + +// removePath attempts to remove the directory. Returns nil if the directory was removed or does not exist. +func removePath(mountPath string) error { + klog.Warningf("Warning: deleting mount path %q", mountPath) + err := os.Remove(mountPath) + if os.IsNotExist(err) { + klog.V(4).Infof("%q does not exist", mountPath) + return nil + } + return err +} diff --git a/staging/src/k8s.io/mount-utils/mount_linux.go b/staging/src/k8s.io/mount-utils/mount_linux.go index aaa592161d4..a0def77afd7 100644 --- a/staging/src/k8s.io/mount-utils/mount_linux.go +++ b/staging/src/k8s.io/mount-utils/mount_linux.go @@ -22,6 +22,7 @@ package mount import ( "context" "fmt" + "io/ioutil" "os" "os/exec" "path/filepath" @@ -48,14 +49,17 @@ const ( fsckErrorsUncorrected = 4 // Error thrown by exec cmd.Run() when process spawned by cmd.Start() completes before cmd.Wait() is called (see - k/k issue #103753) errNoChildProcesses = "wait: no child processes" + // Error returned by some `umount` implementations when the specified path is not a mount point + errNotMounted = "not mounted" ) // Mounter provides the default implementation of mount.Interface // for the linux platform. This implementation assumes that the // kubelet is running in the host's root mount namespace. type Mounter struct { - mounterPath string - withSystemd bool + mounterPath string + withSystemd bool + withSafeNotMountedBehavior bool } var _ MounterForceUnmounter = &Mounter{} @@ -65,8 +69,9 @@ var _ MounterForceUnmounter = &Mounter{} // mounterPath allows using an alternative to `/bin/mount` for mounting. func New(mounterPath string) Interface { return &Mounter{ - mounterPath: mounterPath, - withSystemd: detectSystemd(), + mounterPath: mounterPath, + withSystemd: detectSystemd(), + withSafeNotMountedBehavior: detectSafeNotMountedBehavior(), } } @@ -223,6 +228,37 @@ func detectSystemd() bool { return true } +// detectSafeNotMountedBehavior returns true if the umount implementation replies "not mounted" +// when the specified path is not mounted. When not sure (permission errors, ...), it returns false. +// When possible, we will trust umount's message and avoid doing our own mount point checks. +// More info: https://github.com/util-linux/util-linux/blob/v2.2/mount/umount.c#L179 +func detectSafeNotMountedBehavior() bool { + if _, err := exec.LookPath("umount"); err != nil { + klog.V(2).Infof("Failed to locate umount executable to detect safe 'not mounted' behavior") + return false + } + // create a temp dir and try to umount it + path, err := ioutil.TempDir("", "kubelet-detect-safe-umount") + if err != nil { + klog.V(2).Infof("Cannot create temp dir to detect safe 'not mounted' behavior: %v", err) + return false + } + defer os.RemoveAll(path) + cmd := exec.Command("umount", path) + output, err := cmd.CombinedOutput() + if err != nil { + klog.V(2).Infof("Cannot run umount to detect safe 'not mounted' behavior: %v", err) + klog.V(4).Infof("'umount %s' output: %s, failed with: %v", path, string(output), err) + return false + } + if !strings.Contains(string(output), errNotMounted) { + klog.V(2).Infof("Detected umount with unsafe 'not mounted' behavior") + return false + } + klog.V(2).Infof("Detected umount with safe 'not mounted' behavior") + return true +} + // MakeMountArgs makes the arguments to the mount(8) command. // options MUST not contain sensitive material (like passwords). func MakeMountArgs(source, target, fstype string, options []string) (mountArgs []string) { @@ -303,6 +339,9 @@ func (mounter *Mounter) Unmount(target string) error { // Rewrite err with the actual exit error of the process. err = &exec.ExitError{ProcessState: command.ProcessState} } + if mounter.withSafeNotMountedBehavior && strings.Contains(string(output), errNotMounted) { + return nil + } return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", err, target, string(output)) } return nil @@ -351,6 +390,11 @@ func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) { return true, nil } +// CanSafelySkipMountPointCheck relies on the detected behavior of umount when given a target that is not a mount point. +func (mounter *Mounter) CanSafelySkipMountPointCheck() bool { + return mounter.withSafeNotMountedBehavior +} + // GetMountRefs finds all mount references to pathname, returns a // list of paths. Path could be a mountpoint or a normal // directory (for bind mount). diff --git a/staging/src/k8s.io/mount-utils/mount_unsupported.go b/staging/src/k8s.io/mount-utils/mount_unsupported.go index 93ba9b2e3f9..817d9e3e76e 100644 --- a/staging/src/k8s.io/mount-utils/mount_unsupported.go +++ b/staging/src/k8s.io/mount-utils/mount_unsupported.go @@ -74,6 +74,11 @@ func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) { return true, errUnsupported } +// CanSafelySkipMountPointCheck always returns false on unsupported platforms +func (mounter *Mounter) CanSafelySkipMountPointCheck() bool { + return false +} + // GetMountRefs always returns an error on unsupported platforms func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { return nil, errUnsupported diff --git a/staging/src/k8s.io/mount-utils/mount_windows.go b/staging/src/k8s.io/mount-utils/mount_windows.go index 3286a69c46b..d35fdc85afa 100644 --- a/staging/src/k8s.io/mount-utils/mount_windows.go +++ b/staging/src/k8s.io/mount-utils/mount_windows.go @@ -244,6 +244,11 @@ func (mounter *Mounter) IsLikelyNotMountPoint(file string) (bool, error) { return true, nil } +// CanSafelySkipMountPointCheck always returns false on Windows +func (mounter *Mounter) CanSafelySkipMountPointCheck() bool { + return false +} + // GetMountRefs : empty implementation here since there is no place to query all mount points on Windows func (mounter *Mounter) GetMountRefs(pathname string) ([]string, error) { windowsPath := NormalizeWindowsPath(pathname)