-
Notifications
You must be signed in to change notification settings - Fork 119
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
ci: move tshark build to separate workflow #1590
Conversation
233eb6e
to
6555f44
Compare
6555f44
to
23c2042
Compare
45a3de4
to
dcab3bd
Compare
512108e
to
fdf803a
Compare
fdf803a
to
d6ea847
Compare
# run every morning at 10am Pacific Time | ||
# Running this every day makes sure the static dependencies are up to date | ||
- cron: '0 17 * * *' |
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.
prob lightweight but can we do this at night so that it doesnt interfere with dev operation during the day?
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.
The idea is to run it in the morning when people are coming online so we can address any issues.
branches: | ||
- main |
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 do we need push to main if we have schedule and paths already? seems like this is likely to be noisy and noop?
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's filtered to only the relevant paths so it should only run if updated.
- name: Upload to S3 | ||
if: github.event_name == 'schedule' || github.event_name == 'push' || github.repository == github.event.pull_request.head.repo.full_name | ||
run: | | ||
aws s3 sync target/tshark "s3://s2n-quic-ci-artifacts/tshark" --acl private --follow-symlinks |
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.
whats the difference between this url and https://dnglbrstg7yg.cloudfront.net/tshark used elsewhere?
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.
That CF distribution points to that bucket. So this is just uploading so it can be accessed later.
version: | ||
description: 'wireshark version' | ||
required: true | ||
default: '3.7.1' |
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.
4.0.2
or if you dont want to re-test then can you set the version in the dockerfile to 3.7.1 to match this and avoid confusion
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 maybe dont specify version here since it defaults to 3.7.1 in the job below?
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.
This value shows up in the GitHub Actions UI. So I wanted to communicate there what current version we were on.
Description of changes:
The tshark and interop runner started failing after
ubuntu-latest
changed to 22.04 instead of 20.04. This change pins the broken jobs to 20.04 until the interop runner docker image is updated.Call-outs:
The latest version of wireshark has a problem with the longrtt test. This will need a separate fix applied upstream.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.