Missing Test for InitContainers & EphemeralContainers GPU Quota Checks

[aniket866]

Path Under Test

• webhook.go:L131-L178 (fitResourceQuota)

Explanation & Bug Gap

The admission webhook's quota controller only checks resource limits inside pod.Spec.Containers. If a pod runs an InitContainer requesting GPUs, the quota checks are bypassed entirely. The current tests in webhook_test.go only verify resource configurations within regular containers.

Why Add This Test

To verify that init containers and ephemeral containers cannot bypass namespace GPU quota constraints, ensuring proper resource isolation inside multi-tenant clusters.

Mermaid Diagram

graph TD
    A[Test Case: InitContainer with GPUs] --> B[fitResourceQuota]
    B --> C{Scan Containers, InitContainers}
    C --> D[Aggregate Total GPU Core/Memory]
    D --> E{Exceeds Quota?}
    E -- Yes --> F[Assert: Webhook Denies Admission]
    E -- No --> G[Assert: Webhook Allows Admission]

Loading

Proposed Test Implementation

Create a test inside webhook_test.go that mocks an admission request for a Pod where Spec.InitContainers requests a GPU count or memory limit exceeding the namespace resource quota. Assert that the admission response has Allowed: false and mentions exceeding the quota.

---

Summary by CodeRabbit

• Bug Fixes

  • Prevented crashes when GPUs report asymmetric peer-to-peer link information.
  • Added a warning and safely returns a fallback score of zero for affected GPU pairs.

• Documentation

  • Added a changelog entry documenting the fix in version 2.9.1.

[hami-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aniket866
Once this PR has been reviewed and has the lgtm label, please assign archlitchi 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

[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: d65cb76a-1247-4ef2-aca6-c19721594dda

📥 Commits

Reviewing files that changed from the base of the PR and between 2cabe29 and b08299b.

📒 Files selected for processing (3)

• CHANGELOG.md
• pkg/device/nvidia/calculate_score.go
• pkg/device/nvidia/calculate_score_test.go

---

📝 Walkthrough

Walkthrough

The GPU pair scoring path now handles asymmetric NVML P2P-link counts without panicking. It logs a warning and returns zero scores. A regression test covers both directions, and the changelog records the fix.

Changes

Asymmetric GPU link handling

Layer / File(s)	Summary
Warning fallback and regression coverage pkg/device/nvidia/calculate_score.go, pkg/device/nvidia/calculate_score_test.go, CHANGELOG.md	calculateGPUPairScore logs asymmetric link counts with GPU UUIDs and returns zero. The test validates both GPU directions. The changelog records the v2.9.1 fix.

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

Possibly related issues

• Missing Test for Asymmetric NVML GPU Link Topologies #2365 — Requests the asymmetric NVML GPU link handling and regression test implemented here.

Possibly related PRs

• Project-HAMi/HAMi#2174 — Updates calculateGPUPairScore and tests for asymmetric-link behavior.
• Project-HAMi/HAMi#2217 — Handles asymmetric GPU P2P links without panicking.
• Project-HAMi/HAMi#2229 — Replaces the asymmetric-link panic with logging and a zero score.

Suggested reviewers: wawa0210, ouyangluwei163

Poem

A rabbit found two GPUs in line,
One link showed, the other drew none.
“No panic,” said Bun, “log what we see,”
Then scored both paths at zero, neatly.
The changelog hopped after,
And tests thumped with laughter.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title describes GPU quota tests for container types, but the changes fix asymmetric NVML GPU link handling and add related tests.	Update the title to describe the asymmetric NVML GPU link panic fix and the added regression test.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
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.}

Warning

⚠️ This pull request shows signs of AI-generated slop (description_diff_mismatch). It has been flagged by CodeRabbit slop detection and should be reviewed carefully.

[mesutoezdil]

pls read and understand what are you doing with codes first