[thc1006]

What type of PR is this?

/kind bug

What this PR does / why we need it:

countDevicesPerClass sums per-request device counts into an int64 with no overflow guard, and device counts are unbounded (the apiserver only requires count > 0, so it accepts up to MaxInt64). A ResourceClaimTemplate whose counts sum past MaxInt64 wraps to a negative value: the quota usage goes negative, which admits a Workload that exceeds quota and leaves a negative used-quota in the ClusterQueue status.

This accumulates with utilmath.SaturatingAdd (matching the Amount arithmetic the scheduler and cache already use), so the sum saturates at MaxInt64 instead of wrapping negative. A huge count then stays a large positive that cannot fit quota, rather than being admitted on a negative. The count is turned into a resource.Quantity right after this, so the saturated value is handled by overflow-safe Quantity arithmetic downstream.

A unit test covers both the normal sum and the saturation case.

Which issue(s) this PR fixes:

Fixes #12896

Special notes for your reviewer:

Switched from an explicit validation error to SaturatingAdd per review feedback, to match the resilient Amount arithmetic used across the scheduler and cache. The extended-resource path already uses Quantity arithmetic and is unaffected.

Does this PR introduce a user-facing change?

DRA: Fix an integer overflow in device-count quota accounting where a ResourceClaimTemplate with very large device counts could be admitted over quota and leave a negative used-quota in the ClusterQueue status.

[netlify]

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name	Link
🔨 Latest commit	f0fef9b
🔍 Latest deploy log	https://app.netlify.com/projects/kubernetes-sigs-kueue/deploys/6a4fc8a4d6d53c0008251c0b

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[kubernetes-prow]

Hi @thc1006. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds saturating arithmetic to per-device-class device counting in pkg/dra/claims.go and adds a regression test covering normal accumulation and math.MaxInt64 saturation.

Changes

Device Count Overflow Protection

Layer / File(s)	Summary
Overflow-safe aggregation pkg/dra/claims.go, pkg/dra/claims_test.go	Uses utilmath.SaturatingAdd in countDevicesPerClass, adds an exactReq helper, and adds Test_countDevicesPerClass_overflow to check normal summation and saturation at math.MaxInt64.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• kubernetes-sigs/kueue#12451: Also modifies countDevicesPerClass in pkg/dra/claims.go, touching the same device count aggregation logic.

Suggested reviewers: kshalot, sohankunkerkar

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	The change saturates counts instead of rejecting overflow with a field error, so it misses the issue's required behavior.	Detect per-class overflow and return a field error instead of clamping the sum.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Out of Scope Changes check	✅ Passed	The diff stays focused on DRA device counting and its regression test, with no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main fix: handling device-count sums that can overflow int64.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mimowo]

/ok-to-test

[mimowo]

Wdyt about using the "Amount" structure we use for scheduling. This strucutre instead of validation error caps at int64. I'm ok with both, but maybe worth to consider using Amount more in DRA code for better resilience instead of checking individual operations, wdyt @sohankunkerkar ?

Validation is also ok, just a matter of preference, because values near 64 don't have much of practical use anyway.

[thc1006]

Good point. SaturatingAdd is what the scheduler and cache code already use everywhere via Amount, and countDevicesPerClass is really the outlier still doing raw int64. Aligning it with that pattern is cleaner than a one-off validation error, and I agree counts near MaxInt64 have no practical use.

I've switched the accumulation to utilmath.SaturatingAdd(out[dc], q) and updated the test to assert it saturates rather than wraps negative. Happy to go back to the validation error if you or @sohankunkerkar would rather keep an explicit signal.

[mimowo]

Yeah, both approaches have some merit, but I think Amount is ok and more consistent with the rest of the codebase

[thc1006]

Thanks for confirming. The latest revision (f0fef9b) already accumulates with SaturatingAdd, which is the same helper that Amount.AddInt64 wraps, so countDevicesPerClass now uses the scheduler saturating arithmetic while keeping the Requests int64 return type unchanged. If you would prefer this call site to use the Amount type directly, I am glad to switch it.

[mimowo]

/lgtm
/approve
/cherrypick release-0.18
/cherrypick release-0.17
Thank you for hardening Kueue 👍

[k8s-infra-cherrypick-robot]

@mimowo: once the present PR merges, I will cherry-pick it on top of release-0.17, release-0.18 in new PRs and assign them to you.

Details

In response to this:

/lgtm
/approve
/cherrypick release-0.18
/cherrypick release-0.17
Thank you for hardening Kueue 👍

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[kubernetes-prow]

LGTM label has been added.

Details

Git tree hash: 3409fee07535b4daed59768ddfb2a9e6b6d59574

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mimowo, thc1006

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• pkg/dra/OWNERS [mimowo]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[k8s-infra-cherrypick-robot]

@mimowo: #12897 failed to apply on top of branch "release-0.17":

Applying: dra: use a saturating add for device counts to avoid int64 overflow
Using index info to reconstruct a base tree...
M	pkg/dra/claims.go
M	pkg/dra/claims_test.go
Falling back to patching base and 3-way merge...
Auto-merging pkg/dra/claims.go
Auto-merging pkg/dra/claims_test.go
CONFLICT (content): Merge conflict in pkg/dra/claims_test.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0001 dra: use a saturating add for device counts to avoid int64 overflow

Details

In response to this:

/lgtm
/approve
/cherrypick release-0.18
/cherrypick release-0.17
Thank you for hardening Kueue 👍

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[k8s-infra-cherrypick-robot]

@mimowo: new pull request created: #12901

Details

In response to this:

/lgtm
/approve
/cherrypick release-0.18
/cherrypick release-0.17
Thank you for hardening Kueue 👍

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[mimowo]

/release-note-edit

DRA: Fix an integer overflow in device-count quota accounting where a ResourceClaimTemplate with very large device counts could be admitted over quota and leave a negative used-quota in the ClusterQueue status.