fix(ascend): stop whole-card/memory-less hami-core requests from bypassing the node mode gate by Wangmin362 · Pull Request #2029 · Project-HAMi/HAMi

Wangmin362 · 2026-07-08T00:52:38Z

What this PR does

Fixes #2028.

The node soft/hard-split exclusivity added in #1812 rejects a mismatched pod with ModeNotFit, but the check sits inside request.Memreq > 0 && request.Memreq < totalMemPerCard && request.Nums > 0. A whole-card memory request (Memreq == card capacity) or a memory-less request (Memreq == 0) skips the block, so a hami-core (soft-split) pod can be scheduled onto a hard-split node.

This PR lifts the soft-pod arm out of the memory-range condition: a pod that declares vnpu-mode: hami-core must land on a hami-core node regardless of how much memory it requests.

// applies to whole-card and memory-less requests too
if isHAMiCore && !nodeSupportHamiCore {
    reason[common.ModeNotFit]++
    return false, nil, common.GenReason(reason, len(devices))
}

// reverse arm kept inside the memory-range condition on purpose
if request.Memreq > 0 && request.Memreq < totalMemPerCard && request.Nums > 0 {
    if nodeSupportHamiCore && !isHAMiCore {
        reason[common.ModeNotFit]++
        return false, nil, common.GenReason(reason, len(devices))
    }
}

Why only the soft-pod arm (not a symmetric change)

The reverse arm (a legacy/whole-card pod on a hami-core node) is deliberately left inside the memory-range condition. Making it symmetric would newly reject plain whole-card jobs on any hami-core node, and because nodeSupportHamiCore defaults to the global hamiVnpuCore value, a cluster with hamiVnpuCore: true would then reject every whole-card legacy request. A hami-core pod on a hard node is a correctness problem (no soft-split limiter to enforce its limits); a legacy whole-card pod on a hami-core node is not — so only the soft-pod direction is unconditional.

Testing

Unit tests

pkg/device/ascend TestDevices_Fit: 115/115 pass, go build/vet/gofmt clean. Added 4 cases:

whole-card (Memreq == capacity) hami-core on hard node → ModeNotFit
memory-less (Memreq == 0) hami-core on hard node → ModeNotFit
whole-card hami-core on hami-core node → fits (not over-blocked)
whole-card legacy on hami-core node → fits (regression guard for the asymmetry above)

Real hardware (8× Ascend 910B4, hard node) — before/after A/B

Same scheduler build, toggling only this change (v2.9.0 and master have this gate byte-identical):

case (all `vnpu-mode: hami-core`)	before	after
partial mem 16384	Pending `ModeNotFit`	Pending `ModeNotFit`
partial mem 32767	Pending `ModeNotFit`	Pending `ModeNotFit`
whole-card mem 32768	bound to hard node (bypass)	Pending `ModeNotFit`
memory-less	bound to hard node (bypass)	Pending `ModeNotFit`
legacy whole-card (no vnpu-mode)	Running	Running (not over-blocked)

Before: FilteringSucceed: 1 nodes fit(node-001) + BindingSucceed for the whole-card hami-core pod.
After: NodeUnfitPod ... reason="1/8 ModeNotFit" for the same pod; the legacy whole-card pod still schedules.

Scope note

The bypass is confirmed on real hardware. Demonstrating the runtime failure of a whole-card soft pod on a hard-only node needs a second node without soft-split assets, which I can't construct on a single node (the device-plugin always ships soft-split assets). See the linked issue.

This PR was written with AI assistance (Claude Code): the analysis is from my own testing on real Ascend 910B4 hardware, and the code and tests were drafted with AI and reviewed/validated by me.

Summary by CodeRabbit

Bug Fixes
- Improved core-mode (hami-core) scheduling by rejecting incompatible nodes earlier when the pod requests core-mode but the node doesn’t support it.
- Tightened compatibility behavior for whole-card and memory-less requests to prevent incorrect placement decisions.
- Preserved existing legacy workload behavior on nodes reserved for hami-core.
Tests
- Expanded core-mode gating tests with additional whole-card and memory-less scenarios, including negative cases and regression/positive coverage for reserved-node compatibility.

…ssing the node mode gate The node soft/hard exclusivity added in Project-HAMi#1812 rejects a mismatched pod with ModeNotFit, but the check is nested inside `request.Memreq > 0 && request.Memreq < totalMemPerCard && request.Nums > 0`. A whole-card memory request (Memreq == card capacity) or a memory-less request (Memreq == 0) never enters that block, so a hami-core (soft-split) pod can be scheduled onto a hard-split node, bypassing the exclusivity. Lift the soft-pod arm out of the memory-range condition: a pod that declares hami-core mode must land on a hami-core node regardless of how much memory it requests. The reverse arm (a legacy pod on a hami-core reserved node) stays inside the memory-range condition on purpose, so plain whole-card jobs are not rejected when a node -- or the global hamiVnpuCore default -- marks the node as hami-core. Verified on real hardware (8x Ascend 910B4): a hami-core pod requesting the whole card (32768 MiB) or no memory field is now held Pending with ModeNotFit on a hard node, matching the sub-capacity behaviour, while matching-mode and whole-card legacy requests keep scheduling. Signed-off-by: wangmin <wangmin@riseunion.io>

coderabbitai · 2026-07-08T00:52:59Z

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: 95766056-57e9-4306-8a9b-657c579ed8ea

📥 Commits

Reviewing files that changed from the base of the PR and between d5dc085 and 6c508a8.

📒 Files selected for processing (2)

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

💤 Files with no reviewable changes (2)

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

📝 Walkthrough

Walkthrough

The Ascend Fit logic now rejects hami-core pods on nodes without hami-core support before the memory-based gate, and the test table adds whole-card and memory-less cases to cover the updated mode checks.

Changes

Ascend hami-core mode gate fix

Layer / File(s)	Summary
Early hami-core node-support guard in Fit `pkg/device/ascend/device.go`	Fit now immediately rejects a hami-core pod when the node does not support hami-core, moved outside the memory-range (`Memreq`/`Nums`) gated block; the legacy vNPU rejection remains under the original gated condition.
Mode gate test coverage `pkg/device/ascend/device_test.go`	New `TestDevices_Fit` cases assert `ModeNotFit` for hami-core pods on legacy nodes with whole-card or zero memory requests, and confirm successful fits for matching-mode whole-card and legacy whole-card scenarios.

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

Poem

A rabbit hopped through checks so neat,
A full-card gap got tucked away from heat.
Soft and hard now meet their proper gate,
And tests agree the fit is straight. 🐇

🚥 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 main fix: preventing hami-core requests from bypassing the node mode gate.
Linked Issues check	✅ Passed	The code moves the hami-core rejection outside the memory-range gate and adds tests for the whole-card and memory-less cases described in `#2028`.
Out of Scope Changes check	✅ Passed	The changes stay focused on the Ascend mode-gating bug and its test coverage, with no明显 unrelated code paths introduced.
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

_{Comment @coderabbitai help to get the list of available commands.}

gemini-code-assist

Code Review

This pull request ensures that hami-core pods are correctly restricted to hami-core nodes regardless of their memory requests (including whole-card and memory-less requests) by moving the node compatibility check outside of the memory-range condition. Corresponding unit tests were added to verify these scenarios. Feedback was provided regarding a potential nil pointer dereference when logging nodeInfo.Node.Name and pod.Name in the legacy vNPU filtering block, which could lead to runtime panics.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

codecov · 2026-07-08T00:58:16Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

Flag	Coverage Δ
unittests	`59.55% <100.00%> (+0.03%)`	⬆️

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

Files with missing lines	Coverage Δ
pkg/device/ascend/device.go	`83.68% <100.00%> (+0.46%)`	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

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

Use klog.KObj(pod) instead of dereferencing nodeInfo.Node.Name and pod.Name directly, matching the hami-core arm above and avoiding a potential nil dereference (both are guarded as possibly nil earlier in Fit). Signed-off-by: wangmin <wangmin@riseunion.io>

DSFans2014

/lgtm

ouyangluwei163

Please remove the AI-generated code comments and follow the new specification #1998

Signed-off-by: wangmin <wangmin@riseunion.io>

Wangmin362 · 2026-07-08T09:44:06Z

Please remove the AI-generated code comments and follow the new specification #1998

done,

Please remove the AI-generated code comments and follow the new specification #1998

done, I have removed the redundant comments.

ouyangluwei163

/lgtm

archlitchi

/lgtm

hami-robot · 2026-07-09T08:41:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: archlitchi, ouyangluwei163, Wangmin362

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

hami-robot Bot added the dco-signoff: yes label Jul 8, 2026

hami-robot Bot requested review from archlitchi and wawa0210 July 8, 2026 00:52

github-actions Bot added the kind/bug Something isn't working label Jul 8, 2026

hami-robot Bot added the size/L label Jul 8, 2026

gemini-code-assist Bot reviewed Jul 8, 2026

View reviewed changes

Comment thread pkg/device/ascend/device.go

Wangmin362 temporarily deployed to nvidia July 8, 2026 01:01 — with GitHub Actions Inactive

Wangmin362 temporarily deployed to nvidia July 8, 2026 01:22 — with GitHub Actions Inactive

Wangmin362 mentioned this pull request Jul 8, 2026

[Discussion][Ascend] hami-core (soft split): scheduler still aligns vNPU memory to templates when no -core is requested — require -core, or thread the pod? (roadmap #1889) #2030

Closed

DSFans2014 reviewed Jul 8, 2026

View reviewed changes

hami-robot Bot assigned DSFans2014 Jul 8, 2026

hami-robot Bot added the lgtm label Jul 8, 2026

ouyangluwei163 reviewed Jul 8, 2026

View reviewed changes

archlitchi reviewed Jul 8, 2026

View reviewed changes

Comment thread pkg/device/ascend/device.go Outdated

archlitchi reviewed Jul 8, 2026

View reviewed changes

Comment thread pkg/device/ascend/device.go Outdated

address review: drop verbose comments and internal markers

cdbaaae

Signed-off-by: wangmin <wangmin@riseunion.io>

hami-robot Bot added size/M and removed lgtm size/L labels Jul 8, 2026

Wangmin362 temporarily deployed to nvidia July 8, 2026 09:45 — with GitHub Actions Inactive

Wangmin362 force-pushed the fix/ascend-hamicore-wholecard-mode-gate branch from 6c508a8 to cdbaaae Compare July 8, 2026 09:47

Wangmin362 temporarily deployed to nvidia July 8, 2026 09:56 — with GitHub Actions Inactive

ouyangluwei163 approved these changes Jul 9, 2026

View reviewed changes

hami-robot Bot assigned ouyangluwei163 Jul 9, 2026

hami-robot Bot added the lgtm label Jul 9, 2026

archlitchi approved these changes Jul 9, 2026

View reviewed changes

hami-robot Bot assigned archlitchi Jul 9, 2026

hami-robot Bot added the approved label Jul 9, 2026

hami-robot Bot merged commit 3412885 into Project-HAMi:master Jul 9, 2026
14 checks passed

coderabbitai Bot mentioned this pull request Jul 9, 2026

Feat: Allow mode-agnostic Ascend pods to schedule across both vNPU-template and HAMi-core nodes #2035

Merged

This was referenced Jul 17, 2026

[BUG] Scheduler infinite loop in PlatternMIG on safecast.Convert failure #2084

Closed

[Bug] fitInDevices leaks one device type's devices into another type's allocation for multi-type containers #2104

Closed

This was referenced Aug 11, 2026

## Title MIG-adopt nodes: scheduler health check and extender don't support nvidia.com/mig-* resources #2564

Closed

test(pkg/device): add nil checks and unit tests for unparseable pod specs #2676

Closed

This was referenced Aug 19, 2026

Ascend 910B4-1 hami-core 软切模式下单卡最多 4 个 Pod，第 5 个漂移 #2727

Closed

test: establish a staged E2E coverage baseline for runtime-critical paths #2741

Closed

Navigation Menu

Uh oh!

fix(ascend): stop whole-card/memory-less hami-core requests from bypassing the node mode gate - #2029

fix(ascend): stop whole-card/memory-less hami-core requests from bypassing the node mode gate#2029
hami-robot[bot] merged 3 commits into
Project-HAMi:masterfrom
Wangmin362:fix/ascend-hamicore-wholecard-mode-gate

Wangmin362 commented Jul 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

Walkthrough

Changes

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

codecov Bot commented Jul 8, 2026 •

edited

Loading

Uh oh!

DSFans2014 left a comment

Uh oh!

ouyangluwei163 left a comment

Uh oh!

Uh oh!

Uh oh!

Wangmin362 commented Jul 8, 2026

Uh oh!

ouyangluwei163 left a comment

Uh oh!

archlitchi left a comment

Uh oh!

hami-robot Bot commented Jul 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

Wangmin362 commented Jul 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does

Why only the soft-pod arm (not a symmetric change)

Testing

Unit tests

Real hardware (8× Ascend 910B4, hard node) — before/after A/B

Scope note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

codecov Bot commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DSFans2014 left a comment

Choose a reason for hiding this comment

Uh oh!

ouyangluwei163 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Wangmin362 commented Jul 8, 2026

Uh oh!

ouyangluwei163 left a comment

Choose a reason for hiding this comment

Uh oh!

archlitchi left a comment

Choose a reason for hiding this comment

Uh oh!

hami-robot Bot commented Jul 9, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Wangmin362 commented Jul 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 8, 2026 •

edited

Loading

codecov Bot commented Jul 8, 2026 •

edited

Loading