From 3ebebe1fe898423263eca5cc5c073c3eb4147dbc Mon Sep 17 00:00:00 2001 From: tallaxes <18728999+tallaxes@users.noreply.github.com> Date: Thu, 18 Apr 2024 02:58:54 +0000 Subject: [PATCH] chore: use vmname-nic for network interface names --- pkg/providers/instance/armutils.go | 10 +++++++++- pkg/providers/instance/instance.go | 20 +++++++++++++------- pkg/providers/instance/suite_test.go | 15 +++++++++++++++ 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/pkg/providers/instance/armutils.go b/pkg/providers/instance/armutils.go index 11bd6adac..09a365ae1 100644 --- a/pkg/providers/instance/armutils.go +++ b/pkg/providers/instance/armutils.go @@ -107,7 +107,15 @@ func deleteNicIfExists(ctx context.Context, client NetworkInterfacesAPI, rg, nic _, err := client.Get(ctx, rg, nicName, nil) if err != nil { if sdkerrors.IsNotFoundErr(err) { - return nil + // try older name; remove the -nic suffix from the nicName, if exists + if len(nicName) > 4 && nicName[len(nicName)-4:] == "-nic" { + nicName = nicName[:len(nicName)-4] + _, oerr := client.Get(ctx, rg, nicName, nil) + if sdkerrors.IsNotFoundErr(oerr) { + return nil + } + return oerr + } } return err } diff --git a/pkg/providers/instance/instance.go b/pkg/providers/instance/instance.go index 06d37810b..e4b0107e1 100644 --- a/pkg/providers/instance/instance.go +++ b/pkg/providers/instance/instance.go @@ -178,7 +178,7 @@ func (p *Provider) List(ctx context.Context) ([]*armcompute.VirtualMachine, erro } func (p *Provider) Delete(ctx context.Context, resourceName string) error { - logging.FromContext(ctx).Debugf("Deleting virtual machine %s and associated resources") + logging.FromContext(ctx).Debugf("Deleting virtual machine %s and associated resources", resourceName) return p.cleanupAzureResources(ctx, resourceName) } @@ -196,7 +196,7 @@ func (p *Provider) createAKSIdentifyingExtension(ctx context.Context, vmName str return nil } -func (p *Provider) newNetworkInterfaceForVM(vmName string, backendPools *loadbalancer.BackendAddressPools, instanceType *corecloudprovider.InstanceType) armnetwork.Interface { +func (p *Provider) newNetworkInterfaceForVM(backendPools *loadbalancer.BackendAddressPools, instanceType *corecloudprovider.InstanceType) armnetwork.Interface { var ipv4BackendPools []*armnetwork.BackendAddressPool for _, poolID := range backendPools.IPv4PoolIDs { poolID := poolID @@ -217,7 +217,7 @@ func (p *Provider) newNetworkInterfaceForVM(vmName string, backendPools *loadbal Properties: &armnetwork.InterfacePropertiesFormat{ IPConfigurations: []*armnetwork.InterfaceIPConfiguration{ { - Name: &vmName, + Name: to.Ptr("ipconfig1"), Properties: &armnetwork.InterfaceIPConfigurationPropertiesFormat{ Primary: to.Ptr(true), PrivateIPAllocationMethod: to.Ptr(armnetwork.IPAllocationMethodDynamic), @@ -244,7 +244,7 @@ func (p *Provider) createNetworkInterface(ctx context.Context, nicName string, l return "", err } - nic := p.newNetworkInterfaceForVM(nicName, backendPools, instanceType) + nic := p.newNetworkInterfaceForVM(backendPools, instanceType) p.applyTemplateToNic(&nic, launchTemplateConfig) logging.FromContext(ctx).Debugf("Creating network interface %s", nicName) res, err := createNic(ctx, p.azClient.networkInterfacesClient, p.resourceGroup, nicName, nic) @@ -386,7 +386,7 @@ func (p *Provider) launchInstance( resourceName := GenerateResourceName(nodeClaim.Name) // create network interface - nicReference, err := p.createNetworkInterface(ctx, resourceName, launchTemplate, instanceType) + nicReference, err := p.createNetworkInterface(ctx, getNetworkInterfaceName(resourceName), launchTemplate, instanceType) if err != nil { return nil, nil, err } @@ -410,6 +410,11 @@ func (p *Provider) launchInstance( return resp, instanceType, nil } +func getNetworkInterfaceName(vmName string) string { + // cloud-provider-azure has certain optimizations (can avoid extra API calls) if this pattern is followed + return vmName + "-nic" +} + // nolint:gocyclo func (p *Provider) handleResponseErrors(ctx context.Context, instanceType *corecloudprovider.InstanceType, zone, capacityType string, err error) error { if sdkerrors.LowPriorityQuotaHasBeenReached(err) { @@ -560,9 +565,10 @@ func (p *Provider) cleanupAzureResources(ctx context.Context, resourceName strin // The order here is intentional, if the VM was created successfully, then we attempt to delete the vm, the // nic, disk and all associated resources will be removed. If the VM was not created successfully and a nic was found, // then we attempt to delete the nic. - nicErr := deleteNicIfExists(ctx, p.azClient.networkInterfacesClient, p.resourceGroup, resourceName) + nicName := getNetworkInterfaceName(resourceName) + nicErr := deleteNicIfExists(ctx, p.azClient.networkInterfacesClient, p.resourceGroup, nicName) if nicErr != nil { - logging.FromContext(ctx).Errorf("networkInterface.Delete for %s failed: %v", resourceName, nicErr) + logging.FromContext(ctx).Errorf("networkInterface.Delete for %s failed: %v", nicName, nicErr) } return errors.Join(vmErr, nicErr) diff --git a/pkg/providers/instance/suite_test.go b/pkg/providers/instance/suite_test.go index f6a4da5b0..fa281cf19 100644 --- a/pkg/providers/instance/suite_test.go +++ b/pkg/providers/instance/suite_test.go @@ -118,6 +118,7 @@ var _ = Describe("InstanceProvider", func() { }) azureEnv.Reset() azureEnvNonZonal.Reset() + cluster.Reset() }) var _ = AfterEach(func() { @@ -165,4 +166,18 @@ var _ = Describe("InstanceProvider", func() { return strings.Contains(key, "/") // ARM tags can't contain '/' })).To(HaveLen(0)) }) + + It("should create Network Interfaces named vmname-nic", func() { + ExpectApplied(ctx, env.Client, nodePool, nodeClass) + pod := coretest.UnschedulablePod() + ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, coreProvisioner, pod) + ExpectScheduled(ctx, env.Client, pod) + newVMs := &azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput + Expect(newVMs.Len()).To(Equal(1)) + vmName := newVMs.Pop().VMName + newNics := &azureEnv.NetworkInterfacesAPI.NetworkInterfacesCreateOrUpdateBehavior.CalledWithInput + Expect(newNics.Len()).To(Equal(1)) + nicName := newNics.Pop().InterfaceName + Expect(nicName).To(Equal(vmName + "-nic")) + }) })