Allow 404 error on lb deletion in azure

When trying to delete a loadbalancer after all resources in azure
including the resource group have been deleted we currently get an an
error when trying to delete the loadbalancer afterwards because the
resource group hasn't been found.

Change EnsureLoadBalancerDeleted function to not fail when the resource
group has already been deleted.

Author: phiphi282

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
 	}
 

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)

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
+}

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)
+}