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-4779] Ensure valid charm origin for local charm switches #978

Merged

Conversation

jack-w-shaw
Copy link
Member

Ensure that we provide the server with a valid charm origin when switching to local charms. Previously, we simply sent back the same charm origin when switching to a local charm.

However, this is not valid, since we don't know where we switched from. If it's a charmhub charm, it's origin will not be valid.

Also, parse the revision from the new charm url the server sends us

This is equivalent to: juju/juju#16434

QA Steps

Edit line 26 of examples/local_refresh.py to point to a local ubuntu charm

$ juju deploy ubuntu
$ juju mongo
juju:PRIMARY> db.applications.find().pretty()
...
{
	"_id" : "277dae9c-abe3-4aba-83e4-0a63622e2981:ubuntu",
	"name" : "ubuntu",
	"model-uuid" : "277dae9c-abe3-4aba-83e4-0a63622e2981",
	"subordinate" : false,
	"charmurl" : "ch:amd64/focal/ubuntu-24",
	"charm-origin" : {
		"source" : "charm-hub",
		"type" : "charm",
		"id" : "DksXQKAQTZfsUmBAGanZAhpoS4dpmXel",
		"hash" : "9b2877f7ebf04b348cf40ace32fdfc6d1cc0157dea7fd6740c4ba74e0e201e5b",
		"revision" : 24,
		"channel" : {
			"risk" : "stable"
		},
		"platform" : {
			"architecture" : "amd64",
			"os" : "ubuntu",
			"channel" : "22.04"
		}
	},
	"charmmodifiedversion" : 0,
	"forcecharm" : false,
	"life" : 0,
	"unitcount" : 1,
	"relationcount" : 0,
	"minunits" : 0,
	"txn-revno" : NumberLong(3),
	"metric-credentials" : BinData(0,""),
	"exposed" : false,
	"scale" : 0,
	"passwordhash" : "",
	"provisioning-state" : null
}
 
$ python examples/local_refresh.py
$ juju mongo
juju:PRIMARY> db.applications.find().pretty()
...
{
	"_id" : "277dae9c-abe3-4aba-83e4-0a63622e2981:ubuntu",
	"name" : "ubuntu",
	"model-uuid" : "277dae9c-abe3-4aba-83e4-0a63622e2981",
	"subordinate" : false,
	"charmurl" : "local:jammy/ubuntu-13",
	"charm-origin" : {
		"source" : "local",
		"type" : "charm",
		"id" : "DksXQKAQTZfsUmBAGanZAhpoS4dpmXel",
		"hash" : "9b2877f7ebf04b348cf40ace32fdfc6d1cc0157dea7fd6740c4ba74e0e201e5b",
		"revision" : 13,
		"channel" : {
			"risk" : "stable"
		},
		"platform" : {
			"architecture" : "amd64",
			"os" : "ubuntu",
			"channel" : "22.04"
		}
	},
	"charmmodifiedversion" : 1,
	"forcecharm" : false,
	"life" : 0,
	"unitcount" : 1,
	"relationcount" : 0,
	"minunits" : 0,
	"txn-revno" : NumberLong(4),
	"metric-credentials" : BinData(0,""),
	"exposed" : false,
	"scale" : 0,
	"passwordhash" : "",
	"provisioning-state" : null
}

NOTE: This doesn't quite work as expected, but this is not a python-libjuju bug, but in juju/juju here:
https://github.com/juju/juju/blob/3.3/state/application.go#L1792-L1840

However, @hmlanigan is working on a fix (juju/juju#16315) that will resolve this, so proposing this change to python-libjuju as a stepping stone towards to complete solution

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

All CI tests need to pass.

@jack-w-shaw jack-w-shaw force-pushed the JUJU-4779_switch_to_local_properly-master branch from f70e983 to aa03f10 Compare October 26, 2023 19:35
@cderici cderici added the 3.x label Oct 26, 2023
@cderici
Copy link
Member

cderici commented Oct 26, 2023

/merge

Ensure that we provide the server with a valid charm origin when
switching to local charms. Previosuly, we simply sent back the same
charm origin when switching to a local charm.

However, this is not valid, since we don't know where we switched from.
If it's a charmhub charm, it's origin will not be valid.

Also, parse the revision from the new charm url the server sends us

This is equivalent to: juju/juju#16434
@jack-w-shaw jack-w-shaw force-pushed the JUJU-4779_switch_to_local_properly-master branch from aa03f10 to d78c5e0 Compare October 26, 2023 20:55
@cderici
Copy link
Member

cderici commented Oct 26, 2023

/merge

@jujubot jujubot merged commit 9e0e39d into juju:master Oct 26, 2023
7 of 8 checks passed
@jack-w-shaw jack-w-shaw deleted the JUJU-4779_switch_to_local_properly-master branch October 26, 2023 21:06
@cderici cderici mentioned this pull request Oct 26, 2023
jujubot added a commit that referenced this pull request Oct 26, 2023
#980

## What's Changed
* Repository Maintenance Improvements by @cderici in #922
* Stale bot to not bother feature requests by @cderici in #926
* Fix linter issues by @cderici in #928
* Fix docstring typo by @DanielArndt in #927
* Fix asyncio on README by @marceloneppel in #930
* Fix integration/test_application.test_action by @cderici in #932
* Update 3.2 facade clients by @cderici in #931
* [JUJU-4488] Add licence headers to source files on 3.x by @cderici in #934
* Update async tests to use builtin python suite by @DanielArndt in #935
* Pass correct charm url to series selector by @cderici in #942
* Green CI cleanup for python-libjuju by @cderici in #939
* Bring forward support for nested assumes expressions on 3x by @cderici in #943
* Release 3.2.2.0 notes by @cderici in #945
* Cleanup release process for 3.x by @cderici in #946
* Use new DeployFromRepository endpoint for deploy by @cderici in #949
* Handle pending upload resources deployfromrepository by @cderici in #953
* Optimize connection teardown by @cderici in #952
* Use `log.warning` instead of the deprecated `warn` by @sed-i in #954
* Find controller name by endpoint on 3.x track by @cderici in #966
* Optimize & fix unit removal by @cderici in #967
* Allow switch kwarg in refresh to switch to local charms by @jack-w-shaw in #971
* Parse charm URLs consistantly for local charms by @jack-w-shaw in #974
* Juju config directory location fix on 3.x by @cderici in #976
* [JUJU-4779] Ensure valid charm origin for local charm switches by @jack-w-shaw in #978
* Application refresh with resources on 3.x by @cderici in #973

#### Notes & Discussion

JUJU-4851

[JUJU-4488]: https://warthogs.atlassian.net/browse/JUJU-4488?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
[JUJU-4779]: https://warthogs.atlassian.net/browse/JUJU-4779?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants