Skip to content

[release-1.26] feat: add accountQuota parameter #1366

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/driver-parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 <br><br> Note: <br> `false` means driver would leverage kubelet identity to get account key | `true`,`false` | No | `true`
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions pkg/azurefile/azurefile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -131,6 +132,7 @@ const (
shareNamePrefixField = "sharenameprefix"
requireInfraEncryptionField = "requireinfraencryption"
enableMultichannelField = "enablemultichannel"
accountQuotaField = "accountquota"
premium = "premium"

accountNotProvisioned = "StorageAccountIsNotProvisioned"
Expand Down Expand Up @@ -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
Expand Down
76 changes: 76 additions & 0 deletions pkg/azurefile/azurefile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand All @@ -79,6 +86,10 @@ func NewFakeDriverCustomOptions(opts DriverOptions) *Driver {
},
},
}
driver.AddControllerServiceCapabilities(
[]csi.ControllerServiceCapability_RPC_Type{
csi.ControllerServiceCapability_RPC_CREATE_DELETE_VOLUME,
})
return driver
}

Expand Down Expand Up @@ -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)
}
}
31 changes: 26 additions & 5 deletions pkg/azurefile/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
}
Expand Down Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
Expand Down
Loading