github/kubernetes
1
0
Fork 0
mirror of https://github.com/k3s-io/kubernetes.git synced 2026-07-17 11:28:58 +00:00
Code Issues Projects Releases Wiki Activity

DRA: support non-pod references in ReservedFor (#136450)

Browse Source 
* DRA: support non-pod references in ReservedFor

Signed-off-by: MohammedSaalif <salifud2004@gmail.com>

* Expand reservation validation comment in syncClaim as suggested by mortent

* Address feedback: rename valid to remaining and remove obsolete TODO

---------

Signed-off-by: MohammedSaalif <salifud2004@gmail.com>
This commit is contained in:
MohammedSaalif
2026-01-25 00:28:13 +05:30
committed by GitHub
parent b4947a5891
commit 4925c6bea4
2 changed files with 47 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
pkg/controller/resourceclaim/controller.go
View File

@@ -841,8 +841,10 @@ func (ec *Controller) syncClaim(ctx context.Context, namespace, name string) err
return err
}
// Check if the ReservedFor entries are all still valid.
valid := make([]resourceapi.ResourceClaimConsumerReference, 0, len(claim.Status.ReservedFor))
// Check if the ReservedFor entries are all still valid. Essentially we are
// validating and potentially removing pod references, but just leaving any
// non-pod references in the list.
remaining := make([]resourceapi.ResourceClaimConsumerReference, 0, len(claim.Status.ReservedFor))
for _, reservedFor := range claim.Status.ReservedFor {
if reservedFor.APIGroup == "" &&
reservedFor.Resource == "pods" {
@@ -892,22 +894,24 @@ func (ec *Controller) syncClaim(ctx context.Context, namespace, name string) err
}
if keepEntry {
valid = append(valid, reservedFor)
remaining = append(remaining, reservedFor)
}
continue
}
// TODO: support generic object lookup
return fmt.Errorf("unsupported ReservedFor entry: %v", reservedFor)
// We don't know how to check this entry, so we just keep it to avoid
// accidentally removing a reservation for a non-pod consumer that we
// don't support yet.
remaining = append(remaining, reservedFor)
}
builtinControllerFinalizer := slices.Index(claim.Finalizers, resourceapi.Finalizer)
logger.V(5).Info("Claim reserved for counts", "currentCount", len(claim.Status.ReservedFor), "claim", klog.KRef(namespace, name), "updatedCount", len(valid), "builtinController", builtinControllerFinalizer >= 0)
if len(valid) < len(claim.Status.ReservedFor) {
logger.V(5).Info("Claim reserved for counts", "currentCount", len(claim.Status.ReservedFor), "claim", klog.KRef(namespace, name), "updatedCount", len(remaining), "builtinController", builtinControllerFinalizer >= 0)
if len(remaining) < len(claim.Status.ReservedFor) {
// This is not using a patch because we want the update to fail if anything
// changed in the meantime.
claim := claim.DeepCopy()
claim.Status.ReservedFor = valid
claim.Status.ReservedFor = remaining
// DRA always performs delayed allocations. Relatedly, it also
// deallocates a claim as soon as the last consumer stops using
@@ -925,7 +929,7 @@ func (ec *Controller) syncClaim(ctx context.Context, namespace, name string) err
// those, the resource claim controller will trigger deletion when the
// pod is done. However, it doesn't hurt to also trigger deallocation
// for such claims and not checking for them keeps this code simpler.
if len(valid) == 0 {
if len(remaining) == 0 {
// This is a sanity check. There shouldn't be any claims without this
// finalizer because there's no longer any other way of allocating claims.
// Classic DRA was the alternative earlier.
@@ -950,7 +954,7 @@ func (ec *Controller) syncClaim(ctx context.Context, namespace, name string) err
return err
}
}
} else if builtinControllerFinalizer >= 0 && claim.DeletionTimestamp != nil && len(valid) == 0 {
} else if builtinControllerFinalizer >= 0 && claim.DeletionTimestamp != nil && len(remaining) == 0 {
claim := claim.DeepCopy()
if claim.Status.Allocation != nil {
// This can happen when a claim with immediate allocation
@@ -971,7 +975,7 @@ func (ec *Controller) syncClaim(ctx context.Context, namespace, name string) err
}
}
if len(valid) == 0 {
if len(remaining) == 0 {
// Claim is not reserved. If it was generated for a pod and
// that pod is not going to run, the claim can be
// deleted. Normally the garbage collector does that, but the

32
pkg/controller/resourceclaim/controller_test.go
View File

@@ -400,6 +400,38 @@ func TestSyncHandler(t *testing.T) {
},
expectedMetrics: expectedMetrics{0, 0, 0, 0},
},
{
name: "clean up pod reservation with non-pod reservation present",
pods: func() []*v1.Pod {
pod := testPodWithResource.DeepCopy()
pod.Status.Phase = v1.PodSucceeded
return []*v1.Pod{pod}
}(),
claims: func() []*resourceapi.ResourceClaim {
claim := testClaimReserved.DeepCopy()
nonPodRef := resourceapi.ResourceClaimConsumerReference{
APIGroup: "foo.com",
Resource: "foo",
Name: "foo",
UID: "123",
}
claim.Status.ReservedFor = append(claim.Status.ReservedFor, nonPodRef)
return []*resourceapi.ResourceClaim{claim}
}(),
key: testClaimKey,
expectedClaims: []resourceapi.ResourceClaim{func() resourceapi.ResourceClaim {
claim := testClaimReserved.DeepCopy()
nonPodRef := resourceapi.ResourceClaimConsumerReference{
APIGroup: "foo.com",
Resource: "foo",
Name: "foo",
UID: "123",
}
claim.Status.ReservedFor = []resourceapi.ResourceClaimConsumerReference{nonPodRef}
return *claim
}()},
expectedMetrics: expectedMetrics{0, 0, 0, 0},
},
}
for _, tc := range tests {