Skip to content
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

Merged
merged 10 commits into from
Apr 3, 2025

Conversation

petrutlucian94
Copy link
Contributor

@petrutlucian94 petrutlucian94 commented Apr 3, 2025

  • 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.
Copy link
Contributor

@louiseschmidtgen louiseschmidtgen left a 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
Copy link
Contributor

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?

Copy link
Contributor Author

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.

@petrutlucian94
Copy link
Contributor Author

petrutlucian94 commented Apr 3, 2025

@louiseschmidtgen turns out that test-addons.py::TestAddons::test_cis uses rook and the test fails without iscsid. I assume that the test uses the Ceph iSCSI gateway, I'll go ahead and add it back.

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.
@petrutlucian94 petrutlucian94 merged commit a47d34d into master Apr 3, 2025
12 checks passed
@petrutlucian94 petrutlucian94 deleted the lpetrut/unblock-ci branch April 3, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants