diff --git a/pkg/azurefile/azurefile.go b/pkg/azurefile/azurefile.go index 8451f33aa8..5f4829f9e7 100644 --- a/pkg/azurefile/azurefile.go +++ b/pkg/azurefile/azurefile.go @@ -1082,6 +1082,15 @@ func (d *Driver) CreateFileShare(ctx context.Context, accountOptions *storage.Ac return true, err } + // Check if the file share already exists before attempting to create it. + // This allows read-only permissions to suffice when no creation is needed. + _, quotaErr := fileClient.GetFileShareQuota(ctx, shareOptions.Name) + if quotaErr == nil { + klog.V(2).Infof("file share(%s) already exists, skip creating", shareOptions.Name) + return true, nil + } + klog.V(6).Infof("GetFileShareQuota(%s) on account(%s) returned error(%v), proceeding to create", shareOptions.Name, accountOptions.Name, quotaErr) + if err = fileClient.CreateFileShare(ctx, shareOptions); err != nil { if strings.Contains(err.Error(), "ShareAlreadyExists") { klog.Warningf("CreateFileShare(%s) on account(%s) failed with error(%v), return as success", shareOptions.Name, accountOptions.Name, err) diff --git a/pkg/azurefile/azurefile_test.go b/pkg/azurefile/azurefile_test.go index 8873b2cbba..e58b59cfbe 100644 --- a/pkg/azurefile/azurefile_test.go +++ b/pkg/azurefile/azurefile_test.go @@ -1003,6 +1003,72 @@ func TestGetFileShareQuota(t *testing.T) { } } +func TestDriverCreateFileShare(t *testing.T) { + d := NewFakeDriver() + d.cloud = &storage.AccountRepo{} + ctrl := gomock.NewController(t) + defer ctrl.Finish() + shareQuota := int32(10) + resourceGroupName := "rg" + accountName := "accountname" + fileShareName := "filesharename" + + tests := []struct { + desc string + mockedFileShareResp *armstorage.FileShare + mockedFileShareErr error + mockedCreateResp *armstorage.FileShare + mockedCreateErr error + expectedError error + expectCreate bool + }{ + { + desc: "file share already exists, skip create", + mockedFileShareResp: &armstorage.FileShare{FileShareProperties: &armstorage.FileShareProperties{ShareQuota: &shareQuota}}, + mockedFileShareErr: nil, + expectedError: nil, + expectCreate: false, + }, + { + desc: "file share does not exist, create it", + mockedFileShareResp: &armstorage.FileShare{}, + mockedFileShareErr: &azcore.ResponseError{StatusCode: http.StatusNotFound}, + mockedCreateResp: &armstorage.FileShare{}, + mockedCreateErr: nil, + expectedError: nil, + expectCreate: true, + }, + { + desc: "GetFileShareQuota returns non-404 error, fall through to create", + mockedFileShareResp: &armstorage.FileShare{}, + mockedFileShareErr: fmt.Errorf("quota check error"), + mockedCreateResp: &armstorage.FileShare{}, + mockedCreateErr: nil, + expectedError: nil, + expectCreate: true, + }, + } + + for _, test := range tests { + mockFileClient := mock_fileshareclient.NewMockInterface(ctrl) + clientFactory := mock_azclient.NewMockClientFactory(ctrl) + clientFactory.EXPECT().GetFileShareClientForSub(gomock.Any()).Return(mockFileClient, nil).AnyTimes() + d.cloud.ComputeClientFactory = clientFactory + mockFileClient.EXPECT().Get(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockedFileShareResp, test.mockedFileShareErr).AnyTimes() + if test.expectCreate { + mockFileClient.EXPECT().Create(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockedCreateResp, test.mockedCreateErr).Times(1) + } + err := d.CreateFileShare(context.TODO(), &storage.AccountOptions{ + ResourceGroup: resourceGroupName, + Name: accountName, + SubscriptionID: "subsID", + }, &ShareOptions{Name: fileShareName, RequestGiB: int(shareQuota)}, map[string]string{}, "") + if !reflect.DeepEqual(err, test.expectedError) { + t.Errorf("test[%s]: unexpected error: %v, expected error: %v", test.desc, err, test.expectedError) + } + } +} + func TestRun(t *testing.T) { fakeCredFile := "fake-cred-file.json" fakeCredContent := `{ diff --git a/pkg/azurefile/controllerserver_test.go b/pkg/azurefile/controllerserver_test.go index 382673e050..a92d12bf39 100644 --- a/pkg/azurefile/controllerserver_test.go +++ b/pkg/azurefile/controllerserver_test.go @@ -1196,7 +1196,7 @@ var _ = ginkgo.Describe("TestCreateVolume", func() { mockStorageAccountsClient.EXPECT().Create(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() mockStorageAccountsClient.EXPECT().GetProperties(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&armstorage.Account{Tags: map[string]*string{"TestKey": &tagValue}}, nil).AnyTimes() mockStorageAccountsClient.EXPECT().Update(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(nil, nil).AnyTimes() - mockFileClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&armstorage.FileShare{FileShareProperties: &armstorage.FileShareProperties{ShareQuota: &fakeShareQuota}}, nil).AnyTimes() + mockFileClient.EXPECT().Get(ctx, gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(&armstorage.FileShare{}, &azcore.ResponseError{StatusCode: http.StatusNotFound}).AnyTimes() _, err = d.CreateVolume(ctx, req) gomega.Expect(err).NotTo(gomega.HaveOccurred())