-
Notifications
You must be signed in to change notification settings - Fork 790
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: unblock the CI and add a few improvements #5008
Conversation
petrutlucian94
commented
Apr 3, 2025
•
edited
Loading
edited
- GH actions no longer support 20.04 runners, so we'll switch to 22.04
- need to apply the well known iptables workaround to address the compatibility issue between LXD and Docker
- we're adding the run-all-tests label which controls the UNDER_TIME_PRESSURE env variable
- to avoid duplicate logic, we'll add a composite action that takes care of the test prerequisites, making them easier to maintain.
- we'll avoid installing python packages globally (no longer allowed by recent Python versions). Instead, we'll use actions/setup-python, which will also cache the packages.
- define job timeouts
There's an env variable that controls whether to run all tests or not. In some cases, we may want to run all the tests to ensure that a certain PR doesn't introduce regressions before actually merging it. As such, we're introducing a new label called "run-all-tests". If set, it will set the "UNDER_TIME_PRESSURE" env variable to "false". While at it, we're making the variable case insensitive, making it easier to use with GH action expressions such as "contains", used to check if the "run-all-test" label is set.
GH actions no longer support 20.04 runners, so we'll switch to 22.04.
Recent Python versions no longer allow installing packages with pip against the global Python environment. We'll just use "actions/setup-python" to prepare an isolated Python.
This might not be required, let's see if anything fails.
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.
Nice work, one little comment:
shell: bash | ||
run: | | ||
set -x | ||
sudo pip3 install -R ./tests/requirements.txt |
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.
Do we need to pin pytest in this requirements.txt?
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.
probably not, I don't think we have specific pytest requirements. as long as all the methods that we use are there, it should be ok.
@louiseschmidtgen turns out that LE: nvm, there's a brittle assertion on the number of warnings: canonical/microk8s-core-addons#333 |
One of the test-addons.py::TestAddons::test_cis has an explicit assertion on the number of warnings. Unless we pin the pytest version, we get an additional warning and the test assertion fails... https://github.com/canonical/microk8s-core-addons/blob/f2716f1d4c0e169762f0bff37d7496f7ac33db62/addons/cis-hardening/cfg/cis-1.24-microk8s/etcd.yaml#L13 https://github.com/canonical/microk8s-core-addons/blob/8e163efe9b8e4739e96685f3459af02c94fc54bb/tests/validators.py#L461-L469 We'll go back to pinning pytest.
87b6c03
to
d9872d0
Compare