BUILD: add RAPIDS wrapper-image build tooling - #11633
Conversation
520b5eb to
9be8851
Compare
|
🤖 Starting review — findings will be posted here when done. |
| @@ -0,0 +1,44 @@ | |||
| # Multi-arch (amd64 + arm64) build of the UCXX rapidsai CI wrapper images. | |||
| # | |||
| # Driven by build-rapidsai-images.sh (sets up a 2-node buildx builder, then | |||
There was a problem hiding this comment.
build-rapidsai-images.sh doesn't exist — the script added here is rapidsai-images-build.sh. Same on line 6.
There was a problem hiding this comment.
Fixed, thanks.
| @@ -0,0 +1,94 @@ | |||
| #!/bin/bash -eE | |||
There was a problem hiding this comment.
| #!/bin/bash -eE | |
| #!/bin/bash -eEo pipefail |
Missing pipefail: otherwise the docker manifest inspect ... | grep -o at the end (L90-91) can't fail, and the verify step won't catch a missing push.
There was a problem hiding this comment.
Fixed, though as set -eE -o pipefail in the body rather than on the shebang: shebang flags are dropped when a script is run as bash <path>.
| rapidsai-ci-wheel:${RAPIDS_VER}-cuda12-azp-1 \ | ||
| rapidsai-ci-wheel:${RAPIDS_VER}-cuda13-azp-1; do | ||
| printf '%-42s ' "$img" | ||
| docker manifest inspect "$REGISTRY/ucx/$img" 2>/dev/null \ |
There was a problem hiding this comment.
Verify step doesn't assert multi-arch — the loop just prints the architectures it finds. A manifest pushed with only one arch (the very failure mode the script is designed to prevent, called out in the docs troubleshooting section) won't fail the script. Given the whole point is multi-arch, can we check that both amd64 and arm64 are present and exit non-zero otherwise?
There was a problem hiding this comment.
Good catch, fixed. It now fails if either amd64 or arm64 is missing.
| : "${HARBOR_UCX_USER:?set HARBOR_UCX_USER}" "${HARBOR_UCX_PASSWORD:?set HARBOR_UCX_PASSWORD}" | ||
| BUILDER=${BUILDER:-ucx-rapidsai} | ||
| REMOTE_CTX=${REMOTE_CTX:-ucx-remote-buildnode} | ||
| REGISTRY=${REGISTRY:-harbor.mellanox.com} |
There was a problem hiding this comment.
REGISTRY is misleading: it only affects docker login and the final docker manifest inspect; the actual push target is hardcoded in the compose image: fields as harbor.mellanox.com/ucx/.... Setting REGISTRY=other.example.com would log in to the wrong host and then verify a manifest that was pushed to harbor. Either drop the knob or thread it through the compose too.
There was a problem hiding this comment.
Fixed - the compose image: fields use ${REGISTRY:-harbor.mellanox.com} and the script exports it, so login, push and verify all follow the same value.
| build: | ||
| context: . | ||
| dockerfile: rapidsai-ci-conda.Dockerfile | ||
| platforms: |
There was a problem hiding this comment.
Inert platforms: key here (and at L29-31, L40-42) — the script's own comment at L76-77 says bake doesn't honor build.platforms and forces platforms via --set. Leaving platforms: in the compose invites confusion (readers assume it's the source of truth). Maybe drop it, or add a one-line comment marking it as documentation-only.
There was a problem hiding this comment.
Right, removed. The header now says the platforms come from the script's --set.
|
|
||
| Bump the old `<VER>` to the new one in the only places that reference it: | ||
|
|
||
| - `buildlib/dockers/rapidsai-ci-conda.Dockerfile` (`BASE_IMAGE`) |
There was a problem hiding this comment.
This references rapidsai-ci-conda.Dockerfile and rapidsai-ci-wheel.Dockerfile (L82-83), which don't exist in this diff or in the tree. Is this the first of a series, or should those files land here too? If merged alone, the compose + build script won't actually build anything.
There was a problem hiding this comment.
Correct - they come with the UCXX CI integration PR. Noted that in the README; this PR is the tooling only.
Add a declarative compose + one-command buildx-bake script to rebuild the UCXX rapidsai CI wrapper images multi-arch (amd64+arm64) natively, plus a doc for the RAPIDS version-bump procedure. The compose is the single source for the RAPIDS base tags - the script derives its preflight from it. Node endpoints and harbor credentials are supplied via the environment; nothing environment-specific is committed.
9be8851 to
cb1b7a5
Compare
3 participants
What?
Adds build tooling + a doc for the UCXX RAPIDS wrapper CI images (
buildlib/dockers/): a multi-archdocker buildx bakecompose, a one-command build script, and a procedure doc for the RAPIDS version bump.Why?
Makes rebuilding the wrapper images on a RAPIDS version bump reproducible and documented - one command builds both arches natively and pushes the manifest, instead of a manual per-arch loop.
Notes
rapidsai-ci-*.Dockerfiles the compose references.