Merge pull request #48402 from ianchakeres/local-storage-teardown-fix

Automatic merge from submit-queue

Local storage teardown fix

**What this PR does / why we need it**: Local storage uses bindmounts and the method IsLikelyNotMountPoint does not detect these as mountpoints. Therefore, local PVs are not properly unmounted when they are deleted.

**Which issue this PR fixes**: fixes #48331

**Special notes for your reviewer**:

You can use these e2e tests to reproduce the issue and validate the fix works appropriately kubernetes/kubernetes#47999

The existing method IsLikelyNotMountPoint purposely does not check mountpoints reliability (https://github.com/kubernetes/kubernetes/blob/4c5b22d4c6b630ff1d76b1d15d74c6597c0aa037/pkg/util/mount/mount_linux.go#L161), since the number of mountpoints can be large. https://github.com/kubernetes/kubernetes/blob/4c5b22d4c6b630ff1d76b1d15d74c6597c0aa037/pkg/util/mount/mount.go#L46

This implementation changes the behavior for local storage to detect mountpoints reliably, and avoids changing the behavior for any other callers to a UnmountPath.

**Release note**:

```
Fixes bind-mount teardown failure with non-mount point Local volumes (issue kubernetes/kubernetes#48331).
```

pkg/volume/util/util.go

@@ -75,23 +75,42 @@ func SetReady(dir string) {
 // UnmountPath is a common unmount routine that unmounts the given path and
 // deletes the remaining directory if successful.
 func UnmountPath(mountPath string, mounter mount.Interface) error {
+	return UnmountMountPoint(mountPath, mounter, false /* extensiveMountPointCheck */)
+}
+
+// UnmountMountPoint is a common unmount routine that unmounts the given path and
+// deletes the remaining directory if successful.
+// if extensiveMountPointCheck is true
+// IsNotMountPoint will be called instead of IsLikelyNotMountPoint.
+// IsNotMountPoint is more expensive but properly handles bind mounts.
+func UnmountMountPoint(mountPath string, mounter mount.Interface, extensiveMountPointCheck bool) error {
 	if pathExists, pathErr := PathExists(mountPath); pathErr != nil {
 		return fmt.Errorf("Error checking if path exists: %v", pathErr)
 	} else if !pathExists {
 		glog.Warningf("Warning: Unmount skipped because path does not exist: %v", mountPath)
 		return nil
 	}
 
-	notMnt, err := mounter.IsLikelyNotMountPoint(mountPath)
+	var notMnt bool
+	var err error
+
+	if extensiveMountPointCheck {
+		notMnt, err = mount.IsNotMountPoint(mounter, mountPath)
+	} else {
+		notMnt, err = mounter.IsLikelyNotMountPoint(mountPath)
+	}
+
 	if err != nil {
 		return err
 	}
+
 	if notMnt {
 		glog.Warningf("Warning: %q is not a mountpoint, deleting", mountPath)
 		return os.Remove(mountPath)
 	}
 
 	// Unmount the mount path
+	glog.V(4).Infof("%q is a mountpoint, unmounting", mountPath)
 	if err := mounter.Unmount(mountPath); err != nil {
 		return err
 	}