diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go index 9b150397d02..9744507c8cb 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer.go @@ -36,6 +36,7 @@ import ( servicehelpers "k8s.io/cloud-provider/service/helpers" "k8s.io/klog/v2" azcache "k8s.io/legacy-cloud-providers/azure/cache" + "k8s.io/legacy-cloud-providers/azure/retry" utilnet "k8s.io/utils/net" ) @@ -216,7 +217,7 @@ func (az *Cloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName stri klog.V(5).Infof("Delete service (%s): START clusterName=%q", serviceName, clusterName) serviceIPToCleanup, err := az.findServiceIPAddress(ctx, clusterName, service, isInternal) - if err != nil { + if err != nil && !retry.HasStatusForbiddenOrIgnoredError(err) { return err } @@ -225,7 +226,7 @@ func (az *Cloud) EnsureLoadBalancerDeleted(ctx context.Context, clusterName stri return err } - if _, err := az.reconcileLoadBalancer(clusterName, service, nil, false /* wantLb */); err != nil { + if _, err := az.reconcileLoadBalancer(clusterName, service, nil, false /* wantLb */); err != nil && !retry.HasStatusForbiddenOrIgnoredError(err) { return err } diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go index ceb249d3e12..f7eaffd730e 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/azure_loadbalancer_test.go @@ -366,6 +366,7 @@ func TestEnsureLoadBalancerDeleted(t *testing.T) { service v1.Service isInternalSvc bool expectCreateError bool + wrongRGAtDelete bool }{ { desc: "external service should be created and deleted successfully", @@ -385,6 +386,12 @@ func TestEnsureLoadBalancerDeleted(t *testing.T) { service: getResourceGroupTestService("service4", "random-rg", "1.2.3.4", 80), expectCreateError: true, }, + { + desc: "annotated service with different resourceGroup shouldn't be created but should be deleted successfully", + service: getResourceGroupTestService("service5", "random-rg", "", 80), + expectCreateError: true, + wrongRGAtDelete: true, + }, } ctrl := gomock.NewController(t) @@ -419,6 +426,9 @@ func TestEnsureLoadBalancerDeleted(t *testing.T) { expectedLBs = make([]network.LoadBalancer, 0) setMockLBs(az, ctrl, &expectedLBs, "service", 1, i+1, c.isInternalSvc) // finally, delete it. + if c.wrongRGAtDelete { + az.LoadBalancerResourceGroup = "nil" + } err = az.EnsureLoadBalancerDeleted(context.TODO(), testClusterName, &c.service) expectedLBs = make([]network.LoadBalancer, 0) mockLBsClient := mockloadbalancerclient.NewMockInterface(ctrl) diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_error.go b/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_error.go index 02403e7ef01..a0cedbef2d4 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_error.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_error.go @@ -286,3 +286,20 @@ func IsErrorRetriable(err error) bool { return strings.Contains(err.Error(), "Retriable: true") } + +// HasStatusForbiddenOrIgnoredError return true if the given error code is part of the error message +// This should only be used when trying to delete resources +func HasStatusForbiddenOrIgnoredError(err error) bool { + if err == nil { + return false + } + + if strings.Contains(err.Error(), fmt.Sprintf("HTTPStatusCode: %d", http.StatusNotFound)) { + return true + } + + if strings.Contains(err.Error(), fmt.Sprintf("HTTPStatusCode: %d", http.StatusForbidden)) { + return true + } + return false +} diff --git a/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_error_test.go b/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_error_test.go index ff905cb4209..eafb772172b 100644 --- a/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_error_test.go +++ b/staging/src/k8s.io/legacy-cloud-providers/azure/retry/azure_error_test.go @@ -352,3 +352,17 @@ func TestIsErrorRetriable(t *testing.T) { result = IsErrorRetriable(fmt.Errorf("Retriable: true")) assert.Equal(t, true, result) } + +func TestHasErrorCode(t *testing.T) { + // false case + result := HasStatusForbiddenOrIgnoredError(fmt.Errorf("HTTPStatusCode: 408")) + assert.False(t, result) + + // true case 404 + result = HasStatusForbiddenOrIgnoredError(fmt.Errorf("HTTPStatusCode: %d", http.StatusNotFound)) + assert.True(t, result) + + // true case 403 + result = HasStatusForbiddenOrIgnoredError(fmt.Errorf("HTTPStatusCode: %d", http.StatusForbidden)) + assert.True(t, result) +}