Support wrapped release names

Charm instances where the release is > zed does not work using the
default_get_charm_instance. The keys stored in the _releases dictionary
are sorted in their alphabetical sorting order, thus causing releases
such as Bobcat to be treated as older releases than Zed.

This change causes the release names to be sorted using the
CompareOpenStackReleases object from the charmhelpers library. This will
ensure that the release names are sorted according the release order
rather than the alphabetical name.

Change-Id: I528a9dcc5511b74186132b57948e18e4f3a1ef0b

charms_openstack/charm/core.py

@@ -103,6 +103,39 @@ class provide_charm_instance(object):
         return False
 
 
+def _sort_releases(releases):
+    """Sorts the list of releases using the known OpenStack releases.
+
+    Sorts the list of releases by their release name. This handles
+    the scenario where the release is newer than the wrapped release of
+    Zed.
+
+    If the releases provided do not contain a known OpenStack release
+    name, this will fall back to sorting by normal string comparison
+    which was the previous behavior.
+
+    :param releases: the iterable of releases to sort
+    :type releases: iterable
+    :return: a list of releases in sorted order according to the OpenStack
+             release date.
+    """
+    try:
+        # Note: the CompareOpenStackReleases class is not compatible
+        # with an old Python 2 cmp parameter nor is it compatible with
+        # the Python 3 alternative to use the functools.cmp_to_key.
+        # Instead, sort the list on the CompareOpenStackReleases objects
+        # and then convert to a list of strings to handle this.
+        openstack_releases = [os_utils.CompareOpenStackReleases(rel)
+                              for rel in releases]
+        openstack_releases = sorted(openstack_releases)
+        return [str(rel) for rel in openstack_releases]
+    except KeyError:
+        # Raised by the CompareOpenStackReleases class when attempting
+        # to compare an unknown openstack release name. Fall back to
+        # standard string comparison logic.
+        return sorted(releases)
+
+
 def default_get_charm_instance(release=None, package_type='deb', *args,
                                **kwargs):
     """Get an instance of the charm based on the release (or use the
@@ -121,8 +154,9 @@ def default_get_charm_instance(release=None, package_type='deb', *args,
     if len(_releases.keys()) == 0:
         raise RuntimeError(
             "No derived BaseOpenStackCharm() classes registered")
-    # Note that this relies on OS releases being in alphabetical order
-    known_releases = sorted(_releases.keys())
+
+    # Ensure the releases are sorted based upon the OpenStack release order
+    known_releases = _sort_releases(_releases.keys())
     cls = None
     if release is None:
         # take the latest version of the charm if no release is passed.

unit_tests/__init__.py

@@ -72,12 +72,76 @@ charmhelpers.contrib.openstack.utils.OPENSTACK_RELEASES = (
     'queens',
     'rocky',
     'stein',
+    'train',
+    'ussuri',
+    'victoria',
+    'wallaby',
+    'xena',
+    'yoga',
+    'zed',
+    'antelope',
+    'bobcat',
+    'caracal',
 )
 
 # charms.reactive uses hookenv.charm_dir which must return a directory
 charmhelpers.core.hookenv.charm_dir.return_value = os.path.curdir
 
 
+class CompareOpenStackReleases(object):
+    # This class is included due to mocking out the charmhelpers libraries
+    # as they behave poorly with apt installs.
+
+    _list = charmhelpers.contrib.openstack.utils.OPENSTACK_RELEASES
+
+    def __init__(self, item):
+        if self._list is None:
+            raise Exception("Must define the _list in the class definition!")
+        try:
+            self.index = self._list.index(item)
+        except Exception:
+            raise KeyError("Item '{}' is not in list '{}'"
+                           .format(item, self._list))
+
+    def __eq__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index == self._list.index(other)
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def __lt__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index < self._list.index(other)
+
+    def __ge__(self, other):
+        return not self.__lt__(other)
+
+    def __gt__(self, other):
+        assert isinstance(other, str) or isinstance(other, self.__class__)
+        return self.index > self._list.index(other)
+
+    def __le__(self, other):
+        return not self.__gt__(other)
+
+    def __str__(self):
+        """Always give back the item at the index so it can be used in
+        comparisons like:
+
+        s_mitaka = CompareOpenStack('mitaka')
+        s_newton = CompareOpenstack('newton')
+
+        assert s_newton > s_mitaka
+
+        @returns: <string>
+        """
+        return self._list[self.index]
+
+
+charmhelpers.contrib.openstack.utils.CompareOpenStackReleases = \
+    CompareOpenStackReleases
+
+
 def _fake_retry(num_retries, base_delay=0, exc_type=Exception):
     def _retry_on_exception_inner_1(f):
         def _retry_on_exception_inner_2(*args, **kwargs):

unit_tests/charms_openstack/charm/test_core.py

@@ -191,7 +191,10 @@ class TestFunctions(BaseOpenStackCharmTest):
         class TestC3(chm_core.BaseOpenStackCharm):
             release = 'mitaka'
 
-        self.C1, self.C2, self.C3 = TestC1, TestC2, TestC3
+        class TestC4(chm_core.BaseOpenStackCharm):
+            release = 'bobcat'
+
+        self.C1, self.C2, self.C3, self.C4 = TestC1, TestC2, TestC3, TestC4
 
     def test_get_exact(self):
         self.assertTrue(
@@ -208,10 +211,14 @@ class TestFunctions(BaseOpenStackCharmTest):
         with self.assertRaises(RuntimeError):
             chm_core.get_charm_instance(release='havana')
 
+    def test_wrapped_release(self):
+        self.assertTrue(
+            isinstance(chm_core.get_charm_instance(release='bobcat'), self.C4))
+
     def test_get_default_release(self):
         # TODO this may be the wrong logic.  Assume latest release if no
         # release is passed?
-        self.assertIsInstance(chm_core.get_charm_instance(), self.C3)
+        self.assertIsInstance(chm_core.get_charm_instance(), self.C4)
 
     def test_optional_interfaces(self):
         self.patch_object(chm_core.relations, 'endpoint_from_flag')