Skip to content

Commit 7e7c636

Browse files
committed
fix
Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>
1 parent 73637ed commit 7e7c636

File tree

2 files changed

+24
-25
lines changed

2 files changed

+24
-25
lines changed

pkg/propertychecker/azure/checker.go

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ func (s *PropertyChecker) CheckIfMeetSKUCapacityRequirement(
9393
Location: location,
9494
RegularPriorityProfile: &computev1.RegularPriorityProfile{
9595
CapacityUnitType: computev1.CapacityUnitType_CAPACITY_UNIT_TYPE_VM_INSTANCE_COUNT,
96-
TargetCapacity: uint32(capacity.Value()),
96+
TargetCapacity: capacity,
9797
},
9898
ResourceProperties: &computev1.ResourceProperties{
9999
VmAttributes: &computev1.VMAttributes{
@@ -127,49 +127,53 @@ func (s *PropertyChecker) CheckIfMeetSKUCapacityRequirement(
127127
// "kubernetes.azure.com/vm-sizes/{sku}/capacity"
128128
// Returns the capacity as a resource.Quantity and the SKU name if the requirement is valid,
129129
// or an error if the requirement is invalid or not a capacity property.
130-
func extractCapacityRequirements(req placementv1beta1.PropertySelectorRequirement) (*resource.Quantity, string, error) {
130+
func extractCapacityRequirements(req placementv1beta1.PropertySelectorRequirement) (uint32, string, error) {
131131
// Extract SKU from the property name using regex
132132
// Expected format: "kubernetes.azure.com/vm-sizes/{sku}/capacity"
133133
matches := skuCapacityRegex.FindStringSubmatch(req.Name)
134134
if len(matches) != 2 {
135-
return nil, "", fmt.Errorf("property name %q does not match expected SKU capacity format %q", req.Name, azure.SKUCapacityPropertyTmpl)
135+
return 0, "", fmt.Errorf("property name %q does not match expected SKU capacity format %q", req.Name, azure.SKUCapacityPropertyTmpl)
136136
}
137137
sku := matches[1]
138138

139139
// Validate that we have exactly one value
140140
if len(req.Values) != 1 {
141-
return nil, "", fmt.Errorf("azure SKU capacity property must have exactly one value, got %d", len(req.Values))
141+
return 0, "", fmt.Errorf("azure SKU capacity property must have exactly one value, got %d", len(req.Values))
142142
}
143143

144144
// Parse the capacity value
145145
capacity, err := resource.ParseQuantity(req.Values[0])
146146
if err != nil {
147-
return nil, "", fmt.Errorf("failed to parse capacity value %q: %w", req.Values[0], err)
147+
return 0, "", fmt.Errorf("failed to parse capacity value %q: %w", req.Values[0], err)
148148
}
149149

150150
// Ensure capacity is a whole number (integer) since VM instance count cannot be fractional
151+
milliValue := capacity.MilliValue()
152+
if milliValue%1000 != 0 {
153+
return 0, "", fmt.Errorf("capacity value %q must be a whole number, decimal values are not allowed for VM instance count", req.Values[0])
154+
}
155+
156+
// Get the integer value for validation
151157
capacityValue := capacity.Value()
152158

159+
// Validate capacity is non-negative
160+
if capacityValue < 0 {
161+
return 0, "", fmt.Errorf("capacity value %d cannot be negative", capacityValue)
162+
}
163+
153164
// Validate capacity bounds
154165
if capacityValue < MinVMInstanceCapacity {
155-
return nil, "", fmt.Errorf("capacity value %d is below minimum allowed value of %d", capacityValue, MinVMInstanceCapacity)
166+
return 0, "", fmt.Errorf("capacity value %d is below minimum allowed value of %d", capacityValue, MinVMInstanceCapacity)
156167
}
157168
if capacityValue > MaxVMInstanceCapacity {
158-
return nil, "", fmt.Errorf("capacity value %d exceeds maximum allowed value of %d", capacityValue, MaxVMInstanceCapacity)
169+
return 0, "", fmt.Errorf("capacity value %d exceeds maximum allowed value of %d", capacityValue, MaxVMInstanceCapacity)
159170
}
160171

161172
// Ensure capacity can be safely converted to uint32
162173
if capacityValue > uint32MaxValue {
163-
return nil, "", fmt.Errorf("capacity value %d exceeds uint32 maximum value %d", capacityValue, uint32MaxValue)
164-
}
165-
if capacityValue < 0 {
166-
return nil, "", fmt.Errorf("capacity value %d cannot be negative", capacityValue)
167-
}
168-
169-
milliValue := capacity.MilliValue()
170-
if milliValue%1000 != 0 {
171-
return nil, "", fmt.Errorf("capacity value %q must be a whole number, decimal values are not allowed for VM instance count", req.Values[0])
174+
return 0, "", fmt.Errorf("capacity value %d exceeds uint32 maximum value %d", capacityValue, uint32MaxValue)
172175
}
173176

174-
return &capacity, sku, nil
177+
// Safe conversion to uint32 - all validations passed
178+
return uint32(capacityValue), sku, nil
175179
}

pkg/propertychecker/azure/checker_test.go

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func TestExtractCapacityRequirements(t *testing.T) {
2727
name string
2828
req placementv1beta1.PropertySelectorRequirement
2929
wantSKU string
30-
wantQty int64
30+
wantQty uint32
3131
wantError bool
3232
errorSubstring string
3333
}{
@@ -128,13 +128,8 @@ func TestExtractCapacityRequirements(t *testing.T) {
128128
t.Errorf("extractCapacityRequirements() sku = %v, got %v", tt.wantSKU, sku)
129129
}
130130

131-
if capacity == nil {
132-
t.Errorf("extractCapacityRequirements() capacity is nil")
133-
return
134-
}
135-
136-
if capacity.Value() != tt.wantQty {
137-
t.Errorf("extractCapacityRequirements() capacity = %v, got %v", tt.wantQty, capacity.String())
131+
if capacity != tt.wantQty {
132+
t.Errorf("extractCapacityRequirements() capacity = %v, got %v", tt.wantQty, capacity)
138133
}
139134
})
140135
}

0 commit comments

Comments
 (0)