-
Notifications
You must be signed in to change notification settings - Fork 191
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
GHA: Docker build refactor #6396
Changes from 46 commits
02498d2
7ab005a
29768ad
b141d5b
967b637
e64965a
afe8448
fc00eb0
0bada85
9c0c257
e16c127
f088c29
9491a2e
d470378
8a2941d
6c4fe7b
6d71fc0
ff0c49f
a9e6290
d8b979c
8f4e7cb
9f9b694
4c906c0
d9443f5
2fe8562
eb69696
a06b549
b7f8136
fad7c28
115b2f4
13f69c2
cc4bd58
c420452
608aff8
ab2ebd1
f5d125e
0130717
09931e9
57f209d
2f92c27
5d0c012
49a0b09
ec12d50
ed3621a
cb6f1fe
b53ff48
c9c5cd6
f0a9a94
bd9cc39
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
[pytest] | ||
minversion = 7.0 | ||
addopts = -ra -q | ||
addopts = -ra -q --strict-markers | ||
testpaths = | ||
tests |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,4 @@ | ||
docker | ||
pre-commit | ||
pytest | ||
requests | ||
tabulate | ||
pytest-docker | ||
docker-compose | ||
pyyaml<=5.3.1 | ||
docker~=7.0.0 | ||
pytest~=8.2.0 | ||
requests~=2.32.0 | ||
pytest-docker~=3.1.0 |
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,7 @@ name: continuous-integration-code | |
|
||
on: | ||
push: | ||
branches-ignore: [gh-pages] | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this intentional? Now the normal unit tests workflow will only run on merge to main right and no longer on push to branches? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The unit tests will still run on pull_request event. I made this change for this PR in particular since I was opening the pull request from origin, and so the status checks were all duplicated. This is what happens for PRs from forks as well, essentially wasting CI compute imo. So this change would only make a difference to you if you care about running tests on push before you open your PR. Given that you can always open a draft PR I feel like this use case is not as important, but I am happy to revert this in case you think differently. Thanks for spotting this! I totally forgot I did this here. Sorry about that. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that in most cases, having it run doubly is not ideal.
This is the critical point for me though. In many cases, I rely on running the CI on my fork. I don't really like the concept of draft PRs and so I rarely open them. If the work is not ready, there is rarely a reason to open a (draft) PR. The downside for me as a reviewer is that it adds a lot of noise. I look at the PR page often to see what needs to be done and when. Having a lot of open draft PRs where everytime I have to assess whether to consider or skip it, adds noise. So I don't really like the idea that users will be forced to open them just to get the CI to run. I could be convinced otherwise, but I think this discussion shows that we probably shouldn't sneak the change into this PR, so for now please remove it and I will merge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem, done. |
||
branches: [main] | ||
pull_request: | ||
branches-ignore: [gh-pages] | ||
paths-ignore: [docs/**] | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
name: Build Docker images and upload them to ghcr.io | ||
|
||
env: | ||
BUILDKIT_PROGRESS: plain | ||
|
||
on: | ||
workflow_call: | ||
inputs: | ||
runsOn: | ||
description: GitHub Actions Runner image | ||
required: true | ||
type: string | ||
platforms: | ||
description: Target platforms for the build (linux/amd64 and/or linux/arm64) | ||
required: true | ||
type: string | ||
outputs: | ||
images: | ||
description: Images identified by digests | ||
value: ${{ jobs.build.outputs.images }} | ||
|
||
jobs: | ||
build: | ||
name: ${{ inputs.platforms }} | ||
runs-on: ${{ inputs.runsOn }} | ||
timeout-minutes: 60 | ||
defaults: | ||
run: | ||
# Make sure we fail if any command in a piped command sequence fails | ||
shell: bash -e -o pipefail {0} | ||
|
||
outputs: | ||
images: ${{ steps.bake_metadata.outputs.images }} | ||
|
||
steps: | ||
|
||
- name: Checkout Repo ⚡️ | ||
uses: actions/checkout@v4 | ||
|
||
- name: Set up QEMU | ||
if: ${{ inputs.platforms != 'linux/amd64' }} | ||
uses: docker/setup-qemu-action@v3 | ||
|
||
- name: Set up Docker Buildx | ||
uses: docker/setup-buildx-action@v3 | ||
|
||
- name: Login to GitHub Container Registry 🔑 | ||
uses: docker/login-action@v3 | ||
with: | ||
registry: ghcr.io | ||
username: ${{ github.actor }} | ||
password: ${{ secrets.GITHUB_TOKEN }} | ||
|
||
- name: Build and upload to ghcr.io 📤 | ||
id: build | ||
uses: docker/bake-action@v4 | ||
with: | ||
push: true | ||
workdir: .docker/ | ||
# Using provenance to disable default attestation so it will build only desired images: | ||
# https://github.com/orgs/community/discussions/45969 | ||
provenance: false | ||
set: | | ||
*.platform=${{ inputs.platforms }} | ||
*.output=type=registry,push-by-digest=true,name-canonical=true | ||
*.cache-to=type=gha,scope=${{ github.workflow }},mode=max | ||
*.cache-from=type=gha,scope=${{ github.workflow }} | ||
files: | | ||
docker-bake.hcl | ||
build.json | ||
|
||
- name: Set output variables | ||
id: bake_metadata | ||
run: | | ||
.github/workflows/extract-docker-image-names.sh | tee -a "${GITHUB_OUTPUT}" | ||
env: | ||
BAKE_METADATA: ${{ steps.build.outputs.metadata }} |
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.
It would be better to use
--target
here to be consistent withaiidalab-docker-stack
but I'll do that in a separate PR.