chore: use original Cloud backoff for Delete and Resize file share

kubernetes-sigs · Jan 13, 2025 · 747813a · 747813a
1 parent 0ab46a6
commit 747813a
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 24 deletions.
diff --git 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
@@ -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
@@ -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)
+		}
+	}
+}