-
Notifications
You must be signed in to change notification settings - Fork 47
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
Juju 3.x Support #608
Juju 3.x Support #608
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #608 +/- ##
==========================================
+ Coverage 89.43% 89.50% +0.07%
==========================================
Files 44 45 +1
Lines 4693 4744 +51
==========================================
+ Hits 4197 4246 +49
- Misses 496 498 +2 ☔ View full report in Codecov by Sentry. |
|
When we try to use
If we do this other change we can pull in the latest version of libjuju by default unless an extra is specified, although this won't prevent us from submitting a batch that changes the test-requirements.txt, and if this was the approach we may not even need to define extras and just a constraints file to lock down libjuju to an specific range. diff --git a/setup.py b/setup.py
index 8fd19e5..96c5d00 100644
--- a/setup.py
+++ b/setup.py
@@ -48,6 +48,7 @@ tests_require = [
extras_require={
'testing': tests_require,
+ '': ['juju'],
'juju-29': ['juju<3.0'],
'juju-31': ['juju>=3.1.0,<3.2.0'],
'juju-32': ['juju>=3.2.0,<3.3.0'], |
Added a new commit that gives a more sane default 084fced
|
This behavior would still break the charms gate unless we change zosci-config default juju_channel value[0] from 2.9/stable to 3.2/stable in lockstep. It would prevent us from doing a batch and "just" deal with the possible breakages because we would be using juju 3.2 now. If this is the path we decide to pursue, the change needs to be made "git branch aware", so only master/main use 3.2 while "stable/.*" keep using juju 2.9 |
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.
Hi Felipe, I added a couple of comments.
zaza/model.py
Outdated
results = action.data.get('results') | ||
await action.wait() | ||
results = None | ||
try: |
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.
Can we use _normalise_action_object() here?
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.
ack, addressed by 10351db
zaza/model.py
Outdated
results = action.data.get('results') | ||
await action.wait() | ||
results = None | ||
try: |
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.
Can we use _normalise_action_object() here?
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.
ack, addressed by 10351db
zaza/model.py
Outdated
@@ -1096,6 +1135,7 @@ async def async_run_action(unit_name, action_name, model_name=None, | |||
unit = await async_get_unit_from_name(unit_name, model) | |||
action_obj = await unit.run_action(action_name, **action_params) | |||
await action_obj.wait() | |||
action_obj = _normalise_action_object(action_obj) | |||
if raise_on_failure and action_obj.status != 'completed': |
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.
Global comment: should we switch this to action_obj.data['status'] ?
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.
addressed by 7a5dba6
@@ -2184,7 +2231,15 @@ async def _check_for_file(model): | |||
for unit in units: | |||
try: | |||
output = await unit.run('test -e "{}"; echo $?'.format(path)) | |||
contents = output.data.get('results')['Stdout'] | |||
await output.wait() |
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.
Can we use _normalise_action_object() here?
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.
ack, addressed by 10351db
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.
I'm generally okay, note @coreycb comments on using _normalize_action_object
.
Hi folks, can we merge this one along with #617? Thanks |
constraints seem to not be making effect
|
Hi, I am wondering if we should also wrap def _normalise_action_object(action_obj):
try:
# libjuju 3.x
action_obj.data['status'] = action_obj._status
action_obj.data['results'] = _normalise_action_results(action_obj.results) # wrap here
except (AttributeError, KeyError):
# libjuju 2.x format, no changes needed
pass
return action_obj Because currently running actions with not return an action object with normalised |
Add separate workflow for testing on Python 3.6 as this requires an older Ubuntu release. Fixup unit tests for compatibility with newer versions of mock. Drop latest/stable functional test targets - this is actually 2.9.x of Juju and is already covered by the 2.9 targets and we want to avoid suddenly picking up a new Juju version because this will break with the new approach to version alignment in the Python module for Juju. Drop 2.8 functional test target - its broken and we don't really support this version any longer. Fixup iptables forwarding issues from LXD containers with a flush and re-create of rules. (cherry picked from commit 9277a94)
Juju is pinned to <3.0 earlier. This patch pins the juju version to <3.2 so that libjuju 3.1 version is used. Modified run_on_unit to wait for completion and update results based on output.
Update channel in github workflows to use Juju 3.1. Drop --no-gui flag usage as this is the default now.
Update model configuration default-series to focal. Drop --classic flag for Juju installation.
When juju is strictly confined, random temp directories under /tmp are not accessible - render any templated bundle files under $HOME instead as this should be readable.
python-libjuju is released in lockstep with juju, hence if zaza uses a 2.9 controller, it should use libjuju-2.9.x, for a 3.1 controller it should use libjuju-3.1 and so on. This change makes libjuju an extra, which means depending on the juju controller version will be used the right extra should be passed at install time. For juju-2.9: pip install zaza[juju-29] For juju-3.1: pip install zaza[juju-31]
Summary of changes: - Add juju-3.2 to the github workflow matrix - Add 'juju-32' extra to install juju-3.2.x
This forces tox to install the zaza python package and honor the 'extras' defined.
By default depend on 'juju' (no pinning), the pinning only comes into place when an explicit extra is passed. This makes the extras effectively nothing more than a pinning alias, basically `pip install juju<3.0 zaza` becomes equivalent to `pip install zaza[juju-29]`.
By default libjuju 2.9 will be used, this can overriden by passing the PIP_CONSTRAINTS environment variable Examples: PIP_CONSTRAINTS=./constraints-juju31.txt tox -e pep8 --recreate This allows running functional tests with different versions of juju
The testing runners use Python 3.8 (Focal), hence no need to keep py36 alive.
Switching to draft until this issue gets sorted out.
https://openstack-ci-reports.ubuntu.com/artifacts/f0b/908183/1/check/charm-build/f0b6c6e/job-output.txt |
Related PR: openstack-charmers/zosci-config#309 |
Juju 3.x replaced the `series` status key with a `base` key that consists of Distribution type and version number. To avoid maintenance burden we add a Launchpad module that implements functions to look up available Ubuntu series data. Update the `get_machine_series` helper function to determine Ubuntu series from `base` when no `series` key is available. Signed-off-by: Frode Nordahl <[email protected]>
This file aims to hold the default version of juju that it's expected charms to be tested with. It helps to serve as a sane default for tox.ini
Issue found here in the CI runs - juju/python-libjuju#1028 |
The juju snap can't run programs that are outside the snap due to the confinement restrictions
Here I'm attaching the logs of running
Full logs: func-target.thirds.juju31.log |
This pull request adds juju-3.x support.
Summary of changes:
By default when installing 'zaza' won't have a pinned libjuju, hence users will get the latest juju available in pypi, although downstream consumers are encouraged to pin the library to the version compatible with the juju controller they want to connect to.
Running functional tests locally (
tox -e func-target -- first
) will use libjuju-2.9 by default, if contributors need/want to use a different version they can override the constraint setting the PIP_CONSTRAINTS environment variable, e.g.:Closes #545