fix fs bind

Signed-off-by: Aleksandr Zimin <alexandr.zimin@flant.com>

images/sds-local-volume-csi/driver/node.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"sds-local-volume-csi/internal"
+	"slices"
 	"strings"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
@@ -93,7 +94,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, request *csi.NodeStageVolu
 		return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("[NodeStageVolume] Invalid fsType: %s. Supported values: %v", fsType, ValidFSTypes))
 	}
 
-	mountOptions := collectMountOptions(fsType, mountVolume.GetMountFlags())
+	mountOptions := collectMountOptions(fsType, mountVolume.GetMountFlags(), []string{})
 
 	d.log.Debug(fmt.Sprintf("[NodeStageVolume] Volume %s operation started", volumeID))
 	ok = d.inFlight.Insert(volumeID)
@@ -123,7 +124,7 @@ func (d *Driver) NodeStageVolume(ctx context.Context, request *csi.NodeStageVolu
 	d.log.Trace(fmt.Sprintf("lvmThinPoolName = %s", lvmThinPoolName))
 	d.log.Trace(fmt.Sprintf("fsType = %s", fsType))
 
-	err = d.storeManager.Mount(devPath, target, isBlock, fsType, false, mountOptions, lvmType, lvmThinPoolName)
+	err = d.storeManager.FormatAndMount(devPath, target, isBlock, fsType, false, mountOptions, lvmType, lvmThinPoolName)
 	if err != nil {
 		d.log.Error(err, "[NodeStageVolume] Error mounting volume")
 		return nil, status.Errorf(codes.Internal, "[NodeStageVolume] Error format device %q and mounting volume at %q: %v", devPath, target, err)
@@ -203,11 +204,6 @@ func (d *Driver) NodePublishVolume(ctx context.Context, request *csi.NodePublish
 		return nil, status.Error(codes.InvalidArgument, "[NodePublishVolume] Volume capability cannot be empty")
 	}
 
-	// vgName, ok := request.GetVolumeContext()[internal.VGNameKey]
-	// if !ok {
-	// 	return nil, status.Error(codes.InvalidArgument, "[NodePublishVolume] Volume group name cannot be empty")
-	// }
-
 	mountOptions := []string{"bind"}
 	if request.GetReadonly() {
 		mountOptions = append(mountOptions, "ro")
@@ -225,12 +221,13 @@ func (d *Driver) NodePublishVolume(ctx context.Context, request *csi.NodePublish
 
 	switch volCap.GetAccessType().(type) {
 	case *csi.VolumeCapability_Block:
-		d.log.Trace("NodePublishVolume[] Block volume detected.")
-		err := d.storeManager.Mount(source, target, true, "", request.GetReadonly(), mountOptions, "", "")
+		d.log.Trace("[NodePublishVolume] Block volume detected.")
+		err := d.storeManager.BindMount(source, target, "", mountOptions)
 		if err != nil {
 			return nil, status.Errorf(codes.Internal, "[NodePublishVolume] Error bind mounting block volume %q. Source: %q. Target: %q: %v", volumeID, source, target, err)
 		}
 	case *csi.VolumeCapability_Mount:
+		d.log.Trace("[NodePublishVolume] Mount volume detected.")
 		mountVolume := volCap.GetMount()
 		if mountVolume == nil {
 			return nil, status.Error(codes.InvalidArgument, "[NodePublishVolume] Volume capability mount cannot be empty")
@@ -245,13 +242,10 @@ func (d *Driver) NodePublishVolume(ctx context.Context, request *csi.NodePublish
 			return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("[NodeStageVolume] Invalid fsType: %s. Supported values: %v", fsType, ValidFSTypes))
 		}
 
-		for _, mnt := range mountVolume.GetMountFlags() {
-			if !hasMountOption(mountOptions, mnt) {
-				mountOptions = append(mountOptions, mnt)
-			}
-		}
+		mountOptions = collectMountOptions(fsType, mountVolume.GetMountFlags(), mountOptions)
+
+		err := d.storeManager.BindMount(source, target, fsType, mountOptions)
 
-		err := d.storeManager.Mount(source, target, false, fsType, request.GetReadonly(), mountOptions, "", "")
 		if err != nil {
 			return nil, status.Errorf(codes.Internal, "[NodePublishVolume] Error bind mounting volume %q. Source: %q. Target: %q: %v", volumeID, source, target, err)
 		}
@@ -359,35 +353,22 @@ func (d *Driver) NodeGetInfo(ctx context.Context, request *csi.NodeGetInfoReques
 	}, nil
 }
 
-// hasMountOption returns a boolean indicating whether the given
-// slice already contains a mount option. This is used to prevent
-// passing duplicate option to the mount command.
-func hasMountOption(options []string, opt string) bool {
-	for _, o := range options {
-		if o == opt {
-			return true
-		}
-	}
-	return false
-}
-
 // collectMountOptions returns array of mount options from
 // VolumeCapability_MountVolume and special mount options for
 // given filesystem.
-func collectMountOptions(fsType string, mntFlags []string) []string {
-	var options []string
-	for _, opt := range mntFlags {
-		if !hasMountOption(options, opt) {
-			options = append(options, opt)
+func collectMountOptions(fsType string, mountFlags, mountOptions []string) []string {
+	for _, opt := range mountFlags {
+		if !slices.Contains(mountOptions, opt) {
+			mountOptions = append(mountOptions, opt)
 		}
 	}
 
 	// // By default, xfs does not allow mounting of two volumes with the same filesystem uuid.
 	// // Force ignore this uuid to be able to mount volume + its clone / restored snapshot on the same node.
 	// if fsType == FSTypeXfs {
-	// 	if !hasMountOption(options, "nouuid") {
-	// 		options = append(options, "nouuid")
+	// 	if !slices.Contains(mountOptions, "nouuid") {
+	// 		mountOptions = append(mountOptions, "nouuid")
 	// 	}
 	// }
-	return options
+	return mountOptions
 }

images/sds-local-volume-csi/pkg/utils/volume.go

@@ -21,20 +21,22 @@ import (
 	"os"
 	"sds-local-volume-csi/internal"
 	"sds-local-volume-csi/pkg/logger"
+	"slices"
 	"strings"
 
 	mountutils "k8s.io/mount-utils"
 	utilexec "k8s.io/utils/exec"
 )
 
 type NodeStoreManager interface {
-	Mount(source, target string, isBlock bool, fsType string, readonly bool, mntOpts []string, lvmType, lvmThinPoolName string) error
+	FormatAndMount(source, target string, isBlock bool, fsType string, readonly bool, mountOpts []string, lvmType, lvmThinPoolName string) error
 	Unstage(target string) error
 	Unpublish(target string) error
 	IsNotMountPoint(target string) (bool, error)
 	ResizeFS(target string) error
 	PathExists(path string) (bool, error)
 	NeedResize(devicePath string, deviceMountPath string) (bool, error)
+	BindMount(source, target, fsType string, mountOpts []string) error
 }
 
 type Store struct {
@@ -52,7 +54,7 @@ func NewStore(logger *logger.Logger) *Store {
 	}
 }
 
-func (s *Store) Mount(devSourcePath, target string, isBlock bool, fsType string, readonly bool, mntOpts []string, lvmType, lvmThinPoolName string) error {
+func (s *Store) FormatAndMount(devSourcePath, target string, isBlock bool, fsType string, readonly bool, mntOpts []string, lvmType, lvmThinPoolName string) error {
 	s.Log.Info(" ----== Node Mount ==---- ")
 
 	s.Log.Trace("≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈ Mount options ≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈≈")
@@ -216,6 +218,47 @@ func (s *Store) NeedResize(devicePath string, deviceMountPath string) (bool, err
 	return mountutils.NewResizeFs(s.NodeStorage.Exec).NeedResize(devicePath, deviceMountPath)
 }
 
+func (s *Store) BindMount(source, target, fsType string, mountOpts []string) error {
+	s.Log.Info(" ----== Bind Mount ==---- ")
+	s.Log.Trace(fmt.Sprintf("[BindMount] params source=%q target=%q mountOptions=%v", source, target, mountOpts))
+	isMountPoint := false
+	exists, err := s.PathExists(target)
+	if err != nil {
+		return fmt.Errorf("[BindMount] could not check if target directory %s exists: %w", target, err)
+	}
+
+	if exists {
+		s.Log.Trace(fmt.Sprintf("[BindMount] target directory %s already exists", target))
+		isMountPoint, err = s.NodeStorage.IsMountPoint(target)
+		if err != nil {
+			return fmt.Errorf("[BindMount] could not check if target directory %s is a mount point: %w", target, err)
+		}
+	} else {
+		s.Log.Trace(fmt.Sprintf("[BindMount] creating target directory %q", target))
+		if err := os.MkdirAll(target, os.FileMode(0755)); err != nil {
+			return fmt.Errorf("[BindMount] could not create target directory %q: %w", target, err)
+		}
+	}
+
+	if isMountPoint {
+		s.Log.Trace(fmt.Sprintf("[BindMount] target directory %q is a mount point. Check mount", target))
+		err := checkMount(s, source, target, mountOpts)
+		if err != nil {
+			return fmt.Errorf("[BindMount] failed to check mount info for %q: %w", target, err)
+		}
+		s.Log.Trace(fmt.Sprintf("[BindMount] target directory %q is a mount point and already mounted to source %q", target, source))
+		return nil
+	}
+	// if err != nil {
+
+	err = s.NodeStorage.Interface.Mount(source, target, fsType, mountOpts)
+	if err != nil {
+		return fmt.Errorf("[BindMount] failed to bind mount %q to %q with mount options %v: %w", source, target, mountOpts, err)
+	}
+
+	return nil
+}
+
 func toMapperPath(devPath string) string {
 	if !strings.HasPrefix(devPath, "/dev/") {
 		return ""
@@ -226,3 +269,32 @@ func toMapperPath(devPath string) string {
 	mapperPath = strings.Replace(mapperPath, "/", "-", -1)
 	return "/dev/mapper/" + mapperPath
 }
+
+func checkMount(s *Store, source, target string, mountOpts []string) error {
+	mntInfo, err := s.NodeStorage.Interface.List()
+	if err != nil {
+		return fmt.Errorf("[checkMount] failed to list mounts: %w", err)
+	}
+
+	for _, m := range mntInfo {
+		if m.Path == target {
+			if m.Device != source {
+				return fmt.Errorf("[checkMount] device from mount point %q does not match expected source %q", m.Device, source)
+			}
+
+			if slices.Contains(mountOpts, "ro") {
+				if !slices.Contains(m.Opts, "ro") {
+					return fmt.Errorf("[checkMount] passed mount options contain 'ro' but mount options from mount point %q do not", target)
+				}
+
+				if slices.Equal(m.Opts, mountOpts) {
+					return fmt.Errorf("mount options %v do not match expected mount options %v", m.Opts, mountOpts)
+				}
+			}
+
+			return nil
+		}
+	}
+
+	return fmt.Errorf("[checkMount] mount point %q not found in mount info", target)
+}