fix: fix Capacity Value Requirements (#1237)

Author: britaniar

pkg/propertychecker/azure/checker.go

@@ -27,6 +27,11 @@ const (
 	// The value is constrained to a reasonable upper bound of 200 for most production workloads.
 	// Upper bound is enforced after adjusting for operator semantics.
 	maxVMInstanceCapacity = 200
+
+	// minVMInstanceCapacity defines the minimum allowed VM instance capacity for SKU capacity requirements.
+	// The Azure vmSizeRecommender API requires capacity values greater than 0, so minimum is set to 1.
+	// Lower bound is enforced after adjusting for operator semantics.
+	minVMInstanceCapacity = 1
 )
 
 // PropertyChecker provides Azure-specific property validation for member clusters.
@@ -48,8 +53,8 @@ func NewPropertyChecker(vmSizeRecommenderClient compute.AttributeBasedVMSizeReco
 
 // CheckIfMeetSKUCapacityRequirement validates whether a member cluster can meet the specified
 // SKU capacity requirement. It extracts the required SKU and capacity from the property selector
-// requirement and checks to determine if the cluster's Azure subscription
-// and location can provision the requested VM instances.
+// requirement and checks whether the cluster's Azure subscription and location can provision
+// the requested VM instances.
 //
 // The cluster must have both Azure location and subscription ID labels configured.
 // Returns true if the SKU capacity requirement can be met, false otherwise.
@@ -75,14 +80,14 @@ func (s *PropertyChecker) CheckIfMeetSKUCapacityRequirement(
 	}
 
 	// Request VM size recommendations to validate SKU availability and capacity.
-	// The capacity is checked by ensuring the current allocatable capacity is greater than the requested capacity.
+	// The capacity is checked by ensuring the current allocatable capacity is greater than or equal to the requested capacity.
 	klog.V(2).Infof("Checking SKU %s with capacity %d in cluster %s", sku, capacity, cluster.Name)
 	request := &computev1.GenerateAttributeBasedRecommendationsRequest{
 		SubscriptionId: subID,
 		Location:       location,
 		RegularPriorityProfile: &computev1.RegularPriorityProfile{
 			CapacityUnitType: computev1.CapacityUnitType_CAPACITY_UNIT_TYPE_VM_INSTANCE_COUNT,
-			TargetCapacity:   capacity, // CurrentAllocatableCapacity > RequestedCapacity
+			TargetCapacity:   capacity, // CurrentAllocatableCapacity >= RequestedCapacity
 		},
 		ResourceProperties: &computev1.ResourceProperties{
 			VmAttributes: &computev1.VMAttributes{
@@ -116,9 +121,8 @@ func (s *PropertyChecker) CheckIfMeetSKUCapacityRequirement(
 // extractCapacityRequirements extracts the capacity value from a PropertySelectorRequirement.
 // This function is specifically designed for Azure SKU capacity properties that follow the pattern:
 // "kubernetes.azure.com/vm-sizes/{sku}/capacity"
-// Returns the capacity if the requirement is valid;
-// The capacity will be updated based on the configured operator as VMSizeRecommender API
-// checks if the current allocatableCapacity > the requested capacity.
+// Returns the capacity value adjusted for the operator semantics, as the VMSizeRecommender API
+// checks whether current allocatable capacity is greater than or equal to the requested capacity.
 func extractCapacityRequirements(req placementv1beta1.PropertySelectorRequirement) (uint32, error) {
 	if req.Operator != placementv1beta1.PropertySelectorGreaterThan && req.Operator != placementv1beta1.PropertySelectorGreaterThanOrEqualTo {
 		return 0, fmt.Errorf("unsupported operator %q for SKU capacity property, only GreaterThan (Gt) and GreaterThanOrEqualTo (Ge) are supported", req.Operator)
@@ -149,18 +153,24 @@ func validateCapacity(value string, operator placementv1beta1.PropertySelectorOp
 
 	capacity := uint32(valueUint) // capacity is >= 0 since it's parsed as uint.
 
-	if operator == placementv1beta1.PropertySelectorGreaterThanOrEqualTo && capacity > 0 {
-		capacity -= 1
+	// Adjust capacity for VMSizeRecommender API semantics.
+	// The API requires requested capacity >= 1.
+	// If operator is GreaterThan (Gt) , increment the requested value by 1.
+	// Otherwise, use the requested value as-is.
+	// These adjustments ensure the API interprets the request correctly
+	if operator == placementv1beta1.PropertySelectorGreaterThan {
+		capacity += 1
 	}
 
 	// Validate against maximum allowed capacity (exceed maxVMInstanceCapacity).
-	if capacity >= maxVMInstanceCapacity {
+	if capacity > maxVMInstanceCapacity {
 		return 0, fmt.Errorf("capacity value %d exceeds maximum allowed value of %d", capacity, maxVMInstanceCapacity)
 	}
 
-	// A capacity of zero is only valid for GreaterThan operator.
-	if capacity == 0 && operator != placementv1beta1.PropertySelectorGreaterThan {
-		return 0, fmt.Errorf("capacity value cannot be zero for operator %q", operator)
+	// Ensure capacity meets minimum requirements (minVMInstanceCapacity) after operator adjustment.
+	// The VMSizeRecommender API requires capacity > 0.
+	if capacity < minVMInstanceCapacity {
+		return 0, fmt.Errorf("capacity value %d is below minimum allowed value of %d", capacity, minVMInstanceCapacity)
 	}
 
 	return capacity, nil

pkg/propertychecker/azure/checker_test.go

@@ -33,14 +33,14 @@ func TestValidateCapacity(t *testing.T) {
 			name:      "valid capacity value for GreaterThan operator",
 			value:     "10",
 			operator:  placementv1beta1.PropertySelectorGreaterThan,
-			wantValue: 10,
+			wantValue: 11,
 			wantError: false,
 		},
 		{
 			name:      "valid capacity value for GreaterThanOrEqualTo operator",
 			value:     "50",
 			operator:  placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
-			wantValue: 49,
+			wantValue: 50,
 			wantError: false,
 		},
 		{
@@ -72,11 +72,11 @@ func TestValidateCapacity(t *testing.T) {
 			errorSubstring: "invalid capacity value",
 		},
 		{
-			name:           "unsupported operator for capacity of zero",
+			name:           "invalid value of zero for capacity",
 			value:          "0",
 			operator:       placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
 			wantError:      true,
-			errorSubstring: "capacity value cannot be zero for operator",
+			errorSubstring: "is below minimum allowed value",
 		},
 		{
 			name:           "capacity equal to max limit with GreaterThan operator",
@@ -85,18 +85,11 @@ func TestValidateCapacity(t *testing.T) {
 			wantError:      true,
 			errorSubstring: "exceeds maximum allowed value",
 		},
-		{
-			name:      "supported operator with capacity of zero",
-			value:     "0",
-			operator:  placementv1beta1.PropertySelectorGreaterThan,
-			wantValue: 0,
-			wantError: false,
-		},
 		{
 			name:      "capacity equal to max limit with supported operator",
 			value:     "200",
 			operator:  placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
-			wantValue: 199,
+			wantValue: 200,
 			wantError: false,
 		},
 		{
@@ -107,12 +100,19 @@ func TestValidateCapacity(t *testing.T) {
 			errorSubstring: "exceeds maximum allowed value",
 		},
 		{
-			name:      "capacity at minimum limit",
-			value:     "1",
+			name:      "capacity at minimum limit (Gt)",
+			value:     "0",
 			operator:  placementv1beta1.PropertySelectorGreaterThan,
 			wantValue: 1,
 			wantError: false,
 		},
+		{
+			name:      "capacity at minimum limit (Ge)",
+			value:     "1",
+			operator:  placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
+			wantValue: 1,
+			wantError: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -158,7 +158,7 @@ func TestExtractCapacityRequirements(t *testing.T) {
 				Values:   []string{"4"},
 			},
 			wantSKU:           "Standard_D4s_v3",
-			wantCapacityValue: 4,
+			wantCapacityValue: 5,
 			wantError:         false,
 		},
 		{
@@ -172,6 +172,17 @@ func TestExtractCapacityRequirements(t *testing.T) {
 			wantError:      true,
 			errorSubstring: "exceeds maximum allowed value of 200",
 		},
+		{
+			name: "invalid Azure SKU capacity property below min limit (Ge)",
+			req: placementv1beta1.PropertySelectorRequirement{
+				Name:     fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_B2ms"),
+				Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
+				Values:   []string{"0"},
+			},
+			wantSKU:        "Standard_B2ms",
+			wantError:      true,
+			errorSubstring: "is below minimum allowed value",
+		},
 		{
 			name: "invalid Azure SKU capacity property with decimal",
 			req: placementv1beta1.PropertySelectorRequirement{
@@ -281,7 +292,7 @@ func TestExtractCapacityRequirements(t *testing.T) {
 }
 
 func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
-	// Prepare test data
+	// Prepare test data.
 	validSKU := "Standard_D2s_v3"
 	validPropertySelectorRequirement := placementv1beta1.PropertySelectorRequirement{
 		Name:     fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, validSKU),
@@ -312,7 +323,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
 			name:           "valid capacity request",
 			cluster:        cluster,
 			sku:            validSKU,
-			targetCapacity: 3,
+			targetCapacity: 4,
 			req:            validPropertySelectorRequirement,
 			mockStatusCode: http.StatusOK,
 			wantAvailable:  true,
@@ -322,7 +333,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
 			name:           "unavailable SKU request",
 			cluster:        cluster,
 			sku:            "Standard_D2s_v4",
-			targetCapacity: 1,
+			targetCapacity: 2,
 			req: placementv1beta1.PropertySelectorRequirement{
 				Name:     fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_D2s_v4"),
 				Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
@@ -381,7 +392,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
 			name:           "Azure API returns error",
 			cluster:        cluster,
 			sku:            validSKU,
-			targetCapacity: 3,
+			targetCapacity: 4,
 			req:            validPropertySelectorRequirement,
 			mockStatusCode: http.StatusInternalServerError,
 			wantError:      true,
@@ -402,7 +413,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
 			errorSubstring: "unsupported operator \"Eq\" for SKU capacity property, only GreaterThan (Gt) and GreaterThanOrEqualTo (Ge) are supported",
 		},
 		{
-			name:           "unsupported operator in requirement",
+			name:           "too low value in requirement",
 			cluster:        cluster,
 			sku:            validSKU,
 			targetCapacity: 0,
@@ -411,15 +422,14 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
 				Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
 				Values:   []string{"0"},
 			},
-			mockStatusCode: http.StatusOK,
 			wantError:      true,
-			errorSubstring: "capacity value cannot be zero for operator",
+			errorSubstring: "is below minimum allowed value",
 		},
 		{
 			name:           "cases-insensitive request - available SKU",
 			cluster:        cluster,
 			sku:            "STANDARD_D2S_V3",
-			targetCapacity: 1,
+			targetCapacity: 2,
 			req: placementv1beta1.PropertySelectorRequirement{
 				Name:     fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "STANDARD_D2S_V3"),
 				Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,

pkg/scheduler/framework/plugins/clusteraffinity/filtering_test.go

@@ -831,7 +831,7 @@ func TestFilter_PropertyChecker(t *testing.T) {
 				},
 			},
 			vmSize:         "Standard_D2s_v3",
-			targetCapacity: 1,
+			targetCapacity: 2,
 		},
 		{
 			name: "single cluster capacity based term, not matched",
@@ -878,7 +878,7 @@ func TestFilter_PropertyChecker(t *testing.T) {
 				},
 			},
 			vmSize:         "Standard_B2ms",
-			targetCapacity: 3,
+			targetCapacity: 4,
 			wantStatus:     framework.NewNonErrorStatus(framework.ClusterUnschedulable, p.Name(), "cluster does not match with any of the required cluster affinity terms"),
 		},
 	}

pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go

@@ -124,7 +124,7 @@ func TestMatchPropertiesInPropertyChecker(t *testing.T) {
 				Values:   []string{"2"},
 			},
 			vmSize:         "Standard_D2s_v3",
-			targetCapacity: 2,
+			targetCapacity: 3,
 			wantHandled:    true,
 			wantAvailable:  true,
 		},
@@ -137,7 +137,7 @@ func TestMatchPropertiesInPropertyChecker(t *testing.T) {
 				Values:   []string{"2"},
 			},
 			vmSize:         "NonExistentSKU",
-			targetCapacity: 1,
+			targetCapacity: 2,
 			wantHandled:    true,
 			wantAvailable:  false,
 		},

pkg/scheduler/framework/plugins/clusteraffinity/types_test.go

@@ -1356,7 +1356,23 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) {
 			},
 			cluster:        cluster,
 			sku:            "Standard_D2s_v3",
-			targetCapacity: 0,
+			targetCapacity: 1,
+			want:           true,
+		},
+		{
+			name: "op >=, matched (min limit)",
+			matchExpression: []placementv1beta1.PropertySelectorRequirement{
+				{
+					Name:     validPropertyName,
+					Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
+					Values: []string{
+						"1",
+					},
+				},
+			},
+			cluster:        cluster,
+			sku:            "Standard_D2s_v3",
+			targetCapacity: 1,
 			want:           true,
 		},
 		{
@@ -1372,7 +1388,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) {
 			},
 			cluster:        cluster,
 			sku:            "Standard_D4s_v3",
-			targetCapacity: 8,
+			targetCapacity: 9,
 		},
 		{
 			name: "op >=, matched (max limit)",
@@ -1387,7 +1403,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) {
 			},
 			cluster:        cluster,
 			sku:            "Standard_D2s_v3",
-			targetCapacity: 199,
+			targetCapacity: 200,
 			want:           true,
 		},
 		{
@@ -1403,7 +1419,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) {
 			},
 			cluster:        cluster,
 			sku:            "Standard_D4s_v3",
-			targetCapacity: 79,
+			targetCapacity: 80,
 		},
 	}