add amd support - #705
Conversation
Signed-off-by: Jifei Wang <jifei.wang@dynamia.ai>
hami-robot
Bot
added
kind/documentation
✅ Deploy Preview for project-hami ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe documentation adds AMD GPU sharing guides and allocation examples. It updates English and Chinese navigation, supported-device tables, and contributor roadmaps to describe AMD GPU support. ChangesAMD GPU documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/userguide/amd-device/enable-amd-gpu-sharing.md (1)
172-177: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the failing Prettier check before merge.
The pipeline reports a Prettier formatting failure, but it does not identify the file or token. Run Prettier across all four changed Markdown files and rerun the check.
npx prettier --write \ docs/userguide/amd-device/enable-amd-gpu-sharing.md \ i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.md \ docs/userguide/amd-device/examples/allocate-core-and-memory.md \ i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/examples/allocate-core-and-memory.md npx prettier --check \ docs/userguide/amd-device/enable-amd-gpu-sharing.md \ i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.md \ docs/userguide/amd-device/examples/allocate-core-and-memory.md \ i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/examples/allocate-core-and-memory.md🤖 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 `@docs/userguide/amd-device/enable-amd-gpu-sharing.md` around lines 172 - 177, Run Prettier with write mode on all four changed Markdown documents, then rerun the same targeted Prettier check and retain the formatter’s changes so every file passes.Source: Pipeline failures
🧹 Nitpick comments (1)
docs/userguide/amd-device/enable-amd-gpu-sharing.md (1)
120-128: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the workload image used by the examples.
rocm/pytorch:latestdoes not identify the tested ROCm/PyTorch build behind the displayed output. Use a concrete tag or digest, then apply it consistently to the Chinese guide and both allocation examples.Proposed change
- image: rocm/pytorch:latest + image: rocm/pytorch:<tested-tag-or-digest>🤖 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 `@docs/userguide/amd-device/enable-amd-gpu-sharing.md` around lines 120 - 128, Update the workload image reference in the example manifests, including the visible pytorch container configuration, from rocm/pytorch:latest to one concrete tested tag or digest. Apply the same pinned reference consistently across the Chinese guide and both allocation examples, preserving the existing commands and behavior.
🤖 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 `@docs/userguide/amd-device/enable-amd-gpu-sharing.md`:
- Around line 76-81: Update the Helm install command in
docs/userguide/amd-device/enable-amd-gpu-sharing.md lines 76-81 and the
corresponding command in
i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.md
lines 77-82 to include --dependency-update.
---
Outside diff comments:
In `@docs/userguide/amd-device/enable-amd-gpu-sharing.md`:
- Around line 172-177: Run Prettier with write mode on all four changed Markdown
documents, then rerun the same targeted Prettier check and retain the
formatter’s changes so every file passes.
---
Nitpick comments:
In `@docs/userguide/amd-device/enable-amd-gpu-sharing.md`:
- Around line 120-128: Update the workload image reference in the example
manifests, including the visible pytorch container configuration, from
rocm/pytorch:latest to one concrete tested tag or digest. Apply the same pinned
reference consistently across the Chinese guide and both allocation examples,
preserving the existing commands and behavior.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 480b35ab-e243-43a2-a898-c391c49064a7
📒 Files selected for processing (10)
docs/contributor/roadmap.mddocs/userguide/amd-device/enable-amd-gpu-sharing.mddocs/userguide/amd-device/examples/allocate-core-and-memory.mddocs/userguide/device-supported.mdi18n/zh/docusaurus-plugin-content-docs/current.jsoni18n/zh/docusaurus-plugin-content-docs/current/contributor/roadmap.mdi18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.mdi18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/examples/allocate-core-and-memory.mdi18n/zh/docusaurus-plugin-content-docs/current/userguide/device-supported.mdsidebars.js
Signed-off-by: Jifei Wang <jifei.wang@dynamia.ai> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Jifei Wang <jifei.wang@dynamia.ai> Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/userguide/amd-device/enable-amd-gpu-sharing.md (1)
42-75: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict the privileged DaemonSet to AMD nodes.
Chart
0.0.1setsnode_selector_enabled: false, so the command does not render the chart’snodeSelector. The container isprivileged: true, which runs every DaemonSet daemon on all eligible nodes. Enable node selection in the install command or override the bundlednode_selectorvalue, and document the AMD-node label or cluster-specific selector.Suggested command change
--namespace kube-system \ - --create-namespace + --create-namespace \ + --set node_selector_enabled=true🤖 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 `@docs/userguide/amd-device/enable-amd-gpu-sharing.md` around lines 42 - 75, Update the “Deploy amd-device-plugin” installation instructions to enable the chart’s node selection and constrain the privileged DaemonSet to AMD-capable nodes. Include the required AMD node label or an explicitly documented cluster-specific selector, and ensure the command overrides node_selector_enabled and node_selector rather than relying on the chart 0.0.1 defaults.i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.md (1)
43-75: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict the privileged DaemonSet to AMD nodes.
amd-device-pluginchart0.0.1defaultsnode_selector_enabled: falsewhile the container runssecurityContext.privileged: true. After the suggestedhelm upgrade --install, add a node selector that limits pods to AMD nodes, such as enabling and settingnode_selector_enabled: trueplus a cluster-specificnode_selectorlabel, or labeling AMD nodes and documenting that required label.🤖 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 `@i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.md` around lines 43 - 75, Update the amd-device-plugin deployment guidance to restrict its privileged DaemonSet to AMD nodes. Document enabling node_selector_enabled and configuring a cluster-specific node_selector, or require and document an equivalent AMD-node label, alongside the Helm installation instructions.
🤖 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 `@docs/userguide/amd-device/enable-amd-gpu-sharing.md`:
- Around line 76-83: Update the Helm installation commands in
docs/userguide/amd-device/enable-amd-gpu-sharing.md lines 76-83 and
i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.md
lines 77-84 to enable node selectors and restrict the DaemonSet to AMD GPU nodes
using the documented AMD PCI label and amd64 architecture selector.
---
Outside diff comments:
In `@docs/userguide/amd-device/enable-amd-gpu-sharing.md`:
- Around line 42-75: Update the “Deploy amd-device-plugin” installation
instructions to enable the chart’s node selection and constrain the privileged
DaemonSet to AMD-capable nodes. Include the required AMD node label or an
explicitly documented cluster-specific selector, and ensure the command
overrides node_selector_enabled and node_selector rather than relying on the
chart 0.0.1 defaults.
In
`@i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.md`:
- Around line 43-75: Update the amd-device-plugin deployment guidance to
restrict its privileged DaemonSet to AMD nodes. Document enabling
node_selector_enabled and configuring a cluster-specific node_selector, or
require and document an equivalent AMD-node label, alongside the Helm
installation instructions.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 859272e8-b6e6-4075-b138-7f7472a4b0d0
📒 Files selected for processing (3)
docs/userguide/amd-device/enable-amd-gpu-sharing.mdi18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.mdsidebars.js
🚧 Files skipped from review as they are similar to previous changes (1)
- sidebars.js
| Deploy [amd-device-plugin](https://github.com/Project-HAMi/amd-device-plugin) to all AMD GPU nodes. Chart `0.0.1` defaults to image `ghcr.io/project-hami/amd-device-plugin:0.0.1` and installs the bundled `libamvgpu.so` hook onto the node through a `postStart` lifecycle hook: | ||
|
|
||
| ```bash | ||
| helm upgrade --install amd-gpu \ | ||
| https://github.com/Project-HAMi/amd-device-plugin/releases/download/amd-gpu-helm-0.0.1/amd-gpu-0.0.1.tgz \ | ||
| --namespace kube-system \ | ||
| --create-namespace | ||
| ``` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Project-HAMi amd-device-plugin helm values node_selector_enabled values.yaml
💡 Result:
The node_selector_enabled parameter and related node_selector settings are features of the official ROCm k8s-device-plugin Helm chart [1][2], not the Project-HAMi Helm chart [3][4]. Project-HAMi does not use the node_selector_enabled flag in its values.yaml [3][4]. Instead, HAMi manages device plugin deployment and node configuration through its own parameters, such as devicePlugin.nvidiaNodeSelector (which defaults to {"gpu": "on"}) [4] and per-node configurations managed via ConfigMaps [5]. If you are using the ROCm k8s-device-plugin Helm chart (found on Artifact Hub [1]), the node_selector_enabled value in values.yaml functions as follows: 1. Purpose: When set to true, it enables the use of node_selector in the DaemonSet specification [2][6]. 2. Default: It defaults to false [1][2]. 3. Configuration: When node_selector_enabled is true, the chart uses the labels defined under node_selector in values.yaml [1][2]. A common default selector used by this chart is feature.node.kubernetes.io/pci-0300_1002.present: "true" [1][2]. If you are attempting to configure node selection in Project-HAMi, you should refer to the devicePlugin.nvidiaNodeSelector field in the HAMi Helm chart values.yaml or define per-node overrides in the hami-device-plugin ConfigMap [3][4][5].
Citations:
- 1: https://artifacthub.io/packages/helm/amd-gpu-helm/amd-gpu
- 2: https://github.com/ROCm/k8s-device-plugin/tree/master/helm/amd-gpu
- 3: https://github.com/Project-HAMi/HAMi/blob/c005cbd1/charts/hami/values.yaml
- 4: https://github.com/Project-HAMi/HAMi/blob/c005cbd1/charts/hami/README.md
- 5: https://github.com/Project-HAMi/HAMi/blob/c005cbd1/docs/config.md
- 6: https://github.com/ROCm/k8s-device-plugin/commits/4607bf06b700e53803d566e0bf9555f773f0b4f1/
🌐 Web query:
raw.githubusercontent.com/Project-HAMi/amd-device-plugin/amd-gpu-helm-0.0.1/helm/amd-gpu/values.yaml
💡 Result:
The URL provided, raw.githubusercontent.com/Project-HAMi/amd-device-plugin/amd-gpu-helm-0.0.1/helm/amd-gpu/values.yaml, refers to a location that does not correspond to an official or maintained component of the Project-HAMi repository [1]. Project-HAMi is a middleware project designed to facilitate the virtualization and sharing of heterogeneous AI accelerators (such as GPUs, NPUs, and DCUs) across Kubernetes workloads [1]. While the project actively supports multiple hardware vendors, the official device management, scheduling, and configuration for all supported accelerators, including AMD, are centralized within the main Project-HAMi Helm chart (located in the charts/hami directory of the Project-HAMi/HAMi repository) [2][3][1]. Key distinctions to note: - AMD device support: Project-HAMi has been actively developing support for AMD Instinct GPUs, including features for GPU memory limiting and computing unit (CU) partitioning [4][5]. This implementation is integrated directly into the core HAMi project infrastructure [1]. - Independent Plugins: Native AMD device plugins (such as the one found at github.com/ROCm/k8s-device-plugin) are designed for the native Kubernetes scheduler and are not directly compatible with the HAMi scheduler, which requires specific protocols for device annotation and vGPU allocation [6][1]. - Configuration: All configuration for HAMi, including device-specific settings, is managed through the central HAMi Helm chart values.yaml and associated ConfigMaps [7][1]. If you are looking to configure AMD support within HAMi, you should refer to the official documentation and the main values.yaml file found in the Project-HAMi/HAMi repository [7][1].
Citations:
- 1: https://github.com/Project-HAMi/HAMi?tab=readme-ov-file
- 2: https://github.com/Project-HAMi/HAMi/blob/c005cbd1/charts/hami/values.yaml
- 3: https://github.com/Project-HAMi/HAMi/blob/master/charts/hami/README.md
- 4: feat: Add AMD Instinct GPU isolation (GPU memory limiting + Computing unit (CU) partitioning) HAMi#1707
- 5: If I'd like to support AMD, how to implement it myself easily? HAMi#1119
- 6: which amd device plugin is supported in hami HAMi#1601
- 7: https://project-hami.io/docs/userguide/configure
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Fetch and inspect only the chart values/template metadata needed for DaemonSet scheduling,
# without running any repository code.
url="https://raw.githubusercontent.com/Project-HAMi/amd-device-plugin/amd-gpu-helm-0.0.1/helm/amd-gpu/values.yaml"
template_url="https://raw.githubusercontent.com/Project-HAMi/amd-device-plugin/amd-gpu-helm-0.0.1/helm/amd-gpu/templates/daemonset.yaml"
tmpdir="$(mktemp -d)"
trap 'rm -rf "$tmpdir"' EXIT
wget -qO- "$url" 2>/dev/null > "$tmpdir/values.yaml" || curl -sSL "$url" > "$tmpdir/values.yaml"
wget -qO- "$template_url" 2>/dev/null > "$tmpdir/daemonset.yaml" || curl -sSL "$template_url" > "$tmpdir/daemonset.yaml"
python3 - <<'PY' "$tmpdir/values.yaml" "$tmpdir/daemonset.yaml"
import sys
from pathlib import Path
for p in sys.argv[1:]:
print(f"\n--- {Path(p).name}:")
data = Path(p).read_text()
for needle in ["node_selector_enabled", "nodeSelector", "nodeAffinity", "podAffinity", "privileged", "postStart"]:
idx = data.find(needle)
if idx >= 0:
start = max(0, idx-120)
end = min(len(data), idx+220)
print(f"\n[context] {needle}:")
print(data[start:end].replace("\n", "\n"))
data = Path(sys.argv[1]).read_text()
print("\n--- relevant selectors/config in values.yaml ---")
for line in data.splitlines():
if any(s in line for s in ["node_selector_enabled", "nodeSelector", "nodeAffinity", "postStart"]):
print(line)
PYRepository: Project-HAMi/website
Length of output: 1336
Scope the AMD device-plugin DaemonSet to AMD nodes.
values.yaml sets node_selector_enabled: false, so the privileged plugin DaemonSet is not limited to the AMD node_selector labels documented in the chart. Add a Helm selector override, such as --set node_selector_enabled=true --set stringArrayValues=node_selector={feature.node.kubernetes.io/pci-0300_1002.present=true,kubernetes.io/arch=amd64}, to both the English and mirrored Chinese instructions.
📍 Affects 2 files
docs/userguide/amd-device/enable-amd-gpu-sharing.md#L76-L83(this comment)i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.md#L77-L84
🤖 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 `@docs/userguide/amd-device/enable-amd-gpu-sharing.md` around lines 76 - 83,
Update the Helm installation commands in
docs/userguide/amd-device/enable-amd-gpu-sharing.md lines 76-83 and
i18n/zh/docusaurus-plugin-content-docs/current/userguide/amd-device/enable-amd-gpu-sharing.md
lines 77-84 to enable node selectors and restrict the DaemonSet to AMD GPU nodes
using the documented AMD PCI label and amd64 architecture selector.
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: archlitchi, FouoF The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
2 participants
What type of PR is this?
/kind documentation
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Checklist:
npm run lintandnpm run format:checkpassnpm run buildsucceeds for bothenandzhgit commit -s)Summary by CodeRabbit
New Features
Documentation