[JUJU-3517] Revisit _build_facades in connection #826
Merged
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
The main change here is that we no longer raise an exception in the case of an unknown facade.
juanmanuel-tirado
approved these changes
Apr 14, 2023
juanmanuel-tirado
added a commit
that referenced
this pull request
May 5, 2023
* [JUJU-3202] Add facades for 3.1.1. (#807) * Add facades for 3.1.1. * Update secrets backend test. * Add destroy-units to destroy several units at once Fixes #811 * Add integration test for destroy_units * [JUJU-3517] Revisit _build_facades in connection (#826) The main change here is that we no longer raise an exception in the case of an unknown facade. * Added 3.1.2 and 3.2-beta2 schemas. * Add capability for deploying by revision by passing the revision info through the ResolveCharm api call. fixes #690 * Add example for deploy with revision that deploys juju-qa-test --channel 2.0/stable --revision 22 * Add revision parameter in LocalDeployType.resolve Note that --revision is only used for charmhub charms, so we're not passing this info into the CharmOrigin for local charms. * Add integration test for deploy by revision success * Add validation for --revision flag to require --channel flag This is needed because in libjuju we default to latest/stable if no channel is specified. It is dangerous if we don't add this check because without it the libjuju would deploy the wrong revision without an exception if --revision is used but no --channel is given. * Add validation for bundles to require either --revision or --channel Otherwise fail early without making any additional API calls. * Add integration test for making sure the --required and --channel flags are validated before deploying charms or bundles * [JUJU-3253] add missing force in bundle deployment (#815) * Propagate force parameter to resolve function to enable force for remote bundles. * Pass series info into origin for ResolveCharm Without the series the ResolveCharm will select the latest base for the charm. E.g. even if we want focal, the ResolveCharm will return 22.04 (jammy) for the base channel in the resulting origin if the charm supports both jammy and focal. This communicates the series info with the ResolveCharm via the inputted origin so the resulting origin will have the correct base channel. Fixes #822 * Fix _resolve_charm errors That are accidentally introduced in #825. * _resolve_charm's force parameter is made optional * fixed the _resolve_charm calls in bundle.py to have less number of variables to unpack onto * Change charm channel in bundle for test * Rename example with consistent name * Implement series selector for charm resolution This selector will be used after the ResolveCharms api call, and use the supported series and the base information returned to select the appropriate series to construct the correct base for the charm. Then it's given to the AddCharm in normal the deploy process. * Add unit tests for series selector functionality * Utilize the series selector to construct the correct base for charms The main change is in the _resolve_charm that's used in the model and bundle deploy code path. We pass in the series info whenever we can into the ResolveCharm call, then use its result to run the series selector, then construct the base using that and finally pass the origin to the AddCharm. Note that the ResolveCharm API call is made no more than once. * Fix unit tests for bundle change runs * Add integration test to challenge the resolver to find an old corrrect revision This is basically testing what is manually reported by @juanmanuel-tirado in the following review: #830 (comment) * [JUJU-3552] Prepare 3.1.2.1 release (#836) * Add juju 3.1.2 missing facades. * Fix secrets-bakend-lifecycle test. * Increase testing timeouts. * Revisit postgresql charm to be downloaded. --------- Co-authored-by: Juju bot <[email protected]> Co-authored-by: Caner Derici <[email protected]> * Prepare release notes for 3.1.2.0. (#843) * Prepare release notes for 3.1.2.0. --------- Co-authored-by: Caner Derici <[email protected]> Co-authored-by: Juju bot <[email protected]>
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
The main change here is that we no longer raise an exception in the case of an unknown facade.
Related LP bug : https://bugs.launchpad.net/charmed-kubernetes-testing/+bug/2012078
QA Steps
All CI tests need to pass.
Additionally, you may comment out a facade inside the
client_facadesinclient/connection.pyand run:Notes & Discussion
So this is not exactly the solution for the LP bug above, because we're not hitting the
raisethere as I initially thought we were. I've yet to figure out what the problem in the LP bug is because I can't reproduce it (seems to be working w/ juju3.1.1as I commented on the LP bug too)