feat: add subDir in storage class parameters

fix subdir

internal mount fix

docs/driver-parameters.md

@@ -8,6 +8,7 @@ Name | Meaning | Example Value | Mandatory | Default value
 --- | --- | --- | --- | ---
 server | NFS Server address | domain name `nfs-server.default.svc.cluster.local` <br>or IP address `127.0.0.1` | Yes |
 share | NFS share path | `/` | Yes |
+subDir | sub directory under nfs share |  | No | if sub directory does not exist, this driver would create a new one
 mountPermissions | mounted folder permissions. The default is `0777`, if set as `0`, driver will not perform `chmod` after mount |  | No |
 
 ### PV/PVC usage (static provisioning)

pkg/nfs/controllerserver.go

@@ -52,6 +52,8 @@ type nfsVolume struct {
 	subDir string
 	// size of volume
 	size int64
+	// pv name when subDir is not empty
+	uuid string
 }
 
 // Ordering of elements in the CSI volume id.
@@ -64,6 +66,7 @@ const (
 	idServer = iota
 	idBaseDir
 	idSubDir
+	idUUID
 	totalIDElements // Always last
 )
 
@@ -93,6 +96,8 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 			// no op
 		case paramShare:
 			// no op
+		case paramSubDir:
+			// no op
 		case mountPermissionsField:
 			if v != "" {
 				var err error
@@ -126,7 +131,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 
 	fileMode := os.FileMode(mountPermissions)
 	// Create subdirectory under base-dir
-	internalVolumePath := cs.getInternalVolumePath(nfsVol)
+	internalVolumePath := getInternalVolumePath(cs.Driver.workingMountDir, nfsVol)
 	if err = os.Mkdir(internalVolumePath, fileMode); err != nil && !os.IsExist(err) {
 		return nil, status.Errorf(codes.Internal, "failed to make subdirectory: %v", err.Error())
 	}
@@ -136,7 +141,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 	}
 
 	parameters[paramServer] = nfsVol.server
-	parameters[paramShare] = cs.getVolumeSharePath(nfsVol)
+	parameters[paramShare] = getVolumeSharePath(nfsVol)
 	return &csi.CreateVolumeResponse{
 		Volume: &csi.Volume{
 			VolumeId:      nfsVol.id,
@@ -183,7 +188,7 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
 	}()
 
 	// delete subdirectory under base-dir
-	internalVolumePath := cs.getInternalVolumePath(nfsVol)
+	internalVolumePath := getInternalVolumePath(cs.Driver.workingMountDir, nfsVol)
 
 	klog.V(2).Infof("Removing subdirectory at %v", internalVolumePath)
 	if err = os.RemoveAll(internalVolumePath); err != nil {
@@ -256,7 +261,7 @@ func (cs *ControllerServer) ControllerExpandVolume(ctx context.Context, req *csi
 // Mount nfs server at base-dir
 func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume, volumeContext map[string]string, volCap *csi.VolumeCapability) error {
 	sharePath := filepath.Join(string(filepath.Separator) + vol.baseDir)
-	targetPath := cs.getInternalMountPath(vol)
+	targetPath := getInternalMountPath(cs.Driver.workingMountDir, vol)
 
 	if volCap == nil {
 		volCap = &csi.VolumeCapability{
@@ -266,16 +271,20 @@ func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume, v
 		}
 	}
 
-	if volumeContext == nil {
-		volumeContext = make(map[string]string)
+	volContext := map[string]string{
+		paramServer: vol.server,
+		paramShare:  sharePath,
+	}
+	for k, v := range volumeContext {
+		volContext[k] = v
 	}
-	volumeContext[paramServer] = vol.server
-	volumeContext[paramShare] = sharePath
+	// reset subDir field since we only want to mount nfs-server:/share in CreateVolume
+	volContext[paramSubDir] = ""
 
 	klog.V(2).Infof("internally mounting %v:%v at %v", vol.server, sharePath, targetPath)
 	_, err := cs.Driver.ns.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{
 		TargetPath:       targetPath,
-		VolumeContext:    volumeContext,
+		VolumeContext:    volContext,
 		VolumeCapability: volCap,
 		VolumeId:         vol.id,
 	})
@@ -284,20 +293,20 @@ func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume, v
 
 // Unmount nfs server at base-dir
 func (cs *ControllerServer) internalUnmount(ctx context.Context, vol *nfsVolume) error {
-	targetPath := cs.getInternalMountPath(vol)
+	targetPath := getInternalMountPath(cs.Driver.workingMountDir, vol)
 
 	// Unmount nfs server at base-dir
 	klog.V(4).Infof("internally unmounting %v", targetPath)
 	_, err := cs.Driver.ns.NodeUnpublishVolume(ctx, &csi.NodeUnpublishVolumeRequest{
 		VolumeId:   vol.id,
-		TargetPath: cs.getInternalMountPath(vol),
+		TargetPath: targetPath,
 	})
 	return err
 }
 
 // Convert VolumeCreate parameters to an nfsVolume
 func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[string]string) (*nfsVolume, error) {
-	var server, baseDir string
+	var server, baseDir, subDir string
 
 	// validate parameters (case-insensitive)
 	for k, v := range params {
@@ -306,6 +315,8 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str
 			server = v
 		case paramShare:
 			baseDir = v
+		case paramSubDir:
+			subDir = v
 		}
 	}
 
@@ -319,17 +330,30 @@ func (cs *ControllerServer) newNFSVolume(name string, size int64, params map[str
 	vol := &nfsVolume{
 		server:  server,
 		baseDir: baseDir,
-		subDir:  name,
 		size:    size,
 	}
+	if subDir == "" {
+		// use pv name by default if not specified
+		vol.subDir = name
+	} else {
+		vol.subDir = subDir
+		// make volume id unique if subDir is provided
+		vol.uuid = name
+	}
 	vol.id = cs.getVolumeIDFromNfsVol(vol)
-
 	return vol, nil
 }
 
-// Get working directory for CreateVolume and DeleteVolume
-func (cs *ControllerServer) getInternalMountPath(vol *nfsVolume) string {
-	return filepath.Join(cs.Driver.workingMountDir, vol.subDir)
+// getInternalMountPath: get working directory for CreateVolume and DeleteVolume
+func getInternalMountPath(workingMountDir string, vol *nfsVolume) string {
+	if vol == nil {
+		return ""
+	}
+	mountDir := vol.uuid
+	if vol.uuid == "" {
+		mountDir = vol.subDir
+	}
+	return filepath.Join(workingMountDir, mountDir)
 }
 
 // Get internal path where the volume is created
@@ -339,12 +363,12 @@ func (cs *ControllerServer) getInternalMountPath(vol *nfsVolume) string {
 //     CreateVolume calls in parallel and they may use the same underlying share.
 //     Instead of refcounting how many CreateVolume calls are using the same
 //     share, it's simpler to just do a mount per request.
-func (cs *ControllerServer) getInternalVolumePath(vol *nfsVolume) string {
-	return filepath.Join(cs.getInternalMountPath(vol), vol.subDir)
+func getInternalVolumePath(workingMountDir string, vol *nfsVolume) string {
+	return filepath.Join(getInternalMountPath(workingMountDir, vol), vol.subDir)
 }
 
 // Get user-visible share path for the volume
-func (cs *ControllerServer) getVolumeSharePath(vol *nfsVolume) string {
+func getVolumeSharePath(vol *nfsVolume) string {
 	return filepath.Join(string(filepath.Separator), vol.baseDir, vol.subDir)
 }
 
@@ -354,22 +378,25 @@ func (cs *ControllerServer) getVolumeIDFromNfsVol(vol *nfsVolume) string {
 	idElements[idServer] = strings.Trim(vol.server, "/")
 	idElements[idBaseDir] = strings.Trim(vol.baseDir, "/")
 	idElements[idSubDir] = strings.Trim(vol.subDir, "/")
+	idElements[idUUID] = vol.uuid
 	return strings.Join(idElements, separator)
 }
 
 // Given a CSI volume id, return a nfsVolume
 // sample volume Id:
-//   new volumeID: nfs-server.default.svc.cluster.local#share#pvc-4bcbf944-b6f7-4bd0-b50f-3c3dd00efc64
+//   new volumeID:
+//	    nfs-server.default.svc.cluster.local#share#pvc-4bcbf944-b6f7-4bd0-b50f-3c3dd00efc64
+//	    nfs-server.default.svc.cluster.local#share#subdir#pvc-4bcbf944-b6f7-4bd0-b50f-3c3dd00efc64
 //   old volumeID: nfs-server.default.svc.cluster.local/share/pvc-4bcbf944-b6f7-4bd0-b50f-3c3dd00efc64
 func getNfsVolFromID(id string) (*nfsVolume, error) {
-	var server, baseDir, subDir string
+	var server, baseDir, subDir, uuid string
 	segments := strings.Split(id, separator)
 	if len(segments) < 3 {
 		klog.V(2).Infof("could not split %s into server, baseDir and subDir with separator(%s)", id, separator)
-		// try with separator "/""
+		// try with separator "/"
 		volRegex := regexp.MustCompile("^([^/]+)/(.*)/([^/]+)$")
 		tokens := volRegex.FindStringSubmatch(id)
-		if tokens == nil {
+		if tokens == nil || len(tokens) < 4 {
 			return nil, fmt.Errorf("could not split %s into server, baseDir and subDir with separator(%s)", id, "/")
 		}
 		server = tokens[1]
@@ -379,13 +406,17 @@ func getNfsVolFromID(id string) (*nfsVolume, error) {
 		server = segments[0]
 		baseDir = segments[1]
 		subDir = segments[2]
+		if len(segments) >= 4 {
+			uuid = segments[3]
+		}
 	}
 
 	return &nfsVolume{
 		id:      id,
 		server:  server,
 		baseDir: baseDir,
 		subDir:  subDir,
+		uuid:    uuid,
 	}, nil
 }
 

pkg/nfs/controllerserver_test.go

@@ -27,6 +27,7 @@ import (
 	"fmt"
 
 	"github.com/container-storage-interface/spec/lib/go/csi"
+	"github.com/stretchr/testify/assert"
 	"golang.org/x/net/context"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
@@ -39,9 +40,10 @@ const (
 	testBaseDirNested     = "test/base/dir"
 	testCSIVolume         = "test-csi"
 	testVolumeID          = "test-server/test-base-dir/test-csi"
-	newTestVolumeID       = "test-server#test-base-dir#test-csi"
+	newTestVolumeID       = "test-server#test-base-dir#test-csi#"
 	testVolumeIDNested    = "test-server/test/base/dir/test-csi"
-	newTestVolumeIDNested = "test-server#test/base/dir#test-csi"
+	newTestVolumeIDNested = "test-server#test/base/dir#test-csi#"
+	newTestVolumeIDUUID   = "test-server#test-base-dir#test-csi#uuid"
 )
 
 // for Windows support in the future
@@ -417,6 +419,18 @@ func TestNfsVolFromId(t *testing.T) {
 			},
 			expectErr: false,
 		},
+		{
+			name:     "valid request nested baseDir with newTestVolumeIDNested",
+			volumeID: newTestVolumeIDUUID,
+			resp: &nfsVolume{
+				id:      newTestVolumeIDUUID,
+				server:  testServer,
+				baseDir: testBaseDir,
+				subDir:  testCSIVolume,
+				uuid:    "uuid",
+			},
+			expectErr: false,
+		},
 	}
 
 	for _, test := range cases {
@@ -479,3 +493,41 @@ func TestIsValidVolumeCapabilities(t *testing.T) {
 		}
 	}
 }
+
+func TestGetInternalMountPath(t *testing.T) {
+	cases := []struct {
+		desc            string
+		workingMountDir string
+		vol             *nfsVolume
+		result          string
+	}{
+		{
+			desc:            "nil volume",
+			workingMountDir: "/tmp",
+			result:          "",
+		},
+		{
+			desc:            "uuid not empty",
+			workingMountDir: "/tmp",
+			vol: &nfsVolume{
+				subDir: "subdir",
+				uuid:   "uuid",
+			},
+			result: filepath.Join("/tmp", "uuid"),
+		},
+		{
+			desc:            "uuid empty",
+			workingMountDir: "/tmp",
+			vol: &nfsVolume{
+				subDir: "subdir",
+				uuid:   "",
+			},
+			result: filepath.Join("/tmp", "subdir"),
+		},
+	}
+
+	for _, test := range cases {
+		path := getInternalMountPath(test.workingMountDir, test.vol)
+		assert.Equal(t, path, test.result)
+	}
+}

pkg/nfs/nfs.go

@@ -55,6 +55,7 @@ const (
 	// The root directory is omitted from the string, for example:
 	//     "base" instead of "/base"
 	paramShare            = "share"
+	paramSubDir           = "subdir"
 	mountOptionsField     = "mountoptions"
 	mountPermissionsField = "mountpermissions"
 )

pkg/nfs/nodeserver.go

@@ -56,7 +56,7 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 		mountOptions = append(mountOptions, "ro")
 	}
 
-	var server, baseDir string
+	var server, baseDir, subDir string
 	mountPermissions := ns.Driver.mountPermissions
 	performChmodOp := (mountPermissions > 0)
 	for k, v := range req.GetVolumeContext() {
@@ -65,6 +65,8 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 			server = v
 		case paramShare:
 			baseDir = v
+		case paramSubDir:
+			subDir = v
 		case mountOptionsField:
 			if v != "" {
 				mountOptions = append(mountOptions, v)
@@ -93,6 +95,10 @@ func (ns *NodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	}
 	server = getServerFromSource(server)
 	source := fmt.Sprintf("%s:%s", server, baseDir)
+	if subDir != "" {
+		source = strings.TrimRight(source, "/")
+		source = fmt.Sprintf("%s/%s", source, subDir)
+	}
 
 	notMnt, err := ns.mounter.IsLikelyNotMountPoint(targetPath)
 	if err != nil {

test/e2e/dynamic_provisioning_test.go

@@ -242,7 +242,7 @@ var _ = ginkgo.Describe("Dynamic Provisioning", func() {
 		test := testsuites.DynamicallyProvisionedPodWithMultiplePVsTest{
 			CSIDriver:              testDriver,
 			Pods:                   pods,
-			StorageClassParameters: defaultStorageClassParameters,
+			StorageClassParameters: subDirStorageClassParameters,
 		}
 		test.Run(cs, ns)
 	})

test/e2e/e2e_suite_test.go

@@ -59,6 +59,14 @@ var (
 		"csi.storage.k8s.io/provisioner-secret-namespace": "default",
 		"mountPermissions": "0",
 	}
+	subDirStorageClassParameters = map[string]string{
+		"server": nfsServerAddress,
+		"share":  nfsShare,
+		"subDir": "subDirectory",
+		"csi.storage.k8s.io/provisioner-secret-name":      "mount-options",
+		"csi.storage.k8s.io/provisioner-secret-namespace": "default",
+		"mountPermissions": "0755",
+	}
 	controllerServer *nfs.ControllerServer
 )