diff --git a/charms_openstack/charm/core.py b/charms_openstack/charm/core.py index d849dff..6ccef08 100644 --- a/charms_openstack/charm/core.py +++ b/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. diff --git a/unit_tests/__init__.py b/unit_tests/__init__.py index 4def149..3a52e37 100644 --- a/unit_tests/__init__.py +++ b/unit_tests/__init__.py @@ -72,12 +72,75 @@ 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: + """ + 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): diff --git a/unit_tests/charms_openstack/charm/test_core.py b/unit_tests/charms_openstack/charm/test_core.py index a72e56d..fbfa867 100644 --- a/unit_tests/charms_openstack/charm/test_core.py +++ b/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')