fix: add getFileServicePropertiesCache to fix throttling issue

pkg/provider/azure.go

@@ -437,6 +437,8 @@ type Cloud struct {
 	plsCache azcache.Resource
 	// a timed cache storing storage account properties to avoid querying storage account frequently
 	storageAccountCache azcache.Resource
+	// a timed cache storing storage account file service properties to avoid querying storage account file service properties frequently
+	fileServicePropertiesCache azcache.Resource
 
 	// Add service lister to always get latest service
 	serviceLister corelisters.ServiceLister
@@ -860,7 +862,11 @@ func (az *Cloud) initCaches() (err error) {
 		return err
 	}
 
-	if az.storageAccountCache, err = az.newStorageAccountCache(); err != nil {
+	getter := func(key string) (interface{}, error) { return nil, nil }
+	if az.storageAccountCache, err = azcache.NewTimedCache(time.Minute, getter, az.Config.DisableAPICallCache); err != nil {
+		return err
+	}
+	if az.fileServicePropertiesCache, err = azcache.NewTimedCache(5*time.Minute, getter, az.Config.DisableAPICallCache); err != nil {
 		return err
 	}
 	return nil

pkg/provider/azure_fakes.go

@@ -18,6 +18,7 @@ package provider
 
 import (
 	"context"
+	"time"
 
 	"go.uber.org/mock/gomock"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -40,6 +41,7 @@ import (
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmclient/mockvmclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmssclient/mockvmssclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/azureclients/vmssvmclient/mockvmssvmclient"
+	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 	"sigs.k8s.io/cloud-provider-azure/pkg/provider/config"
 	utilsets "sigs.k8s.io/cloud-provider-azure/pkg/util/sets"
@@ -129,7 +131,10 @@ func GetTestCloud(ctrl *gomock.Controller) (az *Cloud) {
 	az.pipCache, _ = az.newPIPCache()
 	az.plsCache, _ = az.newPLSCache()
 	az.LoadBalancerBackendPool = NewMockBackendPool(ctrl)
-	az.storageAccountCache, _ = az.newStorageAccountCache()
+
+	getter := func(key string) (interface{}, error) { return nil, nil }
+	az.storageAccountCache, _ = azcache.NewTimedCache(time.Minute, getter, az.Config.DisableAPICallCache)
+	az.fileServicePropertiesCache, _ = azcache.NewTimedCache(5*time.Minute, getter, az.Config.DisableAPICallCache)
 
 	az.regionZonesMap = map[string][]string{az.Location: {"1", "2", "3"}}
 

pkg/provider/azure_storageaccount.go

@@ -35,7 +35,6 @@ import (
 
 	"sigs.k8s.io/cloud-provider-azure/pkg/azclient/accountclient"
 	"sigs.k8s.io/cloud-provider-azure/pkg/cache"
-	azcache "sigs.k8s.io/cloud-provider-azure/pkg/cache"
 	"sigs.k8s.io/cloud-provider-azure/pkg/consts"
 	"sigs.k8s.io/cloud-provider-azure/pkg/retry"
 )
@@ -513,7 +512,7 @@ func (az *Cloud) EnsureStorageAccount(ctx context.Context, accountOptions *Accou
 		}
 
 		if accountOptions.DisableFileServiceDeleteRetentionPolicy != nil || accountOptions.IsMultichannelEnabled != nil {
-			prop, err := az.FileClient.WithSubscriptionID(subsID).GetServiceProperties(ctx, resourceGroup, accountName)
+			prop, err := az.getFileServicePropertiesCache(ctx, accountOptions.SubscriptionID, accountOptions.ResourceGroup, accountName)
 			if err != nil {
 				return "", "", err
 			}
@@ -676,11 +675,6 @@ func (az *Cloud) createPrivateDNSZoneGroup(ctx context.Context, dnsZoneGroupName
 	return az.privatednszonegroupclient.CreateOrUpdate(ctx, vnetResourceGroup, privateEndpointName, dnsZoneGroupName, privateDNSZoneGroup, "", false).Error()
 }
 
-func (az *Cloud) newStorageAccountCache() (azcache.Resource, error) {
-	getter := func(key string) (interface{}, error) { return nil, nil }
-	return azcache.NewTimedCache(time.Minute, getter, az.Config.DisableAPICallCache)
-}
-
 func (az *Cloud) getStorageAccountWithCache(ctx context.Context, subsID, resourceGroup, account string) (storage.Account, *retry.Error) {
 	if az.StorageAccountClient == nil {
 		return storage.Account{}, retry.NewError(false, fmt.Errorf("StorageAccountClient is nil"))
@@ -711,6 +705,34 @@ func (az *Cloud) getStorageAccountWithCache(ctx context.Context, subsID, resourc
 	return result, nil
 }
 
+func (az *Cloud) getFileServicePropertiesCache(ctx context.Context, subsID, resourceGroup, account string) (storage.FileServiceProperties, error) {
+	if az.FileClient == nil {
+		return storage.FileServiceProperties{}, fmt.Errorf("FileClient is nil")
+	}
+	if az.fileServicePropertiesCache == nil {
+		return storage.FileServiceProperties{}, fmt.Errorf("fileServicePropertiesCache is nil")
+	}
+
+	// search in cache first
+	cache, err := az.fileServicePropertiesCache.Get(account, cache.CacheReadTypeDefault)
+	if err != nil {
+		return storage.FileServiceProperties{}, err
+	}
+	var result storage.FileServiceProperties
+	if cache != nil {
+		result = cache.(storage.FileServiceProperties)
+		klog.V(2).Infof("Get service properties(%s) from cache", account)
+	} else {
+		result, err = az.FileClient.WithSubscriptionID(subsID).GetServiceProperties(ctx, resourceGroup, account)
+		if err != nil {
+			return storage.FileServiceProperties{}, err
+		}
+		az.fileServicePropertiesCache.Set(account, result)
+	}
+
+	return result, nil
+}
+
 // AddStorageAccountTags add tags to storage account
 func (az *Cloud) AddStorageAccountTags(ctx context.Context, subsID, resourceGroup, account string, tags map[string]*string) *retry.Error {
 	// add lock to avoid concurrent update on the cache
@@ -917,7 +939,7 @@ func (az *Cloud) isMultichannelEnabledEqual(ctx context.Context, account storage
 		return false
 	}
 
-	prop, err := az.FileClient.WithSubscriptionID(accountOptions.SubscriptionID).GetServiceProperties(ctx, accountOptions.ResourceGroup, *account.Name)
+	prop, err := az.getFileServicePropertiesCache(ctx, accountOptions.SubscriptionID, accountOptions.ResourceGroup, *account.Name)
 	if err != nil {
 		klog.Warningf("GetServiceProperties(%s) under resource group(%s) failed with %v", *account.Name, accountOptions.ResourceGroup, err)
 		return false

pkg/provider/azure_storageaccount_test.go

@@ -619,6 +619,61 @@ func TestGetStorageAccountWithCache(t *testing.T) {
 	}
 }
 
+func TestGetFileServicePropertiesCache(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	ctx, cancel := getContextWithCancel()
+	defer cancel()
+
+	cloud := &Cloud{}
+
+	tests := []struct {
+		name                          string
+		subsID                        string
+		resourceGroup                 string
+		account                       string
+		setFileClient                 bool
+		setFileServicePropertiesCache bool
+		expectedErr                   string
+	}{
+		{
+			name:        "[failure] FileClient is nil",
+			expectedErr: "FileClient is nil",
+		},
+		{
+			name:          "[failure] fileServicePropertiesCache is nil",
+			setFileClient: true,
+			expectedErr:   "fileServicePropertiesCache is nil",
+		},
+		{
+			name:                          "[Success]",
+			setFileClient:                 true,
+			setFileServicePropertiesCache: true,
+			expectedErr:                   "",
+		},
+	}
+
+	for _, test := range tests {
+		if test.setFileClient {
+			mockFileClient := mockfileclient.NewMockInterface(ctrl)
+			cloud.FileClient = mockFileClient
+			mockFileClient.EXPECT().WithSubscriptionID(gomock.Any()).Return(mockFileClient).AnyTimes()
+			mockFileClient.EXPECT().GetServiceProperties(gomock.Any(), gomock.Any(), gomock.Any()).Return(storage.FileServiceProperties{}, nil).AnyTimes()
+		}
+		if test.setFileServicePropertiesCache {
+			getter := func(key string) (interface{}, error) { return nil, nil }
+			cloud.fileServicePropertiesCache, _ = cache.NewTimedCache(time.Minute, getter, false)
+		}
+
+		_, err := cloud.getFileServicePropertiesCache(ctx, test.subsID, test.resourceGroup, test.account)
+		assert.Equal(t, err == nil, test.expectedErr == "", fmt.Sprintf("returned error: %v", err), test.name)
+		if test.expectedErr != "" && err != nil {
+			assert.Equal(t, err.Error(), test.expectedErr, err.Error(), test.name)
+		}
+	}
+}
+
 func TestAddStorageAccountTags(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()