refactor: more instance-generic async provisioning

cmd/controller/main.go

@@ -52,7 +52,7 @@ func main() {
 
 	aksCloudProvider := cloudprovider.New(
 		op.InstanceTypesProvider,
-		op.InstanceProvider,
+		op.VMInstanceProvider,
 		op.EventRecorder,
 		op.GetClient(),
 		op.ImageProvider,
@@ -79,7 +79,7 @@ func main() {
 			op.GetClient(),
 			op.EventRecorder,
 			aksCloudProvider,
-			op.InstanceProvider,
+			op.VMInstanceProvider,
 			// TODO: still need to refactor ImageProvider side of things.
 			op.KubernetesVersionProvider,
 			op.ImageProvider,

cmd/controller/main_ccp.go

@@ -52,7 +52,7 @@ func main() {
 
 	aksCloudProvider := cloudprovider.New(
 		op.InstanceTypesProvider,
-		op.InstanceProvider,
+		op.VMInstanceProvider,
 		op.EventRecorder,
 		op.GetClient(),
 		op.ImageProvider,
@@ -79,7 +79,7 @@ func main() {
 			op.GetClient(),
 			op.EventRecorder,
 			aksCloudProvider,
-			op.InstanceProvider,
+			op.VMInstanceProvider,
 			// TODO: still need to refactor ImageProvider side of things.
 			op.KubernetesVersionProvider,
 			op.ImageProvider,

pkg/cloudprovider/drift.go

@@ -145,7 +145,7 @@ func (c *CloudProvider) isImageVersionDrifted(
 		return "", err
 	}
 
-	vm, err := c.instanceProvider.Get(ctx, id)
+	vm, err := c.vmInstanceProvider.Get(ctx, id)
 	if err != nil {
 		// TODO (charliedmcb): Do we need to handle vm not found here before its provisioned?
 		//     I don't think we can get to Drift, until after ProviderID is set, so this should be a real issue.
@@ -203,7 +203,7 @@ func (c *CloudProvider) isSubnetDrifted(ctx context.Context, nodeClaim *karpv1.N
 	nicName := instance.GenerateResourceName(nodeClaim.Name)
 
 	// TODO: Refactor all of AzConfig to be part of options
-	nic, err := c.instanceProvider.GetNic(ctx, options.FromContext(ctx).NodeResourceGroup, nicName)
+	nic, err := c.vmInstanceProvider.GetNic(ctx, options.FromContext(ctx).NodeResourceGroup, nicName)
 	if err != nil {
 		if sdkerrors.IsNotFoundErr(err) {
 			return "", nil

pkg/cloudprovider/suite_test.go

@@ -82,7 +82,7 @@ var _ = BeforeSuite(func() {
 	azureEnv = test.NewEnvironment(ctx, env)
 	fakeClock = clock.NewFakeClock(time.Now())
 	recorder = events.NewRecorder(&record.FakeRecorder{})
-	cloudProvider = New(azureEnv.InstanceTypesProvider, azureEnv.InstanceProvider, recorder, env.Client, azureEnv.ImageProvider)
+	cloudProvider = New(azureEnv.InstanceTypesProvider, azureEnv.VMInstanceProvider, recorder, env.Client, azureEnv.ImageProvider)
 	cluster = state.NewCluster(fakeClock, env.Client, cloudProvider)
 	prov = provisioning.NewProvisioner(env.Client, recorder, cloudProvider, cluster, fakeClock)
 })
@@ -208,7 +208,7 @@ var _ = Describe("CloudProvider", func() {
 					//     https://github.com/Azure/karpenter-provider-azure/blob/84e449787ec72268efb0c7af81ec87a6b3ee95fa/pkg/providers/instance/instance.go#L604
 					//     which has the sub const 12345678-1234-1234-1234-123456789012 passed in here:
 					//     https://github.com/Azure/karpenter-provider-azure/blob/84e449787ec72268efb0c7af81ec87a6b3ee95fa/pkg/test/environment.go#L152
-					ProviderID: utils.ResourceIDToProviderID(ctx, fmt.Sprintf("/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/%s/providers/Microsoft.Compute/virtualMachines/%s", rg, vmName)),
+					ProviderID: utils.VMResourceIDToProviderID(ctx, fmt.Sprintf("/subscriptions/12345678-1234-1234-1234-123456789012/resourceGroups/%s/providers/Microsoft.Compute/virtualMachines/%s", rg, vmName)),
 				},
 				ObjectMeta: metav1.ObjectMeta{
 					Labels: map[string]string{

pkg/controllers/controllers.go

@@ -47,7 +47,7 @@ func NewControllers(
 	kubeClient client.Client,
 	recorder events.Recorder,
 	cloudProvider cloudprovider.CloudProvider,
-	instanceProvider instance.Provider,
+	vmInstanceProvider instance.VMProvider,
 	kubernetesVersionProvider kubernetesversion.KubernetesVersionProvider,
 	nodeImageProvider imagefamily.NodeImageProvider,
 	inClusterKubernetesInterface kubernetes.Interface,
@@ -59,10 +59,10 @@ func NewControllers(
 		nodeclasstermination.NewController(kubeClient, recorder),
 
 		nodeclaimgarbagecollection.NewVirtualMachine(kubeClient, cloudProvider),
-		nodeclaimgarbagecollection.NewNetworkInterface(kubeClient, instanceProvider),
+		nodeclaimgarbagecollection.NewNetworkInterface(kubeClient, vmInstanceProvider),
 
 		// TODO: nodeclaim tagging
-		inplaceupdate.NewController(kubeClient, instanceProvider),
+		inplaceupdate.NewController(kubeClient, vmInstanceProvider),
 		status.NewController[*v1beta1.AKSNodeClass](kubeClient, mgr.GetEventRecorderFor("karpenter")),
 	}
 	return controllers

pkg/controllers/nodeclaim/garbagecollection/nic_garbagecollection.go

@@ -45,20 +45,20 @@ const (
 )
 
 type NetworkInterface struct {
-	kubeClient       client.Client
-	instanceProvider instance.Provider
+	kubeClient         client.Client
+	vmInstanceProvider instance.VMProvider
 }
 
-func NewNetworkInterface(kubeClient client.Client, instanceProvider instance.Provider) *NetworkInterface {
+func NewNetworkInterface(kubeClient client.Client, vmInstanceProvider instance.VMProvider) *NetworkInterface {
 	return &NetworkInterface{
-		kubeClient:       kubeClient,
-		instanceProvider: instanceProvider,
+		kubeClient:         kubeClient,
+		vmInstanceProvider: vmInstanceProvider,
 	}
 }
 
 func (c *NetworkInterface) populateUnremovableInterfaces(ctx context.Context) (sets.Set[string], error) {
 	unremovableInterfaces := sets.New[string]()
-	vms, err := c.instanceProvider.List(ctx)
+	vms, err := c.vmInstanceProvider.List(ctx)
 	if err != nil {
 		return unremovableInterfaces, fmt.Errorf("listing VMs: %w", err)
 	}
@@ -78,7 +78,7 @@ func (c *NetworkInterface) populateUnremovableInterfaces(ctx context.Context) (s
 
 func (c *NetworkInterface) Reconcile(ctx context.Context) (reconcile.Result, error) {
 	ctx = injection.WithControllerName(ctx, "networkinterface.garbagecollection")
-	nics, err := c.instanceProvider.ListNics(ctx)
+	nics, err := c.vmInstanceProvider.ListNics(ctx)
 	if err != nil {
 		return reconcile.Result{}, fmt.Errorf("listing NICs: %w", err)
 	}
@@ -90,7 +90,7 @@ func (c *NetworkInterface) Reconcile(ctx context.Context) (reconcile.Result, 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)
+			err := c.vmInstanceProvider.DeleteNic(ctx, nicName)
 			if err != nil {
 				log.FromContext(ctx).Error(err, "")
 				return

pkg/controllers/nodeclaim/garbagecollection/suite_test.go

@@ -81,9 +81,9 @@ var _ = BeforeSuite(func() {
 	env = coretest.NewEnvironment(coretest.WithCRDs(apis.CRDs...), coretest.WithCRDs(v1alpha1.CRDs...))
 	//	ctx, stop = context.WithCancel(ctx)
 	azureEnv = test.NewEnvironment(ctx, env)
-	cloudProvider = cloudprovider.New(azureEnv.InstanceTypesProvider, azureEnv.InstanceProvider, events.NewRecorder(&record.FakeRecorder{}), env.Client, azureEnv.ImageProvider)
+	cloudProvider = cloudprovider.New(azureEnv.InstanceTypesProvider, azureEnv.VMInstanceProvider, events.NewRecorder(&record.FakeRecorder{}), env.Client, azureEnv.ImageProvider)
 	virtualMachineGCController = garbagecollection.NewVirtualMachine(env.Client, cloudProvider)
-	networkInterfaceGCController = garbagecollection.NewNetworkInterface(env.Client, azureEnv.InstanceProvider)
+	networkInterfaceGCController = garbagecollection.NewNetworkInterface(env.Client, azureEnv.VMInstanceProvider)
 	fakeClock = &clock.FakeClock{}
 	cluster = state.NewCluster(fakeClock, env.Client, cloudProvider)
 	prov = provisioning.NewProvisioner(env.Client, events.NewRecorder(&record.FakeRecorder{}), cloudProvider, cluster, fakeClock)
@@ -137,9 +137,9 @@ var _ = Describe("VirtualMachine Garbage Collection", func() {
 			ExpectScheduled(ctx, env.Client, pod)
 			Expect(azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Len()).To(Equal(1))
 			vmName := azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VMName
-			vm, err = azureEnv.InstanceProvider.Get(ctx, vmName)
+			vm, err = azureEnv.VMInstanceProvider.Get(ctx, vmName)
 			Expect(err).To(BeNil())
-			providerID = utils.ResourceIDToProviderID(ctx, *vm.ID)
+			providerID = utils.VMResourceIDToProviderID(ctx, *vm.ID)
 		})
 
 		It("should not delete an instance if it was not launched by a NodeClaim", func() {
@@ -166,9 +166,9 @@ var _ = Describe("VirtualMachine Garbage Collection", func() {
 				ExpectScheduled(ctx, env.Client, pod)
 				if azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Len() == 1 {
 					vmName = azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VMName
-					vm, err = azureEnv.InstanceProvider.Get(ctx, vmName)
+					vm, err = azureEnv.VMInstanceProvider.Get(ctx, vmName)
 					Expect(err).To(BeNil())
-					providerID = utils.ResourceIDToProviderID(ctx, *vm.ID)
+					providerID = utils.VMResourceIDToProviderID(ctx, *vm.ID)
 					newVM := test.VirtualMachine(test.VirtualMachineOptions{
 						Name:         vmName,
 						NodepoolName: "default",
@@ -189,7 +189,7 @@ var _ = Describe("VirtualMachine Garbage Collection", func() {
 					defer GinkgoRecover()
 					defer wg.Done()
 
-					_, err := cloudProvider.Get(ctx, utils.ResourceIDToProviderID(ctx, id))
+					_, err := cloudProvider.Get(ctx, utils.VMResourceIDToProviderID(ctx, id))
 					Expect(err).To(HaveOccurred())
 					Expect(corecloudprovider.IsNodeClaimNotFoundError(err)).To(BeTrue())
 				}(id)
@@ -207,9 +207,9 @@ var _ = Describe("VirtualMachine Garbage Collection", func() {
 				ExpectScheduled(ctx, env.Client, pod)
 				if azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Len() == 1 {
 					vmName = azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VMName
-					vm, err = azureEnv.InstanceProvider.Get(ctx, vmName)
+					vm, err = azureEnv.VMInstanceProvider.Get(ctx, vmName)
 					Expect(err).To(BeNil())
-					providerID = utils.ResourceIDToProviderID(ctx, *vm.ID)
+					providerID = utils.VMResourceIDToProviderID(ctx, *vm.ID)
 					newVM := test.VirtualMachine(test.VirtualMachineOptions{
 						Name:         vmName,
 						NodepoolName: "default",
@@ -220,7 +220,7 @@ var _ = Describe("VirtualMachine Garbage Collection", func() {
 					azureEnv.VirtualMachinesAPI.Instances.Store(lo.FromPtr(newVM.ID), newVM)
 					nodeClaim := coretest.NodeClaim(karpv1.NodeClaim{
 						Status: karpv1.NodeClaimStatus{
-							ProviderID: utils.ResourceIDToProviderID(ctx, *vm.ID),
+							ProviderID: utils.VMResourceIDToProviderID(ctx, *vm.ID),
 						},
 					})
 					ids = append(ids, *vm.ID)
@@ -237,7 +237,7 @@ var _ = Describe("VirtualMachine Garbage Collection", func() {
 					defer GinkgoRecover()
 					defer wg.Done()
 
-					_, err := cloudProvider.Get(ctx, utils.ResourceIDToProviderID(ctx, id))
+					_, err := cloudProvider.Get(ctx, utils.VMResourceIDToProviderID(ctx, id))
 					Expect(err).ToNot(HaveOccurred())
 				}(id)
 			}
@@ -281,7 +281,7 @@ var _ = Describe("VirtualMachine Garbage Collection", func() {
 	var _ = Context("Basic", func() {
 		BeforeEach(func() {
 			vm = test.VirtualMachine(test.VirtualMachineOptions{Name: "vm-a", NodepoolName: "default"})
-			providerID = utils.ResourceIDToProviderID(ctx, lo.FromPtr(vm.ID))
+			providerID = utils.VMResourceIDToProviderID(ctx, lo.FromPtr(vm.ID))
 		})
 		It("should delete an instance if there is no NodeClaim owner", func() {
 			// Launch happened 10m ago
@@ -329,15 +329,15 @@ var _ = Describe("NetworkInterface Garbage Collection", func() {
 		})
 		azureEnv.NetworkInterfacesAPI.NetworkInterfaces.Store(lo.FromPtr(nic.ID), *nic)
 
-		nicsBeforeGC, err := azureEnv.InstanceProvider.ListNics(ctx)
+		nicsBeforeGC, err := azureEnv.VMInstanceProvider.ListNics(ctx)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(len(nicsBeforeGC)).To(Equal(1))
 
 		// Run garbage collection
 		ExpectSingletonReconciled(ctx, networkInterfaceGCController)
 
 		// Verify NIC still exists after GC
-		nicsAfterGC, err := azureEnv.InstanceProvider.ListNics(ctx)
+		nicsAfterGC, err := azureEnv.VMInstanceProvider.ListNics(ctx)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(len(nicsAfterGC)).To(Equal(1))
 	})
@@ -350,13 +350,13 @@ var _ = Describe("NetworkInterface Garbage Collection", func() {
 		})
 		azureEnv.NetworkInterfacesAPI.NetworkInterfaces.Store(lo.FromPtr(nic.ID), *nic)
 		azureEnv.NetworkInterfacesAPI.NetworkInterfaces.Store(lo.FromPtr(nic2.ID), *nic2)
-		nicsBeforeGC, err := azureEnv.InstanceProvider.ListNics(ctx)
+		nicsBeforeGC, err := azureEnv.VMInstanceProvider.ListNics(ctx)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(len(nicsBeforeGC)).To(Equal(2))
 		// 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)
-		nicsAfterGC, err := azureEnv.InstanceProvider.ListNics(ctx)
+		nicsAfterGC, err := azureEnv.VMInstanceProvider.ListNics(ctx)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(len(nicsAfterGC)).To(Equal(0))
 	})
@@ -369,7 +369,7 @@ var _ = Describe("NetworkInterface Garbage Collection", func() {
 		azureEnv.VirtualMachinesAPI.VirtualMachinesBehavior.Instances.Store(lo.FromPtr(managedVM.ID), *managedVM)
 		ExpectSingletonReconciled(ctx, networkInterfaceGCController)
 		// We should still have a network interface here
-		nicsAfterGC, err := azureEnv.InstanceProvider.ListNics(ctx)
+		nicsAfterGC, err := azureEnv.VMInstanceProvider.ListNics(ctx)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(len(nicsAfterGC)).To(Equal(1))
 
@@ -389,12 +389,12 @@ var _ = Describe("NetworkInterface Garbage Collection", func() {
 		azureEnv.VirtualMachinesAPI.VirtualMachinesBehavior.Instances.Store(lo.FromPtr(managedVM.ID), *managedVM)
 		ExpectSingletonReconciled(ctx, networkInterfaceGCController)
 		// We should still have a network interface here
-		nicsAfterGC, err := azureEnv.InstanceProvider.ListNics(ctx)
+		nicsAfterGC, err := azureEnv.VMInstanceProvider.ListNics(ctx)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(len(nicsAfterGC)).To(Equal(1))
 
 		ExpectSingletonReconciled(ctx, virtualMachineGCController)
-		nicsAfterVMReconciliation, err := azureEnv.InstanceProvider.ListNics(ctx)
+		nicsAfterVMReconciliation, err := azureEnv.VMInstanceProvider.ListNics(ctx)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(len(nicsAfterVMReconciliation)).To(Equal(0))
 

pkg/controllers/nodeclaim/inplaceupdate/controller.go

@@ -43,17 +43,17 @@ import (
 )
 
 type Controller struct {
-	kubeClient       client.Client
-	instanceProvider instance.Provider
+	kubeClient         client.Client
+	vmInstanceProvider instance.VMProvider
 }
 
 func NewController(
 	kubeClient client.Client,
-	instanceProvider instance.Provider,
+	vmInstanceProvider instance.VMProvider,
 ) *Controller {
 	return &Controller{
-		kubeClient:       kubeClient,
-		instanceProvider: instanceProvider,
+		kubeClient:         kubeClient,
+		vmInstanceProvider: vmInstanceProvider,
 	}
 }
 
@@ -92,7 +92,7 @@ func (c *Controller) Reconcile(ctx context.Context, nodeClaim *karpv1.NodeClaim)
 
 	stored := nodeClaim.DeepCopy()
 
-	vm, err := nodeclaimutils.GetVM(ctx, c.instanceProvider, nodeClaim)
+	vm, err := nodeclaimutils.GetVM(ctx, c.vmInstanceProvider, nodeClaim)
 	if err != nil {
 		return reconcile.Result{}, fmt.Errorf("getting VM for nodeClaim %s: %w", nodeClaim.Name, err)
 	}
@@ -150,7 +150,7 @@ func (c *Controller) applyPatch(
 
 	// Apply the update, if one is needed
 	if update != nil {
-		err := c.instanceProvider.Update(ctx, lo.FromPtr(vm.Name), *update)
+		err := c.vmInstanceProvider.Update(ctx, lo.FromPtr(vm.Name), *update)
 		if err != nil {
 			return fmt.Errorf("failed to apply update to VM, %w", err)
 		}

pkg/controllers/nodeclaim/inplaceupdate/patch.go

@@ -51,9 +51,9 @@ type patchParameters struct {
 	nodeClass *v1beta1.AKSNodeClass
 }
 
-var patchers = []func(*armcompute.VirtualMachineUpdate, *patchParameters, *armcompute.VirtualMachine) bool{
-	patchIdentities,
-	patchTags,
+var vmPatchers = []func(*armcompute.VirtualMachineUpdate, *patchParameters, *armcompute.VirtualMachine) bool{
+	patchVMIdentities,
+	patchVMTags,
 }
 
 func CalculateVMPatch(
@@ -70,7 +70,7 @@ func CalculateVMPatch(
 		nodeClaim: nodeClaim,
 	}
 
-	for _, patcher := range patchers {
+	for _, patcher := range vmPatchers {
 		patched := patcher(update, params, currentVM)
 		hasPatches = hasPatches || patched
 	}
@@ -82,7 +82,7 @@ func CalculateVMPatch(
 	return update
 }
 
-func patchIdentities(
+func patchVMIdentities(
 	update *armcompute.VirtualMachineUpdate,
 	params *patchParameters,
 	currentVM *armcompute.VirtualMachine,
@@ -105,7 +105,7 @@ func patchIdentities(
 	return true
 }
 
-func patchTags(
+func patchVMTags(
 	update *armcompute.VirtualMachineUpdate,
 	params *patchParameters,
 	currentVM *armcompute.VirtualMachine,