Merge pull request #349 from andyzhangx/subDir

feat: add subDir in storage class parameters

Author: andyzhangx

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())
 	}
@@ -135,8 +140,7 @@ func (cs *ControllerServer) CreateVolume(ctx context.Context, req *csi.CreateVol
 		klog.Warningf("failed to chmod subdirectory: %v", err.Error())
 	}
 
-	parameters[paramServer] = nfsVol.server
-	parameters[paramShare] = cs.getVolumeSharePath(nfsVol)
+	setKeyValueInMap(parameters, paramSubDir, nfsVol.subDir)
 	return &csi.CreateVolumeResponse{
 		Volume: &csi.Volume{
 			VolumeId:      nfsVol.id,
@@ -183,7 +187,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 {
@@ -255,9 +259,6 @@ 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)
-
 	if volCap == nil {
 		volCap = &csi.VolumeCapability{
 			AccessType: &csi.VolumeCapability_Mount{
@@ -266,16 +267,24 @@ func (cs *ControllerServer) internalMount(ctx context.Context, vol *nfsVolume, v
 		}
 	}
 
-	if volumeContext == nil {
-		volumeContext = make(map[string]string)
+	sharePath := filepath.Join(string(filepath.Separator) + vol.baseDir)
+	targetPath := getInternalMountPath(cs.Driver.workingMountDir, vol)
+
+	volContext := map[string]string{
+		paramServer: vol.server,
+		paramShare:  sharePath,
+	}
+	for k, v := range volumeContext {
+		// don't set subDir field since only nfs-server:/share should be mounted in CreateVolume/DeleteVolume
+		if strings.ToLower(k) != paramSubDir {
+			volContext[k] = v
+		}
 	}
-	volumeContext[paramServer] = vol.server
-	volumeContext[paramShare] = sharePath
 
-	klog.V(2).Infof("internally mounting %v:%v at %v", vol.server, sharePath, targetPath)
+	klog.V(2).Infof("internally mounting %s:%s at %s", 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,13 +363,8 @@ 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)
-}
-
-// Get user-visible share path for the volume
-func (cs *ControllerServer) getVolumeSharePath(vol *nfsVolume) string {
-	return filepath.Join(string(filepath.Separator), vol.baseDir, vol.subDir)
+func getInternalVolumePath(workingMountDir string, vol *nfsVolume) string {
+	return filepath.Join(getInternalMountPath(workingMountDir, vol), vol.subDir)
 }
 
 // Given a nfsVolume, return a CSI volume id
@@ -354,22 +373,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 +401,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"
@@ -37,16 +38,12 @@ const (
 	testServer            = "test-server"
 	testBaseDir           = "test-base-dir"
 	testBaseDirNested     = "test/base/dir"
-	testCSIVolume         = "test-csi"
-	testVolumeID          = "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"
-)
-
-// for Windows support in the future
-var (
-	testShare = filepath.Join(string(filepath.Separator), testBaseDir, string(filepath.Separator), testCSIVolume)
+	testCSIVolume         = "volume-name"
+	testVolumeID          = "test-server/test-base-dir/volume-name"
+	newTestVolumeID       = "test-server#test-base-dir#volume-name#"
+	testVolumeIDNested    = "test-server/test/base/dir/volume-name"
+	newTestVolumeIDNested = "test-server#test/base/dir#volume-name#"
+	newTestVolumeIDUUID   = "test-server#test-base-dir#volume-name#uuid"
 )
 
 func initTestController(t *testing.T) *ControllerServer {
@@ -110,7 +107,8 @@ func TestCreateVolume(t *testing.T) {
 					VolumeId: newTestVolumeID,
 					VolumeContext: map[string]string{
 						paramServer:           testServer,
-						paramShare:            testShare,
+						paramShare:            testBaseDir,
+						paramSubDir:           testCSIVolume,
 						mountPermissionsField: "0750",
 					},
 				},
@@ -133,14 +131,16 @@ func TestCreateVolume(t *testing.T) {
 				Parameters: map[string]string{
 					paramServer: testServer,
 					paramShare:  testBaseDir,
+					paramSubDir: testCSIVolume,
 				},
 			},
 			resp: &csi.CreateVolumeResponse{
 				Volume: &csi.Volume{
-					VolumeId: newTestVolumeID,
+					VolumeId: newTestVolumeID + testCSIVolume,
 					VolumeContext: map[string]string{
 						paramServer: testServer,
-						paramShare:  testShare,
+						paramShare:  testBaseDir,
+						paramSubDir: testCSIVolume,
 					},
 				},
 			},
@@ -417,6 +417,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 +491,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 {