diff --git a/deploy/example/azure.json b/deploy/example/azure.json index 54995a9253..18a8178e9d 100644 --- a/deploy/example/azure.json +++ b/deploy/example/azure.json @@ -6,8 +6,8 @@ "location": "eastus2", // mandatory "aadClientId": "xxxx-xxxx-xxxx-xxxx-xxxx", // mandatory if using service principal "aadClientSecret": "xxxx-xxxx-xxxx-xxxx-xxxx", // mandatory if using service principal - "useManagedIdentityExtension": false, // set true if using managed idenitty - "userAssignedIdentityID": "", // mandatory if using managed idenitty + "useManagedIdentityExtension": false, // set true if using managed identity + "userAssignedIdentityID": "", // mandatory if using managed identity "useInstanceMetadata": true, // optional "vmType": "standard", // optional "subnetName": "k8s-subnet", // optional diff --git a/docs/csi-debug.md b/docs/csi-debug.md index 846e88f455..2cc4dba6e5 100644 --- a/docs/csi-debug.md +++ b/docs/csi-debug.md @@ -173,7 +173,7 @@ Enable [large file shares](https://docs.microsoft.com/azure/storage/files/storag ##### Premium Files Azure premium files follows provisioned model where IOPS and throughput are associated to the quota. See this article that explains the co-relation between share size and IOPS and throughput - [link](https://docs.microsoft.com/azure/storage/files/understanding-billing#provisioned-model). Increase the share quota by following this guide - [link](https://github.com/kubernetes-sigs/azurefile-csi-driver/tree/master/deploy/example/resize). -##### For more, refer to this doc for perforance troubleshooting tips - [Link to performance troubleshooting tips](https://docs.microsoft.com/en-us/azure/storage/files/storage-troubleshooting-files-performance) +##### For more, refer to this doc for performance troubleshooting tips - [Link to performance troubleshooting tips](https://docs.microsoft.com/en-us/azure/storage/files/storage-troubleshooting-files-performance) ##### [Storage considerations for Azure Kubernetes Service (AKS)](https://learn.microsoft.com/en-us/azure/cloud-adoption-framework/scenarios/app-platform/aks/storage) ##### [Compare access to Azure Files, Blob Storage, and Azure NetApp Files with NFS](https://learn.microsoft.com/en-us/azure/storage/common/nfs-comparison#comparison) diff --git a/pkg/azurefile/controllerserver.go b/pkg/azurefile/controllerserver.go index bdb8e2ff3d..4621111458 100644 --- a/pkg/azurefile/controllerserver.go +++ b/pkg/azurefile/controllerserver.go @@ -336,7 +336,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) shareProtocol = storage.EnabledProtocolsNFS // NFS protocol does not need account key storeAccountKey = false - // reset protocol field (compatble with "fsType: nfs") + // reset protocol field (compatible with "fsType: nfs") setKeyValueInMap(parameters, protocolField, protocol) if !pointer.BoolDeref(createPrivateEndpoint, false) { @@ -359,7 +359,22 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) } } + if resourceGroup == "" { + resourceGroup = d.cloud.ResourceGroup + } + fileShareSize := int(requestGiB) + + if account != "" && resourceGroup != "" && sku == "" && fileShareSize < minimumPremiumShareSize { + accountProperties, err := d.cloud.StorageAccountClient.GetProperties(ctx, subsID, resourceGroup, account) + if err != nil { + klog.Warningf("failed to get properties on storage account account(%s) rg(%s), error: %v", account, resourceGroup, err) + } + if accountProperties.Sku != nil { + sku = string(accountProperties.Sku.Name) + } + } + // account kind should be FileStorage for Premium File accountKind := string(storage.KindStorageV2) if strings.HasPrefix(strings.ToLower(sku), premium) { @@ -387,10 +402,6 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) validFileShareName = getValidFileShareName(name) } - if resourceGroup == "" { - resourceGroup = d.cloud.ResourceGroup - } - tags, err := ConvertTagsToMap(customTags) if err != nil { return nil, status.Errorf(codes.InvalidArgument, err.Error()) diff --git a/pkg/azurefile/controllerserver_test.go b/pkg/azurefile/controllerserver_test.go index 2c57ad312a..b79a4e62f6 100644 --- a/pkg/azurefile/controllerserver_test.go +++ b/pkg/azurefile/controllerserver_test.go @@ -23,13 +23,13 @@ import ( "net/http" "net/url" "reflect" + "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient" "strings" "sync" "testing" "time" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network" - "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/subnetclient/mocksubnetclient" azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" @@ -1267,6 +1267,187 @@ func TestCreateVolume(t *testing.T) { } }, }, + { + name: "Premium storage account type (sku) loads from storage account when not given as parameter and share request size is increased to min. size required by premium", + testFunc: func(t *testing.T) { + name := "stoacc" + sku := "premium" + value := "foo bar" + accounts := []storage.Account{ + {Name: &name, Sku: &storage.Sku{Name: storage.SkuName(sku)}}, + } + keys := storage.AccountListKeysResult{ + Keys: &[]storage.AccountKey{ + {Value: &value}, + }, + } + capRange := &csi.CapacityRange{RequiredBytes: 1024 * 1024 * 1024, LimitBytes: 1024 * 1024 * 1024} + + allParam := map[string]string{ + locationField: "loc", + storageAccountField: "stoacc", + resourceGroupField: "rg", + shareNameField: "", + diskNameField: "diskname.vhd", + fsTypeField: "", + storeAccountKeyField: "storeaccountkey", + secretNamespaceField: "default", + mountPermissionsField: "0755", + accountQuotaField: "1000", + protocolField: smb, + } + req := &csi.CreateVolumeRequest{ + Name: "vol-1", + Parameters: allParam, + VolumeCapabilities: stdVolCap, + CapacityRange: capRange, + } + + expectedShareOptions := &fileclient.ShareOptions{Name: "vol-1", Protocol: "SMB", RequestGiB: 100, AccessTier: "", RootSquash: "", Metadata: nil} + + d := NewFakeDriver() + + ctrl := gomock.NewController(t) + mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) + mockFileClient := mockfileclient.NewMockInterface(ctrl) + d.cloud.FileClient = mockFileClient + d.cloud.StorageAccountClient = mockStorageAccountsClient + + mockFileClient.EXPECT().WithSubscriptionID(gomock.Any()).Return(mockFileClient).AnyTimes() + mockFileClient.EXPECT().CreateFileShare(context.TODO(), gomock.Any(), gomock.Any(), expectedShareOptions, gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: nil}}, 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() + mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(keys, nil).AnyTimes() + 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() + mockStorageAccountsClient.EXPECT().GetProperties(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(accounts[0], nil).AnyTimes() + + _, err := d.CreateVolume(ctx, req) + + if !reflect.DeepEqual(err, nil) { + t.Errorf("Unexpected error: %v", err) + } + }, + }, + { + name: "Premium storage account type (sku) does not load from storage account for size request above min. premium size", + testFunc: func(t *testing.T) { + name := "stoacc" + sku := "premium" + value := "foo bar" + accounts := []storage.Account{ + {Name: &name, Sku: &storage.Sku{Name: storage.SkuName(sku)}}, + } + keys := storage.AccountListKeysResult{ + Keys: &[]storage.AccountKey{ + {Value: &value}, + }, + } + capRange := &csi.CapacityRange{RequiredBytes: 1024 * 1024 * 1024 * 100, LimitBytes: 1024 * 1024 * 1024 * 100} + + allParam := map[string]string{ + locationField: "loc", + storageAccountField: "stoacc", + resourceGroupField: "rg", + shareNameField: "", + diskNameField: "diskname.vhd", + fsTypeField: "", + storeAccountKeyField: "storeaccountkey", + secretNamespaceField: "default", + mountPermissionsField: "0755", + accountQuotaField: "1000", + protocolField: smb, + } + req := &csi.CreateVolumeRequest{ + Name: "vol-1", + Parameters: allParam, + VolumeCapabilities: stdVolCap, + CapacityRange: capRange, + } + + expectedShareOptions := &fileclient.ShareOptions{Name: "vol-1", Protocol: "SMB", RequestGiB: 100, AccessTier: "", RootSquash: "", Metadata: nil} + + d := NewFakeDriver() + + ctrl := gomock.NewController(t) + mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) + mockFileClient := mockfileclient.NewMockInterface(ctrl) + d.cloud.FileClient = mockFileClient + d.cloud.StorageAccountClient = mockStorageAccountsClient + + mockFileClient.EXPECT().WithSubscriptionID(gomock.Any()).Return(mockFileClient).AnyTimes() + mockFileClient.EXPECT().CreateFileShare(context.TODO(), gomock.Any(), gomock.Any(), expectedShareOptions, gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: nil}}, 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() + mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(keys, nil).AnyTimes() + 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() + + _, err := d.CreateVolume(ctx, req) + + if !reflect.DeepEqual(err, nil) { + t.Errorf("Unexpected error: %v", err) + } + }, + }, + { + name: "Storage account type (sku) defaults to standard type and share request size is unchanged", + testFunc: func(t *testing.T) { + name := "stoacc" + sku := "" + value := "foo bar" + accounts := []storage.Account{ + {Name: &name, Sku: &storage.Sku{Name: storage.SkuName(sku)}}, + } + keys := storage.AccountListKeysResult{ + Keys: &[]storage.AccountKey{ + {Value: &value}, + }, + } + capRange := &csi.CapacityRange{RequiredBytes: 1024 * 1024 * 1024, LimitBytes: 1024 * 1024 * 1024} + + allParam := map[string]string{ + locationField: "loc", + storageAccountField: "stoacc", + resourceGroupField: "rg", + shareNameField: "", + diskNameField: "diskname.vhd", + fsTypeField: "", + storeAccountKeyField: "storeaccountkey", + secretNamespaceField: "default", + mountPermissionsField: "0755", + accountQuotaField: "1000", + } + req := &csi.CreateVolumeRequest{ + Name: "vol-1", + Parameters: allParam, + VolumeCapabilities: stdVolCap, + CapacityRange: capRange, + } + + expectedShareOptions := &fileclient.ShareOptions{Name: "vol-1", Protocol: "SMB", RequestGiB: 1, AccessTier: "", RootSquash: "", Metadata: nil} + + d := NewFakeDriver() + + ctrl := gomock.NewController(t) + mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) + mockFileClient := mockfileclient.NewMockInterface(ctrl) + d.cloud.FileClient = mockFileClient + d.cloud.StorageAccountClient = mockStorageAccountsClient + + mockFileClient.EXPECT().WithSubscriptionID(gomock.Any()).Return(mockFileClient).AnyTimes() + mockFileClient.EXPECT().CreateFileShare(context.TODO(), gomock.Any(), gomock.Any(), expectedShareOptions, gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: nil}}, 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() + mockStorageAccountsClient.EXPECT().ListKeys(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(keys, nil).AnyTimes() + 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() + mockStorageAccountsClient.EXPECT().GetProperties(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(accounts[0], nil).AnyTimes() + + _, err := d.CreateVolume(ctx, req) + + if !reflect.DeepEqual(err, nil) { + t.Errorf("Unexpected error: %v", err) + } + }, + }, } for _, tc := range testCases { diff --git a/test/e2e/driver/azurefile_driver.go b/test/e2e/driver/azurefile_driver.go index c85e8afa05..efc19103e1 100644 --- a/test/e2e/driver/azurefile_driver.go +++ b/test/e2e/driver/azurefile_driver.go @@ -39,7 +39,7 @@ type AzureFileDriver struct { driverName string } -// InitAzureFileDriver returns AzureFileDriver that implemnts DynamicPVTestDriver interface +// InitAzureFileDriver returns AzureFileDriver that implements DynamicPVTestDriver interface func InitAzureFileDriver() PVTestDriver { driverName := os.Getenv(AzureDriverNameVar) if driverName == "" {