Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abachmann
Copy link

@abachmann abachmann commented Sep 8, 2024

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 Python build-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 with docker and podman. Building the images with docker seems to work, while building with podman failes when running the install-packages.sh script.

Open topics:

  • Install docker-buildx in GitHub Actions
  • Fix podman build
  • Test container image builds with docker and podman
  • Cleanup

I'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 ;)

@abachmann abachmann changed the title WIP add multi-arch support WIP: add multi-arch support Sep 8, 2024
Copy link
Collaborator

@phlogistonjohn phlogistonjohn left a 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?

@phlogistonjohn
Copy link
Collaborator

phlogistonjohn commented Sep 9, 2024

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:

  • samba-server:default-fedora-amd64
  • samba-server:default-fedora-arm64
  • samba-server:default-fedora-ppc64le

either using emulation locally OR by importing the images from other build jobs
and then combining them with a manifest - possibly with a separate command.

I hope that's clearer than my earlier comment.

@abachmann
Copy link
Author

First of all, thank you for the quick feedback.

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.

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:

  1. Switch from Docker to Podman in GitHub Actions
  2. Extend the GitHub Action Workflow to build architectures other than amd64
  3. Build and push the actual multi-arch image (this can be done in this PR)

What do you think?

@phlogistonjohn
Copy link
Collaborator

First of all, thank you for the quick feedback.

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.

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.

Not a problem. I'm pleased to see you stepping in and trying. :-)

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.

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.

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.

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 :-)

To implement the changes in smaller independent increments, we could split the work into multiple PRs:

1. Switch from Docker to Podman in GitHub Actions

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.

2. Extend the GitHub Action Workflow to build architectures other than amd64

Sounds reasonable.

3. Build and push the actual multi-arch image (this can be done in this PR)

What do you think?

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),
that's fine with me. One word of caution is that you should feel free to modify but not break the current workflows as I use them to build test images locally (a lot).

But keep the changes small and incremental and we can discuss each piece as needed!
Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants