Skip to content

Add AMD MI300 GPU lab - #5970

Open
Runzhen (runzhen) wants to merge 11 commits into
masterfrom
amd
Open

Runzhen (runzhen) wants to merge 11 commits into
masterfrom
amd

Conversation

@runzhen

Copy link
Copy Markdown

Add AMD MI300 GPU lab for users who wants to use AMD GPU in AKS

Copilot AI 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.

🟡 Changes recommended

Critical setup and DRA manifest issues remain unresolved, preventing reliable installation and workload scheduling.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds an AMD MI300X GPU lab for AKS, covering GPU Operator setup, DRA configuration, and shared GPU workloads.

Changes:

  • Provisions an MI300X node pool and AMD GPU Operator.
  • Documents DRA enablement and shared ResourceClaim usage.
  • Adds manifests for two GPU-consuming Pods.
File summaries
File Review findings
examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md Critical (1 vote): ACR scope maps and tokens require Premium, but the procedure uses Standard.
Moderate (2 votes): Pass the declared subscription to get-credentials; ensure Kubernetes 1.34+; use a valid subscription GUID placeholder; add Helm to prerequisites.
Nits (1–3 votes): Pin the verification image, correct the Secret/ConfigMap heading, fix typos and grammar, and identify the device-plugin Pod clearly.
examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md Critical (1–3 votes): Correct the node-label selector and ensure the applied manifest creates shared-gpu-claim.
Nits (1–3 votes): Use non-following log commands, pin the DRA driver and ROCm images, and fix the typo.
examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml Critical (2 votes): Define or create the referenced shared-gpu-claim.
Nits (1–3 votes): Pin both image tags and correct comments that refer to rocm-smi instead of amd-smi.
Review details

Suppressed comments (7)

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:12

  • Correct the typo in this heading so the prerequisite section reads naturally.
**Before your start**

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:172

  • The sentence is not grammatical and describes the resource incorrectly. Use clear wording that identifies the device-plugin Pod.
Verify the device-plugin is Pod running. 

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:18

  • This example subscription value is not a valid Azure subscription ID shape, so copying the prerequisite table literally makes the first Azure CLI command fail. Use a 36-character GUID-shaped placeholder.
| `SUBSCRIPTION` | Azure subscription ID | `000000-0000-0000-0000-00000000` |

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:8

  • The procedure later invokes helm repo and helm install, but Helm is not listed in the prerequisites. A fresh environment following this lab will stop at the installation step. Add Helm to the prerequisite list.
- kubectl command installed.

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:117

  • The second Pod also runs sleep infinity, so kubectl logs -f pod2 never returns after printing the sample output. Use a non-following log command unless a separate terminal is explicitly required.
kubectl logs -f pod2

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:13

  • The comment says this command runs rocm-smi, but the command actually runs amd-smi. The mismatch makes the example harder to troubleshoot; update the comment to name the command that is executed.
    # Simplified args: run rocm-smi, then sleep

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:36

  • The comment says this command runs rocm-smi, but the command actually runs amd-smi. Update the comment so it matches the command shown and does not mislead troubleshooting.
    # Simplified args: run rocm-smi, then sleep
  • Files reviewed: 3/3 changed files
  • Comments generated: 15
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md Outdated
Comment thread examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md Outdated
Comment thread examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md
Comment thread examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml
Comment thread examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md
Comment thread examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md
Comment thread examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md
Comment thread examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md Outdated
Comment thread examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml Outdated
Comment thread examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml Outdated
Copilot AI review requested due to automatic review settings September 16, 2026 22:46
Runzhen (runzhen) and others added 2 commits September 16, 2026 15:48
Fix command syntax for checking logs of pod1.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI 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.

🔵 Needs a closer look

Unresolved moderate issues affect prerequisites, DRA compatibility, reproducibility, and ResourceClaim setup.

Review details

Suppressed comments (19)

Previously missed (1) — in code that hasn't changed since the last review.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:169

  • The inline selector contains an unmatched closing brace inside the code span, which makes the documented label expression malformed. Remove the extra }.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:39

  • DRA resources are available on AKS starting with Kubernetes 1.34, but this command leaves the version to the cluster default. On an older default, the later deviceclass/resourceslice checks and ResourceClaim API cannot work; pin a supported 1.34+ version or add an explicit version check.
--location "$LOCATION" \

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:125

  • This heading calls the object a ConfigMap, but the commands create a Kubernetes Secret. Rename the heading so readers know which object they are about to create.
# Add the secret configmap

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:189

  • The example uses the mutable :latest tag while the expected output below depends on specific ROCm and AMDSMI versions. Pin a tested rocm-terminal tag or digest so rerunning the lab does not silently change the workload or its output.
    image: rocm/rocm-terminal:latest

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:8

  • The prerequisites omit Helm even though the procedure invokes helm repo and helm install below. A user who follows the listed prerequisites can reach the operator step without the required command; list a supported Helm 3 installation.
- kubectl command installed.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:58

  • This command omits the SUBSCRIPTION used to create the cluster. If the Azure CLI's current subscription differs from $SUBSCRIPTION, get-credentials searches the wrong subscription and the lab stops here; pass the same subscription explicitly.
 az aks get-credentials --admin \
    --resource-group "${RESOURCE_GROUP}" \
    --name "${AKS_NAME}" \
    --overwrite-existing

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:122

  • az acr token credential generate requires selecting a password slot (--password1 or --password2). Without that flag this command does not generate the value queried into ACR_PASSWORD, so the registry Secret is created with an unusable password.
ACR_PASSWORD=$(az acr token credential generate \
--subscription "$SUBSCRIPTION" \
--registry "$ACR_NAME" \
--name amd-gpu-operator \
--query 'passwords[0].value' -o tsv)

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:12

  • Use “Before you start”; “Before your start” is grammatically incorrect.
**Before your start**

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:172

  • The sentence is ungrammatical and uses “Pod” as a proper noun. State that the device-plugin pod is running.
Verify the device-plugin is Pod running. 

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:31

  • The MI300X setup above targets the VF-passthrough label feature.node.kubernetes.io/amd-vgpu: "true", but this edit changes the DeviceConfig selector to amd-gpu: "true". The DRA driver therefore won't select the GPU node and won't publish its ResourceSlice; keep the selector aligned with the node label used during operator installation.
    feature.node.kubernetes.io/amd-gpu: "true"

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:93

  • The linked manifest contains only the two Pods; it never creates the shared-gpu-claim defined in the preceding example, nor is there an apply step for that claim. Following this instruction from a fresh namespace leaves the referenced claim missing, so the Pods cannot be allocated. Add the ResourceClaim to the manifest or explicitly apply it first.
You can simply apply the yaml file [manifests/1-dra-multiple-pods-share.yaml](manifests/1-dra-multiple-pods-share.yaml).

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:106

  • Because the container runs sleep infinity, kubectl logs -f pod1 never returns to the next command in this sequential example. Use a non-following log command or explicitly tell the reader to stop the stream before checking Pod 2.
kubectl logs pod1

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:27

  • Using the mutable latest DRA driver image makes this lab non-reproducible and can pull an image incompatible with the pinned GPU Operator chart version v1.5.1. Pin a driver image tag or digest that is tested with this chart.
    image: rocm/k8s-gpu-dra-driver:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:82

  • This Pod example also uses a mutable :latest ROCm image, so the documented shared-GPU behavior can change independently of the lab instructions. Pin it to the tested ROCm image tag or digest.
    image: docker.io/rocm/pytorch:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:42

  • Use “these two objects” rather than “this two objects.”
DRA Driver will automatically create `deviceclass` and `resourceslice` in your AKS. Verify this two objects exist

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:11

  • This workload uses the mutable :latest tag even though its command depends on a particular ROCm image layout and amd-smi path. Pin the image to a tested tag or digest so the manifest remains reproducible.
    image: docker.io/rocm/pytorch:latest # Or a specific ROCm version, e.g., rocm/pytorch:rocm-6.4.2-ubuntu-22.04

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:34

  • The second Pod repeats the mutable :latest tag, so it can run a different image from the first Pod after a registry update and invalidate the sharing comparison. Pin the same tested ROCm tag or digest used by pod1.
    image: docker.io/rocm/pytorch:latest # Or a specific ROCm version

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:13

  • The command invokes amd-smi, not rocm-smi, so this comment describes a different tool than the manifest actually runs. Update the comment to match the command.
    # Simplified args: run rocm-smi, then sleep

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:21

  • This manifest references shared-gpu-claim from both Pods, but it never creates that ResourceClaim. Applying the linked file therefore leaves both Pods unable to allocate a device; include the claim object in this manifest (before the Pods) or make the instructions apply the claim first.
  resourceClaims:
  - name: gpu
    # 2. Reference the *explicitly created* ResourceClaim, not a template
    resourceClaimName: shared-gpu-claim
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 16, 2026 22:51

Copilot AI 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.

🔵 Needs a closer look

Unresolved correctness, reproducibility, and documentation issues remain.

Review details

Suppressed comments (19)

Previously missed (7) — in code that hasn't changed since the last review.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:8

  • The procedure invokes helm repo and helm install below, but Helm is not listed in the prerequisites or installed anywhere in the guide. A user following this lab with only the listed tools reaches these commands with no helm executable; add Helm to the prerequisites or provide its installation step.
    examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:31
  • This edit example changes the selector to amd-gpu: "true", which conflicts with the installation command above that targets amd-vgpu: "true" for the MI300X VF-passthrough node. If this full example is copied, the DRA driver may stop matching the GPU node; keep the selector consistent with the installation configuration.
    examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:12
  • The heading says “Before your start”; the possessive form is incorrect here. Use “Before you start”.
    examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:169
  • The selector is shown as an incomplete JSON fragment with an unmatched closing brace, which makes the required node label difficult to copy or verify. Show the label in normal Kubernetes selector syntax or provide the complete JSON object.
    examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:172
  • The sentence is ungrammatical and makes the verification step harder to follow. Reword it so the resource being checked is the device-plugin Pod.
    examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:42
  • This sentence has incorrect grammar and lowercases the Kubernetes resource types. Use the standard DeviceClass and ResourceSlice names and refer to both objects with “these” or “both”.
    examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:13
  • The comment says this command runs rocm-smi, but the command actually runs amd-smi. This mismatch can send users looking for the wrong executable when troubleshooting the workload.

This issue also appears on line 36 of the same file.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:125

  • This heading calls the object a ConfigMap, but the command creates a Kubernetes Secret containing registry credentials. Rename the heading so it matches the resource users must create.
# Add the secret configmap

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:39

  • This command leaves the Kubernetes version at the Azure/region default, but the later resource.k8s.io/v1 DRA steps require Kubernetes 1.34 or later. On an older default, deviceclasses and resourceslices are unavailable and the lab cannot proceed; pin a supported version or validate it before continuing.
az aks create \
--subscription "$SUBSCRIPTION" \
--resource-group "$RESOURCE_GROUP" \
--name "$AKS_NAME" \
--location "$LOCATION" \

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:3

  • The sentence uses “and install” after the singular subject “This document”. Change it to “and installs” so the prerequisite description is grammatical.
`Standard_ND96isr_MI300X_v5` node, and install AMD GPU Operator to build and deploy the AMD driver.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:189

  • The first workload uses the mutable :latest image even though the output documents a specific ROCm version. Pin the image to the version used to produce that output, or to an immutable digest, so the verification remains reproducible.
    image: rocm/rocm-terminal:latest

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:58

  • All other Azure CLI calls in this walkthrough select "$SUBSCRIPTION", but get-credentials does not. If the user's default Azure CLI subscription differs, this command can fail to find the cluster or load credentials for a same-named cluster in another subscription. Pass --subscription "${SUBSCRIPTION}" here as well.
 az aks get-credentials --admin \
    --resource-group "${RESOURCE_GROUP}" \
    --name "${AKS_NAME}" \
    --overwrite-existing

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:93

  • The linked manifest contains only the two Pods, while both Pods reference shared-gpu-claim; the ResourceClaim shown above is never created by this instruction. Applying the file as written leaves the Pods pending because the claim does not exist. Create and apply the claim first, or include it in the linked manifest.
Verify the two pods are running. 

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:27

  • The example sets the DRA driver image to the mutable :latest tag. Editing this resource later can silently install a different driver than the one validated with this operator and Kubernetes version; pin the image to a tested release or digest.
    image: rocm/k8s-gpu-dra-driver:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:82

  • This workload image also uses latest, so a future ROCm image change can invalidate /opt/venv/bin/amd-smi or produce output different from the documented result. Pin it to the tested ROCm image version instead of relying on a mutable tag.
      claims:

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:36

  • This second Pod has the same documentation mismatch: its command runs amd-smi, while the preceding comment says rocm-smi. Update the comment so it describes the command users actually run.
    # Simplified args: run rocm-smi, then sleep

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:11

  • Using docker.io/rocm/pytorch:latest makes both Pods depend on a mutable image, which can change the amd-smi path or behavior and make the shared-GPU verification non-reproducible. Pin this image to the tested ROCm version shown in the comment.
    image: docker.io/rocm/pytorch:latest # Or a specific ROCm version, e.g., rocm/pytorch:rocm-6.4.2-ubuntu-22.04

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:34

  • The second Pod repeats the mutable latest image tag, so it can run a different image from the first Pod after a registry update and invalidate the comparison. Use the same pinned ROCm image as pod1.
    image: docker.io/rocm/pytorch:latest # Or a specific ROCm version

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:21

  • shared-gpu-claim is referenced here, but this manifest contains only the two Pods; the ResourceClaim shown in the guide is never applied. Because the instructions tell users to apply this file directly, both Pods remain unschedulable with a missing claim. Add the ResourceClaim as a document in this file before the Pods, or add an explicit apply step for it.
    resourceClaimName: shared-gpu-claim
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 16, 2026 22:55

Copilot AI 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.

🔵 Needs a closer look

Several moderate issues affect setup reliability, reproducibility, and workload validation.

Review details

Suppressed comments (18)

Previously missed (3) — in code that hasn't changed since the last review.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:8

  • The prerequisites list kubectl but omit Helm, even though the next setup section runs helm repo and helm install. A user meeting the documented prerequisites cannot complete this lab without discovering the missing dependency; list Helm as a prerequisite.
    examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:12
  • Correct the heading grammar from “Before your start” to “Before you start.”
    examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:13
  • This comment says the container runs rocm-smi, but the command immediately below actually invokes /opt/venv/bin/amd-smi. Update the comment so the documented validation matches what the manifest runs.

This issue also appears on line 36 of the same file.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:40

  • The cluster creation command does not require Kubernetes 1.34 or later, but the DRA resources used below are unavailable on older AKS versions. A user whose region defaults to an older supported version will reach this lab with no DeviceClass/ResourceSlice API and the claims cannot work. Pin a supported minimum here or add an explicit version check before enabling DRA.
az aks create \
--subscription "$SUBSCRIPTION" \
--resource-group "$RESOURCE_GROUP" \
--name "$AKS_NAME" \
--location "$LOCATION" \
--generate-ssh-keys

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:58

  • All preceding Azure CLI commands select $SUBSCRIPTION, but this get-credentials call does not. If the user's active CLI subscription differs from the value used to create the cluster, this step can target the wrong subscription or fail to find the cluster. Pass --subscription "${SUBSCRIPTION}" here as well.
 az aks get-credentials --admin \
    --resource-group "${RESOURCE_GROUP}" \
    --name "${AKS_NAME}" \
    --overwrite-existing

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:189

  • The validation output below reports ROCm 6.4.0, but :latest can resolve to a different ROCm/AMDSMI stack and can change the command behavior over time. Pin this image to the tested ROCm tag so the workload and its expected output remain reproducible.
    image: rocm/rocm-terminal:latest

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:125

  • The command below creates a Kubernetes Secret, not a ConfigMap. Rename this heading so readers do not look for or document the wrong resource type.
# Add the secret configmap

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:172

  • This sentence is grammatically incorrect. Use “Verify that the device-plugin Pod is running.”
Verify the device-plugin is Pod running. 

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:122

  • az acr token credential generate requires selecting which token password to regenerate. Without --password1 or --password2, this command fails before ACR_PASSWORD is assigned, so the subsequent secret creation cannot work.
ACR_PASSWORD=$(az acr token credential generate \
--subscription "$SUBSCRIPTION" \
--registry "$ACR_NAME" \
--name amd-gpu-operator \
--query 'passwords[0].value' -o tsv)

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:27

  • The DRA driver image is left on the mutable :latest tag while the operator itself is pinned to v1.5.1. A future image update can introduce an incompatible driver without any change to this walkthrough, making the edit fail or change allocation behavior. Pin this image to the driver version tested with the operator release.
    image: rocm/k8s-gpu-dra-driver:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:80

  • The workload example in this walkthrough also uses the mutable rocm/pytorch:latest image, so it can drift from the image used by the manifest and make the documented validation inconsistent. Pin it to the same tested ROCm tag used by the manifest.
    image: docker.io/rocm/pytorch:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:91

  • The linked manifest only creates the two Pods; it never creates shared-gpu-claim. Applying it as instructed therefore leaves both Pods Pending because their referenced ResourceClaim does not exist. Include the ResourceClaim in the manifest or add an explicit apply step for the claim before applying the Pods.
You can simply apply the yaml file [manifests/1-dra-multiple-pods-share.yaml](manifests/1-dra-multiple-pods-share.yaml).

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:21

  • This manifest references shared-gpu-claim, but it never creates that ResourceClaim. Because the walkthrough tells users to apply this file directly, both Pods remain Pending with an unresolved claim unless they manually apply the earlier snippet. Include the claim as a YAML document before the Pods or make its separate apply step explicit.
    resourceClaimName: shared-gpu-claim

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:11

  • Using rocm/pytorch:latest makes the first verification Pod non-reproducible and can change or remove /opt/venv/bin/amd-smi as the upstream image moves. Pin this Pod to the tested ROCm image tag; the current inline example already identifies rocm-6.4.2-ubuntu-22.04 as a candidate.
    image: docker.io/rocm/pytorch:latest # Or a specific ROCm version, e.g., rocm/pytorch:rocm-6.4.2-ubuntu-22.04

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:34

  • The second verification Pod repeats the mutable rocm/pytorch:latest tag, so it can run a different image from the first Pod after a registry update. Pin it to the same tested ROCm tag to keep the shared-device comparison meaningful.
    image: docker.io/rocm/pytorch:latest # Or a specific ROCm version

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:36

  • The second Pod has the same mismatch: the comment says rocm-smi, while the command invokes /opt/venv/bin/amd-smi. Keep the workload description consistent with the actual command.
    # Simplified args: run rocm-smi, then sleep

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:14

  • Because the shell command separates amd-smi with ; and does not enable set -e, a failed GPU probe is ignored and the Pod still prints “complete” and sleeps. The verification can therefore report a Running Pod even when the GPU is unavailable; make the shell exit on probe failure before sleeping.
    args: ["echo '--- Pod 1 ---'; /opt/venv/bin/amd-smi list; echo 'Pod 1 complete. Sleeping...'; sleep infinity"]

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:37

  • This Pod has the same false-success path: a nonzero exit from amd-smi is ignored because the commands are chained with ;, then the container sleeps forever. Enable set -e or otherwise chain the probe so the Pod fails when GPU access is broken.
    args: ["echo '--- Pod 2 ---'; /opt/venv/bin/amd-smi list; echo 'Pod 2 complete. Sleeping...'; sleep infinity"]
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 17, 2026 01:26

Copilot AI 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.

🔵 Needs a closer look

Multiple moderate issues remain, including unpinned images and documentation correctness problems.

Review details

Suppressed comments (16)

Previously missed (2) — in code that hasn't changed since the last review.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:3

  • Because the subject is singular, “install” should be “installs”; the current sentence is grammatically incorrect in the opening description.

This issue also appears in the following locations of the same file:

  • line 8
  • line 12
  • line 172
    examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:40
  • The sentence says deviceclass and resourceslice, then refers to them as “this two objects,” which makes the verification instruction unclear. Use the Kubernetes resource names and a clear plural construction.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:40

  • This cluster creation command leaves the Kubernetes version to AKS's default, but the next section depends on DRA APIs that are available only from Kubernetes 1.34. A user can therefore follow this guide and then find that deviceclasses and resourceslices do not exist; pin the cluster to 1.34 or later.
--generate-ssh-keys

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:8

  • This procedure invokes Helm below, but Helm is not listed in the prerequisites. Add a Helm 3 prerequisite so the setup is complete for a fresh environment.
- kubectl command installed.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:125

  • The command creates a Kubernetes Secret, not a ConfigMap. Calling it a "secret configmap" is misleading when users are troubleshooting or looking for the generated object.
# Add the secret configmap

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:189

  • The repository's manifest guidance requires pinned image tags rather than :latest (.github/copilot-instructions.md:179). Pin this workload image to the tested ROCm release or an immutable digest; otherwise rerunning the lab can silently change the runtime and invalidate the documented output.
    image: rocm/rocm-terminal:latest

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:18

  • The example subscription ID has the wrong UUID shape (6-4-4-4-8 instead of 8-4-4-4-12), so copying the documented value produces an invalid Azure subscription identifier. Use a valid-shaped placeholder or <subscription-id>.
| `SUBSCRIPTION` | Azure subscription ID | `000000-0000-0000-0000-00000000` |

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:172

  • This instruction is grammatically unclear. It should identify the device-plugin Pod and state that it must be running.
Verify the device-plugin is Pod running. 

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:184

  • This workload Pod has no labels, while the repository manifest guidance requires labels on workloads (.github/copilot-instructions.md:189). Add a stable application label so the example is consistent with repository conventions and can be selected or identified reliably.
metadata:
  name: amd-smi

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:12

  • The heading uses “your start,” which is grammatically incorrect and makes the setup section look unfinished.
**Before your start**

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:22

  • GPU_POOL is user-provided, but the table does not state AKS's naming constraint. The repository validates user node pool names as lowercase alphanumeric and at most 12 characters (examples/kueue-and-ray-on-aks/1-infrastructure/terraform/variables.tf:80-87); document that here to prevent a later az aks nodepool add failure.
| `GPU_POOL` | Name of the AKS GPU node pool | `gpunp` |

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:27

  • Using latest here lets the DRA driver change independently of the pinned GPU Operator chart (v1.5.1). A future image can be incompatible and leave the DRA driver unable to start, so pin this to the tested release or an immutable digest.
    image: rocm/k8s-gpu-dra-driver:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:80

  • The repository's manifest guidance requires pinned image tags rather than :latest (.github/copilot-instructions.md:179). Use the tested ROCm tag here so the documented DRA workload remains reproducible.
    image: docker.io/rocm/pytorch:latest

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:25

  • The repository's manifest guidance requires pinned image tags rather than :latest (.github/copilot-instructions.md:179). The file already names a tested ROCm tag in its comment, so use that tag instead of allowing this workload to change on every pull.
    image: docker.io/rocm/pytorch:latest

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:48

  • This second Pod uses the same unpinned :latest image as pod1, so rerunning the lab can change the runtime independently of the documented result. Pin it to the same tested ROCm tag or an immutable digest.
    command: ["bash", "-c"]

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:27

  • The comment says the command runs rocm-smi, but the command actually invokes amd-smi. Correct the comment so it describes the diagnostic users will see when troubleshooting the Pod.
    args: ["echo '--- Pod 1 ---'; /opt/venv/bin/amd-smi list; echo 'Pod 1 complete. Sleeping...'; sleep infinity"]
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Copilot AI review requested due to automatic review settings September 17, 2026 01:32

Copilot AI 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.

🔵 Needs a closer look

Moderate correctness and reproducibility issues remain unresolved.

Review details

Suppressed comments (16)

Previously missed (4) — in code that hasn't changed since the last review.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:12

  • The heading contains a grammatical error: Before your start should be Before you start.
    examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:172
  • This sentence is grammatically unclear. Refer to the device-plugin Pod directly so the verification step is unambiguous.
    examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:179
  • This fenced block is a shell command that starts with kubectl apply, not YAML. Marking it as YAML gives incorrect syntax guidance when users copy or inspect the lab command.
    examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:40
  • This sentence has a grammar error and uses lowercase API resource names. Use the Kubernetes resource names and explicitly say that the objects are created in the cluster.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:40

  • This command leaves the Kubernetes version to the AKS default, but the DRA steps below require Kubernetes 1.34 or later. In a subscription or region whose default is older, the resource.k8s.io/v1 API and deviceclasses/resourceslices checks will fail; pin a supported version for this lab.
--generate-ssh-keys

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:189

  • The verification workload uses a mutable :latest tag, so the logged AMDSMI and ROCm versions can change independently of the expected output. Pin the image to an immutable, tested version.
    image: rocm/rocm-terminal:latest

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:125

  • This heading calls the Kubernetes Secret a ConfigMap, which can mislead users about the object being created by the following command. Rename it to describe the registry Secret.
# Add the secret configmap

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:169

  • The selector explanation has an extra closing brace after "true", so it no longer matches the JSON selector in the command above. Remove the extra }.
> The `"feature.node.kubernetes.io/amd-vgpu":"true"}` selector is required for Azure's MI300X VF-passthrough node.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:18

  • Azure subscription IDs use the 8-4-4-4-12 GUID format, but this example has 6-4-4-4-8 groups. Copying it into $SUBSCRIPTION makes every Azure CLI command fail validation; use a correctly shaped placeholder or tell readers to replace it.
| `SUBSCRIPTION` | Azure subscription ID | `000000-0000-0000-0000-00000000` |

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:8

  • The prerequisites omit Helm, but the installation steps invoke helm repo, helm install, and helm chart options below. Add Helm to this list so users following the documented prerequisites don't reach the operator install with a missing command.
- kubectl command installed.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:55

  • The cluster is created with $SUBSCRIPTION, but this get-credentials call omits that option and uses the Azure CLI's current default subscription instead. If the defaults differ, the command can fail to find the cluster or retrieve credentials for another cluster; pass $SUBSCRIPTION here as well.
 az aks get-credentials --admin \

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:27

  • The documented DRA driver image is also mutable. A later kubectl edit can switch the driver implementation without any change to this guide, so pin it to the version compatible with the installed GPU Operator.
    image: rocm/k8s-gpu-dra-driver:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:80

  • This workload example uses :latest, making the verification dependent on a moving image rather than the ROCm version shown in the expected output. Pin an immutable, tested image tag.
    image: docker.io/rocm/pytorch:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:95

  • The manifest explicitly creates the claim and both Pods in default, but the verification commands below use the current kubectl namespace. If the user's context is configured for another namespace, kubectl get pod and both kubectl logs commands report that the Pods are missing; add -n default to each command.
kubectl get pod

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:25

  • This sample uses the mutable :latest tag, so applying the manifest later can silently change the ROCm/PyTorch runtime and invalidate the documented behavior. Pin the image to an immutable, tested version.
    image: docker.io/rocm/pytorch:latest

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:47

  • This second workload also uses the mutable :latest tag, so the two pods can run different runtime contents across reruns or pulls. Use the same immutable, tested image tag as the first pod.
    image: docker.io/rocm/pytorch:latest 
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Comment thread examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md Outdated
Copilot AI review requested due to automatic review settings September 17, 2026 21:28

Copilot AI 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.

🔵 Needs a closer look

Moderate compatibility, reproducibility, and setup issues remain unresolved.

Review details

Suppressed comments (14)

Previously missed (1) — in code that hasn't changed since the last review.

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:25

  • Using :latest makes this lab resolve to a mutable image, so the documented amd-smi behavior can change or break without a repository change. Pin this image to the tested ROCm/PyTorch tag and use the same pinned tag for the second Pod.

This issue also appears on line 47 of the same file.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:8

  • Helm is used throughout the installation block below, but it is not listed as a prerequisite. A user following this lab from a clean environment will stop at helm repo add; add Helm 3 to the prerequisites.
- kubectl command installed.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:12

  • Fix the typo in this heading: Before your start should be Before you start.
**Before your start**

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:169

  • This explanatory selector is malformed: it contains an unmatched closing brace and does not show the JSON object passed to Helm on line 165. Correct the inline selector so readers can reproduce the intended node selection.
> The `"feature.node.kubernetes.io/amd-vgpu":"true"}` selector is required for Azure's MI300X VF-passthrough node.

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:189

  • This validation workload also uses the mutable :latest tag. Pin the ROCm terminal image to the tested release so amd-smi and its output remain reproducible.
    image: rocm/rocm-terminal:latest

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:125

  • This heading calls the resource a ConfigMap, but the commands create a Kubernetes Secret (kubectl create secret docker-registry). Rename the heading to avoid sending users looking for a ConfigMap.
# Add the secret configmap

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:58

  • All preceding Azure commands explicitly use $SUBSCRIPTION, but az aks get-credentials relies on the CLI's current default subscription. If that default differs from the value used to create the cluster, this step fails to find the cluster or targets the wrong subscription. Pass --subscription "$SUBSCRIPTION" here as well.
 az aks get-credentials --admin \
    --resource-group "${RESOURCE_GROUP}" \
    --name "${AKS_NAME}" \
    --overwrite-existing

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:40

  • Because this command leaves the Kubernetes version at the AKS default, it can create a cluster earlier than 1.34 where the resource.k8s.io/v1 DRA types used by the next guide (DeviceClass and ResourceSlice) are unavailable. Pin --kubernetes-version to the tested 1.35 minor version (or otherwise enforce 1.34+) before this lab continues.
az aks create \
--subscription "$SUBSCRIPTION" \
--resource-group "$RESOURCE_GROUP" \
--name "$AKS_NAME" \
--location "$LOCATION" \
--generate-ssh-keys

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:40

  • This command creates the default system node pool without specifying its size, and the output below shows three additional nodepool1 nodes before the single MI300X node is added. That adds avoidable cost for a lab described as creating one GPU node; set the system pool size explicitly or document the intentional high-availability choice.
az aks create \
--subscription "$SUBSCRIPTION" \
--resource-group "$RESOURCE_GROUP" \
--name "$AKS_NAME" \
--location "$LOCATION" \
--generate-ssh-keys

examples/amd-gpu-on-aks/0-install-gpu-driver-and-device-plugin.md:18

  • The subscription placeholder has an invalid UUID shape (6-4-4-4-8), so copying this example into SUBSCRIPTION produces an invalid Azure subscription ID. Use a standard 8-4-4-4-12 placeholder or a clearly non-copyable value.
| `SUBSCRIPTION` | Azure subscription ID | `000000-0000-0000-0000-00000000` |

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:27

  • The DRA driver image is mutable because it uses :latest; a later pull can introduce a driver version that is incompatible with the pinned GPU Operator chart or the documented fields. Pin this image to the tested DRA-driver release.
    image: rocm/k8s-gpu-dra-driver:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:80

  • The workload image uses :latest, so the DRA validation can silently change independently of the driver setup and the recorded output. Pin it to the tested ROCm/PyTorch release.
    image: docker.io/rocm/pytorch:latest

examples/amd-gpu-on-aks/1-dra-driver-for-amd-gpu.md:9

  • The sample amd-smi Pod from the preceding guide keeps running and requests all eight amd.com/gpu devices. Switching this same DeviceConfig to DRA without deleting that Pod leaves the legacy device-plugin allocation active, so the subsequent claim test can fail or overlap outside DRA. Add a cleanup command before editing the DeviceConfig.
The device-plugin is currently enabled by default if you follow the instructions in [0-install-gpu-driver-and-device-plugin.md](0-install-gpu-driver-and-device-plugin.md), so we need to **disable** `device-plugin` and **enable** DRA. 

examples/amd-gpu-on-aks/manifests/1-dra-multiple-pods-share.yaml:47

  • This second Pod also uses the mutable :latest image, so it can drift from the first Pod and from the documented test output. Pin it to the same tested ROCm/PyTorch tag used by pod1.
    image: docker.io/rocm/pytorch:latest 
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to 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