fix: properly garbage collecting orphaned network interfaces

pkg/cloudprovider/cloudprovider.go

@@ -138,6 +138,7 @@ func (c *CloudProvider) List(ctx context.Context) ([]*karpv1.NodeClaim, error) {
 	if err != nil {
 		return nil, fmt.Errorf("listing instances, %w", err)
 	}
+
 	var nodeClaims []*karpv1.NodeClaim
 	for _, instance := range instances {
 		instanceType, err := c.resolveInstanceTypeFromInstance(ctx, instance)
@@ -337,7 +338,6 @@ func (c *CloudProvider) instanceToNodeClaim(ctx context.Context, vm *armcompute.
 
 	labels[karpv1.CapacityTypeLabelKey] = instance.GetCapacityType(vm)
 
-	// TODO: v1beta1 new kes/labels
 	if tag, ok := vm.Tags[instance.NodePoolTagKey]; ok {
 		labels[karpv1.NodePoolLabelKey] = *tag
 	}

pkg/cloudprovider/suite_test.go

@@ -144,7 +144,7 @@ var _ = Describe("CloudProvider", func() {
 		nodeClaims, _ := cloudProvider.List(ctx)
 		Expect(azureEnv.AzureResourceGraphAPI.AzureResourceGraphResourcesBehavior.CalledWithInput.Len()).To(Equal(1))
 		queryRequest := azureEnv.AzureResourceGraphAPI.AzureResourceGraphResourcesBehavior.CalledWithInput.Pop().Query
-		Expect(*queryRequest.Query).To(Equal(instance.GetListQueryBuilder(azureEnv.AzureResourceGraphAPI.ResourceGroup).String()))
+		Expect(*queryRequest.Query).To(Equal(instance.GetVMListQueryBuilder(azureEnv.AzureResourceGraphAPI.ResourceGroup).String()))
 		Expect(nodeClaims).To(HaveLen(1))
 		Expect(nodeClaims[0]).ToNot(BeNil())
 		resp, _ := azureEnv.VirtualMachinesAPI.Get(ctx, azureEnv.AzureResourceGraphAPI.ResourceGroup, nodeClaims[0].Name, nil)

pkg/controllers/controllers.go

@@ -44,7 +44,10 @@ func NewControllers(ctx context.Context, mgr manager.Manager, kubeClient client.
 		nodeclasshash.NewController(kubeClient),
 		nodeclassstatus.NewController(kubeClient),
 		nodeclasstermination.NewController(kubeClient, recorder),
-		nodeclaimgarbagecollection.NewController(kubeClient, cloudProvider),
+
+		nodeclaimgarbagecollection.NewVirtualMachine(kubeClient, cloudProvider),
+		nodeclaimgarbagecollection.NewNetworkInterface(kubeClient, instanceProvider),
+
 		// TODO: nodeclaim tagging
 		inplaceupdate.NewController(kubeClient, instanceProvider),
 		status.NewController[*v1alpha2.AKSNodeClass](kubeClient, mgr.GetEventRecorderFor("karpenter")),

pkg/controllers/nodeclaim/garbagecollection/controller.go → pkg/controllers/nodeclaim/garbagecollection/instance_garbagecollection.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/awslabs/operatorpkg/singleton"
 
-	// "github.com/Azure/karpenter-provider-azure/pkg/cloudprovider"
 	"github.com/samber/lo"
 	"go.uber.org/multierr"
 	v1 "k8s.io/api/core/v1"
@@ -41,21 +40,21 @@ import (
 	corecloudprovider "sigs.k8s.io/karpenter/pkg/cloudprovider"
 )
 
-type Controller struct {
+type VirtualMachine struct {
 	kubeClient      client.Client
 	cloudProvider   corecloudprovider.CloudProvider
-	successfulCount uint64 // keeps track of successful reconciles for more aggressive requeueing near the start of the controller
+	successfulCount uint64 // keeps track of successful reconciles for more aggressive requeuing near the start of the controller
 }
 
-func NewController(kubeClient client.Client, cloudProvider corecloudprovider.CloudProvider) *Controller {
-	return &Controller{
+func NewVirtualMachine(kubeClient client.Client, cloudProvider corecloudprovider.CloudProvider) *VirtualMachine {
+	return &VirtualMachine{
 		kubeClient:      kubeClient,
 		cloudProvider:   cloudProvider,
 		successfulCount: 0,
 	}
 }
 
-func (c *Controller) Reconcile(ctx context.Context) (reconcile.Result, error) {
+func (c *VirtualMachine) Reconcile(ctx context.Context) (reconcile.Result, error) {
 	ctx = injection.WithControllerName(ctx, "instance.garbagecollection")
 
 	// We LIST VMs on the CloudProvider BEFORE we grab NodeClaims/Nodes on the cluster so that we make sure that, if
@@ -65,6 +64,7 @@ func (c *Controller) Reconcile(ctx context.Context) (reconcile.Result, error) {
 	if err != nil {
 		return reconcile.Result{}, fmt.Errorf("listing cloudprovider VMs, %w", err)
 	}
+
 	managedRetrieved := lo.Filter(retrieved, func(nc *karpv1.NodeClaim, _ int) bool {
 		return nc.DeletionTimestamp.IsZero()
 	})
@@ -93,7 +93,7 @@ func (c *Controller) Reconcile(ctx context.Context) (reconcile.Result, error) {
 	return reconcile.Result{RequeueAfter: lo.Ternary(c.successfulCount <= 20, time.Second*10, time.Minute*2)}, nil
 }
 
-func (c *Controller) garbageCollect(ctx context.Context, nodeClaim *karpv1.NodeClaim, nodeList *v1.NodeList) error {
+func (c *VirtualMachine) garbageCollect(ctx context.Context, nodeClaim *karpv1.NodeClaim, nodeList *v1.NodeList) error {
 	ctx = logging.WithLogger(ctx, logging.FromContext(ctx).With("provider-id", nodeClaim.Status.ProviderID))
 	if err := c.cloudProvider.Delete(ctx, nodeClaim); err != nil {
 		return corecloudprovider.IgnoreNodeClaimNotFoundError(err)
@@ -112,7 +112,7 @@ func (c *Controller) garbageCollect(ctx context.Context, nodeClaim *karpv1.NodeC
 	return nil
 }
 
-func (c *Controller) Register(_ context.Context, m manager.Manager) error {
+func (c *VirtualMachine) Register(_ context.Context, m manager.Manager) error {
 	return controllerruntime.NewControllerManagedBy(m).
 		Named("instance.garbagecollection").
 		WatchesRawSource(singleton.Source()).

pkg/controllers/nodeclaim/garbagecollection/nic_garbagecollection.go

@@ -0,0 +1,112 @@
+/*
+Portions Copyright (c) Microsoft Corporation.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package garbagecollection
+
+import (
+	"context"
+	"fmt"
+	"time"
+
+	"github.com/samber/lo"
+	"knative.dev/pkg/logging"
+
+	"github.com/awslabs/operatorpkg/singleton"
+	"k8s.io/apimachinery/pkg/util/sets"
+	"k8s.io/client-go/util/workqueue"
+	controllerruntime "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/manager"
+	"sigs.k8s.io/controller-runtime/pkg/reconcile"
+	karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1"
+	"sigs.k8s.io/karpenter/pkg/operator/injection"
+
+	"github.com/Azure/karpenter-provider-azure/pkg/providers/instance"
+)
+
+const (
+	NicReservationDuration = time.Second * 180
+	// We set this interval at 5 minutes, as thats how often our NRP limits are reset.
+	// See: https://learn.microsoft.com/en-us/azure/azure-resource-manager/management/request-limits-and-throttling#network-throttling
+	NicGarbageCollectionInterval = time.Minute * 5
+)
+
+type NetworkInterface struct {
+	kubeClient       client.Client
+	instanceProvider instance.Provider
+}
+
+func NewNetworkInterface(kubeClient client.Client, instanceProvider instance.Provider) *NetworkInterface {
+	return &NetworkInterface{
+		kubeClient:       kubeClient,
+		instanceProvider: instanceProvider,
+	}
+}
+
+func (c *NetworkInterface) populateUnremovableInterfaces(ctx context.Context) (sets.Set[string], error) {
+	unremovableInterfaces := sets.New[string]()
+	vms, err := c.instanceProvider.List(ctx)
+	if err != nil {
+		return unremovableInterfaces, fmt.Errorf("listing VMs: %w", err)
+	}
+	for _, vm := range vms {
+		unremovableInterfaces.Insert(lo.FromPtr(vm.Name))
+	}
+	nodeClaimList := &karpv1.NodeClaimList{}
+	if err := c.kubeClient.List(ctx, nodeClaimList); err != nil {
+		return unremovableInterfaces, fmt.Errorf("listing NodeClaims for NIC GC: %w", err)
+	}
+
+	for _, nodeClaim := range nodeClaimList.Items {
+		unremovableInterfaces.Insert(instance.GenerateResourceName(nodeClaim.Name))
+	}
+	return unremovableInterfaces, nil
+}
+
+func (c *NetworkInterface) Reconcile(ctx context.Context) (reconcile.Result, error) {
+	ctx = injection.WithControllerName(ctx, "networkinterface.garbagecollection")
+	nics, err := c.instanceProvider.ListNics(ctx)
+	if err != nil {
+		return reconcile.Result{}, fmt.Errorf("listing NICs: %w", err)
+	}
+
+	unremovableInterfaces, err := c.populateUnremovableInterfaces(ctx)
+	if err != nil {
+		return reconcile.Result{}, fmt.Errorf("error listing resources needed to populate unremovable nics %w", err)
+	}
+	workqueue.ParallelizeUntil(ctx, 100, len(nics), func(i int) {
+		nicName := lo.FromPtr(nics[i].Name)
+		if !unremovableInterfaces.Has(nicName) {
+			err := c.instanceProvider.DeleteNic(ctx, nicName)
+			if err != nil {
+				logging.FromContext(ctx).Error(err)
+				return
+			}
+
+			logging.FromContext(ctx).With("nic", nicName).Infof("garbage collected NIC")
+		}
+	})
+	return reconcile.Result{
+		RequeueAfter: NicGarbageCollectionInterval,
+	}, nil
+}
+
+func (c *NetworkInterface) Register(_ context.Context, m manager.Manager) error {
+	return controllerruntime.NewControllerManagedBy(m).
+		Named("networkinterface.garbagecollection").
+		WatchesRawSource(singleton.Source()).
+		Complete(singleton.AsReconciler(c))
+}