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

[JUJU-367] Improve get_charm_series to check the model for series for a local charm #607

Merged
merged 5 commits into from
Dec 17, 2021

Conversation

cderici
Copy link
Contributor

@cderici cderici commented Dec 16, 2021

Description

get_charm_series method for a local charm extracts the series information for a local charm. It's often called from the _handle_local_charms from within the bundle.py when deploying local charms, which handles the case where no series information found by also checking the model config data.

The get_charm_series is also called from application.py in refresh (upgrade_charm) method, which doesn't handle the "no series" case.

This PR moves that logic (of also checking the model for series info) into the get_charm_series by adding an additional model parameter. So, whichever entity that's calling the get_charm_series also provides the associated model and need to do nothing else.

Fixes #606

QA Steps

juju bootstrap microk8s test && python examples/upgrade_local_charm_k8s.py

Also related:

tox -e integration -- tests/integration/test_application.py::test_upgrade_local_charm

This one ☝️ is using machine charms and was already passing before this PR too.

Notes & Discussion

@cderici cderici changed the title Improve get_charm_series to check the model for series for a local charm [JUJU-367] Improve get_charm_series to check the model for series for a local charm Dec 16, 2021

1. Connects to the current model
2. Deploy a bundle and waits until it reports itself active
3. Destroys the units and applications
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing: upgrades the charm from local path.

juju/bundle.py Outdated

if series is None:
model_config = await model.get_config()
default_series = model_config.get("default-series")
Copy link
Contributor

Choose a reason for hiding this comment

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

In my case I have a k8s charm, but the "default-series" it has stored is:
<ConfigValue source='default' value='focal'>
Is that normal?

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 that's normal, that's why we get the .value out of it (#L440), but we need to check if it's None first, though maybe I can make the coding a bit clearer there.

Copy link
Contributor

@juanmanuel-tirado juanmanuel-tirado left a comment

Choose a reason for hiding this comment

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

LG2M

Copy link
Contributor

@juanmanuel-tirado juanmanuel-tirado left a comment

Choose a reason for hiding this comment

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

LG2M

@cderici
Copy link
Contributor Author

cderici commented Dec 17, 2021

!!build!!

@cderici
Copy link
Contributor Author

cderici commented Dec 17, 2021

$$merge$$

@jujubot jujubot merged commit 8d62315 into juju:master Dec 17, 2021
jujubot added a commit that referenced this pull request Jan 31, 2022
#619

## What's Changed
* [JUJU-320] Unit public address by @SimonRichardson in #600
* [JUJU-244] Add attach-resource by @cderici in #601
* [JUJU-140] Model.wait_for_idle -- for apps with no units yet by @cderici in #575
* [JUJU-367] Improve `get_charm_series` to check the model for series for a local charm by @cderici in #607
* [JUJU-366] Utility for connecting directly to existing connection by @cderici in #605
* Use public-address key instead of public_address by @wolsen in #610
* [JUJU-376] `wait_for_idle` to support scale down by @cderici in #613
* [JUJU-378] Utility for block_until-ing with a custom coroutine by @cderici in #614
* Fallback to 'local-fan' by @dparv in #612
* Minor comments on docs for block_until related functions. by @juanmanuel-tirado in #617
* Additional checks in print status. by @juanmanuel-tirado in #622

## New Contributors
* @wolsen made their first contribution in #610
* @dparv made their first contribution in #612


[JUJU-320]: https://warthogs.atlassian.net/browse/JUJU-320?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-244]: https://warthogs.atlassian.net/browse/JUJU-244?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-140]: https://warthogs.atlassian.net/browse/JUJU-140?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-367]: https://warthogs.atlassian.net/browse/JUJU-367?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-366]: https://warthogs.atlassian.net/browse/JUJU-366?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-376]: https://warthogs.atlassian.net/browse/JUJU-376?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-378]: https://warthogs.atlassian.net/browse/JUJU-378?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
@cderici cderici deleted the fix-upgrade-sidecar-charm branch February 16, 2022 18:22
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.

application refresh (upgrade charm) fails with ConnectionResetError
4 participants