-
Notifications
You must be signed in to change notification settings - Fork 7
CI: Separate builds with native runners #277
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
Conversation
This avoids emitting lines such as
[None] booting buildkit
[None] load build definition from Dockerfile
Preparing to add a matrix to the build job. Variables should only be set once and can be done outside of the matrix.
Instead of using one long-running job to build 6 images (3 stages x 2 platforms) and 3 manifest lists, use 2 platform-specific jobs to build 3 images (one per stage) and a third job to build the manifest lists. This change comes with two functional improvements: 1. Most significantly, per-runner disk space usage and build times go down. This is due to both parallelizing the build per platform and removing emulation for the linux/arm64 build. The new time bottleneck for the workflow is now the linux/arm64 test job, which allows for an easy follow-up improvement (see added comment). 2. As a byproduct of the platform-specific images being tagged, they can now be retrieved from GitHub REST API results, removing the need to fetch from GHCR's undocumented Docker Registry API.
9b4087e to
52477eb
Compare
joverlee521
left a comment
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.
Nice! Can't say I fully understand the details, but does seem like the added bit of complexity is worth it.
| - name: Create base-builder-build-platform image | ||
| run: | | ||
| docker buildx imagetools create \ | ||
| -t "ghcr.io/nextstrain/base-builder-build-platform:$TAG" \ | ||
| "ghcr.io/nextstrain/base-builder-build-platform:$TAG-amd64" \ | ||
| "ghcr.io/nextstrain/base-builder-build-platform:$TAG-arm64" | ||
| - name: Create base-builder-target-platform image | ||
| run: | | ||
| docker buildx imagetools create \ | ||
| -t "ghcr.io/nextstrain/base-builder-target-platform:$TAG" \ | ||
| "ghcr.io/nextstrain/base-builder-target-platform:$TAG-amd64" \ | ||
| "ghcr.io/nextstrain/base-builder-target-platform:$TAG-arm64" | ||
| - name: Create base image | ||
| run: | | ||
| docker buildx imagetools create \ | ||
| -t "ghcr.io/nextstrain/base:$TAG" \ | ||
| "ghcr.io/nextstrain/base:$TAG-amd64" \ | ||
| "ghcr.io/nextstrain/base:$TAG-arm64" |
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.
non-blocking
The general pattern in this repo seems to run docker buildx commands in /devel/ scripts, should these steps be wrapped in a separate script?
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.
Good point, I'll keep it as is for a couple reasons:
- The README instructions for building locally using the existing
develscripts still work fine. The split build +merge-buildspattern is an optimization specific to the GitHub Actions workflow. - Since these commands are simple, keeping them inline and split across 3 steps feels more readable, both in the code and on the GitHub run page. If it ever gets more complex, we can move to a
develwrapper script.
|
Merging, but happy to walk through details in a dev chat session! |
Description of proposed changes
This PR contains two prep commits and one main commit. Message from main commit:
Instead of using one long-running job to build 6 images (3 stages x 2 platforms) and 3 manifest lists, use 2 platform-specific jobs to build 3 images (one per stage) and a third job to build the manifest lists.
This change comes with two functional improvements:
Most significantly, per-runner disk space usage and build times go down. This is due to both parallelizing the build per platform and removing emulation for the linux/arm64 build. The new time bottleneck for the workflow is now the linux/arm64 test job, which allows for an easy follow-up improvement (see added comment).
As a byproduct of the platform-specific images being tagged, they can now be retrieved from GitHub REST API results, removing the need to fetch from GHCR's undocumented Docker Registry API.
Related issue(s)
Checklist
Checks pass
linux/arm64image works on M1 macTested with
zika-tutorial:- [ ] Update changelog