From 142778ac6c8263ded803158b6ca9415296ba6163 Mon Sep 17 00:00:00 2001 From: andyzhangx Date: Sat, 8 Feb 2020 04:26:57 +0000 Subject: [PATCH] fix: add non-retriable errors in azure clients --- .../legacy-cloud-providers/azure/azure.go | 14 +++++++++---- .../azure/retry/azure_retry.go | 12 +++++++++-- .../azure/retry/azure_retry_test.go | 21 ++++++++++++++++++- 3 files changed, 40 insertions(+), 7 deletions(-) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go index 3b8aec0f063..1f8b7031d04 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure.go +++ b/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 } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_retry.go b/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_retry.go index 120b7dffaa1..209a0c2bec0 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_retry.go +++ b/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 } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_retry_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_retry_test.go index 8799792a96f..17401b2f314 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_retry_test.go +++ b/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()) }