Skip to content

Commit 937d6d9

Browse files
comtalystclaude
andcommitted
refactor: extract vminstanceutils.go from vminstance.go and cloudprovider.go
Move VM utility functions into a dedicated vminstanceutils.go file: - Priority maps (KarpCapacityTypeToVMPriority, VMPriorityToKarpCapacityType, VMPriorityToScaleSetPriority) - Extension-related constants and helpers (aksIdentifyingExtensionName, GetManagedExtensionNames, isAKSIdentifyingExtensionEnabled) - ErrorCodeForMetrics, ConvertToVirtualMachineIdentity, GetCapacityTypeFromVM, GetScaleSetPriorityLabelFromVM Move vmInstanceToNodeClaim from CloudProvider to BuildNodeClaimFromVM in instance package, paralleling BuildNodeClaimFromAKSMachine. Move GetNodeClaimNameFromVMName to instance package. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 9288de1 commit 937d6d9

File tree

5 files changed

+210
-166
lines changed

5 files changed

+210
-166
lines changed

pkg/cloudprovider/cloudprovider.go

Lines changed: 5 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,12 @@ import (
2121
stderrors "errors"
2222
"fmt"
2323
"net/http"
24-
"strings"
2524
"sync"
2625
"time"
2726

2827
"github.com/awslabs/operatorpkg/status"
2928
corev1 "k8s.io/api/core/v1"
3029
"k8s.io/apimachinery/pkg/api/errors"
31-
"k8s.io/apimachinery/pkg/api/resource"
32-
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3330
"k8s.io/apimachinery/pkg/runtime/schema"
3431
"sigs.k8s.io/karpenter/pkg/controllers/nodeoverlay"
3532

@@ -195,7 +192,7 @@ func (c *CloudProvider) createVMInstance(ctx context.Context, nodeClass *v1beta1
195192
instanceType, _ := lo.Find(instanceTypes, func(i *cloudprovider.InstanceType) bool {
196193
return i.Name == string(lo.FromPtr(vm.Properties.HardwareProfile.VMSize))
197194
})
198-
newNodeClaim, err := c.vmInstanceToNodeClaim(ctx, vm, instanceType)
195+
newNodeClaim, err := instance.BuildNodeClaimFromVM(ctx, vm, instanceType)
199196
if err != nil {
200197
return nil, err
201198
}
@@ -394,12 +391,12 @@ func (c *CloudProvider) List(ctx context.Context) ([]*karpv1.NodeClaim, error) {
394391
return nil, fmt.Errorf("listing VM instances, %w", err)
395392
}
396393

397-
for _, instance := range vmInstances {
398-
instanceType, err := c.resolveInstanceTypeFromVMInstance(ctx, instance)
394+
for _, vmInst := range vmInstances {
395+
instanceType, err := c.resolveInstanceTypeFromVMInstance(ctx, vmInst)
399396
if err != nil {
400397
return nil, fmt.Errorf("resolving instance type for VM instance, %w", err)
401398
}
402-
nodeClaim, err := c.vmInstanceToNodeClaim(ctx, instance, instanceType)
399+
nodeClaim, err := instance.BuildNodeClaimFromVM(ctx, vmInst, instanceType)
403400
if err != nil {
404401
return nil, fmt.Errorf("converting VM instance to node claim, %w", err)
405402
}
@@ -434,7 +431,7 @@ func (c *CloudProvider) Get(ctx context.Context, providerID string) (*karpv1.Nod
434431
if err != nil {
435432
return nil, fmt.Errorf("resolving instance type, %w", err)
436433
}
437-
return c.vmInstanceToNodeClaim(ctx, vm, instanceType)
434+
return instance.BuildNodeClaimFromVM(ctx, vm, instanceType)
438435
}
439436
}
440437

@@ -617,60 +614,6 @@ func (c *CloudProvider) resolveNodePoolFromVMInstance(ctx context.Context, vm *a
617614
return nil, errors.NewNotFound(schema.GroupResource{Group: coreapis.Group, Resource: "nodepools"}, "")
618615
}
619616

620-
func (c *CloudProvider) vmInstanceToNodeClaim(ctx context.Context, vm *armcompute.VirtualMachine, instanceType *cloudprovider.InstanceType) (*karpv1.NodeClaim, error) {
621-
nodeClaim := &karpv1.NodeClaim{}
622-
labels := map[string]string{}
623-
annotations := map[string]string{}
624-
625-
if instanceType != nil {
626-
labels = labelspkg.GetAllSingleValuedRequirementLabels(instanceType.Requirements)
627-
nodeClaim.Status.Capacity = lo.PickBy(instanceType.Capacity, func(_ corev1.ResourceName, v resource.Quantity) bool { return !resources.IsZero(v) })
628-
nodeClaim.Status.Allocatable = lo.PickBy(instanceType.Allocatable(), func(_ corev1.ResourceName, v resource.Quantity) bool { return !resources.IsZero(v) })
629-
}
630-
631-
if zone, err := utils.MakeAKSLabelZoneFromVM(vm); err != nil {
632-
log.FromContext(ctx).Info("failed to get zone for VM, zone label will be empty", "vmName", *vm.Name, "error", err)
633-
} else {
634-
labels[corev1.LabelTopologyZone] = zone
635-
}
636-
637-
labels[karpv1.CapacityTypeLabelKey] = instance.GetCapacityTypeFromVM(vm)
638-
labels[v1beta1.AKSLabelScaleSetPriority] = instance.GetScaleSetPriorityLabelFromVM(vm)
639-
640-
if tag, ok := vm.Tags[launchtemplate.NodePoolTagKey]; ok {
641-
labels[karpv1.NodePoolLabelKey] = *tag
642-
}
643-
644-
nodeClaim.Name = GetNodeClaimNameFromVMName(*vm.Name)
645-
nodeClaim.Labels = labels
646-
nodeClaim.Annotations = annotations
647-
if vm.Properties != nil && vm.Properties.TimeCreated != nil {
648-
nodeClaim.CreationTimestamp = metav1.Time{Time: *vm.Properties.TimeCreated}
649-
} else {
650-
// Fallback to current time to ensure garbage collection grace period is enforced
651-
// when TimeCreated is unavailable. Without this, CreationTimestamp would be epoch (zero value)
652-
// and the instance could be immediately garbage collected, bypassing the 5-minute grace period.
653-
// TODO: Investigate a more fail-safe approach. If vm.Properties.TimeCreated is NEVER populated,
654-
// this fallback means the VM will never be garbage collected since we call this helper every time
655-
// we create an in-memory NodeClaim. We currently assume this shouldn't happen because VMs that fail
656-
// to come up should eventually stop appearing in Azure API responses.
657-
nodeClaim.CreationTimestamp = metav1.Time{Time: time.Now()}
658-
}
659-
// Set the deletionTimestamp to be the current time if the instance is currently terminating
660-
if utils.IsVMDeleting(*vm) {
661-
nodeClaim.DeletionTimestamp = &metav1.Time{Time: time.Now()}
662-
}
663-
nodeClaim.Status.ProviderID = utils.VMResourceIDToProviderID(ctx, *vm.ID)
664-
if vm.Properties != nil && vm.Properties.StorageProfile != nil && vm.Properties.StorageProfile.ImageReference != nil {
665-
nodeClaim.Status.ImageID = utils.ImageReferenceToString(vm.Properties.StorageProfile.ImageReference)
666-
}
667-
return nodeClaim, nil
668-
}
669-
670-
func GetNodeClaimNameFromVMName(vmName string) string {
671-
return strings.TrimPrefix(vmName, "aks-")
672-
}
673-
674617
const truncateAt = 1200
675618

676619
func isNodeClaimStandalone(nodeClaim *karpv1.NodeClaim) bool {

pkg/cloudprovider/cloudprovider_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ import (
2525
. "github.com/onsi/gomega"
2626
"github.com/samber/lo"
2727
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
28+
29+
"github.com/Azure/karpenter-provider-azure/pkg/providers/instance"
2830
)
2931

3032
func TestGenerateNodeClaimName(t *testing.T) {
@@ -54,13 +56,13 @@ func TestGenerateNodeClaimName(t *testing.T) {
5456
t.Run(tt.name, func(t *testing.T) {
5557
g := NewWithT(t)
5658

57-
result := GetNodeClaimNameFromVMName(tt.vmName)
59+
result := instance.GetNodeClaimNameFromVMName(tt.vmName)
5860
g.Expect(result).To(Equal(tt.expected))
5961
})
6062
}
6163
}
6264

63-
func TestVmInstanceToNodeClaim_NilProperties(t *testing.T) {
65+
func TestBuildNodeClaimFromVM_NilProperties(t *testing.T) {
6466
tests := []struct {
6567
name string
6668
vm *armcompute.VirtualMachine
@@ -102,9 +104,8 @@ func TestVmInstanceToNodeClaim_NilProperties(t *testing.T) {
102104
g := NewWithT(t)
103105
ctx := context.Background()
104106

105-
cp := &CloudProvider{}
106107
before := time.Now()
107-
nodeClaim, err := cp.vmInstanceToNodeClaim(ctx, tt.vm, nil)
108+
nodeClaim, err := instance.BuildNodeClaimFromVM(ctx, tt.vm, nil)
108109
after := time.Now()
109110

110111
g.Expect(err).ToNot(HaveOccurred())

pkg/cloudprovider/suite_drift_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"github.com/Azure/karpenter-provider-azure/pkg/apis/v1beta1"
3737
"github.com/Azure/karpenter-provider-azure/pkg/fake"
3838
"github.com/Azure/karpenter-provider-azure/pkg/operator/options"
39+
"github.com/Azure/karpenter-provider-azure/pkg/providers/instance"
3940
"github.com/Azure/karpenter-provider-azure/pkg/test"
4041
. "github.com/Azure/karpenter-provider-azure/pkg/test/expectations"
4142
)
@@ -352,7 +353,7 @@ var _ = Describe("CloudProvider", func() {
352353
Expect(azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Len()).To(Equal(1))
353354
input := azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop()
354355
// Corresponding NodeClaim - fetch from API server so it has all status fields (including ImageID)
355-
nodeClaimName := GetNodeClaimNameFromVMName(input.VMName)
356+
nodeClaimName := instance.GetNodeClaimNameFromVMName(input.VMName)
356357
driftNodeClaim = &karpv1.NodeClaim{}
357358
Expect(env.Client.Get(ctx, types.NamespacedName{Name: nodeClaimName}, driftNodeClaim)).To(Succeed())
358359
// ExpectProvisioned doesn't set Status.NodeName

pkg/providers/instance/vminstance.go

Lines changed: 0 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import (
3131
"github.com/samber/lo"
3232
v1 "k8s.io/api/core/v1"
3333
"k8s.io/apimachinery/pkg/util/sets"
34-
azureclouds "sigs.k8s.io/cloud-provider-azure/pkg/azclient"
3534
"sigs.k8s.io/controller-runtime/pkg/log"
3635
karpv1 "sigs.k8s.io/karpenter/pkg/apis/v1"
3736
corecloudprovider "sigs.k8s.io/karpenter/pkg/cloudprovider"
@@ -54,71 +53,6 @@ import (
5453
"github.com/Azure/karpenter-provider-azure/pkg/utils"
5554
)
5655

57-
var (
58-
KarpCapacityTypeToVMPriority = map[string]armcompute.VirtualMachinePriorityTypes{
59-
karpv1.CapacityTypeSpot: armcompute.VirtualMachinePriorityTypesSpot,
60-
karpv1.CapacityTypeOnDemand: armcompute.VirtualMachinePriorityTypesRegular,
61-
}
62-
VMPriorityToKarpCapacityType = map[armcompute.VirtualMachinePriorityTypes]string{
63-
armcompute.VirtualMachinePriorityTypesSpot: karpv1.CapacityTypeSpot,
64-
armcompute.VirtualMachinePriorityTypesRegular: karpv1.CapacityTypeOnDemand,
65-
}
66-
// Note that there is no ScaleSetPriorityToKarpCapacityType because the karpenter.sh/capacity-type
67-
// label is the "official" label that we actually key priority off of. Selection still works though
68-
// because when we list instance types on-demand offerings always have v1beta1.ScaleSetPriorityRegular
69-
// and spot instances always have v1beta1.ScaleSetPrioritySpot, so the correct karpenter.sh/capacity-type
70-
// label is still selected even if the user is using kubernetes.azure.com/scalesetpriority only on the NodePool.
71-
VMPriorityToScaleSetPriority = map[armcompute.VirtualMachinePriorityTypes]string{
72-
armcompute.VirtualMachinePriorityTypesSpot: v1beta1.ScaleSetPrioritySpot,
73-
armcompute.VirtualMachinePriorityTypesRegular: v1beta1.ScaleSetPriorityRegular,
74-
}
75-
76-
aksIdentifyingExtensionEnvs = sets.New(
77-
azureclouds.PublicCloud.Name,
78-
azureclouds.ChinaCloud.Name,
79-
azureclouds.USGovernmentCloud.Name,
80-
)
81-
)
82-
83-
const (
84-
aksIdentifyingExtensionName = "computeAksLinuxBilling"
85-
// TODO: Why bother with a different CSE name for Windows?
86-
cseNameWindows = "windows-cse-agent-karpenter"
87-
cseNameLinux = "cse-agent-karpenter"
88-
)
89-
90-
// ErrorCodeForMetrics extracts a stable Azure error code for metric labeling when possible.
91-
func ErrorCodeForMetrics(err error) string {
92-
if err == nil {
93-
return "UnknownError"
94-
}
95-
if azErr := sdkerrors.IsResponseError(err); azErr != nil {
96-
if azErr.ErrorCode != "" {
97-
return azErr.ErrorCode
98-
}
99-
return "UnknownError"
100-
}
101-
return "UnknownError"
102-
}
103-
104-
// GetManagedExtensionNames gets the names of the VM extensions managed by Karpenter.
105-
// This is a set of 1 or 2 extensions (depending on provisionMode): aksIdentifyingExtension and (sometimes) cse.
106-
func GetManagedExtensionNames(provisionMode string, env *auth.Environment) []string {
107-
var result []string
108-
// Only including AKS identifying extension in the clouds it is supported in
109-
if isAKSIdentifyingExtensionEnabled(env) {
110-
result = append(result, aksIdentifyingExtensionName)
111-
}
112-
if provisionMode == consts.ProvisionModeBootstrappingClient {
113-
result = append(result, cseNameLinux) // TODO: Windows
114-
}
115-
return result
116-
}
117-
118-
func isAKSIdentifyingExtensionEnabled(env *auth.Environment) bool {
119-
return aksIdentifyingExtensionEnvs.Has(env.Environment.Name)
120-
}
121-
12256
type Resource = map[string]interface{}
12357

12458
type VirtualMachinePromise struct {
@@ -1053,36 +987,3 @@ func (p *DefaultVMProvider) getCSExtension(cse string, isWindows bool, tags map[
1053987
Tags: tags,
1054988
}
1055989
}
1056-
1057-
func ConvertToVirtualMachineIdentity(nodeIdentities []string) *armcompute.VirtualMachineIdentity {
1058-
var identity *armcompute.VirtualMachineIdentity
1059-
if len(nodeIdentities) > 0 {
1060-
identityMap := make(map[string]*armcompute.UserAssignedIdentitiesValue)
1061-
for _, identityID := range nodeIdentities {
1062-
identityMap[identityID] = &armcompute.UserAssignedIdentitiesValue{}
1063-
}
1064-
1065-
if len(identityMap) > 0 {
1066-
identity = &armcompute.VirtualMachineIdentity{
1067-
Type: lo.ToPtr(armcompute.ResourceIdentityTypeUserAssigned),
1068-
UserAssignedIdentities: identityMap,
1069-
}
1070-
}
1071-
}
1072-
1073-
return identity
1074-
}
1075-
1076-
func GetCapacityTypeFromVM(vm *armcompute.VirtualMachine) string {
1077-
if vm != nil && vm.Properties != nil && vm.Properties.Priority != nil {
1078-
return VMPriorityToKarpCapacityType[*vm.Properties.Priority]
1079-
}
1080-
return ""
1081-
}
1082-
1083-
func GetScaleSetPriorityLabelFromVM(vm *armcompute.VirtualMachine) string {
1084-
if vm != nil && vm.Properties != nil && vm.Properties.Priority != nil {
1085-
return VMPriorityToScaleSetPriority[*vm.Properties.Priority]
1086-
}
1087-
return ""
1088-
}

0 commit comments

Comments
 (0)