samba-container icon indicating copy to clipboard operation
samba-container copied to clipboard

build/push arm64 container images

Open abachmann opened this issue 1 year ago • 4 comments

Extends the GA workflow to build and push ARM64 container images. The build-image script has been modified to use Docker Buildx for cross-building.

Preparation for: https://github.com/samba-in-kubernetes/samba-container/pull/179 Fixes: https://github.com/samba-in-kubernetes/samba-container/issues/155

abachmann avatar Sep 13 '24 16:09 abachmann

@phlogistonjohn do you have time to look into this topic? It would be nice to see if the pipeline is running with my changes.

abachmann avatar Sep 17 '24 15:09 abachmann

I am currently traveling and dont have a lot of time too look, but I will allow the CI jobs to run!

phlogistonjohn avatar Sep 17 '24 18:09 phlogistonjohn

@obnoxxx Thanks for the review. I have added a message body to all commits.

I found a potential reason for the pipeline failure: https://github.com/samba-in-kubernetes/samba-container/pull/180/commits/bd8e932a1ef052a37e171fba645674cab80e7a04

If it's okay to remove the arch parameter, I can squash the following commits:

  • https://github.com/samba-in-kubernetes/samba-container/pull/180/commits/bd8e932a1ef052a37e171fba645674cab80e7a04
  • https://github.com/samba-in-kubernetes/samba-container/pull/180/commits/b78b782be1e7394d49cfee3d10fc16e0fefbf9ae

abachmann avatar Sep 24 '24 20:09 abachmann

Things are getting a bit more interesting now. 49.85 - Status code: 404 for https://mirrors.centos.org/metalink?repo=centos-resilientstorage-9-stream&arch=aarch64&protocol=https,http (IP: 8.43.85.73)

The centos-resilientstorage-9-stream repository is only available for the following architectures:

  • ppc64le
  • s390x
  • x86_64

It’s also possible to set the arch parameter to source. This will increase the build time, but we could add a check: if the target architecture is not supported, then fall back to source. This way, the amd64 builds won’t be affected.

I checked the list of available repositories and their architecture support. A lot of them support aarch64, but not the resilientstorage repositories. So, if we start building the packages from source, we might run into issues.

Before I proceed, I’d like to get your thoughts on this. Maybe you're already aware of the issue and have some solutions in mind.

abachmann avatar Sep 25 '24 15:09 abachmann

@abachmann, any updates here?

We'd really like to take the changes, but please address the requests.

obnoxxx avatar Oct 14 '24 08:10 obnoxxx

@obnoxxx, sorry for the late response. I was busy with other things.

I didn't quite understand yet what makes it necessary to remove the explicit invocation of the arch from the workflows. I do prefer we keep testing arm builds in the ci.

Yeah, I initially thought the explicit invocation of the arch was causing the pipeline issue, but it turned out that wasn’t the case. So, I reverted the commit.

As I wrote in my last comment:

Things are getting a bit more interesting now. 49.85 - Status code: 404 for https://mirrors.centos.org/metalink?repo=centos-resilientstorage-9-stream&arch=aarch64&protocol=https,http (IP: 8.43.85.73)

The centos-resilientstorage-9-stream repository is only available for the following architectures:

  • ppc64le
  • s390x
  • x86_64

Addressing the pipeline issue isn't so easy. I'll try to find some time over the weekend to fix it.

abachmann avatar Oct 14 '24 16:10 abachmann

@abachmann wrote:

@obnoxxx, sorry for the late response. I was busy with other things.

sure. Thanks for following up! And sorry for nagging so badly!

I didn't quite understand yet what makes it necessary to remove the explicit invocation of the arch from the workflows. I do prefer we keep testing arm builds in the ci.

Yeah, I initially thought the explicit invocation of the arch was causing the pipeline issue, but it turned out that wasn’t the case. So, I reverted the commit.

Thanks for explaining. This makes more sense now.

As I wrote in my last comment:

Things are getting a bit more interesting now. 49.85 - Status code: 404 for https://mirrors.centos.org/metalink?repo=centos-resilientstorage-9-stream&arch=aarch64&protocol=https,http (IP: 8.43.85.73)

right. The centos repo mirrors do sometimes have issues with availability.

The centos-resilientstorage-9-stream repository is only available for the following architectures:

  • ppc64le
  • s390x
  • x86_64

Interesting.

Addressing the pipeline issue isn't so easy. I'll try to find some time over the weekend to fix it.

Thanks! Looking forward to seeing a solution.

obnoxxx avatar Oct 15 '24 18:10 obnoxxx

@obnoxxx

sure. Thanks for following up! And sorry for nagging so badly!

This topic was completely out of mind, so nagging was actually a good idea ;)

Over the weekend, I had some time to look more deeply into the issue where the resilientstorage repository was not found during the image build. As I mentioned before, no aarch64 packages are provided in the official repositories. So, I tried to build the packages from source. To do this, I modified the install-packeges.sh file as follows:

-     dnf_cmd+=(--enablerepo=crb --enablerepo=resilientstorage)
+     dnf_cmd+=(--enablerepo=crb --enablerepo=resilientstorage-source)

This resolved the 404 error, but I ran into another issue afterward: the ctdb package, which is provided by resilientstorage repository, was not installed. So, building the packages from source didn’t work as expected. Unfortunately, I wasn’t able to find another repository that provides the necessary aarch64 package.

If you know a repository which provides the missing package, please let me know. Otherwise, I would suggest to skip the centos arch64 images for for now, because all other OSes seem to work. I'll double-check it this week.

abachmann avatar Oct 22 '24 11:10 abachmann

I've had very little time to follow up with you on this but I did want to pop in and say that the idea to start with arm64 on the fedora based images alone would be fine - in fact, highly recommended.

FWIW, while I'm thinking about this topic, I would envision that the github actions build matrix have a task to build the arm64 images as an explicit include, for only fedora with default package source, similar to how we extend the build matrix for the devbuilds image: https://github.com/samba-in-kubernetes/samba-container/blob/a5244a3568ad70da0768f11cebeb5337ebe043d7/.github/workflows/container-image.yml#L53-L56

One reason is that is is clean (IMO) and another is that I'm 90% sure the nightly builds don't have arm64 versions either. We'd need to work with @anoopcs9 and make changes in the https://github.com/samba-in-kubernetes/samba-build repo before we could consume those nightly samba builds in the images.

phlogistonjohn avatar Oct 22 '24 14:10 phlogistonjohn

I wasn’t able to find another repository that provides the necessary arch64 package.

Both AlmaLinux and Rocky Linux have ctdb aarch64 packages - https://pkgs.org/search/?q=ctdb

nerijus avatar Oct 22 '24 17:10 nerijus

Finally, do we need to use docker for the container build, or can we use podman instead?

@obnoxxx, I believe building the images with podman should work as well, but I have not tested it.

Finally, the pipeline built the following Fedora arm64 images successfully:

  • server
  • ad-server
  • client

@phlogistonjohn, what about the toolbox image? This image is only used for testing, so we can skip it, right? If we need to build it, then we have an issue because the Dockerfile is using the latest tag, which points to the Fedora amd64 image.

abachmann avatar Oct 23 '24 19:10 abachmann

@phlogistonjohn, what about the toolbox image? This image is only used for testing, so we can skip it, right? If we need to build it, then we have an issue because the Dockerfile is using the latest tag, which points to the Fedora amd64 image.

I'm OK with skipping it for now. I'm in favor of moving forward in baby steps for this work :-)

phlogistonjohn avatar Oct 23 '24 21:10 phlogistonjohn

I created a request to add arm builds to samba-build: https://github.com/samba-in-kubernetes/samba-build/issues/63

Once we have that, using those RPMs for Arm containers should not be a problem

obnoxxx avatar Oct 24 '24 13:10 obnoxxx

I created a request to add arm builds to samba-build: samba-in-kubernetes/samba-build#63

Once we have that, using those RPMs for Arm containers should not be a problem

@abachmann , FYI:

RPM builds for ARM have now been implemented in the samba-build repo and er have first nightly builds from master and release 4.21.

https://artifacts.ci.centos.org/samba/pkgs/master/fedora/samba-nightly-master.repo https://artifacts.ci.centos.org/samba/pkgs/4.21/fedora/samba-nightly-4.21.repo

https://artifacts.ci.centos.org/samba/pkgs/master/centos/samba-nightly-master.repo https://artifacts.ci.centos.org/samba/pkgs/4.21/centos/samba-nightly-4.21.repo

This samba-container project already has mechanisms to use those custom yum repos.

obnoxxx avatar Oct 30 '24 17:10 obnoxxx

Because of the work @anoopcs9 did on those builds I learned that centos ci has real arm builders - I was not aware of this before. My suggestion is to drive this PR to completion but once it is done we should rethink how our "nightly" images are built. It may make way more sense to switch to the centos ci infrastructure. We can discuss the topic in more detail later but I don't think building up everything using github actions is ideal path forward when we have access to actual arm builders.

phlogistonjohn avatar Oct 30 '24 17:10 phlogistonjohn

@obnoxxx, thanks for the info. I’ll try to find some time over the weekend to take a closer look.

abachmann avatar Nov 06 '24 11:11 abachmann

@phlogistonjohn, I addressed the review concerns in a separate commit to ease the review process. If you're okay with these changes, I'd like to combine the commit hack: use buildx to cross-build images with docker and hack: rework container_build function into a single commit, as I don't see any benefit to keeping them separate. I also want to ensure we're all on the same page and fine with the current changes before I start working on the CentOS topic.

abachmann avatar Nov 06 '24 11:11 abachmann

@phlogistonjohn, @obnoxxx, I have a question: what about OpenSUSE images? I see that images are built for this OS, but they aren’t pushed. I was able to build OpenSUSE images locally for arm64 without any issues. What’s the plan for OpenSUSE?

abachmann avatar Nov 06 '24 12:11 abachmann

I'm in favor of starting small. Let's handle the fedora images and then move on to opensuse later. If they're easy enough to handle, I'm in favor of doing it.

Go ahead and squash commits as you see fit.

phlogistonjohn avatar Nov 06 '24 14:11 phlogistonjohn

I extended the GH workflow to build Fedora nightly images as well. For testing, I added a separate commit for the opensuse images to check if they are built in the pipeline. Should I create a new PR for the opensuse changes?

abachmann avatar Nov 21 '24 18:11 abachmann

@mergifyio rebase

phlogistonjohn avatar Nov 26 '24 14:11 phlogistonjohn

rebase

✅ Branch has been successfully rebased

mergify[bot] avatar Nov 26 '24 14:11 mergify[bot]

I have approved this PR and am prepared to merge it. I do want to let you know that @anoopcs9 and I are looking into switching the process we use to build the images that are pushed to quay.io to use centos CI because this platform has native arm64 machines. I don't want to invest too much more effort in doing cross-arch builds in github CI. I do want to continue doing the PR tests in github CI because of the nice feedback it provides.

See https://github.com/samba-in-kubernetes/samba-centosci/pull/73 for this initial effort.

phlogistonjohn avatar Nov 26 '24 14:11 phlogistonjohn

mergify rules require a 2nd approval (since this is coming from a contributor and not a maintainer) - @obnoxxx @anoopcs9 @gd - thanks! I'd like not to have to override mergify :-)

phlogistonjohn avatar Nov 26 '24 16:11 phlogistonjohn

@mergifyio rebase

phlogistonjohn avatar Nov 27 '24 15:11 phlogistonjohn

rebase

✅ Branch has been successfully rebased

mergify[bot] avatar Nov 27 '24 15:11 mergify[bot]

Totally bizarre - I swear the comment I wrote was on a ceph RP I had open in a completely different tab.

phlogistonjohn avatar Nov 27 '24 15:11 phlogistonjohn