fix(scheduler): use phase-aware accounting during NUMA refit

[asadjan4611]

What type of PR is this?

/kind bug

What this PR does / why we need it:

NUMA refit previously treated a pod's regular init-container memory and application-container memory as concurrent usage. This could reject a valid placement with CardInsufficientMemory.

This PR uses HAMi's existing init-container accounting rules while checking allowed NUMA devices. Regular init and application usage now use their phase peak, while sidecars and application containers remain concurrent.

GPU-b capacity: 40 GB

Regular init: 30 GB ─┐
                     ├── max(30, 20) = 30 GB ──> placement fits
Application:  20 GB ─┘

Sidecar:      30 GB ─┐
                     ├── 30 + 20 = 50 GB ──> placement rejected
Application:  20 GB ─┘

The candidate calculation reuses CollapseInitContainerUsage and SteadyStateDeviceUsage. It preserves the accounting of other pods and updates the cached reservation after a successful refit.

Which issue(s) this PR fixes:

Fixes #2856

Special notes for your reviewer:

Regression tests cover:

• A 30-GB regular init allocation and 20-GB application allocation fitting on a 40-GB GPU.
• Usage belonging to another pod remaining accounted.
• A sidecar starting after a larger regular init container.
• Sidecar and application memory remaining concurrent.
• Successful annotation and cached-accounting updates.

Only the NUMA-refit implementation and its focused scheduler tests are changed.

AI assistance disclosure:

I used OpenAI Codex to review the implementation and for this PR description.

Does this PR introduce a user-facing change?:

NUMA refit now uses phase-aware init and application container accounting, preventing valid placements from being rejected by double-counted GPU memory.

Summary by CodeRabbit

• Bug Fixes

  • Improved NUMA-aware resource recalculation as workloads transition from initialization to steady state.
  • Refined device capacity calculations for init containers, sidecars, reserved devices, and concurrent resource usage.
  • Corrected resource release tracking to restore the appropriate allocated capacity.
  • Device type matching is now more precise, and unsupported registered device types are rejected clearly.

• Tests

  • Added coverage for phase-aware NUMA allocation, concurrent sidecars, multiple application containers, and unsupported device types.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8ae02bd3-7007-4864-a859-2448875075f2

📥 Commits

Reviewing files that changed from the base of the PR and between 48835cc and 1385bea.

📒 Files selected for processing (5)

• pkg/device/numa_refit.go
• pkg/scheduler/numa_refit.go
• pkg/scheduler/numa_refit_handler.go
• pkg/scheduler/numa_refit_handler_test.go
• pkg/scheduler/score.go

Included review availability: Your plan provides up to 8 included reviews per hour; 6 remain after this review.

---

📝 Walkthrough

Walkthrough

The NUMA refit path now rebuilds candidate capacity from phase-aware init and application container usage. It aggregates per-device reservations, applies candidate-specific usage, releases recorded slot counts, and rejects unsupported device types. Tests cover sidecars, multiple applications, and Enflame validation.

Changes

NUMA refit accounting

Layer / File(s)	Summary
Device type filtering contract pkg/device/numa_refit.go, pkg/scheduler/score.go, pkg/scheduler/numa_refit.go	The request documentation defines NVIDIA-only support. Scheduler device filtering uses the shared deviceTypeMatches helper.
Phase-aware capacity reconstruction pkg/scheduler/numa_refit_handler.go	Refit preparation selects collapsed or steady-state usage, aggregates reservations by physical device, and preloads candidate-specific usage before fitting.
Reservation release and refit validation pkg/scheduler/numa_refit_handler.go, pkg/scheduler/numa_refit_handler_test.go	Reservation release subtracts recorded slot counts. Fixtures and tests cover sidecars, multiple application containers, concurrent slot totals, and unsupported Enflame devices.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: 🔵 Low · up to 1385b

NUMA refit now avoids double-counting regular init and application memory, allowing valid placements while keeping sidecars concurrent. The change is mergeable with owner awareness that an interruption during the annotation and accounting update sequence could temporarily leave scheduler reservations inconsistent.

Sequence Diagram(s)

sequenceDiagram
  participant RefitNumaAllocation
  participant PodManager
  participant effectivePodDeviceUsage
  participant CandidateDeviceCapacity
  RefitNumaAllocation->>PodManager: release moved-container usage
  RefitNumaAllocation->>effectivePodDeviceUsage: calculate effective phase usage
  effectivePodDeviceUsage->>CandidateDeviceCapacity: preload candidate-specific usage
  CandidateDeviceCapacity-->>RefitNumaAllocation: return capacity-fit result

Loading

Suggested reviewers: saiyam1814

Poem

A rabbit maps each GPU lane

Init peaks and app use share the frame
Recorded slots return in count
Candidate usage joins the amount
NVIDIA checks guide the train

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The phase-aware accounting changes are in scope for issue #2856. However, the broader device-type matching changes and non-NVIDIA rejection coverage are not directly required by the linked issue and a…	Move the device-type matching and non-NVIDIA rejection changes to a separate pull request, or document their direct dependency on the NUMA refit accounting fix and add issue scope that requires them.
Docstring Coverage	⚠️ Warning	Docstring coverage is 58.82% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: phase-aware accounting during NUMA refit.
Linked Issues check	✅ Passed	The changes address issue #2856 by reconstructing phase-aware usage, applying peak accounting for regular init and application containers, preserving concurrent sidecar usage, retaining other-pod usag…

Full details: Linked Issues check

Explanation

The changes address issue #2856 by reconstructing phase-aware usage, applying peak accounting for regular init and application containers, preserving concurrent sidecar usage, retaining other-pod usage, and adding focused coverage for valid NUMA refits and device validation.

Full details: Out of Scope Changes check

Explanation

The phase-aware accounting changes are in scope for issue #2856. However, the broader device-type matching changes and non-NVIDIA rejection coverage are not directly required by the linked issue and affect general scheduler behavior.

• Fix all pre-merge checks with AI

✨ 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.}

[codecov]

Codecov Report

❌ Patch coverage is 98.27586% with 1 line in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
pkg/scheduler/numa_refit_handler.go	98.18%	1 Missing ⚠️

Flag	Coverage Δ
unittests	69.36% <98.27%> (+3.88%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
pkg/scheduler/numa_refit.go	96.55% <100.00%> (+3.00%)	⬆️
pkg/scheduler/score.go	91.60% <100.00%> (+2.30%)	⬆️
pkg/scheduler/numa_refit_handler.go	87.67% <98.18%> (+13.53%)	⬆️

... and 102 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[archlitchi]

/lgtm

[hami-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: archlitchi, asadjan4611

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:

• OWNERS [archlitchi]

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