From ebe87bc3719e611eca8057a01d6d387b947e4ebc Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Tue, 15 Aug 2023 07:18:02 +0000 Subject: [PATCH] feat: add accountQuota parameter --- docs/driver-parameters.md | 1 + go.mod | 2 +- pkg/azurefile/azurefile.go | 17 ++ pkg/azurefile/azurefile_test.go | 76 ++++++++ pkg/azurefile/controllerserver.go | 31 ++- pkg/azurefile/controllerserver_test.go | 254 ++++--------------------- 6 files changed, 156 insertions(+), 225 deletions(-) diff --git a/docs/driver-parameters.md b/docs/driver-parameters.md index b4912cd719..bc2ea49df8 100644 --- a/docs/driver-parameters.md +++ b/docs/driver-parameters.md @@ -25,6 +25,7 @@ requireInfraEncryption | specify whether or not the service applies a secondary storageEndpointSuffix | specify Azure storage endpoint suffix | `core.windows.net`, `core.chinacloudapi.cn`, etc | No | if empty, driver will use default storage endpoint suffix according to cloud environment, e.g. `core.windows.net` tags | [tags](https://docs.microsoft.com/en-us/azure/azure-resource-manager/management/tag-resources) would be created in newly created storage account | tag format: 'foo=aaa,bar=bbb' | No | "" matchTags | whether matching tags when driver tries to find a suitable storage account | `true`,`false` | No | `false` +accountQuota | to limit the quota for an account, you can specify a maximum quota in GB (`102400`GB by default). If the account exceeds the specified quota, the driver would skip selecting the account | `` | No | `102400` --- | **Following parameters are only for SMB protocol** | --- | --- | subscriptionID | specify Azure subscription ID in which Azure file share will be created | Azure subscription ID | No | if not empty, `resourceGroup` must be provided storeAccountKey | whether store account key to k8s secret

Note:
`false` means driver would leverage kubelet identity to get account key | `true`,`false` | No | `true` diff --git a/go.mod b/go.mod index 86ee46e8cd..7c0308065f 100644 --- a/go.mod +++ b/go.mod @@ -7,7 +7,7 @@ require ( github.com/Azure/azure-storage-file-go v0.8.0 github.com/Azure/go-autorest/autorest v0.11.28 github.com/Azure/go-autorest/autorest/adal v0.9.21 // indirect - github.com/Azure/go-autorest/autorest/to v0.4.0 // indirect + github.com/Azure/go-autorest/autorest/to v0.4.0 github.com/container-storage-interface/spec v1.7.0 github.com/gofrs/uuid v4.2.0+incompatible // indirect github.com/golang/mock v1.6.0 diff --git a/pkg/azurefile/azurefile.go b/pkg/azurefile/azurefile.go index e635192ab4..d8e255e557 100644 --- a/pkg/azurefile/azurefile.go +++ b/pkg/azurefile/azurefile.go @@ -72,6 +72,7 @@ const ( // Minimum size of Azure Premium Files is 100GiB // See https://docs.microsoft.com/en-us/azure/storage/files/storage-files-planning#provisioned-shares defaultAzureFileQuota = 100 + minimumAccountQuota = 100 // GB // key of snapshot name in metadata snapshotNameKey = "initiator" @@ -131,6 +132,7 @@ const ( shareNamePrefixField = "sharenameprefix" requireInfraEncryptionField = "requireinfraencryption" enableMultichannelField = "enablemultichannel" + accountQuotaField = "accountquota" premium = "premium" accountNotProvisioned = "StorageAccountIsNotProvisioned" @@ -850,6 +852,21 @@ func (d *Driver) ResizeFileShare(ctx context.Context, subsID, resourceGroup, acc }) } +// GetTotalAccountQuota returns the total quota in GB of all file shares in the storage account and the number of file shares +func (d *Driver) GetTotalAccountQuota(ctx context.Context, subsID, resourceGroup, accountName string) (int32, int32, error) { + fileshares, err := d.cloud.FileClient.WithSubscriptionID(subsID).ListFileShare(ctx, resourceGroup, accountName, "", "") + if err != nil { + return -1, -1, err + } + var totalQuotaGB int32 + for _, fs := range fileshares { + if fs.ShareQuota != nil { + totalQuotaGB += *fs.ShareQuota + } + } + return totalQuotaGB, int32(len(fileshares)), nil +} + // RemoveStorageAccountTag remove tag from storage account func (d *Driver) RemoveStorageAccountTag(ctx context.Context, subsID, resourceGroup, account, key string) error { // search in cache first diff --git a/pkg/azurefile/azurefile_test.go b/pkg/azurefile/azurefile_test.go index 572e657faf..a3f562874b 100644 --- a/pkg/azurefile/azurefile_test.go +++ b/pkg/azurefile/azurefile_test.go @@ -30,6 +30,8 @@ import ( "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage" azure2 "github.com/Azure/go-autorest/autorest/azure" + "github.com/Azure/go-autorest/autorest/to" + "github.com/container-storage-interface/spec/lib/go/csi" "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "k8s.io/client-go/kubernetes/fake" @@ -64,6 +66,11 @@ func NewFakeDriver() *Driver { }, }, } + driver.AddControllerServiceCapabilities( + []csi.ControllerServiceCapability_RPC_Type{ + csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, + csi.ControllerServiceCapability_RPC_EXPAND_VOLUME, + }) return driver } @@ -79,6 +86,10 @@ func NewFakeDriverCustomOptions(opts DriverOptions) *Driver { }, }, } + driver.AddControllerServiceCapabilities( + []csi.ControllerServiceCapability_RPC_Type{ + csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, + }) return driver } @@ -1249,3 +1260,68 @@ func TestGetSubnetResourceID(t *testing.T) { t.Run(tc.name, tc.testFunc) } } + +func TestGetTotalAccountQuota(t *testing.T) { + d := NewFakeDriver() + d.cloud = &azure.Cloud{} + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + fileShareItemsWithQuota := []storage.FileShareItem{ + { + FileShareProperties: &storage.FileShareProperties{ + ShareQuota: to.Int32Ptr(100), + }, + }, + { + FileShareProperties: &storage.FileShareProperties{ + ShareQuota: to.Int32Ptr(200), + }, + }, + } + + tests := []struct { + name string + subsID string + resourceGroup string + accountName string + fileShareItems []storage.FileShareItem + listFileShareErr error + expectedQuota int32 + expectedShareNum int32 + expectedErr error + }{ + { + name: "GetTotalAccountQuota success", + expectedQuota: 0, + expectedShareNum: 0, + }, + { + name: "GetTotalAccountQuota success (with 2 file shares)", + fileShareItems: fileShareItemsWithQuota, + expectedQuota: 300, + expectedShareNum: 2, + }, + { + name: "list file share error", + listFileShareErr: fmt.Errorf("list file share error"), + expectedQuota: -1, + expectedShareNum: -1, + expectedErr: fmt.Errorf("list file share error"), + }, + } + + for _, test := range tests { + mockFileClient := mockfileclient.NewMockInterface(ctrl) + d.cloud.FileClient = mockFileClient + + mockFileClient.EXPECT().WithSubscriptionID(gomock.Any()).Return(mockFileClient).AnyTimes() + mockFileClient.EXPECT().ListFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(test.fileShareItems, test.listFileShareErr).AnyTimes() + + quota, fileShareNum, err := d.GetTotalAccountQuota(context.Background(), test.subsID, test.resourceGroup, test.accountName) + assert.Equal(t, test.expectedErr, err, test.name) + assert.Equal(t, test.expectedQuota, quota, test.name) + assert.Equal(t, test.expectedShareNum, fileShareNum, test.name) + } +} diff --git a/pkg/azurefile/controllerserver.go b/pkg/azurefile/controllerserver.go index 0d8510ba30..b998cf3e6c 100644 --- a/pkg/azurefile/controllerserver.go +++ b/pkg/azurefile/controllerserver.go @@ -77,6 +77,7 @@ var ( Mode: csi.VolumeCapability_AccessMode_MULTI_NODE_MULTI_WRITER, }, } + skipMatchingTag = map[string]*string{azure.SkipMatchingTag: pointer.String("")} ) // CreateVolume provisions an azure file @@ -123,6 +124,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) // store account key to k8s secret by default storeAccountKey := true + var accountQuota int32 // Apply ProvisionerParameters (case-insensitive). We leave validation of // the values to the cloud provider. for k, v := range parameters { @@ -233,6 +235,12 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid %s: %s in storage class", enableMultichannelField, v)) } isMultichannelEnabled = &value + case accountQuotaField: + value, err := strconv.ParseInt(v, 10, 32) + if err != nil || value < minimumAccountQuota { + return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid accountQuota %s in storage class, minimum quota: %d", v, minimumAccountQuota)) + } + accountQuota = int32(value) default: return nil, status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid parameter %q in storage class", k)) } @@ -438,6 +446,22 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) if err != nil { return nil, status.Errorf(codes.Internal, "failed to ensure storage account: %v", err) } + if accountQuota > minimumAccountQuota { + totalQuotaGB, fileshareNum, err := d.GetTotalAccountQuota(ctx, subsID, resourceGroup, accountName) + if err != nil { + return nil, status.Errorf(codes.Internal, "failed to get total quota on account(%s), error: %v", accountName, err) + } + klog.V(2).Infof("total used quota on account(%s) is %d GB, file share number: %d", accountName, totalQuotaGB, fileshareNum) + if totalQuotaGB > accountQuota { + klog.Warningf("account(%s) used quota(%d GB) is over %d GB, skip matching current account", accountName, accountQuota, totalQuotaGB) + if rerr := d.cloud.AddStorageAccountTags(ctx, subsID, resourceGroup, accountName, skipMatchingTag); rerr != nil { + klog.Warningf("AddStorageAccountTags(%v) on account(%s) subsID(%s) rg(%s) failed with error: %v", tags, accountName, subsID, resourceGroup, rerr.Error()) + } + // release volume lock first to prevent deadlock + d.volumeLocks.Release(volName) + return d.CreateVolume(ctx, req) + } + } d.accountSearchCache.Set(lockKey, accountName) d.volMap.Store(volName, accountName) if accountKey != "" { @@ -488,10 +512,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) if err := d.CreateFileShare(ctx, accountOptions, shareOptions, secret); err != nil { if strings.Contains(err.Error(), accountLimitExceedManagementAPI) || strings.Contains(err.Error(), accountLimitExceedDataPlaneAPI) { klog.Warningf("create file share(%s) on account(%s) type(%s) subID(%s) rg(%s) location(%s) size(%d), error: %v, skip matching current account", validFileShareName, accountName, sku, subsID, resourceGroup, location, fileShareSize, err) - tags := map[string]*string{ - azure.SkipMatchingTag: pointer.String(""), - } - if rerr := d.cloud.AddStorageAccountTags(ctx, subsID, resourceGroup, accountName, tags); rerr != nil { + if rerr := d.cloud.AddStorageAccountTags(ctx, subsID, resourceGroup, accountName, skipMatchingTag); rerr != nil { klog.Warningf("AddStorageAccountTags(%v) on account(%s) subsID(%s) rg(%s) failed with error: %v", tags, accountName, subsID, resourceGroup, rerr.Error()) } // release volume lock first to prevent deadlock @@ -500,7 +521,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) if err := d.accountSearchCache.Delete(lockKey); err != nil { return nil, status.Errorf(codes.Internal, err.Error()) } - // remove the volName from the volMap to stop it matching the same storage account + // remove the volName from the volMap to stop matching the same storage account d.volMap.Delete(volName) return d.CreateVolume(ctx, req) } diff --git a/pkg/azurefile/controllerserver_test.go b/pkg/azurefile/controllerserver_test.go index defcb529c3..83855e060a 100644 --- a/pkg/azurefile/controllerserver_test.go +++ b/pkg/azurefile/controllerserver_test.go @@ -69,6 +69,7 @@ func TestCreateVolume(t *testing.T) { stdCapRange := &csi.CapacityRange{RequiredBytes: stdVolSize} zeroCapRange := &csi.CapacityRange{RequiredBytes: int64(0)} lessThanPremCapRange := &csi.CapacityRange{RequiredBytes: int64(fakeShareQuota * 1024 * 1024 * 1024)} + ctx := context.Background() testCases := []struct { name string @@ -84,8 +85,8 @@ func TestCreateVolume(t *testing.T) { Parameters: nil, } - ctx := context.Background() d := NewFakeDriver() + d.Cap = []*csi.ControllerServiceCapability{} expectedErr := status.Errorf(codes.InvalidArgument, "CREATE_DELETE_VOLUME") _, err := d.CreateVolume(ctx, req) @@ -103,13 +104,7 @@ func TestCreateVolume(t *testing.T) { VolumeCapabilities: stdVolCap, Parameters: nil, } - - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) expectedErr := status.Error(codes.InvalidArgument, "CreateVolume Name must be provided") _, err := d.CreateVolume(ctx, req) @@ -127,12 +122,7 @@ func TestCreateVolume(t *testing.T) { Parameters: nil, } - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) expectedErr := status.Error(codes.InvalidArgument, "CreateVolume Volume capabilities not valid: CreateVolume Volume capabilities must be provided") _, err := d.CreateVolume(ctx, req) @@ -160,12 +150,7 @@ func TestCreateVolume(t *testing.T) { Parameters: nil, } - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) expectedErr := status.Error(codes.InvalidArgument, "CreateVolume Volume capabilities not valid: driver does not support block volumes") _, err := d.CreateVolume(ctx, req) @@ -184,17 +169,11 @@ func TestCreateVolume(t *testing.T) { Parameters: nil, } - ctx := context.Background() d := NewFakeDriver() locks := newVolumeLocks() locks.locks.Insert(req.GetName()) d.volumeLocks = locks - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Error(codes.Aborted, "An operation with the given Volume ID random-vol-name-vol-cap-invalid already exists") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -218,8 +197,6 @@ func TestCreateVolume(t *testing.T) { Parameters: allParam, } - ctx := context.Background() - driverOptions := DriverOptions{ NodeID: fakeNodeID, DriverName: DefaultDriverName, @@ -227,11 +204,6 @@ func TestCreateVolume(t *testing.T) { } d := NewFakeDriverCustomOptions(driverOptions) - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "fsType storage class parameter enables experimental VDH disk feature which is currently disabled, use --enable-vhd driver option to enable it") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -253,8 +225,6 @@ func TestCreateVolume(t *testing.T) { Parameters: allParam, } - ctx := context.Background() - driverOptions := DriverOptions{ NodeID: fakeNodeID, DriverName: DefaultDriverName, @@ -262,11 +232,6 @@ func TestCreateVolume(t *testing.T) { } d := NewFakeDriverCustomOptions(driverOptions) - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "fsType(test_fs) is not supported, supported fsType list: [cifs smb nfs ext4 ext3 ext2 xfs]") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -288,14 +253,8 @@ func TestCreateVolume(t *testing.T) { Parameters: allParam, } - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "protocol(test_protocol) is not supported, supported protocol list: [smb nfs]") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -318,14 +277,8 @@ func TestCreateVolume(t *testing.T) { Parameters: allParam, } - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "shareAccessTier(test_accessTier) is not supported, supported ShareAccessTier list: [Cool Hot Premium TransactionOptimized]") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -347,14 +300,8 @@ func TestCreateVolume(t *testing.T) { Parameters: allParam, } - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "rootSquashType(test_rootSquashType) is not supported, supported RootSquashType list: [AllSquash NoRootSquash RootSquash]") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -376,14 +323,8 @@ func TestCreateVolume(t *testing.T) { Parameters: allParam, } - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "fsGroupChangePolicy(test_fsGroupChangePolicy) is not supported, supported fsGroupChangePolicy list: [None Always OnRootMismatch]") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -404,15 +345,8 @@ func TestCreateVolume(t *testing.T) { VolumeCapabilities: stdVolCap, Parameters: allParam, } - - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "shareNamePrefix(-invalid) can only contain lowercase letters, numbers, hyphens, and length should be less than 21") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -420,6 +354,29 @@ func TestCreateVolume(t *testing.T) { } }, }, + { + name: "Invalid accountQuota", + testFunc: func(t *testing.T) { + allParam := map[string]string{ + accountQuotaField: "10", + } + + req := &csi.CreateVolumeRequest{ + Name: "random-vol-name-vol-cap-invalid", + CapacityRange: stdCapRange, + VolumeCapabilities: stdVolCap, + Parameters: allParam, + } + + d := NewFakeDriver() + + expectedErr := status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid accountQuota %d in storage class, minimum quota: %d", 10, minimumAccountQuota)) + _, err := d.CreateVolume(ctx, req) + if !reflect.DeepEqual(err, expectedErr) { + t.Errorf("Unexpected error: %v", err) + } + }, + }, { name: "invalid tags format to convert to map", testFunc: func(t *testing.T) { @@ -443,14 +400,8 @@ func TestCreateVolume(t *testing.T) { Parameters: allParam, } - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, fmt.Errorf("Tags 'tags' are invalid, the format should like: 'key1=value1,key2=value2'").Error()) _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -483,18 +434,12 @@ func TestCreateVolume(t *testing.T) { Parameters: allParam, } - ctx := context.Background() d := NewFakeDriver() d.cloud = fakeCloud d.volMap = sync.Map{} d.volMap.Store("random-vol-name-vol-cap-invalid", "account") d.fileClient = &azureFileClient{} - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.Internal, "failed to GetStorageAccesskey on account(account) rg(), error: StorageAccountClient is nil") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -517,8 +462,6 @@ func TestCreateVolume(t *testing.T) { Parameters: allParam, } - ctx := context.Background() - driverOptions := DriverOptions{ NodeID: fakeNodeID, DriverName: DefaultDriverName, @@ -526,11 +469,6 @@ func TestCreateVolume(t *testing.T) { } d := NewFakeDriverCustomOptions(driverOptions) - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "fsType(ext4) is not supported with protocol(nfs)") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -558,13 +496,8 @@ func TestCreateVolume(t *testing.T) { Config: azure.Config{}, } - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "resourceGroup must be provided in cross subscription(abc)") - _, err := d.CreateVolume(context.Background(), req) + _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -590,13 +523,8 @@ func TestCreateVolume(t *testing.T) { Config: azure.Config{}, } - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.InvalidArgument, "matchTags must set as false when storageAccount(abc) is provided") - _, err := d.CreateVolume(context.Background(), req) + _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -625,7 +553,6 @@ func TestCreateVolume(t *testing.T) { VolumeCapabilities: stdVolCap, Parameters: allParam, } - ctx := context.Background() d := NewFakeDriver() d.cloud = fakeCloud @@ -636,11 +563,6 @@ func TestCreateVolume(t *testing.T) { mockSubnetClient.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(network.Subnet{}, retErr).Times(1) - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - expectedErr := status.Errorf(codes.Internal, "update service endpoints failed with error: failed to get the subnet fake-subnet under vnet fake-vnet: &{false 0 0001-01-01 00:00:00 +0000 UTC the subnet does not exist}") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -708,12 +630,6 @@ func TestCreateVolume(t *testing.T) { mockFileClient.EXPECT().GetServiceProperties(context.TODO(), gomock.Any(), gomock.Any()).Return(fileServiceProperties, nil).AnyTimes() mockFileClient.EXPECT().SetServiceProperties(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any()).Return(fileServiceProperties, nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() expectedErr := fmt.Errorf("no valid keys") _, err := d.CreateVolume(ctx, req) @@ -776,12 +692,6 @@ func TestCreateVolume(t *testing.T) { mockStorageAccountsClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any()).Return(accounts, nil).AnyTimes() mockStorageAccountsClient.EXPECT().Create(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() expectedErr := fmt.Errorf("no valid keys") _, err := d.CreateVolume(ctx, req) @@ -832,13 +742,8 @@ func TestCreateVolume(t *testing.T) { mockStorageAccountsClient.EXPECT().ListByResourceGroup(gomock.Any(), gomock.Any(), gomock.Any()).Return(accounts, nil).AnyTimes() mockStorageAccountsClient.EXPECT().Create(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: nil}}, fmt.Errorf("test error")).AnyTimes() + mockFileClient.EXPECT().ListFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return([]storage.FileShareItem{}, nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() expectedErr := status.Errorf(codes.Internal, "test error") _, err := d.CreateVolume(ctx, req) @@ -906,13 +811,6 @@ func TestCreateVolume(t *testing.T) { mockFileClient.EXPECT().WithSubscriptionID(gomock.Any()).Return(mockFileClient).AnyTimes() mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: nil}}, nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() - expectedErr := status.Errorf(codes.Internal, "FileShareProperties or FileShareProperties.ShareQuota is nil") _, err := d.CreateVolume(ctx, req) @@ -980,13 +878,6 @@ func TestCreateVolume(t *testing.T) { mockFileClient.EXPECT().WithSubscriptionID(gomock.Any()).Return(mockFileClient).AnyTimes() mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: pointer.Int32(1)}}, nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() - expectedErr := status.Errorf(codes.AlreadyExists, "request file share(random-vol-name-crete-file-error) already exists, but its capacity 1 is smaller than 100") _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -1074,13 +965,6 @@ func TestCreateVolume(t *testing.T) { mockStorageAccountsClient.EXPECT().Create(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: &fakeShareQuota}}, nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() - _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, test.expectedErr) { t.Errorf("Unexpected error: %v", err) @@ -1117,6 +1001,7 @@ func TestCreateVolume(t *testing.T) { storeAccountKeyField: "storeaccountkey", secretNamespaceField: "default", mountPermissionsField: "0755", + accountQuotaField: "1000", } req := &csi.CreateVolumeRequest{ @@ -1146,13 +1031,6 @@ func TestCreateVolume(t *testing.T) { mockStorageAccountsClient.EXPECT().Create(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: &fakeShareQuota}}, nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() - _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, nil) { t.Errorf("Unexpected error: %v", err) @@ -1207,13 +1085,6 @@ func TestCreateVolume(t *testing.T) { mockStorageAccountsClient.EXPECT().Create(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: &fakeShareQuota}}, nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() - expectedErr := status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid %s %s in storage class", "mountPermissions", "0abc")) _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -1269,13 +1140,6 @@ func TestCreateVolume(t *testing.T) { mockStorageAccountsClient.EXPECT().Create(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: &fakeShareQuota}}, nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() - expectedErr := status.Errorf(codes.InvalidArgument, fmt.Sprintf("invalid parameter %q in storage class", "invalidparameter")) _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { @@ -1343,13 +1207,6 @@ func TestCreateVolume(t *testing.T) { mockStorageAccountsClient.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: &fakeShareQuota}}, nil).AnyTimes() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, - }) - - ctx := context.Background() - _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, nil) { t.Errorf("Unexpected error: %v", err) @@ -1364,6 +1221,8 @@ func TestCreateVolume(t *testing.T) { } func TestDeleteVolume(t *testing.T) { + ctx := context.Background() + testCases := []struct { name string testFunc func(t *testing.T) @@ -1375,7 +1234,6 @@ func TestDeleteVolume(t *testing.T) { Secrets: map[string]string{}, } - ctx := context.Background() d := NewFakeDriver() expectedErr := status.Error(codes.InvalidArgument, "Volume ID missing in request") @@ -1393,8 +1251,8 @@ func TestDeleteVolume(t *testing.T) { Secrets: map[string]string{}, } - ctx := context.Background() d := NewFakeDriver() + d.Cap = []*csi.ControllerServiceCapability{} expectedErr := status.Errorf(codes.InvalidArgument, "invalid delete volume request: %v", req) _, err := d.DeleteVolume(ctx, req) @@ -1411,7 +1269,6 @@ func TestDeleteVolume(t *testing.T) { Secrets: map[string]string{}, } - ctx := context.Background() d := NewFakeDriver() d.Cap = []*csi.ControllerServiceCapability{ { @@ -1435,7 +1292,6 @@ func TestDeleteVolume(t *testing.T) { Secrets: map[string]string{}, } - ctx := context.Background() d := NewFakeDriver() d.Cap = []*csi.ControllerServiceCapability{ { @@ -1463,15 +1319,7 @@ func TestDeleteVolume(t *testing.T) { Secrets: map[string]string{}, } - ctx := context.Background() d := NewFakeDriver() - d.Cap = []*csi.ControllerServiceCapability{ - { - Type: &csi.ControllerServiceCapability_Rpc{ - Rpc: &csi.ControllerServiceCapability_RPC{Type: csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME}, - }, - }, - } ctrl := gomock.NewController(t) defer ctrl.Finish() mockFileClient := mockfileclient.NewMockInterface(ctrl) @@ -1495,15 +1343,7 @@ func TestDeleteVolume(t *testing.T) { Secrets: map[string]string{}, } - ctx := context.Background() d := NewFakeDriver() - d.Cap = []*csi.ControllerServiceCapability{ - { - Type: &csi.ControllerServiceCapability_Rpc{ - Rpc: &csi.ControllerServiceCapability_RPC{Type: csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME}, - }, - }, - } ctrl := gomock.NewController(t) defer ctrl.Finish() mockFileClient := mockfileclient.NewMockInterface(ctrl) @@ -2016,6 +1856,7 @@ func TestDeleteSnapshot(t *testing.T) { func TestControllerExpandVolume(t *testing.T) { stdVolSize := int64(5 * 1024 * 1024 * 1024) stdCapRange := &csi.CapacityRange{RequiredBytes: stdVolSize} + ctx := context.Background() testCases := []struct { name string @@ -2026,7 +1867,6 @@ func TestControllerExpandVolume(t *testing.T) { testFunc: func(t *testing.T) { req := &csi.ControllerExpandVolumeRequest{} - ctx := context.Background() d := NewFakeDriver() expectedErr := status.Error(codes.InvalidArgument, "Volume ID missing in request") @@ -2043,8 +1883,8 @@ func TestControllerExpandVolume(t *testing.T) { VolumeId: "vol_1", } - ctx := context.Background() d := NewFakeDriver() + d.Cap = []*csi.ControllerServiceCapability{} expectedErr := status.Error(codes.InvalidArgument, "volume capacity range missing in request") _, err := d.ControllerExpandVolume(ctx, req) @@ -2061,8 +1901,8 @@ func TestControllerExpandVolume(t *testing.T) { CapacityRange: stdCapRange, } - ctx := context.Background() d := NewFakeDriver() + d.Cap = []*csi.ControllerServiceCapability{} expectedErr := status.Errorf(codes.InvalidArgument, "invalid expand volume request: volume_id:\"vol_1\" capacity_range: ") _, err := d.ControllerExpandVolume(ctx, req) @@ -2079,12 +1919,7 @@ func TestControllerExpandVolume(t *testing.T) { CapacityRange: stdCapRange, } - ctx := context.Background() d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_EXPAND_VOLUME, - }) expectedErr := status.Errorf(codes.InvalidArgument, "GetFileShareInfo(vol_1) failed with error: error parsing volume id: \"vol_1\", should at least contain two #") _, err := d.ControllerExpandVolume(ctx, req) @@ -2097,10 +1932,6 @@ func TestControllerExpandVolume(t *testing.T) { name: "Disk name not empty", testFunc: func(t *testing.T) { d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_EXPAND_VOLUME, - }) d.cloud = &azure.Cloud{} ctrl := gomock.NewController(t) @@ -2117,7 +1948,6 @@ func TestControllerExpandVolume(t *testing.T) { CapacityRange: stdCapRange, } - ctx := context.Background() mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) d.cloud.StorageAccountClient = mockStorageAccountsClient d.cloud.KubeClient = clientSet @@ -2135,10 +1965,6 @@ func TestControllerExpandVolume(t *testing.T) { name: "Resize file share returns error", testFunc: func(t *testing.T) { d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_EXPAND_VOLUME, - }) d.cloud = &azure.Cloud{} ctrl := gomock.NewController(t) @@ -2155,7 +1981,6 @@ func TestControllerExpandVolume(t *testing.T) { CapacityRange: stdCapRange, } - ctx := context.Background() mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) d.cloud.StorageAccountClient = mockStorageAccountsClient d.cloud.KubeClient = clientSet @@ -2177,10 +2002,6 @@ func TestControllerExpandVolume(t *testing.T) { name: "get account info failed", testFunc: func(t *testing.T) { d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_EXPAND_VOLUME, - }) d.cloud = &azure.Cloud{ Config: azure.Config{ ResourceGroup: "vol_2", @@ -2220,10 +2041,6 @@ func TestControllerExpandVolume(t *testing.T) { name: "Valid request", testFunc: func(t *testing.T) { d := NewFakeDriver() - d.AddControllerServiceCapabilities( - []csi.ControllerServiceCapability_RPC_Type{ - csi.ControllerServiceCapability_RPC_EXPAND_VOLUME, - }) d.cloud = &azure.Cloud{} ctrl := gomock.NewController(t) @@ -2240,7 +2057,6 @@ func TestControllerExpandVolume(t *testing.T) { CapacityRange: stdCapRange, } - ctx := context.Background() mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) d.cloud.StorageAccountClient = mockStorageAccountsClient d.cloud.KubeClient = clientSet