fix: check if Azure file share exists before calling Create to allow read-only permissions for existing shares

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(2).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)

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 := `{

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())