Merge pull request #375 from dell/bugfix-1582-mountoptions

Fix mountOptions not applied during node stage and expand volume

.github/workflows/actions.yml

@@ -12,7 +12,7 @@ jobs:
       - name: Checkout the code
         uses: actions/checkout@v4
       - name: Run the formatter, linter, and vetter
-        uses: dell/common-github-actions/go-code-formatter-linter-vetter@main
+        uses: dell/common-github-actions/go-code-formatter-vetter@main
         with:
           directories: ./...
   test:

pkg/array/array.go

@@ -278,7 +278,7 @@ func GetPowerStoreArrays(fs fs.Interface, filePath string) (map[string]*PowerSto
 // It will do that by querying default powerstore array passed as one of the arguments
 func ParseVolumeID(ctx context.Context, volumeHandle string,
 	defaultArray *PowerStoreArray, /*legacy support*/
-	cap *csi.VolumeCapability, /*legacy support*/
+	vc *csi.VolumeCapability, /*legacy support*/
 ) (localVolumeID, arrayID, protocol, remoteVolumeID, remoteArrayID string, e error) {
 	if volumeHandle == "" {
 		return "", "", "", "", "", status.Errorf(codes.FailedPrecondition,
@@ -305,8 +305,8 @@ func ParseVolumeID(ctx context.Context, volumeHandle string,
 		arrayID = defaultArray.GetGlobalID()
 
 		// If we have volume capability in request we can check FsType
-		if cap != nil && cap.GetMount() != nil {
-			if cap.GetMount().GetFsType() == "nfs" {
+		if vc != nil && vc.GetMount() != nil {
+			if vc.GetMount().GetFsType() == "nfs" {
 				protocol = "nfs"
 			} else {
 				protocol = "scsi"

pkg/common/common.go

@@ -297,8 +297,8 @@ func SetLogFields(ctx context.Context, fields log.Fields) context.Context {
 
 // RandomString returns a random string of specified length.
 // String is generated by using crypto/rand.
-func RandomString(len int) string {
-	b := make([]byte, len)
+func RandomString(length int) string {
+	b := make([]byte, length)
 	_, err := rand.Read(b)
 	if err != nil {
 		log.Errorf("Can't generate random string; error = %v", err)
@@ -525,3 +525,12 @@ func ReachableEndPoint(endpoint string) bool {
 	}
 	return true
 }
+
+func GetMountFlags(vc *csi.VolumeCapability) []string {
+	if vc != nil {
+		if mount := vc.GetMount(); mount != nil {
+			return mount.GetMountFlags()
+		}
+	}
+	return nil
+}

pkg/common/common_test.go

@@ -439,3 +439,51 @@ func TestReachableEndPoint(t *testing.T) {
 		})
 	}
 }
+
+func TestGetMountFlags(t *testing.T) {
+	tests := []struct {
+		name     string
+		vc       *csi.VolumeCapability
+		expected []string
+	}{
+		{
+			name:     "Nil VolumeCapability",
+			vc:       nil,
+			expected: nil,
+		},
+		{
+			name:     "Nil Mount",
+			vc:       &csi.VolumeCapability{},
+			expected: nil,
+		},
+		{
+			name: "With Mount Flags",
+			vc: &csi.VolumeCapability{
+				AccessType: &csi.VolumeCapability_Mount{
+					Mount: &csi.VolumeCapability_MountVolume{
+						MountFlags: []string{"ro", "noexec"},
+					},
+				},
+			},
+			expected: []string{"ro", "noexec"},
+		},
+		{
+			name: "Empty Mount Flags",
+			vc: &csi.VolumeCapability{
+				AccessType: &csi.VolumeCapability_Mount{
+					Mount: &csi.VolumeCapability_MountVolume{
+						MountFlags: []string{},
+					},
+				},
+			},
+			expected: []string{},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := common.GetMountFlags(tt.vc)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}

pkg/common/fs/fs.go

@@ -117,8 +117,8 @@ func (fs *Fs) OpenFile(name string, flag int, perm os.FileMode) (*os.File, error
 }
 
 // WriteString is a wrapper of file.WriteString
-func (fs *Fs) WriteString(file *os.File, string string) (int, error) {
-	return file.WriteString(string) // #nosec G304
+func (fs *Fs) WriteString(file *os.File, str string) (int, error) {
+	return file.WriteString(str) // #nosec G304
 }
 
 // Create is a wrapper of os.Create

pkg/controller/controller.go

@@ -1104,11 +1104,11 @@ func cacheMaximumVolumeSize(key string, value int64) {
 
 // ControllerGetCapabilities returns list of capabilities that are supported by the driver.
 func (s *Service) ControllerGetCapabilities(_ context.Context, _ *csi.ControllerGetCapabilitiesRequest) (*csi.ControllerGetCapabilitiesResponse, error) {
-	newCap := func(cap csi.ControllerServiceCapability_RPC_Type) *csi.ControllerServiceCapability {
+	newCap := func(capability csi.ControllerServiceCapability_RPC_Type) *csi.ControllerServiceCapability {
 		return &csi.ControllerServiceCapability{
 			Type: &csi.ControllerServiceCapability_Rpc{
 				Rpc: &csi.ControllerServiceCapability_RPC{
-					Type: cap,
+					Type: capability,
 				},
 			},
 		}

pkg/node/base.go

@@ -312,8 +312,8 @@ func deleteMapping(volID, tmpDir string, fs fs.Interface) error {
 	return err
 }
 
-func isBlock(cap *csi.VolumeCapability) bool {
-	_, isBlock := cap.GetAccessType().(*csi.VolumeCapability_Block)
+func isBlock(vc *csi.VolumeCapability) bool {
+	_, isBlock := vc.GetAccessType().(*csi.VolumeCapability_Block)
 	return isBlock
 }
 

pkg/node/node.go

@@ -885,7 +885,9 @@ func (s *Service) NodeExpandVolume(ctx context.Context, req *csi.NodeExpandVolum
 			return nil, status.Error(codes.Internal,
 				fmt.Sprintf("Failed to find mount info for (%s) with error (%s)", vol.Name, err.Error()))
 		}
-		err = s.Fs.GetUtil().Mount(ctx, disklocation, targetmount, "")
+
+		mntFlags := common.GetMountFlags(req.GetVolumeCapability())
+		err = s.Fs.GetUtil().Mount(ctx, disklocation, targetmount, "", mntFlags...)
 		if err != nil {
 			return nil, status.Error(codes.Internal,
 				fmt.Sprintf("Failed to find mount info for (%s) with error (%s)", vol.Name, err.Error()))
@@ -1072,11 +1074,11 @@ func (s *Service) nodeExpandRawBlockVolume(ctx context.Context, volumeWWN string
 
 // NodeGetCapabilities returns supported features by the node service
 func (s *Service) NodeGetCapabilities(_ context.Context, _ *csi.NodeGetCapabilitiesRequest) (*csi.NodeGetCapabilitiesResponse, error) {
-	newCap := func(cap csi.NodeServiceCapability_RPC_Type) *csi.NodeServiceCapability {
+	newCap := func(capability csi.NodeServiceCapability_RPC_Type) *csi.NodeServiceCapability {
 		return &csi.NodeServiceCapability{
 			Type: &csi.NodeServiceCapability_Rpc{
 				Rpc: &csi.NodeServiceCapability_RPC{
-					Type: cap,
+					Type: capability,
 				},
 			},
 		}

pkg/node/publisher.go

@@ -22,6 +22,7 @@ import (
 	"context"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
+	"github.com/dell/csi-powerstore/v2/pkg/common"
 	"github.com/dell/csi-powerstore/v2/pkg/common/fs"
 	log "github.com/sirupsen/logrus"
 	"google.golang.org/grpc/codes"
@@ -31,7 +32,7 @@ import (
 // VolumePublisher allows to node publish a volume
 type VolumePublisher interface {
 	Publish(ctx context.Context, logFields log.Fields, fs fs.Interface,
-		cap *csi.VolumeCapability, isRO bool, targetPath string, stagingPath string) (*csi.NodePublishVolumeResponse, error)
+		vc *csi.VolumeCapability, isRO bool, targetPath string, stagingPath string) (*csi.NodePublishVolumeResponse, error)
 }
 
 // SCSIPublisher implementation of NodeVolumePublisher for SCSI based (FC, iSCSI) volumes
@@ -40,7 +41,7 @@ type SCSIPublisher struct {
 }
 
 // Publish publishes volume as either raw block or mount by mounting it to the target path
-func (sp *SCSIPublisher) Publish(ctx context.Context, logFields log.Fields, fs fs.Interface, cap *csi.VolumeCapability, isRO bool, targetPath string, stagingPath string) (*csi.NodePublishVolumeResponse, error) {
+func (sp *SCSIPublisher) Publish(ctx context.Context, logFields log.Fields, fs fs.Interface, vc *csi.VolumeCapability, isRO bool, targetPath string, stagingPath string) (*csi.NodePublishVolumeResponse, error) {
 	published, err := isAlreadyPublished(ctx, targetPath, getRWModeString(isRO), fs)
 	if err != nil {
 		return nil, err
@@ -51,9 +52,9 @@ func (sp *SCSIPublisher) Publish(ctx context.Context, logFields log.Fields, fs f
 	}
 
 	if sp.isBlock {
-		return sp.publishBlock(ctx, logFields, fs, cap, isRO, targetPath, stagingPath)
+		return sp.publishBlock(ctx, logFields, fs, vc, isRO, targetPath, stagingPath)
 	}
-	return sp.publishMount(ctx, logFields, fs, cap, isRO, targetPath, stagingPath)
+	return sp.publishMount(ctx, logFields, fs, vc, isRO, targetPath, stagingPath)
 }
 
 func (sp *SCSIPublisher) publishBlock(ctx context.Context, logFields log.Fields, fs fs.Interface, _ *csi.VolumeCapability, isRO bool, targetPath string, stagingPath string) (*csi.NodePublishVolumeResponse, error) {
@@ -78,21 +79,21 @@ func (sp *SCSIPublisher) publishBlock(ctx context.Context, logFields log.Fields,
 	return &csi.NodePublishVolumeResponse{}, nil
 }
 
-func (sp *SCSIPublisher) publishMount(ctx context.Context, logFields log.Fields, fs fs.Interface, cap *csi.VolumeCapability, isRO bool, targetPath string, stagingPath string) (*csi.NodePublishVolumeResponse, error) {
-	if cap.GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER {
+func (sp *SCSIPublisher) publishMount(ctx context.Context, logFields log.Fields, fs fs.Interface, vc *csi.VolumeCapability, isRO bool, targetPath string, stagingPath string) (*csi.NodePublishVolumeResponse, error) {
+	if vc.GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER {
 		// MULTI_WRITER not supported for mount volumes
 		return nil, status.Error(codes.Unimplemented, "Mount volumes do not support AccessMode MULTI_NODE_MULTI_WRITER")
 	}
 
-	if cap.GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY {
+	if vc.GetAccessMode().GetMode() == csi.VolumeCapability_AccessMode_MULTI_NODE_READER_ONLY {
 		// Warning in case of MULTI_NODE_READER_ONLY for mount volumes
 		log.Warningf("Mount volume with the AccessMode ReadOnlyMany")
 	}
 
 	var opts []string
-	mountCap := cap.GetMount()
+	mountCap := vc.GetMount()
 	mountFsType := mountCap.GetFsType()
-	mntFlags := mountCap.GetMountFlags()
+	mntFlags := common.GetMountFlags(vc)
 	if mountFsType == "xfs" {
 		mntFlags = append(mntFlags, "nouuid")
 	}
@@ -150,7 +151,7 @@ type NFSPublisher struct{}
 
 // Publish publishes nfs volume by mounting it to the target path
 func (np *NFSPublisher) Publish(ctx context.Context, logFields log.Fields, fs fs.Interface,
-	cap *csi.VolumeCapability, isRO bool, targetPath string, stagingPath string,
+	vc *csi.VolumeCapability, isRO bool, targetPath string, stagingPath string,
 ) (*csi.NodePublishVolumeResponse, error) {
 	published, err := isAlreadyPublished(ctx, targetPath, getRWModeString(isRO), fs)
 	if err != nil {
@@ -167,8 +168,7 @@ func (np *NFSPublisher) Publish(ctx context.Context, logFields log.Fields, fs fs
 	}
 	log.WithFields(logFields).Info("target path successfully created")
 
-	mountCap := cap.GetMount()
-	mntFlags := mountCap.GetMountFlags()
+	mntFlags := common.GetMountFlags(vc)
 
 	if isRO {
 		mntFlags = append(mntFlags, "ro")

pkg/node/stager.go

@@ -118,7 +118,8 @@ func (s *SCSIStager) Stage(ctx context.Context, req *csi.NodeStageVolumeRequest,
 	}
 	log.WithFields(logFields).Info("target path successfully created")
 
-	if err := fs.GetUtil().BindMount(ctx, devicePath, stagingPath); err != nil {
+	mntFlags := common.GetMountFlags(req.GetVolumeCapability())
+	if err := fs.GetUtil().BindMount(ctx, devicePath, stagingPath, mntFlags...); err != nil {
 		return nil, status.Errorf(codes.Internal,
 			"error bind disk %s to target path: %s", devicePath, err.Error())
 	}
@@ -177,7 +178,8 @@ func (n *NFSStager) Stage(ctx context.Context, req *csi.NodeStageVolumeRequest,
 	}
 	log.WithFields(logFields).Info("stage path successfully created")
 
-	if err := fs.GetUtil().Mount(ctx, nfsExport, stagingPath, ""); err != nil {
+	mntFlags := common.GetMountFlags(req.GetVolumeCapability())
+	if err := fs.GetUtil().Mount(ctx, nfsExport, stagingPath, "", mntFlags...); err != nil {
 		return nil, status.Errorf(codes.Internal,
 			"error mount nfs share %s to target path: %s", nfsExport, err.Error())
 	}