-
Notifications
You must be signed in to change notification settings - Fork 507
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-1823] Bug 1990182 refresh cs to ch #14628
Conversation
In case an OS for the Series is not provided in the Platform, determine the OS if a series is specified. Helps to ensure that charmhub requests are valid.
In case an OS for the Series is not provided in the Platform, determine the OS if a series is specified. Helps to ensure that charmhub requests are valid. Validate the origin has a platform.
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.
Aside from bundles complaining, this LGTM.
Represent changes to DeduceOrigin and Resolve charm where an OS is added to the origin if a series is present.
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.
This looks good to me, assuming integration tests come in a follow-up PR. It's a pity we need to do the GetOSFromSeries
dance, but oh well. I didn't get a chance to run QA steps today, but will tomorrow morning.
/merge |
2 similar comments
/merge |
/merge |
#14661 Merge 2.9 #14637 #14638 #14641 #14612 #14646 #14647 #14649 #14628 #14654 Conflicts ``` # Conflicts: # apiserver/facades/client/application/deploy.go # apiserver/facades/client/charmhub/charmhub_test.go # apiserver/facades/client/charmhub/convert.go # apiserver/facades/client/charms/conversions.go # apiserver/facades/client/machinemanager/machinemanager.go # apiserver/facades/client/machinemanager/machinemanager_test.go # apiserver/facades/client/machinemanager/register.go # apiserver/facades/client/resources/repository.go # apiserver/facades/schema.json # cmd/juju/application/deploy_test.go # cmd/juju/application/deployer/bundlehandler.go # cmd/juju/machine/add.go # core/charm/strategies.go # core/charm/strategies_test.go # core/series/supportedseries.go # core/series/supportedseries_test.go # environs/manual/winrmprovisioner/provisioner_test.go # rpc/params/applications.go # rpc/params/params.go # state/charm.go # tests/includes/storage.sh ``` ## Checklist - [X] Code style: imports ordered, good names, simple structure, etc - [X] Comments saying why design decisions were made - [X] Go unit tests, with comments saying what you're testing - ~[ ] [Integration tests](https://github.com/juju/juju/tree/develop/tests), with comments saying what you're testing~ - ~[ ] [doc.go](https://discourse.charmhub.io/t/readme-in-packages/451) added or updated in changed packages~ ## QA steps See PRs
The charm origin for charmstore charms does not contain an os, but does contain a series. This creates a problem when refreshing an application using the switch flag to go from a charmstore version of the charm to a charmhub version of the charm. The refresh request made to charmhub is containing an Series while missing the OS and fails.
No upgrade step is necessary due to the validation and fixing done by these changes in this PR. If a series is correct, juju can always determine an OS for it. Do so if DeduceOrigin or ResolveCharm only have a series in the arguments. This is helps protect against bad requests from non juju clients as well when ResolveCharm is called.
Drive by change to not require mongo in testing when it's not used included
QA steps
Bug reference
This PR fixes part of the bug. To fix the remainer, an
apt update
,apt upgrade
of the controller machines is required. The/usr/share/distro-info/ubuntu.csv
file is out of date. Jujud on the controller(s) must be restarted after the file is updated.https://bugs.launchpad.net/juju/+bug/1990182