Build multiarch (amd64 and arm64) images #1512
Conversation
|
Hi, thanks for the PR. Can you please rebase it? I think after that all the tests would start passing. |
zregvart
force-pushed
the
pr/multiarch-build
branch
from
403a7fe to
a666980
Compare
Makes changes to the scripts in build/tools to build and push multiarch images. Now runs two `docker build` commands to build images for individual architectures, and an additional `docker manifest create` to create the multiarch manifest. When pushing `docker manifest push` is used instead of `docker push`. Signed-off-by: Zoran Regvart <[email protected]>
liranmauda
force-pushed
the
pr/multiarch-build
branch
from
a666980 to
bf4c9a2
Compare
WalkthroughIntroduces multi-architecture OCI image builds in builder.sh by parameterizing build_oci with OS/arch, adding build_oci_for_all to build amd64/arm64 images and create a versioned manifest, and updating main to use it. In releaser.sh, replaces pushing images with pushing the multi-arch manifest. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer/CI
participant B as builder.sh
participant D as Docker CLI/Buildx
participant R as Registry
Dev->>B: invoke build_oci_for_all(repository)
B->>D: build_oci(tag_prefix, linux, amd64)
D-->>R: push image linux/amd64
B->>D: build_oci(tag_prefix, linux, arm64)
D-->>R: push image linux/arm64
B->>D: manifest create version tag
D-->>R: push manifest (refs amd64/arm64 images)
sequenceDiagram
participant Rel as releaser.sh
participant D as Docker CLI
participant R as Registry (Docker/Quay)
Rel->>D: docker manifest push repository/noobaa-operator:<version>
D-->>R: upload multi-arch manifest
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
build/tools/releaser.sh (1)
193-211: Retag and push per-architecture images before creating and pushing manifest listsThe current releaser.sh only runs
docker manifest pushfor the combined image tags, but never pushes the individual per-arch (“linux-amd64-…”, “linux-arm64-…”) images. Since the manifest list references those leaf images, the push will fail unless they exist in the registry.Please update build/tools/releaser.sh (around lines 193–211) as follows:
- Capture the version and repo roots:
version="$(get_noobaa_version)" docker_repo="$DOCKERHUB_USERNAME/noobaa-operator" quay_repo="quay.io/$QUAY_USERNAME/noobaa-operator"- For each registry, loop over architectures to tag and push the leaf images:
for arch in amd64 arm64; do src="$OCI_ORG/noobaa-operator:linux-${arch}-${version}" dst="${docker_repo}:linux-${arch}-${version}" docker tag "$src" "$dst" docker push "$dst" done- Create and push the manifest list for each registry:
docker manifest create "${docker_repo}:${version}" \ --amend "${docker_repo}:linux-amd64-${version}" \ --amend "${docker_repo}:linux-arm64-${version}" docker manifest push "${docker_repo}:${version}" # Repeat the same for ${quay_repo}Proposed diff snippet:
- local quay_image="quay.io/$QUAY_USERNAME/noobaa-operator:$(get_noobaa_version)" - local docker_image="$DOCKERHUB_USERNAME/noobaa-operator:$(get_noobaa_version)" + version="$(get_noobaa_version)" + docker_repo="$DOCKERHUB_USERNAME/noobaa-operator" + quay_repo="quay.io/$QUAY_USERNAME/noobaa-operator" - echo "Tagging the images..." - docker tag "$OCI_ORG/noobaa-operator:${version}" $quay_image - docker tag "$OCI_ORG/noobaa-operator:${version}" $docker_image + echo "Tagging and pushing per-arch images to Docker Hub..." + for arch in amd64 arm64; do + src="$OCI_ORG/noobaa-operator:linux-${arch}-${version}" + dst="${docker_repo}:linux-${arch}-${version}" + docker tag "$src" "$dst" + docker push "$dst" + done echo "Logging in to docker.io..." echo "$DOCKERHUB_TOKEN" | docker login -u "$DOCKERHUB_USERNAME" --password-stdin - echo "Pushing the images to docker.io..." - docker manifest push $docker_image + echo "Creating and pushing multi-arch manifest to Docker Hub..." + docker manifest create "${docker_repo}:${version}" \ + --amend "${docker_repo}:linux-amd64-${version}" \ + --amend "${docker_repo}:linux-arm64-${version}" + docker manifest push "${docker_repo}:${version}" echo "Logging in to quay.io..." echo "$QUAY_TOKEN" | docker login -u "$QUAY_USERNAME" --password-stdin quay.io - echo "Pushing the images to quay.io..." - docker manifest push $quay_image + echo "Tagging and pushing per-arch images to Quay.io..." + for arch in amd64 arm64; do + src="$OCI_ORG/noobaa-operator:linux-${arch}-${version}" + dst="${quay_repo}:linux-${arch}-${version}" + docker tag "$src" "$dst" + docker push "$dst" + done + + echo "Creating and pushing multi-arch manifest to Quay.io..." + docker manifest create "${quay_repo}:${version}" \ + --amend "${quay_repo}:linux-amd64-${version}" \ + --amend "${quay_repo}:linux-arm64-${version}" + docker manifest push "${quay_repo}:${version}"
🧹 Nitpick comments (3)
build/tools/builder.sh (3)
106-110: Quote expansions and address ShellCheck warnings; ensure cross-arch builds are reproducible
- SC2046: Quote command substitutions and variables to avoid word-splitting.
- Building for a non-native arch with plain
docker build --platformrelies on BuildKit/binfmt emulation being available. If not guaranteed in CI, this may silently produce host-arch images. Consider verifying BuildKit is enabled and/or usingdocker buildx build.Patch:
- docker build --platform $os/$arch --build-arg NOOBAA_BIN_PATH=$(generate_full_bin_path $os $arch) -t $tag_prefix/noobaa-operator:${os}-${arch}-$(get_noobaa_version) -f build/Dockerfile . + docker build \ + --platform "${os}/${arch}" \ + --build-arg NOOBAA_BIN_PATH="$(generate_full_bin_path "$os" "$arch")" \ + -t "${tag_prefix}/noobaa-operator:${os}-${arch}-$(get_noobaa_version)" \ + -f build/Dockerfile .If BuildKit/buildx is available, prefer:
- docker buildx create/use
- docker buildx build --platform linux/amd64,linux/arm64 --push ...
Ensure CI runners have binfmt_misc set up for arm64 when running on amd64 hosts, or switch to buildx with QEMU baked in.
117-121: Avoid masking return values; assign local version separatelySC2155: Declare and assign separately for clarity and to avoid masking return values in subshell failure cases.
- local version=$(get_noobaa_version) + local version + version="$(get_noobaa_version)"
112-121: Create manifest close to where images are pushed, or push images hereThis function creates a local manifest referencing tags under "${repository}" but does not push the per-arch images nor the manifest. Since releaser.sh pushes manifests to user/org-specific repos, the manifest should be created per target repo after retagging and pushing the per-arch images (see releaser.sh review). Alternatively, push the per-arch images and the manifest here to "${repository}" and have releaser mirror across repos.
Suggested direction:
- Either remove the manifest creation here and move it entirely to releaser.sh per target repo, or
- Add optional push logic here (docker push per-arch tags, then docker manifest push) gated behind a flag.
Confirm desired source of truth for manifests: built once under "${OCI_ORG}" and mirrored, or created separately per target repository.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
build/tools/builder.sh(2 hunks)build/tools/releaser.sh(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
build/tools/builder.sh (1)
build/tools/utils.sh (1)
get_noobaa_version(44-51)
🪛 Shellcheck (0.10.0)
build/tools/builder.sh
[warning] 109-109: Quote this to prevent word splitting.
(SC2046)
[warning] 109-109: Quote this to prevent word splitting.
(SC2046)
[warning] 117-117: Declare and assign separately to avoid masking return values.
(SC2155)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: run-azure-vault-test
- GitHub Check: run-kms-kmip-test
- GitHub Check: cnpg-deployment-test
- GitHub Check: run-kms-tls-token-test
- GitHub Check: run-cli-tests
- GitHub Check: run-kms-key-rotate-test
- GitHub Check: run-hac-test
- GitHub Check: run-operator-tests
- GitHub Check: run-kms-dev-test
- GitHub Check: run-core-config-map-tests
- GitHub Check: run-admission-test
- GitHub Check: run-kms-tls-sa-test
🔇 Additional comments (1)
build/tools/builder.sh (1)
223-224: Good: switching main flow to build multi-arch artifactsUsing build_oci_for_all aligns the build with the new multi-arch release flow.
|
@tangledbytes Could you take a look? |
Explain the changes
docker buildcommands to build images for individual architectures, and an additionaldocker manifest createto create the multiarch manifest. When pushingdocker manifest pushis used instead ofdocker push.Issues: Fixed #xxx / Gap #xxx
Testing Instructions:
Summary by CodeRabbit
New Features
Chores