From fa0cb98c2119edffbb9c10aaefa929d4a8767f70 Mon Sep 17 00:00:00 2001 From: Ruby Loo Date: Thu, 2 Aug 2018 13:43:51 +0000 Subject: [PATCH] Use max version of an object In the code for handling rolling upgrades (and pinned ironic versions), we were assuming that for an object class (eg 'Node') that had more than one version in a release, the first version was the latest one. This was error-prone as humans don't always know/remember to put the latest version as the first one in the list. This patch removes the assumption and gets the maximum version in the list. Change-Id: Ie0a789f8bfa6f286b58cc7211be85ee18b2800f5 --- ironic/common/release_mappings.py | 7 +++--- ironic/objects/base.py | 24 +++++++++++++++--- ironic/tests/unit/objects/test_objects.py | 30 +++++++++++++++++++++++ 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 855702b42a..a64adb1cca 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -40,9 +40,10 @@ from ironic.common.i18n import _ # sent over RPC. Notifications/Payloads are not being included here since we # don't need to pin them during rolling upgrades. # -# For each object, the versions of that object are listed from latest version -# to oldest version. This is a list since an object could be in more than one -# version in a particular release. +# For each object, list the versions that the object can be in for a particular +# release. That is, any new versions that were added in that release. If there +# were no new versions, it should have the same (latest) version as the +# previous release. # NOTE(rloo): We need a list, not just the latest version, for the DB queries # that filter for objects that are not in particular versions; for more info, # see comments after L1128 of diff --git a/ironic/objects/base.py b/ironic/objects/base.py index f29e4e3472..a266215394 100644 --- a/ironic/objects/base.py +++ b/ironic/objects/base.py @@ -27,6 +27,24 @@ from ironic.objects import fields as object_fields LOG = log.getLogger(__name__) +def max_version(versions): + """Return the maximum version in the list. + + :param versions: a list of (string) versions; assumed to have at + least one entry + :returns: the maximum version (string) + """ + if len(versions) == 1: + return versions[0] + + int_versions = [] + for v in versions: + int_versions.append(versionutils.convert_version_to_int(v)) + max_val = max(int_versions) + ind = int_versions.index(max_val) + return versions[ind] + + class IronicObjectRegistry(object_base.VersionedObjectRegistry): def registration_hook(self, cls, index): # NOTE(jroll): blatantly stolen from nova @@ -187,9 +205,9 @@ class IronicObject(object_base.VersionedObject): return cls.VERSION version_manifest = versions.RELEASE_MAPPING[pin]['objects'] - pinned_version = version_manifest.get(cls.obj_name()) - if pinned_version: - pinned_version = pinned_version[0] + pinned_versions = version_manifest.get(cls.obj_name()) + if pinned_versions: + pinned_version = max_version(pinned_versions) if not versionutils.is_compatible(pinned_version, cls.VERSION): LOG.error( diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 2e89f0ebb3..3c7d1782e4 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -386,6 +386,19 @@ class _TestObject(object): } self._test_get_changes(target_version='1.4') + @mock.patch('ironic.common.release_mappings.RELEASE_MAPPING', + autospec=True) + def test_get_changes_pinned_2versions(self, mock_release_mapping): + # obj_get_changes() is not affected by pinning + CONF.set_override('pin_release_version', + release_mappings.RELEASE_VERSIONS[-1]) + mock_release_mapping.__getitem__.return_value = { + 'objects': { + 'MyObj': ['1.3', '1.4'], + } + } + self._test_get_changes(target_version='1.4') + def test_convert_to_version_same(self): # no changes obj = MyObj(self.context) @@ -986,3 +999,20 @@ class TestRegistry(test_base.TestCase): self.assertEqual(MyObj, mock_objects.MyObj) reg.registration_hook(MyOlderObj, 0) self.assertEqual(MyObj, mock_objects.MyObj) + + +class TestMisc(test_base.TestCase): + def test_max_version(self): + versions = ['1.25', '1.33', '1.3'] + maxv = base.max_version(versions) + self.assertEqual('1.33', maxv) + + def test_max_version_one(self): + versions = ['1.25'] + maxv = base.max_version(versions) + self.assertEqual('1.25', maxv) + + def test_max_version_two(self): + versions = ['1.25', '1.26'] + maxv = base.max_version(versions) + self.assertEqual('1.26', maxv)