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: run the smoke tests on a schedule #1387

Merged
merged 29 commits into from
Sep 25, 2024

Conversation

tonyandrewmeyer
Copy link
Contributor

@tonyandrewmeyer tonyandrewmeyer commented Sep 24, 2024

We have some basic smoke tests, which are currently the only tests we regularly run against an actual Juju model. The tests deploy a basic charm and verify that it goes to active status (which happens in an install handler). However, these are currently only run if someone manually runs tox -e smoke. This PR adds running the tests on a schedule (currently monthly, in the days before we will likely do a release).

This tests:

  • Juju 3.5 (latest stable 3.x - I suggest we change this to 3.6 when that's released, rather than adding 3.6). The smoke tests do not currently work with Juju 2.9 (I'm not sure if it's worth adding that at this point), but if that changes, it should only require a matrix change in the workload. The smoke tests use python-libjuju, which doesn't yet support Juju 4.0, but when that changes, this should also only require a matrix change.
  • Charmcraft 2.x and 3.x.
  • LXD and MicroK8s.

The smoke test is changed to use a plain subprocess.run to pack the charm, rather than use pytest-operator for this. The pytest-operator approach doesn't work out-of-the-box in the workflow, and it seems cleaner to just do the pack ourselves.

There's an example run on my fork.

This could be optimised in the future to store the packed charms and share them across Juju versions and cloud types, but it doesn't seem worth doing that for now.

Fixes #1322

@tonyandrewmeyer tonyandrewmeyer marked this pull request as draft September 24, 2024 00:58
@tonyandrewmeyer tonyandrewmeyer changed the title Regular smoke tests ci: run the smoke tests on a schedule Sep 24, 2024
@tonyandrewmeyer tonyandrewmeyer marked this pull request as ready for review September 24, 2024 03:51
Copy link
Collaborator

@benhoyt benhoyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks a lot for this. Just one optional nit comment.

@@ -147,6 +148,8 @@ commands =
python -m build --sdist --outdir={toxinidir}/test/charms/test_smoke/
# Inject the tarball into the smoke test charm's requirements.
bash -c 'echo "./$(ls -1 ./test/charms/test_smoke/ | grep tar.gz)" > ./test/charms/test_smoke/requirements.txt'
# If a specific Juju version is set, then make sure we are using that version of pylibjuju.
bash -c 'if [ -n "$JUJU_VERSION" ]; then pip install "juju ~= $JUJU_VERSION"; fi'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever!

addons: '["dns", "hostpath-storage"]'

- name: Set up Juju (classic)
if: matrix.juju-version == '2.9'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we just drop the "if 2.9" stuff here, seeing we're not running these on 2.9? Or do you think we should add that in the near future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good question. I started out planning to do 2.9, 3.[latest stable], and 4.0, thinking that would be a good range of coverage. 2.9 because it's still in "security fix support" and seems to have non-trivial use still, 3.[latest stable] because it seems unlikely that we'd break deploying only for one 3.x, and 4.0 because it's good to be proactive. I then ran into a bunch of issues and ended up dropping down to just 3.x.

tox -e smoke passes for me (except for trying to deploy on 24.04) locally, so I think it shouldn't be too much work to get it working in CI as well. I'd like to use a quite small amount of time to try to get that going and if not dropping it would make sense yes. I could cut it from this PR and add it back later if that was preferred.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I'm happy either way -- no specific urgency on this.

@tonyandrewmeyer tonyandrewmeyer merged commit d342885 into canonical:main Sep 25, 2024
33 checks passed
@tonyandrewmeyer tonyandrewmeyer deleted the regular-smoke-tests branch September 25, 2024 21:36
tonyandrewmeyer added a commit to tonyandrewmeyer/operator that referenced this pull request Oct 4, 2024
We have some basic smoke tests, which are currently the only tests we
regularly run against an actual Juju model. The tests deploy a basic
charm and verify that it goes to `active` status (which happens in an
`install` handler). However, these are currently only run if someone
manually runs `tox -e smoke`. This PR adds running the tests on a
schedule (currently monthly, in the days before we will likely do a
release).

This tests:
* Juju 3.5 (latest stable 3.x - I suggest we *change* this to 3.6 when
that's released, rather than *adding* 3.6). The smoke tests do not
currently work with Juju 2.9 (I'm not sure if it's worth adding that at
this point), but if that changes, it should only require a matrix change
in the workload. The smoke tests use python-libjuju, which doesn't yet
support Juju 4.0, but when that changes, this should also only require a
matrix change.
 * Charmcraft 2.x and 3.x.
 * LXD and MicroK8s.

The smoke test is changed to use a plain `subprocess.run` to pack the
charm, rather than use pytest-operator for this. The `pytest-operator`
approach doesn't work out-of-the-box in the workflow, and it seems
cleaner to just do the pack ourselves.

There's an [example run on my
fork](https://github.com/tonyandrewmeyer/operator/actions/runs/11006199263/job/30560942709).

This could be optimised in the future to store the packed charms and
share them across Juju versions and cloud types, but it doesn't seem
worth doing that for now.

Fixes canonical#1322
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.

Run 'tox -e smoke' in CI
3 participants