Skip to content

Commit 7b2dee7

Browse files
andyzhangxandyzhangx
authored
Merge pull request #616 from andyzhangx/delete-vol-issue
fix: delete volume failure when account is deleted
2 parents d93744f + 0b3833a commit 7b2dee7

11 files changed

+63
-56
lines changed

‎charts/README.md

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -98,17 +98,17 @@ The following table lists the configurable parameters of the latest Azure Blob S
9898
| `controller.runOnMaster` | run controller on master node | `true` |
9999
| `controller.logLevel` | controller driver log level | `5` |
100100
| `controller.resources.csiProvisioner.limits.memory` | csi-provisioner memory limits | 100Mi |
101-
| `controller.resources.csiProvisioner.requests.cpu` | csi-provisioner cpu requests limits | 10m |
102-
| `controller.resources.csiProvisioner.requests.memory` | csi-provisioner memory requests limits | 20Mi |
101+
| `controller.resources.csiProvisioner.requests.cpu` | csi-provisioner cpu requests | 10m |
102+
| `controller.resources.csiProvisioner.requests.memory` | csi-provisioner memory requests | 20Mi |
103103
| `controller.resources.livenessProbe.limits.memory` | liveness-probe memory limits | 300Mi |
104-
| `controller.resources.livenessProbe.requests.cpu` | liveness-probe cpu requests limits | 10m |
105-
| `controller.resources.livenessProbe.requests.memory` | liveness-probe memory requests limits | 20Mi |
104+
| `controller.resources.livenessProbe.requests.cpu` | liveness-probe cpu requests | 10m |
105+
| `controller.resources.livenessProbe.requests.memory` | liveness-probe memory requests | 20Mi |
106106
| `controller.resources.blob.limits.memory` | blob-csi-driver memory limits | 200Mi |
107-
| `controller.resources.blob.requests.cpu` | blob-csi-driver cpu requests limits | 10m |
108-
| `controller.resources.blob.requests.memory` | blob-csi-driver memory requests limits | 20Mi |
107+
| `controller.resources.blob.requests.cpu` | blob-csi-driver cpu requests | 10m |
108+
| `controller.resources.blob.requests.memory` | blob-csi-driver memory requests | 20Mi |
109109
| `controller.resources.csiResizer.limits.memory` | csi-resizer memory limits | 300Mi |
110-
| `controller.resources.csiResizer.requests.cpu` | csi-resizer cpu requests limits | 10m |
111-
| `controller.resources.csiResizer.requests.memory` | csi-resizer memory requests limits | 20Mi |
110+
| `controller.resources.csiResizer.requests.cpu` | csi-resizer cpu requests | 10m |
111+
| `controller.resources.csiResizer.requests.memory` | csi-resizer memory requests | 20Mi |
112112
| `controller.affinity` | controller pod affinity | {} |
113113
| `controller.nodeSelector` | controller pod node selector | {} |
114114
| `controller.tolerations` | controller pod tolerations | [] |
@@ -129,14 +129,14 @@ The following table lists the configurable parameters of the latest Azure Blob S
129129
| `node.blobfuseProxy.disableUpdateDB` | whether disable updateDB on blobfuse (saving storage account list usage) | `true` |
130130
| `node.blobfuseCachePath` | blobfuse cache path(`tmp-path`) | `/mnt` |
131131
| `node.resources.livenessProbe.limits.memory` | liveness-probe memory limits | 100Mi |
132-
| `node.resources.livenessProbe.requests.cpu` | liveness-probe cpu requests limits | 10m |
133-
| `node.resources.livenessProbe.requests.memory` | liveness-probe memory requests limits | 20Mi |
132+
| `node.resources.livenessProbe.requests.cpu` | liveness-probe cpu requests | 10m |
133+
| `node.resources.livenessProbe.requests.memory` | liveness-probe memory requests | 20Mi |
134134
| `node.resources.nodeDriverRegistrar.limits.memory` | csi-node-driver-registrar memory limits | 100Mi |
135-
| `node.resources.nodeDriverRegistrar.requests.cpu` | csi-node-driver-registrar cpu requests limits | 10m |
136-
| `node.resources.nodeDriverRegistrar.requests.memory` | csi-node-driver-registrar memory requests limits | 20Mi |
135+
| `node.resources.nodeDriverRegistrar.requests.cpu` | csi-node-driver-registrar cpu requests | 10m |
136+
| `node.resources.nodeDriverRegistrar.requests.memory` | csi-node-driver-registrar memory requests | 20Mi |
137137
| `node.resources.blob.limits.memory` | blob-csi-driver memory limits | 2100Mi |
138-
| `node.resources.blob.requests.cpu` | blob-csi-driver cpu requests limits | 10m |
139-
| `node.resources.blob.requests.memory` | blob-csi-driver memory requests limits | 20Mi |
138+
| `node.resources.blob.requests.cpu` | blob-csi-driver cpu requests | 10m |
139+
| `node.resources.blob.requests.memory` | blob-csi-driver memory requests | 20Mi |
140140
| `node.affinity` | node pod affinity | {} |
141141
| `node.nodeSelector` | node pod node selector | {} |
142142
| `node.tolerations` | node pod tolerations | [] |

‎deploy/example/storageclass-blob-nfs.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ provisioner: blob.csi.azure.com
77
parameters:
88
protocol: nfs
99
volumeBindingMode: Immediate
10+
allowVolumeExpansion: true

‎deploy/example/storageclass-blob-secret.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ kind: StorageClass
44
metadata:
55
name: blob
66
provisioner: blob.csi.azure.com
7+
allowVolumeExpansion: true
78
parameters:
89
csi.storage.k8s.io/provisioner-secret-name: azure-secret
910
csi.storage.k8s.io/provisioner-secret-namespace: default

‎deploy/example/storageclass-blobfuse-existing-container.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ parameters:
1111
server: SERVER_ADDRESS # optional, provide a new address to replace default "accountname.blob.core.windows.net"
1212
reclaimPolicy: Retain # if set as "Delete" container would be removed after pvc deletion
1313
volumeBindingMode: Immediate
14+
allowVolumeExpansion: true
1415
mountOptions:
1516
- -o allow_other
1617
- --file-cache-timeout-in-seconds=120

‎deploy/example/storageclass-blobfuse-readonly.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ parameters:
1010
containerName: EXISTING_CONTAINER_NAME
1111
reclaimPolicy: Retain # if set as "Delete" container would be removed after pvc deletion
1212
volumeBindingMode: Immediate
13+
allowVolumeExpansion: true
1314
mountOptions:
1415
- -o ro
1516
- -o allow_other

‎deploy/example/storageclass-blobfuse.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,10 @@ metadata:
55
name: blob
66
provisioner: blob.csi.azure.com
77
parameters:
8-
skuName: Standard_LRS # available values: Standard_LRS, Premium_LRS, Standard_GRS, Standard_RAGRS
8+
skuName: Premium_LRS # available values: Standard_LRS, Premium_LRS, Standard_GRS, Standard_RAGRS
99
reclaimPolicy: Delete
1010
volumeBindingMode: Immediate
11+
allowVolumeExpansion: true
1112
mountOptions:
1213
- -o allow_other
1314
- --file-cache-timeout-in-seconds=120

‎pkg/blob/blob.go

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,10 @@ const (
9090

9191
accountNotProvisioned = "StorageAccountIsNotProvisioned"
9292
tooManyRequests = "TooManyRequests"
93-
shareNotFound = "The specified share does not exist"
94-
shareBeingDeleted = "The specified share is being deleted"
9593
clientThrottled = "client throttled"
94+
containerBeingDeleted = "ContainerBeingDeleted"
95+
statusCodeNotFound = "StatusCode=404"
96+
httpCodeNotFound = "HTTPStatusCode: 404"
9697

9798
// containerMaxSize is the max size of the blob container. See https://docs.microsoft.com/en-us/azure/storage/blobs/scalability-targets#scale-targets-for-blob-storage
9899
containerMaxSize = 100 * util.TiB
@@ -106,7 +107,7 @@ const (
106107

107108
var (
108109
supportedProtocolList = []string{fuse, nfs}
109-
retriableErrors = []string{accountNotProvisioned, tooManyRequests, shareNotFound, shareBeingDeleted, clientThrottled}
110+
retriableErrors = []string{accountNotProvisioned, tooManyRequests, statusCodeNotFound, containerBeingDeleted, clientThrottled}
110111
)
111112

112113
// DriverOptions defines driver parameters specified in driver deployment
@@ -295,11 +296,11 @@ func isSASToken(key string) bool {
295296
}
296297

297298
// GetAuthEnv return <accountName, containerName, authEnv, error>
298-
func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attrib, secrets map[string]string) (string, string, []string, error) {
299+
func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attrib, secrets map[string]string) (string, string, string, string, []string, error) {
299300
rgName, accountName, containerName, err := GetContainerInfo(volumeID)
300301
if err != nil {
301302
// ignore volumeID parsing error
302-
klog.Warningf("parsing volumeID(%s) return with error: %v", volumeID, err)
303+
klog.V(2).Info("parsing volumeID(%s) return with error: %v", volumeID, err)
303304
err = nil
304305
}
305306

@@ -364,7 +365,7 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
364365

365366
if protocol == nfs {
366367
// nfs protocol does not need account key, return directly
367-
return accountName, containerName, authEnv, err
368+
return rgName, accountName, accountKey, containerName, authEnv, err
368369
}
369370

370371
// backward compatibility, old CSI driver PV does not have secretNamespace field
@@ -382,7 +383,7 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
382383
if keyVaultURL != "" {
383384
key, err := d.getKeyVaultSecretContent(ctx, keyVaultURL, keyVaultSecretName, keyVaultSecretVersion)
384385
if err != nil {
385-
return accountName, containerName, authEnv, err
386+
return rgName, accountName, accountKey, containerName, authEnv, err
386387
}
387388
if isSASToken(key) {
388389
accountSasToken = key
@@ -407,7 +408,7 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
407408
accountName, secretNamespace, secretName, err)
408409
accountKey, err = d.cloud.GetStorageAccesskey(ctx, accountName, rgName)
409410
if err != nil {
410-
return accountName, containerName, authEnv, fmt.Errorf("no key for storage account(%s) under resource group(%s), err %w", accountName, rgName, err)
411+
return rgName, accountName, accountKey, containerName, authEnv, fmt.Errorf("no key for storage account(%s) under resource group(%s), err %w", accountName, rgName, err)
411412
}
412413
}
413414
}
@@ -446,7 +447,7 @@ func (d *Driver) GetAuthEnv(ctx context.Context, volumeID, protocol string, attr
446447
authEnv = append(authEnv, "AZURE_STORAGE_ACCESS_KEY="+accountKey)
447448
}
448449

449-
return accountName, containerName, authEnv, err
450+
return rgName, accountName, accountKey, containerName, authEnv, err
450451
}
451452

452453
// GetStorageAccountAndContainer get storage account and container info

‎pkg/blob/blob_test.go

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -249,16 +249,6 @@ func TestIsRetriableError(t *testing.T) {
249249
rpcErr: errors.New("could not get storage key for storage account : could not list storage accounts for account type Premium_LRS: Retriable: true, RetryAfter: 0001-01-01 00:00:00 +0000 UTC m=+231.866923225, HTTPStatusCode: 429, RawError: storage.AccountsClient#ListByResourceGroup: Failure responding to request: StatusCode=429 -- Original Error: autorest/azure: Service returned an error. Status=429 Code=\"TooManyRequests\" Message=\"The request is being throttled as the limit has been reached for operation type - List. For more information, see - https://aka.ms/srpthrottlinglimits\""),
250250
expectedBool: true,
251251
},
252-
{
253-
desc: "shareNotFound",
254-
rpcErr: errors.New("storage.FileSharesClient#Get: Failure responding to request: StatusCode=404 -- Original Error: autorest/azure: Service returned an error. Status=404 Code=\"ShareNotFound\" Message=\"The specified share does not exist\""),
255-
expectedBool: true,
256-
},
257-
{
258-
desc: "shareBeingDeleted",
259-
rpcErr: errors.New("storage.FileSharesClient#Create: Failure sending request: StatusCode=409 -- Original Error: autorest/azure: Service returned an error. Status=<nil> Code=\"ShareBeingDeleted\" Message=\"The specified share is being deleted. Try operation later.\""),
260-
expectedBool: true,
261-
},
262252
{
263253
desc: "clientThrottled",
264254
rpcErr: errors.New("could not list storage accounts for account type : Retriable: true, RetryAfter: 16s, HTTPStatusCode: 0, RawError: azure cloud provider throttled for operation StorageAccountListByResourceGroup with reason \"client throttled\""),
@@ -471,7 +461,7 @@ func TestGetAuthEnv(t *testing.T) {
471461
RawError: fmt.Errorf("test"),
472462
}
473463
mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any()).Return(accountListKeysResult, rerr).AnyTimes()
474-
_, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
464+
_, _, _, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
475465
expectedErr := fmt.Errorf("no key for storage account(storageaccountname) under resource group(rg), err Retriable: false, RetryAfter: 0s, HTTPStatusCode: 0, RawError: test")
476466
if !strings.EqualFold(err.Error(), expectedErr.Error()) {
477467
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
@@ -500,7 +490,7 @@ func TestGetAuthEnv(t *testing.T) {
500490
Keys: &accountkeylist,
501491
}
502492
mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any()).Return(list, nil).AnyTimes()
503-
_, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
493+
_, _, _, _, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
504494
expectedErr := error(nil)
505495
if !reflect.DeepEqual(err, expectedErr) {
506496
t.Errorf("actualErr: (%v), expectedErr: (%v)", err, expectedErr)
@@ -521,11 +511,13 @@ func TestGetAuthEnv(t *testing.T) {
521511
secret["azurestorageaccountsastoken"] = "unit-test"
522512
secret["msisecret"] = "unit-test"
523513
secret["azurestoragespnclientsecret"] = "unit-test"
524-
accountName, containerName, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
514+
rg, accountName, accountkey, containerName, _, err := d.GetAuthEnv(context.TODO(), volumeID, "", attrib, secret)
525515
if err != nil {
526516
t.Errorf("actualErr: (%v), expectedErr: nil", err)
527517
}
518+
assert.Equal(t, rg, "rg")
528519
assert.Equal(t, accountName, "accountname")
520+
assert.Equal(t, accountkey, "unit-test")
529521
assert.Equal(t, containerName, "containername")
530522
},
531523
},
@@ -538,12 +530,14 @@ func TestGetAuthEnv(t *testing.T) {
538530
volumeID := "unique-volumeid"
539531
attrib[storageAccountField] = "accountname"
540532
attrib[containerNameField] = "containername"
541-
accountName, containerName, authEnv, err := d.GetAuthEnv(context.TODO(), volumeID, nfs, attrib, secret)
533+
rg, accountName, accountkey, containerName, authEnv, err := d.GetAuthEnv(context.TODO(), volumeID, nfs, attrib, secret)
542534
if err != nil {
543535
t.Errorf("actualErr: (%v), expect no error", err)
544536
}
545537

538+
assert.Equal(t, rg, "")
546539
assert.Equal(t, accountName, "accountname")
540+
assert.Equal(t, accountkey, "")
547541
assert.Equal(t, containerName, "containername")
548542
assert.Equal(t, len(authEnv), 0)
549543
},

0 commit comments

Comments
 (0)