ScaleSetPriority for AzureManagedMachinePool

azure/converters/managedagentpool.go

@@ -42,5 +42,6 @@ func AgentPoolToManagedClusterAgentPoolProfile(pool containerservice.AgentPool)
 		OsDiskType:          properties.OsDiskType,
 		NodeLabels:          properties.NodeLabels,
 		EnableUltraSSD:      properties.EnableUltraSSD,
+		ScaleSetPriority:    properties.ScaleSetPriority,
 	}
 }

azure/scope/managedmachinepool.go

@@ -175,6 +175,7 @@ func buildAgentPoolSpec(managedControlPlane *infrav1exp.AzureManagedControlPlane
 		OsDiskType:        managedMachinePool.Spec.OsDiskType,
 		EnableUltraSSD:    managedMachinePool.Spec.EnableUltraSSD,
 		Headers:           maps.FilterByKeyPrefix(agentPoolAnnotations, azure.CustomHeaderPrefix),
+		ScaleSetPriority:  managedMachinePool.Spec.ScaleSetPriority,
 	}
 
 	if managedMachinePool.Spec.OSDiskSizeGB != nil {

azure/services/agentpools/agentpools_test.go

@@ -193,3 +193,82 @@ func TestDeleteAgentPools(t *testing.T) {
 		})
 	}
 }
+
+func TestMergeSystemNodeLabels(t *testing.T) {
+	testcases := []struct {
+		name       string
+		capzLabels map[string]*string
+		aksLabels  map[string]*string
+		expected   map[string]*string
+	}{
+		{
+			name: "update an existing label",
+			capzLabels: map[string]*string{
+				"foo": to.StringPtr("bar"),
+			},
+			aksLabels: map[string]*string{
+				"foo": to.StringPtr("baz"),
+			},
+			expected: map[string]*string{
+				"foo": to.StringPtr("bar"),
+			},
+		},
+		{
+			name:       "delete labels",
+			capzLabels: map[string]*string{},
+			aksLabels: map[string]*string{
+				"foo":   to.StringPtr("bar"),
+				"hello": to.StringPtr("world"),
+			},
+			expected: map[string]*string{},
+		},
+		{
+			name: "delete one label",
+			capzLabels: map[string]*string{
+				"foo": to.StringPtr("bar"),
+			},
+			aksLabels: map[string]*string{
+				"foo":   to.StringPtr("bar"),
+				"hello": to.StringPtr("world"),
+			},
+			expected: map[string]*string{
+				"foo": to.StringPtr("bar"),
+			},
+		},
+		{
+			name: "retain system label during update",
+			capzLabels: map[string]*string{
+				"foo": to.StringPtr("bar"),
+			},
+			aksLabels: map[string]*string{
+				"kubernetes.azure.com/scalesetpriority": to.StringPtr("spot"),
+			},
+			expected: map[string]*string{
+				"foo":                                   to.StringPtr("bar"),
+				"kubernetes.azure.com/scalesetpriority": to.StringPtr("spot"),
+			},
+		},
+		{
+			name:       "retain system label during delete",
+			capzLabels: map[string]*string{},
+			aksLabels: map[string]*string{
+				"kubernetes.azure.com/scalesetpriority": to.StringPtr("spot"),
+			},
+			expected: map[string]*string{
+				"kubernetes.azure.com/scalesetpriority": to.StringPtr("spot"),
+			},
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Logf("Testing " + tc.name)
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			g := NewWithT(t)
+			t.Parallel()
+
+			ret := mergeSystemNodeLabels(tc.capzLabels, tc.aksLabels)
+			g.Expect(ret).To(Equal(tc.expected))
+		})
+	}
+}

azure/services/agentpools/spec.go

@@ -26,6 +26,7 @@ import (
 	"github.com/pkg/errors"
 	infrav1 "sigs.k8s.io/cluster-api-provider-azure/api/v1beta1"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
+	azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
 )
 
 // AgentPoolSpec contains agent pool specification details.
@@ -89,6 +90,9 @@ type AgentPoolSpec struct {
 
 	// Headers is the list of headers to add to the HTTP requests to update this resource.
 	Headers map[string]string
+
+	// ScaleSetPriority specifies the ScaleSetPriority for the node pool. Allowed values are 'Spot' and 'Regular'
+	ScaleSetPriority *string `json:"scaleSetPriority,omitempty"`
 }
 
 // ResourceName returns the name of the agent pool.
@@ -113,6 +117,7 @@ func (s *AgentPoolSpec) CustomHeaders() map[string]string {
 
 // Parameters returns the parameters for the agent pool.
 func (s *AgentPoolSpec) Parameters(existing interface{}) (params interface{}, err error) {
+	var nodeLabels map[string]*string
 	if existing != nil {
 		existingPool, ok := existing.(containerservice.AgentPool)
 		if !ok {
@@ -164,6 +169,12 @@ func (s *AgentPoolSpec) Parameters(existing interface{}) (params interface{}, er
 			// agent pool is up to date, nothing to do
 			return nil, nil
 		}
+		// We do a just-in-time merge of existent kubernetes.azure.com-prefixed labels
+		// So that we don't unintentionally delete them
+		// See https://github.com/Azure/AKS/issues/3152
+		if normalizedProfile.NodeLabels != nil {
+			nodeLabels = mergeSystemNodeLabels(normalizedProfile.NodeLabels, existingPool.NodeLabels)
+		}
 	}
 
 	var availabilityZones *[]string
@@ -197,15 +208,29 @@ func (s *AgentPoolSpec) Parameters(existing interface{}) (params interface{}, er
 			MaxPods:             s.MaxPods,
 			MinCount:            s.MinCount,
 			Mode:                containerservice.AgentPoolMode(s.Mode),
-			NodeLabels:          s.NodeLabels,
+			NodeLabels:          nodeLabels,
 			NodeTaints:          nodeTaints,
 			OrchestratorVersion: s.Version,
 			OsDiskSizeGB:        &s.OSDiskSizeGB,
 			OsDiskType:          containerservice.OSDiskType(to.String(s.OsDiskType)),
 			OsType:              containerservice.OSType(to.String(s.OSType)),
+			ScaleSetPriority:    containerservice.ScaleSetPriority(to.String(s.ScaleSetPriority)),
 			Type:                containerservice.AgentPoolTypeVirtualMachineScaleSets,
 			VMSize:              sku,
 			VnetSubnetID:        vnetSubnetID,
 		},
 	}, nil
 }
+
+// mergeSystemNodeLabels appends any kubernetes.azure.com-prefixed labels from the AKS label set
+// into the local capz label set.
+func mergeSystemNodeLabels(capz, aks map[string]*string) map[string]*string {
+	ret := capz
+	// Look for labels returned from the AKS node pool API that begin with kubernetes.azure.com
+	for aksNodeLabelKey := range aks {
+		if azureutil.IsAzureSystemNodeLabelKey(aksNodeLabelKey) {
+			ret[aksNodeLabelKey] = aks[aksNodeLabelKey]
+		}
+	}
+	return ret
+}

config/crd/bases/infrastructure.cluster.x-k8s.io_azuremanagedmachinepools.yaml

@@ -258,6 +258,13 @@ spec:
                 items:
                   type: string
                 type: array
+              scaleSetPriority:
+                description: 'ScaleSetPriority specifies the ScaleSetPriority value.
+                  Default to Regular. Possible values include: ''Regular'', ''Spot'''
+                enum:
+                - Regular
+                - Spot
+                type: string
               scaling:
                 description: Scaling specifies the autoscaling parameters for the
                   node pool.

exp/api/v1alpha3/azuremanagedmachinepool_conversion.go

@@ -45,6 +45,7 @@ func (src *AzureManagedMachinePool) ConvertTo(dstRaw conversion.Hub) error {
 	dst.Spec.OSType = restored.Spec.OSType
 	dst.Spec.NodeLabels = restored.Spec.NodeLabels
 	dst.Spec.EnableUltraSSD = restored.Spec.EnableUltraSSD
+	dst.Spec.ScaleSetPriority = restored.Spec.ScaleSetPriority
 
 	dst.Status.LongRunningOperationStates = restored.Status.LongRunningOperationStates
 	dst.Status.Conditions = restored.Status.Conditions

exp/api/v1beta1/azuremanagedmachinepool_types.go

@@ -98,6 +98,11 @@ type AzureManagedMachinePoolSpec struct {
 	// +kubebuilder:validation:Enum=Linux;Windows
 	// +optional
 	OSType *string `json:"osType,omitempty"`
+
+	// ScaleSetPriority specifies the ScaleSetPriority value. Default to Regular. Possible values include: 'Regular', 'Spot'
+	// +kubebuilder:validation:Enum=Regular;Spot
+	// +optional
+	ScaleSetPriority *string `json:"scaleSetPriority,omitempty"`
 }
 
 // ManagedMachinePoolScaling specifies scaling options.

exp/api/v1beta1/azuremanagedmachinepool_webhook.go

@@ -28,6 +28,7 @@ import (
 	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"sigs.k8s.io/cluster-api-provider-azure/azure"
+	azureutil "sigs.k8s.io/cluster-api-provider-azure/util/azure"
 	"sigs.k8s.io/cluster-api-provider-azure/util/maps"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -59,6 +60,7 @@ func (m *AzureManagedMachinePool) ValidateCreate(client client.Client) error {
 		m.validateMaxPods,
 		m.validateOSType,
 		m.validateName,
+		m.validateNodeLabels,
 	}
 
 	var errs []error
@@ -223,6 +225,48 @@ func (m *AzureManagedMachinePool) ValidateUpdate(oldRaw runtime.Object, client c
 					"field is immutable, unsetting is not allowed"))
 		}
 	}
+
+	if old.Spec.ScaleSetPriority != nil {
+		// Prevent ScaleSetPriority modification if it was already set to some value
+		if m.Spec.ScaleSetPriority == nil {
+			// unsetting the field is not allowed
+			allErrs = append(allErrs,
+				field.Invalid(
+					field.NewPath("Spec", "ScaleSetPriority"),
+					m.Spec.ScaleSetPriority,
+					"field is immutable, unsetting is not allowed"))
+		} else if *m.Spec.ScaleSetPriority != *old.Spec.ScaleSetPriority {
+			// changing the field is not allowed
+			allErrs = append(allErrs,
+				field.Invalid(
+					field.NewPath("Spec", "ScaleSetPriority"),
+					m.Spec.ScaleSetPriority,
+					"field is immutable"))
+		}
+	} else {
+		if m.Spec.ScaleSetPriority != nil {
+			allErrs = append(allErrs,
+				field.Invalid(
+					field.NewPath("Spec", "ScaleSetPriority"),
+					m.Spec.ScaleSetPriority,
+					"field is immutable, unsetting is not allowed"))
+		}
+	}
+
+	updateValidators := []func() error{
+		m.validateNodeLabels,
+	}
+
+	for _, validator := range updateValidators {
+		if err := validator(); err != nil {
+			allErrs = append(allErrs,
+				field.Invalid(
+					field.NewPath("Spec", "NodeLabels"),
+					m.Spec.NodeLabels,
+					fmt.Sprintf("Node pool labels must not start with %s", azureutil.AzureSystemNodeLabelPrefix)))
+		}
+	}
+
 	if len(allErrs) != 0 {
 		return apierrors.NewInvalid(GroupVersion.WithKind("AzureManagedMachinePool").GroupKind(), m.Name, allErrs)
 	}
@@ -319,6 +363,21 @@ func (m *AzureManagedMachinePool) validateName() error {
 	return nil
 }
 
+func (m *AzureManagedMachinePool) validateNodeLabels() error {
+	if m.Spec.NodeLabels != nil {
+		for key := range m.Spec.NodeLabels {
+			if azureutil.IsAzureSystemNodeLabelKey(key) {
+				return field.Invalid(
+					field.NewPath("Spec", "NodeLabels"),
+					m.Spec.NodeLabels[key],
+					fmt.Sprintf("Node pool label must not start with %s", azureutil.AzureSystemNodeLabelPrefix))
+			}
+		}
+	}
+
+	return nil
+}
+
 func ensureStringSlicesAreEqual(a []string, b []string) bool {
 	if len(a) != len(b) {
 		return false

exp/api/v1beta1/azuremanagedmachinepool_webhook_test.go

@@ -463,6 +463,25 @@ func TestAzureManagedMachinePoolUpdatingWebhook(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "Can't add a node label that begins with kubernetes.azure.com",
+			new: &AzureManagedMachinePool{
+				Spec: AzureManagedMachinePoolSpec{
+					NodeLabels: map[string]string{
+						"foo":                                   "bar",
+						"kubernetes.azure.com/scalesetpriority": "spot",
+					},
+				},
+			},
+			old: &AzureManagedMachinePool{
+				Spec: AzureManagedMachinePoolSpec{
+					NodeLabels: map[string]string{
+						"foo": "bar",
+					},
+				},
+			},
+			wantErr: true,
+		},
 	}
 	var client client.Client
 	for _, tc := range tests {
@@ -581,6 +600,33 @@ func TestAzureManagedMachinePool_ValidateCreate(t *testing.T) {
 			wantErr:  true,
 			errorLen: 1,
 		},
+		{
+			name: "valid label",
+			ammp: &AzureManagedMachinePool{
+				Spec: AzureManagedMachinePoolSpec{
+					Mode:   "User",
+					OSType: to.StringPtr(azure.LinuxOS),
+					NodeLabels: map[string]string{
+						"foo": "bar",
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "kubernetes.azure.com label",
+			ammp: &AzureManagedMachinePool{
+				Spec: AzureManagedMachinePoolSpec{
+					Mode:   "User",
+					OSType: to.StringPtr(azure.LinuxOS),
+					NodeLabels: map[string]string{
+						"kubernetes.azure.com/scalesetpriority": "spot",
+					},
+				},
+			},
+			wantErr:  true,
+			errorLen: 1,
+		},
 	}
 	var client client.Client
 	for _, tc := range tests {

templates/flavors/aks/cluster-template.yaml

@@ -104,3 +104,4 @@ spec:
   mode: User
   osDiskSizeGB: 40
   sku: "${AZURE_NODE_MACHINE_TYPE}"
+  scaleSetPriority: Spot