canonical / juju-verify

https://launchpad.net/juju-verify
GNU General Public License v3.0
2 stars 7 forks source link

New naming conventions in charmhub #121

Open esunar opened 2 years ago

esunar commented 2 years ago

We'll have to rethink the helper function parse_charm_name()[1] in regards to migration from jaas.ai to charmhub. This function uses "charm_url" to determine what software is the charm running and with the migration to charmhub.io, the naming conventions are changed.

There are no longer a namespaces separated by "/" and what was previously "openstack-charmers-next/nova-compute" is now "openstack- charmer-next-nova-compute", this will break the parsing algorithm. We'll have to also revise how tracks and channels are reflected in the charm_url.


[1] https://github.com/canonical/juju-verify/blob/f29a090cb36cbc59a89086da733cfd3a94f10882/juju_verify/utils/unit.py#L153


Imported from Launchpad using lp2gh.

esunar commented 2 years ago

(by martin-kalcok) After tests with new charmhub URLs, it seems that we wont have to actually change the regexp we use to match charm name.

I tested deployments with charms defined with just their name

nova-compute:
  charm: nova-compute
  channel: candidate

or with more specific url

nova-compute:
  charm: ch:bionic/nova-compute
  channel: candidate

and they are always resolved to full url "ch:amd64/bionic/nova-compute-549". This url then works well with our current regex pattern "^(.):(./)?(?P.*)(-\d+)$" and produces expected charm name.

Now, regarding charm namespaces that were previously used (like "openstack-charmers-next"), these namespaces are no longer separated by "/". This might not be problem for production usage, but it's likely that during the development we might be forced to use non-standard charm and we'll need some way to tell juju-verify which verifier it should use to test target units.

The most simplistic solution would be to ignore "known" prefixes (e.g. openstack-charmers-next) but that's pretty simplistic approach and wont cover all the scenarios.

Another option is to introduce new CLI option that would force mapping between units. Let's call it map-charm with following usage --map-charm <UNIT_NAME>:<CHARM_NAME>. Automatic charm discovery [1] would be disabled for any unit that would be specified via --map-charm.

Example of usage:

# Single unit
juju-verify shutdown --unit nova-compute/0 --map-charm nova-compute/0:nova-compute

# Multiple units and charms
juju-verify shutdown --unit nova-compute/0 nova-compute/1 ceph-osd/0 --map-charm nova-compute/0:nova-compute --map-charm nova-compute/0:nova-compute --map-charm ceph-osd/0:ceph-osd

This CLI option would come with warning that user should know what they are doing with this explicit mapping and that it can lead to errors if used incorrectly. I understand that this solution is bit verbose, especially when dealing with multiple units, but I think that it's most comprehensive and verbosity is not such issue as it's mostly meant to be used in development.


[1] https://github.com/canonical/juju-verify/blob/f29a090cb36cbc59a89086da733cfd3a94f10882/juju_verify/verifiers/__init__.py#L59

esunar commented 2 years ago

(by rgildein) What do you think about adding openstack-charmers-next-nova-compute to SUPPORTED_CHARMS 1. IMO it would be more explicit because there are charms like percona-cluster that don't have openstack-charmers-next version.

For custom spells purposes, I like the idea of adding --map-charm CLI option, however I think it could map applications names. e.g. : The warning message is also a good idea and IMO we should also add a check for .

e.g.

juju-verify ... --map-charm custom-nova-compute:nova-cmpute  # typo in nova-compute
usage: juju-verify [-h] [--model MODEL] [-l {trace,debug,info}] [-s]
                   (--units UNITS [UNITS ...] | --machines MACHINES [MACHINES ...])
                   {shutdown,reboot}
juju-verify: error: argument map-charm: invalid charm: 'nova-cmpute' (choose from 'nova-compute`, `ceph-osd`, ...)

esunar commented 2 years ago

(by martin-kalcok) I think that adding explicitly things like openstack-charmers-next-nova-compute into the internal mappings as supported charms is just a bandaid solution and does not cover all the use cases during development. For example when an early implementation of charm feature is held in personal namespace before being approved and merged to the pre-release.

I do like the idea of mapping whole application instead of units. It'd remove an unnecessary repetition when checking multiple units of the same application.

As for the checking validity of the mapped charm name, of course there'd be some checks in place, I didn't go into such implementation details in my previous comments.

esunar commented 2 years ago

(by aluria) Alternatively, mapping between the charm_url used (e.g. openstack-charmers-next-nova-compute) and the supported verifier (e.g. nova-compute) would be added via "--map-charm :" (or "--map-verifier :").

If multiple applications of the same charm are deployed, this would help specify a single time the verifier that would be needed for all the applications.

esunar commented 2 years ago

(by rgildein) Hi Martin, was is status of this issue? Do we want to fixed in 22.10 release? Thanks

esunar commented 2 years ago

(by martin-kalcok) The charm mapping was actually implemented here: https://github.com/canonical/juju-verify/pull/66

I think we just forgot to mark this issue as "Done"

Edit: The change was merged after the last release. So it should be part of the 22.10 release.