Skip to content

Commit f987dbb

Browse files
committed
fixCapacityValueRequirements
Signed-off-by: Britania Rodriguez Reyes <[email protected]>
1 parent 92e7b42 commit f987dbb

File tree

2 files changed

+44
-18
lines changed

2 files changed

+44
-18
lines changed

pkg/propertychecker/azure/checker.go

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ const (
2727
// The value is constrained to a reasonable upper bound of 200 for most production workloads.
2828
// Upper bound is enforced after adjusting for operator semantics.
2929
maxVMInstanceCapacity = 200
30+
31+
// minVMInstanceCapacity defines the minimum allowed VM instance capacity for SKU capacity requirements.
32+
// The Azure Capacity API requires capacity values greater than 0, so minimum is set to 1.
33+
// Lower bound is enforced after adjusting for operator semantics.
34+
minVMInstanceCapacity = 1
3035
)
3136

3237
// PropertyChecker provides Azure-specific property validation for member clusters.
@@ -48,8 +53,8 @@ func NewPropertyChecker(vmSizeRecommenderClient compute.AttributeBasedVMSizeReco
4853

4954
// CheckIfMeetSKUCapacityRequirement validates whether a member cluster can meet the specified
5055
// SKU capacity requirement. It extracts the required SKU and capacity from the property selector
51-
// requirement and checks to determine if the cluster's Azure subscription
52-
// and location can provision the requested VM instances.
56+
// requirement and checks whether the cluster's Azure subscription and location can provision
57+
// the requested VM instances.
5358
//
5459
// The cluster must have both Azure location and subscription ID labels configured.
5560
// Returns true if the SKU capacity requirement can be met, false otherwise.
@@ -116,9 +121,8 @@ func (s *PropertyChecker) CheckIfMeetSKUCapacityRequirement(
116121
// extractCapacityRequirements extracts the capacity value from a PropertySelectorRequirement.
117122
// This function is specifically designed for Azure SKU capacity properties that follow the pattern:
118123
// "kubernetes.azure.com/vm-sizes/{sku}/capacity"
119-
// Returns the capacity if the requirement is valid;
120-
// The capacity will be updated based on the configured operator as VMSizeRecommender API
121-
// checks if the current allocatableCapacity > the requested capacity.
124+
// Returns the capacity value adjusted for the operator semantics, as the VMSizeRecommender API
125+
// checks whether current allocatable capacity is greater than the requested capacity.
122126
func extractCapacityRequirements(req placementv1beta1.PropertySelectorRequirement) (uint32, error) {
123127
if req.Operator != placementv1beta1.PropertySelectorGreaterThan && req.Operator != placementv1beta1.PropertySelectorGreaterThanOrEqualTo {
124128
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
149153

150154
capacity := uint32(valueUint) // capacity is >= 0 since it's parsed as uint.
151155

152-
if operator == placementv1beta1.PropertySelectorGreaterThanOrEqualTo && capacity > 0 {
156+
// Adjust capacity based on operator semantics for VMSizeRecommender API.
157+
// If operator is GreaterThanOrEqualTo, decrement capacity by 1 as the API checks for strictly greater than.
158+
if operator == placementv1beta1.PropertySelectorGreaterThanOrEqualTo && capacity >= minVMInstanceCapacity {
153159
capacity -= 1
154160
}
155161

@@ -158,9 +164,10 @@ func validateCapacity(value string, operator placementv1beta1.PropertySelectorOp
158164
return 0, fmt.Errorf("capacity value %d exceeds maximum allowed value of %d", capacity, maxVMInstanceCapacity)
159165
}
160166

161-
// A capacity of zero is only valid for GreaterThan operator.
162-
if capacity == 0 && operator != placementv1beta1.PropertySelectorGreaterThan {
163-
return 0, fmt.Errorf("capacity value cannot be zero for operator %q", operator)
167+
// Ensure capacity meets minimum requirements (minVMInstanceCapacity) after operator adjustment.
168+
// The VMSizeRecommender API requires capacity > 0.
169+
if capacity < minVMInstanceCapacity {
170+
return 0, fmt.Errorf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity)
164171
}
165172

166173
return capacity, nil

pkg/propertychecker/azure/checker_test.go

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,11 @@ func TestValidateCapacity(t *testing.T) {
7272
errorSubstring: "invalid capacity value",
7373
},
7474
{
75-
name: "unsupported operator for capacity of zero",
75+
name: "invalid value of zero for capacity",
7676
value: "0",
7777
operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
7878
wantError: true,
79-
errorSubstring: "capacity value cannot be zero for operator",
79+
errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity),
8080
},
8181
{
8282
name: "capacity equal to max limit with GreaterThan operator",
@@ -86,11 +86,12 @@ func TestValidateCapacity(t *testing.T) {
8686
errorSubstring: "exceeds maximum allowed value",
8787
},
8888
{
89-
name: "supported operator with capacity of zero",
90-
value: "0",
91-
operator: placementv1beta1.PropertySelectorGreaterThan,
92-
wantValue: 0,
93-
wantError: false,
89+
name: "unsupported operator with capacity of 1",
90+
value: "1",
91+
operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
92+
wantValue: 0,
93+
wantError: true,
94+
errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity),
9495
},
9596
{
9697
name: "capacity equal to max limit with supported operator",
@@ -113,6 +114,13 @@ func TestValidateCapacity(t *testing.T) {
113114
wantValue: 1,
114115
wantError: false,
115116
},
117+
{
118+
name: "capacity at minimum limit after adjustment",
119+
value: "2",
120+
operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
121+
wantValue: 1,
122+
wantError: false,
123+
},
116124
}
117125
for _, tt := range tests {
118126
t.Run(tt.name, func(t *testing.T) {
@@ -172,6 +180,17 @@ func TestExtractCapacityRequirements(t *testing.T) {
172180
wantError: true,
173181
errorSubstring: "exceeds maximum allowed value of 200",
174182
},
183+
{
184+
name: "invalid Azure SKU capacity property exceeding min limit",
185+
req: placementv1beta1.PropertySelectorRequirement{
186+
Name: fmt.Sprintf(azure.CapacityPerSKUPropertyTmpl, "Standard_B2ms"),
187+
Operator: placementv1beta1.PropertySelectorGreaterThanOrEqualTo,
188+
Values: []string{"1"},
189+
},
190+
wantSKU: "Standard_B2ms",
191+
wantError: true,
192+
errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity),
193+
},
175194
{
176195
name: "invalid Azure SKU capacity property with decimal",
177196
req: placementv1beta1.PropertySelectorRequirement{
@@ -402,7 +421,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
402421
errorSubstring: "unsupported operator \"Eq\" for SKU capacity property, only GreaterThan (Gt) and GreaterThanOrEqualTo (Ge) are supported",
403422
},
404423
{
405-
name: "unsupported operator in requirement",
424+
name: "invalid value in requirement",
406425
cluster: cluster,
407426
sku: validSKU,
408427
targetCapacity: 0,
@@ -413,7 +432,7 @@ func TestCheckIfMeetSKUCapacityRequirement(t *testing.T) {
413432
},
414433
mockStatusCode: http.StatusOK,
415434
wantError: true,
416-
errorSubstring: "capacity value cannot be zero for operator",
435+
errorSubstring: fmt.Sprintf("capacity value cannot be below minimum %d after adjustment", minVMInstanceCapacity),
417436
},
418437
{
419438
name: "cases-insensitive request - available SKU",

0 commit comments

Comments
 (0)