Add A4X MAX Qwen3-30B-A3B FP8mx 8 GPUs recipe - #277
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a Helm chart and launcher script to run a qwen3-30b-a3b pretraining workload on a4x-max GKE Node pools using the Nvidia Megatron-Bridge framework. The reviewer identified several critical issues, including the lack of 'set -eo pipefail' in launcher.sh (which masks training failures), a hardcoded HF_TOKEN placeholder that overwrites environment variables, and hardcoded 'default' namespaces in the JobSet metadata and FQDNs that prevent multi-namespace deployments. Additionally, the reviewer recommended pre-building system packages and the Megatron-Bridge repository into the container image rather than installing and cloning them at runtime, and suggested setting the default gcsMounts to an empty list to avoid Helm rendering failures.
| @@ -0,0 +1,185 @@ | |||
| usage() | |||
There was a problem hiding this comment.
The script does not enable errexit (-e) or pipefail (-o pipefail). Since shell options are not inherited by child shell processes, any failure inside this script (such as git clone failing or torchrun crashing) will not cause the script to exit immediately. Specifically, because torchrun is piped to python3 for logging, without pipefail enabled, the exit status of the pipeline will be the exit status of the python3 command (which is 0 on successful EOF), completely masking any training failures from torchrun. This will cause Kubernetes to report the job as successful even if the training crashed. Enable set -eo pipefail at the very beginning of launcher.sh to ensure failures are correctly propagated.
| usage() | |
| #!/bin/bash | |
| set -eo pipefail | |
| usage() |
| echo "VERSION_DIAGNOSTICS: ${kv}" | ||
|
|
||
|
|
||
| export HF_TOKEN=<YOUR_HF_TOKEN> |
There was a problem hiding this comment.
The hardcoded export HF_TOKEN=<YOUR_HF_TOKEN> in launcher.sh will overwrite any valid HF_TOKEN passed to the container from Kubernetes environment variables with the literal string <YOUR_HF_TOKEN>. Since launcher.sh is uploaded as-is via helm install --set-file, this will break Hugging Face authentication inside the pod. Allow HF_TOKEN to be inherited from the environment if already set, and avoid overwriting it.
| export HF_TOKEN=<YOUR_HF_TOKEN> | |
| export HF_TOKEN="${HF_TOKEN:-}" |
| kind: JobSet | ||
| metadata: | ||
| name: "{{ .Release.Name }}" | ||
| namespace: default |
There was a problem hiding this comment.
| - name: RANK_0_FQDN | ||
| value: "{{.Release.Name}}-workload-0-0.{{.Release.Name}}.default.svc.cluster.local" | ||
| - name: HOSTNAME_PREFIX | ||
| value: "{{.Release.Name}}-workload-" | ||
| - name: DOMAIN_NAME | ||
| value: "{{.Release.Name}}.default.svc.cluster.local" | ||
| - name: MASTER_ADDR | ||
| value: "{{.Release.Name}}-workload-0-0.{{.Release.Name}}.default.svc.cluster.local" | ||
| - name: MASTER_PORT | ||
| value: "6002" |
There was a problem hiding this comment.
The namespace default is hardcoded in the FQDNs for RANK_0_FQDN, DOMAIN_NAME, and MASTER_ADDR. If this Helm chart is deployed to any namespace other than default, the pods will fail to resolve the master node's address, causing the distributed training initialization to fail. Use {{ .Release.Namespace }} instead of the hardcoded default namespace.
- name: RANK_0_FQDN
value: "{{.Release.Name}}-workload-0-0.{{.Release.Name}}.{{.Release.Namespace}}.svc.cluster.local"
- name: HOSTNAME_PREFIX
value: "{{.Release.Name}}-workload-"
- name: DOMAIN_NAME
value: "{{.Release.Name}}.{{.Release.Namespace}}.svc.cluster.local"
- name: MASTER_ADDR
value: "{{.Release.Name}}-workload-0-0.{{.Release.Name}}.{{.Release.Namespace}}.svc.cluster.local"
- name: MASTER_PORT
value: "6002"| apt update -y | ||
| apt install -y curl | ||
| export DOCA_URL="https://linux.mellanox.com/public/repo/doca/3.1.0/ubuntu22.04/arm64-sbsa/" | ||
| BASE_URL=$([ "${DOCA_PREPUBLISH:-false}" = "true" ] && echo https://doca-repo-prod.nvidia.com/public/repo/doca || echo https://linux.mellanox.com/public/repo/doca) | ||
| DOCA_SUFFIX=${DOCA_URL#*public/repo/doca/}; DOCA_URL="$BASE_URL/$DOCA_SUFFIX" | ||
| curl $BASE_URL/GPG-KEY-Mellanox.pub | gpg --dearmor > /etc/apt/trusted.gpg.d/GPG-KEY-Mellanox.pub | ||
| echo "deb [signed-by=/etc/apt/trusted.gpg.d/GPG-KEY-Mellanox.pub] $DOCA_URL ./" > /etc/apt/sources.list.d/doca.list | ||
| apt update | ||
| apt install -y --allow-downgrades --allow-change-held-packages -o Dpkg::Options::="--force-overwrite" doca-ofed-userspace || apt --fix-broken install -y | ||
|
|
||
| # Install NCCL and nccl-gib-plugins package | ||
| apt install --only-upgrade --allow-change-held-packages -y libnccl2 libnccl-dev | ||
|
|
||
| # If image not from Google, trust the GCP signing key | ||
| curl https://packages.cloud.google.com/apt/doc/apt-key.gpg | gpg --dearmor -o /etc/apt/trusted.gpg.d/cloud.google.gpg | ||
|
|
||
| # Add gpudirect-gib-apt repo | ||
| echo 'deb https://packages.cloud.google.com/apt gpudirect-gib-apt main' | tee /etc/apt/sources.list.d/nccl-gib.list | ||
|
|
||
| apt update | ||
| apt install -y nccl-gib-plugins |
There was a problem hiding this comment.
Installing system packages (apt update and apt install for doca-ofed-userspace, libnccl2, and nccl-gib-plugins) at container runtime is highly discouraged for Kubernetes workloads. This introduces reliability issues if external repositories are down, adds significant startup latency, and fails in private/air-gapped GKE environments. Pre-build these dependencies into a custom Docker image extending nvcr.io/nvidia/nemo:26.06.01 instead of installing them at runtime.
| gcsMounts: | ||
| - bucketName: null | ||
| mountPath: null |
There was a problem hiding this comment.
The default gcsMounts list contains a null element. If a user deploys this chart without specifying GCS mounts, Helm will render invalid Kubernetes volume and volumeMount definitions with empty or <nil> names, causing the deployment to fail. Set gcsMounts to an empty list [] by default.
gcsMounts: []| cd /opt | ||
| rm -rf Megatron-Bridge | ||
| git clone https://github.com/NVIDIA-NeMo/Megatron-Bridge.git | ||
| cd Megatron-Bridge | ||
| git checkout 5cb3444c43f7499cf3872b2d46870cf8bc2e00ce | ||
| git submodule update --init --recursive |
There was a problem hiding this comment.
Cloning the Megatron-Bridge repository and updating submodules at runtime inside the launcher script is prone to network failures, GitHub rate-limiting (especially when multiple nodes clone concurrently), and increased startup latency. Pre-clone this repository and build it into the container image, or package it as part of the workspace/volume.
Add A4X MAX Qwen3-30B-A3B 8 GPUs FP8mx recipe