Merge pull request #911 from andyzhangx/create-respect-mountoption

chore: create voume/snapshot should respect mountOptions in secret

pkg/nfs/controllerserver.go

@@ -163,7 +163,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
 
-	var volCap *csi.VolumeCapability
+	volCap := getVolumeCapabilityFromSecret(name, req.GetSecrets())
 	if len(req.GetVolumeCapabilities()) > 0 {
 		volCap = req.GetVolumeCapabilities()[0]
 	}
@@ -220,19 +220,6 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 		return &csi.DeleteVolumeResponse{}, nil
 	}
 
-	var volCap *csi.VolumeCapability
-	mountOptions := getMountOptions(req.GetSecrets())
-	if mountOptions != "" {
-		klog.V(2).Infof("DeleteVolume: found mountOptions(%s) for volume(%s)", mountOptions, volumeID)
-		volCap = &csi.VolumeCapability{
-			AccessType: &csi.VolumeCapability_Mount{
-				Mount: &csi.VolumeCapability_MountVolume{
-					MountFlags: []string{mountOptions},
-				},
-			},
-		}
-	}
-
 	if nfsVol.onDelete == "" {
 		nfsVol.onDelete = cs.Driver.defaultOnDeletePolicy
 	}
@@ -253,6 +240,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 			return &csi.DeleteVolumeResponse{}, nil
 		}
 		// mount nfs base share so we can delete the subdirectory
+		volCap := getVolumeCapabilityFromSecret(volumeID, req.GetSecrets())
 		if err = cs.internalMount(ctx, nfsVol, nil, volCap); err != nil {
 			return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err)
 		}
@@ -376,7 +364,8 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 		return nil, status.Errorf(codes.NotFound, "failed to create nfsSnapshot: %v", err)
 	}
 	snapVol := volumeFromSnapshot(snapshot)
-	if err = cs.internalMount(ctx, snapVol, req.GetParameters(), nil); err != nil {
+	volCap := getVolumeCapabilityFromSecret(req.GetSourceVolumeId(), req.GetSecrets())
+	if err = cs.internalMount(ctx, snapVol, req.GetParameters(), volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount snapshot nfs server: %v", err)
 	}
 	defer func() {
@@ -392,7 +381,7 @@ func (cs *ControllerServer) CreateSnapshot(ctx context.Context, req *csi.CreateS
 		return nil, err
 	}
 
-	if err = cs.internalMount(ctx, srcVol, req.GetParameters(), nil); err != nil {
+	if err = cs.internalMount(ctx, srcVol, req.GetParameters(), volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount src nfs server: %v", err)
 	}
 	defer func() {
@@ -445,18 +434,7 @@ func (cs *ControllerServer) DeleteSnapshot(ctx context.Context, req *csi.DeleteS
 		return &csi.DeleteSnapshotResponse{}, nil
 	}
 
-	var volCap *csi.VolumeCapability
+	volCap := getVolumeCapabilityFromSecret(req.SnapshotId, req.GetSecrets())
-	mountOptions := getMountOptions(req.GetSecrets())
-	if mountOptions != "" {
-		klog.V(2).Infof("DeleteSnapshot: found mountOptions(%s) for snapshot(%s)", mountOptions, req.GetSnapshotId())
-		volCap = &csi.VolumeCapability{
-			AccessType: &csi.VolumeCapability_Mount{
-				Mount: &csi.VolumeCapability_MountVolume{
-					MountFlags: []string{mountOptions},
-				},
-			},
-		}
-	}
 	vol := volumeFromSnapshot(snap)
 	if err = cs.internalMount(ctx, vol, nil, volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount nfs server for snapshot deletion: %v", err)

pkg/nfs/utils.go

@@ -289,3 +289,21 @@ func WaitUntilTimeout(timeout time.Duration, execFunc ExecFunc, timeoutFunc Time
 		return timeoutFunc()
 	}
 }
+
+// getVolumeCapabilityFromSecret retrieves the volume capability from the secret
+// if secret contains mountOptions, it will return the volume capability
+// if secret does not contain mountOptions, it will return nil
+func getVolumeCapabilityFromSecret(volumeID string, secret map[string]string) *csi.VolumeCapability {
+	mountOptions := getMountOptions(secret)
+	if mountOptions != "" {
+		klog.V(2).Infof("found mountOptions(%s) for volume(%s)", mountOptions, volumeID)
+		return &csi.VolumeCapability{
+			AccessType: &csi.VolumeCapability_Mount{
+				Mount: &csi.VolumeCapability_MountVolume{
+					MountFlags: []string{mountOptions},
+				},
+			},
+		}
+	}
+	return nil
+}

pkg/nfs/utils_test.go

@@ -24,6 +24,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/container-storage-interface/spec/lib/go/csi"
 	"go.uber.org/goleak"
 )
 
@@ -505,3 +506,51 @@ func TestWaitUntilTimeout(t *testing.T) {
 		}
 	}
 }
+func TestGetVolumeCapabilityFromSecret(t *testing.T) {
+	tests := []struct {
+		desc     string
+		volumeID string
+		secret   map[string]string
+		expected *csi.VolumeCapability
+	}{
+		{
+			desc:     "secret contains mountOptions",
+			volumeID: "vol-123",
+			secret:   map[string]string{"mountOptions": "nfsvers=3"},
+			expected: &csi.VolumeCapability{
+				AccessType: &csi.VolumeCapability_Mount{
+					Mount: &csi.VolumeCapability_MountVolume{
+						MountFlags: []string{"nfsvers=3"},
+					},
+				},
+			},
+		},
+		{
+			desc:     "secret does not contain mountOptions",
+			volumeID: "vol-456",
+			secret:   map[string]string{"otherKey": "otherValue"},
+			expected: nil,
+		},
+		{
+			desc:     "empty secret",
+			volumeID: "vol-789",
+			secret:   map[string]string{},
+			expected: nil,
+		},
+		{
+			desc:     "nil secret",
+			volumeID: "vol-000",
+			secret:   nil,
+			expected: nil,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.desc, func(t *testing.T) {
+			result := getVolumeCapabilityFromSecret(test.volumeID, test.secret)
+			if !reflect.DeepEqual(result, test.expected) {
+				t.Errorf("test[%s]: unexpected result: %v, expected: %v", test.desc, result, test.expected)
+			}
+		})
+	}
+}