Merge pull request #402 from andyzhangx/mountoptions-deletevolume

feat: support mountOptions in DeleteVolume

Author: andyzhangx

Makefile

@@ -213,7 +213,7 @@ clean:
 .PHONY: install-smb-provisioner
 install-smb-provisioner:
 	kubectl delete secret smbcreds --ignore-not-found
-	kubectl create secret generic smbcreds --from-literal username=USERNAME --from-literal password="PASSWORD"
+	kubectl create secret generic smbcreds --from-literal username=USERNAME --from-literal password="PASSWORD" --from-literal mountOptions="dir_mode=0777,file_mode=0777,uid=0,gid=0,mfsymlinks"
 ifdef TEST_WINDOWS
 	kubectl apply -f deploy/example/smb-provisioner/smb-server-lb.yaml
 else

docs/driver-parameters.md

@@ -25,4 +25,14 @@ nodeStageSecretRef.namespace | namespace where the secret is | k8s namespace  |
  - Use `kubectl create secret` to create `smbcreds` secret to store Samba Server username, password
 ```console
 kubectl create secret generic smbcreds --from-literal username=USERNAME --from-literal password="PASSWORD"
-```
+```
+
+### Tips
+#### provide `mountOptions` for `DeleteVolume`
+> since `DeleteVolumeRequest` does not provide `mountOptions`, following is the workaround to provide `mountOptions` for `DeleteVolume`
+  - create a secret `smbcreds` with `mountOptions`
+```console
+kubectl create secret generic smbcreds --from-literal username=USERNAME --from-literal password="PASSWORD" --from-literal mountOptions="dir_mode=0777,file_mode=0777,uid=0,gid=0,mfsymlinks"
+```
+
+ - set `csi.storage.k8s.io/provisioner-secret-name: "smbcreds"` in storage class

pkg/smb/controllerserver.go

@@ -123,13 +123,26 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest)
 		return &csi.DeleteVolumeResponse{}, nil
 	}
 
+	var volCap *csi.VolumeCapability
+	mountOptions := getMountOptions(req.GetSecrets())
+	if mountOptions != "" {
+		klog.V(2).Infof("DeleteVolume: found mountOptions(%v) for volume(%s)", mountOptions, volumeID)
+		volCap = &csi.VolumeCapability{
+			AccessType: &csi.VolumeCapability_Mount{
+				Mount: &csi.VolumeCapability_MountVolume{
+					MountFlags: []string{mountOptions},
+				},
+			},
+		}
+	}
+
 	secrets := req.GetSecrets()
 	if len(secrets) > 0 {
 		if len(smbVol.uuid) > 0 {
 			klog.V(2).Infof("existing subDir(%s) is provided, skip subdirectory creation", smbVol.subDir)
 		} else {
 			// Mount smb base share so we can delete the subdirectory
-			if err = d.internalMount(ctx, smbVol, nil, secrets); err != nil {
+			if err = d.internalMount(ctx, smbVol, volCap, secrets); err != nil {
 				return nil, status.Errorf(codes.Internal, "failed to mount smb server: %v", err.Error())
 			}
 			defer func() {
@@ -146,7 +159,7 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest)
 			}
 		}
 	} else {
-		klog.Infof("DeleteVolume(%s) does not provide secrets", volumeID)
+		klog.V(2).Infof("DeleteVolume(%s) does not provide secrets", volumeID)
 	}
 
 	return &csi.DeleteVolumeResponse{}, nil

pkg/smb/smb.go

@@ -17,7 +17,7 @@ limitations under the License.
 package smb
 
 import (
-	"fmt"
+	"strings"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
 
@@ -35,6 +35,7 @@ const (
 	sourceField       = "source"
 	subDirField       = "subdir"
 	domainField       = "domain"
+	mountOptionsField = "mountoptions"
 	defaultDomainName = "AZURE"
 )
 
@@ -67,7 +68,7 @@ func (d *Driver) Run(endpoint, kubeconfig string, testMode bool) {
 	if err != nil {
 		klog.Fatalf("%v", err)
 	}
-	klog.Infof("\nDRIVER INFORMATION:\n-------------------\n%s\n\nStreaming logs below:", versionMeta)
+	klog.V(2).Infof("\nDRIVER INFORMATION:\n-------------------\n%s\n\nStreaming logs below:", versionMeta)
 
 	d.mounter, err = mounter.NewSafeMounter()
 	if err != nil {
@@ -109,6 +110,16 @@ func (d *Driver) Run(endpoint, kubeconfig string, testMode bool) {
 
 func IsCorruptedDir(dir string) bool {
 	_, pathErr := mount.PathExists(dir)
-	fmt.Printf("IsCorruptedDir(%s) returned with error: %v", dir, pathErr)
 	return pathErr != nil && mount.IsCorruptedMnt(pathErr)
 }
+
+// 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/smb/smb_test.go

@@ -100,3 +100,39 @@ func TestRun(t *testing.T) {
 		t.Run(tc.name, tc.testFunc)
 	}
 }
+
+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": "dir_mode=0777"},
+			result:  "dir_mode=0777",
+		},
+		{
+			desc:    "valid mountOptions(lowercase)",
+			context: map[string]string{"mountoptions": "dir_mode=0777,file_mode=0777,uid=0,gid=0,mfsymlinks"},
+			result:  "dir_mode=0777,file_mode=0777,uid=0,gid=0,mfsymlinks",
+		},
+	}
+
+	for _, test := range tests {
+		result := getMountOptions(test.context)
+		if result != test.result {
+			t.Errorf("Unexpected result: %s, expected: %s", result, test.result)
+		}
+	}
+}