Expand NVIDIA device test coverage

[rishisulakhe]

What type of PR is this?

What this PR does / why we need it:

Added test coverage for three functions that were previously untested or under-tested.

1. TestMutateContainerResource (new, 8 sub-cases)
   mutateContainerResource had zero test coverage despite being called by MutateAdmission on every pod admission. The test covers:

• GPU count already in Limits → returns true without mutating
• Cores / memory / mem-percentage in Limits → injects DefaultGPUNum
• DefaultGPUNum=0 → no injection even when GPU sub-resources are present
• Resources in Requests only → not detected (documents that this function reads Limits only, unlike GenerateResourceRequests and resourceValue which fall back to Requests)
• GPU count of 0 in Limits → returns true

2. TestGenerateResourceRequests (+3 sub-cases)
   The existing test already had one Requests-fallback case (GPU count only). Added three more cases covering mem, cores, and mem-percentage in Requests-only — confirming that GenerateResourceRequests falls back to Requests for all resource types.
3. TestFit_MpsMode (new)
   Tests that Fit() succeeds when devices have Mode: "mps" and the pod has the nvidia.com/vgpu-mode: mps annotation. This code path in checkType was previously untested.
4. TestFit_NumaBindDifferentNumaFails (new)
   Complements the existing TestFit_NumaSwitching (which tests the success case). Verifies that Fit() returns false with reason containing NumaNotFit and AllocatedCardsInsufficientRequest when NumaBind: "true" is set but devices span different NUMA nodes.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Summary by CodeRabbit

• Tests
  • Expanded coverage for GPU memory, core, and memory-percentage requests specified only in container requests.
  • Added validation for default GPU resource mutation behavior.
  • Added coverage for multi-process service (MPS) allocation scenarios.
  • Added checks that allocations fail when NUMA requirements cannot be satisfied.
  • Improved test documentation and formatting to support more reliable GPU resource allocation behavior.

[hami-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: rishisulakhe
Once this PR has been reviewed and has the lgtm label, please assign fouof for approval. For more information see the Kubernetes Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• OWNERS

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

[hami-robot]

Welcome @rishisulakhe! It looks like this is your first PR to Project-HAMi/HAMi 🎉

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

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: c4e03090-3551-4770-a304-7c5481261154

📥 Commits

Reviewing files that changed from the base of the PR and between c35cb50 and d38e065.

📒 Files selected for processing (1)

• pkg/device/nvidia/device_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• pkg/device/nvidia/device_test.go

---

📝 Walkthrough

Walkthrough

The NVIDIA device tests add coverage for request-only resources, container resource mutation defaults, MPS allocation, and NUMA-spanning allocation failures. The opening block-comment delimiter is also corrected.

Changes

NVIDIA device behavior tests

Layer / File(s)	Summary
Resource request and mutation coverage pkg/device/nvidia/device_test.go	Adds request-only resource cases and table-driven mutation tests for defaults, disabled defaults, and ignored request-only resources.
MPS allocation coverage pkg/device/nvidia/device_test.go	Adds an MPS-mode test that allocates two devices.
NUMA binding failure coverage pkg/device/nvidia/device_test.go	Adds a test that rejects a two-device request across NUMA nodes and checks both failure reasons.

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

Possibly related PRs

• Project-HAMi/HAMi#2245: Adds related NVIDIA allocation tests and covers resource handling in the same device area.

Suggested reviewers: mesutoezdil, ouyangluwei163

Poem

A rabbit checks each GPU lane,
MPS hops across the testing plane.
Requests and defaults stand clear,
NUMA failures appear.
Clean tests thump paws with cheer.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the PR's main change: expanding NVIDIA device test coverage.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

[rishisulakhe]

@mesutoezdil This PR is ready for review. Please have a look when you get time.

[Eshiv-Pandey]

Ci is failing so run gofmt and make verify and push the fix. Consider adding /kind cleanup to the pr.
this pr has merge conflicts with master. do rebase/merge with master and resolve conflicts.

Also no ai disclosure is present add that also ;).

[Eshiv-Pandey]

noticed a tiny inconsistency.. TestFit_NumaSwitching uses assert.Equal(t, fit, true), so consider sticking to tht argument order only. TestFit_MpsMode, TestFit_NumaBindDifferentNumaFails and TestMutateContainerResource make them follow the norm

[archlitchi]

please fix this CI

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[rishisulakhe]

@archlitchi failing CI unit test is not related to PR

[Eshiv-Pandey]

@rishisulakhe the failing unit test CI was caused by invalid test cases, which have now been fixed. Could you please sync with the latest master and push again?

[FouoF]

Thanks for improving test coverage. This PR adds a substantial set of NVIDIA tests without connecting them to a reported defect, a behavior change, or a maintainer-confirmed coverage gap. The current unit-test check is failing, and the branch has not been updated after the maintainer requested a sync. Pure coverage growth is not enough to justify the ongoing review and maintenance cost here, so we are closing this PR. Future tests should accompany a concrete fix or protect an explicitly agreed contract.