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.
This commit is contained in:
Phillip Stagnet 2020-08-13 16:41:40 +02:00
parent e23d83eead
commit eed98104af
No known key found for this signature in database
GPG Key ID: 029E4A7879FBE381
4 changed files with 44 additions and 2 deletions

View File

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

View File

@ -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)

View File

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

View File

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