-
Notifications
You must be signed in to change notification settings - Fork 7
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(docker): build multiplatform images on push to main or workflow dispatch #22
Conversation
.github/workflows/ci.yml
Outdated
to: ${{ secrets.NOTIFY_EMAIL_TO }} | ||
password: ${{ secrets.NOTIFY_EMAIL_PASSWORD }} | ||
context: . | ||
platforms: linux/amd64,linux/arm64/v8 |
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.
Why not have a single unconditional docker/build-push-action
step, and make the platforms
value based on the event ?
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.
I hesitated before due to my lack of familiarity with buildx --platform
, but after reading the docs I'll do that.
.github/workflows/ci.yml
Outdated
if: failure() && github.event_name != 'pull_request' | ||
uses: ./.github/actions/notify-status | ||
|
||
- name: Set up QEMU for cross-platform builds |
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.
what would it take to use the M1 runner? https://github.blog/2023-10-02-introducing-the-new-apple-silicon-powered-m1-macos-larger-runner-for-github-actions/
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.
That runner is a bit of a fortune. Though not sure how much a large runner in emulation mode would cost.
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.
We only need to build with it once per passed proposal. The announcement says $0.12/minute so $10 buys 83 minutes. That's probably enough.
Of course if QEMU works fine and it's only for main
, then waiting a long time is not so bad.
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.
I see that the non-main
and non-workflow_dispatch
case doesn't take long to build. @turadg or @mhofman, do you know why I don't see any way to trigger the workflow_dispatch
from Github UI? I'm not an admin on this repo, FWIW.
If somebody with privs could issue a .github/workflows/ci.yml
workflow_dispatch
for this mfig-multiplatform-docker-main
branch, that would be great as a comparison with this PR's single-platform build.
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.
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.
GH only adds the trigger UI once the workflow has workflow_dispatch
in master, aka it should show up once this PR merges.
3150c00
to
8d53e38
Compare
8d53e38
to
faa4c5f
Compare
Do any of you want to approve? After that, it should be possible to merge to |
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.
I'd prefer to know it works before merging, but accept the risk since it's easy to fix if it doesn't.
Publish multiplatform images for main branch pushes.