diff --git a/pkg/volume/azure_dd/azure_provision.go b/pkg/volume/azure_dd/azure_provision.go index af7d0957670..f6853f0b4d8 100644 --- a/pkg/volume/azure_dd/azure_provision.go +++ b/pkg/volume/azure_dd/azure_provision.go @@ -35,11 +35,6 @@ import ( "k8s.io/legacy-cloud-providers/azure" ) -const ( - TagsDelimiter = "," - TagKeyValueDelimiter = "=" -) - type azureDiskProvisioner struct { plugin *azureDataDiskPlugin options volume.VolumeOptions @@ -269,7 +264,7 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie diskURI := "" labels := map[string]string{} if kind == v1.AzureManagedDisk { - tags, err := ConvertTagsToMap(customTags) + tags, err := azure.ConvertTagsToMap(customTags) if err != nil { return nil, err } @@ -399,28 +394,3 @@ func (p *azureDiskProvisioner) Provision(selectedNode *v1.Node, allowedTopologie return pv, nil } - -// ConvertTagsToMap convert the tags from string to map -// the valid tags fomat is "key1=value1,key2=value2", which could be converted to -// {"key1": "value1", "key2": "value2"} -func ConvertTagsToMap(tags string) (map[string]string, error) { - m := make(map[string]string) - if tags == "" { - return m, nil - } - s := strings.Split(tags, TagsDelimiter) - for _, tag := range s { - kv := strings.Split(tag, TagKeyValueDelimiter) - if len(kv) != 2 { - return nil, fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", tags) - } - key := strings.TrimSpace(kv[0]) - if key == "" { - return nil, fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", tags) - } - value := strings.TrimSpace(kv[1]) - m[key] = value - } - - return m, nil -} diff --git a/pkg/volume/azure_dd/azure_provision_test.go b/pkg/volume/azure_dd/azure_provision_test.go index 49320b2b9bd..298b6cbb564 100644 --- a/pkg/volume/azure_dd/azure_provision_test.go +++ b/pkg/volume/azure_dd/azure_provision_test.go @@ -20,7 +20,6 @@ package azure_dd import ( "fmt" - "reflect" "testing" "github.com/stretchr/testify/assert" @@ -97,69 +96,3 @@ func TestParseZoned(t *testing.T) { } } } - -func TestConvertTagsToMap(t *testing.T) { - testCases := []struct { - desc string - tags string - expectedOutput map[string]string - expectedError bool - }{ - { - desc: "should return empty map when tag is empty", - tags: "", - expectedOutput: map[string]string{}, - expectedError: false, - }, - { - desc: "sing valid tag should be converted", - tags: "key=value", - expectedOutput: map[string]string{ - "key": "value", - }, - expectedError: false, - }, - { - desc: "multiple valid tags should be converted", - tags: "key1=value1,key2=value2", - expectedOutput: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - expectedError: false, - }, - { - desc: "whitespaces should be trimmed", - tags: "key1=value1, key2=value2", - expectedOutput: map[string]string{ - "key1": "value1", - "key2": "value2", - }, - expectedError: false, - }, - { - desc: "should return error for invalid format", - tags: "foo,bar", - expectedOutput: nil, - expectedError: true, - }, - { - desc: "should return error for when key is missed", - tags: "key1=value1,=bar", - expectedOutput: nil, - expectedError: true, - }, - } - - for i, c := range testCases { - m, err := ConvertTagsToMap(c.tags) - if c.expectedError { - assert.NotNil(t, err, "TestCase[%d]: %s", i, c.desc) - } else { - assert.Nil(t, err, "TestCase[%d]: %s", i, c.desc) - if !reflect.DeepEqual(m, c.expectedOutput) { - t.Errorf("got: %v, expected: %v, desc: %v", m, c.expectedOutput, c.desc) - } - } - } -} diff --git a/pkg/volume/azure_file/BUILD b/pkg/volume/azure_file/BUILD index fcada0c84a1..0fcbef38088 100644 --- a/pkg/volume/azure_file/BUILD +++ b/pkg/volume/azure_file/BUILD @@ -22,6 +22,7 @@ go_library( "//staging/src/k8s.io/cloud-provider:go_default_library", "//staging/src/k8s.io/cloud-provider/volume/helpers:go_default_library", "//staging/src/k8s.io/legacy-cloud-providers/azure:go_default_library", + "//staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient:go_default_library", "//vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage:go_default_library", "//vendor/k8s.io/klog/v2:go_default_library", "//vendor/k8s.io/utils/mount:go_default_library", diff --git a/pkg/volume/azure_file/azure_provision.go b/pkg/volume/azure_file/azure_provision.go index b33db7c6aa9..d547fc6e9b0 100644 --- a/pkg/volume/azure_file/azure_provision.go +++ b/pkg/volume/azure_file/azure_provision.go @@ -33,6 +33,7 @@ import ( "k8s.io/kubernetes/pkg/volume" "k8s.io/kubernetes/pkg/volume/util" "k8s.io/legacy-cloud-providers/azure" + "k8s.io/legacy-cloud-providers/azure/clients/fileclient" utilstrings "k8s.io/utils/strings" ) @@ -47,7 +48,7 @@ var ( // azure cloud provider should implement it type azureCloudProvider interface { // create a file share - CreateFileShare(shareName, accountName, accountType, accountKind, resourceGroup, location string, protocol storage.EnabledProtocols, requestGiB int) (string, string, error) + CreateFileShare(account *azure.AccountOptions, fileShare *fileclient.ShareOptions) (string, string, error) // delete a file share DeleteFileShare(resourceGroup, accountName, shareName string) error // resize a file share @@ -155,7 +156,7 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie return nil, fmt.Errorf("%s does not support block volume provisioning", a.plugin.GetPluginName()) } - var sku, resourceGroup, location, account, shareName string + var sku, resourceGroup, location, account, shareName, customTags string capacity := a.options.PVC.Spec.Resources.Requests[v1.ResourceName(v1.ResourceStorage)] requestGiB, err := volumehelpers.RoundUpToGiBInt(capacity) @@ -180,6 +181,8 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie resourceGroup = v case "sharename": shareName = v + case "tags": + customTags = v default: return nil, fmt.Errorf("invalid option %q for volume plugin %s", k, a.plugin.GetPluginName()) } @@ -189,6 +192,11 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie return nil, fmt.Errorf("claim.Spec.Selector is not supported for dynamic provisioning on Azure file") } + tags, err := azure.ConvertTagsToMap(customTags) + if err != nil { + return nil, err + } + if shareName == "" { // File share name has a length limit of 63, and it cannot contain two consecutive '-'s. name := util.GenerateVolumeName(a.options.ClusterName, a.options.PVName, 63) @@ -204,7 +212,23 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie if strings.HasPrefix(strings.ToLower(sku), "premium") { accountKind = string(storage.FileStorage) } - account, key, err := a.azureProvider.CreateFileShare(shareName, account, sku, accountKind, resourceGroup, location, storage.SMB, requestGiB) + + accountOptions := &azure.AccountOptions{ + Name: account, + Type: sku, + Kind: accountKind, + ResourceGroup: resourceGroup, + Location: location, + Tags: tags, + } + + shareOptions := &fileclient.ShareOptions{ + Name: shareName, + Protocol: storage.SMB, + RequestGiB: requestGiB, + } + + account, key, err := a.azureProvider.CreateFileShare(accountOptions, shareOptions) if err != nil { return nil, err } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD b/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD index 805c2824408..5f1287be000 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/BUILD @@ -151,6 +151,7 @@ go_test( "//staging/src/k8s.io/legacy-cloud-providers/azure/cache:go_default_library", "//staging/src/k8s.io/legacy-cloud-providers/azure/clients:go_default_library", "//staging/src/k8s.io/legacy-cloud-providers/azure/clients/diskclient/mockdiskclient:go_default_library", + "//staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient:go_default_library", "//staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient:go_default_library", "//staging/src/k8s.io/legacy-cloud-providers/azure/clients/interfaceclient/mockinterfaceclient:go_default_library", "//staging/src/k8s.io/legacy-cloud-providers/azure/clients/loadbalancerclient/mockloadbalancerclient:go_default_library", diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_blobDiskController.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_blobDiskController.go index 5ca0d4a287e..15d93a3dbda 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_blobDiskController.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_blobDiskController.go @@ -84,7 +84,15 @@ func (c *BlobDiskController) initStorageAccounts() { // If no storage account is given, search all the storage accounts associated with the resource group and pick one that // fits storage type and location. func (c *BlobDiskController) CreateVolume(blobName, accountName, accountType, location string, requestGB int) (string, string, int, error) { - account, key, err := c.common.cloud.EnsureStorageAccount(accountName, accountType, string(defaultStorageAccountKind), c.common.resourceGroup, location, dedicatedDiskAccountNamePrefix, true) + accountOptions := &AccountOptions{ + Name: accountName, + Type: accountType, + Kind: string(defaultStorageAccountKind), + ResourceGroup: c.common.resourceGroup, + Location: location, + EnableHTTPSTrafficOnly: true, + } + account, key, err := c.common.cloud.EnsureStorageAccount(accountOptions, dedicatedDiskAccountNamePrefix) if err != nil { return "", "", 0, fmt.Errorf("could not get storage key for storage account %s: %v", accountName, err) } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_file.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_file.go index dbb4c63dfae..04b6c2e7b8c 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_file.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_file.go @@ -20,11 +20,12 @@ package azure import ( "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" + "k8s.io/legacy-cloud-providers/azure/clients/fileclient" ) // create file share -func (az *Cloud) createFileShare(resourceGroupName, accountName, name string, protocol storage.EnabledProtocols, sizeGiB int) error { - return az.FileClient.CreateFileShare(resourceGroupName, accountName, name, protocol, sizeGiB) +func (az *Cloud) createFileShare(resourceGroupName, accountName string, shareOptions *fileclient.ShareOptions) error { + return az.FileClient.CreateFileShare(resourceGroupName, accountName, shareOptions) } func (az *Cloud) deleteFileShare(resourceGroupName, accountName, name string) error { diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storage.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storage.go index a8390c77450..a653bfcf823 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storage.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storage.go @@ -24,6 +24,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" "k8s.io/klog/v2" + "k8s.io/legacy-cloud-providers/azure/clients/fileclient" ) const ( @@ -36,25 +37,32 @@ const ( // CreateFileShare creates a file share, using a matching storage account type, account kind, etc. // storage account will be created if specified account is not found -func (az *Cloud) CreateFileShare(shareName, accountName, accountType, accountKind, resourceGroup, location string, protocol storage.EnabledProtocols, requestGiB int) (string, string, error) { - if resourceGroup == "" { - resourceGroup = az.resourceGroup +func (az *Cloud) CreateFileShare(accountOptions *AccountOptions, shareOptions *fileclient.ShareOptions) (string, string, error) { + if accountOptions == nil { + return "", "", fmt.Errorf("account options is nil") + } + if shareOptions == nil { + return "", "", fmt.Errorf("share options is nil") + } + if accountOptions.ResourceGroup == "" { + accountOptions.ResourceGroup = az.resourceGroup } - enableHTTPSTrafficOnly := true - if protocol == storage.NFS { - enableHTTPSTrafficOnly = false + accountOptions.EnableHTTPSTrafficOnly = true + if shareOptions.Protocol == storage.NFS { + accountOptions.EnableHTTPSTrafficOnly = false } - account, key, err := az.EnsureStorageAccount(accountName, accountType, accountKind, resourceGroup, location, fileShareAccountNamePrefix, enableHTTPSTrafficOnly) + + accountName, accountKey, err := az.EnsureStorageAccount(accountOptions, fileShareAccountNamePrefix) if err != nil { - return "", "", fmt.Errorf("could not get storage key for storage account %s: %v", accountName, err) + return "", "", fmt.Errorf("could not get storage key for storage account %s: %v", accountOptions.Name, err) } - if err := az.createFileShare(resourceGroup, account, shareName, protocol, requestGiB); err != nil { - return "", "", fmt.Errorf("failed to create share %s in account %s: %v", shareName, account, err) + if err := az.createFileShare(accountOptions.ResourceGroup, accountName, shareOptions); err != nil { + return "", "", fmt.Errorf("failed to create share %s in account %s: %v", shareOptions.Name, accountName, err) } - klog.V(4).Infof("created share %s in account %s", shareName, account) - return account, key, nil + klog.V(4).Infof("created share %s in account %s", shareOptions.Name, accountOptions.Name) + return accountName, accountKey, nil } // DeleteFileShare deletes a file share using storage account name and key diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storage_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storage_test.go index 82a95becfbb..3a969778eaf 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storage_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storage_test.go @@ -25,6 +25,7 @@ import ( "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" "github.com/golang/mock/gomock" + "k8s.io/legacy-cloud-providers/azure/clients/fileclient" "k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient" "k8s.io/legacy-cloud-providers/azure/clients/storageaccountclient/mockstorageaccountclient" ) @@ -141,7 +142,7 @@ func TestCreateFileShare(t *testing.T) { for _, test := range tests { mockFileClient := mockfileclient.NewMockInterface(ctrl) cloud.FileClient = mockFileClient - mockFileClient.EXPECT().CreateFileShare(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(test.err).AnyTimes() + mockFileClient.EXPECT().CreateFileShare(gomock.Any(), gomock.Any(), gomock.Any()).Return(test.err).AnyTimes() mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) cloud.StorageAccountClient = mockStorageAccountsClient @@ -149,7 +150,21 @@ func TestCreateFileShare(t *testing.T) { mockStorageAccountsClient.EXPECT().ListByResourceGroup(gomock.Any(), "rg").Return(test.accounts, nil).AnyTimes() mockStorageAccountsClient.EXPECT().Create(gomock.Any(), "rg", gomock.Any(), gomock.Any()).Return(nil).AnyTimes() - account, key, err := cloud.CreateFileShare(test.name, test.acct, test.acctType, test.acctKind, test.rg, test.loc, storage.SMB, test.gb) + mockAccount := &AccountOptions{ + Name: test.acct, + Type: test.acctType, + Kind: test.acctKind, + ResourceGroup: test.rg, + Location: test.loc, + } + + mockFileShare := &fileclient.ShareOptions{ + Name: test.name, + Protocol: storage.SMB, + RequestGiB: test.gb, + } + + account, key, err := cloud.CreateFileShare(mockAccount, mockFileShare) if test.expectErr && err == nil { t.Errorf("unexpected non-error") continue diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storageaccount.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storageaccount.go index 32a20efc37b..bfc153171d5 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storageaccount.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_storageaccount.go @@ -23,11 +23,17 @@ import ( "strings" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" - "github.com/Azure/go-autorest/autorest/to" "k8s.io/klog/v2" ) +// AccountOptions contains the fields which are used to create storage account. +type AccountOptions struct { + Name, Type, Kind, ResourceGroup, Location string + EnableHTTPSTrafficOnly bool + Tags map[string]string +} + type accountWithLocation struct { Name, StorageType, Location string } @@ -90,7 +96,16 @@ func (az *Cloud) GetStorageAccesskey(account, resourceGroup string) (string, err } // EnsureStorageAccount search storage account, create one storage account(with genAccountNamePrefix) if not found, return accountName, accountKey -func (az *Cloud) EnsureStorageAccount(accountName, accountType, accountKind, resourceGroup, location, genAccountNamePrefix string, enableHTTPSTrafficOnly bool) (string, string, error) { +func (az *Cloud) EnsureStorageAccount(accountOptions *AccountOptions, genAccountNamePrefix string) (string, string, error) { + if accountOptions == nil { + return "", "", fmt.Errorf("account options is nil") + } + accountName := accountOptions.Name + accountType := accountOptions.Type + accountKind := accountOptions.Kind + resourceGroup := accountOptions.ResourceGroup + location := accountOptions.Location + enableHTTPSTrafficOnly := accountOptions.EnableHTTPSTrafficOnly if len(accountName) == 0 { // find a storage account that matches accountType accounts, err := az.getStorageAccounts(accountType, accountKind, resourceGroup, location) @@ -118,13 +133,20 @@ func (az *Cloud) EnsureStorageAccount(accountName, accountType, accountKind, res if accountKind != "" { kind = storage.Kind(accountKind) } - klog.V(2).Infof("azure - no matching account found, begin to create a new account %s in resource group %s, location: %s, accountType: %s, accountKind: %s", - accountName, resourceGroup, location, accountType, kind) + if len(accountOptions.Tags) == 0 { + accountOptions.Tags = make(map[string]string) + } + accountOptions.Tags["created-by"] = "azure" + tags := convertMaptoMapPointer(accountOptions.Tags) + + klog.V(2).Infof("azure - no matching account found, begin to create a new account %s in resource group %s, location: %s, accountType: %s, accountKind: %s, tags: %+v", + accountName, resourceGroup, location, accountType, kind, accountOptions.Tags) + cp := storage.AccountCreateParameters{ Sku: &storage.Sku{Name: storage.SkuName(accountType)}, Kind: kind, AccountPropertiesCreateParameters: &storage.AccountPropertiesCreateParameters{EnableHTTPSTrafficOnly: &enableHTTPSTrafficOnly}, - Tags: map[string]*string{"created-by": to.StringPtr("azure")}, + Tags: tags, Location: &location} ctx, cancel := getContextWithCancel() diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go index 78ca666bddd..e6e287c5e00 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils.go @@ -20,9 +20,16 @@ package azure import ( "context" + "fmt" + "strings" "sync" ) +const ( + tagsDelimiter = "," + tagKeyValueDelimiter = "=" +) + // lockMap used to lock on entries type lockMap struct { sync.Mutex @@ -74,3 +81,36 @@ func (lm *lockMap) unlockEntry(entry string) { func getContextWithCancel() (context.Context, context.CancelFunc) { return context.WithCancel(context.Background()) } + +// ConvertTagsToMap convert the tags from string to map +// the valid tags fomat is "key1=value1,key2=value2", which could be converted to +// {"key1": "value1", "key2": "value2"} +func ConvertTagsToMap(tags string) (map[string]string, error) { + m := make(map[string]string) + if tags == "" { + return m, nil + } + s := strings.Split(tags, tagsDelimiter) + for _, tag := range s { + kv := strings.Split(tag, tagKeyValueDelimiter) + if len(kv) != 2 { + return nil, fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", tags) + } + key := strings.TrimSpace(kv[0]) + if key == "" { + return nil, fmt.Errorf("Tags '%s' are invalid, the format should like: 'key1=value1,key2=value2'", tags) + } + value := strings.TrimSpace(kv[1]) + m[key] = value + } + + return m, nil +} + +func convertMaptoMapPointer(origin map[string]string) map[string]*string { + newly := make(map[string]*string) + for k, v := range origin { + newly[k] = &v + } + return newly +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go index cb527f37dd2..cf148ec4dbb 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_utils_test.go @@ -19,8 +19,11 @@ limitations under the License. package azure import ( + "reflect" "testing" "time" + + "github.com/stretchr/testify/assert" ) func TestSimpleLockEntry(t *testing.T) { @@ -83,3 +86,69 @@ func ensureNoCallback(t *testing.T, callbackChan <-chan interface{}) bool { return true } } + +func TestConvertTagsToMap(t *testing.T) { + testCases := []struct { + desc string + tags string + expectedOutput map[string]string + expectedError bool + }{ + { + desc: "should return empty map when tag is empty", + tags: "", + expectedOutput: map[string]string{}, + expectedError: false, + }, + { + desc: "sing valid tag should be converted", + tags: "key=value", + expectedOutput: map[string]string{ + "key": "value", + }, + expectedError: false, + }, + { + desc: "multiple valid tags should be converted", + tags: "key1=value1,key2=value2", + expectedOutput: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedError: false, + }, + { + desc: "whitespaces should be trimmed", + tags: "key1=value1, key2=value2", + expectedOutput: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedError: false, + }, + { + desc: "should return error for invalid format", + tags: "foo,bar", + expectedOutput: nil, + expectedError: true, + }, + { + desc: "should return error for when key is missed", + tags: "key1=value1,=bar", + expectedOutput: nil, + expectedError: true, + }, + } + + for i, c := range testCases { + m, err := ConvertTagsToMap(c.tags) + if c.expectedError { + assert.NotNil(t, err, "TestCase[%d]: %s", i, c.desc) + } else { + assert.Nil(t, err, "TestCase[%d]: %s", i, c.desc) + if !reflect.DeepEqual(m, c.expectedOutput) { + t.Errorf("got: %v, expected: %v, desc: %v", m, c.expectedOutput, c.desc) + } + } + } +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/azure_fileclient.go b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/azure_fileclient.go index b9ca953af34..197a7c29f82 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/azure_fileclient.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/azure_fileclient.go @@ -35,6 +35,13 @@ type Client struct { fileSharesClient storage.FileSharesClient } +// ShareOptions contains the fields which are used to create file share. +type ShareOptions struct { + Name string + Protocol storage.EnabledProtocols + RequestGiB int +} + // New creates a azure file client func New(config *azclients.ClientConfig) *Client { client := storage.NewFileSharesClientWithBaseURI(config.ResourceManagerEndpoint, config.SubscriptionID) @@ -46,27 +53,30 @@ func New(config *azclients.ClientConfig) *Client { } // CreateFileShare creates a file share -func (c *Client) CreateFileShare(resourceGroupName, accountName, name string, protocol storage.EnabledProtocols, sizeGiB int) error { - result, err := c.GetFileShare(resourceGroupName, accountName, name) +func (c *Client) CreateFileShare(resourceGroupName, accountName string, shareOptions *ShareOptions) error { + if shareOptions == nil { + return fmt.Errorf("share options is nil") + } + result, err := c.GetFileShare(resourceGroupName, accountName, shareOptions.Name) if err == nil { - klog.V(2).Infof("file share(%s) under account(%s) rg(%s) already exists", name, accountName, resourceGroupName) + klog.V(2).Infof("file share(%s) under account(%s) rg(%s) already exists", shareOptions.Name, accountName, resourceGroupName) return nil - } else if err != nil && result.Response.StatusCode != http.StatusNotFound && !strings.Contains(err.Error(), "ShareNotFound") { - return fmt.Errorf("failed to get file share(%s), err: %v", name, err) + } else if result.Response.Response == nil || (err != nil && result.Response.Response.StatusCode != http.StatusNotFound && !strings.Contains(err.Error(), "ShareNotFound")) { + return fmt.Errorf("failed to get file share(%s), err: %v", shareOptions.Name, err) } - quota := int32(sizeGiB) + quota := int32(shareOptions.RequestGiB) fileShareProperties := &storage.FileShareProperties{ ShareQuota: "a, } - if protocol == storage.NFS { - fileShareProperties.EnabledProtocols = protocol + if shareOptions.Protocol == storage.NFS { + fileShareProperties.EnabledProtocols = shareOptions.Protocol } fileShare := storage.FileShare{ - Name: &name, + Name: &shareOptions.Name, FileShareProperties: fileShareProperties, } - _, err = c.fileSharesClient.Create(context.Background(), resourceGroupName, accountName, name, fileShare) + _, err = c.fileSharesClient.Create(context.Background(), resourceGroupName, accountName, shareOptions.Name, fileShare) return err } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/interface.go b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/interface.go index f1f603143b2..0646763aa9c 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/interface.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/interface.go @@ -25,7 +25,7 @@ import ( // Interface is the client interface for creating file shares, interface for test injection. // mockgen -source=$GOPATH/src/k8s.io/kubernetes/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/interface.go -package=mockfileclient Interface > $GOPATH/src/k8s.io/kubernetes/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient/interface.go type Interface interface { - CreateFileShare(resourceGroupName, accountName, name string, protocol storage.EnabledProtocols, sizeGiB int) error + CreateFileShare(resourceGroupName, accountName string, shareOptions *ShareOptions) error DeleteFileShare(resourceGroupName, accountName, name string) error ResizeFileShare(resourceGroupName, accountName, name string, sizeGiB int) error GetFileShare(resourceGroupName, accountName, name string) (storage.FileShare, error) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient/BUILD b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient/BUILD index c249679c9a5..bb5c9f36b29 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient/BUILD +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient/BUILD @@ -10,6 +10,7 @@ go_library( importpath = "k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient", visibility = ["//visibility:public"], deps = [ + "//staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient:go_default_library", "//vendor/github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage:go_default_library", "//vendor/github.com/golang/mock/gomock:go_default_library", ], diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient/interface.go b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient/interface.go index b48858bc9a6..6d2a1703bc6 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient/interface.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/mockfileclient/interface.go @@ -19,10 +19,10 @@ limitations under the License. package mockfileclient import ( - reflect "reflect" - storage "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" gomock "github.com/golang/mock/gomock" + fileclient "k8s.io/legacy-cloud-providers/azure/clients/fileclient" + reflect "reflect" ) // MockInterface is a mock of Interface interface @@ -49,17 +49,17 @@ func (m *MockInterface) EXPECT() *MockInterfaceMockRecorder { } // CreateFileShare mocks base method -func (m *MockInterface) CreateFileShare(resourceGroupName, accountName, name string, protocol storage.EnabledProtocols, sizeGiB int) error { +func (m *MockInterface) CreateFileShare(resourceGroupName, accountName string, shareOptions *fileclient.ShareOptions) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateFileShare", resourceGroupName, accountName, name, protocol, sizeGiB) + ret := m.ctrl.Call(m, "CreateFileShare", resourceGroupName, accountName, shareOptions) ret0, _ := ret[0].(error) return ret0 } // CreateFileShare indicates an expected call of CreateFileShare -func (mr *MockInterfaceMockRecorder) CreateFileShare(resourceGroupName, accountName, name, protocol, sizeGiB interface{}) *gomock.Call { +func (mr *MockInterfaceMockRecorder) CreateFileShare(resourceGroupName, accountName, shareOptions interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFileShare", reflect.TypeOf((*MockInterface)(nil).CreateFileShare), resourceGroupName, accountName, name, protocol, sizeGiB) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFileShare", reflect.TypeOf((*MockInterface)(nil).CreateFileShare), resourceGroupName, accountName, shareOptions) } // DeleteFileShare mocks base method