Merge pull request #96844 from gnufied/use-force-unmount

Use force umount for nfs volumes

pkg/volume/nfs/nfs.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 	"runtime"
+	"time"
 
 	"k8s.io/klog/v2"
 	"k8s.io/mount-utils"
@@ -61,7 +62,8 @@ var _ volume.PersistentVolumePlugin = &nfsPlugin{}
 var _ volume.RecyclableVolumePlugin = &nfsPlugin{}
 
 const (
-	nfsPluginName = "kubernetes.io/nfs"
+	nfsPluginName  = "kubernetes.io/nfs"
+	unMountTimeout = time.Minute
 )
 
 func (plugin *nfsPlugin) Init(host volume.VolumeHost) error {
@@ -302,6 +304,11 @@ func (c *nfsUnmounter) TearDownAt(dir string) error {
 	// Use extensiveMountPointCheck to consult /proc/mounts. We can't use faster
 	// IsLikelyNotMountPoint (lstat()), since there may be root_squash on the
 	// NFS server and kubelet may not be able to do lstat/stat() there.
+	forceUmounter, ok := c.mounter.(mount.MounterForceUnmounter)
+	if ok {
+		klog.V(4).Infof("Using force unmounter interface")
+		return mount.CleanupMountWithForce(dir, forceUmounter, true /* extensiveMountPointCheck */, unMountTimeout)
+	}
 	return mount.CleanupMountPoint(dir, c.mounter, true /* extensiveMountPointCheck */)
 }
 

staging/src/k8s.io/mount-utils/mount.go

@@ -24,6 +24,7 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"time"
 
 	utilexec "k8s.io/utils/exec"
 )
@@ -78,6 +79,13 @@ type Interface interface {
 // the mount interface.
 var _ Interface = &Mounter{}
 
+type MounterForceUnmounter interface {
+	Interface
+	// UnmountWithForce unmounts given target but will retry unmounting with force option
+	// after given timeout.
+	UnmountWithForce(target string, umountTimeout time.Duration) error
+}
+
 // MountPoint represents a single line in /proc/mounts or /etc/fstab.
 type MountPoint struct { // nolint: golint
 	Device string

staging/src/k8s.io/mount-utils/mount_helper_common.go

@@ -19,6 +19,7 @@ package mount
 import (
 	"fmt"
 	"os"
+	"time"
 
 	"k8s.io/klog/v2"
 )
@@ -29,7 +30,7 @@ import (
 // but properly handles bind mounts within the same fs.
 func CleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool) error {
 	pathExists, pathErr := PathExists(mountPath)
-	if !pathExists {
+	if !pathExists && pathErr == nil {
 		klog.Warningf("Warning: Unmount skipped because path does not exist: %v", mountPath)
 		return nil
 	}
@@ -40,6 +41,41 @@ func CleanupMountPoint(mountPath string, mounter Interface, extensiveMountPointC
 	return doCleanupMountPoint(mountPath, mounter, extensiveMountPointCheck, corruptedMnt)
 }
 
+func CleanupMountWithForce(mountPath string, mounter MounterForceUnmounter, extensiveMountPointCheck bool, umountTimeout time.Duration) error {
+	pathExists, pathErr := PathExists(mountPath)
+	if !pathExists && pathErr == nil {
+		klog.Warningf("Warning: Unmount skipped because path does not exist: %v", mountPath)
+		return nil
+	}
+	corruptedMnt := IsCorruptedMnt(pathErr)
+	if pathErr != nil && !corruptedMnt {
+		return fmt.Errorf("Error checking path: %v", pathErr)
+	}
+	var notMnt bool
+	var err error
+	if !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.
+		if err != nil || notMnt {
+			return err
+		}
+	}
+
+	// Unmount the mount path
+	klog.V(4).Infof("%q is a mountpoint, unmounting", mountPath)
+	if err := mounter.UnmountWithForce(mountPath, umountTimeout); err != nil {
+		return err
+	}
+
+	notMnt, err = removePathIfNotMountPoint(mountPath, mounter, extensiveMountPointCheck)
+	// mountPath is not a mount point we should return whatever error we saw
+	if notMnt {
+		return err
+	}
+	return fmt.Errorf("Failed to unmount path %v", mountPath)
+}
+
 // doCleanupMountPoint unmounts the given path and
 // deletes the remaining directory if successful.
 // if extensiveMountPointCheck is true
@@ -51,20 +87,12 @@ func doCleanupMountPoint(mountPath string, mounter Interface, extensiveMountPoin
 	var notMnt bool
 	var err error
 	if !corruptedMnt {
-		if extensiveMountPointCheck {
-			notMnt, err = IsNotMountPoint(mounter, mountPath)
-		} else {
-			notMnt, err = mounter.IsLikelyNotMountPoint(mountPath)
-		}
-
-		if err != nil {
+		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.
+		if err != nil || notMnt {
 			return err
 		}
-
-		if notMnt {
-			klog.Warningf("Warning: %q is not a mountpoint, deleting", mountPath)
-			return os.Remove(mountPath)
-		}
 	}
 
 	// Unmount the mount path
@@ -73,19 +101,35 @@ func doCleanupMountPoint(mountPath string, mounter Interface, extensiveMountPoin
 		return err
 	}
 
+	notMnt, err = removePathIfNotMountPoint(mountPath, mounter, extensiveMountPointCheck)
+	// mountPath is not a mount point we should return whatever error we saw
+	if notMnt {
+		return err
+	}
+	return fmt.Errorf("Failed to unmount path %v", mountPath)
+}
+
+// removePathIfNotMountPoint verifies if given mountPath is a mount point if not it attempts
+// to remove the directory. Returns true and nil if directory was not a mount point and removed.
+func removePathIfNotMountPoint(mountPath string, mounter Interface, extensiveMountPointCheck bool) (bool, error) {
+	var notMnt bool
+	var err error
+
 	if extensiveMountPointCheck {
 		notMnt, err = IsNotMountPoint(mounter, mountPath)
 	} else {
 		notMnt, err = mounter.IsLikelyNotMountPoint(mountPath)
 	}
+
 	if err != nil {
-		return err
+		return notMnt, err
 	}
+
 	if notMnt {
-		klog.V(4).Infof("%q is unmounted, deleting the directory", mountPath)
-		return os.Remove(mountPath)
+		klog.Warningf("Warning: %q is not a mountpoint, deleting", mountPath)
+		return notMnt, os.Remove(mountPath)
 	}
-	return fmt.Errorf("Failed to unmount path %v", mountPath)
+	return notMnt, nil
 }
 
 // PathExists returns true if the specified path exists.

staging/src/k8s.io/mount-utils/mount_linux.go

@@ -19,6 +19,7 @@ limitations under the License.
 package mount
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"os/exec"
@@ -26,6 +27,7 @@ import (
 	"strconv"
 	"strings"
 	"syscall"
+	"time"
 
 	"k8s.io/klog/v2"
 	utilexec "k8s.io/utils/exec"
@@ -53,6 +55,8 @@ type Mounter struct {
 	withSystemd bool
 }
 
+var _ MounterForceUnmounter = &Mounter{}
+
 // New returns a mount.Interface for the current system.
 // It provides options to override the default mounter behavior.
 // mounterPath allows using an alternative to `/bin/mount` for mounting.
@@ -268,6 +272,20 @@ func (mounter *Mounter) Unmount(target string) error {
 	return nil
 }
 
+// UnmountWithForce unmounts given target but will retry unmounting with force option
+// after given timeout.
+func (mounter *Mounter) UnmountWithForce(target string, umountTimeout time.Duration) error {
+	err := tryUnmount(target, umountTimeout)
+	if err != nil {
+		if err == context.DeadlineExceeded {
+			klog.V(2).Infof("Timed out waiting for unmount of %s, trying with -f", target)
+			err = forceUmount(target)
+		}
+		return err
+	}
+	return nil
+}
+
 // List returns a list of all mounted filesystems.
 func (*Mounter) List() ([]MountPoint, error) {
 	return ListProcMounts(procMountsPath)
@@ -573,3 +591,34 @@ func SearchMountPoints(hostSource, mountInfoPath string) ([]string, error) {
 
 	return refs, nil
 }
+
+// tryUnmount calls plain "umount" and waits for unmountTimeout for it to finish.
+func tryUnmount(path string, unmountTimeout time.Duration) error {
+	klog.V(4).Infof("Unmounting %s", path)
+	ctx, cancel := context.WithTimeout(context.Background(), unmountTimeout)
+	defer cancel()
+
+	cmd := exec.CommandContext(ctx, "umount", path)
+	out, cmderr := cmd.CombinedOutput()
+
+	// CombinedOutput() does not return DeadlineExceeded, make sure it's
+	// propagated on timeout.
+	if ctx.Err() != nil {
+		return ctx.Err()
+	}
+
+	if cmderr != nil {
+		return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", cmderr, path, string(out))
+	}
+	return nil
+}
+
+func forceUmount(path string) error {
+	cmd := exec.Command("umount", "-f", path)
+	out, cmderr := cmd.CombinedOutput()
+
+	if cmderr != nil {
+		return fmt.Errorf("unmount failed: %v\nUnmounting arguments: %s\nOutput: %s", cmderr, path, string(out))
+	}
+	return nil
+}