-
Notifications
You must be signed in to change notification settings - Fork 59
build(container): Build multi-arch images for release #744
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
Signed-off-by: jannfis <[email protected]>
📝 WalkthroughWalkthroughThe release script was updated to replace single-architecture image builds with multi-architecture builds using the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
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: 2
🤖 Fix all issues with AI agents
In `@hack/release.sh`:
- Line 78: The podman branch in MULTIARCH_BUILD_CMDS is invalid because it uses
DOCKER_BIN build with --manifest; update the podman path so it loops
IMAGE_MULTIARCH_PLATFORMS, builds each platform image individually (using
DOCKER_BIN or podman build with --arch/--os flags), then creates and pushes a
manifest (podman manifest create <manifest> and podman manifest add <manifest>
<platform-image> for each built image) and finally pushes the manifest; modify
the Makefile rule(s) referenced by MULTIARCH_BUILD_CMDS and the image-multiarch
target to implement per-platform builds and manifest create/add steps instead of
a single --manifest build invocation.
- Around line 90-94: The release instructions incorrectly suggest using a
separate "docker push ${IMAGE_REPOSITORY}/argocd-agent:${TARGET_TAG}" after a
buildx build; because buildx stores multi-arch images in the builder cache,
update the instructions to require using "docker buildx build --push ..." (i.e.,
push during build) or explicitly tell maintainers to run the build with --push
instead of a subsequent docker push; keep the existing podman manifest path but
replace the standalone docker push guidance with a buildx --push recommendation
referencing IMAGE_REPOSITORY and TARGET_TAG.
| echo "(if built with podman):" | ||
| echo "podman manifest push ${IMAGE_REPOSITORY}/argocd-agent:${TARGET_TAG}" | ||
| echo | ||
| echo "(or if built with docker buildx):" | ||
| echo "docker push ${IMAGE_REPOSITORY}/argocd-agent:${TARGET_TAG}" |
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.
🧩 Analysis chain
🏁 Script executed:
find . -name "Makefile*" -type f | head -20Repository: argoproj-labs/argocd-agent
Length of output: 104
🏁 Script executed:
rg -n -A 15 'image-multiarch' --type makefileRepository: argoproj-labs/argocd-agent
Length of output: 102
🏁 Script executed:
rg -n -A 15 'image-multiarch' ./MakefileRepository: argoproj-labs/argocd-agent
Length of output: 623
🏁 Script executed:
rg -n -B 5 -A 15 'buildx' ./Makefile | head -60Repository: argoproj-labs/argocd-agent
Length of output: 1048
🏁 Script executed:
sed -n '75,105p' hack/release.shRepository: argoproj-labs/argocd-agent
Length of output: 717
Docker buildx multi-arch images cannot be pushed with a separate docker push command—the workflow is broken.
When docker buildx build runs without the --push flag (as shown in the Makefile line 14), the multi-arch images are built into the builder cache, not loaded into the local Docker daemon. A subsequent docker push will fail because the image isn't available locally.
The correct approach is to add --push to the buildx invocation so images are pushed during the build step, or update the release instructions to use docker buildx build --push in a single command. The podman path (podman manifest push) is correct, but the docker buildx instructions need to be fixed.
🤖 Prompt for AI Agents
In `@hack/release.sh` around lines 90 - 94, The release instructions incorrectly
suggest using a separate "docker push
${IMAGE_REPOSITORY}/argocd-agent:${TARGET_TAG}" after a buildx build; because
buildx stores multi-arch images in the builder cache, update the instructions to
require using "docker buildx build --push ..." (i.e., push during build) or
explicitly tell maintainers to run the build with --push instead of a subsequent
docker push; keep the existing podman manifest path but replace the standalone
docker push guidance with a buildx --push recommendation referencing
IMAGE_REPOSITORY and TARGET_TAG.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #744 +/- ##
==========================================
+ Coverage 46.30% 46.33% +0.03%
==========================================
Files 101 101
Lines 12480 12480
==========================================
+ Hits 5779 5783 +4
+ Misses 6146 6141 -5
- Partials 555 556 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hey Jann, I looked into the stuff we discussed relating this today. It seems that Docker buildx can place the built image in the local Docker image list and it can just be pushed with the push command so I'm thinking the release script can just call a new Makefile target for pushing multi arch images and just have it be like the build one where it picks based on binary except this time do it for podman because that has the special steps in push manifest. In terms of the builder problem, I think it's best to just remove the create builder and remove builder commands and add the WDYT? |
What does this PR do / why we need it:
Build multi-arch image for releases and update instructions.
Which issue(s) this PR fixes:
Fixes #?
How to test changes / Special notes to the reviewer:
Checklist
Summary by CodeRabbit