Skip to content

Commit

Permalink
test: adding network garbage collection suite and happy path
Browse files Browse the repository at this point in the history
  • Loading branch information
Bryce-Soghigian committed Jan 16, 2025
1 parent 67a2c28 commit fd3609b
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ import (
const (
NICGCControllerName = "networkinterface.garbagecollection"
NicReservationDuration = time.Second * 180
// We set this interval at 5 minutes, as thats how often our NRP limits are reset.
// 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
VMReason = "vm"
NicReason = "nic"
NodeclaimReason = "nc"
NicGarbageCollectionInterval = time.Minute * 5
VMReason = "vm"
NicReason = "nic"
NodeclaimReason = "nc"
)

type NetworkInterfaceController struct {
Expand All @@ -55,7 +55,7 @@ type NetworkInterfaceController struct {
// A network interface is considered unremovable if it meets the following 3 criteria
// 1: Reserved by NRP: When creating a nic and attempting to assign it to a vm, the nic will be reserved for that vm arm_resource_id for 180 seconds
// 2: Belongs to a Nodeclaim: If a nodeclaim exists in the cluster we shouldn't attempt removing it
// 3: Belongs to VM: If the VM Garbage Collection controller is removing a vm, we should not attempt removing it in this controller, and delegate that responsibility to the vm gc controller since deleting a successfully provisioned vm has delete options to also clean up the associated nic
// 3: Belongs to VM: If the VM Garbage Collection controller is removing a vm, we should not attempt removing it in this controller, and delegate that responsibility to the vm gc controller since deleting a successfully provisioned vm has delete options to also clean up the associated nic
unremovableNics *cache.Cache
}

Expand All @@ -74,7 +74,7 @@ func NewNetworkInterfaceController(kubeClient client.Client, instanceProvider in
// 2: Belongs to a Nodeclaim: If a nodeclaim exists in the cluster we shouldn't attempt removing it
// 3: Belongs to VM: If the VM Garbage Collection controller is removing a vm, we should not attempt removing it in this controller, and delegate that responsibility to the vm gc controller since deleting a successfully provisioned vm has delete options to also clean up the associated nic
func (c *NetworkInterfaceController) populateUnremovableNics(ctx context.Context) error {
vms, err := c.instanceProvider.List(ctx)
vms, err := c.instanceProvider.List(ctx)
if err != nil {
return fmt.Errorf("listing VMs: %w", err)
}
Expand All @@ -85,7 +85,7 @@ func (c *NetworkInterfaceController) populateUnremovableNics(ctx context.Context
if err := c.kubeClient.List(ctx, nodeClaimList); err != nil {
return fmt.Errorf("listing NodeClaims for NIC GC: %w", err)
}

for _, nodeClaim := range nodeClaimList.Items {
c.unremovableNics.SetDefault(instance.GenerateResourceName(nodeClaim.Name), NodeclaimReason)
}
Expand All @@ -107,7 +107,7 @@ func (c *NetworkInterfaceController) Reconcile(ctx context.Context) (reconcile.R
if err != nil {
return reconcile.Result{}, fmt.Errorf("listing NICs: %w", err)
}

c.populateUnremovableNics(ctx)
errs := make([]error, len(nics))
workqueue.ParallelizeUntil(ctx, 100, len(nics), func(i int) {
Expand Down
29 changes: 28 additions & 1 deletion pkg/controllers/nodeclaim/garbagecollection/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import (
"github.com/Azure/karpenter-provider-azure/pkg/operator/options"
"github.com/Azure/karpenter-provider-azure/pkg/providers/instance"
"github.com/Azure/karpenter-provider-azure/pkg/utils"
. "github.com/Azure/karpenter-provider-azure/pkg/test/expectations"
. "github.com/Azure/karpenter-provider-azure/pkg/test"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -52,7 +54,7 @@ import (
coreoptions "sigs.k8s.io/karpenter/pkg/operator/options"
coretest "sigs.k8s.io/karpenter/pkg/test"
. "sigs.k8s.io/karpenter/pkg/test/expectations"

karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1"
)

Expand All @@ -65,6 +67,7 @@ var nodeClass *v1alpha2.AKSNodeClass
var cluster *state.Cluster
var cloudProvider *cloudprovider.CloudProvider
var virtualMachineGCController *garbagecollection.VirtualMachineController
var networkInterfaceGCController *garbagecollection.NetworkInterfaceController
var prov *provisioning.Provisioner

func TestAPIs(t *testing.T) {
Expand All @@ -81,6 +84,7 @@ var _ = BeforeSuite(func() {
azureEnv = test.NewEnvironment(ctx, env)
cloudProvider = cloudprovider.New(azureEnv.InstanceTypesProvider, azureEnv.InstanceProvider, events.NewRecorder(&record.FakeRecorder{}), env.Client, azureEnv.ImageProvider)
virtualMachineGCController = garbagecollection.NewVirtualMachineController(env.Client, cloudProvider)
networkInterfaceGCController = garbagecollection.NewNetworkInterfaceController(env.Client, azureEnv.InstanceProvider)
fakeClock = &clock.FakeClock{}
cluster = state.NewCluster(fakeClock, env.Client)
prov = provisioning.NewProvisioner(env.Client, events.NewRecorder(&record.FakeRecorder{}), cloudProvider, cluster, fakeClock)
Expand Down Expand Up @@ -332,3 +336,26 @@ var _ = Describe("VirtualMachine Garbage Collection", func() {
})
})
})

var _ = Describe("NetworkInterface Garbage Collection", func() {
Context("Basic", func() {

It("should delete a NIC if there is no associated VM", func() {
nic := Interface(
InterfaceOptions{
Tags: ManagedTags("default-np"),
},
)

azureEnv.NetworkInterfacesAPI.NetworkInterfaces.Store(lo.FromPtr(nic.ID), *nic)
nicsBeforeGC, err := azureEnv.InstanceProvider.ListNics(ctx)
ExpectNoError(err)
Expect(len(nicsBeforeGC)).To(Equal(1))
// add a nic to azure env, and call reconcile. It should show up in the list before reconcile
// then it should not showup after
ExpectSingletonReconciled(ctx, networkInterfaceGCController)
})

})
})

4 changes: 1 addition & 3 deletions pkg/providers/instance/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,7 @@ var _ = Describe("InstanceProvider", func() {
It("should only list nics that belong to karpenter", func() {
managedNic := test.Interface(
test.InterfaceOptions{
Tags: map[string]*string{
instance.NodePoolTagKey: lo.ToPtr(nodePool.Name),
},
Tags: test.ManagedTags(nodePool.Name),
},
)
unmanagedNic := test.Interface()
Expand Down
7 changes: 7 additions & 0 deletions pkg/test/networkinterfaces.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,3 +69,10 @@ func RandomName(prefix string) string {
// You could make this more robust by including additional random characters.
return prefix + "-" + k8srand.String(10)
}

func ManagedTags(nodepoolName string) map[string]*string {
return map[string]*string{
"karpenter.sh_cluster": lo.ToPtr("test-cluster"),
"karpenter.sh_nodepool": lo.ToPtr(nodepoolName),
}
}

0 comments on commit fd3609b

Please sign in to comment.