Merge pull request #52687 from mtanino/teardown-refactor

Automatic merge from submit-queue (batch tested with PRs 51902, 52718, 52687, 52137, 52697). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>..

Refactoring and improvements for iSCSI and FC storage plugins

**What this PR does / why we need it**:

This PR makes following changes.

- Simplify volume tearDown path for iSCSI and FC using
  util.UnmountPath().
- Log lastErr during iscsi connection
  If iscsid fails to connect second portal, currently
  the error is ignored silently. The lastErr should be
  logged to find the root cause of problem.
- Remove iscsi plugin directory after iscsi connection
  is successfully closed.

**Which issue this PR fixes** : No related issues.

**Special notes for your reviewer**:

/cc @rootfs @jsafrane 

**Release note**:

```
NONE
```

pkg/volume/fc/disk_manager.go

@@ -89,33 +89,3 @@ func diskSetUp(manager diskManager, b fcDiskMounter, volPath string, mounter mou
 
 	return nil
 }
-
-// utility to tear down a disk based filesystem
-func diskTearDown(manager diskManager, c fcDiskUnmounter, volPath string, mounter mount.Interface) error {
-	noMnt, err := mounter.IsLikelyNotMountPoint(volPath)
-	if err != nil {
-		glog.Errorf("cannot validate mountpoint %s", volPath)
-		return err
-	}
-	if noMnt {
-		return os.Remove(volPath)
-	}
-
-	if err := mounter.Unmount(volPath); err != nil {
-		glog.Errorf("failed to unmount %s", volPath)
-		return err
-	}
-
-	noMnt, mntErr := mounter.IsLikelyNotMountPoint(volPath)
-	if mntErr != nil {
-		glog.Errorf("isMountpoint check failed: %v", mntErr)
-		return err
-	}
-	if noMnt {
-		if err := os.Remove(volPath); err != nil {
-			return err
-		}
-	}
-	return nil
-
-}

pkg/volume/fc/fc.go

@@ -243,13 +243,7 @@ func (c *fcDiskUnmounter) TearDown() error {
 }
 
 func (c *fcDiskUnmounter) TearDownAt(dir string) error {
-	if pathExists, pathErr := util.PathExists(dir); 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", dir)
-		return nil
-	}
-	return diskTearDown(c.manager, *c, dir, c.mounter)
+	return util.UnmountPath(dir, c.mounter)
 }
 
 func getVolumeSource(spec *volume.Spec) (*v1.FCVolumeSource, bool, error) {

pkg/volume/iscsi/attacher.go

@@ -147,6 +147,11 @@ func (detacher *iscsiDetacher) UnmountDevice(deviceMountPath string) error {
 	if err != nil {
 		return fmt.Errorf("iscsi: failed to detach disk: %s\nError: %v", deviceMountPath, err)
 	}
+	glog.V(4).Infof("iscsi: %q is unmounted, deleting the directory", deviceMountPath)
+	err = os.RemoveAll(deviceMountPath)
+	if err != nil {
+		return fmt.Errorf("iscsi: failed to delete the directory: %s\nError: %v", deviceMountPath, err)
+	}
 	glog.V(4).Infof("iscsi: successfully detached disk: %s", deviceMountPath)
 	return nil
 }

pkg/volume/iscsi/disk_manager.go

@@ -94,36 +94,3 @@ func diskSetUp(manager diskManager, b iscsiDiskMounter, volPath string, mounter
 
 	return nil
 }
-
-// utility to tear down a disk based filesystem
-func diskTearDown(manager diskManager, c iscsiDiskUnmounter, volPath string, mounter mount.Interface) error {
-	notMnt, err := mounter.IsLikelyNotMountPoint(volPath)
-	if err != nil {
-		glog.Errorf("cannot validate mountpoint %s", volPath)
-		return err
-	}
-	if notMnt {
-		return os.Remove(volPath)
-	}
-	_, err = mount.GetMountRefs(mounter, volPath)
-	if err != nil {
-		glog.Errorf("failed to get reference count %s", volPath)
-		return err
-	}
-	if err := mounter.Unmount(volPath); err != nil {
-		glog.Errorf("failed to unmount %s", volPath)
-		return err
-	}
-	notMnt, mntErr := mounter.IsLikelyNotMountPoint(volPath)
-	if mntErr != nil {
-		glog.Errorf("IsLikelyNotMountPoint check failed: %v", mntErr)
-		return err
-	}
-	if notMnt {
-		if err := os.Remove(volPath); err != nil {
-			return err
-		}
-	}
-	return nil
-
-}

pkg/volume/iscsi/iscsi.go

@@ -267,13 +267,7 @@ func (c *iscsiDiskUnmounter) TearDown() error {
 }
 
 func (c *iscsiDiskUnmounter) TearDownAt(dir string) error {
-	if pathExists, pathErr := ioutil.PathExists(dir); 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", dir)
-		return nil
-	}
-	return diskTearDown(c.manager, *c, dir, c.mounter)
+	return ioutil.UnmountPath(dir, c.mounter)
 }
 
 func portalMounter(portal string) string {

pkg/volume/iscsi/iscsi_util.go

@@ -293,6 +293,9 @@ func (util *ISCSIUtil) AttachDisk(b iscsiDiskMounter) (string, error) {
 		glog.Errorf("iscsi: failed to get any path for iscsi disk, last err seen:\n%v", lastErr)
 		return "", fmt.Errorf("failed to get any path for iscsi disk, last err seen:\n%v", lastErr)
 	}
+	if lastErr != nil {
+		glog.Errorf("iscsi: last error occurred during iscsi init:\n%v", lastErr)
+	}
 
 	//Make sure we use a valid devicepath to find mpio device.
 	devicePath = devicePaths[0]