diff --git a/ironic/common/rpc_service.py b/ironic/common/rpc_service.py index 5a2aeacfe8..a2aebb56c4 100644 --- a/ironic/common/rpc_service.py +++ b/ironic/common/rpc_service.py @@ -46,7 +46,7 @@ class RPCService(service.Service): target = messaging.Target(topic=self.topic, server=self.host) endpoints = [self.manager] - serializer = objects_base.IronicObjectSerializer() + serializer = objects_base.IronicObjectSerializer(is_server=True) self.rpcserver = rpc.get_server(target, endpoints, serializer) self.rpcserver.start() diff --git a/ironic/objects/base.py b/ironic/objects/base.py index c7dd303c07..78dfedde98 100644 --- a/ironic/objects/base.py +++ b/ironic/objects/base.py @@ -80,26 +80,71 @@ class IronicObject(object_base.VersionedObject): self[field] != loaded_object[field]): self[field] = loaded_object[field] - def _convert_to_version(self, target_version): + def _convert_to_version(self, target_version, remove_unavail_fields=True): """Convert to the target version. - Subclasses should redefine this method, to do the conversion - of the object to the specified version. As a result of any - conversion, the object changes (self.obj_what_changed()) should - be retained. + Subclasses should redefine this method, to do the conversion of the + object to the target version. + + Convert the object to the target version. The target version may be + the same, older, or newer than the version of the object. This is + used for DB interactions as well as for serialization/deserialization. + + The remove_unavail_fields flag is used to distinguish these two cases: + + 1) For serialization/deserialization, we need to remove the unavailable + fields, because the service receiving the object may not know about + these fields. remove_unavail_fields is set to True in this case. + + 2) For DB interactions, we need to set the unavailable fields to their + appropriate values so that these fields are saved in the DB. (If + they are not set, the VersionedObject magic will not know to + save/update them to the DB.) remove_unavail_fields is set to False + in this case. :param target_version: the desired version of the object + :param remove_unavail_fields: True to remove fields that are + unavailable in the target version; set this to True when + (de)serializing. False to set the unavailable fields to appropriate + values; set this to False for DB interactions. """ pass - def convert_to_version(self, target_version): + def convert_to_version(self, target_version, remove_unavail_fields=True): """Convert this object to the target version. + Convert the object to the target version. The target version may be + the same, older, or newer than the version of the object. This is + used for DB interactions as well as for serialization/deserialization. + + The remove_unavail_fields flag is used to distinguish these two cases: + + 1) For serialization/deserialization, we need to remove the unavailable + fields, because the service receiving the object may not know about + these fields. remove_unavail_fields is set to True in this case. + + 2) For DB interactions, we need to set the unavailable fields to their + appropriate values so that these fields are saved in the DB. (If + they are not set, the VersionedObject magic will not know to + save/update them to the DB.) remove_unavail_fields is set to False + in this case. + _convert_to_version() does the actual work. :param target_version: the desired version of the object + :param remove_unavail_fields: True to remove fields that are + unavailable in the target version; set this to True when + (de)serializing. False to set the unavailable fields to appropriate + values; set this to False for DB interactions. """ - self._convert_to_version(target_version) + if self.VERSION != target_version: + self._convert_to_version( + target_version, remove_unavail_fields=remove_unavail_fields) + if remove_unavail_fields: + # NOTE(rloo): We changed the object, but don't keep track of + # any of these changes, since it is inaccurate anyway (because + # it doesn't keep track of any 'changed' unavailable fields). + self.obj_reset_changes() # NOTE(rloo): self.__class__.VERSION is the latest version that # is supported by this service. self.VERSION is the version of @@ -210,15 +255,9 @@ class IronicObject(object_base.VersionedObject): if db_version != obj.__class__.VERSION: # convert to the latest version - obj.convert_to_version(obj.__class__.VERSION) - if obj.get_target_version() == db_version: - # pinned, so no need to keep these changes (we'll end up - # converting back to db_version if obj is saved) - obj.obj_reset_changes() - else: - # keep these changes around because they are needed - # when/if saving to the DB in the latest version - pass + obj.VERSION = db_version + obj.convert_to_version(obj.__class__.VERSION, + remove_unavail_fields=False) return obj @@ -262,16 +301,14 @@ class IronicObject(object_base.VersionedObject): if target_version != self.VERSION: # Convert the object so we can save it in the target version. - self.convert_to_version(target_version) - db_version = target_version - else: - db_version = self.VERSION + self.convert_to_version(target_version, + remove_unavail_fields=False) changes = self.obj_get_changes() # NOTE(rloo): Since this object doesn't keep track of the version that # is saved in the DB and we don't want to make a DB call # just to find out, we always update 'version' in the DB. - changes['version'] = db_version + changes['version'] = self.VERSION return changes @@ -280,6 +317,16 @@ class IronicObjectSerializer(object_base.VersionedObjectSerializer): # Base class to use for object hydration OBJ_BASE_CLASS = IronicObject + def __init__(self, is_server=False): + """Initialization. + + :param is_server: True if the service using this Serializer is a + server (i.e. an ironic-conductor). Default is False for clients + (such as ironic-api). + """ + super(IronicObjectSerializer, self).__init__() + self.is_server = is_server + def _process_object(self, context, objprim): """Process the object. @@ -288,8 +335,8 @@ class IronicObjectSerializer(object_base.VersionedObjectSerializer): deserialization process converts them to Objects. This converts any IronicObjects to be in their latest versions, - so that the services (ironic-api and ironic-conductor) internally, - always deal objects in their latest versions. + so that internally, the services (ironic-api and ironic-conductor) + always deal with objects in their latest versions. :param objprim: a serialized entity that represents an object :returns: the deserialized Object @@ -299,7 +346,11 @@ class IronicObjectSerializer(object_base.VersionedObjectSerializer): context, objprim) if isinstance(obj, IronicObject): if obj.VERSION != obj.__class__.VERSION: - obj.convert_to_version(obj.__class__.VERSION) + # NOTE(rloo): if deserializing at API (client) side, + # we don't want any changes + obj.convert_to_version( + obj.__class__.VERSION, + remove_unavail_fields=not self.is_server) return obj def serialize_entity(self, context, entity): @@ -310,9 +361,14 @@ class IronicObjectSerializer(object_base.VersionedObjectSerializer): 'ironic_object.namespace', 'ironic_object.data', 'ironic_object.name', 'ironic_object.version', and 'ironic_object.changes'. - For IronicObjects, if the service (ironic-api or ironic-conductor) - is pinned, we want the object to be in the target/pinned version, - which is not necessarily the latest version of the object. + We assume that the client (ironic-API) is always talking to a + server (ironic-conductor) that is running the same or a newer + release than the client. The client doesn't need to downgrade + any IronicObjects when sending them over RPC. The server, on + the other hand, will need to do so if the server is pinned and + the target version of an IronicObject is older than the latest + version of that Object. + (Internally, the services deal with the latest versions of objects so we know that these objects are always in the latest versions.) @@ -322,13 +378,14 @@ class IronicObjectSerializer(object_base.VersionedObjectSerializer): :raises: ovo_exception.IncompatibleObjectVersion (via .get_target_version()) """ - if isinstance(entity, IronicObject): + if self.is_server and isinstance(entity, IronicObject): target_version = entity.get_target_version() if target_version != entity.VERSION: # NOTE(xek): If the version is pinned, target_version is an # older object version. We need to backport/convert to target # version before serialization. - entity.convert_to_version(target_version) + entity.convert_to_version(target_version, + remove_unavail_fields=True) return super(IronicObjectSerializer, self).serialize_entity( context, entity) diff --git a/ironic/tests/unit/common/test_rpc_service.py b/ironic/tests/unit/common/test_rpc_service.py index b1e0647306..755df4f81c 100644 --- a/ironic/tests/unit/common/test_rpc_service.py +++ b/ironic/tests/unit/common/test_rpc_service.py @@ -48,6 +48,6 @@ class TestRPCService(base.TestCase): mock_ctx.assert_called_once_with() mock_target.assert_called_once_with(topic=self.rpc_svc.topic, server="fake_host") - mock_ios.assert_called_once_with() + mock_ios.assert_called_once_with(is_server=True) mock_init_method.assert_called_once_with(self.rpc_svc.manager, mock_ctx.return_value) diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 678c11ef0a..b4892b0163 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -90,11 +90,14 @@ class MyObj(base.IronicObject, object_base.VersionedObjectDictCompat): self.save() self.foo = 42 - def _convert_to_version(self, target_version): + def _convert_to_version(self, target_version, remove_unavail_fields=True): if target_version == '1.5': self.missing = 'foo' elif self.missing: - self.missing = '' + if remove_unavail_fields: + delattr(self, 'missing') + else: + self.missing = '' class MyObj2(object): @@ -381,18 +384,18 @@ class _TestObject(object): self._test_get_changes(target_version='1.4') def test_convert_to_version_same(self): - # missing is added + # no changes obj = MyObj(self.context) self.assertEqual('1.5', obj.VERSION) - obj.convert_to_version('1.5') + obj.convert_to_version('1.5', remove_unavail_fields=False) self.assertEqual('1.5', obj.VERSION) self.assertEqual(obj.__class__.VERSION, obj.VERSION) - self.assertEqual({'missing': 'foo'}, obj.obj_get_changes()) + self.assertEqual({}, obj.obj_get_changes()) def test_convert_to_version_new(self): obj = MyObj(self.context) obj.VERSION = '1.4' - obj.convert_to_version('1.5') + obj.convert_to_version('1.5', remove_unavail_fields=False) self.assertEqual('1.5', obj.VERSION) self.assertEqual(obj.__class__.VERSION, obj.VERSION) self.assertEqual({'missing': 'foo'}, obj.obj_get_changes()) @@ -401,7 +404,15 @@ class _TestObject(object): obj = MyObj(self.context) obj.missing = 'something' obj.obj_reset_changes() - obj.convert_to_version('1.4') + obj.convert_to_version('1.4', remove_unavail_fields=True) + self.assertEqual('1.4', obj.VERSION) + self.assertEqual({}, obj.obj_get_changes()) + + def test_convert_to_version_old_keep(self): + obj = MyObj(self.context) + obj.missing = 'something' + obj.obj_reset_changes() + obj.convert_to_version('1.4', remove_unavail_fields=False) self.assertEqual('1.4', obj.VERSION) self.assertEqual({'missing': ''}, obj.obj_get_changes()) @@ -809,10 +820,32 @@ class TestObjectSerializer(test_base.TestCase): @mock.patch.object(base.IronicObject, 'convert_to_version', autospec=True) @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) - def test_serialize_entity_unpinned(self, mock_version, mock_convert): + def test_serialize_entity_unpinned_api(self, mock_version, mock_convert): """Test single element serializer with no backport, unpinned.""" mock_version.return_value = MyObj.VERSION - serializer = base.IronicObjectSerializer() + serializer = base.IronicObjectSerializer(is_server=False) + obj = MyObj(self.context) + obj.foo = 1 + obj.bar = 'text' + obj.missing = 'textt' + primitive = serializer.serialize_entity(self.context, obj) + self.assertEqual('1.5', primitive['ironic_object.version']) + data = primitive['ironic_object.data'] + self.assertEqual(1, data['foo']) + self.assertEqual('text', data['bar']) + self.assertEqual('textt', data['missing']) + changes = primitive['ironic_object.changes'] + self.assertEqual(set(['foo', 'bar', 'missing']), set(changes)) + self.assertFalse(mock_version.called) + self.assertFalse(mock_convert.called) + + @mock.patch.object(base.IronicObject, 'convert_to_version', autospec=True) + @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) + def test_serialize_entity_unpinned_conductor(self, mock_version, + mock_convert): + """Test single element serializer with no backport, unpinned.""" + mock_version.return_value = MyObj.VERSION + serializer = base.IronicObjectSerializer(is_server=True) obj = MyObj(self.context) obj.foo = 1 obj.bar = 'text' @@ -829,11 +862,30 @@ class TestObjectSerializer(test_base.TestCase): self.assertFalse(mock_convert.called) @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) - def test_serialize_entity_pinned(self, mock_version): + def test_serialize_entity_pinned_api(self, mock_version): """Test single element serializer with backport to pinned version.""" mock_version.return_value = '1.4' - serializer = base.IronicObjectSerializer() + serializer = base.IronicObjectSerializer(is_server=False) + obj = MyObj(self.context) + obj.foo = 1 + obj.bar = 'text' + obj.missing = 'miss' + self.assertEqual('1.5', obj.VERSION) + primitive = serializer.serialize_entity(self.context, obj) + self.assertEqual('1.5', primitive['ironic_object.version']) + data = primitive['ironic_object.data'] + self.assertEqual(1, data['foo']) + self.assertEqual('text', data['bar']) + self.assertEqual('miss', data['missing']) + self.assertFalse(mock_version.called) + + @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) + def test_serialize_entity_pinned_conductor(self, mock_version): + """Test single element serializer with backport to pinned version.""" + mock_version.return_value = '1.4' + + serializer = base.IronicObjectSerializer(is_server=True) obj = MyObj(self.context) obj.foo = 1 obj.bar = 'text' @@ -844,9 +896,8 @@ class TestObjectSerializer(test_base.TestCase): data = primitive['ironic_object.data'] self.assertEqual(1, data['foo']) self.assertEqual('text', data['bar']) - self.assertEqual('', data['missing']) - changes = primitive['ironic_object.changes'] - self.assertEqual(set(['foo', 'bar', 'missing']), set(changes)) + self.assertNotIn('missing', data) + self.assertNotIn('ironic_object.changes', primitive) mock_version.assert_called_once_with(mock.ANY) @mock.patch.object(base.IronicObject, 'get_target_version', autospec=True) @@ -854,21 +905,21 @@ class TestObjectSerializer(test_base.TestCase): mock_version.side_effect = object_exception.InvalidTargetVersion( version='1.6') - serializer = base.IronicObjectSerializer() + serializer = base.IronicObjectSerializer(is_server=True) obj = MyObj(self.context) self.assertRaises(object_exception.InvalidTargetVersion, serializer.serialize_entity, self.context, obj) mock_version.assert_called_once_with(mock.ANY) @mock.patch.object(base.IronicObject, 'convert_to_version', autospec=True) - def test__process_object(self, mock_convert): + def _test__process_object(self, mock_convert, is_server=True): obj = MyObj(self.context) obj.foo = 1 obj.bar = 'text' obj.missing = 'miss' primitive = obj.obj_to_primitive() - serializer = base.IronicObjectSerializer() + serializer = base.IronicObjectSerializer(is_server=is_server) obj2 = serializer._process_object(self.context, primitive) self.assertEqual(obj.foo, obj2.foo) self.assertEqual(obj.bar, obj2.bar) @@ -876,8 +927,14 @@ class TestObjectSerializer(test_base.TestCase): self.assertEqual(obj.VERSION, obj2.VERSION) self.assertFalse(mock_convert.called) + def test__process_object_api(self): + self._test__process_object(is_server=False) + + def test__process_object_conductor(self): + self._test__process_object(is_server=True) + @mock.patch.object(base.IronicObject, 'convert_to_version', autospec=True) - def test__process_object_convert(self, mock_convert): + def _test__process_object_convert(self, is_server, mock_convert): obj = MyObj(self.context) obj.foo = 1 obj.bar = 'text' @@ -885,9 +942,16 @@ class TestObjectSerializer(test_base.TestCase): obj.VERSION = '1.4' primitive = obj.obj_to_primitive() - serializer = base.IronicObjectSerializer() + serializer = base.IronicObjectSerializer(is_server=is_server) serializer._process_object(self.context, primitive) - mock_convert.assert_called_once_with(mock.ANY, '1.5') + mock_convert.assert_called_once_with( + mock.ANY, '1.5', remove_unavail_fields=not is_server) + + def test__process_object_convert_api(self): + self._test__process_object_convert(False) + + def test__process_object_convert_conductor(self): + self._test__process_object_convert(True) class TestRegistry(test_base.TestCase):