fix: add non-retriable errors in azure clients

Author: andyzhangx

staging/src/k8s.io/legacy-cloud-providers/azure/azure.go

@@ -499,22 +499,28 @@ func (az *Cloud) InitializeCloudFromConfig(config *Config, fromSecret bool) erro
 	az.RouteTablesClient = routetableclient.New(azClientConfig.WithRateLimiter(config.RouteTableRateLimit))
 	az.LoadBalancerClient = loadbalancerclient.New(azClientConfig.WithRateLimiter(config.LoadBalancerRateLimit))
 	az.SecurityGroupsClient = securitygroupclient.New(azClientConfig.WithRateLimiter(config.SecurityGroupRateLimit))
-	az.VirtualMachinesClient = vmclient.New(azClientConfig.WithRateLimiter(config.VirtualMachineRateLimit))
 	az.PublicIPAddressesClient = publicipclient.New(azClientConfig.WithRateLimiter(config.PublicIPAddressRateLimit))
 	az.VirtualMachineScaleSetsClient = vmssclient.New(azClientConfig.WithRateLimiter(config.VirtualMachineScaleSetRateLimit))
-	az.DisksClient = diskclient.New(azClientConfig.WithRateLimiter(config.DiskRateLimit))
 	az.VirtualMachineSizesClient = vmsizeclient.New(azClientConfig.WithRateLimiter(config.VirtualMachineSizeRateLimit))
 	az.SnapshotsClient = snapshotclient.New(azClientConfig.WithRateLimiter(config.SnapshotRateLimit))
 	az.StorageAccountClient = storageaccountclient.New(azClientConfig.WithRateLimiter(config.StorageAccountRateLimit))
 
 	// fileClient is not based on armclient, but it's still backoff retried.
-	az.FileClient = newAzureFileClient(env, azClientConfig.Backoff)
+	az.FileClient = newAzureFileClient(env, azClientConfig.Backoff.WithNonRetriableErrors([]string{}, []int{http.StatusNotFound}))
+
+	vmClientConfig := azClientConfig.WithRateLimiter(config.VirtualMachineRateLimit)
+	vmClientConfig.Backoff = vmClientConfig.Backoff.WithNonRetriableErrors([]string{}, []int{http.StatusConflict})
+	az.VirtualMachinesClient = vmclient.New(vmClientConfig)
 
 	// Error "not an active Virtual Machine Scale Set VM" is not retriable for VMSS VM.
 	vmssVMClientConfig := azClientConfig.WithRateLimiter(config.VirtualMachineScaleSetRateLimit)
-	vmssVMClientConfig.Backoff = vmssVMClientConfig.Backoff.WithNonRetriableErrors([]string{vmssVMNotActiveErrorMessage})
+	vmssVMClientConfig.Backoff = vmssVMClientConfig.Backoff.WithNonRetriableErrors([]string{vmssVMNotActiveErrorMessage}, []int{http.StatusConflict})
 	az.VirtualMachineScaleSetVMsClient = vmssvmclient.New(vmssVMClientConfig)
 
+	disksClientConfig := azClientConfig.WithRateLimiter(config.DiskRateLimit)
+	disksClientConfig.Backoff = disksClientConfig.Backoff.WithNonRetriableErrors([]string{}, []int{http.StatusNotFound})
+	az.DisksClient = diskclient.New(disksClientConfig)
+
 	if az.MaximumLoadBalancerRuleCount == 0 {
 		az.MaximumLoadBalancerRuleCount = maximumLoadBalancerRuleCount
 	}

staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_retry.go

@@ -58,6 +58,8 @@ type Backoff struct {
 	Cap time.Duration
 	// The errors indicate that the request shouldn't do more retrying.
 	NonRetriableErrors []string
+	// The HTTPStatusCode indicates that the request shouldn't do more retrying.
+	NonRetriableHTTPStatusCodes []int
 }
 
 // NewBackoff creates a new Backoff.
@@ -71,10 +73,11 @@ func NewBackoff(duration time.Duration, factor float64, jitter float64, steps in
 	}
 }
 
-// WithNonRetriableErrors returns a new *Backoff with NonRetriableErrors assigned.
-func (b *Backoff) WithNonRetriableErrors(errs []string) *Backoff {
+// WithNonRetriableErrors returns a new *Backoff with NonRetriableErrors, NonRetriableHTTPStatusCodes assigned.
+func (b *Backoff) WithNonRetriableErrors(errs []string, httpStatusCodes []int) *Backoff {
 	newBackoff := *b
 	newBackoff.NonRetriableErrors = errs
+	newBackoff.NonRetriableHTTPStatusCodes = httpStatusCodes
 	return &newBackoff
 }
 
@@ -90,6 +93,11 @@ func (b *Backoff) isNonRetriableError(rerr *Error) bool {
 		}
 	}
 
+	for _, code := range b.NonRetriableHTTPStatusCodes {
+		if rerr.HTTPStatusCode == code {
+			return true
+		}
+	}
 	return false
 }
 

staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_retry_test.go

@@ -75,7 +75,11 @@ func TestStep(t *testing.T) {
 }
 
 func TestDoBackoffRetry(t *testing.T) {
-	backoff := &Backoff{Factor: 1.0, Steps: 3}
+	backoff := &Backoff{
+		Factor:                      1.0,
+		Steps:                       3,
+		NonRetriableHTTPStatusCodes: []int{http.StatusNotFound},
+	}
 	fakeRequest := &http.Request{
 		URL: &url.URL{
 			Host: "localhost",
@@ -122,4 +126,19 @@ func TestDoBackoffRetry(t *testing.T) {
 	assert.Equal(t, 1, client.Attempts())
 	assert.NotNil(t, resp)
 	assert.Equal(t, 429, resp.StatusCode)
+
+	// retry on non retriable error
+	r = mocks.NewResponseWithStatus("404 StatusNotFound", http.StatusNotFound)
+	client = mocks.NewSender()
+	client.AppendAndRepeatResponse(r, 1)
+	expectedErr = &Error{
+		Retriable:      true,
+		HTTPStatusCode: 404,
+		RawError:       fmt.Errorf("HTTP status code (404)"),
+	}
+	resp, err = doBackoffRetry(client, fakeRequest, backoff)
+	assert.NotNil(t, resp)
+	assert.Equal(t, 404, resp.StatusCode)
+	assert.Equal(t, expectedErr.Error(), err)
+	assert.Equal(t, 1, client.Attempts())
 }