feat(ascend): support 910C vNPU templates vir05_1c_16g and vir10_3c_32g#2005
feat(ascend): support 910C vNPU templates vir05_1c_16g and vir10_3c_32g#2005ouyangluwei163 wants to merge 1 commit into
Conversation
Ascend HDK 26.0.RC1 enables NPU virtualization (vNPU) on the 910C. The 910C was previously registered without any vNPU templates, so it could only be allocated as a whole card and fractional (memory/core) requests could not be scheduled. Add the vir05_1c_16g (16GB/5 aiCore/1 aiCPU) and vir10_3c_32g (32GB/10 aiCore/3 aiCPU) templates to the scheduler device config, and add examples/ascend/job-910C.yaml demonstrating a vNPU request. Signed-off-by: ouyangluwei(riseunion) <ouyangluwei@riseunion.io>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ouyangluwei163 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughWalkthroughAdds vir05_1c_16g and vir10_3c_32g vNPU template definitions (memory, aiCore, aiCPU) to the Ascend910 chip entry in the scheduler's device-configmap.yaml, and adds a new example Pod manifest demonstrating a fractional Ascend910C resource request. ChangesAscend 910C vNPU Template Support
Estimated code review effort: 1 (Trivial) | ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds vNPU templates for Ascend devices in the scheduler configuration and provides an example job configuration for Ascend910C. Feedback points out a critical bug where vNPU requests for Ascend910C are incorrectly rounded up to 2 NPUs during admission mutation, and suggests code modifications to bypass this rounding and adjust the scheduling logic for fractional vNPU requests.
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.
| huawei.com/Ascend910C: 1 # requesting 1 NPU | ||
| # requesting device memory; the value maps to a vNPU template (Ascend HDK 26.0.RC1): | ||
| # 16384 -> vir05_1c_16g (16GB, 5 aiCore, 1 aiCPU) | ||
| # 32768 -> vir10_3c_32g (32GB, 10 aiCore, 3 aiCPU) | ||
| huawei.com/Ascend910C-memory: 16384 |
There was a problem hiding this comment.
There is a critical bug in pkg/device/ascend/device.go's MutateAdmission function that will cause this example job (and any Ascend910C vNPU request) to be mutated to request 2 NPUs instead of 1 NPU.
Why this happens
In MutateAdmission (lines 134-149), the code unconditionally rounds up any Ascend910C request of 1 to 2 because the minimum allocation unit for a whole card is 2 NPUs:
if dev.config.CommonWord == Ascend910CType {
if reqNum == 1 {
// Since the minimum allocation unit is one physical module (2 NPUs), round up the limits and requests to 2.
klog.InfoS("Adjusted Ascend910C device request from 1 to 2 (minimum allocation unit)", "pod", klog.KObj(p))
reqNum = 2
...However, for a vNPU request (where memory or core limits are specified), the user is requesting a fractional slice of a single NPU. Rounding this up to 2 NPUs is incorrect and will result in the scheduler allocating 2 separate NPUs, each with the requested memory, or failing to schedule if resources are tight.
How to fix it
- In
pkg/device/ascend/device.go'sMutateAdmissionfunction, check if it is a vNPU request and skip the rounding up logic:
_, hasMemory := ctr.Resources.Limits[corev1.ResourceName(dev.config.ResourceMemoryName)]
if !hasMemory {
_, hasMemory = ctr.Resources.Requests[corev1.ResourceName(dev.config.ResourceMemoryName)]
}
_, hasCore := ctr.Resources.Limits[corev1.ResourceName(dev.config.ResourceCoreName)]
if !hasCore {
_, hasCore = ctr.Resources.Requests[corev1.ResourceName(dev.config.ResourceCoreName)]
}
isVnpuRequest := hasMemory || hasCore
if dev.config.CommonWord == Ascend910CType && !isVnpuRequest {
if reqNum == 1 {
// Since the minimum allocation unit is one physical module (2 NPUs), round up the limits and requests to 2.
klog.InfoS("Adjusted Ascend910C device request from 1 to 2 (minimum allocation unit)", "pod", klog.KObj(p))
reqNum = 2
ctr.Resources.Limits[corev1.ResourceName(dev.config.ResourceName)] = *resource.NewQuantity(reqNum, resource.DecimalExponent)
if _, exists := ctr.Resources.Requests[corev1.ResourceName(dev.config.ResourceName)]; exists {
ctr.Resources.Requests[corev1.ResourceName(dev.config.ResourceName)] = *resource.NewQuantity(reqNum, resource.DecimalExponent)
}
} else if reqNum%2 != 0 {
// Reject any other odd-numbered request (e.g., 3, 5, 7...)
errMsg := fmt.Sprintf("Ascend910C device request must be 1 or 2*n, got %d", reqNum)
klog.ErrorS(nil, errMsg, "pod", klog.KObj(p))
return false, errors.New(errMsg)
}
}- In
pkg/device/ascend/device.go'sFitfunction, only usecomputeBestCombination910C(which strictly requires full cards with both NPUs available) for whole-card requests. For vNPU requests, use the standardcomputeBestCombinationso that vNPUs can be scheduled on partially-allocated cards:
isVnpuRequest := (k.Memreq > 0 && k.Memreq < totalMemPerCard) || (k.Coresreq > 0 && k.Coresreq < 100)
if k.Type == Ascend910CType && !isVnpuRequest {
// Use topology-aware allocation for Ascend910C: only select full modules (2 NPUs per card).
combination = npu.computeBestCombination910C(nodeInfo, int(originReq), tmpDevs[k.Type])
} else {
combination = npu.computeBestCombination(nodeInfo, int(originReq), tmpDevs[k.Type])
}![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
examples/ascend/job-910C.yaml (2)
12-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winComment "requesting 1 NPU" is misleading given auto-adjustment behavior.
Per
pkg/device/ascend/device.goMutateAdmission, whencommonWord == Ascend910CTypeand the request is 1, HAMi silently rounds it up to 2 (minimum allocation unit is one physical module = 2 NPUs). The example's comment says "requesting 1 NPU" without mentioning this auto-adjustment, which could confuse users about the actual resulting allocation.📝 Suggested comment clarification
- huawei.com/Ascend910C: 1 # requesting 1 NPU + huawei.com/Ascend910C: 1 # requesting 1 NPU (auto-adjusted to 2, the minimum allocation unit)🤖 Prompt for 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. In `@examples/ascend/job-910C.yaml` around lines 12 - 16, Update the example comment in the Ascend 910C job manifest so it reflects the behavior in MutateAdmission for Ascend910CType: a request of 1 is automatically rounded up to 2 NPUs. Clarify the note near the huawei.com/Ascend910C resource to mention the minimum physical module allocation and that the scheduler will adjust the request, so users understand the actual allocated count.
1-16: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueStatic analysis flags root/privilege-escalation defaults on example manifest.
Checkov flags missing
allowPrivilegeEscalation: falseand non-rootsecurityContext(CKV_K8S_20, CKV_K8S_23). Since this is a demo/example manifest, this is low priority, but adding a minimalsecurityContextwould align with security best practices for example docs that users may copy verbatim.🤖 Prompt for 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. In `@examples/ascend/job-910C.yaml` around lines 1 - 16, The example Pod manifest is missing the basic container hardening settings flagged by Checkov. Update the ubuntu-container spec to include a minimal securityContext with allowPrivilegeEscalation set to false and runAsNonRoot enabled (and, if needed, a non-root runAsUser) so the example aligns with safer defaults while keeping the existing Pod and resources fields intact.Source: Linters/SAST tools
🤖 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.
Nitpick comments:
In `@examples/ascend/job-910C.yaml`:
- Around line 12-16: Update the example comment in the Ascend 910C job manifest
so it reflects the behavior in MutateAdmission for Ascend910CType: a request of
1 is automatically rounded up to 2 NPUs. Clarify the note near the
huawei.com/Ascend910C resource to mention the minimum physical module allocation
and that the scheduler will adjust the request, so users understand the actual
allocated count.
- Around line 1-16: The example Pod manifest is missing the basic container
hardening settings flagged by Checkov. Update the ubuntu-container spec to
include a minimal securityContext with allowPrivilegeEscalation set to false and
runAsNonRoot enabled (and, if needed, a non-root runAsUser) so the example
aligns with safer defaults while keeping the existing Pod and resources fields
intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 014964da-43df-4e85-8dfe-f8e1bede193f
📒 Files selected for processing (2)
charts/hami/templates/scheduler/device-configmap.yamlexamples/ascend/job-910C.yaml
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
| command: ["bash", "-c", "sleep 86400"] | ||
| resources: | ||
| limits: | ||
| huawei.com/Ascend910C: 1 # requesting 1 NPU |
There was a problem hiding this comment.
@ouyangluwei163 Before lgtm, one question: issue #2004 says "a single Ascend 910C NPU can be sliced into smaller compute instances", but MutateAdmission unconditionally bumps Ascend910C: 1 to 2. For a vNPU request like this one, GenerateResourceRequests reads the bumped limit (2), Fit allocates 2 physical NPUs each with vir05_1c_16g, and the container gets 2 vNPU instances consuming 32768 MB total, not 1. Should the bump be skipped when memory is specified, so a single-NPU vNPU slice works as described in the issue?
2 participants
Ascend HDK 26.0.RC1 enables NPU virtualization (vNPU) on the 910C. The 910C was previously registered without any vNPU templates, so it could only be allocated as a whole card and fractional (memory/core) requests could not be scheduled.
Add the vir05_1c_16g (16GB/5 aiCore/1 aiCPU) and vir10_3c_32g (32GB/10 aiCore/3 aiCPU) templates to the scheduler device config, and add examples/ascend/job-910C.yaml demonstrating a vNPU request.
What type of PR is this?
/kind feature
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #2004
Special notes for your reviewer:
Does this PR introduce a user-facing change?:
Summary by CodeRabbit