diff --git a/charts/latest/csi-driver-nfs-v4.1.0.tgz b/charts/latest/csi-driver-nfs-v4.1.0.tgz index 6ee357fa..b20cd3d2 100644 Binary files a/charts/latest/csi-driver-nfs-v4.1.0.tgz and b/charts/latest/csi-driver-nfs-v4.1.0.tgz differ diff --git a/charts/latest/csi-driver-nfs/templates/csi-nfs-controller.yaml b/charts/latest/csi-driver-nfs/templates/csi-nfs-controller.yaml index a734ff4e..5cba9758 100644 --- a/charts/latest/csi-driver-nfs/templates/csi-nfs-controller.yaml +++ b/charts/latest/csi-driver-nfs/templates/csi-nfs-controller.yaml @@ -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 diff --git a/deploy/csi-nfs-controller.yaml b/deploy/csi-nfs-controller.yaml index 28a0165c..3db5d834 100644 --- a/deploy/csi-nfs-controller.yaml +++ b/deploy/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 diff --git a/docs/driver-parameters.md b/docs/driver-parameters.md index bb702878..f2db0354 100644 --- a/docs/driver-parameters.md +++ b/docs/driver-parameters.md @@ -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` diff --git a/pkg/nfs/controllerserver.go b/pkg/nfs/controllerserver.go index c1147dfe..9015e7fb 100644 --- a/pkg/nfs/controllerserver.go +++ b/pkg/nfs/controllerserver.go @@ -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, "/") diff --git a/pkg/nfs/controllerserver_test.go b/pkg/nfs/controllerserver_test.go index 4b0a52f4..9f3c8c87 100644 --- a/pkg/nfs/controllerserver_test.go +++ b/pkg/nfs/controllerserver_test.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) + } + } +} diff --git a/pkg/nfs/nfs.go b/pkg/nfs/nfs.go index b45d707e..1ccd04c6 100644 --- a/pkg/nfs/nfs.go +++ b/pkg/nfs/nfs.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 +} diff --git a/pkg/nfs/nfs_test.go b/pkg/nfs/nfs_test.go index e1035832..c4e23f89 100644 --- a/pkg/nfs/nfs_test.go +++ b/pkg/nfs/nfs_test.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) + } + } +} diff --git a/pkg/nfs/nodeserver.go b/pkg/nfs/nodeserver.go index 0688c78a..e7aae1c7 100644 --- a/pkg/nfs/nodeserver.go +++ b/pkg/nfs/nodeserver.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) } diff --git a/pkg/nfs/nodeserver_test.go b/pkg/nfs/nodeserver_test.go index bc21caee..b4e62cb7 100644 --- a/pkg/nfs/nodeserver_test.go +++ b/pkg/nfs/nodeserver_test.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{ diff --git a/test/e2e/e2e_suite_test.go b/test/e2e/e2e_suite_test.go index 9c225f30..e0d402a4 100644 --- a/test/e2e/e2e_suite_test.go +++ b/test/e2e/e2e_suite_test.go @@ -62,7 +62,7 @@ var ( subDirStorageClassParameters = map[string]string{ "server": nfsServerAddress, "share": nfsShare, - "subDir": "subDirectory", + "subDir": "subDirectory-${pvc.metadata.namespace}", "csi.storage.k8s.io/provisioner-secret-name": "mount-options", "csi.storage.k8s.io/provisioner-secret-namespace": "default", "mountPermissions": "0755",