Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions pkg/propertychecker/azure/checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand All @@ -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{
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
56 changes: 33 additions & 23 deletions pkg/propertychecker/azure/checker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand Down Expand Up @@ -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",
Expand All @@ -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,
},
{
Expand All @@ -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) {
Expand Down Expand Up @@ -158,7 +158,7 @@ func TestExtractCapacityRequirements(t *testing.T) {
Values: []string{"4"},
},
wantSKU: "Standard_D4s_v3",
wantCapacityValue: 4,
wantCapacityValue: 5,
wantError: false,
},
{
Expand All @@ -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{
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"),
},
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func TestMatchPropertiesInPropertyChecker(t *testing.T) {
Values: []string{"2"},
},
vmSize: "Standard_D2s_v3",
targetCapacity: 2,
targetCapacity: 3,
wantHandled: true,
wantAvailable: true,
},
Expand All @@ -137,7 +137,7 @@ func TestMatchPropertiesInPropertyChecker(t *testing.T) {
Values: []string{"2"},
},
vmSize: "NonExistentSKU",
targetCapacity: 1,
targetCapacity: 2,
wantHandled: true,
wantAvailable: false,
},
Expand Down
24 changes: 20 additions & 4 deletions pkg/scheduler/framework/plugins/clusteraffinity/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
{
Expand All @@ -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)",
Expand All @@ -1387,7 +1403,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) {
},
cluster: cluster,
sku: "Standard_D2s_v3",
targetCapacity: 199,
targetCapacity: 200,
want: true,
},
{
Expand All @@ -1403,7 +1419,7 @@ func TestClusterRequirementMatches_WithPropertyChecker(t *testing.T) {
},
cluster: cluster,
sku: "Standard_D4s_v3",
targetCapacity: 79,
targetCapacity: 80,
},
}

Expand Down
Loading