Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/cloudprovider/cloudprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ func (c *CloudProvider) vmInstanceToNodeClaim(ctx context.Context, vm *armcomput
nodeClaim.Status.Allocatable = lo.PickBy(instanceType.Allocatable(), func(_ corev1.ResourceName, v resource.Quantity) bool { return !resources.IsZero(v) })
}

if zone, err := utils.GetZone(vm); err != nil {
if zone, err := utils.MakeAKSLabelZoneFromVM(vm); err != nil {
log.FromContext(ctx).Info("failed to get zone for VM, zone label will be empty", "vmName", *vm.Name, "error", err)
} else {
labels[corev1.LabelTopologyZone] = zone
Expand Down
2 changes: 1 addition & 1 deletion pkg/providers/instance/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func zoneFromVM(vm *armcompute.VirtualMachine) string {
if zonePtr == nil {
return ""
}
return utils.MakeZone(strings.ToLower(lo.FromPtr(vm.Location)), lo.FromPtr(zonePtr))
return utils.MakeAKSLabelZoneFromARMZone(strings.ToLower(lo.FromPtr(vm.Location)), lo.FromPtr(zonePtr))
}

var _ = Describe("VMInstanceProvider", func() {
Expand Down
4 changes: 2 additions & 2 deletions pkg/providers/instance/vminstance.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (p *DefaultVMProvider) BeginCreate(
return nil, err
}
vm := vmPromise.VM
zone, err := utils.GetZone(vm)
zone, err := utils.MakeAKSLabelZoneFromVM(vm)
if err != nil {
log.FromContext(ctx).V(1).Info("failed to get zone for VM", "vmName", *vm.Name, "error", err)
}
Expand Down Expand Up @@ -557,7 +557,7 @@ func newVMObject(opts *createVMOptions) *armcompute.VirtualMachine {
},
Priority: lo.ToPtr(KarpCapacityTypeToVMPriority[opts.CapacityType]),
},
Zones: utils.MakeVMZone(opts.Zone),
Zones: utils.MakeARMZonesFromAKSLabelZone(opts.Zone),
Tags: opts.LaunchTemplate.Tags,
}
setVMPropertiesOSDiskType(vm.Properties, opts.LaunchTemplate)
Expand Down
2 changes: 1 addition & 1 deletion pkg/providers/instancetype/instancetypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (p *DefaultProvider) instanceTypeZones(sku *skewer.SKU) sets.Set[string] {
skuZones := lo.Keys(sku.AvailabilityZones(p.region))
if len(skuZones) > 0 {
return sets.New(lo.Map(skuZones, func(zone string, _ int) string {
return utils.MakeZone(p.region, zone)
return utils.MakeAKSLabelZoneFromARMZone(p.region, zone)
})...)
}
return sets.New("") // empty string means non-zonal offering
Expand Down
11 changes: 6 additions & 5 deletions pkg/providers/instancetype/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ var fakeClock *clock.FakeClock
var coreProvisioner, coreProvisionerNonZonal, coreProvisionerBootstrap *provisioning.Provisioner
var cluster, clusterNonZonal, clusterBootstrap *state.Cluster
var cloudProvider, cloudProviderNonZonal, cloudProviderBootstrap *cloudprovider.CloudProvider
var fakeZone1 = utils.MakeZone(fake.Region, "1")

var fakeZone1 = utils.MakeAKSLabelZoneFromARMZone(fake.Region, "1")

var defaultTestSKU = &skewer.SKU{Name: lo.ToPtr("Standard_D2_v3"), Family: lo.ToPtr("standardD2v3Family")}

Expand Down Expand Up @@ -393,7 +394,7 @@ var _ = Describe("InstanceType Provider", func() {
ExpectNotScheduled(ctx, env.Client, pod)

// ensure that initial zone was made unavailable
zone, err := utils.GetZone(&azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VM)
zone, err := utils.MakeAKSLabelZoneFromVM(&azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VM)
Expect(err).ToNot(HaveOccurred())
ExpectUnavailable(azureEnv, defaultTestSKU, zone, karpv1.CapacityTypeSpot)

Expand Down Expand Up @@ -463,7 +464,7 @@ var _ = Describe("InstanceType Provider", func() {
// ensure that initial VM size was made unavailable
vm := azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VM
initialVMSize := *vm.Properties.HardwareProfile.VMSize
zone, err := utils.GetZone(&vm)
zone, err := utils.MakeAKSLabelZoneFromVM(&vm)
Expect(err).ToNot(HaveOccurred())
ExpectUnavailable(azureEnv, &skewer.SKU{Name: lo.ToPtr(string(initialVMSize))}, zone, karpv1.CapacityTypeSpot)

Expand Down Expand Up @@ -1214,7 +1215,7 @@ var _ = Describe("InstanceType Provider", func() {
Eventually(func() []*karpv1.NodeClaim { return ExpectNodeClaims(ctx, env.Client) }).To(HaveLen(0))

By("marking whatever zone was picked as unavailable - for both spot and on-demand")
zone, err := utils.GetZone(&azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VM)
zone, err := utils.MakeAKSLabelZoneFromVM(&azureEnv.VirtualMachinesAPI.VirtualMachineCreateOrUpdateBehavior.CalledWithInput.Pop().VM)
Expect(err).ToNot(HaveOccurred())
for _, skuToCheck := range expectedUnavailableSKUs {
Expect(azureEnv.UnavailableOfferingsCache.IsUnavailable(skuToCheck, zone, karpv1.CapacityTypeSpot)).To(BeTrue())
Expand Down Expand Up @@ -1354,7 +1355,7 @@ var _ = Describe("InstanceType Provider", func() {
ExpectProvisioned(ctx, env.Client, cluster, cloudProvider, coreProvisioner, pod)
ExpectNotScheduled(ctx, env.Client, pod)
for _, zoneID := range []string{"1", "2", "3"} {
ExpectUnavailable(azureEnv, sku, utils.MakeZone(fake.Region, zoneID), capacityType)
ExpectUnavailable(azureEnv, sku, utils.MakeAKSLabelZoneFromARMZone(fake.Region, zoneID), capacityType)
}
}

Expand Down
14 changes: 7 additions & 7 deletions pkg/utils/zone.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,25 +23,25 @@ import (
"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute/v7"
)

// MakeZone returns the zone value in format of <region>-<zone-id>.
func MakeZone(location string, zoneID string) string {
// MakeAKSLabelZoneFromARMZone returns the zone value in format of <region>-<zone-id>.
func MakeAKSLabelZoneFromARMZone(location string, zoneID string) string {
if zoneID == "" {
return ""
}
return fmt.Sprintf("%s-%s", strings.ToLower(location), zoneID)
}

// VM Zones field expects just the zone number, without region
func MakeVMZone(zone string) []*string {
// MakeARMZonesFromAKSLabelZone returns the zone ID from <region>-<zone-id>.
func MakeARMZonesFromAKSLabelZone(zone string) []*string {
if zone == "" {
return []*string{}
}
zoneNum := zone[len(zone)-1:]
return []*string{&zoneNum}
}

// GetZone returns the zone for the given virtual machine, or an empty string if there is no zone specified
func GetZone(vm *armcompute.VirtualMachine) (string, error) {
// MakeAKSLabelZoneFromVM returns the zone for the given virtual machine, or an empty string if there is no zone specified
func MakeAKSLabelZoneFromVM(vm *armcompute.VirtualMachine) (string, error) {
if vm == nil {
return "", fmt.Errorf("cannot pass in a nil virtual machine")
}
Expand All @@ -52,7 +52,7 @@ func GetZone(vm *armcompute.VirtualMachine) (string, error) {
if vm.Location == nil {
return "", fmt.Errorf("virtual machine is missing location")
}
return MakeZone(*vm.Location, *(vm.Zones)[0]), nil
return MakeAKSLabelZoneFromARMZone(*vm.Location, *(vm.Zones)[0]), nil
}
if len(vm.Zones) > 1 {
return "", fmt.Errorf("virtual machine has multiple zones")
Expand Down
4 changes: 2 additions & 2 deletions pkg/utils/zone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"github.com/stretchr/testify/assert"
)

func TestGetZone(t *testing.T) {
func TestMakeAKSLabelZoneFromVM(t *testing.T) {
tc := []struct {
testName string
input *armcompute.VirtualMachine
Expand Down Expand Up @@ -74,7 +74,7 @@ func TestGetZone(t *testing.T) {
}

for _, c := range tc {
zone, err := utils.GetZone(c.input)
zone, err := utils.MakeAKSLabelZoneFromVM(c.input)
assert.Equal(t, c.expectedZone, zone, c.testName)
if err == nil && c.expectedError != "" {
assert.Fail(t, "expected error but got nil", c.testName)
Expand Down
Loading