-
Notifications
You must be signed in to change notification settings - Fork 7
ci: use multiarch runners for docker builds #379
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
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.
Thanks for this! Just had a few comments which could all be due to my lack of understanding 😁
A general q: should we wait until grafana/shared-workflows#1348 is merged? As far as I can see we could use a reusable workflow since we don't do anything fancy...
| uses: grafana/shared-workflows/actions/docker-import-digests-push-manifest@cd422befbbda65e0612a63627e8c8820d86bc2a6 # docker-import-digests-push-manifest/v0.1.0 | ||
| with: | ||
| images: ${{ needs.build-and-export-digest.outputs.images }} | ||
| push: false |
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.
Is push: false right?
| needs: build-and-export-digest | ||
| permissions: | ||
| contents: read | ||
| id-token: write |
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.
Is this needed?
| registries: "dockerhub" | ||
| platforms: ${{ matrix.platform }} | ||
| outputs: "type=image,push-by-digest=true,name-canonical=true,push=false" | ||
| push: false |
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.
Is it right that we're never pushing? Perhaps I'm understanding the flag incorrectly
| digest: ${{ steps.build.outputs.digest }} | ||
| platform: ${{ matrix.platform }} | ||
|
|
||
| push-manifest: |
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.
Could this be conditional on whether we're pushing or not? We could skip the job on PRs, AFAIK
This pull request refactors the Docker build workflow in
.github/workflows/build.ymlto improve support for multi-architecture images and streamline digest handling. The workflow now builds images for each platform separately, exports their digests, and then constructs a multi-arch manifest using those digests.Multi-Architecture Build and Digest Export:
mainjob with abuild-and-export-digestjob that uses a matrix strategy to build Docker images for bothlinux/arm64andlinux/amd64platforms, and exports their digests for later use.build-push-to-dockerhubaction to the newerdocker-build-push-imageaction.docker-export-digestaction.Manifest Construction:
push-manifestjob that depends on the completion of the multi-arch builds, and uses thedocker-import-digests-push-manifestaction to construct and upload a manifest referencing the built images for each architecture.