default_select_release does not take release_pkg into account

[Dmitrii Shcherbakov]

default_select_release does not take release_pkg into account which makes this function rather useless with subordinates (that do not include openstack-principle => openstack-origin) due to the fact that os_release in charm-helpers doesn't return a correct release for 'python-keystonemiddleware' if openstack-origin is not present (see https://github.com/juju/charm-helpers/issues/83)

https://github.com/openstack/charms.openstack/blob/389d928ba50915560b2140e022a663035fa86a4b/charms_openstack/charm/defaults.py#L82-L101

The problem with default release handlers is that they are not aware of <concrete-charm-class>.release_pkg that may be set in a derived charm class way down the class hierarchy.

A custom release selector could avoid that but I would rather avoid this in every subordinate charm:

@register_os_release_selector
def select_release():
    """Determine the release based on the keystone package version.

    Note that this function caches the release after the first install so
    that it doesn't need to keep going and getting it from the package
    information.
    """
    import pdb
    pdb.set_trace()
    release_version = unitdata.kv().get(OPENSTACK_RELEASE_KEY, None)
    if release_version is None:
        release_version = os_utils.os_release(KeystoneLDAPCharm.release_pkg)
        unitdata.kv().set(OPENSTACK_RELEASE_KEY, release_version)
    return release_version

class KeystoneLDAPCharm(charms_openstack.charm.OpenStackCharm):
    abstract_class = False
...
    # Package to derive application version from
    release_pkg = 'keystone'

[Alex Kavanagh (ajkavanagh)]

This is quite interesting to solve. Obviously, the selecting a release is concretely "selecting a class implementation" so we don't know which class is the one we want!

The 'release_pkg' class property appeared after the 'selecting a release' functionality was implemented, which is why it doesn't know/use it.

One thing we could do is to look at all of the concrete classes that have registered themselves (by virtue of inheriting from BaseOpenStackCharm or friends). We could work through these in release order, see if 'release_pkg' exists, determine if the package is actually available, as it might have 'gone' in a later release, and then determine the release.

We could do this optionally if the 'python-keystonemiddleware' package doesn't exist.

It would probably need to be abstracted in some way because we have to support both packages and snaps.

Thoughts?

[Dmitrii Shcherbakov (dmitriis)]

What I think we aim to achieve in the end:

* "static" release_pkg name selection in a charm class;
* runtime release_pkg version detection (via dpkg or snap);
* support for version-specific charm classes (detect a versioned class by just calling provide_charm_instance without any arguments);
* upgrades need to be consistent - there is no ordering like "a primary runs before a subordinate" (haven't thought this out well but how does a subordinate gets notified from a primary so that it knows that an OpenStack package it depends on got upgraded to make sure it runs a version specific action like re-rendering of subordinate templates?)

With this in mind:

* release class attribute defines the first version that a charm class supports;
* release_pkg is just one field, but we need to detect deb vs snap;
  - try to detect a snap first and then a deb without any additional meta-information?
  - snap openstack version detection would be done by a snap's track name?
* release_pkg might change so we need to work through all derived classes, and check if a given release_pkg exists and which version does it have.
  - criteria to select a derived class?
  - work from all non-abstract leaves to BaseOpenStackCharm and use release_pkg of the first package for which there is a matching snap or deb and select a version of that package as a version for class selection. A class may have the "release" attribute so detect_release(<charm_class>.release_pkg) != <charm_class>.release in which case we have to skip this class if <charm_class>.release is higher than <charm_class>.release_pkg

===

Another problem to look into (not directly related to versioning): how do we handle deb-based subordinate charms with snap-based primary charms?

In other words, a snap has a fixed set of dependencies while a subordinate might want to install an additional package (a deb). While we could point a subordinate to write config files to the snap-common directory of a package related to a primary charm, even if confinement is "classic" we need to then make sure that a snap can see whatever a subordinate has installed.

I'd call it "directory multi-homing" because a subordinate might need to be aware of a config_dir used by a snap-based primary to write drop-in files there. In this case we would need to provide additional meta-data about a primary charm's package type to make sure a subordinate can handle it appropriately.

Examples:
* policy.json/policy.yaml drop-ins to a policy.d directory of a primary charm's snap package
* keystone domain-specific backend configurations (keystone-ldap)

[Alex Kavanagh (ajkavanagh)]

I think the principal/subordinate discussion might derail this bug. I'd largely regard it is a layer-, boundary- or responsibility-violation if the design of a subordinate charm breaks depending on what the principal charm is installing and where it is installing it.

e.g. in my view:

* Subordinate charms should not write to spaces 'owned' by the principal charm; e.g. the policy example given. Subordinate charms should supply, over a relation, the information necessary to the principal charm to write to the space 'owned' by that charm.

* Subordinate charms should not install packages that are (conceptually) owned by the principal's application; again it should 'ask' that the appropriate functionality is installed IF it has a tight coupling with the main software; I'd regard 'add-on' packages as 'tight', but 'haproxy' as a separate application.

With that in mind, I suspect that quite a few of the concerns raised around snap/deb and locations would go away.

---

Having had more time to reflect on it, the iteration of 'release_pkg' needs to be backwards; starting from the youngest release charm class backwards and testing for release_pkg (or snapped version), and then picking the first one that works.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/633511

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/633511
Committed: https://git.openstack.org/cgit/openstack/charms.openstack/commit/?id=db3c8fb6c223efe53f9b241236b1191c240d2059
Submitter: Zuul
Branch: master

commit db3c8fb6c223efe53f9b241236b1191c240d2059
Author: Liam Young <email address hidden>
Date: Mon Jan 28 14:33:18 2019 +0000

    Fix broken OpenStack pkg version check

    The current OpenStack version check uses a package which does not
    have semantic versioning and is not listed on the dict of package
    version in charmhelpers. This change fixes this by using a package
    which does have semantic versioning and is in the dict of versions.
    If no package is installed which fit these requirements then
    os_release is relied on to fallback to another method (checking
    openstack-origin).

    Change-Id: I256b2782e83e2619108726eeb27505d5f1854284
    Closes-Bug: #1741628