Skip to content

Commit

Permalink
chore: use vmname-nic for network interface names
Browse files Browse the repository at this point in the history
  • Loading branch information
tallaxes committed Apr 18, 2024
1 parent f83fadf commit 3ebebe1
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 8 deletions.
10 changes: 9 additions & 1 deletion pkg/providers/instance/armutils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
20 changes: 13 additions & 7 deletions pkg/providers/instance/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -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
Expand All @@ -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),
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
15 changes: 15 additions & 0 deletions pkg/providers/instance/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ var _ = Describe("InstanceProvider", func() {
})
azureEnv.Reset()
azureEnvNonZonal.Reset()
cluster.Reset()
})

var _ = AfterEach(func() {
Expand Down Expand Up @@ -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"))
})
})

0 comments on commit 3ebebe1

Please sign in to comment.