-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
FIX [CI
/ Docker
] Follow up from #1481
#1487
Merged
Merged
Changes from 6 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
5f0602c
Update test-docker-build.yml
younesbelkada 490113e
Update test-docker-build.yml
younesbelkada ea282d3
Update Dockerfile
younesbelkada 6cfa521
Update test-docker-build.yml
younesbelkada 1925cc5
Update test-docker-build.yml
younesbelkada e021a25
Update Dockerfile
younesbelkada ef6cd9c
Update .github/workflows/test-docker-build.yml
younesbelkada 3ec603b
Update .github/workflows/test-docker-build.yml
younesbelkada 42d6285
Update test-docker-build.yml
younesbelkada c2059b0
Update test-docker-build.yml
younesbelkada 127353a
Update test-docker-build.yml
younesbelkada aec2045
Update test-docker-build.yml
younesbelkada e834b4b
Update test-docker-build.yml
younesbelkada 7adf0f5
Update test-docker-build.yml
younesbelkada e7245ed
Update test-docker-build.yml
younesbelkada 444ebd0
Update test-docker-build.yml
younesbelkada 03e10f8
Update test-docker-build.yml
younesbelkada f4bac2e
Update test-docker-build.yml
younesbelkada 68ae675
Update .github/workflows/test-docker-build.yml
younesbelkada 9552ed9
Update Dockerfile
younesbelkada 28610c0
Update test-docker-build.yml
younesbelkada f5c35d3
Update test-docker-build.yml
younesbelkada cbbab27
Update test-docker-build.yml
younesbelkada b3fbe06
Update test-docker-build.yml
younesbelkada ece7d2f
Update test-docker-build.yml
younesbelkada 65ed5c1
Update test-docker-build.yml
younesbelkada 66c329d
Update test-docker-build.yml
younesbelkada 17efab4
Update test-docker-build.yml
younesbelkada 8546048
Update test-docker-build.yml
younesbelkada 5d6d86c
Update test-docker-build.yml
younesbelkada 7c21f16
Update test-docker-build.yml
younesbelkada 6fc6cdb
Update test-docker-build.yml
younesbelkada 1cf4b19
Update test-docker-build.yml
younesbelkada 7ec91b3
Update test-docker-build.yml
younesbelkada 1672a34
revert
younesbelkada d633baf
Update .github/workflows/test-docker-build.yml
younesbelkada File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,42 +1,50 @@ | ||
name: Test Docker images (scheduled) | ||
name: Test Docker images (on PR) | ||
|
||
on: | ||
pull_request: | ||
paths: | ||
# Run only when DockerFile files are modified | ||
- "docker/**" | ||
paths: | ||
# Run only when DockerFile files are modified | ||
- "docker/**" | ||
|
||
concurrency: | ||
group: docker-image-builds | ||
cancel-in-progress: false | ||
|
||
jobs: | ||
name: "Build all modified docker images" | ||
runs-on: ubuntu-latest | ||
steps: | ||
- name: Set up Docker Buildx | ||
uses: docker/setup-buildx-action@v1 | ||
- name: Check out code | ||
uses: actions/checkout@v3 | ||
- name: Get changed files | ||
id: changed-files | ||
uses: tj-actions/changed-files@3f54ebb830831fc121d3263c1857cfbdc310cdb9 #v42 | ||
with: | ||
files: docker/** | ||
- name: Run step if only the files listed above change | ||
if: steps.changed-files.outputs.only_changed == 'true' | ||
env: | ||
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files}} | ||
run: | | ||
for file in ${CHANGED_FILES}; do | ||
echo "$file was changed" | ||
done | ||
- name: Build Docker images | ||
strategy: | ||
matrix: | ||
docker-file: ${{ steps.changed-files.outputs.all_changed_files}} | ||
uses: docker/build-push-action@v4 | ||
with: | ||
context: ${{ matrix.docker-file }} | ||
push: False | ||
|
||
get_changed_files: | ||
name: "Build all modified docker images" | ||
runs-on: ubuntu-latest | ||
outputs: | ||
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. Could use |
||
matrix: ${{ steps.set-matrix.outputs.docker-matrix }} # https://docs.github.com/en/actions/learn-github-actions/expressions#example-returning-a-json-object | ||
steps: | ||
- name: Set up Docker Buildx | ||
uses: docker/setup-buildx-action@v1 | ||
- name: Check out code | ||
uses: actions/checkout@v3 | ||
- name: Get changed files | ||
id: changed-files | ||
uses: tj-actions/changed-files@3f54ebb830831fc121d3263c1857cfbdc310cdb9 #v42 | ||
with: | ||
files: docker/** | ||
- name: Run step if only the files listed above change | ||
if: steps.changed-files.outputs.only_changed == 'true' | ||
env: | ||
younesbelkada marked this conversation as resolved.
Show resolved
Hide resolved
|
||
CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files}} | ||
run: | | ||
for file in ${CHANGED_FILES}; do | ||
echo "$file was changed" | ||
done | ||
echo "docker-matrix=${CHANGED_FILES}" >> $GITHUB_OUTPUT | ||
build_modified_files: | ||
needs: get_changed_files | ||
name: Build Docker images$ | ||
runs-on: ubuntu-latest | ||
strategy: | ||
matrix: | ||
docker-file: ${{ fromJSON(needs.get_changed_files.outputs.matrix) }} # https://docs.github.com/en/actions/learn-github-actions/expressions#example-returning-a-json-object | ||
steps: | ||
younesbelkada marked this conversation as resolved.
Show resolved
Hide resolved
|
||
- name: Build Docker image | ||
uses: docker/build-push-action@v4 | ||
with: | ||
context: ${{ matrix.docker-file }} | ||
push: False |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 am not sure if we really want to test this in each PR (all of its commits), or just when merged into
main
.I know it's better to keep PR green. But for workflow run on each PR, we have to manually approve the run (for security run), right?
ping @glegendre01 again to see if the infra team prefers not to allow rununing on PRs whenever possible, or it doesn't really matter.
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 think as long as we don't push the docker image on docker hub it's all good, from the conversation I had with guillaume !