Add logic to reconcile failed Nic on az aro delete

Co-authored-by: Ben Vesel <[email protected]>

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)

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