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, mount.New("")); 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

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"os/exec"
 	"path"
 	"path/filepath"
 	"strconv"
@@ -338,6 +339,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.
@@ -370,20 +389,34 @@ 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 {
+	if err != nil && (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)
 		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)
+	// ensure the mount point is unmounted
+	isStillMnt, validateUnmountErr := mounter.IsMountPoint(path)
+	if validateUnmountErr != nil {
+		if os.IsNotExist(validateUnmountErr) {
+			isStillMnt = false
+		} else {
+			return errors.Wrapf(validateUnmountErr, "failed to validate unmounted mount point %v", path)
+		}
+	}
+	if !isStillMnt {
 		return nil
 	}
 
+	logrus.Warnf("Mount point %v is still busy after unmount, trying to lazy unmount to release mount point", path)
+	lazyUnmountCmd := exec.Command("umount", "-l", path)
+	if output, err := lazyUnmountCmd.CombinedOutput(); err != nil {
+		return errors.Wrapf(err, "failed to lazy unmount a mount point %v, output: %s", path, output)
+	}
 	return err
 }
 
-// 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)
@@ -392,8 +425,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.
@@ -439,7 +484,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))