Skip to content

Commit

Permalink
feat: add subDir in storage class parameters
Browse files Browse the repository at this point in the history 
fix subdir

internal mount fix

mount subdir in NodePublishVolume

fix golint

fix

fix
  • Loading branch information
@andyzhangx
andyzhangx committed Jun 12, 2022
1 parent c1fe33f commit cfd8d86
Show file tree
Hide file tree
Showing 7 changed files with 139 additions and 47 deletions.
1 change: 1 addition & 0 deletions docs/driver-parameters.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
90 changes: 58 additions & 32 deletions pkg/nfs/controllerserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -64,6 +66,7 @@ const (
idServer = iota
idBaseDir
idSubDir
idUUID
totalIDElements // Always last
)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
}
Expand All @@ -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)
parameters[paramSubDir] = nfsVol.subDir
return &csi.CreateVolumeResponse{
Volume: &csi.Volume{
VolumeId: nfsVol.id,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{
Expand All @@ -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,
})
Expand All @@ -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 {
Expand All @@ -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
}
}

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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]
Expand All @@ -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
}

Expand Down
76 changes: 63 additions & 13 deletions pkg/nfs/controllerserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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",
},
},
Expand All @@ -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,
},
},
},
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
}
1 change: 1 addition & 0 deletions pkg/nfs/nfs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down
8 changes: 7 additions & 1 deletion pkg/nfs/nodeserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
Loading

0 comments on commit cfd8d86

Please sign in to comment.