Skip mount point checks when possible during mount cleanup.

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) {

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.

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
+}

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,6 +49,8 @@ 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
@@ -56,6 +59,7 @@ const (
 type Mounter struct {
 	mounterPath                string
 	withSystemd                bool
+	withSafeNotMountedBehavior bool
 }
 
 var _ MounterForceUnmounter = &Mounter{}
@@ -67,6 +71,7 @@ func New(mounterPath string) Interface {
 	return &Mounter{
 		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).

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

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)