Skip to content

Commit 8b6d4de

Browse files
comtalystclaude
comtalyst
and
claude
committed
refactor: extract vminstancenic.go for NIC construction
Move NIC construction functions from vminstance.go to vminstancenic.go: - createNICOptions struct - newNetworkInterfaceForVM - applyTemplateToNic - createNetworkInterface Add buildAndCreateNIC orchestrator that consolidates the NIC creation logic (backend pools, NSG resolution, NIC construction) previously scattered inline in beginLaunchInstance. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 937d6d9 commit 8b6d4de

File tree

2 files changed

+180
-130
lines changed

2 files changed

+180
-130
lines changed

‎pkg/providers/instance/vminstance.go‎

Lines changed: 2 additions & 130 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ import (
2929
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v7"
3030
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork"
3131
"github.com/samber/lo"
32-
v1 "k8s.io/api/core/v1"
3332
"k8s.io/apimachinery/pkg/util/sets"
3433
"sigs.k8s.io/controller-runtime/pkg/log"
3534
karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1"
@@ -375,95 +374,11 @@ func (p *DefaultVMProvider) createCSExtension(ctx context.Context, vmName string
375374
return nil
376375
}
377376

378-
func (p *DefaultVMProvider) newNetworkInterfaceForVM(opts *createNICOptions) armnetwork.Interface {
379-
var ipv4BackendPools []*armnetwork.BackendAddressPool
380-
for _, poolID := range opts.BackendPools.IPv4PoolIDs {
381-
ipv4BackendPools = append(ipv4BackendPools, &armnetwork.BackendAddressPool{
382-
ID: &poolID,
383-
})
384-
}
385-
386-
skuAcceleratedNetworkingRequirements := scheduling.NewRequirements(
387-
scheduling.NewRequirement(v1beta1.LabelSKUAcceleratedNetworking, v1.NodeSelectorOpIn, "true"))
388-
389-
enableAcceleratedNetworking := false
390-
if err := opts.InstanceType.Requirements.Compatible(skuAcceleratedNetworkingRequirements); err == nil {
391-
enableAcceleratedNetworking = true
392-
}
393-
394-
var nsgRef *armnetwork.SecurityGroup
395-
if opts.NetworkSecurityGroupID != "" {
396-
nsgRef = &armnetwork.SecurityGroup{
397-
ID: &opts.NetworkSecurityGroupID,
398-
}
399-
}
400-
401-
nic := armnetwork.Interface{
402-
Location: lo.ToPtr(p.location),
403-
Properties: &armnetwork.InterfacePropertiesFormat{
404-
IPConfigurations: []*armnetwork.InterfaceIPConfiguration{
405-
{
406-
Name: &opts.NICName,
407-
Properties: &armnetwork.InterfaceIPConfigurationPropertiesFormat{
408-
Primary: lo.ToPtr(true),
409-
PrivateIPAllocationMethod: lo.ToPtr(armnetwork.IPAllocationMethodDynamic),
410-
411-
LoadBalancerBackendAddressPools: ipv4BackendPools,
412-
},
413-
},
414-
},
415-
NetworkSecurityGroup: nsgRef,
416-
EnableAcceleratedNetworking: lo.ToPtr(enableAcceleratedNetworking),
417-
EnableIPForwarding: lo.ToPtr(false),
418-
},
419-
}
420-
if opts.NetworkPlugin == consts.NetworkPluginAzure && opts.NetworkPluginMode != consts.NetworkPluginModeOverlay {
421-
// AzureCNI without overlay requires secondary IPs, for pods. (These IPs are not included in backend address pools.)
422-
// NOTE: Unlike AKS RP, this logic does not reduce secondary IP count by the number of expected hostNetwork pods, favoring simplicity instead
423-
for i := 1; i < int(opts.MaxPods); i++ {
424-
nic.Properties.IPConfigurations = append(
425-
nic.Properties.IPConfigurations,
426-
&armnetwork.InterfaceIPConfiguration{
427-
Name: lo.ToPtr(fmt.Sprintf("ipconfig%d", i)),
428-
Properties: &armnetwork.InterfaceIPConfigurationPropertiesFormat{
429-
Primary: lo.ToPtr(false),
430-
PrivateIPAllocationMethod: lo.ToPtr(armnetwork.IPAllocationMethodDynamic),
431-
},
432-
},
433-
)
434-
}
435-
}
436-
return nic
437-
}
438-
439377
// E.g., aks-default-2jf98
440378
func GenerateResourceName(nodeClaimName string) string {
441379
return fmt.Sprintf("aks-%s", nodeClaimName)
442380
}
443381

444-
type createNICOptions struct {
445-
NICName string
446-
BackendPools *loadbalancer.BackendAddressPools
447-
InstanceType *corecloudprovider.InstanceType
448-
LaunchTemplate *launchtemplate.Template
449-
NetworkPlugin string
450-
NetworkPluginMode string
451-
MaxPods int32
452-
NetworkSecurityGroupID string
453-
}
454-
455-
func (p *DefaultVMProvider) createNetworkInterface(ctx context.Context, opts *createNICOptions) (string, error) {
456-
nic := p.newNetworkInterfaceForVM(opts)
457-
p.applyTemplateToNic(&nic, opts.LaunchTemplate)
458-
log.FromContext(ctx).V(1).Info("creating network interface", "nicName", opts.NICName)
459-
res, err := createNic(ctx, p.azClient.NetworkInterfacesClient(), p.resourceGroup, opts.NICName, nic)
460-
if err != nil {
461-
return "", err
462-
}
463-
log.FromContext(ctx).V(1).Info("successfully created network interface", "nicName", opts.NICName, "nicID", *res.ID)
464-
return *res.ID, nil
465-
}
466-
467382
// createVMOptions contains all the parameters needed to create a VM
468383
type createVMOptions struct {
469384
VMName string
@@ -684,43 +599,8 @@ func (p *DefaultVMProvider) beginLaunchInstance(
684599
// resourceName for the NIC, VM, and Disk
685600
resourceName := GenerateResourceName(nodeClaim.Name)
686601

687-
backendPools, err := p.loadBalancerProvider.LoadBalancerBackendPools(ctx)
688-
if err != nil {
689-
return nil, fmt.Errorf("getting backend pools: %w", err)
690-
}
691-
networkPlugin := options.FromContext(ctx).NetworkPlugin
692-
networkPluginMode := options.FromContext(ctx).NetworkPluginMode
693-
694-
isAKSManagedVNET, err := utils.IsAKSManagedVNET(options.FromContext(ctx).NodeResourceGroup, launchTemplate.SubnetID)
695-
if err != nil {
696-
return nil, fmt.Errorf("checking if vnet is managed: %w", err)
697-
}
698-
var nsgID string
699-
if !isAKSManagedVNET {
700-
nsg, err := p.networkSecurityGroupProvider.ManagedNetworkSecurityGroup(ctx)
701-
if err != nil {
702-
return nil, fmt.Errorf("getting managed network security group: %w", err)
703-
}
704-
nsgID = lo.FromPtr(nsg.ID)
705-
}
706-
707-
// TODO: Not returning after launching this LRO because
708-
// TODO: doing so would bypass the capacity and other errors that are currently handled by
709-
// TODO: core pkg/controllers/nodeclaim/lifecycle/controller.go - in particular, there are metrics/events
710-
// TODO: emitted in capacity failure cases that we probably want.
711-
nicReference, err := p.createNetworkInterface(
712-
ctx,
713-
&createNICOptions{
714-
NICName: resourceName,
715-
NetworkPlugin: networkPlugin,
716-
NetworkPluginMode: networkPluginMode,
717-
MaxPods: utils.GetMaxPods(nodeClass, networkPlugin, networkPluginMode),
718-
LaunchTemplate: launchTemplate,
719-
BackendPools: backendPools,
720-
InstanceType: instanceType,
721-
NetworkSecurityGroupID: nsgID,
722-
},
723-
)
602+
// Create NIC
603+
nicReference, err := p.buildAndCreateNIC(ctx, resourceName, instanceType, nodeClass, launchTemplate)
724604
if err != nil {
725605
return nil, err
726606
}
@@ -821,14 +701,6 @@ func (p *DefaultVMProvider) beginLaunchInstance(
821701
}, nil
822702
}
823703

824-
func (p *DefaultVMProvider) applyTemplateToNic(nic *armnetwork.Interface, template *launchtemplate.Template) {
825-
// set tags
826-
nic.Tags = template.Tags
827-
for _, ipConfig := range nic.Properties.IPConfigurations {
828-
ipConfig.Properties.Subnet = &armnetwork.Subnet{ID: &template.SubnetID}
829-
}
830-
}
831-
832704
func (p *DefaultVMProvider) getLaunchTemplate(
833705
ctx context.Context,
834706
nodeClass *v1beta1.AKSNodeClass,

‎pkg/providers/instance/vminstancenic.go‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
/*
2+
Portions Copyright (c) Microsoft Corporation.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package instance
18+
19+
import (
20+
"context"
21+
"fmt"
22+
23+
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/network/armnetwork"
24+
"github.com/samber/lo"
25+
v1 "k8s.io/api/core/v1"
26+
"sigs.k8s.io/controller-runtime/pkg/log"
27+
corecloudprovider "sigs.k8s.io/karpenter/pkg/cloudprovider"
28+
"sigs.k8s.io/karpenter/pkg/scheduling"
29+
30+
"github.com/Azure/karpenter-provider-azure/pkg/apis/v1beta1"
31+
"github.com/Azure/karpenter-provider-azure/pkg/consts"
32+
"github.com/Azure/karpenter-provider-azure/pkg/operator/options"
33+
"github.com/Azure/karpenter-provider-azure/pkg/providers/launchtemplate"
34+
"github.com/Azure/karpenter-provider-azure/pkg/providers/loadbalancer"
35+
"github.com/Azure/karpenter-provider-azure/pkg/utils"
36+
)
37+
38+
type createNICOptions struct {
39+
NICName string
40+
BackendPools *loadbalancer.BackendAddressPools
41+
InstanceType *corecloudprovider.InstanceType
42+
LaunchTemplate *launchtemplate.Template
43+
NetworkPlugin string
44+
NetworkPluginMode string
45+
MaxPods int32
46+
NetworkSecurityGroupID string
47+
}
48+
49+
func (p *DefaultVMProvider) newNetworkInterfaceForVM(opts *createNICOptions) armnetwork.Interface {
50+
var ipv4BackendPools []*armnetwork.BackendAddressPool
51+
for _, poolID := range opts.BackendPools.IPv4PoolIDs {
52+
ipv4BackendPools = append(ipv4BackendPools, &armnetwork.BackendAddressPool{
53+
ID: &poolID,
54+
})
55+
}
56+
57+
skuAcceleratedNetworkingRequirements := scheduling.NewRequirements(
58+
scheduling.NewRequirement(v1beta1.LabelSKUAcceleratedNetworking, v1.NodeSelectorOpIn, "true"))
59+
60+
enableAcceleratedNetworking := false
61+
if err := opts.InstanceType.Requirements.Compatible(skuAcceleratedNetworkingRequirements); err == nil {
62+
enableAcceleratedNetworking = true
63+
}
64+
65+
var nsgRef *armnetwork.SecurityGroup
66+
if opts.NetworkSecurityGroupID != "" {
67+
nsgRef = &armnetwork.SecurityGroup{
68+
ID: &opts.NetworkSecurityGroupID,
69+
}
70+
}
71+
72+
nic := armnetwork.Interface{
73+
Location: lo.ToPtr(p.location),
74+
Properties: &armnetwork.InterfacePropertiesFormat{
75+
IPConfigurations: []*armnetwork.InterfaceIPConfiguration{
76+
{
77+
Name: &opts.NICName,
78+
Properties: &armnetwork.InterfaceIPConfigurationPropertiesFormat{
79+
Primary: lo.ToPtr(true),
80+
PrivateIPAllocationMethod: lo.ToPtr(armnetwork.IPAllocationMethodDynamic),
81+
82+
LoadBalancerBackendAddressPools: ipv4BackendPools,
83+
},
84+
},
85+
},
86+
NetworkSecurityGroup: nsgRef,
87+
EnableAcceleratedNetworking: lo.ToPtr(enableAcceleratedNetworking),
88+
EnableIPForwarding: lo.ToPtr(false),
89+
},
90+
}
91+
if opts.NetworkPlugin == consts.NetworkPluginAzure && opts.NetworkPluginMode != consts.NetworkPluginModeOverlay {
92+
// AzureCNI without overlay requires secondary IPs, for pods. (These IPs are not included in backend address pools.)
93+
// NOTE: Unlike AKS RP, this logic does not reduce secondary IP count by the number of expected hostNetwork pods, favoring simplicity instead
94+
for i := 1; i < int(opts.MaxPods); i++ {
95+
nic.Properties.IPConfigurations = append(
96+
nic.Properties.IPConfigurations,
97+
&armnetwork.InterfaceIPConfiguration{
98+
Name: lo.ToPtr(fmt.Sprintf("ipconfig%d", i)),
99+
Properties: &armnetwork.InterfaceIPConfigurationPropertiesFormat{
100+
Primary: lo.ToPtr(false),
101+
PrivateIPAllocationMethod: lo.ToPtr(armnetwork.IPAllocationMethodDynamic),
102+
},
103+
},
104+
)
105+
}
106+
}
107+
return nic
108+
}
109+
110+
func (p *DefaultVMProvider) applyTemplateToNic(nic *armnetwork.Interface, template *launchtemplate.Template) {
111+
// set tags
112+
nic.Tags = template.Tags
113+
for _, ipConfig := range nic.Properties.IPConfigurations {
114+
ipConfig.Properties.Subnet = &armnetwork.Subnet{ID: &template.SubnetID}
115+
}
116+
}
117+
118+
func (p *DefaultVMProvider) createNetworkInterface(ctx context.Context, opts *createNICOptions) (string, error) {
119+
nic := p.newNetworkInterfaceForVM(opts)
120+
p.applyTemplateToNic(&nic, opts.LaunchTemplate)
121+
log.FromContext(ctx).V(1).Info("creating network interface", "nicName", opts.NICName)
122+
res, err := createNic(ctx, p.azClient.NetworkInterfacesClient(), p.resourceGroup, opts.NICName, nic)
123+
if err != nil {
124+
return "", err
125+
}
126+
log.FromContext(ctx).V(1).Info("successfully created network interface", "nicName", opts.NICName, "nicID", *res.ID)
127+
return *res.ID, nil
128+
}
129+
130+
// buildAndCreateNIC consolidates NIC creation: fetches backend pools, resolves NSG if needed, builds and creates the NIC.
131+
func (p *DefaultVMProvider) buildAndCreateNIC(
132+
ctx context.Context,
133+
resourceName string,
134+
instanceType *corecloudprovider.InstanceType,
135+
nodeClass *v1beta1.AKSNodeClass,
136+
launchTemplate *launchtemplate.Template,
137+
) (string, error) {
138+
backendPools, err := p.loadBalancerProvider.LoadBalancerBackendPools(ctx)
139+
if err != nil {
140+
return "", fmt.Errorf("getting backend pools: %w", err)
141+
}
142+
143+
nodeResourceGroup := options.FromContext(ctx).NodeResourceGroup
144+
networkPlugin := options.FromContext(ctx).NetworkPlugin
145+
networkPluginMode := options.FromContext(ctx).NetworkPluginMode
146+
147+
isAKSManagedVNET, err := utils.IsAKSManagedVNET(nodeResourceGroup, launchTemplate.SubnetID)
148+
if err != nil {
149+
return "", fmt.Errorf("checking if vnet is managed: %w", err)
150+
}
151+
var nsgID string
152+
if !isAKSManagedVNET {
153+
nsg, err := p.networkSecurityGroupProvider.ManagedNetworkSecurityGroup(ctx)
154+
if err != nil {
155+
return "", fmt.Errorf("getting managed network security group: %w", err)
156+
}
157+
nsgID = lo.FromPtr(nsg.ID)
158+
}
159+
160+
nicReference, err := p.createNetworkInterface(
161+
ctx,
162+
&createNICOptions{
163+
NICName: resourceName,
164+
NetworkPlugin: networkPlugin,
165+
NetworkPluginMode: networkPluginMode,
166+
MaxPods: utils.GetMaxPods(nodeClass, networkPlugin, networkPluginMode),
167+
LaunchTemplate: launchTemplate,
168+
BackendPools: backendPools,
169+
InstanceType: instanceType,
170+
NetworkSecurityGroupID: nsgID,
171+
},
172+
)
173+
if err != nil {
174+
return "", err
175+
}
176+
177+
return nicReference, nil
178+
}

0 commit comments

Comments
 (0)