Merge pull request #38547 from rkouj/make-unmount-operation-idempotent

Automatic merge from submit-queue

Unmount operation should not fail if volume is already unmounted

**What this PR does / why we need it**:
If the volume is already unmounted from the pod, another unmount operation should not fail.

fixes:  https://github.com/kubernetes/kubernetes/issues/37657

pkg/volume/aws_ebs/aws_ebs.go

@@ -33,6 +33,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/mount"
 	kstrings "k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
+	"k8s.io/kubernetes/pkg/volume/util"
 )
 
 // This is the primary entrypoint for volume plugins.
@@ -393,33 +394,7 @@ func (c *awsElasticBlockStoreUnmounter) TearDown() error {
 
 // Unmounts the bind mount
 func (c *awsElasticBlockStoreUnmounter) TearDownAt(dir string) error {
-	notMnt, err := c.mounter.IsLikelyNotMountPoint(dir)
+	return util.UnmountPath(dir, c.mounter)
-	if err != nil {
-		glog.V(2).Info("Error checking if mountpoint ", dir, ": ", err)
-		return err
-	}
-	if notMnt {
-		glog.V(2).Info("Not mountpoint, deleting")
-		return os.Remove(dir)
-	}
-
-	// Unmount the bind-mount inside this pod
-	if err := c.mounter.Unmount(dir); err != nil {
-		glog.V(2).Info("Error unmounting dir ", dir, ": ", err)
-		return err
-	}
-	notMnt, mntErr := c.mounter.IsLikelyNotMountPoint(dir)
-	if mntErr != nil {
-		glog.Errorf("IsLikelyNotMountPoint check failed: %v", mntErr)
-		return err
-	}
-	if notMnt {
-		if err := os.Remove(dir); err != nil {
-			glog.V(2).Info("Error removing mountpoint ", dir, ": ", err)
-			return err
-		}
-	}
-	return nil
 }
 
 type awsElasticBlockStoreDeleter struct {

pkg/volume/azure_file/BUILD

@@ -23,6 +23,7 @@ go_library(
         "//pkg/util/mount:go_default_library",
         "//pkg/util/strings:go_default_library",
         "//pkg/volume:go_default_library",
+        "//pkg/volume/util:go_default_library",
         "//vendor:github.com/golang/glog",
     ],
 )

pkg/volume/azure_file/azure_file.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/kubernetes/pkg/volume"
 
 	"github.com/golang/glog"
+	"k8s.io/kubernetes/pkg/volume/util"
 )
 
 // This is the primary entrypoint for volume plugins.
@@ -240,31 +241,7 @@ func (c *azureFileUnmounter) TearDown() error {
 }
 
 func (c *azureFileUnmounter) TearDownAt(dir string) error {
-	notMnt, err := c.mounter.IsLikelyNotMountPoint(dir)
+	return util.UnmountPath(dir, c.mounter)
-	if err != nil {
-		glog.Errorf("Error checking IsLikelyNotMountPoint: %v", err)
-		return err
-	}
-	if notMnt {
-		return os.Remove(dir)
-	}
-
-	if err := c.mounter.Unmount(dir); err != nil {
-		glog.Errorf("Unmounting failed: %v", err)
-		return err
-	}
-	notMnt, mntErr := c.mounter.IsLikelyNotMountPoint(dir)
-	if mntErr != nil {
-		glog.Errorf("IsLikelyNotMountPoint check failed: %v", mntErr)
-		return mntErr
-	}
-	if notMnt {
-		if err := os.Remove(dir); err != nil {
-			return err
-		}
-	}
-
-	return nil
 }
 
 func getVolumeSource(

pkg/volume/cephfs/BUILD

@@ -22,6 +22,7 @@ go_library(
         "//pkg/util/mount:go_default_library",
         "//pkg/util/strings:go_default_library",
         "//pkg/volume:go_default_library",
+        "//pkg/volume/util:go_default_library",
         "//vendor:github.com/golang/glog",
     ],
 )

pkg/volume/cephfs/cephfs.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/mount"
 	utilstrings "k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
+	"k8s.io/kubernetes/pkg/volume/util"
 )
 
 // This is the primary entrypoint for volume plugins.
@@ -227,7 +228,7 @@ func (cephfsVolume *cephfsMounter) SetUpAt(dir string, fsGroup *int64) error {
 	}
 
 	// cleanup upon failure
-	cephfsVolume.cleanup(dir)
+	util.UnmountPath(dir, cephfsVolume.mounter)
 	// return error
 	return err
 }
@@ -245,7 +246,7 @@ func (cephfsVolume *cephfsUnmounter) TearDown() error {
 
 // TearDownAt unmounts the bind mount
 func (cephfsVolume *cephfsUnmounter) TearDownAt(dir string) error {
-	return cephfsVolume.cleanup(dir)
+	return util.UnmountPath(dir, cephfsVolume.mounter)
 }
 
 // GatePath creates global mount path
@@ -254,31 +255,6 @@ func (cephfsVolume *cephfs) GetPath() string {
 	return cephfsVolume.plugin.host.GetPodVolumeDir(cephfsVolume.podUID, utilstrings.EscapeQualifiedNameForDisk(name), cephfsVolume.volName)
 }
 
-func (cephfsVolume *cephfs) cleanup(dir string) error {
-	noMnt, err := cephfsVolume.mounter.IsLikelyNotMountPoint(dir)
-	if err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("CephFS: Error checking IsLikelyNotMountPoint: %v", err)
-	}
-	if noMnt {
-		return os.RemoveAll(dir)
-	}
-
-	if err := cephfsVolume.mounter.Unmount(dir); err != nil {
-		return fmt.Errorf("CephFS: Unmounting failed: %v", err)
-	}
-	noMnt, mntErr := cephfsVolume.mounter.IsLikelyNotMountPoint(dir)
-	if mntErr != nil {
-		return fmt.Errorf("CephFS: IsMountpoint check failed: %v", mntErr)
-	}
-	if noMnt {
-		if err := os.RemoveAll(dir); err != nil {
-			return fmt.Errorf("CephFS: removeAll %s/%v", dir, err)
-		}
-	}
-
-	return nil
-}
-
 func (cephfsVolume *cephfs) execMount(mountpoint string) error {
 	// cephfs mount option
 	ceph_opt := ""

pkg/volume/flocker/BUILD

@@ -26,6 +26,7 @@ go_library(
         "//pkg/util/rand:go_default_library",
         "//pkg/util/strings:go_default_library",
         "//pkg/volume:go_default_library",
+        "//pkg/volume/util:go_default_library",
         "//vendor:github.com/clusterhq/flocker-go",
         "//vendor:github.com/golang/glog",
     ],

pkg/volume/flocker/flocker.go

@@ -31,6 +31,7 @@ import (
 	"k8s.io/kubernetes/pkg/volume"
 
 	flockerapi "github.com/clusterhq/flocker-go"
+	"k8s.io/kubernetes/pkg/volume/util"
 )
 
 // This is the primary entrypoint for volume plugins.
@@ -421,25 +422,7 @@ func (c *flockerVolumeUnmounter) TearDown() error {
 
 // TearDownAt unmounts the bind mount
 func (c *flockerVolumeUnmounter) TearDownAt(dir string) error {
-	notMnt, err := c.mounter.IsLikelyNotMountPoint(dir)
+	return util.UnmountPath(dir, c.mounter)
-	if err != nil {
-		return err
-	}
-	if notMnt {
-		return os.Remove(dir)
-	}
-	if err := c.mounter.Unmount(dir); err != nil {
-		return err
-	}
-	notMnt, mntErr := c.mounter.IsLikelyNotMountPoint(dir)
-	if mntErr != nil {
-		glog.Errorf("isLikelyNotMountPoint check failed: %v", mntErr)
-		return err
-	}
-	if notMnt {
-		return os.Remove(dir)
-	}
-	return fmt.Errorf("Failed to unmount volume dir")
 }
 
 func (plugin *flockerPlugin) NewDeleter(spec *volume.Spec) (volume.Deleter, error) {

pkg/volume/gce_pd/gce_pd.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
+	"k8s.io/kubernetes/pkg/volume/util"
 )
 
 // This is the primary entrypoint for volume plugins.
@@ -339,25 +340,7 @@ func (c *gcePersistentDiskUnmounter) TearDown() error {
 
 // TearDownAt unmounts the bind mount
 func (c *gcePersistentDiskUnmounter) TearDownAt(dir string) error {
-	notMnt, err := c.mounter.IsLikelyNotMountPoint(dir)
+	return util.UnmountPath(dir, c.mounter)
-	if err != nil {
-		return err
-	}
-	if notMnt {
-		return os.Remove(dir)
-	}
-	if err := c.mounter.Unmount(dir); err != nil {
-		return err
-	}
-	notMnt, mntErr := c.mounter.IsLikelyNotMountPoint(dir)
-	if mntErr != nil {
-		glog.Errorf("IsLikelyNotMountPoint check failed: %v", mntErr)
-		return err
-	}
-	if notMnt {
-		return os.Remove(dir)
-	}
-	return fmt.Errorf("Failed to unmount volume dir")
 }
 
 type gcePersistentDiskDeleter struct {

pkg/volume/glusterfs/glusterfs.go

@@ -257,8 +257,7 @@ func (b *glusterfsMounter) SetUpAt(dir string, fsGroup *int64) error {
 	}
 
 	// Cleanup upon failure.
-	c := &glusterfsUnmounter{b.glusterfs}
+	volutil.UnmountPath(dir, b.mounter)
-	c.cleanup(dir)
 	return err
 }
 
@@ -278,32 +277,7 @@ func (c *glusterfsUnmounter) TearDown() error {
 }
 
 func (c *glusterfsUnmounter) TearDownAt(dir string) error {
-	return c.cleanup(dir)
+	return volutil.UnmountPath(dir, c.mounter)
-}
-
-func (c *glusterfsUnmounter) cleanup(dir string) error {
-	notMnt, err := c.mounter.IsLikelyNotMountPoint(dir)
-	if err != nil {
-		return fmt.Errorf("glusterfs: Error checking IsLikelyNotMountPoint: %v", err)
-	}
-	if notMnt {
-		return os.RemoveAll(dir)
-	}
-
-	if err := c.mounter.Unmount(dir); err != nil {
-		return fmt.Errorf("glusterfs: Unmounting failed: %v", err)
-	}
-	notMnt, mntErr := c.mounter.IsLikelyNotMountPoint(dir)
-	if mntErr != nil {
-		return fmt.Errorf("glusterfs: IsLikelyNotMountPoint check failed: %v", mntErr)
-	}
-	if notMnt {
-		if err := os.RemoveAll(dir); err != nil {
-			return fmt.Errorf("glusterfs: RemoveAll failed: %v", err)
-		}
-	}
-
-	return nil
 }
 
 func (b *glusterfsMounter) setUpAtInternal(dir string) error {

pkg/volume/nfs/BUILD

@@ -23,6 +23,7 @@ go_library(
         "//pkg/util/mount:go_default_library",
         "//pkg/util/strings:go_default_library",
         "//pkg/volume:go_default_library",
+        "//pkg/volume/util:go_default_library",
         "//vendor:github.com/golang/glog",
     ],
 )

pkg/volume/nfs/nfs.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/mount"
 	"k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
+	"k8s.io/kubernetes/pkg/volume/util"
 )
 
 // This is the primary entrypoint for volume plugins.
@@ -270,31 +271,7 @@ func (c *nfsUnmounter) TearDown() error {
 }
 
 func (c *nfsUnmounter) TearDownAt(dir string) error {
-	notMnt, err := c.mounter.IsLikelyNotMountPoint(dir)
+	return util.UnmountPath(dir, c.mounter)
-	if err != nil {
-		glog.Errorf("Error checking IsLikelyNotMountPoint: %v", err)
-		return err
-	}
-	if notMnt {
-		return os.Remove(dir)
-	}
-
-	if err := c.mounter.Unmount(dir); err != nil {
-		glog.Errorf("Unmounting failed: %v", err)
-		return err
-	}
-	notMnt, mntErr := c.mounter.IsLikelyNotMountPoint(dir)
-	if mntErr != nil {
-		glog.Errorf("IsLikelyNotMountPoint check failed: %v", mntErr)
-		return mntErr
-	}
-	if notMnt {
-		if err := os.Remove(dir); err != nil {
-			return err
-		}
-	}
-
-	return nil
 }
 
 func newRecycler(pvName string, spec *volume.Spec, eventRecorder volume.RecycleEventRecorder, host volume.VolumeHost, volumeConfig volume.VolumeConfig) (volume.Recycler, error) {

pkg/volume/photon_pd/photon_pd.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/mount"
 	utilstrings "k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
+	"k8s.io/kubernetes/pkg/volume/util"
 )
 
 // This is the primary entrypoint for volume plugins.
@@ -264,29 +265,7 @@ func (c *photonPersistentDiskUnmounter) TearDown() error {
 // Unmounts the bind mount, and detaches the disk only if the PD
 // resource was the last reference to that disk on the kubelet.
 func (c *photonPersistentDiskUnmounter) TearDownAt(dir string) error {
-	glog.V(4).Infof("Photon Controller Volume TearDown of %s", dir)
+	return util.UnmountPath(dir, c.mounter)
-	notmnt, err := c.mounter.IsLikelyNotMountPoint(dir)
-	if err != nil {
-		return err
-	}
-	if notmnt {
-		return os.Remove(dir)
-	}
-
-	if err := c.mounter.Unmount(dir); err != nil {
-		glog.Errorf("Unmount failed: %v", err)
-		return err
-	}
-
-	notmnt, mntErr := c.mounter.IsLikelyNotMountPoint(dir)
-	if mntErr != nil {
-		glog.Errorf("IsLikelyNotMountPoint check failed: %v", mntErr)
-		return err
-	}
-	if notmnt {
-		return os.Remove(dir)
-	}
-	return fmt.Errorf("Failed to unmount volume dir")
 }
 
 func makeGlobalPDPath(host volume.VolumeHost, devName string) string {

pkg/volume/util/util.go

@@ -98,7 +98,7 @@ func UnmountPath(mountPath string, mounter mount.Interface) error {
 		glog.V(4).Infof("%q is unmounted, deleting the directory", mountPath)
 		return os.Remove(mountPath)
 	}
-	return nil
+	return fmt.Errorf("Failed to unmount path %v", mountPath)
 }
 
 // PathExists returns true if the specified path exists.

pkg/volume/vsphere_volume/vsphere_volume.go

@@ -30,6 +30,7 @@ import (
 	"k8s.io/kubernetes/pkg/util/mount"
 	utilstrings "k8s.io/kubernetes/pkg/util/strings"
 	"k8s.io/kubernetes/pkg/volume"
+	"k8s.io/kubernetes/pkg/volume/util"
 )
 
 // This is the primary entrypoint for volume plugins.
@@ -262,26 +263,7 @@ func (v *vsphereVolumeUnmounter) TearDown() error {
 // Unmounts the bind mount, and detaches the disk only if the PD
 // resource was the last reference to that disk on the kubelet.
 func (v *vsphereVolumeUnmounter) TearDownAt(dir string) error {
-	glog.V(5).Infof("vSphere Volume TearDown of %s", dir)
+	return util.UnmountPath(dir, v.mounter)
-	notMnt, err := v.mounter.IsLikelyNotMountPoint(dir)
-	if err != nil {
-		return err
-	}
-	if notMnt {
-		return os.Remove(dir)
-	}
-	if err := v.mounter.Unmount(dir); err != nil {
-		return err
-	}
-	notMnt, mntErr := v.mounter.IsLikelyNotMountPoint(dir)
-	if mntErr != nil {
-		glog.Errorf("IsLikelyNotMountPoint check failed: %v", mntErr)
-		return err
-	}
-	if notMnt {
-		return os.Remove(dir)
-	}
-	return fmt.Errorf("Failed to unmount volume dir")
 }
 
 func makeGlobalPDPath(host volume.VolumeHost, devName string) string {