From f3a64e22d97bc04d3c3613898ac6bc29e532b041 Mon Sep 17 00:00:00 2001 From: ZeroMagic Date: Tue, 21 Apr 2020 15:43:26 +0000 Subject: [PATCH 1/2] chore: switch to use AzureFile management SDK Signed-off-by: ZeroMagic --- pkg/volume/azure_file/azure_file.go | 6 +- pkg/volume/azure_file/azure_provision.go | 29 ++--- .../legacy-cloud-providers/azure/azure.go | 5 +- .../azure/azure_file.go | 12 +- .../azure/azure_storage.go | 10 +- .../azure/azure_storage_test.go | 84 +++++++++++++ .../azure/clients/fileclient/BUILD | 5 +- .../clients/fileclient/azure_fileclient.go | 119 +++++++----------- .../azure/clients/fileclient/interface.go | 6 +- .../fileclient/mockfileclient/interface.go | 24 ++-- 10 files changed, 176 insertions(+), 124 deletions(-) diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index 3d7aed3428d..0b0d4bd9d32 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -165,7 +165,7 @@ func (plugin *azureFilePlugin) ExpandVolumeDevice( return oldSize, fmt.Errorf("invalid PV spec") } shareName := spec.PersistentVolume.Spec.AzureFile.ShareName - azure, err := getAzureCloudProvider(plugin.host.GetCloudProvider()) + azure, resourceGroup, err := getAzureCloudProvider(plugin.host.GetCloudProvider()) if err != nil { return oldSize, err } @@ -175,12 +175,12 @@ func (plugin *azureFilePlugin) ExpandVolumeDevice( return oldSize, err } - accountName, accountKey, err := (&azureSvc{}).GetAzureCredentials(plugin.host, secretNamespace, secretName) + accountName, _, err := (&azureSvc{}).GetAzureCredentials(plugin.host, secretNamespace, secretName) if err != nil { return oldSize, err } - if err := azure.ResizeFileShare(accountName, accountKey, shareName, int(volumehelpers.RoundUpToGiB(newSize))); err != nil { + if err := azure.ResizeFileShare(resourceGroup, accountName, shareName, int(volumehelpers.RoundUpToGiB(newSize))); err != nil { return oldSize, err } diff --git a/pkg/volume/azure_file/azure_provision.go b/pkg/volume/azure_file/azure_provision.go index bdae2a3660c..e24651bfb61 100644 --- a/pkg/volume/azure_file/azure_provision.go +++ b/pkg/volume/azure_file/azure_provision.go @@ -45,28 +45,28 @@ type azureCloudProvider interface { // create a file share CreateFileShare(shareName, accountName, accountType, accountKind, resourceGroup, location string, requestGiB int) (string, string, error) // delete a file share - DeleteFileShare(accountName, accountKey, shareName string) error + DeleteFileShare(resourceGroup, accountName, shareName string) error // resize a file share - ResizeFileShare(accountName, accountKey, name string, sizeGiB int) error + ResizeFileShare(resourceGroup, accountName, name string, sizeGiB int) error } type azureFileDeleter struct { *azureFile - accountName, accountKey, shareName string - azureProvider azureCloudProvider + resourceGroup, accountName, shareName string + azureProvider azureCloudProvider } func (plugin *azureFilePlugin) NewDeleter(spec *volume.Spec) (volume.Deleter, error) { - azure, err := getAzureCloudProvider(plugin.host.GetCloudProvider()) + azure, resourceGroup, err := getAzureCloudProvider(plugin.host.GetCloudProvider()) if err != nil { klog.V(4).Infof("failed to get azure provider") return nil, err } - return plugin.newDeleterInternal(spec, &azureSvc{}, azure) + return plugin.newDeleterInternal(spec, &azureSvc{}, azure, resourceGroup) } -func (plugin *azureFilePlugin) newDeleterInternal(spec *volume.Spec, util azureUtil, azure azureCloudProvider) (volume.Deleter, error) { +func (plugin *azureFilePlugin) newDeleterInternal(spec *volume.Spec, util azureUtil, azure azureCloudProvider, resourceGroup string) (volume.Deleter, error) { if spec.PersistentVolume != nil && spec.PersistentVolume.Spec.AzureFile == nil { return nil, fmt.Errorf("invalid PV spec") } @@ -76,24 +76,25 @@ func (plugin *azureFilePlugin) newDeleterInternal(spec *volume.Spec, util azureU return nil, err } shareName := spec.PersistentVolume.Spec.AzureFile.ShareName - if accountName, accountKey, err := util.GetAzureCredentials(plugin.host, secretNamespace, secretName); err != nil { + if accountName, _, err := util.GetAzureCredentials(plugin.host, secretNamespace, secretName); err != nil { return nil, err } else { + return &azureFileDeleter{ azureFile: &azureFile{ volName: spec.Name(), plugin: plugin, }, + resourceGroup: resourceGroup, shareName: shareName, accountName: accountName, - accountKey: accountKey, azureProvider: azure, }, nil } } func (plugin *azureFilePlugin) NewProvisioner(options volume.VolumeOptions) (volume.Provisioner, error) { - azure, err := getAzureCloudProvider(plugin.host.GetCloudProvider()) + azure, _, err := getAzureCloudProvider(plugin.host.GetCloudProvider()) if err != nil { klog.V(4).Infof("failed to get azure provider") return nil, err @@ -124,7 +125,7 @@ func (f *azureFileDeleter) GetPath() string { func (f *azureFileDeleter) Delete() error { klog.V(4).Infof("deleting volume %s", f.shareName) - return f.azureProvider.DeleteFileShare(f.accountName, f.accountKey, f.shareName) + return f.azureProvider.DeleteFileShare(f.resourceGroup, f.accountName, f.shareName) } type azureFileProvisioner struct { @@ -224,11 +225,11 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie } // Return cloud provider -func getAzureCloudProvider(cloudProvider cloudprovider.Interface) (azureCloudProvider, error) { +func getAzureCloudProvider(cloudProvider cloudprovider.Interface) (azureCloudProvider, string, error) { azureCloudProvider, ok := cloudProvider.(*azure.Cloud) if !ok || azureCloudProvider == nil { - return nil, fmt.Errorf("Failed to get Azure Cloud Provider. GetCloudProvider returned %v instead", cloudProvider) + return nil, "", fmt.Errorf("Failed to get Azure Cloud Provider. GetCloudProvider returned %v instead", cloudProvider) } - return azureCloudProvider, nil + return azureCloudProvider, azureCloudProvider.ResourceGroup, nil } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go index 22be4e88a8d..276511c1f4d 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go @@ -549,6 +549,8 @@ func (az *Cloud) configAzureClients( loadBalancerClientConfig := azClientConfig.WithRateLimiter(az.Config.LoadBalancerRateLimit) securityGroupClientConfig := azClientConfig.WithRateLimiter(az.Config.SecurityGroupRateLimit) publicIPClientConfig := azClientConfig.WithRateLimiter(az.Config.PublicIPAddressRateLimit) + // TODO(ZeroMagic): add azurefileRateLimit + fileClientConfig := azClientConfig.WithRateLimiter(nil) // If uses network resources in different AAD Tenant, update Authorizer for VM/VMSS client config if multiTenantServicePrincipalToken != nil { @@ -591,8 +593,7 @@ func (az *Cloud) configAzureClients( az.LoadBalancerClient = loadbalancerclient.New(loadBalancerClientConfig) az.SecurityGroupsClient = securitygroupclient.New(securityGroupClientConfig) az.PublicIPAddressesClient = publicipclient.New(publicIPClientConfig) - // fileClient is not based on armclient, but it's still backoff retried. - az.FileClient = fileclient.NewAzureFileClient(&az.Environment, azClientConfig.Backoff) + az.FileClient = fileclient.New(fileClientConfig) } func (az *Cloud) getAzureClientConfig(servicePrincipalToken *adal.ServicePrincipalToken) *azclients.ClientConfig { 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 5fbfc3b025d..d1ec2f1080a 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 @@ -19,14 +19,14 @@ limitations under the License. package azure // create file share -func (az *Cloud) createFileShare(accountName, accountKey, name string, sizeGiB int) error { - return az.FileClient.CreateFileShare(accountName, accountKey, name, sizeGiB) +func (az *Cloud) createFileShare(resourceGroupName, accountName, name string, sizeGiB int) error { + return az.FileClient.CreateFileShare(resourceGroupName, accountName, name, sizeGiB) } -func (az *Cloud) deleteFileShare(accountName, accountKey, name string) error { - return az.FileClient.DeleteFileShare(accountName, accountKey, name) +func (az *Cloud) deleteFileShare(resourceGroupName, accountName, name string) error { + return az.FileClient.DeleteFileShare(resourceGroupName, accountName, name) } -func (az *Cloud) resizeFileShare(accountName, accountKey, name string, sizeGiB int) error { - return az.FileClient.ResizeFileShare(accountName, accountKey, name, sizeGiB) +func (az *Cloud) resizeFileShare(resourceGroupName, accountName, name string, sizeGiB int) error { + return az.FileClient.ResizeFileShare(resourceGroupName, accountName, name, sizeGiB) } 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 9ca06434a1a..1fd00f7ff36 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 @@ -46,7 +46,7 @@ func (az *Cloud) CreateFileShare(shareName, accountName, accountType, accountKin return "", "", fmt.Errorf("could not get storage key for storage account %s: %v", accountName, err) } - if err := az.createFileShare(account, key, shareName, requestGiB); err != nil { + if err := az.createFileShare(resourceGroup, account, shareName, requestGiB); err != nil { return "", "", fmt.Errorf("failed to create share %s in account %s: %v", shareName, account, err) } klog.V(4).Infof("created share %s in account %s", shareName, account) @@ -54,8 +54,8 @@ func (az *Cloud) CreateFileShare(shareName, accountName, accountType, accountKin } // DeleteFileShare deletes a file share using storage account name and key -func (az *Cloud) DeleteFileShare(accountName, accountKey, shareName string) error { - if err := az.deleteFileShare(accountName, accountKey, shareName); err != nil { +func (az *Cloud) DeleteFileShare(resourceGroup, accountName, shareName string) error { + if err := az.deleteFileShare(resourceGroup, accountName, shareName); err != nil { return err } klog.V(4).Infof("share %s deleted", shareName) @@ -63,6 +63,6 @@ func (az *Cloud) DeleteFileShare(accountName, accountKey, shareName string) erro } // ResizeFileShare resizes a file share -func (az *Cloud) ResizeFileShare(accountName, accountKey, name string, sizeGiB int) error { - return az.resizeFileShare(accountName, accountKey, name, sizeGiB) +func (az *Cloud) ResizeFileShare(resourceGroup, accountName, name string, sizeGiB int) error { + return az.resizeFileShare(resourceGroup, accountName, name, sizeGiB) } 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 5bc240f52f9..f74aaee749b 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 @@ -145,3 +145,87 @@ func TestCreateFileShare(t *testing.T) { } } } + +func TestDeleteFileShare(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + cloud := &Cloud{} + mockFileClient := mockfileclient.NewMockInterface(ctrl) + mockFileClient.EXPECT().DeleteFileShare(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + cloud.FileClient = mockFileClient + + tests := []struct { + rg string + acct string + name string + + expectErr bool + }{ + { + rg: "rg", + acct: "bar", + name: "foo", + + expectErr: false, + }, + } + + for _, test := range tests { + mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) + cloud.StorageAccountClient = mockStorageAccountsClient + + err := cloud.DeleteFileShare(test.rg, test.acct, test.name) + if test.expectErr && err == nil { + t.Errorf("unexpected non-error") + continue + } + if !test.expectErr && err != nil { + t.Errorf("unexpected error: %v", err) + continue + } + } +} + +func TestResizeFileShare(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + cloud := &Cloud{} + mockFileClient := mockfileclient.NewMockInterface(ctrl) + mockFileClient.EXPECT().ResizeFileShare(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil).AnyTimes() + cloud.FileClient = mockFileClient + + tests := []struct { + rg string + acct string + name string + gb int + + expectErr bool + }{ + { + rg: "rg", + acct: "bar", + name: "foo", + gb: 10, + + expectErr: false, + }, + } + + for _, test := range tests { + mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) + cloud.StorageAccountClient = mockStorageAccountsClient + + err := cloud.ResizeFileShare(test.rg, test.acct, test.name, test.gb) + if test.expectErr && err == nil { + t.Errorf("unexpected non-error") + continue + } + if !test.expectErr && err != nil { + t.Errorf("unexpected error: %v", err) + continue + } + } +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/BUILD b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/BUILD index f86dcab702b..836e05a6609 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/BUILD +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/clients/fileclient/BUILD @@ -11,9 +11,8 @@ go_library( importpath = "k8s.io/legacy-cloud-providers/azure/clients/fileclient", visibility = ["//visibility:public"], deps = [ - "//staging/src/k8s.io/legacy-cloud-providers/azure/retry:go_default_library", - "//vendor/github.com/Azure/azure-sdk-for-go/storage:go_default_library", - "//vendor/github.com/Azure/go-autorest/autorest/azure:go_default_library", + "//staging/src/k8s.io/legacy-cloud-providers/azure/clients: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:go_default_library", ], ) 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 a4a5f258929..16af207a2f1 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 @@ -19,106 +19,73 @@ limitations under the License. package fileclient import ( + "context" "fmt" - "net/http" - azs "github.com/Azure/azure-sdk-for-go/storage" - "github.com/Azure/go-autorest/autorest/azure" + "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2019-06-01/storage" "k8s.io/klog" - "k8s.io/legacy-cloud-providers/azure/retry" + azclients "k8s.io/legacy-cloud-providers/azure/clients" ) -const ( - useHTTPS = true -) - -var ( - // refer https://github.com/Azure/azure-sdk-for-go/blob/master/storage/client.go#L88. - defaultValidStatusCodes = []int{ - http.StatusRequestTimeout, // 408 - http.StatusInternalServerError, // 500 - http.StatusBadGateway, // 502 - http.StatusServiceUnavailable, // 503 - http.StatusGatewayTimeout, // 504 - } -) - -// AzureFileClient implements the azure file client interface -type AzureFileClient struct { - env *azure.Environment - backoff *retry.Backoff +// Client implements the azure file client interface +type Client struct { + fileSharesClient storage.FileSharesClient } -// NewAzureFileClient creates a azure file client -func NewAzureFileClient(env *azure.Environment, backoff *retry.Backoff) *AzureFileClient { - return &AzureFileClient{ - env: env, - backoff: backoff, +// New creates a azure file client +func New(config *azclients.ClientConfig) *Client { + client := storage.NewFileSharesClientWithBaseURI(config.ResourceManagerEndpoint, config.SubscriptionID) + client.Authorizer = config.Authorizer + + return &Client{ + fileSharesClient: client, } } // CreateFileShare creates a file share -func (f *AzureFileClient) CreateFileShare(accountName, accountKey, name string, sizeGiB int) error { - fileClient, err := f.getFileSvcClient(accountName, accountKey) - if err != nil { - return err +func (c *Client) CreateFileShare(resourceGroupName, accountName, name string, sizeGiB int) error { + quota := int32(sizeGiB) + fileShare := storage.FileShare{ + Name: &name, + FileShareProperties: &storage.FileShareProperties{ + ShareQuota: "a, + }, } - share := fileClient.GetShareReference(name) - share.Properties.Quota = sizeGiB - newlyCreated, err := share.CreateIfNotExists(nil) - if err != nil { - return fmt.Errorf("failed to create file share, err: %v", err) - } - if !newlyCreated { - klog.V(2).Infof("file share(%s) under account(%s) already exists", name, accountName) - } - return nil + _, err := c.fileSharesClient.Create(context.Background(), resourceGroupName, accountName, name, fileShare) + + return err } // DeleteFileShare deletes a file share -func (f *AzureFileClient) DeleteFileShare(accountName, accountKey, name string) error { - fileClient, err := f.getFileSvcClient(accountName, accountKey) - if err != nil { - return err - } - return fileClient.GetShareReference(name).Delete(nil) +func (c *Client) DeleteFileShare(resourceGroupName, accountName, name string) error { + _, err := c.fileSharesClient.Delete(context.Background(), resourceGroupName, accountName, name) + + return err } // ResizeFileShare resizes a file share -func (f *AzureFileClient) ResizeFileShare(accountName, accountKey, name string, sizeGiB int) error { - fileClient, err := f.getFileSvcClient(accountName, accountKey) +func (c *Client) ResizeFileShare(resourceGroupName, accountName, name string, sizeGiB int) error { + quota := int32(sizeGiB) + + share, err := c.fileSharesClient.Get(context.Background(), resourceGroupName, accountName, name) if err != nil { - return err + return fmt.Errorf("failed to get file share(%s), : %v", name, err) } - share := fileClient.GetShareReference(name) - if share.Properties.Quota >= sizeGiB { + if *share.FileShareProperties.ShareQuota >= quota { klog.Warningf("file share size(%dGi) is already greater or equal than requested size(%dGi), accountName: %s, shareName: %s", - share.Properties.Quota, sizeGiB, accountName, name) + share.FileShareProperties.ShareQuota, sizeGiB, accountName, name) return nil + } - share.Properties.Quota = sizeGiB - if err = share.SetProperties(nil); err != nil { - return fmt.Errorf("failed to set quota on file share %s, err: %v", name, err) + + share.FileShareProperties.ShareQuota = "a + _, err = c.fileSharesClient.Update(context.Background(), resourceGroupName, accountName, name, share) + if err != nil { + return fmt.Errorf("failed to update quota on file share(%s), err: %v", name, err) } - klog.V(4).Infof("resize file share completed, accountName: %s, shareName: %s, sizeGiB: %d", accountName, name, sizeGiB) + + klog.V(4).Infof("resize file share completed, resourceGroupName(%s), accountName: %s, shareName: %s, sizeGiB: %d", resourceGroupName, accountName, name, sizeGiB) + return nil } - -func (f *AzureFileClient) getFileSvcClient(accountName, accountKey string) (*azs.FileServiceClient, error) { - fileClient, err := azs.NewClient(accountName, accountKey, f.env.StorageEndpointSuffix, azs.DefaultAPIVersion, useHTTPS) - if err != nil { - return nil, fmt.Errorf("error creating azure client: %v", err) - } - - if f.backoff != nil { - fileClient.Sender = &azs.DefaultSender{ - RetryAttempts: f.backoff.Steps, - ValidStatusCodes: defaultValidStatusCodes, - RetryDuration: f.backoff.Duration, - } - } - - fc := fileClient.GetFileService() - return &fc, nil -} 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 a5e22927bb3..f24753b6884 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 @@ -21,7 +21,7 @@ package fileclient // 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(accountName, accountKey, name string, sizeGiB int) error - DeleteFileShare(accountName, accountKey, name string) error - ResizeFileShare(accountName, accountKey, name string, sizeGiB int) error + CreateFileShare(resourceGroupName, accountName, name string, sizeGiB int) error + DeleteFileShare(resourceGroupName, accountName, name string) error + ResizeFileShare(resourceGroupName, accountName, name string, sizeGiB int) error } 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 7362218b80e..341ea9ed0ed 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 @@ -48,43 +48,43 @@ func (m *MockInterface) EXPECT() *MockInterfaceMockRecorder { } // CreateFileShare mocks base method -func (m *MockInterface) CreateFileShare(accountName, accountKey, name string, sizeGiB int) error { +func (m *MockInterface) CreateFileShare(resourceGroupName, accountName, name string, sizeGiB int) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "CreateFileShare", accountName, accountKey, name, sizeGiB) + ret := m.ctrl.Call(m, "CreateFileShare", resourceGroupName, accountName, name, sizeGiB) ret0, _ := ret[0].(error) return ret0 } // CreateFileShare indicates an expected call of CreateFileShare -func (mr *MockInterfaceMockRecorder) CreateFileShare(accountName, accountKey, name, sizeGiB interface{}) *gomock.Call { +func (mr *MockInterfaceMockRecorder) CreateFileShare(resourceGroupName, accountName, name, sizeGiB interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFileShare", reflect.TypeOf((*MockInterface)(nil).CreateFileShare), accountName, accountKey, name, sizeGiB) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CreateFileShare", reflect.TypeOf((*MockInterface)(nil).CreateFileShare), resourceGroupName, accountName, name, sizeGiB) } // DeleteFileShare mocks base method -func (m *MockInterface) DeleteFileShare(accountName, accountKey, name string) error { +func (m *MockInterface) DeleteFileShare(resourceGroupName, accountName, name string) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DeleteFileShare", accountName, accountKey, name) + ret := m.ctrl.Call(m, "DeleteFileShare", resourceGroupName, accountName, name) ret0, _ := ret[0].(error) return ret0 } // DeleteFileShare indicates an expected call of DeleteFileShare -func (mr *MockInterfaceMockRecorder) DeleteFileShare(accountName, accountKey, name interface{}) *gomock.Call { +func (mr *MockInterfaceMockRecorder) DeleteFileShare(resourceGroupName, accountName, name interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteFileShare", reflect.TypeOf((*MockInterface)(nil).DeleteFileShare), accountName, accountKey, name) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteFileShare", reflect.TypeOf((*MockInterface)(nil).DeleteFileShare), resourceGroupName, accountName, name) } // ResizeFileShare mocks base method -func (m *MockInterface) ResizeFileShare(accountName, accountKey, name string, sizeGiB int) error { +func (m *MockInterface) ResizeFileShare(resourceGroupName, accountName, name string, sizeGiB int) error { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "ResizeFileShare", accountName, accountKey, name, sizeGiB) + ret := m.ctrl.Call(m, "ResizeFileShare", resourceGroupName, accountName, name, sizeGiB) ret0, _ := ret[0].(error) return ret0 } // ResizeFileShare indicates an expected call of ResizeFileShare -func (mr *MockInterfaceMockRecorder) ResizeFileShare(accountName, accountKey, name, sizeGiB interface{}) *gomock.Call { +func (mr *MockInterfaceMockRecorder) ResizeFileShare(resourceGroupName, accountName, name, sizeGiB interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResizeFileShare", reflect.TypeOf((*MockInterface)(nil).ResizeFileShare), accountName, accountKey, name, sizeGiB) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "ResizeFileShare", reflect.TypeOf((*MockInterface)(nil).ResizeFileShare), resourceGroupName, accountName, name, sizeGiB) } From 01713f2411d8be01aa2742a5a2646dd8e3d923a4 Mon Sep 17 00:00:00 2001 From: ZeroMagic Date: Sat, 9 May 2020 02:51:26 +0000 Subject: [PATCH 2/2] chore: set resourceGroup into azure pv annotation Signed-off-by: ZeroMagic --- pkg/volume/azure_file/azure_file.go | 3 +++ pkg/volume/azure_file/azure_provision.go | 21 ++++++++++++++++++--- 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/pkg/volume/azure_file/azure_file.go b/pkg/volume/azure_file/azure_file.go index 0b0d4bd9d32..3db30787f60 100644 --- a/pkg/volume/azure_file/azure_file.go +++ b/pkg/volume/azure_file/azure_file.go @@ -169,6 +169,9 @@ func (plugin *azureFilePlugin) ExpandVolumeDevice( if err != nil { return oldSize, err } + if spec.PersistentVolume.ObjectMeta.Annotations[resourceGroupAnnotation] != "" { + resourceGroup = spec.PersistentVolume.ObjectMeta.Annotations[resourceGroupAnnotation] + } secretName, secretNamespace, err := getSecretNameAndNamespace(spec, spec.PersistentVolume.Spec.ClaimRef.Namespace) if err != nil { diff --git a/pkg/volume/azure_file/azure_provision.go b/pkg/volume/azure_file/azure_provision.go index e24651bfb61..abb5bb8750b 100644 --- a/pkg/volume/azure_file/azure_provision.go +++ b/pkg/volume/azure_file/azure_provision.go @@ -36,8 +36,12 @@ import ( utilstrings "k8s.io/utils/strings" ) -var _ volume.DeletableVolumePlugin = &azureFilePlugin{} -var _ volume.ProvisionableVolumePlugin = &azureFilePlugin{} +var ( + _ volume.DeletableVolumePlugin = &azureFilePlugin{} + _ volume.ProvisionableVolumePlugin = &azureFilePlugin{} + + resourceGroupAnnotation = "kubernetes.io/azure-file-resource-group" +) // Abstract interface to file share operations. // azure cloud provider should implement it @@ -62,6 +66,9 @@ func (plugin *azureFilePlugin) NewDeleter(spec *volume.Spec) (volume.Deleter, er klog.V(4).Infof("failed to get azure provider") return nil, err } + if spec.PersistentVolume != nil && spec.PersistentVolume.ObjectMeta.Annotations[resourceGroupAnnotation] != "" { + resourceGroup = spec.PersistentVolume.ObjectMeta.Annotations[resourceGroupAnnotation] + } return plugin.newDeleterInternal(spec, &azureSvc{}, azure, resourceGroup) } @@ -94,7 +101,7 @@ func (plugin *azureFilePlugin) newDeleterInternal(spec *volume.Spec, util azureU } func (plugin *azureFilePlugin) NewProvisioner(options volume.VolumeOptions) (volume.Provisioner, error) { - azure, _, err := getAzureCloudProvider(plugin.host.GetCloudProvider()) + azure, resourceGroup, err := getAzureCloudProvider(plugin.host.GetCloudProvider()) if err != nil { klog.V(4).Infof("failed to get azure provider") return nil, err @@ -102,6 +109,9 @@ func (plugin *azureFilePlugin) NewProvisioner(options volume.VolumeOptions) (vol if len(options.PVC.Spec.AccessModes) == 0 { options.PVC.Spec.AccessModes = plugin.GetAccessModes() } + if resourceGroup != "" { + options.PVC.ObjectMeta.Annotations[resourceGroupAnnotation] = resourceGroup + } return plugin.newProvisionerInternal(options, azure) } @@ -181,6 +191,10 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie shareName = strings.Replace(name, "--", "-", -1) } + if resourceGroup == "" { + resourceGroup = a.options.PVC.ObjectMeta.Annotations[resourceGroupAnnotation] + } + // when use azure file premium, account kind should be specified as FileStorage accountKind := string(storage.StorageV2) if strings.HasPrefix(strings.ToLower(sku), "premium") { @@ -203,6 +217,7 @@ func (a *azureFileProvisioner) Provision(selectedNode *v1.Node, allowedTopologie Labels: map[string]string{}, Annotations: map[string]string{ util.VolumeDynamicallyCreatedByKey: "azure-file-dynamic-provisioner", + resourceGroupAnnotation: resourceGroup, }, }, Spec: v1.PersistentVolumeSpec{