-
Notifications
You must be signed in to change notification settings - Fork 18
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
WIP: add multi-arch support #179
base: master
Are you sure you want to change the base?
Conversation
8e16dc0
to
60093df
Compare
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.
First of all, welcome! and thank you for looking into this. We really appreciate it.
This is a very large single change and that makes it hard for me to follow the flow and intent of the changes, I would appreciate it you could break it into smaller pieces.
Next, I am a bit confused with regards to the bulk of the changes that seem to be removing parts of the code with regards to architecture. One of the changes that I made to this script in preparation for eventually having multi-arch builds was to have a "fully-qualify image name" in which the tag reflected the base distro image, the architecture of the image, and some ofther information. It's a bit hard for me to tell but it appears the change is stripping the architecture out of that. I'd rather ensure that prior to combining images with a manifest we have unique ids for all the builds. We may want the option of building on a native host first, and I'm pretty sure we want the CI jobs that could build arm64 with emulation to run in separate "jobs" so that we can get fast feedback on the PRs but then enable all arches on push.
What do you think?
After reading my own comment I was wondering how clear I was about the purpose and value of the "FQINs" (fully qualified image names). My thought is that the script should produce images such as:
either using emulation locally OR by importing the images from other build jobs I hope that's clearer than my earlier comment. |
First of all, thank you for the quick feedback.
I haven't created a multi-arch image before, so I just started working on it and, by the end of the day, I pushed my changes in a single commit. I also wanted to start an early discussion, which is why I created the WIP PR so early. I initially thought building all architectures with one command would be less complex, but I understand your points. Breaking it down would also make debugging easier if any issues arise during the build. When using the Podman manifest for building the multi-arch image, should we switch to Docker in the GitHub Action Workflow? I think Docker is currently being used, and it feels off to build the individual images with Docker but the multi-arch image with Podman. To implement the changes in smaller independent increments, we could split the work into multiple PRs:
What do you think? |
Not a problem. I'm pleased to see you stepping in and trying. :-)
Thanks, we can certainly automate the front-end some more for people who build manually vs on the CI. But keeping the low level stuff separated is cleaner.
Sorry, I use podman on my systems and I tend to think in podman-ese. Most places where you see me say podman, you can usually substitute Docker or $COMPATIBLE_CONTAINER_ENGINE_OF_YOUR_CHOICE :-)
We can switch or we can stay as-is. We just need to make sure the scripts invoke the correct underlying tools. As I noted above I'm most familiar with podman but I have no issue using docker in the CI as long as we can accomplish the same procedures.
Sounds reasonable.
I'm happy see the work split up. You can split things into multiple commits or multiple PRs (or both!). If you want to use this PR just to extend the script(s) to support multi-arch builds (through emulation I assume) and assembling them (only), But keep the changes small and incremental and we can discuss each piece as needed! |
I run an ARM-based Kubernetes cluster and wanted to integrate the samba-operator, but unfortunately, there is no ARM support at the moment. I saw that @synarete already started working on it (samba-in-kubernetes/samba-operator#301), but there hasn't been any progress since April 2024. So, I decided to step in and start working on it to make some progress.
The
--arch
option from the Pythonbuild-image
script has been removed, as well as the architecture name from the container image tag, since the image is now always built for all platforms.The
container_build
function has been extended to support building multi-arch images withdocker
andpodman
. Building the images withdocker
seems to work, while building withpodman
failes when running theinstall-packages.sh
script.Open topics:
docker-buildx
in GitHub Actionspodman
buildI'll try to continue working on this over the next few days/weeks, but time is always limited. So, if anyone wants to step in, feel free ;)