feat: add subDir in storage class parameters

fix subdir

internal mount fix

mount subdir in NodePublishVolume

fix golint

fix

fix

setKeyValueInMap
This commit is contained in:
andyzhangx 2022-06-11 14:55:29 +00:00
parent c1fe33f456
commit 16c442ad78
9 changed files with 209 additions and 48 deletions

View File

@ -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)

View File

@ -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)
}
volumeContext[paramServer] = vol.server
volumeContext[paramShare] = sharePath
sharePath := filepath.Join(string(filepath.Separator) + vol.baseDir)
targetPath := getInternalMountPath(cs.Driver.workingMountDir, vol)
klog.V(2).Infof("internally mounting %v:%v at %v", vol.server, sharePath, targetPath)
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
}
}
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,6 +401,9 @@ func getNfsVolFromID(id string) (*nfsVolume, error) {
server = segments[0]
baseDir = segments[1]
subDir = segments[2]
if len(segments) >= 4 {
uuid = segments[3]
}
}
return &nfsVolume{
@ -386,6 +411,7 @@ func getNfsVolFromID(id string) (*nfsVolume, error) {
server: server,
baseDir: baseDir,
subDir: subDir,
uuid: uuid,
}, nil
}

View File

@ -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)
}
}

View File

@ -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"
)

View File

@ -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 {

View File

@ -160,3 +160,18 @@ func getServerFromSource(server string) string {
}
return server
}
// setKeyValueInMap set key/value pair in map
// key in the map is case insensitive, if key already exists, overwrite existing value
func setKeyValueInMap(m map[string]string, key, value string) {
if m == nil {
return
}
for k := range m {
if strings.EqualFold(k, key) {
m[k] = value
return
}
}
m[key] = value
}

View File

@ -247,3 +247,57 @@ func TestGetServerFromSource(t *testing.T) {
}
}
}
func TestSetKeyValueInMap(t *testing.T) {
tests := []struct {
desc string
m map[string]string
key string
value string
expected map[string]string
}{
{
desc: "nil map",
key: "key",
value: "value",
},
{
desc: "empty map",
m: map[string]string{},
key: "key",
value: "value",
expected: map[string]string{"key": "value"},
},
{
desc: "non-empty map",
m: map[string]string{"k": "v"},
key: "key",
value: "value",
expected: map[string]string{
"k": "v",
"key": "value",
},
},
{
desc: "same key already exists",
m: map[string]string{"subDir": "value2"},
key: "subDir",
value: "value",
expected: map[string]string{"subDir": "value"},
},
{
desc: "case insentive key already exists",
m: map[string]string{"subDir": "value2"},
key: "subdir",
value: "value",
expected: map[string]string{"subDir": "value"},
},
}
for _, test := range tests {
setKeyValueInMap(test.m, test.key, test.value)
if !reflect.DeepEqual(test.m, test.expected) {
t.Errorf("test[%s]: unexpected output: %v, expected result: %v", test.desc, test.m, test.expected)
}
}
}

View File

@ -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)
})

View File

@ -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
)