Merge branch 'main' into tallaxes/test-suites/termination

karpenter-values-template.yaml

@@ -2,8 +2,8 @@
 replicas: 1 # for better debugging experience
 controller:
   env:
-    - name: LEADER_ELECT # disable leader election for better debugging / troubleshooting experience
-      value: "false"
+    - name: DISABLE_LEADER_ELECTION # disable leader election for better debugging / troubleshooting experience
+      value: "true"
     # disable HTTP/2 to reduce ARM throttling on large-scale tests;
     # with this in place write (and read) QPS can be increased too
     #- name: GODEBUG
@@ -37,12 +37,12 @@ controller:
       value: ""
     - name: AZURE_NODE_RESOURCE_GROUP
       value: ${AZURE_RESOURCE_GROUP_MC}
-    
-    # managed karpenter settings 
+
+    # managed karpenter settings
     - name: USE_SIG
       value: "false"
     - name: SIG_SUBSCRIPTION_ID
-      value: "" 
+      value: ""
 serviceAccount:
   name: ${KARPENTER_SERVICE_ACCOUNT_NAME}
   annotations:

pkg/cloudprovider/cloudprovider.go

@@ -328,11 +328,9 @@ func (c *CloudProvider) instanceToNodeClaim(ctx context.Context, vm *armcompute.
 		nodeClaim.Status.Allocatable = lo.PickBy(instanceType.Allocatable(), func(_ v1.ResourceName, v resource.Quantity) bool { return !resources.IsZero(v) })
 	}
 
-	// TODO: review logic for determining zone (AWS uses Zone from subnet resolved and aviailable from NodeClass conditions ...)
-	if zoneID, err := instance.GetZoneID(vm); err != nil {
+	if zone, err := utils.GetZone(vm); err != nil {
 		logging.FromContext(ctx).Warnf("Failed to get zone for VM %s, %v", *vm.Name, err)
 	} else {
-		zone := makeZone(*vm.Location, zoneID)
 		// aks-node-validating-webhook protects v1.LabelTopologyZone, will be set elsewhere, so we use a different label
 		labels[v1alpha2.AlternativeLabelTopologyZone] = zone
 	}
@@ -369,14 +367,6 @@ func GenerateNodeClaimName(vmName string) string {
 	return strings.TrimLeft("aks-", vmName)
 }
 
-// makeZone returns the zone value in format of <region>-<zone-id>.
-func makeZone(location string, zoneID string) string {
-	if zoneID == "" {
-		return ""
-	}
-	return fmt.Sprintf("%s-%s", strings.ToLower(location), zoneID)
-}
-
 // newTerminatingNodeClassError returns a NotFound error for handling by
 func newTerminatingNodeClassError(name string) *errors.StatusError {
 	qualifiedResource := schema.GroupResource{Group: apis.Group, Resource: "aksnodeclasses"}

pkg/fake/types.go

@@ -45,13 +45,12 @@ func (m *MockedFunction[I, O]) Reset() {
 }
 
 func (m *MockedFunction[I, O]) Invoke(input *I, defaultTransformer func(*I) (O, error)) (O, error) {
+	m.CalledWithInput.Add(input)
 	err := m.Error.Get()
 	if err != nil {
 		m.failedCalls.Add(1)
 		return *new(O), err
 	}
-	m.CalledWithInput.Add(input)
-
 	if !m.Output.IsNil() {
 		m.successfulCalls.Add(1)
 		return *m.Output.Clone(), nil
@@ -94,6 +93,8 @@ func (m *MockedLRO[I, O]) Reset() {
 }
 
 func (m *MockedLRO[I, O]) Invoke(input *I, defaultTransformer func(*I) (*O, error)) (*runtime.Poller[O], error) {
+	m.CalledWithInput.Add(input)
+
 	if err := m.BeginError.Get(); err != nil {
 		m.failedCalls.Add(1)
 		return nil, err
@@ -103,8 +104,6 @@ func (m *MockedLRO[I, O]) Invoke(input *I, defaultTransformer func(*I) (*O, erro
 		return newMockPoller[O](nil, err)
 	}
 
-	m.CalledWithInput.Add(input)
-
 	if !m.Output.IsNil() {
 		m.successfulCalls.Add(1)
 		return newMockPoller(m.Output.Clone(), nil)

pkg/providers/instance/armutils.go

@@ -118,7 +118,8 @@ func deleteNicIfExists(ctx context.Context, client NetworkInterfacesAPI, rg, nic
 func deleteVirtualMachineIfExists(ctx context.Context, client VirtualMachinesAPI, rg, vmName string) error {
 	_, err := client.Get(ctx, rg, vmName, nil)
 	if err != nil {
-		if sdkerrors.IsNotFoundErr(err) {
+		azErr := sdkerrors.IsResponseError(err)
+		if azErr != nil && (azErr.ErrorCode == "NotFound" || azErr.ErrorCode == "ResourceNotFound") {
 			return nil
 		}
 		return err

pkg/providers/instance/instance.go

@@ -37,6 +37,7 @@ import (
 	"github.com/Azure/karpenter-provider-azure/pkg/providers/instancetype"
 	"github.com/Azure/karpenter-provider-azure/pkg/providers/launchtemplate"
 	"github.com/Azure/karpenter-provider-azure/pkg/providers/loadbalancer"
+	"github.com/Azure/karpenter-provider-azure/pkg/utils"
 
 	corecloudprovider "sigs.k8s.io/karpenter/pkg/cloudprovider"
 	"sigs.k8s.io/karpenter/pkg/scheduling"
@@ -140,7 +141,7 @@ func (p *DefaultProvider) Create(ctx context.Context, nodeClass *v1alpha2.AKSNod
 		}
 		return nil, err
 	}
-	zone, err := GetZoneID(vm)
+	zone, err := utils.GetZone(vm)
 	if err != nil {
 		logging.FromContext(ctx).Error(err)
 	}
@@ -163,7 +164,8 @@ func (p *DefaultProvider) Get(ctx context.Context, vmName string) (*armcompute.V
 	var err error
 
 	if vm, err = p.azClient.virtualMachinesClient.Get(ctx, p.resourceGroup, vmName, nil); err != nil {
-		if sdkerrors.IsNotFoundErr(err) {
+		azErr := sdkerrors.IsResponseError(err)
+		if azErr != nil && (azErr.ErrorCode == "NotFound" || azErr.ErrorCode == "ResourceNotFound") {
 			return nil, corecloudprovider.NewNodeClaimNotFoundError(err)
 		}
 		return nil, fmt.Errorf("failed to get VM instance, %w", err)
@@ -374,7 +376,7 @@ func newVMObject(
 				CapacityTypeToPriority[capacityType]),
 			),
 		},
-		Zones: lo.Ternary(len(zone) > 0, []*string{&zone}, []*string{}),
+		Zones: utils.MakeVMZone(zone),
 		Tags:  launchTemplate.Tags,
 	}
 	setVMPropertiesOSDiskType(vm.Properties, launchTemplate.StorageProfile)
@@ -627,11 +629,6 @@ func (p *DefaultProvider) pickSkuSizePriorityAndZone(ctx context.Context, nodeCl
 	})
 	zonesWithPriority := lo.Map(priorityOfferings, func(o corecloudprovider.Offering, _ int) string { return getOfferingZone(o) })
 	if zone, ok := sets.New(zonesWithPriority...).PopAny(); ok {
-		if len(zone) > 0 {
-			// Zones in zonal Offerings have <region>-<number> format; the zone returned from here will be used for VM instantiation,
-			// which expects just the zone number, without region
-			zone = string(zone[len(zone)-1])
-		}
 		return instanceType, priority, zone
 	}
 	return nil, "", ""
@@ -751,26 +748,6 @@ func (p *DefaultProvider) getCSExtension(cse string, isWindows bool) *armcompute
 	}
 }
 
-// GetZoneID returns the zone ID for the given virtual machine, or an empty string if there is no zone specified
-func GetZoneID(vm *armcompute.VirtualMachine) (string, error) {
-	if vm == nil {
-		return "", fmt.Errorf("cannot pass in a nil virtual machine")
-	}
-	if vm.Name == nil {
-		return "", fmt.Errorf("virtual machine is missing name")
-	}
-	if vm.Zones == nil {
-		return "", nil
-	}
-	if len(vm.Zones) == 1 {
-		return *(vm.Zones)[0], nil
-	}
-	if len(vm.Zones) > 1 {
-		return "", fmt.Errorf("virtual machine %v has multiple zones", *vm.Name)
-	}
-	return "", nil
-}
-
 func GetListQueryBuilder(rg string) *kql.Builder {
 	return kql.New(`Resources`).
 		AddLiteral(` | where type == "microsoft.compute/virtualmachines"`).

pkg/providers/instance/instance_test.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"testing"
 
-	"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
 	"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/compute/armcompute"
 	"github.com/Azure/karpenter-provider-azure/pkg/cache"
 	"github.com/stretchr/testify/assert"
@@ -79,7 +78,7 @@ func TestGetPriorityCapacityAndInstanceType(t *testing.T) {
 			},
 			nodeClaim:            &karpv1.NodeClaim{},
 			expectedInstanceType: "Standard_D2s_v3",
-			expectedZone:         "2",
+			expectedZone:         "westus-2",
 			expectedPriority:     karpv1.CapacityTypeOnDemand,
 		},
 	}
@@ -101,60 +100,6 @@ func TestGetPriorityCapacityAndInstanceType(t *testing.T) {
 	}
 }
 
-func TestGetZone(t *testing.T) {
-	testVMName := "silly-armcompute"
-	tc := []struct {
-		testName      string
-		input         *armcompute.VirtualMachine
-		expectedZone  string
-		expectedError string
-	}{
-		{
-			testName: "missing name",
-			input: &armcompute.VirtualMachine{
-				Name: nil,
-			},
-			expectedError: "virtual machine is missing name",
-		},
-		{
-			testName:      "invalid virtual machine struct",
-			input:         nil,
-			expectedError: "cannot pass in a nil virtual machine",
-		},
-		{
-			testName: "invalid zones field in virtual machine struct",
-			input: &armcompute.VirtualMachine{
-				Name: &testVMName,
-			},
-			expectedError: "virtual machine silly-armcompute zones are nil",
-		},
-		{
-			testName: "happy case",
-			input: &armcompute.VirtualMachine{
-				Name:  &testVMName,
-				Zones: []*string{to.Ptr("poland-central")},
-			},
-			expectedZone: "poland-central",
-		},
-		{
-			testName: "emptyZones",
-			input: &armcompute.VirtualMachine{
-				Name:  &testVMName,
-				Zones: []*string{},
-			},
-			expectedError: "virtual machine silly-armcompute does not have any zones specified",
-		},
-	}
-
-	for _, c := range tc {
-		zone, err := GetZoneID(c.input)
-		assert.Equal(t, c.expectedZone, zone, c.testName)
-		if err != nil {
-			assert.Equal(t, c.expectedError, err.Error(), c.testName)
-		}
-	}
-}
-
 // Currently tested: ID, Name, Tags, Zones
 // TODO: Add the below attributes for Properties if needed:
 // Priority, InstanceView.HyperVGeneration, TimeCreated

pkg/providers/instancetype/instancetypes.go

@@ -172,7 +172,7 @@ func instanceTypeZones(sku *skewer.SKU, region string) sets.Set[string] {
 	skuZones := lo.Keys(sku.AvailabilityZones(region))
 	if hasZonalSupport(region) && len(skuZones) > 0 {
 		return sets.New(lo.Map(skuZones, func(zone string, _ int) string {
-			return fmt.Sprintf("%s-%s", region, zone)
+			return utils.MakeZone(region, zone)
 		})...)
 	}
 	return sets.New("") // empty string means non-zonal offering