Skip to content

contrib: unified ROCm/CUDA Dockerfile with ROCm build-check CI and wheel workflow - #1735

Open
andyluo7 wants to merge 11 commits into
ai-dynamo:mainfrom
andyluo7:pr3-rocm-docker
Open

andyluo7 wants to merge 11 commits into
ai-dynamo:mainfrom
andyluo7:pr3-rocm-docker

Conversation

@andyluo7

@andyluo7 andyluo7 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Restructures contrib/Dockerfile into a unified three-stage build (common-base → rocm-build / cuda-build) and adds ROCm-specific CI and release infrastructure. Splits the packaging surface out of #1647 per @tvegas1's request.

What this adds

File Purpose
contrib/Dockerfile Three-stage build: common-base (shared apt deps, Abseil/gRPC, Python tooling) → rocm-build (UCX --with-rocm, PyTorch ROCm wheels, nixl_rocm wheel) / cuda-build (DOCA, libfabric, UCX --with-cuda, NIXL EP, nixl CUDA wheel). UCX pinned to 940c1c1d9 (last clean commit before a put_offload.c -Werror regression on v1.21.x).
contrib/build-container.sh --rocm flag to select the rocm-build target, ROCm base image, and nixl_rocm wheel name.
contrib/build-wheel.sh --wheel-name / --backend-dir args for ROCm wheel naming.
contrib/README.rocm.md Runtime guide: AMD Pensando AINIC dmabuf knobs (/boot bind-mount + UCX_ROCM_* env vars), nixlbench launch command.
.github/workflows/rocm-build-check.yml Compile-only ROCm CI on ubuntu-latest via ./contrib/build-container.sh --rocm. Smoke-tests that import nixl dispatches to the nixl_rocm backend and that both wheel files are present. Skips draft PRs.
.github/workflows/wheel-rocm.yml On tagged releases, builds the ROCm container and attaches nixl_rocm-*.whl as a release asset.
src/bindings/python/nixl-meta/nixl/__init__.py Rename _load_cuda_backend_load_backend; dispatch to nixl_rocm first when torch reports a HIP version.
meson.build / meson_options.txt python_backend_dir / python_backend_package options used by build-wheel.sh to emit the correct wheel name for ROCm.

Validated on

  • ROCm: AMD Instinct MI350X (gfx950) on gbt350-odcdh2-b05-1 — full container build, nixl_rocm wheel extracted, smoke test passes.
  • CUDA: NVIDIA B300 on smcb300-ccs-aus-j11-03 — full container build, nixl wheel extracted, smoke test passes.

Key design notes

  • Unified Dockerfile: single file for both backends; CI selects the target via --target rocm-build or --target cuda-build.
  • UCX pin: 940c1c1d9 is the last commit on v1.21.x before a -Werror=int-conversion regression in put_offload.c that breaks both gcc 13 and gcc 14 builds.
  • Docker multi-stage ARG scoping: ARGs declared in common-base do not propagate to child stages. All shared ARGs are re-declared with defaults in both rocm-build and cuda-build.
  • Self-bootstrapping CI: rocm-build-check.yml builds the container from source on each PR rather than consuming a pre-built image, so no external image publishing is required.

Stacks on

Test plan

  • ./contrib/build-container.sh --rocm builds end-to-end on ROCm host (MI350X)
  • import nixl; nixl._pkg.__name__ == "nixl_rocm" smoke test passes
  • nixl_rocm-*.whl and nixl-*-none-any.whl present under /workspace/nixl/dist
  • ./contrib/build-container.sh (CUDA default) builds end-to-end on CUDA host (B300)
  • nixl-*.whl present and imports cleanly

Summary by CodeRabbit

  • New Features
    • Added ROCm (AMD GPU) support with automatic runtime backend selection based on the installed PyTorch build.
  • CI/CD
    • Added ROCm container smoke-check workflow for pull requests.
    • Added release workflow to build and publish ROCm wheel artifacts for version tags.
  • Packaging
    • Improved container and wheel build tooling to generate ROCm-specific images/wheels and packaged wheel outputs.
    • Added Meson build overrides for Python backend directory/package naming.
  • Documentation
    • Added comprehensive ROCm build, run, and configuration guidance for containers and benchmarks.

@copy-pr-bot

copy-pr-bot Bot commented Jun 5, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

👋 Hi andyluo7! Thank you for contributing to ai-dynamo/nixl.

Your PR reviewers will review your contribution then trigger the CI to test your changes.

🚀

@dpressle

dpressle commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

👀 Investigating rocm-build-check

@dpressle

dpressle commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

🤖 CI Triage Agentrocm-build-check · commit 3b09eb65

The PR branch files aren't in the default branch yet. The log evidence is sufficient for a definitive diagnosis. Here is the full analysis:


Summary: The rocm-build-check CI job failed because the Docker image ghcr.io/andyluo7/nixl-rocm:base-latest does not exist in the GitHub Container Registry — the daemon returns manifest unknown on every pull attempt.

Root cause: The workflow file (.github/workflows/rocm-build-check.yml, introduced in PR #1735) references a pre-built base image at ghcr.io/andyluo7/nixl-rocm:base-latest as its job container. That image was never pushed to ghcr.io under the andyluo7 namespace, so the registry returns manifest unknown and Docker fails to pull it after three retries. The entire job is unable to start because the container image is a prerequisite. This is a bootstrap problem: the PR introduces both the workflow that consumes the image and presumably a separate workflow that builds and publishes the image, but the build/publish step either hasn't been run yet or targets a different registry path.

Implicated commit: [REDACTED:Hex High Entropy String] (author: andyluo7, branch pr3-rocm-docker)

File: .github/workflows/rocm-build-check.yml — the container: image: field referencing ghcr.io/andyluo7/nixl-rocm:base-latest

Suggested fix: There are two complementary actions needed:

  1. Push the missing image first. Manually trigger the image-build workflow (or run docker build … && docker push ghcr.io/andyluo7/nixl-rocm:base-latest) so the tag actually exists in GHCR before the CI job runs. Confirm the image is publicly visible (or that the CI runner's GITHUB_TOKEN has packages: read permission on that namespace).

  2. Fix the image namespace / ownership. The image is stored under the personal account andyluo7 rather than the organisation (ai-dynamo). For long-term stability the image should be published to ghcr.io/ai-dynamo/nixl-rocm:base-latest and the workflow updated to match — a personal namespace is fragile if that account's permissions or PAT expire.

  3. (Optional) Gate the build-check on image availability. Add a preliminary step that verifies the image exists (e.g. docker manifest inspect) and fails fast with a human-readable error rather than three silent retries.

Related: PR #1735 — contrib/rocm: add ROCm Dockerfile + compile-only CI + wheel workflow

🛡️ This comment had 1 potential secret(s) redacted (Hex High Entropy String). See request_id 3c6fd533-9b28-416b-9004-2904945bad1f in the triage console for the audit trail.

@andyluo7

andyluo7 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@dpressle thanks for the precise diagnosis — all three recommendations are valid and addressed in 5469470:

1. Image not published (the bootstrap problem) — Confirmed. The image build is in flight on AAC1 (separate workstream); image namespace is moving to the org per #2 below before the push, so the URI in this PR is the final target.

2. Move from personal namespace to org namespace — ✅ done. Retargeted all three references from ghcr.io/andyluo7/nixl-rocmghcr.io/ai-dynamo/nixl-rocm (in rocm-build-check.yml, wheel-rocm.yml, and the README). The image will be published under the ai-dynamo org, which requires write-packages permission on that org — flagging @aranadive @tvegas1 in case the org-side push needs help configuring access (I'll plan to publish from a sbatch job on AAC1 using a token with the right scope; happy to coordinate on which token / which path).

3. Gate the build-check on image availability — ✅ done in a slightly different shape. Added an if: !github.event.pull_request.draft to the job so the workflow simply doesn't run on Draft PRs. That sidesteps the chicken-and-egg cleanly: this PR (Draft) stops generating noise; once flipped to Ready (after the image is published + a smoke run passes), the workflow runs and validates the actual stack. (docker manifest inspect precheck doesn't help here because the container: directive pulls before any steps execute — the draft gate is structurally simpler.)

Where this leaves the PR:

  • Draft until the ghcr.io/ai-dynamo/nixl-rocm:base-latest image is built and pushed
  • All public CI (clang-format, copyright, pre-commit, python-checks, pr-size) ✅ green
  • Test plan checkboxes in the PR description track the remaining smoke gates

Visual review of the Dockerfile + workflow YAML welcomed in the meantime.

# --cap-add=IPC_LOCK --ipc=host \
# -v /boot:/boot:ro \
# -e UCX_ROCM_COPY_DMABUF=yes \
# -e UCX_ROCM_IPC_DMABUF=yes \

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.

UCX_ROCM_IPC_DMABUF does not exist as a parameter in UCX, only UCX_ROCM_COPY_DMABUF exists.

@andyluo7

andyluo7 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

@edgargabriel thanks for the correction — verified against upstream UCX v1.21.x source and you're right:

// src/uct/rocm/copy/rocm_copy_md.c — has DMABUF config field ✅
{"DMABUF", ...}

// src/uct/rocm/ipc/rocm_ipc_md.c — config table is empty (only inherits md super) ❌
static ucs_config_field_t uct_rocm_ipc_md_config_table[] = {
    {"", "", NULL, ucs_offsetof(uct_rocm_ipc_md_config_t, super),
     UCS_CONFIG_TYPE_TABLE(uct_md_config_table)},
    {NULL}
};

So UCX_ROCM_IPC_DMABUF was a phantom in my docs — the rocm_ipc MD doesn't expose its own dmabuf knob; only UCX_ROCM_COPY_DMABUF does. Fixed in 5c9468f:

  • Removed UCX_ROCM_IPC_DMABUF from the runtime ENV in Dockerfile.rocm
  • Removed it from the example docker run invocation in the file header
  • Removed it from the example podman run in README.rocm.md
  • Reworded the README's "why" section to note UCX_ROCM_COPY_DMABUF is the only *_DMABUF knob in upstream, with the rocm_ipc MD inheriting dmabuf behavior via the shared rocm base detection rather than its own switch

UCX_ROCM_IPC_MIN_ZCOPY=0 retained (real config field in rocm_ipc_iface.c, default 128, needed to engage zero-copy on the IPC path per @tvegas1's earlier comment).

CI on the amended HEAD should be unchanged (still all public checks ✅, ROCm compile-only correctly skipped on Draft).

@andyluo7
andyluo7 force-pushed the pr3-rocm-docker branch 5 times, most recently from da8b66b to 397a14b Compare June 6, 2026 22:57
Comment thread .github/workflows/rocm-build-check.yml Outdated
# faster than rebuilding the whole stack each time, and it isolates
# regression risk to nixl/nixlbench code rather than infra.
#
# Future (medium-term): private GitHub runner with real MI300X/MI355X

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.

Future -> TODO: to be able to grep. also separate from header.

Comment thread .github/workflows/rocm-build-check.yml Outdated
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
# SPDX-License-Identifier: Apache-2.0
#
# Short-term compile-only ROCm CI per @edgargabriel's ask on #1647:

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.

maybe don't add discussion in the file just short descriptive comment?

WORKDIR /workspace
COPY . /workspace/nixl

# NIXL — emits a `nixl_${WHEEL_VARIANT}` wheel via #1642's wheel_variant option.

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.

don't reference commits in comment

@@ -0,0 +1,110 @@
# NIXL + nixlbench on AMD ROCm/HIP

Validated on AMD Instinct MI300X (`gfx942`) and MI355X (`gfx950`) with

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.

Supported instead of validated

ghcr.io/ai-dynamo/nixl-rocm:base-latest
```

This image has UCX 1.21.x built with `--with-rocm --with-verbs`, all

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.

do not reference version here?


## Stacks on

- ai-dynamo/nixl#1642 — adds `wheel_variant` Meson option (merged)

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.

don't reference dev/commit details at all


## CI integration

- **`rocm-build-check.yml`** — compile-only CI on every PR touching

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.

need to remove all references to dev workflow, and users comments.

@@ -0,0 +1,196 @@
# SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.

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.

WDYT with approach described in gist:

FROM <img> as common-base
...
FROM common-base as rocm-build
...
FROM common-base as cuda-build
...

Also maybe add similar change to .py as listed in the GIST.

andyluo7 added a commit to andyluo7/nixl that referenced this pull request Jun 9, 2026
Restructure contrib/Dockerfile into three stages:

- common-base: shared apt deps, Abseil/gRPC/etcd-cpp source builds,
  python venv + uv, gtest-parallel, pybind11
- rocm-build: UCX configured --with-rocm, PyTorch from rocm wheels,
  NIXL meson with disable_plugins=LIBFABRIC,GDS,GDS_MT,GPUNETIO and
  build_nixl_ep=false; wheel name = nixl_rocm
- cuda-build: DOCA install, libfabric from source, UCX --with-cuda,
  PyTorch CUDA wheels, optional NIXL EP build

contrib/build-container.sh: add --rocm flag toggling base image to
rocm/dev-ubuntu-24.04:7.2.4-complete, target rocm-build, wheel name
nixl_rocm, PyTorch ROCm index, BUILD_NIXL_EP=false.

contrib/build-wheel.sh: add --wheel-name and --backend-dir args,
skip nvcc CUDA detection when WHEEL_NAME provided, pass new
python_backend_dir/python_backend_package meson opts, extend
auditwheel excludes to cover ROCm runtime libs.

meson.build + meson_options.txt: add python_backend_dir and
python_backend_package options that override cuda_wheel_dir /
package name (used by contrib/build-wheel.sh). Compatible with
the existing wheel_variant option from ai-dynamo#1642 - python_backend_dir
wins when set.

src/bindings/python/nixl-meta/__init__.py: add _get_torch_rocm_version,
rename _load_cuda_backend -> _load_backend, dispatch to nixl_rocm
before nixl_cu* when torch reports a HIP version.

src/bindings/python/nixl-meta/meson.build: use python_backend_package
for the WHEEL_DEPS template substitution so the meta wheel correctly
depends on nixl-rocm when built for ROCm.

Closes architectural ask on PR ai-dynamo#1735.

Signed-off-by: andyluo7 <andy.luo@amd.com>
@pull-request-size pull-request-size Bot added size/XL and removed size/L labels Jun 9, 2026
@andyluo7 andyluo7 changed the title contrib/rocm: add ROCm Dockerfile + compile-only CI + wheel workflow (follow-up to #1642 + #1647) contrib: unified ROCm/CUDA Dockerfile with ROCm build-check CI and wheel workflow Jun 9, 2026
@andyluo7
andyluo7 marked this pull request as ready for review June 9, 2026 00:43
@andyluo7
andyluo7 requested review from a team, mkhazraee, ovidiusm and roiedanino as code owners June 9, 2026 00:43
@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds ROCm backend detection and dynamic backend loading, Meson options for backend naming, multi-stage Docker builds and scripts for ROCm/CUDA wheel creation, CI workflows to validate and publish ROCm wheels, and ROCm contributor documentation.

Changes

ROCm Backend and Build Infrastructure

Layer / File(s) Summary
Python runtime backend detection
src/bindings/python/nixl-meta/nixl/__init__.py
Module detects ROCm/HIP via torch and dynamically loads nixl_rocm when present, otherwise falls back to nixl_cu13 and nixl_cu12, with explicit import errors if no backend is found.
Meson backend and package configuration
meson_options.txt, meson.build, src/bindings/python/nixl-meta/meson.build
Adds python_backend_dir and python_backend_package Meson options; wheel dependency naming uses these overrides or auto-derives from the wheel directory.
Build scripts: ROCm option and wheel/backend params
contrib/build-container.sh, contrib/build-wheel.sh
Adds --rocm option and new build variables (WHEEL_NAME, BACKEND_DIR, PYTORCH_INDEX_URL), conditionally wires Docker build-args, and makes build-wheel.sh compute/accept wheel/backend names and pass them into uv build.
Dockerfile multi-stage refactor for ROCm/CUDA
contrib/Dockerfile
Refactors into common-base and separate rocm-build/cuda-build stages; centralizes Python/uv setup, builds UCX for ROCm, installs PyTorch wheels, builds NIXL and wheels, and updates CUDA/DOCA/libfabric steps and wheel install logic.
CI/CD workflows for ROCm build and release
.github/workflows/rocm-build-check.yml, .github/workflows/wheel-rocm.yml
rocm-build-check validates ROCm container builds on PRs (backend import and wheel presence); wheel-rocm builds/extracts ROCm wheels on tag/manual runs and uploads artifacts or release assets.
ROCm end-user documentation
contrib/README.rocm.md
Documents ROCm container build/run commands, Docker runtime requirements and UCX/IONIC runtime knobs, and in-container nixlbench build steps.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

Suggested reviewers

  • brminich
  • tvegas1
  • aranadive
  • ovidiusm

Poem

🐰 I hopped into Docker's den,
Built ROCm wheels and tuned the pen,
Backends chosen, tests take flight,
CI hums through day and night,
Wheels roll out on AMD light. 🚀

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: a unified Dockerfile supporting both ROCm and CUDA with associated CI and release workflows.
Description check ✅ Passed The description provides all required sections (What, Why, How) with comprehensive details about changes, design rationale, validation, and test plan per the template.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@andyluo7

Copy link
Copy Markdown
Contributor Author

Merged latest main into the branch (9b70bfc) to clear the merge conflict — PR is now MERGEABLE again.

The only conflict was in contrib/Dockerfile, from #1728 (DOCA 3.2 → 3.3 + telemetry exporter), which landed after this branch was cut. Resolution:

The rest of #1728 (the TELEMETRY_DOCA meson plugin logic, etc.) auto-merged cleanly.

@aranadive — branch is rebased and conflict-free, tvegas1 has approved. Could you trigger CI approval / /build when you get a chance? (tvegas1's earlier /build hit a vetter-permission auth failure.)

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 2

🤖 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 `@contrib/Dockerfile`:
- Around line 334-336: Remove the `apt-get upgrade -y` line from the Dockerfile
to ensure reproducible builds. The apt-get update and apt-get install commands
should remain, but delete the middle line containing only the upgrade command.
This prevents non-deterministic package version drift between builds while still
allowing the doca-sdk packages and dependencies to be installed correctly.
- Around line 332-334: The Dockerfile downloads the doca-host.deb package
without verifying its integrity, creating a supply-chain security risk. After
the wget command downloads
https://www.mellanox.com/downloads/DOCA/DOCA_v3.3.0/host/doca-host_3.3.0-088000-26.01-${MELLANOX_OS}_${ARCH_SUFFIX}.deb
to doca-host.deb, add a step to download and verify a checksum file (or GPG
signature if available) from the Mellanox repository. Verify the downloaded
doca-host.deb against this checksum before proceeding with the dpkg -i command,
and fail the build if verification fails.
🪄 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: ASSERTIVE

Plan: Enterprise

Run ID: d7c33bba-c8c9-4b1d-bb48-beeb782e8cc4

📥 Commits

Reviewing files that changed from the base of the PR and between 428d8dd and 9b70bfc.

📒 Files selected for processing (3)
  • contrib/Dockerfile
  • meson.build
  • meson_options.txt

Comment thread contrib/Dockerfile
Comment on lines +332 to +334
wget --tries=3 --waitretry=5 --no-verbose https://www.mellanox.com/downloads/DOCA/DOCA_v3.3.0/host/doca-host_3.3.0-088000-26.01-${MELLANOX_OS}_${ARCH_SUFFIX}.deb -O doca-host.deb && \
dpkg -i doca-host.deb && \
apt-get update && \

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Verify downloaded DOCA artifact integrity before dpkg -i.

This installs a remotely downloaded .deb as root without any checksum/signature verification, which is a supply-chain gap.

🔒 Suggested hardening patch
+ARG DOCA_HOST_SHA256
 RUN ARCH_SUFFIX=$(if [ "${ARCH}" = "aarch64" ]; then echo "arm64"; else echo "amd64"; fi) && \
     MELLANOX_OS="$(. /etc/lsb-release; echo ${DISTRIB_ID}${DISTRIB_RELEASE} | tr A-Z a-z | tr -d .)" && \
     wget --tries=3 --waitretry=5 --no-verbose https://www.mellanox.com/downloads/DOCA/DOCA_v3.3.0/host/doca-host_3.3.0-088000-26.01-${MELLANOX_OS}_${ARCH_SUFFIX}.deb -O doca-host.deb && \
+    echo "${DOCA_HOST_SHA256}  doca-host.deb" | sha256sum -c - && \
     dpkg -i doca-host.deb && \
🤖 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 `@contrib/Dockerfile` around lines 332 - 334, The Dockerfile downloads the
doca-host.deb package without verifying its integrity, creating a supply-chain
security risk. After the wget command downloads
https://www.mellanox.com/downloads/DOCA/DOCA_v3.3.0/host/doca-host_3.3.0-088000-26.01-${MELLANOX_OS}_${ARCH_SUFFIX}.deb
to doca-host.deb, add a step to download and verify a checksum file (or GPG
signature if available) from the Mellanox repository. Verify the downloaded
doca-host.deb against this checksum before proceeding with the dpkg -i command,
and fail the build if verification fails.

Comment thread contrib/Dockerfile Outdated
@andyluo7

Copy link
Copy Markdown
Contributor Author

Follow-up after rebasing onto main — pushed three commits and worked through the CI:

  1. Merge main (9b70bfc) — resolved the contrib/Dockerfile conflict from build: bump DOCA to 3.3 and build the DOCA telemetry exporter in CI #1728 (DOCA 3.3 + telemetry exporter folded into cuda-build).
  2. ci: free disk space (f68d5cb) — the rocm-build-check was failing because the ROCm -complete image (~30GB extracted) exhausted the ubuntu-latest disk during extraction. Added a cleanup step → 46GB free, extraction now succeeds.
  3. ci: make ROCm build-check non-blocking (8158525) — with disk fixed, the build then hit the 90-min timeout having only reached gRPC. A from-scratch build (UCX/gRPC/aws-sdk/etc. from source) doesn't fit the hosted-runner budget, so the job is now continue-on-error and no longer gates PRs.

Real fix (follow-up): switch rocm-build-check to consume the pre-built ghcr.io/ai-dynamo/nixl-rocm base image so each PR only compiles nixl (~minutes), then flip the job back to blocking. That's gated on the base image being published (org write-packages access) — same ask as before.

All other checks are green. @tvegas1 — the rebase dismissed your approval; would you re-approve when you get a chance? @aranadive — still need a vetter /build for blossom-ci.

@andyluo7

Copy link
Copy Markdown
Contributor Author

Update: ROCm container build + smoke is now green ✅ (48m, all other checks pass too).

Root cause of the earlier failures wasn't the build — the full graph (compile, UCX, nixl, wheel, auditwheel, pip-install) succeeded every time. The runner was being OOM-killed/recycled at the final docker image-export step while compressing the multi-GB ROCm image (exit 143 during exporting layers, with 116 GB disk free — so not disk, not auditwheel).

Fix (2321f5f): validate the build without materializing the image.

  • Moved the wheel-presence + import nixl -> nixl_rocm smoke checks into the rocm-build stage as RUN steps.
  • Added a --check-only flag to build-container.sh that builds via docker buildx build --output type=cacheonly (runs the whole graph incl. smokes, skips the image export).
  • Workflow now sets up a docker-container buildx builder and uses --check-only.

This still complements the durable plan (consume a pre-built ghcr.io/ai-dynamo/nixl-rocm base image so per-PR builds only compile nixl); the job remains continue-on-error for now but it actually passes on its own merits and could be flipped to a hard gate.

@tvegas1 the rebase dismissed your approval — would you re-approve? @aranadive still need a vetter /build for blossom-ci. Thanks!

runs-on: ubuntu-latest
# Non-blocking for now: a from-scratch ROCm build exceeds the hosted-runner
# budget (the "-complete" image plus building UCX/gRPC/etc. from source runs
# well past the timeout). Flip to blocking once the build consumes the

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.

can we fix the timeout issue instead?


- name: Smoke - import nixl, dispatch to nixl_rocm backend
# The ROCm "-complete" base image is ~30GB extracted and does not fit on a
# stock ubuntu-latest runner (~14GB free); reclaim space before building.

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.

maybe we should fix the runner to use bigger disk?

Comment thread contrib/Dockerfile
# dispatches to the ROCm backend. Running these here lets CI validate the build
# with --output=cacheonly, avoiding the multi-GB image export that OOMs the
# hosted runner during layer compression.
RUN ls /workspace/nixl/dist/nixl_rocm-*.whl /workspace/nixl/dist/nixl-*-none-any.whl && \

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.

can we rather use the same scheme as done for nixl_cu*wheels?

docker build --platform linux/$ARCH -f $DOCKER_FILE $DOCKER_TARGET $BUILD_ARGS $TAG $NO_CACHE $BUILD_CONTEXT
if [ -n "$CHECK_ONLY" ]; then
# Build the full graph (incl. in-Dockerfile smoke RUNs) but do not export an
# image. Skipping the multi-GB image export avoids OOM-ing hosted CI runners.

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.

we should probably rather fix CI runners?

@andyluo7

Copy link
Copy Markdown
Contributor Author

@tvegas1 thanks — agreed on all four points. The hosted-runner workarounds (free-disk, --check-only/cacheonly, continue-on-error, in-image smokes) are band-aids for ubuntu-latest limits, not the right long-term shape. Plan to align ROCm with the nixl_cu* scheme:

Target scheme (mirrors contrib/Dockerfile.manylinux + .gitlab/test_python.sh):

  • Build nixl_rocm on a manylinux_2_28 base, vendoring deps from source (same recipe as cu*), emit multi-Python wheels via build-wheel.sh + auditwheel.
  • ROCm deltas only: UCX --with-rocm, PyTorch ROCm index, meson -Duse_rocm=true, wheel name nixl_rocm; no DOCA/CUDA.
  • No in-image smokes — validate separately: pip install dist/*.whl + pytest test/python on a self-hosted ROCm runner (real backend tests).

This is also exactly how ROCm/TheRock ships ROCm + PyTorch ROCm wheels (pypa manylinux_2_28, vendor-from-source), so there's good precedent.

Two things to sort out:

  1. Self-hosted ROCm runner doesn't exist yet — it needs to be set up before this CI can actually run. Who owns provisioning it on the AMD side? Until it exists, a manylinux_2_28 build can't be CI-validated.
  2. One open technical question: ROCm into a manylinux_2_28 (AlmaLinux 8) base — via the ROCm el8 yum repo, or by consuming TheRock's rocm[device-*] wheels? I'll de-risk both with a spike on AAC1.

Given the runner dependency, I'd suggest landing the manylinux_2_28 ROCm wheel build as a follow-up PR paired with the runner, rather than reworking this one in place. For #1735 itself: would you prefer I (a) trim it to the uncontroversial bits (the __init__.py backend dispatch + meson options) and move all CI/packaging to that follow-up, or (b) hold #1735 until the follow-up supersedes it? I have a draft Dockerfile.manylinux.rocm ready to share once we align on direction.

andyluo7 added a commit to andyluo7/nixl that referenced this pull request Jun 24, 2026
Restructure contrib/Dockerfile into three stages:

- common-base: shared apt deps, Abseil/gRPC/etcd-cpp source builds,
  python venv + uv, gtest-parallel, pybind11
- rocm-build: UCX configured --with-rocm, PyTorch from rocm wheels,
  NIXL meson with disable_plugins=LIBFABRIC,GDS,GDS_MT,GPUNETIO and
  build_nixl_ep=false; wheel name = nixl_rocm
- cuda-build: DOCA install, libfabric from source, UCX --with-cuda,
  PyTorch CUDA wheels, optional NIXL EP build

contrib/build-container.sh: add --rocm flag toggling base image to
rocm/dev-ubuntu-24.04:7.2.4-complete, target rocm-build, wheel name
nixl_rocm, PyTorch ROCm index, BUILD_NIXL_EP=false.

contrib/build-wheel.sh: add --wheel-name and --backend-dir args,
skip nvcc CUDA detection when WHEEL_NAME provided, pass new
python_backend_dir/python_backend_package meson opts, extend
auditwheel excludes to cover ROCm runtime libs.

meson.build + meson_options.txt: add python_backend_dir and
python_backend_package options that override cuda_wheel_dir /
package name (used by contrib/build-wheel.sh). Compatible with
the existing wheel_variant option from ai-dynamo#1642 - python_backend_dir
wins when set.

src/bindings/python/nixl-meta/__init__.py: add _get_torch_rocm_version,
rename _load_cuda_backend -> _load_backend, dispatch to nixl_rocm
before nixl_cu* when torch reports a HIP version.

src/bindings/python/nixl-meta/meson.build: use python_backend_package
for the WHEEL_DEPS template substitution so the meta wheel correctly
depends on nixl-rocm when built for ROCm.

Closes architectural ask on PR ai-dynamo#1735.

Signed-off-by: andyluo7 <andy.luo@amd.com>
@andyluo7

Copy link
Copy Markdown
Contributor Author

Rebased onto latest main (linear history, the earlier Merge branch 'main' commit is flattened). The contrib/Dockerfile conflict from #1728 was re-resolved — DOCA stays out of common-base and the v3.3.0 + telemetry-exporter forward-port is preserved in cuda-build, so there's no spurious DOCA diff vs main.

All CI is green on the rebased head (aa968c73):

  • ROCm container build + smoke ✅ (49m36s)
  • clang-format, pre-commit, copyright-checks, check-pr-size, all python-checks, CodeRabbit ✅

@tvegas1 — the rebase dismissed your approval; would you re-approve when you get a chance?
@aranadive — could you trigger the vetter /build for blossom-ci? Thanks!

@sbates130272

Copy link
Copy Markdown
Contributor

@andyluo7 after speaking with the NVIDIA team on the topic of building and CI we are going to start with a plan to have separate Dockerfile and build.sh for now. I will work on that but message me or comment here if you have questions.

andyluo7 and others added 11 commits June 30, 2026 17:21
Restructure contrib/Dockerfile into three stages:

- common-base: shared apt deps, Abseil/gRPC/etcd-cpp source builds,
  python venv + uv, gtest-parallel, pybind11
- rocm-build: UCX configured --with-rocm, PyTorch from rocm wheels,
  NIXL meson with disable_plugins=LIBFABRIC,GDS,GDS_MT,GPUNETIO and
  build_nixl_ep=false; wheel name = nixl_rocm
- cuda-build: DOCA install, libfabric from source, UCX --with-cuda,
  PyTorch CUDA wheels, optional NIXL EP build

contrib/build-container.sh: add --rocm flag toggling base image to
rocm/dev-ubuntu-24.04:7.2.4-complete, target rocm-build, wheel name
nixl_rocm, PyTorch ROCm index, BUILD_NIXL_EP=false.

contrib/build-wheel.sh: add --wheel-name and --backend-dir args,
skip nvcc CUDA detection when WHEEL_NAME provided, pass new
python_backend_dir/python_backend_package meson opts, extend
auditwheel excludes to cover ROCm runtime libs.

meson.build + meson_options.txt: add python_backend_dir and
python_backend_package options that override cuda_wheel_dir /
package name (used by contrib/build-wheel.sh). Compatible with
the existing wheel_variant option from ai-dynamo#1642 - python_backend_dir
wins when set.

src/bindings/python/nixl-meta/__init__.py: add _get_torch_rocm_version,
rename _load_cuda_backend -> _load_backend, dispatch to nixl_rocm
before nixl_cu* when torch reports a HIP version.

src/bindings/python/nixl-meta/meson.build: use python_backend_package
for the WHEEL_DEPS template substitution so the meta wheel correctly
depends on nixl-rocm when built for ROCm.

Closes architectural ask on PR ai-dynamo#1735.

Signed-off-by: andyluo7 <andy.luo@amd.com>
rocm-build-check.yml: builds the ROCm container via
./contrib/build-container.sh --rocm and smoke-tests that `import nixl`
dispatches to the nixl_rocm backend and that both nixl-* and nixl_rocm-*
wheels are present under /workspace/nixl/dist. Builds end-to-end in CI
rather than consuming a pre-built image, so the workflow is self-bootstrapping.

wheel-rocm.yml: on tagged releases, builds the same container and
extracts dist/nixl_rocm-*.whl for upload as a release asset.

contrib/README.rocm.md: short ROCm-only runtime guide covering the
build command, the AMD Pensando AINIC dmabuf knobs (/boot mount +
UCX_ROCM_COPY_DMABUF + UCX_ROCM_IPC_MIN_ZCOPY), and the nixlbench
build step. No dev workflow content.

Signed-off-by: andyluo7 <andy.luo@amd.com>
- Dockerfile: collapse 4-line gcc-13 comment to 2-line; drop UCX bug reference
- wheel-rocm.yml: shorten header comment to one line

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
- Pin all action refs to immutable commit SHAs to prevent tag-based
  supply-chain attacks (checkout@v4, upload-artifact@v4, action-gh-release@v2)
- Add persist-credentials: false to both checkout steps
- Add concurrency block to wheel-rocm.yml to prevent parallel builds
  of the same ref

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Both env vars are set by the container; add a note for users building
outside Docker to export them or substitute the defaults directly.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
Cold --no-cache build on ubuntu-latest exceeds the previous 60-min
limit. 90 min matches wheel-rocm.yml and our observed build time.

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
…blocks

Co-Authored-By: Claude Sonnet 4 <noreply@anthropic.com>
The ROCm '-complete' base image is ~30GB extracted and exhausts the stock
ubuntu-latest runner during base-image extraction. Reclaim ~25-30GB by removing
unused toolchains/SDKs and pruning docker images before the build.
A from-scratch ROCm container build does not fit the hosted-runner time budget
(reaches only gRPC before the 90 min timeout). Mark the job continue-on-error so
it no longer blocks PRs; it becomes a real gate once it consumes the pre-built
base image and compiles only nixl.
Reclaim build-cache layers in addition to images so the from-scratch ROCm build
has maximum headroom on the hosted runner.
The build itself (compile, wheel, auditwheel, pip-install) succeeds on the
hosted runner; only the final docker image export OOMs/recycles the runner while
compressing the multi-GB ROCm image. Move the wheel-presence and import-dispatch
smoke checks into the rocm-build stage as RUN steps and build with buildx
--output=cacheonly so the graph (and smokes) run without materializing the image.
WHL_PLATFORM=${WHL_BASE}_${ARCH}
WHL_PYTHON_VERSIONS="3.12"
UCX_REF=${UCX_REF:-v1.21.x}
UCX_REF=${UCX_REF:-940c1c1d9}

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.

Please remove this. The default must not be changed here, but in the caller that drives the script through --ucx-ref.



def _load_cuda_backend() -> str:
def _get_torch_rocm_version() -> str | None:

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.

This will need import latency optimizations similar to #1895

Comment thread contrib/Dockerfile
@@ -15,26 +15,20 @@

ARG BASE_IMAGE="nvcr.io/nvidia/cuda-dl-base"
ARG BASE_IMAGE_TAG="25.10-cuda13.0-devel-ubuntu24.04"

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.

This can technically work but I find the changes in this Dockerfile hard to understand. We also use it in many of our testing workflows so there is a risk of introducing breakage. Wondering if it might make sense to consider other options, maybe 2 Dockerfiles

WDYT @Bohatchuk @aranadive

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.

@sbates130272

Copy link
Copy Markdown
Contributor

@ovidiusm this PR has been superceeded by #1936. Can you priotize 1936 over this one? Thanks!

@andyluo7

Copy link
Copy Markdown
Contributor Author

Thanks @sbates130272 — happy to align with #1936 rather than run two parallel ROCm efforts. Looking at both PRs, I think they overlap on one layer but are actually complementary overall, so I'd propose splitting responsibilities instead of a straight supersede:

#1936 owns the ROCm container/build environment. It's built on the merged #1900, lives in the canonical .ci/dockerfiles/ location, targets ROCm 7.14, and has more thorough multi-stage Docker plumbing. That's the better home for the base/build image, and I'm glad to defer to it there.

#1735 contributes the packaging + runtime layer that #1936 doesn't cover. As noted in #1936 ("NIXL is not automatically built into this image"), it provides a build environment but not shippable artifacts or runtime enablement. #1735 adds, on top of a build env:

  • nixl_rocm wheel packaging + a tagged-release workflow that publishes the wheel as a release asset
  • Runtime backend auto-selectionimport nixl dispatches to nixl_rocm when PyTorch reports a HIP build (_load_backend in nixl-meta/__init__.py)
  • Unified CUDA+ROCm Dockerfile (--target rocm-build / cuda-build) so both backends share one file
  • README.rocm.md runtime guide (AMD Pensando AINIC dmabuf knobs + nixlbench launch)
  • Hardware-validated end to end: MI350X (gfx950) for ROCm and B300 for CUDA, both container-built and smoke-tested

Proposal: once #1936 lands, I'll rebase #1735 on top of it — dropping my .ci Dockerfile duplication and keeping only the wheel packaging, the release workflow, the runtime _load_backend dispatch, the Meson wheel-naming options, and the runtime docs. That turns the two PRs into two clean layers (build env → packaging/runtime) with no overlap, and keeps the hardware-validated release path in-tree.

One small thing worth carrying over from #1735: I pin UCX to the SHA 940c1c1d9 rather than tracking the moving v1.21.x branch. 940c1c1d9 is the last clean commit before a put_offload.c -Werror=int-conversion regression on v1.21.x that breaks gcc 13/14 builds — and pinning a SHA also avoids the stale-layer-cache hazard the #1936 file already calls out for its v1.21.x ref. Happy to fold that pin into the shared build env so both backends benefit.

Does that division work for you and @riley-dixon? If so, I'll start the rebase as soon as #1936 is merged (or against its branch now if that's easier to review). Also happy to move any of my pieces into .ci/ if you'd prefer them there rather than contrib/.

(One small note for reviewers: the earlier rocm-build-check failure referencing ghcr.io/andyluo7/nixl-rocm:base-latest is stale — the current #1735 CI is self-bootstrapping and builds the container from source, so there's no external image dependency anymore.)

@riley-dixon

Copy link
Copy Markdown
Contributor

Hey @andyluo7 ,
Thanks for your patience while I went over your PR here for how we might be able to integrate your work on top of #1936.

I have a few comments but am happy to work with you. I think one of the more significant things to be aware of is that we are planning on keeping the CUDA & ROCm build systems separate, and we do not have plans to unify them. This may change your mind how you approach build-container.sh and build-wheel.sh.

I will also advise that for building & packaging NIXL that those steps occur in a CI workflow file rather than as part of the Dockerfile.

UCX - Pinning it to a specific SHA or static release works for me. It will make caching a lot easier in the long term.
Torch - #1936 fixed the specific torch version that is being used. If you want to, you could parameterize this in the Dockerfile.

Please let me know your thoughts.

@sbates130272

Copy link
Copy Markdown
Contributor

@ovidiusm as I recall the plan is for AMD to publish our own wheels. Due to concerns of AMd wheels on NVIDIA PyPi. Do I recall that correctly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants