Merge pull request #262 from andyzhangx/deletevolume-mountoptions

feat: support mountOptions in DeleteVolume

Makefile

@@ -151,6 +151,8 @@ endif
 .PHONY: install-nfs-server
 install-nfs-server:
 	kubectl apply -f ./deploy/example/nfs-provisioner/nfs-server.yaml
+	kubectl delete secret mount-options --ignore-not-found
+	kubectl create secret generic mount-options --from-literal mountOptions="nfsvers=4.1"
 
 .PHONY: install-helm
 install-helm:

charts/latest/csi-driver-nfs/templates/rbac-csi-nfs-controller.yaml

@@ -37,6 +37,9 @@ rules:
   - apiGroups: ["coordination.k8s.io"]
     resources: ["leases"]
     verbs: ["get", "list", "watch", "create", "update", "patch"]
+  - apiGroups: [""]
+    resources: ["secrets"]
+    verbs: ["get"]
 ---
 kind: ClusterRoleBinding
 apiVersion: rbac.authorization.k8s.io/v1

deploy/rbac-csi-nfs-controller.yaml

@@ -33,6 +33,9 @@ rules:
   - apiGroups: ["coordination.k8s.io"]
     resources: ["leases"]
     verbs: ["get", "list", "watch", "create", "update", "patch"]
+  - apiGroups: [""]
+    resources: ["secrets"]
+    verbs: ["get"]
 ---
 
 kind: ClusterRoleBinding

docs/driver-parameters.md

@@ -1,5 +1,5 @@
 ## Driver Parameters
-> This plugin driver itself only provides a communication layer between resources in the cluser and the NFS server, you need to bring your own NFS server before using this driver.
+> This driver requires existing and already configured NFSv3 or NFSv4 server, it supports dynamic provisioning of Persistent Volumes via Persistent Volume Claims by creating a new sub directory under NFS server.
 
 ### Storage Class Usage (Dynamic Provisioning)
 > [`StorageClass` example](../deploy/example/storageclass-nfs.yaml)

pkg/nfs/controllerserver.go

@@ -127,8 +127,21 @@ 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},
+				},
+			},
+		}
+	}
+
 	// Mount nfs base share so we can delete the subdirectory
-	if err = cs.internalMount(ctx, nfsVol, nil); err != nil {
+	if err = cs.internalMount(ctx, nfsVol, volCap); err != nil {
 		return nil, status.Errorf(codes.Internal, "failed to mount nfs server: %v", err.Error())
 	}
 	defer func() {
@@ -285,8 +298,7 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str
 		baseDir string
 	)
 
-	// Validate parameters (case-insensitive).
-	// TODO do more strict validation.
+	// validate parameters (case-insensitive)
 	for k, v := range params {
 		switch strings.ToLower(k) {
 		case paramServer:
@@ -298,7 +310,7 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str
 		}
 	}
 
-	// Validate required parameters
+	// validate required parameters
 	if server == "" {
 		return nil, fmt.Errorf("%v is a required parameter", paramServer)
 	}

pkg/nfs/nfs.go

@@ -49,7 +49,8 @@ const (
 	// The base directory must be a direct child of the root directory.
 	// The root directory is omitted from the string, for example:
 	//     "base" instead of "/base"
-	paramShare = "share"
+	paramShare        = "share"
+	mountOptionsField = "mountoptions"
 )
 
 func NewDriver(nodeID, driverName, endpoint string, perm *uint32) *Driver {

pkg/nfs/utils.go

@@ -121,3 +121,14 @@ func (vl *VolumeLocks) Release(volumeID string) {
 	defer vl.mux.Unlock()
 	vl.locks.Delete(volumeID)
 }
+
+// getMountOptions get mountOptions value from a map
+func getMountOptions(context map[string]string) string {
+	for k, v := range context {
+		switch strings.ToLower(k) {
+		case mountOptionsField:
+			return v
+		}
+	}
+	return ""
+}

pkg/nfs/utils_test.go

@@ -118,3 +118,39 @@ func TestGetLogLevel(t *testing.T) {
 		}
 	}
 }
+
+func TestGetMountOptions(t *testing.T) {
+	tests := []struct {
+		desc    string
+		context map[string]string
+		result  string
+	}{
+		{
+			desc:    "nil context",
+			context: nil,
+			result:  "",
+		},
+		{
+			desc:    "empty context",
+			context: map[string]string{},
+			result:  "",
+		},
+		{
+			desc:    "valid mountOptions",
+			context: map[string]string{"mountOptions": "nfsvers=3"},
+			result:  "nfsvers=3",
+		},
+		{
+			desc:    "valid mountOptions(lowercase)",
+			context: map[string]string{"mountoptions": "nfsvers=4"},
+			result:  "nfsvers=4",
+		},
+	}
+
+	for _, test := range tests {
+		result := getMountOptions(test.context)
+		if result != test.result {
+			t.Errorf("Unexpected result: %s, expected: %s", result, test.result)
+		}
+	}
+}

test/e2e/e2e_suite_test.go

@@ -47,6 +47,8 @@ var (
 	defaultStorageClassParameters = map[string]string{
 		"server": "nfs-server.default.svc.cluster.local",
 		"share":  "/",
+		"csi.storage.k8s.io/provisioner-secret-name":      "mount-options",
+		"csi.storage.k8s.io/provisioner-secret-namespace": "default",
 	}
 	controllerServer *nfs.ControllerServer
 )