Skip to content

fix(installer): request amd.com/gpu for AMD GPU hosts (backend#2033) - #855

Merged
LukasWodka merged 2 commits into
developfrom
fix/2033-amd-gpu-request
Aug 26, 2026
Merged

fix(installer): request amd.com/gpu for AMD GPU hosts (backend#2033)#855
LukasWodka merged 2 commits into
developfrom
fix/2033-amd-gpu-request

Conversation

@aptracebloc

@aptraceblocaptracebloc commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Closes tracebloc/backend#2033

Problem

On an AMD GPU host the installer already enabled the chart-managed amd device plugin (which advertises amd.com/gpu) and verify_gpu reported the node's GPU "verified and available" — but GPU_LIMITS/GPU_REQUESTS were wired for nvidia only. So spawned training pods requested no GPU and silently ran on CPU: a "verified" GPU that no training job could ever use.

Fix

Wire the GPU request per vendor in install_client_helm (scripts/lib/install-client-helm.sh):

GPU_VENDORGPU_LIMITS / GPU_REQUESTS
nvidianvidia.com/gpu=1
amdamd.com/gpu=1(new)
else""

Each vendor exposes its card as a different scheduler resource, so the request key must match the detected vendor. This makes the "verified" claim honest for AMD: the resource the node advertises is now the resource the pod requests.

A request this fixed single-node cluster can't actually satisfy stays safe — SINGLE_NODE: "true" tells jobs-manager to downgrade a Pending GPU pod to CPU rather than strand it (client-runtime#92).

Tests (scripts/tests/install-client-helm.bats)

  • AMD host → generated values.yaml requests amd.com/gpu=1 and enables the amd device plugin.
  • NVIDIA host → still requests nvidia.com/gpu=1 (parity guard; nothing else pinned this at the values layer).
  • non-GPU host → empty GPU_LIMITS/GPU_REQUESTS, no device plugin.
$ bats -f 'AMD host|NVIDIA host|non-GPU host' install-client-helm.bats
ok 1 install_client_helm: AMD host -> values request amd.com/gpu + enable the amd device plugin
ok 2 install_client_helm: NVIDIA host -> values request nvidia.com/gpu
ok 3 install_client_helm: non-GPU host -> empty GPU_LIMITS/GPU_REQUESTS, no device plugin

Also regenerated scripts/manifest.sha256 for the edited installer lib (scripts/gen-manifest.sh --check passes), and updated a code comment that claimed the GPU request "remains NVIDIA-only".

🤖 Generated with Claude Code


Note

Medium Risk
Changes GPU scheduling env for AMD installs and adopt reconciles—wrong values would still train on CPU, but the fix is localized to installer Helm values and matches existing NVIDIA/device-plugin behavior.

Overview
Fixes backend#2033: AMD hosts could show a verified GPU and run the amd device plugin, but training pods still got emptyGPU_LIMITS/GPU_REQUESTS because only NVIDIA was wired.

Introduces _gpu_request_value() as the single mapping (nvidia.com/gpu=1, amd.com/gpu=1, or empty) for both fresh values.yaml generation and the adopt/reconcile Helm upgrade, which previously reused stale release values and left AMD edges on CPU training.

Adds bats coverage for AMD, NVIDIA, non-GPU values, and adopt-reconcile forcing GPU env on AMD; updates scripts/manifest.sha256 for the edited lib.

Reviewed by Cursor Bugbot for commit df8ddc6. Bugbot is set up for automated code reviews on this repo. Configure here.

On an AMD host the installer enabled the amd device plugin (which
advertises amd.com/gpu) and verify_gpu reported the node's GPU
"available", but GPU_LIMITS/GPU_REQUESTS were wired only for nvidia.
Spawned training pods therefore requested no GPU and silently ran on
CPU — a "verified" GPU that no job could ever use.
Wire the request per vendor: nvidia.com/gpu=1 for nvidia,
amd.com/gpu=1 for amd, empty otherwise. Each vendor exposes its card
as a distinct scheduler resource, so the request key must match the
detected vendor. A request this fixed single-node cluster can't
satisfy is safe: SINGLE_NODE=true tells jobs-manager to downgrade a
Pending GPU pod to CPU rather than strand it (client-runtime#92).
Tests: assert the generated values request amd.com/gpu on an AMD host
and enable the amd device plugin; pin the nvidia arm as a parity
guard; and pin the non-GPU arm to empty GPU_LIMITS with no plugin.
Regenerated scripts/manifest.sha256 for the edited installer lib.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Comment threadscripts/lib/install-client-helm.sh
… too (backend#2033)
Bugbot: the per-vendor GPU request only ran on the values-write path.
_reconcile_adopted_client upgrades an already-installed client with
--reuse-values and healed only clientId, so an AMD edge installed before
this fix kept its stored empty GPU request on re-run and stayed on CPU —
the same "verified but unused" bug, unfixed on the adopt path.
Extract the vendor->resource mapping into _gpu_request_value (single
source of truth so the two paths can't drift again) and force
env.GPU_REQUESTS/GPU_LIMITS onto the reconcile via --set-string,
overriding the reused values — mirroring the PowerShell twin, which
already --set-strings these keys on adopt (#616).
Test: adopt on an AMD host forces --set-string env.GPU_REQUESTS /
GPU_LIMITS=amd.com/gpu=1 onto the reconcile upgrade.
Regenerated scripts/manifest.sha256.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@aptracebloc

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit df8ddc6. Configure here.

@saadqbalsaadqbal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug statement is the valuable part: verify_gpu said the node's GPU was "verified and available", the chart-managed amd plugin was advertising amd.com/gpu, and the pods requested nothing — so a verified GPU no training job could use. Nothing in that chain is red; it just quietly trains on CPU.

Two things I checked rather than took:

The mapping is one source of truth. A single case returns the vendor's resource key, and line 28 sets env.GPU_REQUESTS and env.GPU_LIMITS from the same _gpu_val — so requests and limits cannot drift apart into a pod that requests one card and is limited to none. Worth more than the AMD row itself.

manifest.sha256 is actually regenerated, not stale. The manifest records 60791b3c…74d0 for scripts/lib/install-client-helm.sh and the shipped file hashes to exactly that. A manifest left behind here would break the installer's own integrity check on the very file that changed, and it would fail at install time rather than in CI.

The NVIDIA parity test is the right third case. Fixing AMD is exactly the change that could have quietly moved the nvidia key, and "nothing else pinned this at the values layer" is the honest reason for adding it — the guard exists because nothing was watching, not because someone suspected a specific break.

SINGLE_NODE: "true" downgrading a Pending GPU pod to CPU keeps this safe on a cluster that can't satisfy the request, which is the right thing to say out loud on a change that starts asking for a resource the node may not have.

Green, no threads. Approving.

@LukasWodka
LukasWodka merged commit 0b1507a into developAug 26, 2026
49 checks passed
@LukasWodka
LukasWodka deleted the fix/2033-amd-gpu-request branch August 26, 2026 13:22
aptracebloc added a commit that referenced this pull request Aug 26, 2026
develop advanced again (AMD-GPU #855) — same manifest-serialization race.
Only conflict was scripts/manifest.sha256; resolved by regenerating from the
merged files (gen-manifest.sh --check passes). The installer scripts
auto-merged: #855's amd.com/gpu request is independent of the training-size
fallback. Re-verified: ps1 Pester 770/0, training bats green,
_TRAINING_DEFAULT still derives to cpu=1,memory=2Gi.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@aptracebloc@saadqbal@LukasWodka