Skip to content

Commit

Permalink
Merge pull request #503 from andyzhangx/subdir-pvc-metadata
Browse files Browse the repository at this point in the history
feat: support pv/pvc metadata for subDir parameter
  • Loading branch information
k8s-ci-robot committed Jun 23, 2022
2 parents 8d38502 + b481a50 commit 7647407
Show file tree
Hide file tree
Showing 11 changed files with 140 additions and 10 deletions.
Binary file modified charts/latest/csi-driver-smb-v1.8.0.tgz
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ spec:
- "--csi-address=$(ADDRESS)"
- "--leader-election"
- "--leader-election-namespace={{ .Release.Namespace }}"
- "--extra-create-metadata=true"
env:
- name: ADDRESS
value: /csi/csi.sock
Expand Down
1 change: 1 addition & 0 deletions deploy/csi-smb-controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ spec:
- "--csi-address=$(ADDRESS)"
- "--leader-election"
- "--leader-election-namespace=kube-system"
- "--extra-create-metadata=true"
env:
- name: ADDRESS
value: /csi/csi.sock
Expand Down
6 changes: 6 additions & 0 deletions docs/driver-parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ kubectl create secret generic smbcreds --from-literal username=USERNAME --from-l
```

### Tips
#### `subDir` parameter supports following pv/pvc metadata transform
> if `subDir` value contains following string, 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`
- create a secret `smbcreds` with `mountOptions`
Expand Down
10 changes: 9 additions & 1 deletion pkg/smb/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ func getInternalMountPath(workingMountDir string, vol *smbVolume) string {
// Convert VolumeCreate parameters to an smbVolume
func newSMBVolume(name string, size int64, params map[string]string) (*smbVolume, error) {
var source, subDir string
subDirReplaceMap := map[string]string{}

// validate parameters (case-insensitive).
for k, v := range params {
Expand All @@ -310,6 +311,12 @@ func newSMBVolume(name string, size int64, params map[string]string) (*smbVolume
source = v
case subDirField:
subDir = v
case pvcNamespaceKey:
subDirReplaceMap[pvcNamespaceMetadata] = v
case pvcNameKey:
subDirReplaceMap[pvcNameMetadata] = v
case pvNameKey:
subDirReplaceMap[pvNameMetadata] = v
default:
return nil, fmt.Errorf("invalid parameter %s in storage class", k)
}
Expand All @@ -327,7 +334,8 @@ func newSMBVolume(name string, size int64, params map[string]string) (*smbVolume
// 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
}
Expand Down
19 changes: 19 additions & 0 deletions pkg/smb/controllerserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,6 +612,25 @@ func TestNewSMBVolume(t *testing.T) {
uuid: "pv-name",
},
},
{
desc: "subDir with pv/pvc metadata is specified",
name: "pv-name",
size: 100,
params: map[string]string{
"source": "//smb-server.default.svc.cluster.local/share",
"subDir": fmt.Sprintf("subdir-%s-%s-%s", pvcNameMetadata, pvcNamespaceMetadata, pvNameMetadata),
pvcNameKey: "pvcname",
pvcNamespaceKey: "pvcnamespace",
pvNameKey: "pvname",
},
expectVol: &smbVolume{
id: "smb-server.default.svc.cluster.local/share#subdir-pvcname-pvcnamespace-pvname#pv-name",
source: "//smb-server.default.svc.cluster.local/share",
subDir: "subdir-pvcname-pvcnamespace-pvname",
size: 100,
uuid: "pv-name",
},
},
{
desc: "subDir not specified",
name: "pv-name",
Expand Down
10 changes: 10 additions & 0 deletions pkg/smb/nodeserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,19 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
gidPresent := checkGidPresentInMountFlags(mountFlags)

var source, subDir string
subDirReplaceMap := map[string]string{}
for k, v := range context {
switch strings.ToLower(k) {
case sourceField:
source = v
case subDirField:
subDir = v
case pvcNamespaceKey:
subDirReplaceMap[pvcNamespaceMetadata] = v
case pvcNameKey:
subDirReplaceMap[pvcNameMetadata] = v
case pvNameKey:
subDirReplaceMap[pvNameMetadata] = v
}
}

Expand Down Expand Up @@ -205,6 +212,9 @@ func (d *Driver) NodeStageVolume(ctx context.Context, req *csi.NodeStageVolumeRe
return nil, fmt.Errorf("prepare stage path failed for %s with error: %v", targetPath, err)
}
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)
}
Expand Down
17 changes: 17 additions & 0 deletions pkg/smb/nodeserver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@ func TestNodeStageVolume(t *testing.T) {
volContext := map[string]string{
sourceField: "test_source",
}
volContextWithMetadata := map[string]string{
sourceField: "test_source",
pvcNameKey: "pvcname",
pvcNamespaceKey: "pvcnamespace",
pvNameKey: "pvname",
}
secrets := map[string]string{
usernameField: "test_username",
passwordField: "test_password",
Expand Down Expand Up @@ -167,6 +173,17 @@ func TestNodeStageVolume(t *testing.T) {
sourceTest),
expectedErr: testutil.TestError{},
},
{
desc: "[Success] Valid request with pv/pvc metadata",
req: csi.NodeStageVolumeRequest{VolumeId: "vol_1##", StagingTargetPath: sourceTest,
VolumeCapability: &stdVolCap,
VolumeContext: volContextWithMetadata,
Secrets: secrets},
flakyWindowsErrorMessage: fmt.Sprintf("volume(vol_1##) mount \"test_source\" on %#v failed with "+
"smb mapping failed with error: rpc error: code = Unknown desc = NewSmbGlobalMapping failed.",
sourceTest),
expectedErr: testutil.TestError{},
},
}

// Setup
Expand Down
32 changes: 24 additions & 8 deletions pkg/smb/smb.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,20 @@ import (
)

const (
DefaultDriverName = "smb.csi.k8s.io"
usernameField = "username"
passwordField = "password"
sourceField = "source"
subDirField = "subdir"
domainField = "domain"
mountOptionsField = "mountoptions"
defaultDomainName = "AZURE"
DefaultDriverName = "smb.csi.k8s.io"
usernameField = "username"
passwordField = "password"
sourceField = "source"
subDirField = "subdir"
domainField = "domain"
mountOptionsField = "mountoptions"
defaultDomainName = "AZURE"
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}"
)

// DriverOptions defines driver parameters specified in driver deployment
Expand Down Expand Up @@ -156,3 +162,13 @@ func setKeyValueInMap(m map[string]string, key, value string) {
}
m[key] = value
}

// 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
}
52 changes: 52 additions & 0 deletions pkg/smb/smb_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,55 @@ func TestSetKeyValueInMap(t *testing.T) {
}
}
}

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)
}
}
}
2 changes: 1 addition & 1 deletion test/e2e/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ var (
}
subDirStorageClassParameters = map[string]string{
"source": "//smb-server.default.svc.cluster.local/share",
"subDir": "subDirectory",
"subDir": "subDirectory-${pvc.metadata.name}",
"csi.storage.k8s.io/provisioner-secret-name": "smbcreds",
"csi.storage.k8s.io/provisioner-secret-namespace": "default",
"csi.storage.k8s.io/node-stage-secret-name": "smbcreds",
Expand Down

0 comments on commit 7647407

Please sign in to comment.