Skip to content

Commit 11ce44a

Browse files
Copilotandyzhangx
andcommitted
Check if Azure file share exists before calling Create to allow read-only permission
Co-authored-by: andyzhangx <4178417+andyzhangx@users.noreply.github.com> Agent-Logs-Url: https://github.com/kubernetes-sigs/azurefile-csi-driver/sessions/fd54079c-0b66-4abf-8a0b-bde03213b851
1 parent 2f93eed commit 11ce44a

File tree

2 files changed

+73
-0
lines changed

2 files changed

+73
-0
lines changed

pkg/azurefile/azurefile.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,13 @@ func (d *Driver) CreateFileShare(ctx context.Context, accountOptions *storage.Ac
10821082
return true, err
10831083
}
10841084

1085+
// Check if the file share already exists before attempting to create it.
1086+
// This allows read-only permissions to suffice when no creation is needed.
1087+
if _, err := fileClient.GetFileShareQuota(ctx, shareOptions.Name); err == nil {
1088+
klog.V(2).Infof("file share(%s) already exists, skip creating", shareOptions.Name)
1089+
return true, nil
1090+
}
1091+
10851092
if err = fileClient.CreateFileShare(ctx, shareOptions); err != nil {
10861093
if strings.Contains(err.Error(), "ShareAlreadyExists") {
10871094
klog.Warningf("CreateFileShare(%s) on account(%s) failed with error(%v), return as success", shareOptions.Name, accountOptions.Name, err)

pkg/azurefile/azurefile_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1003,6 +1003,72 @@ func TestGetFileShareQuota(t *testing.T) {
10031003
}
10041004
}
10051005

1006+
func TestDriverCreateFileShare(t *testing.T) {
1007+
d := NewFakeDriver()
1008+
d.cloud = &storage.AccountRepo{}
1009+
ctrl := gomock.NewController(t)
1010+
defer ctrl.Finish()
1011+
shareQuota := int32(10)
1012+
resourceGroupName := "rg"
1013+
accountName := "accountname"
1014+
fileShareName := "filesharename"
1015+
1016+
tests := []struct {
1017+
desc string
1018+
mockedFileShareResp *armstorage.FileShare
1019+
mockedFileShareErr error
1020+
mockedCreateResp *armstorage.FileShare
1021+
mockedCreateErr error
1022+
expectedError error
1023+
expectCreate bool
1024+
}{
1025+
{
1026+
desc: "file share already exists, skip create",
1027+
mockedFileShareResp: &armstorage.FileShare{FileShareProperties: &armstorage.FileShareProperties{ShareQuota: &shareQuota}},
1028+
mockedFileShareErr: nil,
1029+
expectedError: nil,
1030+
expectCreate: false,
1031+
},
1032+
{
1033+
desc: "file share does not exist, create it",
1034+
mockedFileShareResp: &armstorage.FileShare{},
1035+
mockedFileShareErr: &azcore.ResponseError{StatusCode: http.StatusNotFound},
1036+
mockedCreateResp: &armstorage.FileShare{},
1037+
mockedCreateErr: nil,
1038+
expectedError: nil,
1039+
expectCreate: true,
1040+
},
1041+
{
1042+
desc: "GetFileShareQuota returns non-404 error, proceed to create",
1043+
mockedFileShareResp: &armstorage.FileShare{},
1044+
mockedFileShareErr: fmt.Errorf("quota check error"),
1045+
mockedCreateResp: &armstorage.FileShare{},
1046+
mockedCreateErr: nil,
1047+
expectedError: nil,
1048+
expectCreate: true,
1049+
},
1050+
}
1051+
1052+
for _, test := range tests {
1053+
mockFileClient := mock_fileshareclient.NewMockInterface(ctrl)
1054+
clientFactory := mock_azclient.NewMockClientFactory(ctrl)
1055+
clientFactory.EXPECT().GetFileShareClientForSub(gomock.Any()).Return(mockFileClient, nil).AnyTimes()
1056+
d.cloud.ComputeClientFactory = clientFactory
1057+
mockFileClient.EXPECT().Get(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockedFileShareResp, test.mockedFileShareErr).AnyTimes()
1058+
if test.expectCreate {
1059+
mockFileClient.EXPECT().Create(context.TODO(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).Return(test.mockedCreateResp, test.mockedCreateErr).Times(1)
1060+
}
1061+
err := d.CreateFileShare(context.TODO(), &storage.AccountOptions{
1062+
ResourceGroup: resourceGroupName,
1063+
Name: accountName,
1064+
SubscriptionID: "subsID",
1065+
}, &ShareOptions{Name: fileShareName, RequestGiB: int(shareQuota)}, map[string]string{}, "")
1066+
if !reflect.DeepEqual(err, test.expectedError) {
1067+
t.Errorf("test[%s]: unexpected error: %v, expected error: %v", test.desc, err, test.expectedError)
1068+
}
1069+
}
1070+
}
1071+
10061072
func TestRun(t *testing.T) {
10071073
fakeCredFile := "fake-cred-file.json"
10081074
fakeCredContent := `{

0 commit comments

Comments
 (0)