Skip to content

fix(deviceplugin): preserve preferred accelerator order and cap hint size - #804

Merged
thxCode merged 1 commit into
mainfrom
fix/preferred-index-order-and-size
Oct 29, 2026
Merged

thxCode merged 1 commit into
mainfrom
fix/preferred-index-order-and-size

Conversation

@thxCode

@thxCode thxCode commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

/kind bug
/area devicemanager

What this PR does / why we need it:

Fix three edge cases in GetPreferredAllocation when honoring device.gpustack.ai/accelerator.preferred-index and device.gpustack.ai/accelerator.preferred-id annotations:

  • Preserve the user-declared priority order in preferredAcceleratorIDsOf by returning an ordered, deduplicated []string instead of an unordered sets.Set[string].
  • Order matched preferred accelerators by their declared preference rank (after mustInclude accelerators), cap sel.Selected to allocationSize so kubelet does not receive more tokens than requested and pick among them in random map order, and sort the final hinted tokens in ascending hardware order.
  • Only fall back to sel.Unselected (or return an empty hint when len(sel.Unselected) == 0) when remainingSize > 0, preventing a satisfied preferred selection from being discarded when all available accelerators on the node are listed in preferred-index and one listed accelerator is unavailable.
  • Add table-driven unit tests in pkg/deviceplugin/server_test.go covering out-of-order priority selection, over-specified preferred lists, and empty-unselected fallback.

Which issue(s) this PR links to:

NONE

Special notes for your reviewer:

NONE

Does this PR introduce a user-facing change?

Fix GetPreferredAllocation to honor preferred-index / preferred-id priority order, cap hinted devices at the requested allocation size, and keep satisfied preferred selections when no unselected fallback devices remain.

…size

- preserve annotation order in preferredAcceleratorIDsOf for preferred-id and preferred-index
- order matched preferred accelerators by priority, cap selection at allocationSize, and sort returned tokens in ascending hardware order
- only fall back to unselected accelerators or return empty when remainingSize is still positive
- add table-driven unit tests for preferred-index priority order, size cap, and empty-unselected fallback

Task 1 of preferred-index-order-and-size.

Signed-off-by: thxCode <thxcode0824@gmail.com>
@gpustack-code-review

gpustack-code-review Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

🔍 OpenCodeReview found 1 issue(s) in this PR.

  • ✅ Successfully posted inline: 0 comment(s)
  • 📋 Routed to summary by policy: 1 comment(s)

test · low

📄 pkg/deviceplugin/server.go (L479-L481)

⚠️ GitHub could not post this as an inline comment: Routed to summary (severity low · category test)

Reminder (project rule, non-blocking to verify here): this is a bug fix in allocation logic — preference ordering, capping the hint to allocationSize, and eliminating the negative-index panic on the unselected fallback — so it should ship a regression test in the same PR. *_test.go files are excluded from this review, so the presence or absence of such a test cannot be confirmed here.

@thxCode

thxCode commented Oct 29, 2026

Copy link
Copy Markdown
Collaborator Author

Review Triage Summary

ID Location Verdict Outcome
IC_kwDOSVaals8AAAABguvfWQ (6491463513) pkg/deviceplugin/server.go:479-481 False positive (already covered) No code change required. Regression tests are already included in this PR in pkg/deviceplugin/server_test.go (TestResourceServer_PreferredAllocation_OrderSizeAndFallback and TestResourceServer_PreferredAllocation_StalePreferredIDDoesNotPanic), which OpenCodeReview did not see because *_test.go files are excluded from its diff scope.

@thxCode
thxCode merged commit 3e49060 into main Oct 29, 2026
8 checks passed
@thxCode
thxCode deleted the fix/preferred-index-order-and-size branch October 29, 2026 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant