From 4899bcaca8769292f709f77e5e128fd3c2c6aaf9 Mon Sep 17 00:00:00 2001 From: Aleksandr Zimin Date: Fri, 31 May 2024 14:38:18 +0300 Subject: [PATCH] some fixes Signed-off-by: Aleksandr Zimin --- images/sds-local-volume-csi/driver/node.go | 35 ++++++++++--------- .../pkg/utils/node-store-manager.go | 20 ++++------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/images/sds-local-volume-csi/driver/node.go b/images/sds-local-volume-csi/driver/node.go index 5559daa3..85112e87 100644 --- a/images/sds-local-volume-csi/driver/node.go +++ b/images/sds-local-volume-csi/driver/node.go @@ -207,8 +207,24 @@ func (d *Driver) NodePublishVolume(ctx context.Context, request *csi.NodePublish mountOptions = append(mountOptions, "ro") } + vgName, ok := request.GetVolumeContext()[internal.VGNameKey] + if !ok { + return nil, status.Error(codes.InvalidArgument, "[NodePublishVolume] Volume group name cannot be empty") + } + + devPath := fmt.Sprintf("/dev/%s/%s", vgName, request.VolumeId) + d.log.Debug(fmt.Sprintf("[NodePublishVolume] Checking if device exists: %s", devPath)) + exists, err := d.storeManager.PathExists(devPath) + if err != nil { + return nil, status.Errorf(codes.Internal, "[NodePublishVolume] Error checking if device exists: %v", err) + } + if !exists { + return nil, status.Errorf(codes.NotFound, "[NodePublishVolume] Device %q not found", devPath) + } + d.log.Debug(fmt.Sprintf("[NodePublishVolume] Volume %s operation started", volumeID)) - ok := d.inFlight.Insert(volumeID) + + ok = d.inFlight.Insert(volumeID) if !ok { return nil, status.Errorf(codes.Aborted, VolumeOperationAlreadyExists, volumeID) } @@ -220,21 +236,8 @@ 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.") - vgName, ok := request.GetVolumeContext()[internal.VGNameKey] - if !ok { - return nil, status.Error(codes.InvalidArgument, "[NodeStageVolume] Volume group name cannot be empty") - } - devPath := fmt.Sprintf("/dev/%s/%s", vgName, request.VolumeId) - d.log.Debug(fmt.Sprintf("[NodePublishVolume] Checking if device exists: %s", devPath)) - exists, err := d.storeManager.PathExists(devPath) - if err != nil { - return nil, status.Errorf(codes.Internal, "[NodePublishVolume] Error checking if device exists: %v", err) - } - if !exists { - return nil, status.Errorf(codes.NotFound, "[NodePublishVolume] Device %q not found", devPath) - } - err = d.storeManager.NodePublishVolumeBlock(devPath, target, mountOptions) + err := d.storeManager.NodePublishVolumeBlock(devPath, target, mountOptions) if err != nil { return nil, status.Errorf(codes.Internal, "[NodePublishVolume] Error mounting volume %q at %q: %v", devPath, target, err) } @@ -257,7 +260,7 @@ func (d *Driver) NodePublishVolume(ctx context.Context, request *csi.NodePublish mountOptions = collectMountOptions(fsType, mountVolume.GetMountFlags(), mountOptions) - err := d.storeManager.NodePublishVolumeFS(source, target, fsType, mountOptions) + err := d.storeManager.NodePublishVolumeFS(source, devPath, target, fsType, mountOptions) if err != nil { return nil, status.Errorf(codes.Internal, "[NodePublishVolume] Error bind mounting volume %q. Source: %q. Target: %q. Mount options:%v. Err: %v", volumeID, source, target, mountOptions, err) } diff --git a/images/sds-local-volume-csi/pkg/utils/node-store-manager.go b/images/sds-local-volume-csi/pkg/utils/node-store-manager.go index 310f3a8d..12b9f688 100644 --- a/images/sds-local-volume-csi/pkg/utils/node-store-manager.go +++ b/images/sds-local-volume-csi/pkg/utils/node-store-manager.go @@ -31,7 +31,7 @@ import ( type NodeStoreManager interface { NodeStageVolumeFS(source, target string, fsType string, mountOpts []string, lvmType, lvmThinPoolName string) error NodePublishVolumeBlock(source, target string, mountOpts []string) error - NodePublishVolumeFS(source, target, fsType string, mountOpts []string) error + NodePublishVolumeFS(source, devPath, target, fsType string, mountOpts []string) error Unstage(target string) error Unpublish(target string) error IsNotMountPoint(target string) (bool, error) @@ -173,7 +173,7 @@ func (s *Store) NodePublishVolumeBlock(source, target string, mountOpts []string return nil } -func (s *Store) NodePublishVolumeFS(source, target, fsType string, mountOpts []string) error { +func (s *Store) NodePublishVolumeFS(source, devPath, target, fsType string, mountOpts []string) error { s.Log.Info(" ----== Start NodePublishVolumeFS ==---- ") s.Log.Trace(fmt.Sprintf("[NodePublishVolumeFS] params source=%q target=%q mountOptions=%v", source, target, mountOpts)) isMountPoint := false @@ -197,7 +197,7 @@ func (s *Store) NodePublishVolumeFS(source, target, fsType string, mountOpts []s if isMountPoint { s.Log.Trace(fmt.Sprintf("[NodePublishVolumeFS] target directory %q is a mount point. Check mount", target)) - err := checkMount(s, source, target, mountOpts) + err := checkMount(s, devPath, target, mountOpts) if err != nil { return fmt.Errorf("[NodePublishVolumeFS] failed to check mount info for %q: %w", target, err) } @@ -283,23 +283,17 @@ func toMapperPath(devPath string) string { return "/dev/mapper/" + mapperPath } -func checkMount(s *Store, source, target string, mountOpts []string) error { +func checkMount(s *Store, devPath, target string, mountOpts []string) error { mntInfo, err := s.NodeStorage.Interface.List() if err != nil { return fmt.Errorf("[checkMount] failed to list mounts: %w", err) } - s.Log.Trace("================ checkMount mntInfo ================") - s.Log.Trace(fmt.Sprintf(" : %+v", mntInfo)) - s.Log.Trace("================ checkMount mntInfo END ================") for _, m := range mntInfo { if m.Path == target { - s.Log.Trace("===============================") - s.Log.Trace(fmt.Sprintf("mount info for target: %s", target)) - s.Log.Trace(fmt.Sprintf(" : %+v", m)) - s.Log.Trace("================ checkMount END ================") - if m.Device != source { - return fmt.Errorf("[checkMount] device from mount point %q does not match expected source %q", m.Device, source) + mapperDevicePath := toMapperPath(devPath) + if m.Device != devPath && m.Device != mapperDevicePath { + return fmt.Errorf("[checkMount] device from mount point %q does not match expected source device path %s or mapper device path %s", m.Device, devPath, mapperDevicePath) } if slices.Contains(mountOpts, "ro") {