fix(csi): ensure the bind mount point for block volume

longhorn-12006

Signed-off-by: Raphanus Lo <yunchang.lo@suse.com>

Author: COLDTURNIP

csi/node_server.go

@@ -364,22 +364,24 @@ func (ns *NodeServer) nodePublishBlockVolume(volumeID, devicePath, targetPath st
 
 	// we ensure the parent directory exists and is valid
 	if _, err := ensureDirectory(filepath.Dir(targetPath)); err != nil {
-		return status.Errorf(codes.Internal, "failed to prepare mount point for block device %v: %v", devicePath, err)
+		return status.Errorf(codes.Internal, "failed to prepare mount point directory for block device %v: %v", devicePath, err)
 	}
 
-	// create file where we can bind mount the device to
-	if err := makeFile(targetPath); err != nil {
-		return status.Errorf(codes.Internal, "failed to create file %v: %v", targetPath, err)
+	// ensure the bind mount point is clear
+	if err := ensureBindMountPoint(targetPath, mounter); err != nil {
+		return status.Errorf(codes.Internal, "failed to prepare mount point file for block device %v: %v", devicePath, err)
 	}
 
 	log.Infof("Bind mounting device %v at %v", devicePath, targetPath)
-	if err := mounter.Mount(devicePath, targetPath, "", []string{"bind"}); err != nil {
-		if removeErr := os.Remove(targetPath); removeErr != nil {
-			log.WithError(removeErr).Errorf("Failed to remove failed mount target %q", targetPath)
-		}
-		return status.Errorf(codes.Internal, "failed to bind mount %q at %q: %v", devicePath, targetPath, err)
+	mountErr := mounter.Mount(devicePath, targetPath, "", []string{"bind"})
+	if mountErr == nil {
+		return nil
 	}
-	return nil
+	log.WithError(mountErr).Errorf("Clean up the mount target %s due to mount error", targetPath)
+	if removeErr := unmountAndCleanupMountPoint(targetPath, mounter); removeErr != nil {
+		log.WithError(removeErr).Errorf("Failed to clean up the mount target %q for failed mounting", targetPath)
+	}
+	return status.Errorf(codes.Internal, "failed to bind mount %q at %q: %v", devicePath, targetPath, mountErr)
 }
 
 func (ns *NodeServer) NodeUnpublishVolume(ctx context.Context, req *csi.NodeUnpublishVolumeRequest) (*csi.NodeUnpublishVolumeResponse, error) {

csi/util.go

@@ -369,6 +369,24 @@ func ensureMountPoint(path string, mounter mount.Interface) (bool, error) {
 	return isMnt, err
 }
 
+// ensureBindMountPoint evaluates whether a path is a valid mount point for bind mount.
+// In case the path does not exist, it will create a regular file for bind mount.
+// In case where the mount point exists, the mount point will be cleared and replaced by a new regular file.
+func ensureBindMountPoint(path string, mounter mount.Interface) error {
+	logrus.Infof("Trying to ensure bind mount point %v", path)
+
+	// blindly clean up the target mount point for recreation
+	if cleanupErr := unmountAndCleanupMountPoint(path, mounter); cleanupErr != nil {
+		return errors.Wrapf(cleanupErr, "failed to remove corrupt bind mount point at %s", path)
+	}
+
+	// create regular file for bind mount
+	if makeFileErr := makeFile(path); makeFileErr != nil {
+		return errors.Wrapf(makeFileErr, "failed to create regular file for bind mount at %s", path)
+	}
+	return nil
+}
+
 // ensureDirectory checks if a folder exists at the specified path.
 // If not, it creates the folder and returns true, otherwise returns false.
 // If the path exists but is not a folder, it returns an error.
@@ -392,6 +410,18 @@ func ensureDirectory(path string) (bool, error) {
 	return false, fmt.Errorf("path %v exists but is not a folder", path)
 }
 
+func lazyUnmount(path string, exec utilexec.Interface) (err error) {
+	lazyUnmountCmd := exec.Command("umount", "-l", path)
+	output, err := lazyUnmountCmd.CombinedOutput()
+	if err != nil {
+		if isNotMountedError(err) {
+			return nil
+		}
+		return errors.Wrapf(err, "failed to lazy unmount a mount point %v, output: %s", path, output)
+	}
+	return nil
+}
+
 func unmount(path string, mounter mount.Interface) (err error) {
 	forceUnmounter, ok := mounter.(mount.MounterForceUnmounter)
 	if ok {
@@ -401,20 +431,14 @@ func unmount(path string, mounter mount.Interface) (err error) {
 		logrus.Infof("Trying to unmount potential mount point %v", path)
 		err = mounter.Unmount(path)
 	}
-	if err == nil {
-		return nil
-	}
-
-	if strings.Contains(err.Error(), "not mounted") ||
-		strings.Contains(err.Error(), "no mount point specified") {
-		logrus.Infof("No need for unmount not a mount point %v", path)
+	if err == nil || isNotMountedError(err) {
 		return nil
 	}
-
-	return err
+	logrus.WithError(err).Warnf("Error occurred during unmount %v, trying to lazy unmount to release mount point", path)
+	return lazyUnmount(path, utilexec.New())
 }
 
-// unmountAndCleanupMountPoint ensures all mount layers for the path are unmounted and the mount directory is removed
+// unmountAndCleanupMountPoint ensures all mount layers for the path are unmounted and the mount point is removed
 func unmountAndCleanupMountPoint(path string, mounter mount.Interface) error {
 	// we just try to unmount since the path check would get stuck for nfs mounts
 	logrus.Infof("Trying to umount mount point %v", path)
@@ -423,8 +447,20 @@ func unmountAndCleanupMountPoint(path string, mounter mount.Interface) error {
 		return err
 	}
 
+	// ensure the file is removed even when it is a broken symbolic link
 	logrus.Infof("Trying to clean up mount point %v", path)
-	return mount.CleanupMountPoint(path, mounter, true)
+	info, statErr := os.Lstat(path)
+	if statErr != nil {
+		if os.IsNotExist(statErr) {
+			return nil
+		}
+		return errors.Wrapf(statErr, "failed to check cleaned up mount point %v", path)
+	}
+	logrus.WithField("fileMode", info.Mode().String()).Infof("Mount point %v exists, removing the file", path)
+	if rmErr := os.Remove(path); rmErr != nil {
+		return errors.Wrapf(rmErr, "failed to remove mount point %v", path)
+	}
+	return nil
 }
 
 // isBlockDevice return true if volumePath file is a block device, false otherwise.
@@ -470,7 +506,7 @@ func getFilesystemStatistics(volumePath string) (*volumeFilesystemStatistics, er
 	return volStats, nil
 }
 
-// makeFile creates an empty file.
+// makeFile creates an empty regular file.
 // If pathname already exists, whether a file or directory, no error is returned.
 func makeFile(pathname string) error {
 	f, err := os.OpenFile(pathname, os.O_CREATE, os.FileMode(0644))
@@ -534,3 +570,11 @@ func parseNodeID(topology *csi.Topology) (string, error) {
 	}
 	return nodeId, nil
 }
+
+func isNotMountedError(err error) bool {
+	if err == nil {
+		return false
+	}
+	return strings.Contains(err.Error(), "not mounted") ||
+		strings.Contains(err.Error(), "no mount point specified")
+}