-
Notifications
You must be signed in to change notification settings - Fork 243
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
Build e2e and integration test images as part of PR checks #3933
Conversation
8a0e9e5
to
156b8c5
Compare
Good to have CI for these container images as sometimes we use wrong base images in these, which cause build failures we only notice fairly late. |
e45148c
to
6a7ed08
Compare
.github/workflows/build-tests.yml
Outdated
- name: Check out repository code | ||
uses: actions/checkout@v3 | ||
|
||
- name: Login to GitHub Container Registry |
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 are you login?
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.
Because I missed it when changing the flow. Mistake, removed. Thanks!
6a7ed08
to
8a53672
Compare
8a53672
to
7502c1e
Compare
.github/workflows/build-tests.yml
Outdated
- name: Upload e2e artifact | ||
uses: actions/upload-artifact@v3 | ||
with: | ||
name: ${{ env.IMAGE_NAME_E2E }}-pr${{ github.event.number }} |
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 some steps use the ${{ github.event.number }} directly and others use then env section?
.github/workflows/build-tests.yml
Outdated
env: | ||
PR_NUMBER: ${{ github.event.number }} | ||
run: | | ||
podman save -o ${{ env.IMAGE_NAME_E2E }}.tar quay.io/crcont/${{ env.IMAGE_NAME_E2E}}:pr-${PR_NUMBER} |
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.
Also may build and save can be added together on a single step? it will make things a bit easier to read IMHO
.github/workflows/build-tests.yml
Outdated
run: | | ||
podman save -o ${{ env.IMAGE_NAME_E2E }}.tar quay.io/crcont/${{ env.IMAGE_NAME_E2E}}:pr-${PR_NUMBER} | ||
|
||
- name: Upload e2e artifact |
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.
Would change to Upload e2e image (to match previous naming convention for steps)
f6b5530
to
a245fb2
Compare
a245fb2
to
83df475
Compare
@jsliacan: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: adrianriobo The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes: Issue #3931
Introduce a GH workflow with 2 jobs (run in parallel by default). 1 job builds e2e image and the other builds integration image. Both images are tagged with
id-${{github.sha}}
, archived as.tar
files and uploaded as artifacts (instead of pushing to external registry). The purpose of these images is tied to the lifetime of a PR anyways.These artifacts will be consumed by another workflow (windows-e2e/integration) which will use them to run e2e and integration PR checks on windows environment.
Note
Using
${{github.sha}}
as the correlation number should work for bothpull_request
andpush to main
trigger events. For this, I have 2github
context dumps for each case (PR, push). Two workflows (test1 and test2) were triggered by the same event (PR or push to main) and context dump was saved. Comparinggithub.sha
values suggests that they are the same for these workflows (as they should be). This generalizes the previous approach where we used PR# to correlate workflows. Moreover, thesha
approach comes with another advantage. Thesha
changes from trigger to trigger, making sure that if we download an artifact with the correctsha
, it will be the intended one and not one from a previous trigger. This would not be guaranteed when using PR# as id, because that does not change from trigger to trigger.gha-win-e2e.pdf