From ffce2a802f8dcae1998bf9563ad5c97f698eb792 Mon Sep 17 00:00:00 2001 From: Fan Shang Xiang Date: Wed, 25 Sep 2024 16:21:02 +0800 Subject: [PATCH] migrate mgmt client to track2 one --- pkg/azurefile/azurefile.go | 47 ++++--- pkg/azurefile/azurefile_dataplane_client.go | 3 +- .../azurefile_dataplane_client_test.go | 4 +- pkg/azurefile/azurefile_interface.go | 22 ++- pkg/azurefile/azurefile_interface_mock.go | 3 +- pkg/azurefile/azurefile_mgmt_client.go | 125 ++++++++++++++++++ pkg/azurefile/azurefile_mgmt_client_test.go | 67 ++++++++++ pkg/azurefile/azurefile_test.go | 9 +- pkg/azurefile/controllerserver.go | 40 +++--- pkg/azurefile/controllerserver_test.go | 54 +++----- vendor/modules.txt | 1 + .../mock_fileshareclient/interface.go | 116 ++++++++++++++++ 12 files changed, 404 insertions(+), 87 deletions(-) create mode 100644 pkg/azurefile/azurefile_mgmt_client.go create mode 100644 pkg/azurefile/azurefile_mgmt_client_test.go create mode 100644 vendor/sigs.k8s.io/cloud-provider-azure/pkg/azclient/fileshareclient/mock_fileshareclient/interface.go diff --git a/pkg/azurefile/azurefile.go b/pkg/azurefile/azurefile.go index 8f5c728007..1ca89d7184 100644 --- a/pkg/azurefile/azurefile.go +++ b/pkg/azurefile/azurefile.go @@ -28,6 +28,7 @@ import ( "sync" "time" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage" "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage" "github.com/Azure/azure-storage-file-go/azfile" "github.com/container-storage-interface/spec/lib/go/csi" @@ -49,7 +50,6 @@ import ( csicommon "sigs.k8s.io/azurefile-csi-driver/pkg/csi-common" "sigs.k8s.io/azurefile-csi-driver/pkg/mounter" fileutil "sigs.k8s.io/azurefile-csi-driver/pkg/util" - "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient" azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" azure "sigs.k8s.io/cloud-provider-azure/pkg/provider" "sigs.k8s.io/cloud-provider-azure/pkg/retry" @@ -444,31 +444,25 @@ func (d *Driver) Run(ctx context.Context) error { } // getFileShareQuota return (-1, nil) means file share does not exist -func (d *Driver) getFileShareQuota(ctx context.Context, subsID, resourceGroupName, accountName, fileShareName string, secrets map[string]string) (int, error) { +func (d *Driver) getFileShareQuota(ctx context.Context, accountOptions *azure.AccountOptions, fileShareName string, secrets map[string]string) (int, error) { + var fileClient azureFileClient + var err error if len(secrets) > 0 { accountName, accountKey, err := getStorageAccount(secrets) if err != nil { return -1, err } - fileClient, err := newAzureFileClient(accountName, accountKey, d.getStorageEndPointSuffix(), &retry.Backoff{Steps: 1}) + fileClient, err = newAzureFileClient(accountName, accountKey, d.getStorageEndPointSuffix(), &retry.Backoff{Steps: 1}) if err != nil { return -1, err } - return fileClient.GetFileShareQuota(ctx, fileShareName) - } - - fileShare, err := d.cloud.GetFileShare(ctx, subsID, resourceGroupName, accountName, fileShareName) - if err != nil { - if strings.Contains(err.Error(), "ShareNotFound") { - return -1, nil + } else { + fileClient, err = newAzureFileMgmtClient(d.cloud.ComputeClientFactory, accountOptions) + if err != nil { + return -1, err } - return -1, err } - - if fileShare.FileShareProperties == nil || fileShare.FileShareProperties.ShareQuota == nil { - return -1, fmt.Errorf("FileShareProperties or FileShareProperties.ShareQuota is nil") - } - return int(*fileShare.FileShareProperties.ShareQuota), nil + return fileClient.GetFileShareQuota(ctx, fileShareName) } // get file share info according to volume id, e.g. @@ -912,22 +906,31 @@ func isSupportedFSGroupChangePolicy(policy string) bool { } // CreateFileShare creates a file share -func (d *Driver) CreateFileShare(ctx context.Context, accountOptions *azure.AccountOptions, shareOptions *fileclient.ShareOptions, secrets map[string]string) error { +func (d *Driver) CreateFileShare(ctx context.Context, accountOptions *azure.AccountOptions, shareOptions *ShareOptions, secrets map[string]string) error { return wait.ExponentialBackoff(d.cloud.RequestBackoff(), func() (bool, error) { var err error + var fileClient azureFileClient if len(secrets) > 0 { accountName, accountKey, rerr := getStorageAccount(secrets) if rerr != nil { return true, rerr } - fileClient, rerr := newAzureFileClient(accountName, accountKey, d.getStorageEndPointSuffix(), &retry.Backoff{Steps: 1}) + fileClient, rerr = newAzureFileClient(accountName, accountKey, d.getStorageEndPointSuffix(), &retry.Backoff{Steps: 1}) if rerr != nil { return true, rerr } - err = fileClient.CreateFileShare(ctx, shareOptions) } else { - _, err = d.cloud.FileClient.WithSubscriptionID(accountOptions.SubscriptionID).CreateFileShare(ctx, accountOptions.ResourceGroup, accountOptions.Name, shareOptions, "") + _, _, err := d.cloud.EnsureStorageAccount(ctx, accountOptions, FileShareAccountNamePrefix) + if err != nil { + return true, fmt.Errorf("could not get storage key for storage account %s: %w", accountOptions.Name, err) + } + fileClient, err = newAzureFileMgmtClient(d.cloud.ComputeClientFactory, accountOptions) + if err != nil { + return true, err + } } + err = fileClient.CreateFileShare(ctx, shareOptions) + if isRetriableError(err) { klog.Warningf("CreateFileShare(%s) on account(%s) failed with error(%v), waiting for retrying", shareOptions.Name, accountOptions.Name, err) sleepIfThrottled(err, fileOpThrottlingSleepSec) @@ -1005,8 +1008,8 @@ func (d *Driver) ResizeFileShare(ctx context.Context, subsID, resourceGroup, acc } // copyFileShare copies a fileshare, if dstAccountName is empty, then copy in the same account -func (d *Driver) copyFileShare(ctx context.Context, req *csi.CreateVolumeRequest, dstAccountName string, dstAccountSasToken string, authAzcopyEnv []string, secretNamespace string, shareOptions *fileclient.ShareOptions, accountOptions *azure.AccountOptions, storageEndpointSuffix string) error { - if shareOptions.Protocol == storage.EnabledProtocolsNFS { +func (d *Driver) copyFileShare(ctx context.Context, req *csi.CreateVolumeRequest, dstAccountName string, dstAccountSasToken string, authAzcopyEnv []string, secretNamespace string, shareOptions *ShareOptions, accountOptions *azure.AccountOptions, storageEndpointSuffix string) error { + if shareOptions.Protocol == armstorage.EnabledProtocolsNFS { return fmt.Errorf("protocol nfs is not supported for volume cloning") } var sourceVolumeID string diff --git a/pkg/azurefile/azurefile_dataplane_client.go b/pkg/azurefile/azurefile_dataplane_client.go index 7171e14385..06a872bbe3 100644 --- a/pkg/azurefile/azurefile_dataplane_client.go +++ b/pkg/azurefile/azurefile_dataplane_client.go @@ -27,7 +27,6 @@ import ( "k8s.io/klog/v2" "sigs.k8s.io/cloud-provider-azure/pkg/azclient/utils" - "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient" "sigs.k8s.io/cloud-provider-azure/pkg/retry" ) @@ -77,7 +76,7 @@ func newAzureFileClient(accountName, accountKey, storageEndpointSuffix string, b }, nil } -func (f *azureFileDataplaneClient) CreateFileShare(ctx context.Context, shareOptions *fileclient.ShareOptions) error { +func (f *azureFileDataplaneClient) CreateFileShare(ctx context.Context, shareOptions *ShareOptions) error { if shareOptions == nil { return fmt.Errorf("shareOptions of account(%s) is nil", f.accountName) } diff --git a/pkg/azurefile/azurefile_dataplane_client_test.go b/pkg/azurefile/azurefile_dataplane_client_test.go index 9ea2d9aa32..bd230ba63c 100644 --- a/pkg/azurefile/azurefile_dataplane_client_test.go +++ b/pkg/azurefile/azurefile_dataplane_client_test.go @@ -22,8 +22,6 @@ import ( "reflect" "strings" "testing" - - "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient" ) func TestCreateFileShare(t *testing.T) { @@ -35,7 +33,7 @@ func TestCreateFileShare(t *testing.T) { { name: "", testFunc: func(t *testing.T) { - options := &fileclient.ShareOptions{ + options := &ShareOptions{ Name: "devstoreaccount1", RequestGiB: 10, } diff --git a/pkg/azurefile/azurefile_interface.go b/pkg/azurefile/azurefile_interface.go index fcae0241a6..8b73a4f738 100644 --- a/pkg/azurefile/azurefile_interface.go +++ b/pkg/azurefile/azurefile_interface.go @@ -19,11 +19,29 @@ package azurefile import ( "context" - "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage" ) +const ( + // FileShareAccountNamePrefix is the file share account name prefix + FileShareAccountNamePrefix = "f" +) + +// ShareOptions contains the fields which are used to create file share. +type ShareOptions struct { + Name string + Protocol armstorage.EnabledProtocols + RequestGiB int + // supported values: ""(by default), "TransactionOptimized", "Cool", "Hot", "Premium" + AccessTier string + // supported values: ""(by default), "AllSquash", "NoRootSquash", "RootSquash" + RootSquash string + // Metadata - A name-value pair to associate with the share as metadata. + Metadata map[string]*string +} + type azureFileClient interface { - CreateFileShare(ctx context.Context, shareOptions *fileclient.ShareOptions) error + CreateFileShare(ctx context.Context, shareOptions *ShareOptions) error DeleteFileShare(ctx context.Context, name string) error GetFileShareQuota(ctx context.Context, name string) (int, error) ResizeFileShare(ctx context.Context, name string, sizeGiB int) error diff --git a/pkg/azurefile/azurefile_interface_mock.go b/pkg/azurefile/azurefile_interface_mock.go index b0172b819c..2d933bf7e8 100644 --- a/pkg/azurefile/azurefile_interface_mock.go +++ b/pkg/azurefile/azurefile_interface_mock.go @@ -31,7 +31,6 @@ import ( reflect "reflect" gomock "go.uber.org/mock/gomock" - fileclient "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient" ) // MockAzureFileClient is a mock of AzureFileClient interface. @@ -58,7 +57,7 @@ func (m *MockAzureFileClient) EXPECT() *MockAzureFileClientMockRecorder { } // CreateFileShare mocks base method. -func (m *MockAzureFileClient) CreateFileShare(ctx context.Context, shareOptions *fileclient.ShareOptions) error { +func (m *MockAzureFileClient) CreateFileShare(ctx context.Context, shareOptions *ShareOptions) error { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "CreateFileShare", ctx, shareOptions) ret0, _ := ret[0].(error) diff --git a/pkg/azurefile/azurefile_mgmt_client.go b/pkg/azurefile/azurefile_mgmt_client.go new file mode 100644 index 0000000000..099009594c --- /dev/null +++ b/pkg/azurefile/azurefile_mgmt_client.go @@ -0,0 +1,125 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package azurefile + +import ( + "context" + "fmt" + + "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage" + "k8s.io/klog/v2" + "sigs.k8s.io/cloud-provider-azure/pkg/azclient" + "sigs.k8s.io/cloud-provider-azure/pkg/azclient/fileshareclient" + azure "sigs.k8s.io/cloud-provider-azure/pkg/provider" +) + +type azureFileMgmtClient struct { + fileShareClient fileshareclient.Interface + accountOptions *azure.AccountOptions +} + +func newAzureFileMgmtClient(clientFactory azclient.ClientFactory, accountOptions *azure.AccountOptions) (azureFileClient, error) { + if clientFactory == nil { + return nil, fmt.Errorf("clientFactory is nil") + } + if accountOptions == nil { + return nil, fmt.Errorf("accountOptions is nil") + } + fileShareClient, err := clientFactory.GetFileShareClientForSub(accountOptions.SubscriptionID) + if err != nil { + return nil, fmt.Errorf("failed to get file share client for subscription %s: %w", accountOptions.SubscriptionID, err) + } + + return &azureFileMgmtClient{ + accountOptions: accountOptions, + fileShareClient: fileShareClient, + }, nil + +} + +// 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 *azureFileMgmtClient) CreateFileShare(ctx context.Context, shareOptions *ShareOptions) error { + if shareOptions == nil { + return fmt.Errorf("shareOptions of account(%s) is nil", az.accountOptions.Name) + } + az.accountOptions.EnableHTTPSTrafficOnly = true + if shareOptions.Protocol == armstorage.EnabledProtocolsNFS { + az.accountOptions.EnableHTTPSTrafficOnly = false + } + shareOps := armstorage.FileShare{ + Name: to.Ptr(shareOptions.Name), + FileShareProperties: &armstorage.FileShareProperties{}, + } + if shareOptions.RequestGiB > 0 { + quota := int32(shareOptions.RequestGiB) + shareOps.FileShareProperties.ShareQuota = "a + } + if shareOptions.Protocol == armstorage.EnabledProtocolsNFS { + shareOps.FileShareProperties.EnabledProtocols = to.Ptr(shareOptions.Protocol) + } + if shareOptions.AccessTier != "" { + shareOps.FileShareProperties.AccessTier = to.Ptr(armstorage.ShareAccessTier(shareOptions.AccessTier)) + } + if shareOptions.RootSquash != "" { + shareOps.FileShareProperties.RootSquash = to.Ptr(armstorage.RootSquashType(shareOptions.RootSquash)) + } + if shareOptions.Metadata != nil { + shareOps.FileShareProperties.Metadata = shareOptions.Metadata + } + if _, err := az.fileShareClient.Create(ctx, az.accountOptions.ResourceGroup, az.accountOptions.Name, shareOptions.Name, shareOps); err != nil { + return fmt.Errorf("failed to create share %s in account %s: %w", shareOptions.Name, az.accountOptions.Name, err) + } + klog.V(4).Infof("created share %s in account %s", shareOptions.Name, az.accountOptions.Name) + return nil +} + +// DeleteFileShare deletes a file share using storage account name and key +func (az *azureFileMgmtClient) DeleteFileShare(ctx context.Context, shareName string) error { + if err := az.fileShareClient.Delete(ctx, az.accountOptions.ResourceGroup, az.accountOptions.Name, shareName); err != nil { + return err + } + klog.V(4).Infof("share %s deleted", shareName) + return nil +} + +// ResizeFileShare resizes a file share +func (az *azureFileMgmtClient) ResizeFileShare(ctx context.Context, name string, sizeGiB int) error { + fileShare, err := az.fileShareClient.Get(ctx, az.accountOptions.ResourceGroup, az.accountOptions.Name, name) + if err != nil { + return err + } + if int(*fileShare.FileShareProperties.ShareQuota) >= sizeGiB { + klog.Warningf("file share size(%dGi) is already greater or equal than requested size(%dGi), accountName: %s, shareName: %s", + *fileShare.FileShareProperties.ShareQuota, sizeGiB, az.accountOptions.Name, name) + return nil + } + + fileShare.FileShareProperties.ShareQuota = to.Ptr(int32(sizeGiB)) + _, err = az.fileShareClient.Update(ctx, az.accountOptions.ResourceGroup, az.accountOptions.Name, name, *fileShare) + return err +} + +// GetFileShare gets a file share +func (az *azureFileMgmtClient) GetFileShareQuota(ctx context.Context, name string) (int, error) { + share, err := az.fileShareClient.Get(ctx, az.accountOptions.ResourceGroup, az.accountOptions.Name, name) + if err != nil { + return -1, err + } + return int(*share.FileShareProperties.ShareQuota), nil +} diff --git a/pkg/azurefile/azurefile_mgmt_client_test.go b/pkg/azurefile/azurefile_mgmt_client_test.go new file mode 100644 index 0000000000..26b360797d --- /dev/null +++ b/pkg/azurefile/azurefile_mgmt_client_test.go @@ -0,0 +1,67 @@ +/* +Copyright 2020 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package azurefile + +import ( + "fmt" + "reflect" + "testing" + + "go.uber.org/mock/gomock" + "sigs.k8s.io/cloud-provider-azure/pkg/azclient/fileshareclient/mock_fileshareclient" + "sigs.k8s.io/cloud-provider-azure/pkg/azclient/mock_azclient" + "sigs.k8s.io/cloud-provider-azure/pkg/provider" +) + +func TestNewAzureMgmtFileClientClientFactoryNil(t *testing.T) { + _, actualErr := newAzureFileMgmtClient(nil, nil) + if actualErr != nil { + expectedErr := fmt.Errorf("clientFactory is nil") + if !reflect.DeepEqual(actualErr, expectedErr) { + t.Errorf("actualErr: (%v), expectedErr: (%v)", actualErr, expectedErr) + } + } +} +func TestNewAzureMgmtFileClientAccountOptionNil(t *testing.T) { + cntl := gomock.NewController(t) + defer cntl.Finish() + _, actualErr := newAzureFileMgmtClient(mock_azclient.NewMockClientFactory(cntl), nil) + if actualErr != nil { + expectedErr := fmt.Errorf("accountOptions is nil") + if !reflect.DeepEqual(actualErr, expectedErr) { + t.Errorf("actualErr: (%v), expectedErr: (%v)", actualErr, expectedErr) + } + } +} + +func TestNewAzureMgmtFileClient(t *testing.T) { + cntl := gomock.NewController(t) + defer cntl.Finish() + clientFactory := mock_azclient.NewMockClientFactory(cntl) + fileClient := mock_fileshareclient.NewMockInterface(cntl) + clientFactory.EXPECT().GetFileShareClientForSub("testsub").Return(fileClient, nil) + _, actualErr := newAzureFileMgmtClient(clientFactory, &provider.AccountOptions{ + SubscriptionID: "testsub", + ResourceGroup: "testrg", + }) + if actualErr != nil { + expectedErr := fmt.Errorf("accountOptions is nil") + if !reflect.DeepEqual(actualErr, expectedErr) { + t.Errorf("actualErr: (%v), expectedErr: (%v)", actualErr, expectedErr) + } + } +} diff --git a/pkg/azurefile/azurefile_test.go b/pkg/azurefile/azurefile_test.go index e8352897e5..79b2ae5c4b 100644 --- a/pkg/azurefile/azurefile_test.go +++ b/pkg/azurefile/azurefile_test.go @@ -35,6 +35,7 @@ import ( "github.com/stretchr/testify/assert" "go.uber.org/mock/gomock" "k8s.io/client-go/kubernetes/fake" + "github.com/onsi/ginkgo/v2" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient/mockfileclient" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/storageaccountclient/mockstorageaccountclient" @@ -107,6 +108,8 @@ func TestNewFakeDriver(t *testing.T) { assert.NotNil(t, d) } +ginkgo.Describe("AzureFile", func() { +}) func TestAppendDefaultCifsMountOptions(t *testing.T) { tests := []struct { options []string @@ -1010,7 +1013,11 @@ func TestGetFileShareQuota(t *testing.T) { d.cloud.FileClient = mockFileClient mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockedFileShareResp, test.mockedFileShareErr).AnyTimes() mockFileClient.EXPECT().WithSubscriptionID(gomock.Any()).Return(mockFileClient).AnyTimes() - quota, err := d.getFileShareQuota(context.TODO(), "", resourceGroupName, accountName, fileShareName, test.secrets) + quota, err := d.getFileShareQuota(context.TODO(), &azure.AccountOptions{ + ResourceGroup: resourceGroupName, + Name: accountName, + SubscriptionID: "subsID", + }, fileShareName, test.secrets) if !reflect.DeepEqual(err, test.expectedError) { t.Errorf("test name: %s, Unexpected error: %v, expected error: %v", test.desc, err, test.expectedError) } diff --git a/pkg/azurefile/controllerserver.go b/pkg/azurefile/controllerserver.go index 5ff9f336ed..249eb2ccdc 100644 --- a/pkg/azurefile/controllerserver.go +++ b/pkg/azurefile/controllerserver.go @@ -28,20 +28,17 @@ import ( volumehelper "sigs.k8s.io/azurefile-csi-driver/pkg/util" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage" "github.com/Azure/azure-sdk-for-go/sdk/storage/azfile/sas" "github.com/Azure/azure-sdk-for-go/sdk/storage/azfile/service" - "github.com/Azure/azure-sdk-for-go/services/storage/mgmt/2021-09-01/storage" "github.com/Azure/azure-storage-file-go/azfile" "github.com/container-storage-interface/spec/lib/go/csi" "github.com/google/uuid" - "google.golang.org/grpc/codes" "google.golang.org/grpc/status" - + timestamppb "google.golang.org/protobuf/types/known/timestamppb" "k8s.io/klog/v2" "k8s.io/utils/ptr" - - timestamppb "google.golang.org/protobuf/types/known/timestamppb" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient" azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" "sigs.k8s.io/cloud-provider-azure/pkg/metrics" @@ -317,15 +314,15 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) } if !isSupportedShareAccessTier(shareAccessTier) { - return nil, status.Errorf(codes.InvalidArgument, "shareAccessTier(%s) is not supported, supported ShareAccessTier list: %v", shareAccessTier, storage.PossibleShareAccessTierValues()) + return nil, status.Errorf(codes.InvalidArgument, "shareAccessTier(%s) is not supported, supported ShareAccessTier list: %v", shareAccessTier, armstorage.PossibleShareAccessTierValues()) } if !isSupportedAccountAccessTier(accountAccessTier) { - return nil, status.Errorf(codes.InvalidArgument, "accountAccessTier(%s) is not supported, supported AccountAccessTier list: %v", accountAccessTier, storage.PossibleAccessTierValues()) + return nil, status.Errorf(codes.InvalidArgument, "accountAccessTier(%s) is not supported, supported AccountAccessTier list: %v", accountAccessTier, armstorage.PossibleAccessTierValues()) } if !isSupportedRootSquashType(rootSquashType) { - return nil, status.Errorf(codes.InvalidArgument, "rootSquashType(%s) is not supported, supported RootSquashType list: %v", rootSquashType, storage.PossibleRootSquashTypeValues()) + return nil, status.Errorf(codes.InvalidArgument, "rootSquashType(%s) is not supported, supported RootSquashType list: %v", rootSquashType, armstorage.PossibleRootSquashTypeValues()) } if !isSupportedFSGroupChangePolicy(fsGroupChangePolicy) { @@ -341,7 +338,7 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) } enableHTTPSTrafficOnly := true - shareProtocol := storage.EnabledProtocolsSMB + shareProtocol := armstorage.EnabledProtocolsSMB var createPrivateEndpoint *bool if strings.EqualFold(networkEndpointType, privateEndpoint) { if strings.Contains(subnetName, ",") { @@ -353,14 +350,14 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) if fsType == nfs || protocol == nfs { if sku == "" { // NFS protocol only supports Premium storage - sku = string(storage.SkuNamePremiumLRS) + sku = string(armstorage.SKUNamePremiumLRS) } else if strings.HasPrefix(strings.ToLower(sku), standard) { return nil, status.Errorf(codes.InvalidArgument, "nfs protocol only supports premium storage, current account type: %s", sku) } protocol = nfs enableHTTPSTrafficOnly = false - shareProtocol = storage.EnabledProtocolsNFS + shareProtocol = armstorage.EnabledProtocolsNFS // NFS protocol does not need account key storeAccountKey = false // reset protocol field (compatible with "fsType: nfs") @@ -405,9 +402,9 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) } // account kind should be FileStorage for Premium File - accountKind := string(storage.KindStorageV2) + accountKind := string(armstorage.KindStorageV2) if strings.HasPrefix(strings.ToLower(sku), premium) { - accountKind = string(storage.KindFileStorage) + accountKind = string(armstorage.KindFileStorage) if fileShareSize < minimumPremiumShareSize { fileShareSize = minimumPremiumShareSize } @@ -552,14 +549,14 @@ func (d *Driver) CreateVolume(ctx context.Context, req *csi.CreateVolumeRequest) secret = createStorageAccountSecret(accountName, accountKey) // skip validating file share quota if useDataPlaneAPI } else { - if quota, err := d.getFileShareQuota(ctx, subsID, resourceGroup, accountName, validFileShareName, secret); err != nil { + if quota, err := d.getFileShareQuota(ctx, accountOptions, validFileShareName, secret); err != nil { return nil, status.Errorf(codes.Internal, "%v", err) } else if quota != -1 && quota < fileShareSize { return nil, status.Errorf(codes.AlreadyExists, "request file share(%s) already exists, but its capacity %d is smaller than %d", validFileShareName, quota, fileShareSize) } } - shareOptions := &fileclient.ShareOptions{ + shareOptions := &ShareOptions{ Name: validFileShareName, Protocol: shareProtocol, RequestGiB: fileShareSize, @@ -755,7 +752,7 @@ func (d *Driver) DeleteVolume(ctx context.Context, req *csi.DeleteVolumeRequest) } // copyVolume copy an azure file -func (d *Driver) copyVolume(ctx context.Context, req *csi.CreateVolumeRequest, accountName, accountSASToken string, authAzcopyEnv []string, secretNamespace string, shareOptions *fileclient.ShareOptions, accountOptions *azure.AccountOptions, storageEndpointSuffix string) error { +func (d *Driver) copyVolume(ctx context.Context, req *csi.CreateVolumeRequest, accountName, accountSASToken string, authAzcopyEnv []string, secretNamespace string, shareOptions *ShareOptions, accountOptions *azure.AccountOptions, storageEndpointSuffix string) error { vs := req.VolumeContentSource switch vs.Type.(type) { case *csi.VolumeContentSource_Snapshot: @@ -793,8 +790,13 @@ func (d *Driver) ValidateVolumeCapabilities(ctx context.Context, req *csi.Valida if subsID == "" { subsID = d.cloud.SubscriptionID } + accountOptions := &azure.AccountOptions{ + Name: accountName, + SubscriptionID: subsID, + ResourceGroup: resourceGroupName, + } - if quota, err := d.getFileShareQuota(ctx, subsID, resourceGroupName, accountName, fileShareName, req.GetSecrets()); err != nil { + if quota, err := d.getFileShareQuota(ctx, accountOptions, fileShareName, req.GetSecrets()); err != nil { return nil, status.Errorf(codes.Internal, "error checking if volume(%s) exists: %v", volumeID, err) } else if quota == -1 { return nil, status.Errorf(codes.NotFound, "the requested volume(%s) does not exist.", volumeID) @@ -1014,8 +1016,8 @@ func (d *Driver) ListSnapshots(_ context.Context, _ *csi.ListSnapshotsRequest) ( } // restoreSnapshot restores from a snapshot -func (d *Driver) restoreSnapshot(ctx context.Context, req *csi.CreateVolumeRequest, dstAccountName, dstAccountSasToken string, authAzcopyEnv []string, secretNamespace string, shareOptions *fileclient.ShareOptions, accountOptions *azure.AccountOptions, storageEndpointSuffix string) error { - if shareOptions.Protocol == storage.EnabledProtocolsNFS { +func (d *Driver) restoreSnapshot(ctx context.Context, req *csi.CreateVolumeRequest, dstAccountName, dstAccountSasToken string, authAzcopyEnv []string, secretNamespace string, shareOptions *ShareOptions, accountOptions *azure.AccountOptions, storageEndpointSuffix string) error { + if shareOptions.Protocol == armstorage.EnabledProtocolsNFS { return fmt.Errorf("protocol nfs is not supported for snapshot restore") } var sourceSnapshotID string diff --git a/pkg/azurefile/controllerserver_test.go b/pkg/azurefile/controllerserver_test.go index f6f8b315de..d451a7853a 100644 --- a/pkg/azurefile/controllerserver_test.go +++ b/pkg/azurefile/controllerserver_test.go @@ -30,9 +30,11 @@ import ( "sigs.k8s.io/azurefile-csi-driver/pkg/util" "sigs.k8s.io/cloud-provider-azure/pkg/azclient" - "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/fileclient" + "sigs.k8s.io/cloud-provider-azure/pkg/azclient/fileshareclient/mock_fileshareclient" + "sigs.k8s.io/cloud-provider-azure/pkg/azclient/mock_azclient" "sigs.k8s.io/cloud-provider-azure/pkg/provider/config" + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage" "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2022-07-01/network" "sigs.k8s.io/cloud-provider-azure/pkg/azureclients/subnetclient/mocksubnetclient" azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache" @@ -1039,19 +1041,6 @@ func TestCreateVolume(t *testing.T) { name: "Create disk returns error", testFunc: func(t *testing.T) { skipIfTestingOnWindows(t) - name := "baz" - sku := "sku" - kind := "StorageV2" - location := "centralus" - value := "foo bar" - accounts := []storage.Account{ - {Name: &name, Sku: &storage.Sku{Name: storage.SkuName(sku)}, Kind: storage.Kind(kind), Location: &location}, - } - keys := storage.AccountListKeysResult{ - Keys: &[]storage.AccountKey{ - {Value: &value}, - }, - } allParam := map[string]string{ skuNameField: "premium", @@ -1077,12 +1066,10 @@ func TestCreateVolume(t *testing.T) { EnableVHDDiskFeature: true, } d := NewFakeDriverCustomOptions(driverOptions) - - d.cloud = &azure.Cloud{} - d.cloud.KubeClient = fake.NewSimpleClientset() - ctrl := gomock.NewController(t) defer ctrl.Finish() + d.cloud = &azure.Cloud{} + d.cloud.KubeClient = fake.NewSimpleClientset() tests := []struct { desc string @@ -1102,18 +1089,13 @@ func TestCreateVolume(t *testing.T) { } for _, test := range tests { allParam[shareNameField] = test.fileSharename - mockFileClient := mockfileclient.NewMockInterface(ctrl) - d.cloud.FileClient = mockFileClient - mockStorageAccountsClient := mockstorageaccountclient.NewMockInterface(ctrl) - d.cloud.StorageAccountClient = mockStorageAccountsClient + clientFactory := mock_azclient.NewMockClientFactory(ctrl) + d.cloud.ComputeClientFactory = clientFactory + fileshareClient := mock_fileshareclient.NewMockInterface(ctrl) + clientFactory.EXPECT().GetFileShareClientForSub("").Return(fileshareClient, nil).AnyTimes() - mockFileClient.EXPECT().WithSubscriptionID(gomock.Any()).Return(mockFileClient).AnyTimes() - mockFileClient.EXPECT().CreateFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: nil}}, 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() - mockFileClient.EXPECT().GetFileShare(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileShare{FileShareProperties: &storage.FileShareProperties{ShareQuota: &fakeShareQuota}}, nil).AnyTimes() + fileshareClient.EXPECT().Create(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&armstorage.FileShare{FileShareProperties: &armstorage.FileShareProperties{ShareQuota: nil}}, nil).AnyTimes() _, err := d.CreateVolume(ctx, req) if !reflect.DeepEqual(err, test.expectedErr) { @@ -1735,7 +1717,7 @@ func TestCopyVolume(t *testing.T) { ctx := context.Background() expectedErr := fmt.Errorf("protocol nfs is not supported for snapshot restore") - err := d.copyVolume(ctx, req, "", "", []string{}, "", &fileclient.ShareOptions{Protocol: storage.EnabledProtocolsNFS}, nil, "core.windows.net") + err := d.copyVolume(ctx, req, "", "", []string{}, "", &ShareOptions{Protocol: armstorage.EnabledProtocolsNFS}, nil, "core.windows.net") if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -1768,7 +1750,7 @@ func TestCopyVolume(t *testing.T) { ctx := context.Background() expectedErr := status.Errorf(codes.NotFound, "error parsing volume id: \"unit-test\", should at least contain two #") - err := d.copyVolume(ctx, req, "", "", []string{}, "", &fileclient.ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") + err := d.copyVolume(ctx, req, "", "", []string{}, "", &ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -1801,7 +1783,7 @@ func TestCopyVolume(t *testing.T) { ctx := context.Background() expectedErr := fmt.Errorf("one or more of srcAccountName(unit-test), srcFileShareName(), dstFileShareName(dstFileshare) are empty") - err := d.copyVolume(ctx, req, "", "", []string{}, "", &fileclient.ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") + err := d.copyVolume(ctx, req, "", "", []string{}, "", &ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -1834,7 +1816,7 @@ func TestCopyVolume(t *testing.T) { ctx := context.Background() expectedErr := fmt.Errorf("protocol nfs is not supported for volume cloning") - err := d.copyVolume(ctx, req, "", "", []string{}, "", &fileclient.ShareOptions{Protocol: storage.EnabledProtocolsNFS}, nil, "core.windows.net") + err := d.copyVolume(ctx, req, "", "", []string{}, "", &ShareOptions{Protocol: armstorage.EnabledProtocolsNFS}, nil, "core.windows.net") if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -1867,7 +1849,7 @@ func TestCopyVolume(t *testing.T) { ctx := context.Background() expectedErr := status.Errorf(codes.NotFound, "error parsing volume id: \"unit-test\", should at least contain two #") - err := d.copyVolume(ctx, req, "", "", []string{}, "", &fileclient.ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") + err := d.copyVolume(ctx, req, "", "", []string{}, "", &ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -1900,7 +1882,7 @@ func TestCopyVolume(t *testing.T) { ctx := context.Background() expectedErr := fmt.Errorf("one or more of srcAccountName(unit-test), srcFileShareName(), dstFileShareName(dstFileshare) are empty") - err := d.copyVolume(ctx, req, "", "", []string{}, "", &fileclient.ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") + err := d.copyVolume(ctx, req, "", "", []string{}, "", &ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -1933,7 +1915,7 @@ func TestCopyVolume(t *testing.T) { ctx := context.Background() expectedErr := fmt.Errorf("one or more of srcAccountName(f5713de20cde511e8ba4900), srcFileShareName(fileshare), dstFileShareName() are empty") - err := d.copyVolume(ctx, req, "", "", []string{}, "", &fileclient.ShareOptions{}, nil, "core.windows.net") + err := d.copyVolume(ctx, req, "", "", []string{}, "", &ShareOptions{}, nil, "core.windows.net") if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } @@ -1974,7 +1956,7 @@ func TestCopyVolume(t *testing.T) { d.azcopy.ExecCmd = m expectedErr := fmt.Errorf("wait for the existing AzCopy job to complete, current copy percentage is 50.0%%") - err := d.copyVolume(ctx, req, "", "sastoken", []string{}, "", &fileclient.ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") + err := d.copyVolume(ctx, req, "", "sastoken", []string{}, "", &ShareOptions{Name: "dstFileshare"}, nil, "core.windows.net") if !reflect.DeepEqual(err, expectedErr) { t.Errorf("Unexpected error: %v", err) } diff --git a/vendor/modules.txt b/vendor/modules.txt index 24975773db..5a26c98a3b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1716,6 +1716,7 @@ sigs.k8s.io/cloud-provider-azure/pkg/azclient/blobservicepropertiesclient sigs.k8s.io/cloud-provider-azure/pkg/azclient/deploymentclient sigs.k8s.io/cloud-provider-azure/pkg/azclient/diskclient sigs.k8s.io/cloud-provider-azure/pkg/azclient/fileshareclient +sigs.k8s.io/cloud-provider-azure/pkg/azclient/fileshareclient/mock_fileshareclient sigs.k8s.io/cloud-provider-azure/pkg/azclient/identityclient sigs.k8s.io/cloud-provider-azure/pkg/azclient/interfaceclient sigs.k8s.io/cloud-provider-azure/pkg/azclient/ipgroupclient diff --git a/vendor/sigs.k8s.io/cloud-provider-azure/pkg/azclient/fileshareclient/mock_fileshareclient/interface.go b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/azclient/fileshareclient/mock_fileshareclient/interface.go new file mode 100644 index 0000000000..00f6bf3092 --- /dev/null +++ b/vendor/sigs.k8s.io/cloud-provider-azure/pkg/azclient/fileshareclient/mock_fileshareclient/interface.go @@ -0,0 +1,116 @@ +// /* +// Copyright The Kubernetes Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// */ + +// Code generated by MockGen. DO NOT EDIT. +// Source: fileshareclient/interface.go +// +// Generated by this command: +// +// mockgen -package mock_fileshareclient -source fileshareclient/interface.go +// + +// Package mock_fileshareclient is a generated GoMock package. +package mock_fileshareclient + +import ( + context "context" + reflect "reflect" + + armstorage "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/storage/armstorage" + gomock "go.uber.org/mock/gomock" +) + +// MockInterface is a mock of Interface interface. +type MockInterface struct { + ctrl *gomock.Controller + recorder *MockInterfaceMockRecorder +} + +// MockInterfaceMockRecorder is the mock recorder for MockInterface. +type MockInterfaceMockRecorder struct { + mock *MockInterface +} + +// NewMockInterface creates a new mock instance. +func NewMockInterface(ctrl *gomock.Controller) *MockInterface { + mock := &MockInterface{ctrl: ctrl} + mock.recorder = &MockInterfaceMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockInterface) EXPECT() *MockInterfaceMockRecorder { + return m.recorder +} + +// Create mocks base method. +func (m *MockInterface) Create(ctx context.Context, resourceGroupName, resourceName, parentResourceName string, resource armstorage.FileShare) (*armstorage.FileShare, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Create", ctx, resourceGroupName, resourceName, parentResourceName, resource) + ret0, _ := ret[0].(*armstorage.FileShare) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Create indicates an expected call of Create. +func (mr *MockInterfaceMockRecorder) Create(ctx, resourceGroupName, resourceName, parentResourceName, resource any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Create", reflect.TypeOf((*MockInterface)(nil).Create), ctx, resourceGroupName, resourceName, parentResourceName, resource) +} + +// Delete mocks base method. +func (m *MockInterface) Delete(ctx context.Context, resourceGroupName, parentResourceName, resourceName string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Delete", ctx, resourceGroupName, parentResourceName, resourceName) + ret0, _ := ret[0].(error) + return ret0 +} + +// Delete indicates an expected call of Delete. +func (mr *MockInterfaceMockRecorder) Delete(ctx, resourceGroupName, parentResourceName, resourceName any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockInterface)(nil).Delete), ctx, resourceGroupName, parentResourceName, resourceName) +} + +// Get mocks base method. +func (m *MockInterface) Get(ctx context.Context, resourceGroupName, parentResourceName, resourceName string) (*armstorage.FileShare, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Get", ctx, resourceGroupName, parentResourceName, resourceName) + ret0, _ := ret[0].(*armstorage.FileShare) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Get indicates an expected call of Get. +func (mr *MockInterfaceMockRecorder) Get(ctx, resourceGroupName, parentResourceName, resourceName any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Get", reflect.TypeOf((*MockInterface)(nil).Get), ctx, resourceGroupName, parentResourceName, resourceName) +} + +// Update mocks base method. +func (m *MockInterface) Update(ctx context.Context, resourceGroupName, resourceName, parentResourceName string, resource armstorage.FileShare) (*armstorage.FileShare, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Update", ctx, resourceGroupName, resourceName, parentResourceName, resource) + ret0, _ := ret[0].(*armstorage.FileShare) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Update indicates an expected call of Update. +func (mr *MockInterfaceMockRecorder) Update(ctx, resourceGroupName, resourceName, parentResourceName, resource any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Update", reflect.TypeOf((*MockInterface)(nil).Update), ctx, resourceGroupName, resourceName, parentResourceName, resource) +}