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

for res, qt := range r.Status.Allocation.Hard {
// 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.
remaining := qt.DeepCopy()

amount, exists := r.Status.Allocation.Claimed[res]
if exists {
qt.Sub(amount)
remaining.Sub(amount)
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 {
remaining = resource.MustParse("0")
}
Comment thread
AruneshDwivedi marked this conversation as resolved.

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

r.Status.Allocation.Available = available
Expand Down Expand Up @@ -195,6 +206,13 @@ func (r *ResourcePool) GetAvailableClaimableResources() corev1.ResourceList {

qt.Sub(claimed)

// A pool can never offer a negative amount of resources, so clamp at
// zero here too, otherwise callers that build PoolExhausted messages
// from this value can surface negative available quantities.
if qt.Sign() < 0 {
qt = resource.MustParse("0")
}

hard[resourceName] = qt
}

Expand Down
61 changes: 61 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 Expand Up @@ -317,3 +355,26 @@ func TestIsBoundToResourcePool_2(t *testing.T) {
})

}

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

// Claimed exceeds Hard, so Hard-Claimed is negative. The helper must clamp
// at zero so negative available values never leak into PoolExhausted
// condition messages.
claimable := pool.GetAvailableClaimableResources()
got := claimable[corev1.ResourceLimitsCPU]
assert.Equal(t, 0, (&got).Cmp(resource.MustParse("0")))
assert.False(t, (&got).Sign() < 0)
}