address comments

Signed-off-by: Britania Rodriguez Reyes <[email protected]>

Author: britaniar

pkg/propertychecker/azure/checker.go

@@ -29,7 +29,7 @@ const (
 	maxVMInstanceCapacity = 200
 
 	// minVMInstanceCapacity defines the minimum allowed VM instance capacity for SKU capacity requirements.
-	// The Azure Capacity API requires capacity values greater than 0, so minimum is set to 1.
+	// 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
 )
@@ -80,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 equals 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{
@@ -122,7 +122,7 @@ func (s *PropertyChecker) CheckIfMeetSKUCapacityRequirement(
 // This function is specifically designed for Azure SKU capacity properties that follow the pattern:
 // "kubernetes.azure.com/vm-sizes/{sku}/capacity"
 // Returns the capacity value adjusted for the operator semantics, as the VMSizeRecommender API
-// checks whether current allocatable capacity is greater than the requested capacity.
+// 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)
@@ -170,7 +170,7 @@ func validateCapacity(value string, operator placementv1beta1.PropertySelectorOp
 	// Ensure capacity meets minimum requirements (minVMInstanceCapacity) after operator adjustment.
 	// The VMSizeRecommender API requires capacity > 0.
 	if capacity < minVMInstanceCapacity {
-		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

@@ -72,10 +72,11 @@ func TestValidateCapacity(t *testing.T) {
 			errorSubstring: "invalid capacity value",
 		},
 		{
-			name:      "invalid value of zero for capacity replaced with minimum",
-			value:     "0",
-			operator:  placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
-			wantValue: 1,
+			name:           "invalid value of zero for capacity",
+			value:          "0",
+			operator:       placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
+			wantError:      true,
+			errorSubstring: "is below minimum allowed value",
 		},
 		{
 			name:           "capacity equal to max limit with GreaterThan operator",
@@ -178,14 +179,15 @@ func TestExtractCapacityRequirements(t *testing.T) {
 			errorSubstring: "exceeds maximum allowed value of 200",
 		},
 		{
-			name: "invalid Azure SKU capacity property exceeding min limit",
+			name: "invalid Azure SKU capacity property below min limit",
 			req: placementv1beta1.PropertySelectorRequirement{
 				Name:     fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_B2ms"),
 				Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
-				Values:   []string{"1"},
+				Values:   []string{"0"},
 			},
-			wantSKU:           "Standard_B2ms",
-			wantCapacityValue: 1,
+			wantSKU:        "Standard_B2ms",
+			wantError:      true,
+			errorSubstring: "is below minimum allowed value",
 		},
 		{
 			name: "invalid Azure SKU capacity property with decimal",
@@ -296,7 +298,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),
@@ -417,7 +419,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
 			errorSubstring: "unsupported operator \"Eq\" for SKU capacity property, only GreaterThan (Gt) and GreaterThanOrEqualTo (Ge) are supported",
 		},
 		{
-			name:           "too low value in requirement replaced with minimum capacity",
+			name:           "too low value in requirement",
 			cluster:        cluster,
 			sku:            validSKU,
 			targetCapacity: 1,
@@ -426,8 +428,8 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
 				Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
 				Values:   []string{"0"},
 			},
-			mockStatusCode: http.StatusOK,
-			wantAvailable:  true,
+			wantError:      true,
+			errorSubstring: "is below minimum allowed value",
 		},
 		{
 			name:           "cases-insensitive request - available SKU",