From 2c51d333098cfcc255119591c15774ec21d56878 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sat, 12 Aug 2023 13:44:26 +0000 Subject: [PATCH] feat: add accountQuota parameter add e2e test fix golint fix --- 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 | 268 ++++--------------------- test/e2e/dynamic_provisioning_test.go | 1 + 7 files changed, 159 insertions(+), 237 deletions(-) diff --git a/docs/driver-parameters.md b/docs/driver-parameters.md index 0e64ba12fd..fa49641f85 100644 --- a/docs/driver-parameters.md +++ b/docs/driver-parameters.md @@ -41,6 +41,7 @@ storageEndpointSuffix | specify Azure storage endpoint suffix | `core.windows.ne 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` selectRandomMatchingAccount | whether randomly selecting a matching account, by default, the driver would always select the first matching account in alphabetical order | `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 where 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 4e00131511..fe29324b59 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.29 github.com/Azure/go-autorest/autorest/adal v0.9.23 // 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.8.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 be7f55f434..d77ea0b35f 100644 --- a/pkg/azurefile/azurefile.go +++ b/pkg/azurefile/azurefile.go @@ -73,6 +73,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" @@ -135,6 +136,7 @@ const ( enableMultichannelField = "enablemultichannel" premium = "premium" selectRandomMatchingAccountField = "selectrandommatchingaccount" + accountQuotaField = "accountquota" accountNotProvisioned = "StorageAccountIsNotProvisioned" // this is a workaround fix for 429 throttling issue, will update cloud provider for better fix later @@ -870,6 +872,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 06051f5f5e..ea5c4e4660 100644 --- a/pkg/azurefile/azurefile_test.go +++ b/pkg/azurefile/azurefile_test.go @@ -29,6 +29,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" @@ -63,6 +65,11 @@ func NewFakeDriver() *Driver { }, }, } + driver.AddControllerServiceCapabilities( + []csi.ControllerServiceCapability_RPC_Type{ + csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, + csi.ControllerServiceCapability_RPC_EXPAND_VOLUME, + }) return driver } @@ -78,6 +85,10 @@ func NewFakeDriverCustomOptions(opts DriverOptions) *Driver { }, }, } + driver.AddControllerServiceCapabilities( + []csi.ControllerServiceCapability_RPC_Type{ + csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME, + }) return driver } @@ -1274,3 +1285,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 3a8b3d710a..41336552a4 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 { @@ -245,6 +247,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", getLatestAccountKeyField, v)) } getLatestAccountKey = 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)) } @@ -452,6 +460,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 != "" { @@ -502,10 +526,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 @@ -514,7 +535,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 eccbe0b570..742785c00e 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) { @@ -559,13 +497,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, "invalid selectrandommatchingaccount: invalid in storage class") - _, err := d.CreateVolume(context.Background(), req) + _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -621,13 +549,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, "invalid getlatestaccountkey: invalid in storage class") - _, err := d.CreateVolume(context.Background(), req) + _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v, expected error: %v", err, expectedErr) } @@ -653,13 +576,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) } @@ -688,7 +606,6 @@ func TestCreateVolume(t *testing.T) { VolumeCapabilities: stdVolCap, Parameters: allParam, } - ctx := context.Background() d := NewFakeDriver() d.cloud = fakeCloud @@ -699,11 +616,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) { @@ -771,12 +683,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) @@ -839,12 +745,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) @@ -895,13 +795,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) @@ -969,13 +864,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) @@ -1043,13 +931,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) { @@ -1137,13 +1018,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) @@ -1180,6 +1054,7 @@ func TestCreateVolume(t *testing.T) { storeAccountKeyField: "storeaccountkey", secretNamespaceField: "default", mountPermissionsField: "0755", + accountQuotaField: "1000", } req := &csi.CreateVolumeRequest{ @@ -1209,13 +1084,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) @@ -1270,13 +1138,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) { @@ -1332,13 +1193,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) { @@ -1406,13 +1260,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) @@ -1427,6 +1274,8 @@ func TestCreateVolume(t *testing.T) { } func TestDeleteVolume(t *testing.T) { + ctx := context.Background() + testCases := []struct { name string testFunc func(t *testing.T) @@ -1438,7 +1287,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") @@ -1456,8 +1304,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) @@ -1474,7 +1322,6 @@ func TestDeleteVolume(t *testing.T) { Secrets: map[string]string{}, } - ctx := context.Background() d := NewFakeDriver() d.Cap = []*csi.ControllerServiceCapability{ { @@ -1498,7 +1345,6 @@ func TestDeleteVolume(t *testing.T) { Secrets: map[string]string{}, } - ctx := context.Background() d := NewFakeDriver() d.Cap = []*csi.ControllerServiceCapability{ { @@ -1526,15 +1372,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) @@ -1558,15 +1396,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) @@ -2079,6 +1909,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 @@ -2089,7 +1920,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") @@ -2106,8 +1936,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) @@ -2124,8 +1954,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) @@ -2142,12 +1972,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) @@ -2160,10 +1985,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) @@ -2180,7 +2001,6 @@ func TestControllerExpandVolume(t *testing.T) { CapacityRange: stdCapRange, } - ctx := context.Background() mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) d.cloud.StorageAccountClient = mockStorageAccountsClient d.cloud.KubeClient = clientSet @@ -2198,10 +2018,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) @@ -2218,7 +2034,6 @@ func TestControllerExpandVolume(t *testing.T) { CapacityRange: stdCapRange, } - ctx := context.Background() mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) d.cloud.StorageAccountClient = mockStorageAccountsClient d.cloud.KubeClient = clientSet @@ -2240,10 +2055,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", @@ -2283,10 +2094,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) @@ -2303,7 +2110,6 @@ func TestControllerExpandVolume(t *testing.T) { CapacityRange: stdCapRange, } - ctx := context.Background() mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) d.cloud.StorageAccountClient = mockStorageAccountsClient d.cloud.KubeClient = clientSet diff --git a/test/e2e/dynamic_provisioning_test.go b/test/e2e/dynamic_provisioning_test.go index f7af6b6d9b..fd63c2a436 100644 --- a/test/e2e/dynamic_provisioning_test.go +++ b/test/e2e/dynamic_provisioning_test.go @@ -179,6 +179,7 @@ var _ = ginkgo.Describe("Dynamic Provisioning", func() { "skuName": "Premium_LRS", "enableMultichannel": "true", "selectRandomMatchingAccount": "true", + "accountQuota": "200", } test := testsuites.DynamicallyProvisionedCmdVolumeTest{ CSIDriver: testDriver,