Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
14 changes: 12 additions & 2 deletions api/v1beta2/resourcepool_func.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,23 @@
func (r *ResourcePool) CalculateAvailableResources() {
available := corev1.ResourceList{}

for res, qt := range r.Status.Allocation.Hard {

Check failure on line 161 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.33.0)

declared and not used: qt

Check failure on line 161 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / lint

declared and not used: qt

Check failure on line 161 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / Unit tests

declared and not used: qt

Check failure on line 161 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (CE)

declared and not used: qt

Check failure on line 161 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.34.0)

declared and not used: qt

Check failure on line 161 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.32.0)

declared and not used: qt

Check failure on line 161 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.18.0-okd-scos.9)

declared and not used: qt

Check failure on line 161 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E OpenShift Testing (CE)

declared and not used: qt

Check failure on line 161 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.19.0-okd-scos.17)

declared and not used: qt
// Deep-copy qt before subtracting: resource.Quantity contains internal pointers,
// so a shallow copy can alias the map entry and Sub() could mutate Hard.
// Using DeepCopy() ensures Hard remains unchanged.

amount, exists := r.Status.Allocation.Claimed[res]
if exists {
qt.Sub(amount)
remaining.Sub(amount)

Check failure on line 168 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.33.0)

undefined: remaining

Check failure on line 168 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / lint

undefined: remaining

Check failure on line 168 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / Unit tests

undefined: remaining

Check failure on line 168 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (CE)

undefined: remaining

Check failure on line 168 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.34.0)

undefined: remaining

Check failure on line 168 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.32.0)

undefined: remaining

Check failure on line 168 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.18.0-okd-scos.9)

undefined: remaining

Check failure on line 168 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E OpenShift Testing (CE)

undefined: remaining

Check failure on line 168 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.19.0-okd-scos.17)

undefined: remaining
Comment on lines 167 to +169
}

// A pool can never offer a negative amount of resources, so clamp
// at zero. This also keeps Hard intact for downstream math.
if remaining.Sign() < 0 {

Check failure on line 173 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.33.0)

undefined: remaining

Check failure on line 173 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / lint

undefined: remaining

Check failure on line 173 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / Unit tests

undefined: remaining

Check failure on line 173 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (CE)

undefined: remaining

Check failure on line 173 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.34.0)

undefined: remaining

Check failure on line 173 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.32.0)

undefined: remaining

Check failure on line 173 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.18.0-okd-scos.9)

undefined: remaining

Check failure on line 173 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E OpenShift Testing (CE)

undefined: remaining

Check failure on line 173 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.19.0-okd-scos.17)

undefined: remaining
remaining = resource.MustParse("0")

Check failure on line 174 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.33.0)

undefined: remaining

Check failure on line 174 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / lint

undefined: remaining

Check failure on line 174 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / Unit tests

undefined: remaining

Check failure on line 174 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (CE)

undefined: remaining

Check failure on line 174 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.34.0)

undefined: remaining

Check failure on line 174 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.32.0)

undefined: remaining

Check failure on line 174 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.18.0-okd-scos.9)

undefined: remaining

Check failure on line 174 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E OpenShift Testing (CE)

undefined: remaining

Check failure on line 174 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.19.0-okd-scos.17)

undefined: remaining
}
Comment thread
AruneshDwivedi marked this conversation as resolved.

available[res] = qt
available[res] = remaining

Check failure on line 177 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.33.0)

undefined: remaining

Check failure on line 177 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / lint

undefined: remaining (typecheck)

Check failure on line 177 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / Unit tests

undefined: remaining

Check failure on line 177 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (CE)

undefined: remaining

Check failure on line 177 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.34.0)

undefined: remaining

Check failure on line 177 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (v1.32.0)

undefined: remaining

Check failure on line 177 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.18.0-okd-scos.9)

undefined: remaining

Check failure on line 177 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E OpenShift Testing (CE)

undefined: remaining

Check failure on line 177 in api/v1beta2/resourcepool_func.go

View workflow job for this annotation

GitHub Actions / E2E Testing (OpenShift) (4.19.0-okd-scos.17)

undefined: remaining
}

r.Status.Allocation.Available = available
Expand Down
38 changes: 38 additions & 0 deletions api/v1beta2/resourcepool_func_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,44 @@ func TestCalculateResources(t *testing.T) {
assert.Equal(t, 0, (&actualAvailable).Cmp(resource.MustParse("1")))
}

func TestCalculateResources_OverSubscriptionDoesNotGoNegative(t *testing.T) {
pool := &capsulev1beta2.ResourcePool{
Status: capsulev1beta2.ResourcePoolStatus{
Allocation: capsulev1beta2.ResourcePoolQuotaStatus{
Hard: corev1.ResourceList{
corev1.ResourceLimitsCPU: resource.MustParse("10"),
},
},
Claims: capsulev1beta2.ResourcePoolNamespaceClaimsStatus{
"ns": {
&capsulev1beta2.ResourcePoolClaimsItem{
Claims: corev1.ResourceList{
corev1.ResourceLimitsCPU: resource.MustParse("55"),
},
},
},
},
},
}

pool.CalculateClaimedResources()

// Even when claims exceed the hard limit, available must clamp at zero
// and Hard must stay intact (regression: subtracting in place corrupted
// both Hard and Available, producing negative values — see issue #1977).
actualAvailable := pool.Status.Allocation.Available[corev1.ResourceLimitsCPU]
assert.Equal(t, 0, (&actualAvailable).Cmp(resource.MustParse("0")))

actualHard := pool.Status.Allocation.Hard[corev1.ResourceLimitsCPU]
assert.Equal(t, 0, (&actualHard).Cmp(resource.MustParse("10")))

// With zero available, a new oversized claim must be rejected.
errs := pool.CanClaimFromPool(corev1.ResourceList{
corev1.ResourceLimitsCPU: resource.MustParse("55"),
})
assert.Len(t, errs, 1)
}

func TestCanClaimFromPool(t *testing.T) {
pool := &capsulev1beta2.ResourcePool{
Status: capsulev1beta2.ResourcePoolStatus{
Expand Down
Loading