From 747813aed36fdef8a771ad437c889807e4e163e0 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Mon, 13 Jan 2025 13:48:28 +0000 Subject: [PATCH] chore: use original Cloud backoff for Delete and Resize file share --- pkg/azurefile/azurefile.go | 27 +++---------------- pkg/azurefile/utils.go | 16 +++++++++++ pkg/azurefile/utils_test.go | 53 +++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+), 24 deletions(-) diff --git a/pkg/azurefile/azurefile.go b/pkg/azurefile/azurefile.go index 7cca6338bb..e79f617ad4 100644 --- a/pkg/azurefile/azurefile.go +++ b/pkg/azurefile/azurefile.go @@ -943,16 +943,7 @@ func isSupportedFSGroupChangePolicy(policy string) bool { // CreateFileShare creates a file share func (d *Driver) CreateFileShare(ctx context.Context, accountOptions *storage.AccountOptions, shareOptions *ShareOptions, secrets map[string]string) error { - steps := d.cloud.Config.CloudProviderBackoffRetries - if steps < 1 { - steps = 1 - } - return wait.ExponentialBackoff(wait.Backoff{ - Steps: steps, - Factor: d.cloud.Config.CloudProviderBackoffExponent, - Jitter: d.cloud.Config.CloudProviderBackoffJitter, - Duration: time.Duration(d.cloud.Config.CloudProviderBackoffDuration) * time.Second, - }, func() (bool, error) { + return wait.ExponentialBackoff(getBackOff(d.cloud.Config), func() (bool, error) { var err error var fileClient azureFileClient if len(secrets) > 0 { @@ -983,13 +974,7 @@ func (d *Driver) CreateFileShare(ctx context.Context, accountOptions *storage.Ac // DeleteFileShare deletes a file share using storage account name and key func (d *Driver) DeleteFileShare(ctx context.Context, subsID, resourceGroup, accountName, shareName string, secrets map[string]string) error { - steps := d.cloud.Config.CloudProviderBackoffRetries - if steps < 1 { - steps = 1 - } - return wait.ExponentialBackoff(wait.Backoff{ - Steps: steps, - }, func() (bool, error) { + return wait.ExponentialBackoff(getBackOff(d.cloud.Config), func() (bool, error) { var err error if len(secrets) > 0 { accountName, accountKey, rerr := getStorageAccount(secrets) @@ -1035,13 +1020,7 @@ func (d *Driver) DeleteFileShare(ctx context.Context, subsID, resourceGroup, acc // ResizeFileShare resizes a file share func (d *Driver) ResizeFileShare(ctx context.Context, subsID, resourceGroup, accountName, shareName string, sizeGiB int, secrets map[string]string) error { - steps := d.cloud.Config.CloudProviderBackoffRetries - if steps < 1 { - steps = 1 - } - return wait.ExponentialBackoff(wait.Backoff{ - Steps: steps, - }, func() (bool, error) { + return wait.ExponentialBackoff(getBackOff(d.cloud.Config), func() (bool, error) { var err error if len(secrets) > 0 { accountName, accountKey, rerr := getStorageAccount(secrets) diff --git a/pkg/azurefile/utils.go b/pkg/azurefile/utils.go index b5921d43de..4bad8fb8c6 100644 --- a/pkg/azurefile/utils.go +++ b/pkg/azurefile/utils.go @@ -29,9 +29,11 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" clientset "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" "k8s.io/kubernetes/pkg/volume" + azureconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config" ) const ( @@ -345,3 +347,17 @@ func isConfidentialRuntimeClass(ctx context.Context, kubeClient clientset.Interf klog.Infof("runtimeClass %s handler: %s", runtimeClassName, runtimeClass.Handler) return runtimeClass.Handler == confidentialRuntimeClassHandler, nil } + +// getBackOff returns a backoff object based on the config +func getBackOff(config azureconfig.Config) wait.Backoff { + steps := config.CloudProviderBackoffRetries + if steps < 1 { + steps = 1 + } + return wait.Backoff{ + Steps: steps, + Factor: config.CloudProviderBackoffExponent, + Jitter: config.CloudProviderBackoffJitter, + Duration: time.Duration(config.CloudProviderBackoffDuration) * time.Second, + } +} diff --git a/pkg/azurefile/utils_test.go b/pkg/azurefile/utils_test.go index 911c10b74a..a066ba8607 100644 --- a/pkg/azurefile/utils_test.go +++ b/pkg/azurefile/utils_test.go @@ -29,8 +29,10 @@ import ( "github.com/container-storage-interface/spec/lib/go/csi" nodev1 "k8s.io/api/node/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/wait" fake "k8s.io/client-go/kubernetes/fake" utiltesting "k8s.io/client-go/util/testing" + azureconfig "sigs.k8s.io/cloud-provider-azure/pkg/provider/config" ) func TestSimpleLockEntry(t *testing.T) { @@ -856,3 +858,54 @@ func TestIsThrottlingError(t *testing.T) { } } } + +func TestGetBackOff(t *testing.T) { + tests := []struct { + desc string + config azureconfig.Config + expected wait.Backoff + }{ + { + desc: "default backoff", + config: azureconfig.Config{ + AzureClientConfig: azureconfig.AzureClientConfig{ + CloudProviderBackoffRetries: 0, + CloudProviderBackoffDuration: 5, + }, + CloudProviderBackoffExponent: 2, + CloudProviderBackoffJitter: 1, + }, + expected: wait.Backoff{ + Steps: 1, + Duration: 5 * time.Second, + Factor: 2, + Jitter: 1, + }, + }, + { + desc: "backoff with retries > 1", + config: azureconfig.Config{ + AzureClientConfig: azureconfig.AzureClientConfig{ + CloudProviderBackoffRetries: 3, + CloudProviderBackoffDuration: 4, + }, + CloudProviderBackoffExponent: 2, + CloudProviderBackoffJitter: 1, + }, + expected: wait.Backoff{ + Steps: 3, + Duration: 4 * time.Second, + Factor: 2, + Jitter: 1, + }, + }, + } + + for _, test := range tests { + result := getBackOff(test.config) + if !reflect.DeepEqual(result, test.expected) { + t.Errorf("desc: (%s), input: config(%v), getBackOff returned with backoff(%v), not equal to expected(%v)", + test.desc, test.config, result, test.expected) + } + } +}