[JUJU-1457] Merge 3.0 compatibility branch onto master #692
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
to use the ModelInfo from the ModelManagerFacade
previously it was ClientFacade.GetModelConstraints
ClientFacade.DestroyMachines -> MachineManager.DestroyMachineWithParams
- fix bundle actions to use updated facades - update the bundle being used to use charmhub - replace charmhub.info with a manual api call to get charm id - minor fixes for linter etc
juju#689 #### Description This PR is the first of a bunch of PRs to enable pylibjuju to support Juju 3.0. In particular, the aim of this PR is to have all the tests in the `test_model.py` to pass, which constitutes a significant portion of libjuju's operational logic. Includes 12 commits. The summary of the changes is: - Facade versions are updated with the new schema, and the new clients are generated. - Changed incorrect uses of `ClientFacade` for a bunch of functions to use their respective correct Facades. - Update the tests/bundles that are using charmstore charms to use the charmhub instead. - There's a couple of spots where we needed `CharmHub.Info()` to get some basic charm info to deploy stuff, however, since CharmHub facade doesn't exist anymore in juju 3.0, I replaced it with a simple api call to the charmhub itself to get the necessary info. #### QA Steps All the tests in `test_model.py` should pass on a `juju 3.0` controller. So bootstrap from juju's `develop` branch (or `snap install juju --channel=latest/beta`) and run the following in libjuju: `tox -e integration -- tests/integration/test_model.py` #### Notes & Discussion - This PR targets the `juju-3.0-compatibility` branch, not the main branch. - Currently the tests in the CI don't make much sense, since the CI is being run against the latest stable juju (which is still 2.9). I expect most of them to fail until we address the problem of supporting both 2.9 and 3.0 at the same time.
juju#691 #### Description This PR completes the fixes for 3.0 compatibility. With this, `pylibjuju` should be able to work with `juju 3.0`. Includes 8 commits. The summary of the changes is: * `ClientFacade` -> `CharmsFacade` for upgrades * Correct facade functions for setting and unsetting application config. * Added a call to charmhub api to check for subordinate * Changes to use the correct facade functions with correct input/result parameters * Small fixes for tests #### QA Steps All the integration tests should pass on a `juju 3.0` controller. So bootstrap from juju's `develop` branch (or `snap install juju --channel=latest/beta`) and run the following in libjuju: ```sh make test ``` #### Notes & Discussion * I observe in my local runs that some tests get stuck, running them individually makes them pass, there might be intermittent issues, but this is not related to 3.0 compatibility, a further investigation about that might be done later on. * Again, this targets the `juju-3.0-compatibility` branch. Next step on that is to make sure that the `pylibjuju` can work with both `2.9` and `3.0` facades at the same time.
As long as we keep a tight manifold at the client.py and provide the facade classes from the correct _client modules, then those facades will use the corresponding lookup functions where they're from. This way we're sure the correct _client* will be used, so no need to import everything from old_clients/_client.py into the _client.py. The main separation happens at the client.py.
ActionFacade.Run
instead of checking individual facade versions
|
Some notes on the tests:
So both of them passing means that with this change the (the |
juanmanuel-tirado
approved these changes
Jul 26, 2022
|
/merge |
jujubot
added a commit
that referenced
this pull request
Aug 9, 2022
#704 Tuesday August 9 2022 ## What's Changed Switching to semantic versioning. From this release on, at least the major release number matches the most recent Juju supported. Hence the jump to `3.0.0` since this release supports `Juju 3.0`. (This also means that `python-libjuju <= 2.9.11` only support up to `Juju 2.x`) * [JUJU-1439] Initial fixes for `test_model` to pass with juju 3.0 by @cderici in #689 * [JUJU-1464] More fixes for 3.0 compatibility by @cderici in #691 * [JUJU-1457] Merge 3.0 compatibility branch onto master by @cderici in #692 * Fix conditional by @sed-i in #696 * [JUJU-1534] Fix `model.connect_current()` by @cderici in #697 * [JUJU-1542] Fix run actions on units by @cderici in #698 * [JUJU-1577] Replace k8s bundles with machine bundles for tests by @cderici in #703 * [JUJU-1528] Add storage implementation by @cderici in #701 [JUJU-1439]: https://warthogs.atlassian.net/browse/JUJU-1439?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [JUJU-1464]: https://warthogs.atlassian.net/browse/JUJU-1464?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [JUJU-1457]: https://warthogs.atlassian.net/browse/JUJU-1457?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [JUJU-1534]: https://warthogs.atlassian.net/browse/JUJU-1534?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [JUJU-1542]: https://warthogs.atlassian.net/browse/JUJU-1542?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [JUJU-1577]: https://warthogs.atlassian.net/browse/JUJU-1577?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [JUJU-1528]: https://warthogs.atlassian.net/browse/JUJU-1528?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces the mechanism to have both the old and the new clients generated from
2.9and3.0schema respectively. Based on theserver_version'information within the connection, correct facade is picked from the correct client among the correct client group. With this,pylibjujushould be able to work with bothjuju 2.9andjuju 3.0.This PR also merges the
juju-3.0-compatibilitybranch onto themaster.Most important changes include:
juju 3.0schema, and from thejuju 2.9schema. Thefacadecode is updated to generate the_clientcode to have both sets of clients and facades inside, and whenever a facade is needed, to dynamically pick the correct facade from the correct client based on the version set in the underlying connection with juju.client.pymodule is no longer a regular Python module, it's a class instance that acts like a module within Python runtime. Based on the juju version, it pulls all the bindings (_clients and the_definitions) dynamically from respective correct modules, which are named_clientand_definitionsforjuju 3.0, and_2_9_clientand_2_9_definitionsforjuju 2.9. So whenever in the code we pull a binding from the client, likeclient.CharmOrigin, theCharmOriginobject we get comes from the correct_definitionsmodule.QA Steps
All the integration tests should pass here on CI, and also on a
juju 3.0controller.So bootstrap from juju's
developbranch (orsnap install juju --channel=latest/beta) and run the following in libjuju:make testNotes & Discussion
Everything on the CI tests should pass except the intermittent stuff we already know about, and also some charmhub stuff that we observed before I started working on this.
There will be some tests failing for sure, I couldn't check all the tests yet, so I'm labeling this as
do not mergeuntil we see all green (or almost all green).