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-2586] Fix LP19988587 Can't switch from edge channel to stable channel #15141

Merged
merged 6 commits into from Feb 7, 2023

Conversation

hmlanigan
Copy link
Member

Allow an application's channel to be updated when refreshing even if the charm revision is the same so no other change is needed. This fixes a regression in behavior between charm hub and charm store charms. Because it's a regression, only a small change was required.

QA steps

Today, all risk levels of the latest track for the ubuntu charm are the same. Makes a good test candidate.

$ juju deploy ubuntu
$ juju refresh ubuntu --channel edge
charm "ubuntu": already up-to-date. Note: all future refreshes will now use channel "edge"

Bug reference

https://bugs.launchpad.net/juju/+bug/1988587

…not.

Fix for LP1988587. With charm store you could update the channel to be used
in the future, even if the charm itself didn't require upgrade. Allow the
same behavior for charm hub charms.
Copy link
Member

@cderici cderici left a comment

Choose a reason for hiding this comment

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

LGTM. QA steps confirmed. Played with it a little bit, seems to be working well 👍 Also fwiw the newly added tests are passing for me.

Unit tests and SetCharm called by the Update application facade method will
not contain a charm origin. Ensure we don't panic on this in the real world.

The Update facade method is only kept for compatibility with older juju 2.x
clients. With juju 3.0, we can revaluate checking for an empty charm origin
in the facade and updating the unit tests.
@hmlanigan hmlanigan force-pushed the 198858701-refresh-switch-no-rev-change branch from 5d1b92f to 9c50fac Compare February 3, 2023 21:07
@hmlanigan hmlanigan added do not merge Even if a PR has been approved, do not merge the PR! 2.9 labels Feb 3, 2023
Test cover scenarios in LP1988587, where refresh failed due to a lack of new
available charm revision, however the switch or channel flags were used.
Add small tweeks to tests including fixing model names, correcting a
charm name and enhancing comments.
@hmlanigan hmlanigan force-pushed the 198858701-refresh-switch-no-rev-change branch from 2459a2f to 823cf42 Compare February 7, 2023 16:30
Copy link
Member

@cderici cderici left a comment

Choose a reason for hiding this comment

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

Yup seems to be working well now 👍 LGTM

@hmlanigan
Copy link
Member Author

/merge

@jujubot jujubot merged commit c78f9cc into juju:2.9 Feb 7, 2023
@hmlanigan hmlanigan deleted the 198858701-refresh-switch-no-rev-change branch February 7, 2023 19:19
@manadart manadart mentioned this pull request Feb 8, 2023
jujubot added a commit that referenced this pull request Feb 8, 2023
#15164

Merge from 2.9 to bring forward:
- #15129 from jack-w-shaw/JUJU-2615_debug_log_for_caas
- #15141 from hmlanigan/198858701-refresh-switch-no-rev-change
- #15154 from manadart/2.9-migration-rollback
- #15157 from manadart/2.9-sync-agent-binary
- #15117 from cprivitere/whitelist-10-networks

Trivial conflicts.
@hmlanigan hmlanigan removed the do not merge Even if a PR has been approved, do not merge the PR! label Feb 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants