Skip to content

Commit 187b281

Browse files
authored
Merge pull request #295 from andyzhangx/remove-createsubdir
feat: remove createSubDir parameter
2 parents aedd5ae + 5adf884 commit 187b281

File tree

10 files changed

+53
-66
lines changed

10 files changed

+53
-66
lines changed

deploy/example/e2e_usage.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,13 @@ metadata:
2121
provisioner: smb.csi.k8s.io
2222
parameters:
2323
source: "//smb-server.default.svc.cluster.local/share"
24+
# if csi.storage.k8s.io/provisioner-secret is provided, will create a sub directory
25+
# with PV name under source
26+
csi.storage.k8s.io/provisioner-secret-name: "smbcreds"
27+
csi.storage.k8s.io/provisioner-secret-namespace: "default"
2428
csi.storage.k8s.io/node-stage-secret-name: "smbcreds"
2529
csi.storage.k8s.io/node-stage-secret-namespace: "default"
26-
createSubDir: "false" # optional: create a sub dir for new volume
27-
reclaimPolicy: Retain # only retain is supported
30+
reclaimPolicy: Delete # available values: Delete, Retain
2831
volumeBindingMode: Immediate
2932
mountOptions:
3033
- dir_mode=0777
@@ -48,10 +51,13 @@ metadata:
4851
provisioner: smb.csi.k8s.io
4952
parameters:
5053
source: //52.146.58.223/share
54+
# if csi.storage.k8s.io/provisioner-secret is provided, will create a sub directory
55+
# with PV name under source
56+
csi.storage.k8s.io/provisioner-secret-name: "smbcreds"
57+
csi.storage.k8s.io/provisioner-secret-namespace: "default"
5158
csi.storage.k8s.io/node-stage-secret-name: "smbcreds"
5259
csi.storage.k8s.io/node-stage-secret-namespace: "default"
53-
createSubDir: "false" # optional: create a sub dir for new volume
54-
reclaimPolicy: Retain # only retain is supported
60+
reclaimPolicy: Delete # available values: Delete, Retain
5561
volumeBindingMode: Immediate
5662
mountOptions:
5763
- dir_mode=0777

deploy/example/storageclass-smb.yaml

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,12 @@ metadata:
55
name: smb
66
provisioner: smb.csi.k8s.io
77
parameters:
8-
# server address format for agent nodes with different OS:
9-
# Linux: "//server-address/share"
10-
# Windows: "\\server-address\share"
11-
# On Windows, "*.default.svc.cluster.local" could not be recognized by csi-proxy
8+
# On Windows, "*.default.svc.cluster.local" could not be recognized by csi-proxy
129
source: "//smb-server.default.svc.cluster.local/share"
13-
csi.storage.k8s.io/node-stage-secret-name: "smbcreds"
14-
csi.storage.k8s.io/node-stage-secret-namespace: "default"
1510
csi.storage.k8s.io/provisioner-secret-name: "smbcreds"
1611
csi.storage.k8s.io/provisioner-secret-namespace: "default"
17-
createSubDir: "true" # create a sub dir for new volume, "true" by default
12+
csi.storage.k8s.io/node-stage-secret-name: "smbcreds"
13+
csi.storage.k8s.io/node-stage-secret-namespace: "default"
1814
volumeBindingMode: Immediate
1915
mountOptions:
2016
- dir_mode=0777

docs/driver-parameters.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
Name | Meaning | Available Value | Mandatory | Default value
77
--- | --- | --- | --- | ---
88
source | Samba Server address | `//smb-server-address/sharename` </br>([Azure File](https://docs.microsoft.com/en-us/azure/storage/files/storage-files-introduction) format: `//accountname.file.core.windows.net/filesharename`) | Yes |
9-
createSubDir | create a sub dir for new volume | `true`, `false` | `false` |
109
csi.storage.k8s.io/node-stage-secret-name | secret name that stores `username`, `password`(`domain` is optional) | existing secret name | Yes |
1110
csi.storage.k8s.io/node-stage-secret-namespace | namespace where the secret is | k8s namespace | Yes |
1211

pkg/smb/controllerserver.go

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -74,44 +74,39 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
7474

7575
reqCapacity := req.GetCapacityRange().GetRequiredBytes()
7676
parameters := req.GetParameters()
77+
// Validate parameters (case-insensitive).
78+
for k := range parameters {
79+
switch strings.ToLower(k) {
80+
case paramSource:
81+
// no op
82+
default:
83+
return nil, fmt.Errorf("invalid parameter %s in storage class", k)
84+
}
85+
}
86+
7787
smbVol, err := d.newSMBVolume(name, reqCapacity, parameters)
7888
if err != nil {
7989
return nil, status.Error(codes.InvalidArgument, err.Error())
8090
}
8191

82-
// check if create SubDir is enable in storage class parameters
83-
createSubDir := true
84-
for k, v := range parameters {
85-
switch strings.ToLower(k) {
86-
case createSubDirField:
87-
if v == "false" {
88-
createSubDir = false
89-
}
90-
}
91-
}
92-
9392
secrets := req.GetSecrets()
94-
if createSubDir {
95-
if len(secrets) > 0 {
96-
// Mount smb base share so we can create a subdirectory
97-
if err := d.internalMount(ctx, smbVol, volCap, secrets); err != nil {
98-
return nil, status.Errorf(codes.Internal, "failed to mount smb server: %v", err.Error())
99-
}
100-
defer func() {
101-
if err = d.internalUnmount(ctx, smbVol); err != nil {
102-
klog.Warningf("failed to unmount smb server: %v", err.Error())
103-
}
104-
}()
105-
// Create subdirectory under base-dir
106-
// TODO: revisit permissions
107-
internalVolumePath := d.getInternalVolumePath(smbVol)
108-
if err = os.Mkdir(internalVolumePath, 0777); err != nil && !os.IsExist(err) {
109-
return nil, status.Errorf(codes.Internal, "failed to make subdirectory: %v", err.Error())
93+
if len(secrets) > 0 {
94+
// Mount smb base share so we can create a subdirectory
95+
if err := d.internalMount(ctx, smbVol, volCap, secrets); err != nil {
96+
return nil, status.Errorf(codes.Internal, "failed to mount smb server: %v", err.Error())
97+
}
98+
defer func() {
99+
if err = d.internalUnmount(ctx, smbVol); err != nil {
100+
klog.Warningf("failed to unmount smb server: %v", err.Error())
110101
}
111-
parameters[sourceField] = parameters[sourceField] + "/" + smbVol.subDir
112-
} else {
113-
klog.Warningf("CreateVolume: Volume secrets should be provided when createSubDir is true")
102+
}()
103+
// Create subdirectory under base-dir
104+
// TODO: revisit permissions
105+
internalVolumePath := d.getInternalVolumePath(smbVol)
106+
if err = os.Mkdir(internalVolumePath, 0777); err != nil && !os.IsExist(err) {
107+
return nil, status.Errorf(codes.Internal, "failed to make subdirectory: %v", err.Error())
114108
}
109+
parameters[sourceField] = parameters[sourceField] + "/" + smbVol.subDir
115110
}
116111
return &csi.CreateVolumeResponse{Volume: d.smbVolToCSI(smbVol, parameters)}, nil
117112
}
@@ -147,8 +142,6 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest)
147142
if err = os.RemoveAll(internalVolumePath); err != nil {
148143
return nil, status.Errorf(codes.Internal, "failed to delete subdirectory: %v", err.Error())
149144
}
150-
} else {
151-
klog.Warningf("DeleteVolume: Volume secrets should be provided")
152145
}
153146

154147
return &csi.DeleteVolumeResponse{}, nil

pkg/smb/controllerserver_test.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,7 @@ func TestCreateVolume(t *testing.T) {
9494
},
9595
},
9696
Parameters: map[string]string{
97-
paramSource: testServer,
98-
createSubDirField: "true",
97+
paramSource: testServer,
9998
},
10099
Secrets: map[string]string{
101100
usernameField: "test",
@@ -107,8 +106,7 @@ func TestCreateVolume(t *testing.T) {
107106
Volume: &csi.Volume{
108107
VolumeId: testVolumeID,
109108
VolumeContext: map[string]string{
110-
paramSource: filepath.Join(testServer, testCSIVolume),
111-
createSubDirField: "true",
109+
paramSource: filepath.Join(testServer, testCSIVolume),
112110
},
113111
},
114112
},

pkg/smb/smb.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import (
3030

3131
const (
3232
DefaultDriverName = "smb.csi.k8s.io"
33-
createSubDirField = "createsubdir"
3433
paramSource = "source"
3534
)
3635

test/e2e/dynamic_provisioning_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ var _ = ginkgo.Describe("Dynamic Provisioning", func() {
9090
Cmd: podCheckCmd,
9191
ExpectedString: expectedString,
9292
},
93-
StorageClassParameters: storageClassCreateSubDir,
93+
StorageClassParameters: defaultStorageClassParameters,
9494
RestartDriverFunc: func() {
9595
restartDriver := testCmd{
9696
command: "bash",
@@ -209,7 +209,7 @@ var _ = ginkgo.Describe("Dynamic Provisioning", func() {
209209
CSIDriver: testDriver,
210210
Pods: pods,
211211
ColocatePods: true,
212-
StorageClassParameters: storageClassCreateSubDir,
212+
StorageClassParameters: defaultStorageClassParameters,
213213
}
214214
test.Run(cs, ns)
215215
})
@@ -270,7 +270,7 @@ var _ = ginkgo.Describe("Dynamic Provisioning", func() {
270270
Cmd: podCheckCmd,
271271
ExpectedString: expectedString, // pod will be restarted so expect to see 2 instances of string
272272
},
273-
StorageClassParameters: storageClassCreateSubDir,
273+
StorageClassParameters: defaultStorageClassParameters,
274274
}
275275
test.Run(cs, ns)
276276
})
@@ -331,7 +331,7 @@ var _ = ginkgo.Describe("Dynamic Provisioning", func() {
331331
test := testsuites.DynamicallyProvisionedPodWithMultiplePVsTest{
332332
CSIDriver: testDriver,
333333
Pods: pods,
334-
StorageClassParameters: storageClassCreateSubDir,
334+
StorageClassParameters: defaultStorageClassParameters,
335335
}
336336
test.Run(cs, ns)
337337
})
@@ -355,7 +355,7 @@ var _ = ginkgo.Describe("Dynamic Provisioning", func() {
355355
test := testsuites.DynamicallyProvisionedVolumeSubpathTester{
356356
CSIDriver: testDriver,
357357
Pods: pods,
358-
StorageClassParameters: storageClassCreateSubDir,
358+
StorageClassParameters: defaultStorageClassParameters,
359359
}
360360
test.Run(cs, ns)
361361
})

test/e2e/suite_test.go

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,18 +48,11 @@ var (
4848
smbDriver *smb.Driver
4949
isWindowsCluster = os.Getenv(testWindowsEnvVar) != ""
5050
defaultStorageClassParameters = map[string]string{
51-
"source": "//smb-server.default.svc.cluster.local/share",
52-
"csi.storage.k8s.io/node-stage-secret-name": "smbcreds",
53-
"csi.storage.k8s.io/node-stage-secret-namespace": "default",
54-
"createSubDir": "false",
55-
}
56-
storageClassCreateSubDir = map[string]string{
5751
"source": "//smb-server.default.svc.cluster.local/share",
5852
"csi.storage.k8s.io/node-stage-secret-name": "smbcreds",
5953
"csi.storage.k8s.io/node-stage-secret-namespace": "default",
6054
"csi.storage.k8s.io/provisioner-secret-name": "smbcreds",
6155
"csi.storage.k8s.io/provisioner-secret-namespace": "default",
62-
"createSubDir": "true",
6356
}
6457
)
6558

@@ -134,7 +127,6 @@ var _ = ginkgo.BeforeSuite(func() {
134127

135128
log.Printf("use source on Windows: %v\n", source)
136129
defaultStorageClassParameters["source"] = source
137-
storageClassCreateSubDir["source"] = source
138130
}
139131
})
140132

test/integration/run-test.sh

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,7 @@ _output/${ARCH}/smbplugin --endpoint "$endpoint" --nodeid CSINode -v=5 &
5353
trap cleanup EXIT
5454

5555
sleep 5
56-
# set secret for csc node stage
57-
export X_CSI_SECRETS=username=username,"password=test"
58-
params='source=//0.0.0.0/share,createSubDir=false'
56+
params='source=//0.0.0.0/share'
5957
# Begin to run CSI functions one by one
6058
echo 'Create volume test:'
6159
readonly value=$("$CSC_BIN" controller new --endpoint "$endpoint" --cap 1,block "$volname" --req-bytes 2147483648 --params "$params")
@@ -64,6 +62,9 @@ sleep 2
6462
readonly volumeid=$(echo "$value" | awk '{print $1}' | sed 's/"//g')
6563
echo "Got volume id: $volumeid"
6664

65+
# set secret for csc node stage
66+
export X_CSI_SECRETS=username=username,"password=test"
67+
6768
echo "stage volume test:"
6869
"$CSC_BIN" node stage --endpoint "$endpoint" --cap 1,block --vol-context=source="//0.0.0.0/share" --staging-target-path "$staging_target_path" "$volumeid"
6970
sleep 2
@@ -87,6 +88,9 @@ echo "unstage volume test:"
8788
"$CSC_BIN" node unstage --endpoint "$endpoint" --staging-target-path "$staging_target_path" "$volumeid"
8889
sleep 2
8990

91+
# reset secrets as empty
92+
export X_CSI_SECRETS=""
93+
9094
echo 'Delete volume test:'
9195
"$CSC_BIN" controller del --endpoint "$endpoint" "$volumeid"
9296
sleep 2

test/sanity/params.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
source: "//127.0.0.1/share"
1+
source: "//127.0.0.1/share"

0 commit comments

Comments
 (0)