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

Introduce multi-arch builds with support for ARM #578

Merged
merged 1 commit into from
May 4, 2023

Conversation

aiven-anton
Copy link
Contributor

@aiven-anton aiven-anton commented Apr 9, 2023

About this change - What it does

With this commit we start building multi-arch container images to enable running containers on ARM systems.

Closes #521.

References #585.

Why this way

I did experimentation/debugging in this repository: https://github.com/aiven-anton/test-build-arm, see results of build 12 and 13.

Using the Docker actions makes for very low amount of boilerplate code for building containers.

@aiven-anton aiven-anton marked this pull request as ready for review April 9, 2023 13:25
@aiven-anton aiven-anton requested review from a team as code owners April 9, 2023 13:25
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 9, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4d2c0d9
Status: ✅  Deploy successful!
Preview URL: https://7ff45e32.karapace.pages.dev
Branch Preview URL: https://chore-build-arm-container.karapace.pages.dev

View logs

Copy link
Contributor

@tvainika tvainika left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect this does not work.

Sadly our container builds from GitHub Actions are triggered only on main branch. Therefore no container CI action was triggered by this pull request. Therefore I pushed this branch into my own fork's main branch, and this resulted build failure https://github.com/tvainika/karapace/actions/runs/4675463034

I think build failure is because Gather context step does not have all tags with fetched history. Actions/checkout needs fetch-depth flag to fetch all tags.

@aiven-anton
Copy link
Contributor Author

Sadly our container builds from GitHub Actions are triggered only on main branch.

Perhaps I should start in this end, building an image for PRs and adding some very basic smoke test. That would probably make it easier to gain confidence for a PR like this one later on. Very good idea that you tried this out on a fork! 🙏

The failure is a bit odd though, the $(git describe --always) from the "Gather context" step was taken as-is from the current "Build and push container image" step, and checkout hasn't changed so I think if git history/tags is too shallow now, it must have been before too. I think what's triggering this is the explicit passing of KARAPACE_VERSION, which means Karapace is no longer attempting to gather the version from git itself. Inspecting the line where it used to, it's clear that's a different invocation than in "Gather context": it has --tags.

I'll put this PR in draft for now, and will try to come back to it once we have smoke tests 🙂

@aiven-anton aiven-anton marked this pull request as draft April 12, 2023 07:52
@aiven-anton aiven-anton force-pushed the chore/build-arm-container branch 2 times, most recently from 9cc3247 to 4d7084c Compare May 3, 2023 12:42
With this commit we start building multi-arch container images to enable
running containers on ARM systems.

Delete outdated build and publish scripts under container/.
@aiven-anton
Copy link
Contributor Author

@tvainika I managed to get the build working on my fork now, I did these things:

  • Make sure there are actually tags in the repository (did not get cloned by default).
  • Set fetch-depth: 0 as you suggested.
  • Add --tags to git describe invocation.

Passing build: https://github.com/aiven-anton/karapace/actions/runs/4872183301/jobs/8690049653

@aiven-anton aiven-anton marked this pull request as ready for review May 3, 2023 15:27
@aiven-anton aiven-anton requested a review from tvainika May 4, 2023 06:50
@tvainika tvainika merged commit 70957d5 into main May 4, 2023
@tvainika tvainika deleted the chore/build-arm-container branch May 4, 2023 08:24
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.

Provide images for the arm64 architecture
2 participants