Do not try to backport when db has older object version

Instance extras are stored as serialized objects in the database and
because of this it is possible to get a version back that is older
than what the client requested. This is in itself not a problem, but
the way we do things right now in conductor we end up trying to
backport to a newer version, which raises InvalidTargetVersion.

This change adds a check to make sure we skip backporting if the
requested version is newer than the actual db version as long as the
major version is the same.

Change-Id: I34ac0abd016b72d585f83ae2ce34790751082180
Closes-Bug: #1596119
(cherry picked from commit 1e3e7309997f90fbd0291c05cc859dd9ac0ef161)

2 files changed, 66 insertions, 4 deletions

diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py
index 8dbfb4806f..7461b9df03 100644
--- a/nova/conductor/manager.py
+++ b/nova/conductor/manager.py
@@ -20,6 +20,7 @@ from oslo_config import cfg
 from oslo_log import log as logging
 import oslo_messaging as messaging
 from oslo_utils import excutils
+from oslo_utils import versionutils
 import six
 
 from nova.compute import rpcapi as compute_rpcapi
@@ -95,10 +96,21 @@ class ConductorManager(manager.Manager):
         # NOTE(danms): The RPC layer will convert to primitives for us,
         # but in this case, we need to honor the version the client is
         # asking for, so we do it before returning here.
-        return (result.obj_to_primitive(
-            target_version=object_versions[objname],
-            version_manifest=object_versions)
-                if isinstance(result, nova_object.NovaObject) else result)
+        # NOTE(hanlind): Do not convert older than requested objects,
+        # see bug #1596119.
+        if isinstance(result, nova_object.NovaObject):
+            target_version = object_versions[objname]
+            requested_version = versionutils.convert_version_to_tuple(
+                target_version)
+            actual_version = versionutils.convert_version_to_tuple(
+                result.VERSION)
+            do_backport = requested_version < actual_version
+            other_major_version = requested_version[0] != actual_version[0]
+            if do_backport or other_major_version:
+                result = result.obj_to_primitive(
+                    target_version=target_version,
+                    version_manifest=object_versions)
+        return result
 
     def object_action(self, context, objinst, objmethod, args, kwargs):
         """Perform an action on an object."""
diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py
index b89e82fac5..ed14e1dc8b 100644
--- a/nova/tests/unit/conductor/test_conductor.py
+++ b/nova/tests/unit/conductor/test_conductor.py
@@ -22,6 +22,7 @@ import mock
 from mox3 import mox
 import oslo_messaging as messaging
 from oslo_utils import timeutils
+from oslo_versionedobjects import exception as ovo_exc
 import six
 
 from nova.compute import flavors
@@ -182,6 +183,55 @@ class ConductorTestCase(_BaseTestCase, test.TestCase):
             m.return_value.obj_to_primitive.assert_called_once_with(
                 target_version='1.2', version_manifest=versions)
 
+    def test_object_class_action_versions_old_object(self):
+        # Make sure we return older than requested objects unmodified,
+        # see bug #1596119.
+        @obj_base.NovaObjectRegistry.register
+        class TestObject(obj_base.NovaObject):
+            VERSION = '1.10'
+
+            @classmethod
+            def foo(cls, context):
+                return cls()
+
+        versions = {
+            'TestObject': '1.10',
+            'OtherObj': '1.0',
+        }
+        with mock.patch.object(self.conductor_manager,
+                               '_object_dispatch') as m:
+            m.return_value = TestObject()
+            m.return_value.VERSION = '1.9'
+            m.return_value.obj_to_primitive = mock.MagicMock()
+            obj = self.conductor.object_class_action_versions(
+                self.context, TestObject.obj_name(), 'foo', versions,
+                tuple(), {})
+            self.assertFalse(m.return_value.obj_to_primitive.called)
+            self.assertEqual('1.9', obj.VERSION)
+
+    def test_object_class_action_versions_major_version_diff(self):
+        @obj_base.NovaObjectRegistry.register
+        class TestObject(obj_base.NovaObject):
+            VERSION = '2.10'
+
+            @classmethod
+            def foo(cls, context):
+                return cls()
+
+        versions = {
+            'TestObject': '2.10',
+            'OtherObj': '1.0',
+        }
+        with mock.patch.object(self.conductor_manager,
+                               '_object_dispatch') as m:
+            m.return_value = TestObject()
+            m.return_value.VERSION = '1.9'
+            self.assertRaises(
+                ovo_exc.InvalidTargetVersion,
+                self.conductor.object_class_action_versions,
+                self.context, TestObject.obj_name(), 'foo', versions,
+                tuple(), {})
+
     def test_reset(self):
         with mock.patch.object(objects.Service, 'clear_min_version_cache'
                                ) as mock_clear_cache: