Skip to content

Commit

Permalink
check storage account type if parameter is missing
Browse files Browse the repository at this point in the history
  • Loading branch information
@RomanBednar
RomanBednar committed Oct 3, 2023
1 parent 681e505 commit 1b9131b
Show file tree
Hide file tree
Showing 2 changed files with 193 additions and 0 deletions.
11 changes: 11 additions & 0 deletions pkg/azurefile/controllerserver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,17 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest)
}

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) {
Expand Down
182 changes: 182 additions & 0 deletions pkg/azurefile/controllerserver_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"net/http"
"net/url"
"reflect"
"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient"

Check failure on line 26 in pkg/azurefile/controllerserver_test.go

View workflow job for this annotation

GitHub Actions / build (1.16.x, windows-latest) 

other declaration of fileclient

Check failure on line 26 in pkg/azurefile/controllerserver_test.go

View workflow job for this annotation

GitHub Actions / Go Lint 

other declaration of fileclient

Check failure on line 26 in pkg/azurefile/controllerserver_test.go

View workflow job for this annotation

GitHub Actions / Build 

other declaration of fileclient

Check failure on line 26 in pkg/azurefile/controllerserver_test.go

View workflow job for this annotation

GitHub Actions / Build 

other declaration of fileclient
"strings"
"sync"
"testing"
Expand Down Expand Up @@ -1267,6 +1268,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 {
Expand Down

0 comments on commit 1b9131b

Please sign in to comment.