From a78d7b366c99cc421e840477c65590fbc9586b7e Mon Sep 17 00:00:00 2001 From: Tony Schneider Date: Fri, 1 Apr 2022 13:03:39 -0500 Subject: [PATCH] Add logic to reconcile failed Nic on az aro delete Co-authored-by: Ben Vesel --- pkg/cluster/delete.go | 29 ++++++++-- pkg/cluster/delete_test.go | 112 +++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 pkg/cluster/delete_test.go diff --git a/pkg/cluster/delete.go b/pkg/cluster/delete.go index 85f1e290b8e..56b37bb6427 100644 --- a/pkg/cluster/delete.go +++ b/pkg/cluster/delete.go @@ -12,6 +12,7 @@ import ( "strings" "time" + mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network" mgmtfeatures "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-07-01/features" "github.com/Azure/go-autorest/autorest" "github.com/Azure/go-autorest/autorest/azure" @@ -29,6 +30,19 @@ import ( "github.com/Azure/ARO-RP/pkg/util/stringutils" ) +func (m *manager) deleteNic(ctx context.Context, resource mgmtfeatures.GenericResourceExpanded) error { + resourceGroup := stringutils.LastTokenByte(m.doc.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/') + + if resource.ProvisioningState != nil && !strings.EqualFold(*resource.ProvisioningState, "succeeded") { + m.log.Printf("NIC '%s' is not in a succeeded provisioning state, attempting to reconcile prior to deletion.", *resource.ID) + err := m.interfaces.CreateOrUpdateAndWait(ctx, resourceGroup, *resource.Name, mgmtnetwork.Interface{}) + if err != nil { + return err + } + } + return m.interfaces.DeleteAndWait(ctx, resourceGroup, *resource.Name) +} + func (m *manager) deletePrivateDNSVirtualNetworkLinks(ctx context.Context, resourceID string) error { r, err := azure.ParseResourceID(resourceID) if err != nil { @@ -124,16 +138,17 @@ func (m *manager) disconnectSecurityGroup(ctx context.Context, resourceID string // parallel and waiting for completion before we proceed. Any type not in the // map is considered to be at level 0. Keys must be lower case. var deleteOrder = map[string]int{ - "microsoft.compute/virtualmachines": -1, // first, and before microsoft.compute/disks, microsoft.network/networkinterfaces - "microsoft.network/privatelinkservices": -1, // before microsoft.network/loadbalancers - "microsoft.network/privateendpoints": -1, // before microsoft.network/networkinterfaces + "microsoft.compute/virtualmachines": -2, // first, and before microsoft.compute/disks, microsoft.network/networkinterfaces + "microsoft.network/privatelinkservices": -2, // before microsoft.network/loadbalancers + "microsoft.network/privateendpoints": -2, // before microsoft.network/networkinterfaces + "microsoft.network/networkinterfaces": -1, // before microsoft.network/loadbalancers "microsoft.network/privatednszones": 1, // after everything else: get other deletions underway first } func (m *manager) deleteResources(ctx context.Context) error { resourceGroup := stringutils.LastTokenByte(m.doc.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/') - resources, err := m.resources.ListByResourceGroup(ctx, resourceGroup, "", "", nil) + resources, err := m.resources.ListByResourceGroup(ctx, resourceGroup, "", "provisioningState", nil) if detailedErr, ok := err.(autorest.DetailedError); ok && (detailedErr.StatusCode == http.StatusNotFound || detailedErr.StatusCode == http.StatusForbidden) { @@ -187,6 +202,12 @@ func (m *manager) deleteResources(ctx context.Context) error { if err != nil { return err } + + case "microsoft.network/networkinterfaces": + err = m.deleteNic(ctx, *resource) + if err != nil { + return err + } } m.log.Printf("deleting %s", *resource.ID) diff --git a/pkg/cluster/delete_test.go b/pkg/cluster/delete_test.go new file mode 100644 index 00000000000..9abc5544e4a --- /dev/null +++ b/pkg/cluster/delete_test.go @@ -0,0 +1,112 @@ +package cluster + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "context" + "fmt" + "testing" + + mgmtfeatures "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-07-01/features" + "github.com/Azure/go-autorest/autorest/to" + "github.com/golang/mock/gomock" + "github.com/sirupsen/logrus" + + "github.com/Azure/ARO-RP/pkg/api" + mock_network "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/network" + mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env" +) + +func TestDeleteNic(t *testing.T) { + ctx := context.Background() + subscription := "00000000-0000-0000-0000-000000000000" + clusterRG := "cluster-rg" + nicName := "nic-name" + location := "eastus" + + tests := []struct { + name string + mocks func(*mock_network.MockInterfacesClient) + provisioningState *string + wantErr string + }{ + { + name: "nic is in succeeded provisioning state", + mocks: func(networkInterfaces *mock_network.MockInterfacesClient) { + networkInterfaces.EXPECT().DeleteAndWait(gomock.Any(), clusterRG, nicName).Return(nil) + }, + provisioningState: to.StringPtr("SUCCEEDED"), + }, + { + name: "nic is in failed provisioning state", + mocks: func(networkInterfaces *mock_network.MockInterfacesClient) { + networkInterfaces.EXPECT().CreateOrUpdateAndWait(gomock.Any(), clusterRG, nicName, gomock.Any()).Return(nil) + networkInterfaces.EXPECT().DeleteAndWait(gomock.Any(), clusterRG, nicName).Return(nil) + }, + provisioningState: to.StringPtr("FAILED"), + }, + { + name: "provisioning state is failed and CreateOrUpdateAndWait returns error", + mocks: func(networkInterfaces *mock_network.MockInterfacesClient) { + networkInterfaces.EXPECT().CreateOrUpdateAndWait(gomock.Any(), clusterRG, nicName, gomock.Any()).Return(fmt.Errorf("Failed to update")) + }, + provisioningState: to.StringPtr("FAILED"), + wantErr: "Failed to update", + }, + { + name: "DeleteAndWait returns error", + mocks: func(networkInterfaces *mock_network.MockInterfacesClient) { + networkInterfaces.EXPECT().DeleteAndWait(gomock.Any(), clusterRG, nicName).Return(fmt.Errorf("Failed to delete")) + }, + provisioningState: to.StringPtr("SUCCEEDED"), + wantErr: "Failed to delete", + }, + { + name: "provisioningState is nil", + mocks: func(networkInterfaces *mock_network.MockInterfacesClient) { + networkInterfaces.EXPECT().DeleteAndWait(gomock.Any(), clusterRG, nicName).Return(nil) + }, + provisioningState: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + controller := gomock.NewController(t) + defer controller.Finish() + + env := mock_env.NewMockInterface(controller) + env.EXPECT().Location().AnyTimes().Return(location) + + networkInterfaces := mock_network.NewMockInterfacesClient(controller) + + tt.mocks(networkInterfaces) + + m := manager{ + log: logrus.NewEntry(logrus.StandardLogger()), + doc: &api.OpenShiftClusterDocument{ + OpenShiftCluster: &api.OpenShiftCluster{ + Properties: api.OpenShiftClusterProperties{ + ClusterProfile: api.ClusterProfile{ + ResourceGroupID: fmt.Sprintf("/subscriptions/%s/resourceGroups/%s", subscription, clusterRG), + }, + }, + }, + }, + interfaces: networkInterfaces, + } + + resource := mgmtfeatures.GenericResourceExpanded{ + Name: to.StringPtr(nicName), + ID: to.StringPtr(fmt.Sprintf("/subscriptions/%s/resourceGroups/%s/providers/Microsoft.Network/networkInterfaces/%s", subscription, clusterRG, nicName)), + ProvisioningState: tt.provisioningState, + } + + err := m.deleteNic(ctx, resource) + if err != nil && err.Error() != tt.wantErr { + t.Errorf("got error: '%s'", err.Error()) + } + }) + } +}