Merge pull request #353 from andyzhangx/subdir-metadata

feat: support pv/pvc metadata in subDir parameter

Author: k8s-ci-robot

charts/latest/csi-driver-nfs-v4.1.0.tgz

@@ -50,6 +50,7 @@ spec:
             - "--csi-address=$(ADDRESS)"
             - "--leader-election"
             - "--leader-election-namespace={{ .Release.Namespace }}"
+            - "--extra-create-metadata=true"
           env:
             - name: ADDRESS
               value: /csi/csi.sock

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

@@ -38,6 +38,7 @@ spec:
             - "--csi-address=$(ADDRESS)"
             - "--leader-election"
             - "--leader-election-namespace=kube-system"
+            - "--extra-create-metadata=true"
           env:
             - name: ADDRESS
               value: /csi/csi.sock

deploy/csi-nfs-controller.yaml

@@ -21,6 +21,12 @@ volumeAttributes.share | NFS share path | `/` |  Yes  |
 volumeAttributes.mountPermissions | mounted folder permissions. The default is `0777` |  | No |
 
 ### Tips
+#### `subDir` parameter supports following pv/pvc metadata transform
+> if `subDir` value contains following strings, it would transforms into corresponding pv/pvc name or namespace
+ - `${pvc.metadata.name}`
+ - `${pvc.metadata.namespace}`
+ - `${pv.metadata.name}`
+
 #### provide `mountOptions` for `DeleteVolume`
 > since `DeleteVolumeRequest` does not provide `mountOptions`, following is the workaround to provide `mountOptions` for `DeleteVolume`, check details [here](https://github.com/kubernetes-csi/csi-driver-nfs/issues/260)
   - create a secret with `mountOptions`

docs/driver-parameters.md

@@ -93,10 +93,11 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	for k, v := range parameters {
 		switch strings.ToLower(k) {
 		case paramServer:
-			// no op
 		case paramShare:
-			// no op
 		case paramSubDir:
+		case pvcNamespaceKey:
+		case pvcNameKey:
+		case pvNameKey:
 			// no op
 		case mountPermissionsField:
 			if v != "" {
@@ -110,7 +111,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		}
 	}
 
-	nfsVol, err := cs.newNFSVolume(name, reqCapacity, parameters)
+	nfsVol, err := newNFSVolume(name, reqCapacity, parameters)
 	if err != nil {
 		return nil, status.Error(codes.InvalidArgument, err.Error())
 	}
@@ -304,9 +305,10 @@ func (cs *ControllerServer) internalUnmount(ctx context.Context, vol *nfsVolume)
 	return err
 }
 
-// Convert VolumeCreate parameters to an nfsVolume
-func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[string]string) (*nfsVolume, error) {
+// newNFSVolume Convert VolumeCreate parameters to an nfsVolume
+func newNFSVolume(name string, size int64, params map[string]string) (*nfsVolume, error) {
 	var server, baseDir, subDir string
+	subDirReplaceMap := map[string]string{}
 
 	// validate parameters (case-insensitive)
 	for k, v := range params {
@@ -317,15 +319,18 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str
 			baseDir = v
 		case paramSubDir:
 			subDir = v
+		case pvcNamespaceKey:
+			subDirReplaceMap[pvcNamespaceMetadata] = v
+		case pvcNameKey:
+			subDirReplaceMap[pvcNameMetadata] = v
+		case pvNameKey:
+			subDirReplaceMap[pvNameMetadata] = v
 		}
 	}
 
 	if server == "" {
 		return nil, fmt.Errorf("%v is a required parameter", paramServer)
 	}
-	if baseDir == "" {
-		return nil, fmt.Errorf("%v is a required parameter", paramShare)
-	}
 
 	vol := &nfsVolume{
 		server:  server,
@@ -336,11 +341,12 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str
 		// use pv name by default if not specified
 		vol.subDir = name
 	} else {
-		vol.subDir = subDir
+		// replace pv/pvc name namespace metadata in subDir
+		vol.subDir = replaceWithMap(subDir, subDirReplaceMap)
 		// make volume id unique if subDir is provided
 		vol.uuid = name
 	}
-	vol.id = cs.getVolumeIDFromNfsVol(vol)
+	vol.id = getVolumeIDFromNfsVol(vol)
 	return vol, nil
 }
 
@@ -368,7 +374,7 @@ func getInternalVolumePath(workingMountDir string, vol *nfsVolume) string {
 }
 
 // Given a nfsVolume, return a CSI volume id
-func (cs *ControllerServer) getVolumeIDFromNfsVol(vol *nfsVolume) string {
+func getVolumeIDFromNfsVol(vol *nfsVolume) string {
 	idElements := make([]string, totalIDElements)
 	idElements[idServer] = strings.Trim(vol.server, "/")
 	idElements[idBaseDir] = strings.Trim(vol.baseDir, "/")

pkg/nfs/controllerserver.go

@@ -529,3 +529,87 @@ func TestGetInternalMountPath(t *testing.T) {
 		assert.Equal(t, path, test.result)
 	}
 }
+
+func TestNewNFSVolume(t *testing.T) {
+	cases := []struct {
+		desc      string
+		name      string
+		size      int64
+		params    map[string]string
+		expectVol *nfsVolume
+		expectErr error
+	}{
+		{
+			desc: "subDir is specified",
+			name: "pv-name",
+			size: 100,
+			params: map[string]string{
+				paramServer: "//nfs-server.default.svc.cluster.local",
+				paramShare:  "share",
+				paramSubDir: "subdir",
+			},
+			expectVol: &nfsVolume{
+				id:      "nfs-server.default.svc.cluster.local#share#subdir#pv-name",
+				server:  "//nfs-server.default.svc.cluster.local",
+				baseDir: "share",
+				subDir:  "subdir",
+				size:    100,
+				uuid:    "pv-name",
+			},
+		},
+		{
+			desc: "subDir with pv/pvc metadata is specified",
+			name: "pv-name",
+			size: 100,
+			params: map[string]string{
+				paramServer:     "//nfs-server.default.svc.cluster.local",
+				paramShare:      "share",
+				paramSubDir:     fmt.Sprintf("subdir-%s-%s-%s", pvcNameMetadata, pvcNamespaceMetadata, pvNameMetadata),
+				pvcNameKey:      "pvcname",
+				pvcNamespaceKey: "pvcnamespace",
+				pvNameKey:       "pvname",
+			},
+			expectVol: &nfsVolume{
+				id:      "nfs-server.default.svc.cluster.local#share#subdir-pvcname-pvcnamespace-pvname#pv-name",
+				server:  "//nfs-server.default.svc.cluster.local",
+				baseDir: "share",
+				subDir:  "subdir-pvcname-pvcnamespace-pvname",
+				size:    100,
+				uuid:    "pv-name",
+			},
+		},
+		{
+			desc: "subDir not specified",
+			name: "pv-name",
+			size: 200,
+			params: map[string]string{
+				paramServer: "//nfs-server.default.svc.cluster.local",
+				paramShare:  "share",
+			},
+			expectVol: &nfsVolume{
+				id:      "nfs-server.default.svc.cluster.local#share#pv-name#",
+				server:  "//nfs-server.default.svc.cluster.local",
+				baseDir: "share",
+				subDir:  "pv-name",
+				size:    200,
+				uuid:    "",
+			},
+		},
+		{
+			desc:      "server value is empty",
+			params:    map[string]string{},
+			expectVol: nil,
+			expectErr: fmt.Errorf("%s is a required parameter", paramServer),
+		},
+	}
+
+	for _, test := range cases {
+		vol, err := newNFSVolume(test.name, test.size, test.params)
+		if !reflect.DeepEqual(err, test.expectErr) {
+			t.Errorf("[test: %s] Unexpected error: %v, expected error: %v", test.desc, err, test.expectErr)
+		}
+		if !reflect.DeepEqual(vol, test.expectVol) {
+			t.Errorf("[test: %s] Unexpected vol: %v, expected vol: %v", test.desc, vol, test.expectVol)
+		}
+	}
+}

pkg/nfs/controllerserver_test.go

@@ -17,6 +17,8 @@ limitations under the License.
 package nfs
 
 import (
+	"strings"
+
 	"github.com/container-storage-interface/spec/lib/go/csi"
 	"k8s.io/klog/v2"
 	mount "k8s.io/mount-utils"
@@ -58,6 +60,12 @@ const (
 	paramSubDir           = "subdir"
 	mountOptionsField     = "mountoptions"
 	mountPermissionsField = "mountpermissions"
+	pvcNameKey            = "csi.storage.k8s.io/pvc/name"
+	pvcNamespaceKey       = "csi.storage.k8s.io/pvc/namespace"
+	pvNameKey             = "csi.storage.k8s.io/pv/name"
+	pvcNameMetadata       = "${pvc.metadata.name}"
+	pvcNamespaceMetadata  = "${pvc.metadata.namespace}"
+	pvNameMetadata        = "${pv.metadata.name}"
 )
 
 func NewDriver(options *DriverOptions) *Driver {
@@ -132,3 +140,13 @@ func IsCorruptedDir(dir string) bool {
 	_, pathErr := mount.PathExists(dir)
 	return pathErr != nil && mount.IsCorruptedMnt(pathErr)
 }
+
+// replaceWithMap replace key with value for str
+func replaceWithMap(str string, m map[string]string) string {
+	for k, v := range m {
+		if k != "" {
+			str = strings.ReplaceAll(str, k, v)
+		}
+	}
+	return str
+}

pkg/nfs/nfs.go

@@ -178,3 +178,55 @@ func TestNewNodeServiceCapability(t *testing.T) {
 		assert.Equal(t, resp.XXX_sizecache, int32(0))
 	}
 }
+
+func TestReplaceWithMap(t *testing.T) {
+	tests := []struct {
+		desc     string
+		str      string
+		m        map[string]string
+		expected string
+	}{
+		{
+			desc:     "empty string",
+			str:      "",
+			expected: "",
+		},
+		{
+			desc:     "empty map",
+			str:      "",
+			m:        map[string]string{},
+			expected: "",
+		},
+		{
+			desc:     "empty key",
+			str:      "prefix-" + pvNameMetadata,
+			m:        map[string]string{"": "pv"},
+			expected: "prefix-" + pvNameMetadata,
+		},
+		{
+			desc:     "empty value",
+			str:      "prefix-" + pvNameMetadata,
+			m:        map[string]string{pvNameMetadata: ""},
+			expected: "prefix-",
+		},
+		{
+			desc:     "one replacement",
+			str:      "prefix-" + pvNameMetadata,
+			m:        map[string]string{pvNameMetadata: "pv"},
+			expected: "prefix-pv",
+		},
+		{
+			desc:     "multiple replacements",
+			str:      pvcNamespaceMetadata + pvcNameMetadata,
+			m:        map[string]string{pvcNamespaceMetadata: "namespace", pvcNameMetadata: "pvcname"},
+			expected: "namespacepvcname",
+		},
+	}
+
+	for _, test := range tests {
+		result := replaceWithMap(test.str, test.m)
+		if result != test.expected {
+			t.Errorf("test[%s]: unexpected output: %v, expected result: %v", test.desc, result, test.expected)
+		}
+	}
+}

pkg/nfs/nfs_test.go

@@ -57,6 +57,8 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	}
 
 	var server, baseDir, subDir string
+	subDirReplaceMap := map[string]string{}
+
 	mountPermissions := ns.Driver.mountPermissions
 	performChmodOp := (mountPermissions > 0)
 	for k, v := range req.GetVolumeContext() {
@@ -67,6 +69,12 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 			baseDir = v
 		case paramSubDir:
 			subDir = v
+		case pvcNamespaceKey:
+			subDirReplaceMap[pvcNamespaceMetadata] = v
+		case pvcNameKey:
+			subDirReplaceMap[pvcNameMetadata] = v
+		case pvNameKey:
+			subDirReplaceMap[pvNameMetadata] = v
 		case mountOptionsField:
 			if v != "" {
 				mountOptions = append(mountOptions, v)
@@ -96,6 +104,9 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	server = getServerFromSource(server)
 	source := fmt.Sprintf("%s:%s", server, baseDir)
 	if subDir != "" {
+		// replace pv/pvc name namespace metadata in subDir
+		subDir = replaceWithMap(subDir, subDirReplaceMap)
+
 		source = strings.TrimRight(source, "/")
 		source = fmt.Sprintf("%s/%s", source, subDir)
 	}

pkg/nfs/nodeserver.go

@@ -47,6 +47,13 @@ func TestNodePublishVolume(t *testing.T) {
 		"share":               "share",
 		mountPermissionsField: "0755",
 	}
+	paramsWithMetadata := map[string]string{
+		"server":        "server",
+		"share":         "share",
+		pvcNameKey:      "pvcname",
+		pvcNamespaceKey: "pvcnamespace",
+		pvNameKey:       "pvname",
+	}
 	paramsWithZeroPermissions := map[string]string{
 		"server":              "server",
 		"share":               "share",
@@ -126,6 +133,16 @@ func TestNodePublishVolume(t *testing.T) {
 				Readonly:         true},
 			expectedErr: nil,
 		},
+		{
+			desc: "[Success] Valid request with pv/pvc metadata",
+			req: csi.NodePublishVolumeRequest{
+				VolumeContext:    paramsWithMetadata,
+				VolumeCapability: &csi.VolumeCapability{AccessMode: &volumeCap},
+				VolumeId:         "vol_1",
+				TargetPath:       targetTest,
+				Readonly:         true},
+			expectedErr: nil,
+		},
 		{
 			desc: "[Success] Valid request with 0 mountPermissions",
 			req: csi.NodePublishVolumeRequest{