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

@@ -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

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(

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)