fix(ascend): reject over-capacity memory on requests-only pods

[VedantMadane]

Description of the change

Ascend admission rejected over-capacity memory under limits but not under requests only. MutateAdmission returned early when the device count was missing from limits so the memory check never ran. GenerateResourceRequests fell back to requests and called trimMemory but treated a zero result (over capacity) as "no memory requested" and set MemPercentagereq to 100.

This change:

1. Makes MutateAdmission fall back to requests for device count and memory (same as GenerateResourceRequests).
2. Only applies the whole-card memory default when memory was not requested.
3. Keeps an over-capacity explicit request as Memreq instead of collapsing it to the whole-card path.
4. Adds regression tests for both paths.

Benefits

• Requests-only pods with impossible Ascend memory fail admission like limits-only pods.
• Scheduler path no longer rewrites invalid memory into a full card.

Possible drawbacks

• Pods that previously scheduled with requests-only over-capacity memory will now be rejected. That matches intended validation.

Applicable issues

• fixes [Ascend] Requests-only pods skip memory validation, so an over-capacity request becomes a whole-card request #2532

Additional information

go test ./pkg/device/ascend -run GenerateResourceRequests/MutateAdmission_RequestsOnly passes locally.

Summary by CodeRabbit

• Bug Fixes

  • Resource allocation now falls back to requested values when limits are not provided.
  • Prevented failures when resource configurations are initially empty.
  • Preserved explicitly requested memory values, including values above capacity.
  • Invalid over-capacity memory requests are now correctly rejected.
  • Whole-card defaults are applied only when no memory request is specified.
  • Improved requests-only allocation handling, including correct device-count rounding.

• Tests

  • Added regression coverage for requests-only memory configurations, over-capacity requests, and empty resource limits.

[hami-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: VedantMadane
Once this PR has been reviewed and has the lgtm label, please assign wawa0210 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 @VedantMadane! It looks like this is your first PR to Project-HAMi/HAMi 🎉

[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: a3ab15b4-1247-4796-b022-477a473b5c90

📥 Commits

Reviewing files that changed from the base of the PR and between 0b96931 and 1a821f3.

📒 Files selected for processing (2)

• pkg/device/ascend/device.go
• pkg/device/ascend/device_test.go

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

• pkg/device/ascend/device_test.go
• pkg/device/ascend/device.go

---

📝 Walkthrough

Walkthrough

Ascend resource handling now falls back from limits to requests, initializes resource maps, preserves oversized memory requests, and validates requests-only memory allocations. Regression tests cover request generation and admission behavior.

Changes

Ascend resource validation

Layer / File(s)	Summary
Admission resource fallback and map initialization pkg/device/ascend/device.go, pkg/device/ascend/device_test.go	MutateAdmission uses requests when limits are absent, initializes nil resource maps, validates requests-only memory, and adjusts SuperPod device counts.
Request generation memory handling pkg/device/ascend/device.go, pkg/device/ascend/device_test.go	GenerateResourceRequests preserves explicit over-capacity memory values and applies whole-card defaults only when memory is not requested. Tests cover this behavior.

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

Possibly related issues

• [Ascend] whole-card / memory-less hami-core (soft-split) requests bypass the node soft/hard mode gate (#1812) #2028 — Also changes Ascend resource admission and allocation handling.
• [bug]: ascend MutateAdmission panics on nil Requests (assignment to nil map) #2415 — Addresses nil resource map handling in Ascend admission.

Possibly related PRs

• Project-HAMi/HAMi#2338 — Modifies Ascend handling of oversized memory requests.
• Project-HAMi/HAMi#2416 — Adds related Ascend nil-map initialization behavior.
• Project-HAMi/HAMi#2538 — Addresses Ascend requests-only memory validation.

Suggested reviewers: ouyangluwei163, archlitchi

Poem

A rabbit checks each memory plea,
Requests and limits now agree.
Nil maps wait before they grow,
Oversized values trigger “no.”
Tests guard the card allocation flow.

🚥 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 summarizes the primary change: rejecting over-capacity memory requests for Ascend requests-only pods.
Linked Issues check	✅ Passed	The changes satisfy issue #2532 by validating requests-only pods and preserving over-capacity memory requests for rejection.
Out of Scope Changes check	✅ Passed	The code and regression tests directly support issue #2532 and the stated Ascend validation objectives.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@pkg/device/ascend/device.go`:
- Around line 119-124: Initialize ctr.Resources.Limits and
ctr.Resources.Requests immediately after the resource count fallback and before
the Ascend910C SuperPod mutation branch, so requests-only containers can be
updated without a nil-map panic. Preserve the existing count lookup and mutation
behavior, and add a regression test covering an Ascend910C SuperPod container
with only a one-device request.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7ba64a23-7cf9-451a-accf-431393633e0d

📥 Commits

Reviewing files that changed from the base of the PR and between 8812b79 and 0b96931.

📒 Files selected for processing (2)

• pkg/device/ascend/device.go
• pkg/device/ascend/device_test.go

[mesutoezdil]

You can view the relevant rule here.
https://github.com/Project-HAMi/HAMi/blob/master/CONTRIBUTING.md#contribution-gates