From 6bd05d64c7e477bc3fe20ef42f800e7bfbc8fbff Mon Sep 17 00:00:00 2001 From: John Belamaric Date: Tue, 12 May 2026 21:51:11 +0000 Subject: [PATCH] Fix DRA scoring bug with mixed allocated and unallocated claims --- .../dynamicresources/dynamicresources.go | 26 +++++++++++++++---- .../dynamicresources/dynamicresources_test.go | 13 ++++++++++ 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go index 11b5287fc67..c298b176668 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources.go @@ -1071,7 +1071,7 @@ func (pl *DynamicResources) unreservePodGroupClaims(ctx context.Context, pod *v1 } func (pl *DynamicResources) Score(ctx context.Context, cs fwk.CycleState, pod *v1.Pod, nodeInfo fwk.NodeInfo) (int64, *fwk.Status) { - if !pl.enabled { + if !pl.enabled || !pl.fts.EnableDRAPrioritizedList { return 0, nil } logger := klog.FromContext(ctx) @@ -1100,13 +1100,29 @@ func (pl *DynamicResources) Score(ctx context.Context, cs fwk.CycleState, pod *v func computeScore(iterator iter.Seq2[int, *resourceapi.ResourceClaim], allocations nodeAllocation) (int64, error) { var score int64 - for i, claim := range iterator { + unallocatedIndex := 0 + for _, claim := range iterator { // Collect the names for all allocated subrequests. allocatedSubRequests := sets.New[string]() - if i >= len(allocations.allocationResults) { - return 0, fmt.Errorf("number of allocations %d is smaller than number of claims", len(allocations.allocationResults)) + + var allocation *resourceapi.AllocationResult + // The allocation for a claim can be in two places: + // 1. For claims allocated in a previous cycle (e.g. PodGroup claims), the allocation + // is already in claim.Status.Allocation. + // 2. For claims allocated in this cycle (in Filter), the allocation is in + // allocations.allocationResults. + // Since we iterate over all claims, we must check both and maintain a separate index + // for claims that needed allocation in this cycle. + if claim.Status.Allocation != nil { + allocation = claim.Status.Allocation + } else { + if unallocatedIndex >= len(allocations.allocationResults) { + return 0, fmt.Errorf("number of allocations %d is smaller than number of claims needing allocation", len(allocations.allocationResults)) + } + allocation = &allocations.allocationResults[unallocatedIndex] + unallocatedIndex++ } - allocation := allocations.allocationResults[i] + for _, res := range allocation.Devices.Results { request := res.Request if resourceclaim.IsSubRequestRef(request) { diff --git a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go index 0efabe99fe8..0b33065a38b 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/dynamicresources_test.go @@ -4467,6 +4467,19 @@ func Test_computesScore(t *testing.T) { allocations: nodeAllocation{}, expectErr: true, }, + "mix-of-allocated-and-unallocated-claims": { + claims: []*resourceapi.ResourceClaim{ + allocatedClaim, + pendingClaim2, + }, + allocations: nodeAllocation{ + allocationResults: []resourceapi.AllocationResult{ + *allocationResult2, + }, + }, + expectedScore: 0, + expectErr: false, + }, "single-request-only-subrequest-allocated": { claims: []*resourceapi.ResourceClaim{ st.MakeResourceClaim().