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

ci: split arm64 and amd64 build steps for faster compilation #98

Merged
merged 7 commits into from
Oct 20, 2023

Conversation

thalesmg
Copy link
Contributor

@thalesmg thalesmg commented Oct 18, 2023

@id
Copy link
Collaborator

id commented Oct 18, 2023

I do not think this will work, docker image names do not contain architecture, they will conflict with each other:

- uses: docker/metadata-action@v4
  id: base_meta
  with:
    images: ghcr.io/${{ github.repository }}/base-${{ matrix.base_image_vsn }}
    tags: type=raw,value=${{ matrix.platform[0] }}

Here in the image tag we put matrix.platform[0] which is OS name only, like ubuntu22.04.

In fact, I'm not sure how this will work exactly. Best guess - last write wins. I.e. if arm64 build finishes after amd64, the resulting image will be the one for arm64 architecture.

To make use of arm64 runners we need something like this: https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners

@thalesmg thalesmg force-pushed the ci-split-arm64-amd64 branch 2 times, most recently from e42f4a1 to 3008289 Compare October 19, 2023 17:28
.github/workflows/base.yaml Outdated Show resolved Hide resolved
@thalesmg thalesmg force-pushed the ci-split-arm64-amd64 branch 3 times, most recently from 8d8a149 to c3599a3 Compare October 19, 2023 18:28
@thalesmg
Copy link
Contributor Author

thalesmg commented Oct 19, 2023

ubuntu16.04 is not present on the public.ecr.aws mirror, so I had to leave it in docker hub.

@thalesmg thalesmg marked this pull request as ready for review October 19, 2023 19:26
.github/workflows/main.yaml Outdated Show resolved Hide resolved
.github/workflows/test.yaml Outdated Show resolved Hide resolved
@thalesmg thalesmg force-pushed the ci-split-arm64-amd64 branch 3 times, most recently from b15a0be to 380676d Compare October 19, 2023 22:07
@thalesmg
Copy link
Contributor Author

after this is merged, I'll tag 5.2-3 and update the builder image in emqx

@thalesmg thalesmg requested review from id and zmstone October 19, 2023 23:00
Copy link
Collaborator

@id id left a comment

Choose a reason for hiding this comment

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

💯

@id id merged commit 9c0709e into main Oct 20, 2023
173 checks passed
@id id deleted the ci-split-arm64-amd64 branch October 20, 2023 13:03
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