-
Notifications
You must be signed in to change notification settings - Fork 6
Build multi-arch images #54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
4c51e6c
to
b7f5def
Compare
📝 WalkthroughWalkthroughCircleCI pipeline updates remove registry-based buildx caching from all docker buildx steps, add multi-arch builds (linux/amd64, linux/arm64) across affected image jobs, and set no_output_timeout: 60m for the python image build step. The Conda Dockerfile adds TARGETARCH-based Miniconda selection (amd64 -> x86_64, arm64 -> aarch64), installs Miniconda to /opt/conda, and configures conda to prefer the conda-forge channel (removing defaults) before installing Python. Workflow structure and public interfaces remain unchanged. Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CI as CircleCI
participant Buildx as Docker Buildx
participant Reg as Container Registry
Dev->>CI: Push/PR triggers pipeline
CI->>Buildx: buildx build --platform linux/amd64,linux/arm64 (no registry cache)
Buildx->>Reg: Push images/tags (multi-arch)
sequenceDiagram
participant Docker as Dockerfile.conda
participant Arch as TARGETARCH logic
participant Miniconda as Miniconda Installer
participant Conda as Conda runtime
Docker->>Arch: Determine TARGETARCH (amd64 → x86_64, arm64 → aarch64)
Arch->>Miniconda: Download Miniconda3-latest-Linux-${MINICONDA_ARCH}.sh
Docker->>Miniconda: Install to /opt/conda
Docker->>Conda: conda config --add channels conda-forge
Docker->>Conda: conda config --remove channels defaults || true
Docker->>Conda: conda install python=${PYTHON_VERSION}
Docker->>Docker: Cleanup and PATH update
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🔭 Outside diff range comments (3)
.circleci/config.yml (3)
149-153
: python-conda job still builds amd64 only. Add arm64 and the platform fix.To align with multi-arch goals, include arm64 and use the comma-separated form.
- --platform linux/amd64 \ + --platform linux/amd64,linux/arm64 \Also consider adding no_output_timeout here; Conda resolves can exceed 10m quiet periods.
13-17
: Install binfmt for reliable cross-arch builds.Machine executors may lack qemu handlers. Install binfmt before creating the builder to ensure arm64 builds work.
- run: name: Setup BuildKit builder command: | + docker run --privileged --rm tonistiigi/binfmt --install arm64,amd64 docker buildx create \ --name custom-builder \ --use \ --driver docker-container \
41-54
: Reduce duplication: define PLATFORMS once.Optional: pass PLATFORMS="linux/amd64,linux/arm64" via env to all jobs to avoid drift.
Example:
- docker buildx build \ + PLATFORMS="${PLATFORMS:-linux/amd64,linux/arm64}" && \ + docker buildx build \ --file=./python/base/Dockerfile.base \ ... \ - --platform linux/amd64,linux/arm64 \ + --platform "${PLATFORMS}" \Also applies to: 76-87, 107-119, 139-153
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.circleci/config.yml
(4 hunks)python/conda/Dockerfile.conda
(1 hunks)
🔇 Additional comments (4)
.circleci/config.yml (4)
70-70
: Good call on extending timeout.Python source builds can be chatty-silent. 60m no_output_timeout is prudent.
185-189
: IR image is amd64-only. Intentional?If IR supports arm64, add it; otherwise, document why it’s excluded.
- --platform linux/amd64 \ + --platform linux/amd64,linux/arm64 \
220-224
: IR-with-libs: same platform consideration.Add arm64 if supported.
- --platform linux/amd64 \ + --platform linux/amd64,linux/arm64 \
265-269
: GPU builds: please verify ARM64 support for CUDA base imagesWe weren’t able to run
docker buildx imagetools inspect
in this environment (no Docker available), so please manually confirm that each CUDA tag includes an arm64 manifest before adding--platform linux/amd64
. If a tag is amd64-only, either retain the current single-platform setting or document the limitation.Points to check:
- Inspect each tag with:
or use skopeo/crane if preferred.docker buildx imagetools inspect nvidia/cuda:<TAG> | grep -A2 Manifests
- Ensure the resulting manifest list includes an entry for “arm64”.
- If any tag lacks ARM64, omit
linux/arm64
support and note this in the CI config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (3)
python/conda/Dockerfile.conda (1)
30-34
: Channel config + strict priority: nice.Switch to conda-forge, tolerate missing defaults, and enable strict priority. Matches prior guidance.
.circleci/config.yml (2)
49-49
: Combined --platform is correct.Builds both amd64 and arm64 in one flag. LGTM.
112-112
: Multi-arch for datascience: LGTM.Matches base/python jobs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
.circleci/config.yml
(4 hunks)python/conda/Dockerfile.conda
(1 hunks)
🔇 Additional comments (3)
.circleci/config.yml (3)
69-69
: no_output_timeout: 60m is sensible for Python builds.Prevents premature timeout on slow solves/builds.
49-50
: Ensure binfmt is registered to enable arm64 builds on amd64 runners.CircleCI machine images may lack QEMU/binfmt. If arm64 builds fail, add:
setup_buildkit_builder: steps: - run: name: Setup BuildKit builder command: | + docker run --privileged --rm tonistiigi/binfmt --install arm64,amd64 docker buildx create \ --name custom-builder \ --use \ --driver docker-container \
Verify with: docker buildx ls (should list linux/amd64, linux/arm64).
Also applies to: 81-82, 112-113
81-83
: Note: AI summary says cache removed “from all steps”, but cache remains in some jobs.Cache flags still present in python-conda, ir, ir-with-libs, and gpu jobs. If intentional, ignore. If not, remove for consistency.
--cache-to type=registry,ref=docker.io/${REPOSITORY}:${TAG}${TAG_SUFFIX}-buildcache,mode=max,image-manifest=true,oci-mediatypes=true \ | ||
--progress plain \ | ||
--platform linux/amd64 \ | ||
--platform linux/amd64,linux/arm64 \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Multi-arch for python: LGTM. Also align python-conda job.
This job targets both platforms. The python-conda job (Lines 142–147) still builds only linux/amd64 and uses registry cache. Consider aligning it:
- --cache-from type=registry,ref=docker.io/${REPOSITORY}:${TAG}-buildcache \
- --cache-from type=registry,ref=docker.io/${REPOSITORY}:${TAG}${TAG_SUFFIX}-buildcache \
- --cache-to type=registry,ref=docker.io/${REPOSITORY}:${TAG}${TAG_SUFFIX}-buildcache,mode=max,image-manifest=true,oci-mediatypes=true \
--progress plain \
- --platform linux/amd64 \
+ --platform linux/amd64,linux/arm64 \
--provenance=false \
--sbom=false \
If you want consistent cache removal across jobs, drop the cache flags too.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
--platform linux/amd64,linux/arm64 \ | |
--progress plain \ | |
--platform linux/amd64,linux/arm64 \ | |
--provenance=false \ | |
--sbom=false \ |
ARG TARGETARCH | ||
RUN case "$TARGETARCH" in \ | ||
amd64) MINICONDA_ARCH="x86_64" ;; \ | ||
arm64) MINICONDA_ARCH="aarch64" ;; \ | ||
*) echo "Unsupported TARGETARCH: $TARGETARCH" >&2; exit 1 ;; \ | ||
esac && \ | ||
wget --quiet "https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-${MINICONDA_ARCH}.sh" -O /tmp/miniconda.sh && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Pin installer + verify checksum to harden supply chain and ensure reproducibility.
“latest” is mutable and unverified. Recommend pinning a specific installer and checking SHA256.
-ARG TARGETARCH
+ARG TARGETARCH
+# Pin Miniconda installer in CI (set these via build args or here)
+ARG MINICONDA_VERSION # e.g., py311_24.7.1-0
+ARG MINICONDA_SHA256 # corresponding sha256
RUN case "$TARGETARCH" in \
amd64) MINICONDA_ARCH="x86_64" ;; \
arm64) MINICONDA_ARCH="aarch64" ;; \
*) echo "Unsupported TARGETARCH: $TARGETARCH" >&2; exit 1 ;; \
esac && \
- wget --quiet "https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-${MINICONDA_ARCH}.sh" -O /tmp/miniconda.sh && \
+ : "${MINICONDA_VERSION:?Set MINICONDA_VERSION}" && \
+ : "${MINICONDA_SHA256:?Set MINICONDA_SHA256}" && \
+ INSTALLER="Miniconda3-${MINICONDA_VERSION}-Linux-${MINICONDA_ARCH}.sh" && \
+ wget --quiet "https://repo.anaconda.com/miniconda/${INSTALLER}" -O /tmp/miniconda.sh && \
+ echo "${MINICONDA_SHA256} /tmp/miniconda.sh" | sha256sum -c - && \
/bin/bash /tmp/miniconda.sh -b -p /opt/conda && \
rm /tmp/miniconda.sh && \
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
ARG TARGETARCH | |
RUN case "$TARGETARCH" in \ | |
amd64) MINICONDA_ARCH="x86_64" ;; \ | |
arm64) MINICONDA_ARCH="aarch64" ;; \ | |
*) echo "Unsupported TARGETARCH: $TARGETARCH" >&2; exit 1 ;; \ | |
esac && \ | |
wget --quiet "https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-${MINICONDA_ARCH}.sh" -O /tmp/miniconda.sh && \ | |
ARG TARGETARCH | |
# Pin Miniconda installer in CI (set these via build args or here) | |
ARG MINICONDA_VERSION # e.g., py311_24.7.1-0 | |
ARG MINICONDA_SHA256 # corresponding sha256 | |
RUN case "$TARGETARCH" in \ | |
amd64) MINICONDA_ARCH="x86_64" ;; \ | |
arm64) MINICONDA_ARCH="aarch64" ;; \ | |
*) echo "Unsupported TARGETARCH: $TARGETARCH" >&2; exit 1 ;; \ | |
esac && \ | |
: "${MINICONDA_VERSION:?Set MINICONDA_VERSION}" && \ | |
: "${MINICONDA_SHA256:?Set MINICONDA_SHA256}" && \ | |
INSTALLER="Miniconda3-${MINICONDA_VERSION}-Linux-${MINICONDA_ARCH}.sh" && \ | |
wget --quiet "https://repo.anaconda.com/miniconda/${INSTALLER}" -O /tmp/miniconda.sh && \ | |
echo "${MINICONDA_SHA256} /tmp/miniconda.sh" | sha256sum -c - && \ | |
/bin/bash /tmp/miniconda.sh -b -p /opt/conda && \ | |
rm /tmp/miniconda.sh && \ |
🛠️ Refactor suggestion
Good TARGETARCH mapping; add a safe default for local builds.
Buildx injects TARGETARCH, but plain docker build won’t. Default to amd64 to keep local builds working.
-ARG TARGETARCH
-RUN case "$TARGETARCH" in \
+ARG TARGETARCH
+# Default TARGETARCH for local non-buildx builds
+RUN : "${TARGETARCH:=amd64}" && \
+ case "$TARGETARCH" in \
amd64) MINICONDA_ARCH="x86_64" ;; \
arm64) MINICONDA_ARCH="aarch64" ;; \
*) echo "Unsupported TARGETARCH: $TARGETARCH" >&2; exit 1 ;; \
- esac && \
+ esac && \
wget --quiet "https://repo.anaconda.com/miniconda/Miniconda3-latest-Linux-${MINICONDA_ARCH}.sh" -O /tmp/miniconda.sh && \
🤖 Prompt for AI Agents
In python/conda/Dockerfile.conda around lines 21 to 27, the build assumes
TARGETARCH is provided by buildx which breaks plain docker build locally; set a
safe default by declaring ARG TARGETARCH=amd64 (or add a fallback in the case
statement to treat empty/unknown as amd64) so local builds default to amd64 and
the existing architecture mapping and wget URL resolution continue to work.
Build base and Python images for arm64 so they can be used on M-series MacBooks without emulation (Rosetta)
Summary by CodeRabbit
New Features
Chores