-
Notifications
You must be signed in to change notification settings - Fork 3.4k
workflows/ingress: Run basic checks #35683
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
489c1b8
to
b581aac
Compare
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 ✔️
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.
We should also add Upload Junits
step after Upload artifacts
- name: Upload JUnits [junit]
if: ${{ always() }}
uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3
with:
name: cilium-junits-${{ matrix.name }}
path: cilium-junits/*.xml
and merge-upload
job after ingress-conformance-test
job
merge-upload
job can be same from other workflows
b581aac
to
9d95c60
Compare
@brlbil Thanks for the review! Seeing this I'm wondering if the JUnit stuff is even needed here, considering we're only going to run 2-3 tests tops (and only one currently). What do we gain from reporting those results via JUnit? |
@pchaigno Junits are used in reporting tools to list which tests failed with workflow etc... But if you think it is not necessary then by all means remove the |
9d95c60
to
76379d7
Compare
This workflow doesn't run the CLI connectivity tests, so it's missing some basic checks such as unexpected packet drops. This commit adds it. The check-log-errors test should also be enabled, but it is currently failing because there are errors in logs whenever enabling Ingress. It can be added once those errors are fixed. Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
76379d7
to
6a042e5
Compare
@brlbil We discussed it with Tam and the JUnit stuff probably isn't needed here for the time being (probably to be reconsidered if more connectivity tests are covered.) |
/test |
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.
Looks good 👍
This workflow doesn't run the CLI connectivity tests, so it's missing some basic checks such as unexpected packet drops. This commit adds it.
The check-log-errors test should also be enabled, but it is currently failing because there are errors in logs whenever enabling Ingress. It can be added once those errors are fixed.