From 7cf4f3b6560d8b2edccc1ba5093e2b515576101c Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes Date: Mon, 1 Dec 2025 10:36:02 -0600 Subject: [PATCH 1/6] fixCapacityValueRequirements Signed-off-by: Britania Rodriguez Reyes --- pkg/propertychecker/azure/checker.go | 25 ++++++++----- pkg/propertychecker/azure/checker_test.go | 37 ++++++++++++++----- .../plugins/clusteraffinity/types_test.go | 20 +++++++++- 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/pkg/propertychecker/azure/checker.go b/pkg/propertychecker/azure/checker.go index d2e94e9be..b505c4e22 100644 --- a/pkg/propertychecker/azure/checker.go +++ b/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 Capacity 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. @@ -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 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,7 +153,9 @@ 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 { + // Adjust capacity based on operator semantics for VMSizeRecommender API. + // If operator is GreaterThanOrEqualTo, decrement capacity by 1 as the API checks for strictly greater than. + if operator == placementv1beta1.PropertySelectorGreaterThanOrEqualTo && capacity >= minVMInstanceCapacity { capacity -= 1 } @@ -158,9 +164,10 @@ func validateCapacity(value string, operator placementv1beta1.PropertySelectorOp 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 cannot be below minimum %d after adjustment", minVMInstanceCapacity) } return capacity, nil diff --git a/pkg/propertychecker/azure/checker_test.go b/pkg/propertychecker/azure/checker_test.go index dfe80ddb4..3542725a9 100644 --- a/pkg/propertychecker/azure/checker_test.go +++ b/pkg/propertychecker/azure/checker_test.go @@ -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: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity), }, { name: "capacity equal to max limit with GreaterThan operator", @@ -86,11 +86,12 @@ func TestValidateCapacity(t *testing.T) { errorSubstring: "exceeds maximum allowed value", }, { - name: "supported operator with capacity of zero", - value: "0", - operator: placementv1beta1.PropertySelectorGreaterThan, - wantValue: 0, - wantError: false, + name: "unsupported operator with capacity of 1", + value: "1", + operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + wantValue: 0, + wantError: true, + errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity), }, { name: "capacity equal to max limit with supported operator", @@ -113,6 +114,13 @@ func TestValidateCapacity(t *testing.T) { wantValue: 1, wantError: false, }, + { + name: "capacity at minimum limit after adjustment", + value: "2", + operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + wantValue: 1, + wantError: false, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -172,6 +180,17 @@ func TestExtractCapacityRequirements(t *testing.T) { wantError: true, errorSubstring: "exceeds maximum allowed value of 200", }, + { + name: "invalid Azure SKU capacity property exceeding min limit", + req: placementv1beta1.PropertySelectorRequirement{ + Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_B2ms"), + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{"1"}, + }, + wantSKU: "Standard_B2ms", + wantError: true, + errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity), + }, { name: "invalid Azure SKU capacity property with decimal", req: placementv1beta1.PropertySelectorRequirement{ @@ -402,7 +421,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: "invalid value in requirement", cluster: cluster, sku: validSKU, targetCapacity: 0, @@ -413,7 +432,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { }, mockStatusCode: http.StatusOK, wantError: true, - errorSubstring: "capacity value cannot be zero for operator", + errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity), }, { name: "cases-insensitive request - available SKU", diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go index 648f34b44..18c32fe12 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go +++ b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go @@ -1350,15 +1350,31 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) { Name: validPropertyName, Operator: placementv1beta1.PropertySelectorGreaterThan, Values: []string{ - "0", + "1", }, }, }, cluster: cluster, sku: "Standard_D2s_v3", - targetCapacity: 0, + targetCapacity: 1, want: true, }, + { + name: "op >=, not matched (min limit)", + matchExpression: []placementv1beta1.PropertySelectorRequirement{ + { + Name: validPropertyName, + Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + Values: []string{ + "1", + }, + }, + }, + cluster: cluster, + sku: "Standard_D2s_v3", + targetCapacity: 0, + wantError: true, + }, { name: "op >, not matched", matchExpression: []placementv1beta1.PropertySelectorRequirement{ From ea8def194fc7b554a09726a2888683b7e01801c4 Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes Date: Thu, 4 Dec 2025 17:15:25 -0600 Subject: [PATCH 2/6] address some comments Signed-off-by: Britania Rodriguez Reyes --- pkg/propertychecker/azure/checker.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/propertychecker/azure/checker.go b/pkg/propertychecker/azure/checker.go index b505c4e22..247879956 100644 --- a/pkg/propertychecker/azure/checker.go +++ b/pkg/propertychecker/azure/checker.go @@ -155,7 +155,7 @@ func validateCapacity(value string, operator placementv1beta1.PropertySelectorOp // Adjust capacity based on operator semantics for VMSizeRecommender API. // If operator is GreaterThanOrEqualTo, decrement capacity by 1 as the API checks for strictly greater than. - if operator == placementv1beta1.PropertySelectorGreaterThanOrEqualTo && capacity >= minVMInstanceCapacity { + if operator == placementv1beta1.PropertySelectorGreaterThanOrEqualTo && capacity > minVMInstanceCapacity { capacity -= 1 } @@ -167,7 +167,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 { - return 0, fmt.Errorf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity) + capacity = minVMInstanceCapacity } return capacity, nil From b0167ca46619eb3d799212d29d1b07e36bd03ab0 Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes Date: Thu, 4 Dec 2025 17:29:19 -0600 Subject: [PATCH 3/6] fix UTs Signed-off-by: Britania Rodriguez Reyes --- pkg/propertychecker/azure/checker_test.go | 31 ++++++++----------- .../plugins/clusteraffinity/types_test.go | 6 ++-- 2 files changed, 16 insertions(+), 21 deletions(-) diff --git a/pkg/propertychecker/azure/checker_test.go b/pkg/propertychecker/azure/checker_test.go index 3542725a9..3a15c6460 100644 --- a/pkg/propertychecker/azure/checker_test.go +++ b/pkg/propertychecker/azure/checker_test.go @@ -72,11 +72,10 @@ func TestValidateCapacity(t *testing.T) { errorSubstring: "invalid capacity value", }, { - name: "invalid value of zero for capacity", - value: "0", - operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, - wantError: true, - errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity), + name: "invalid value of zero for capacity replaced with minimum", + value: "0", + operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + wantValue: 1, }, { name: "capacity equal to max limit with GreaterThan operator", @@ -86,12 +85,10 @@ func TestValidateCapacity(t *testing.T) { errorSubstring: "exceeds maximum allowed value", }, { - name: "unsupported operator with capacity of 1", - value: "1", - operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, - wantValue: 0, - wantError: true, - errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity), + name: "capacity at minimum limit with GreaterThanOrEqualTo operator", + value: "1", + operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, + wantValue: 1, }, { name: "capacity equal to max limit with supported operator", @@ -187,9 +184,8 @@ func TestExtractCapacityRequirements(t *testing.T) { Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, Values: []string{"1"}, }, - wantSKU: "Standard_B2ms", - wantError: true, - errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity), + wantSKU: "Standard_B2ms", + wantCapacityValue: 1, }, { name: "invalid Azure SKU capacity property with decimal", @@ -421,18 +417,17 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { errorSubstring: "unsupported operator \"Eq\" for SKU capacity property, only GreaterThan (Gt) and GreaterThanOrEqualTo (Ge) are supported", }, { - name: "invalid value in requirement", + name: "too low value in requirement replaced with minimum capacity", cluster: cluster, sku: validSKU, - targetCapacity: 0, + targetCapacity: 1, req: placementv1beta1.PropertySelectorRequirement{ Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, validSKU), Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, Values: []string{"0"}, }, mockStatusCode: http.StatusOK, - wantError: true, - errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity), + wantAvailable: true, }, { name: "cases-insensitive request - available SKU", diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go index 18c32fe12..bb00cc5bb 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go +++ b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go @@ -1360,7 +1360,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) { want: true, }, { - name: "op >=, not matched (min limit)", + name: "op >=, matched (min limit)", matchExpression: []placementv1beta1.PropertySelectorRequirement{ { Name: validPropertyName, @@ -1372,8 +1372,8 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) { }, cluster: cluster, sku: "Standard_D2s_v3", - targetCapacity: 0, - wantError: true, + targetCapacity: 1, + want: true, }, { name: "op >, not matched", From 279b054f18d13c6fa9bb5a03ababdbf07347532c Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes Date: Fri, 5 Dec 2025 12:11:36 -0600 Subject: [PATCH 4/6] fix capcity value adjustment and tests Signed-off-by: Britania Rodriguez Reyes --- pkg/propertychecker/azure/checker.go | 13 ++++++---- pkg/propertychecker/azure/checker_test.go | 24 +++++++++---------- .../plugins/clusteraffinity/filtering_test.go | 4 ++-- .../clusteraffinity/types_azure_test.go | 4 ++-- .../plugins/clusteraffinity/types_test.go | 8 +++---- 5 files changed, 28 insertions(+), 25 deletions(-) diff --git a/pkg/propertychecker/azure/checker.go b/pkg/propertychecker/azure/checker.go index 247879956..79c163b11 100644 --- a/pkg/propertychecker/azure/checker.go +++ b/pkg/propertychecker/azure/checker.go @@ -153,14 +153,17 @@ func validateCapacity(value string, operator placementv1beta1.PropertySelectorOp capacity := uint32(valueUint) // capacity is >= 0 since it's parsed as uint. - // Adjust capacity based on operator semantics for VMSizeRecommender API. - // If operator is GreaterThanOrEqualTo, decrement capacity by 1 as the API checks for strictly greater than. - if operator == placementv1beta1.PropertySelectorGreaterThanOrEqualTo && capacity > minVMInstanceCapacity { - 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) } diff --git a/pkg/propertychecker/azure/checker_test.go b/pkg/propertychecker/azure/checker_test.go index 3a15c6460..85279dce3 100644 --- a/pkg/propertychecker/azure/checker_test.go +++ b/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, }, { @@ -94,7 +94,7 @@ func TestValidateCapacity(t *testing.T) { name: "capacity equal to max limit with supported operator", value: "200", operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, - wantValue: 199, + wantValue: 200, wantError: false, }, { @@ -105,15 +105,15 @@ func TestValidateCapacity(t *testing.T) { errorSubstring: "exceeds maximum allowed value", }, { - name: "capacity at minimum limit", + name: "capacity at minimum limit (Gt)", value: "1", operator: placementv1beta1.PropertySelectorGreaterThan, - wantValue: 1, + wantValue: 2, wantError: false, }, { - name: "capacity at minimum limit after adjustment", - value: "2", + name: "capacity at minimum limit (Ge)", + value: "1", operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, wantValue: 1, wantError: false, @@ -163,7 +163,7 @@ func TestExtractCapacityRequirements(t *testing.T) { Values: []string{"4"}, }, wantSKU: "Standard_D4s_v3", - wantCapacityValue: 4, + wantCapacityValue: 5, wantError: false, }, { @@ -327,7 +327,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, @@ -337,7 +337,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, @@ -396,7 +396,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, @@ -433,7 +433,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { 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, diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/filtering_test.go b/pkg/scheduler/framework/plugins/clusteraffinity/filtering_test.go index 6a68a1240..a45885e20 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/filtering_test.go +++ b/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"), }, } diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go b/pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go index 6dceda911..165c81c29 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/types_azure_test.go +++ b/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, }, diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go index bb00cc5bb..1e56e44ee 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go +++ b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go @@ -1356,7 +1356,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) { }, cluster: cluster, sku: "Standard_D2s_v3", - targetCapacity: 1, + targetCapacity: 2, want: true, }, { @@ -1388,7 +1388,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) { }, cluster: cluster, sku: "Standard_D4s_v3", - targetCapacity: 8, + targetCapacity: 9, }, { name: "op >=, matched (max limit)", @@ -1403,7 +1403,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) { }, cluster: cluster, sku: "Standard_D2s_v3", - targetCapacity: 199, + targetCapacity: 200, want: true, }, { @@ -1419,7 +1419,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) { }, cluster: cluster, sku: "Standard_D4s_v3", - targetCapacity: 79, + targetCapacity: 80, }, } From 19d402a454e8ef06512dd536a6a1a9bcc2c5df5c Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes Date: Mon, 8 Dec 2025 12:25:18 -0600 Subject: [PATCH 5/6] address comments Signed-off-by: Britania Rodriguez Reyes --- pkg/propertychecker/azure/checker.go | 10 ++++----- pkg/propertychecker/azure/checker_test.go | 26 ++++++++++++----------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/pkg/propertychecker/azure/checker.go b/pkg/propertychecker/azure/checker.go index 79c163b11..b24a521e0 100644 --- a/pkg/propertychecker/azure/checker.go +++ b/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 diff --git a/pkg/propertychecker/azure/checker_test.go b/pkg/propertychecker/azure/checker_test.go index 85279dce3..a79273533 100644 --- a/pkg/propertychecker/azure/checker_test.go +++ b/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", From da8f7fc7b6655197d05bedecb0d662894337d3a2 Mon Sep 17 00:00:00 2001 From: Britania Rodriguez Reyes Date: Mon, 8 Dec 2025 13:43:04 -0600 Subject: [PATCH 6/6] address comments Signed-off-by: Britania Rodriguez Reyes --- pkg/propertychecker/azure/checker.go | 2 +- pkg/propertychecker/azure/checker_test.go | 14 ++++---------- .../plugins/clusteraffinity/types_test.go | 4 ++-- 3 files changed, 7 insertions(+), 13 deletions(-) diff --git a/pkg/propertychecker/azure/checker.go b/pkg/propertychecker/azure/checker.go index b24a521e0..c031beee6 100644 --- a/pkg/propertychecker/azure/checker.go +++ b/pkg/propertychecker/azure/checker.go @@ -80,7 +80,7 @@ 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 or equals to 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, diff --git a/pkg/propertychecker/azure/checker_test.go b/pkg/propertychecker/azure/checker_test.go index a79273533..749ff4ff9 100644 --- a/pkg/propertychecker/azure/checker_test.go +++ b/pkg/propertychecker/azure/checker_test.go @@ -85,12 +85,6 @@ func TestValidateCapacity(t *testing.T) { wantError: true, errorSubstring: "exceeds maximum allowed value", }, - { - name: "capacity at minimum limit with GreaterThanOrEqualTo operator", - value: "1", - operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, - wantValue: 1, - }, { name: "capacity equal to max limit with supported operator", value: "200", @@ -107,9 +101,9 @@ func TestValidateCapacity(t *testing.T) { }, { name: "capacity at minimum limit (Gt)", - value: "1", + value: "0", operator: placementv1beta1.PropertySelectorGreaterThan, - wantValue: 2, + wantValue: 1, wantError: false, }, { @@ -179,7 +173,7 @@ func TestExtractCapacityRequirements(t *testing.T) { errorSubstring: "exceeds maximum allowed value of 200", }, { - name: "invalid Azure SKU capacity property below min limit", + name: "invalid Azure SKU capacity property below min limit (Ge)", req: placementv1beta1.PropertySelectorRequirement{ Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_B2ms"), Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, @@ -422,7 +416,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) { name: "too low value in requirement", cluster: cluster, sku: validSKU, - targetCapacity: 1, + targetCapacity: 0, req: placementv1beta1.PropertySelectorRequirement{ Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, validSKU), Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo, diff --git a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go index 1e56e44ee..879aa0267 100644 --- a/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go +++ b/pkg/scheduler/framework/plugins/clusteraffinity/types_test.go @@ -1350,13 +1350,13 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) { Name: validPropertyName, Operator: placementv1beta1.PropertySelectorGreaterThan, Values: []string{ - "1", + "0", }, }, }, cluster: cluster, sku: "Standard_D2s_v3", - targetCapacity: 2, + targetCapacity: 1, want: true, }, {