Made unmounting safer and more robust by doing the same thing as Kubernetes

Author: davidz627

pkg/gce-pd-csi-driver/node.go

@@ -66,7 +66,7 @@ func (ns *GCENodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePub
 		return nil, status.Error(codes.InvalidArgument, "NodePublishVolume Volume Capability must be provided")
 	}
 
-	notMnt, err := ns.Mounter.Interface.IsLikelyNotMountPoint(targetPath)
+	notMnt, err := ns.Mounter.Interface.IsNotMountPoint(targetPath)
 	if err != nil && !os.IsNotExist(err) {
 		glog.Errorf("cannot validate mount point: %s %v", targetPath, err)
 		return nil, err
@@ -133,9 +133,7 @@ func (ns *GCENodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeU
 		return nil, status.Error(codes.InvalidArgument, "NodeUnpublishVolume Target Path must be provided")
 	}
 
-	// TODO(#96): Check volume still exists
-
-	err := ns.Mounter.Interface.Unmount(targetPath)
+	err := unmountMountPoint(targetPath, ns.Mounter.Interface, true /* bind mount */)
 	if err != nil {
 		return nil, status.Error(codes.Internal, fmt.Sprintf("Unmount failed: %v\nUnmounting arguments: %s\n", err, targetPath))
 	}
@@ -242,7 +240,7 @@ func (ns *GCENodeServer) NodeUnstageVolume(ctx context.Context, req *csi.NodeUns
 		return nil, status.Error(codes.InvalidArgument, "NodeUnstageVolume Staging Target Path must be provided")
 	}
 
-	err := ns.Mounter.Interface.Unmount(stagingTargetPath)
+	err := unmountMountPoint(stagingTargetPath, ns.Mounter.Interface, false /* bind mount */)
 	if err != nil {
 		return nil, status.Error(codes.Internal, fmt.Sprintf("NodeUnstageVolume failed to unmount at path %s: %v", stagingTargetPath, err))
 	}

pkg/gce-pd-csi-driver/utils.go

@@ -17,10 +17,15 @@ limitations under the License.
 package gceGCEDriver
 
 import (
+	"fmt"
+	"os"
+	"syscall"
+
 	csi "github.com/container-storage-interface/spec/lib/go/csi/v0"
 	"github.com/golang/glog"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc"
+	"k8s.io/kubernetes/pkg/util/mount"
 )
 
 func NewVolumeCapabilityAccessMode(mode csi.VolumeCapability_AccessMode_Mode) *csi.VolumeCapability_AccessMode {
@@ -58,3 +63,97 @@ func logGRPC(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, h
 	}
 	return resp, err
 }
+
+// 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 {
+	pathExists, pathErr := pathExists(mountPath)
+	if !pathExists {
+		glog.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)
+	}
+	return doUnmountMountPoint(mountPath, mounter, extensiveMountPointCheck, corruptedMnt)
+}
+
+// doUnmountMountPoint 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.
+// if corruptedMnt is true, it means that the mountPath is a corrupted mountpoint, Take it as an argument for convenience of testing
+func doUnmountMountPoint(mountPath string, mounter mount.Interface, extensiveMountPointCheck bool, corruptedMnt bool) error {
+	if !corruptedMnt {
+		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
+	}
+	notMnt, mntErr := mounter.IsLikelyNotMountPoint(mountPath)
+	if mntErr != nil {
+		return mntErr
+	}
+	if notMnt {
+		glog.V(4).Infof("%q is unmounted, deleting the directory", mountPath)
+		return os.Remove(mountPath)
+	}
+	return fmt.Errorf("Failed to unmount path %v", mountPath)
+}
+
+// pathExists returns true if the specified path exists.
+func pathExists(path string) (bool, error) {
+	_, err := os.Stat(path)
+	if err == nil {
+		return true, nil
+	} else if os.IsNotExist(err) {
+		return false, nil
+	} else if isCorruptedMnt(err) {
+		return true, err
+	} else {
+		return false, err
+	}
+}
+
+// isCorruptedMnt return true if err is about corrupted mount point
+func isCorruptedMnt(err error) bool {
+	if err == nil {
+		return false
+	}
+	var underlyingError error
+	switch pe := err.(type) {
+	case nil:
+		return false
+	case *os.PathError:
+		underlyingError = pe.Err
+	case *os.LinkError:
+		underlyingError = pe.Err
+	case *os.SyscallError:
+		underlyingError = pe.Err
+	}
+
+	return underlyingError == syscall.ENOTCONN || underlyingError == syscall.ESTALE || underlyingError == syscall.EIO
+}