From db9ddd39d3f3d854d38e4b05655ebddd98f1a727 Mon Sep 17 00:00:00 2001 From: Grzegorz Grasza Date: Wed, 16 Sep 2015 15:12:42 +0200 Subject: [PATCH] Implement indirection_api During a rolling upgrade, ironic conductor and api services are running with different versions. When an object is received in an incompatible version, IncompatibleObjectVersion is raised. Implementation of the indirection API allows the object to be backported to a supported version by the conductor (conductor has to be the first service to be upgraded). This change enables backporting of objects from Mitaka. This lays the foundation to be able to do rolling upgrades from Liberty (or from this patch onwards) to Mitaka. There may still be other issues that will need fixing in Mitaka before we will be able to do a full rolling upgrade. Enabling the indirection_api causes all object methods decorated with the remotable or remotable_classmethod to do RPC from ironic-api to ironic-conductor service. To keep the current behavior, I'm removing all remotable decorators on object methods and thus not enabling object RPC calls in this patch. These calls caused random timeouts on the CI gates (probably due to a race in which Nova calls the ironic-api service before ironic-conductor is up). RPC calls made via the indirection_api should be enabled one-by-one, when the implications are fully understood. Change-Id: Ia381348da93f95d764c83f3ec2a2ed5a1db5ad6d Closes-Bug: 1493816 Depends-On: I81400305f166d62aa4612aab54602abb8178b64c --- ironic/cmd/api.py | 4 + ironic/common/exception.py | 4 - ironic/conductor/manager.py | 105 ++++++++++++++++++++++++- ironic/conductor/rpcapi.py | 76 +++++++++++++++++- ironic/objects/base.py | 30 +++++++ ironic/objects/chassis.py | 44 +++++++++-- ironic/objects/conductor.py | 16 +++- ironic/objects/node.py | 73 ++++++++++++++--- ironic/objects/port.py | 56 ++++++++++--- ironic/tests/conductor/test_manager.py | 105 +++++++++++++++++++++++++ ironic/tests/conductor/test_rpcapi.py | 54 ++++++++++--- ironic/tests/objects/test_objects.py | 46 +++++++++++ 12 files changed, 564 insertions(+), 49 deletions(-) diff --git a/ironic/cmd/api.py b/ironic/cmd/api.py index 0ad81996de..ce531a79da 100644 --- a/ironic/cmd/api.py +++ b/ironic/cmd/api.py @@ -28,6 +28,7 @@ from six.moves import socketserver from ironic.api import app from ironic.common.i18n import _LI from ironic.common import service as ironic_service +from ironic.objects import base CONF = cfg.CONF @@ -42,6 +43,9 @@ def main(): # Parse config file and command line options, then start logging ironic_service.prepare_service(sys.argv) + # Enable object backporting via the conductor + base.IronicObject.indirection_api = base.IronicObjectIndirectionAPI() + # Build and start the WSGI app host = CONF.api.host_ip port = CONF.api.port diff --git a/ironic/common/exception.py b/ironic/common/exception.py index afa140ddaf..5a46d67eab 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -350,10 +350,6 @@ class UnsupportedDriverExtension(Invalid): '(disabled or not implemented).') -class IncompatibleObjectVersion(IronicException): - message = _('Version %(objver)s of %(objname)s is not supported') - - class GlanceConnectionFailed(IronicException): message = _("Connection to glance host %(host)s:%(port)s failed: " "%(reason)s") diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 931515d53a..1354a39be6 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -77,6 +77,7 @@ from ironic.conductor import task_manager from ironic.conductor import utils from ironic.db import api as dbapi from ironic import objects +from ironic.objects import base as objects_base MANAGER_TOPIC = 'ironic.conductor_manager' WORKER_SPAWN_lOCK = "conductor_worker_spawn" @@ -211,7 +212,7 @@ class ConductorManager(periodic_task.PeriodicTasks): """Ironic Conductor manager main class.""" # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's. - RPC_API_VERSION = '1.30' + RPC_API_VERSION = '1.31' target = messaging.Target(version=RPC_API_VERSION) @@ -2030,6 +2031,108 @@ class ConductorManager(periodic_task.PeriodicTasks): return driver.raid.get_logical_disk_properties() + def _object_dispatch(self, target, method, context, args, kwargs): + """Dispatch a call to an object method. + + This ensures that object methods get called and any exception + that is raised gets wrapped in an ExpectedException for forwarding + back to the caller (without spamming the conductor logs). + """ + try: + # NOTE(danms): Keep the getattr inside the try block since + # a missing method is really a client problem + return getattr(target, method)(context, *args, **kwargs) + except Exception: + # NOTE(danms): This is oslo.messaging fu. ExpectedException() + # grabs sys.exc_info here and forwards it along. This allows the + # caller to see the exception information, but causes us *not* to + # log it as such in this service. This is something that is quite + # critical so that things that conductor does on behalf of another + # node are not logged as exceptions in conductor logs. Otherwise, + # you'd have the same thing logged in both places, even though an + # exception here *always* means that the caller screwed up, so + # there's no reason to log it here. + raise messaging.ExpectedException() + + def object_class_action_versions(self, context, objname, objmethod, + object_versions, args, kwargs): + """Perform an action on a VersionedObject class. + + :param context: The context within which to perform the action + :param objname: The registry name of the object + :param objmethod: The name of the action method to call + :param object_versions: A dict of {objname: version} mappings + :param args: The positional arguments to the action method + :param kwargs: The keyword arguments to the action method + :returns: The result of the action method, which may (or may not) + be an instance of the implementing VersionedObject class. + """ + objclass = objects_base.IronicObject.obj_class_from_name( + objname, object_versions[objname]) + result = self._object_dispatch(objclass, objmethod, context, + args, kwargs) + # 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. + if isinstance(result, objects_base.IronicObject): + result = result.obj_to_primitive( + target_version=object_versions[objname], + version_manifest=object_versions) + return result + + def object_action(self, context, objinst, objmethod, args, kwargs): + """Perform an action on a VersionedObject instance. + + :param context: The context within which to perform the action + :param objinst: The object instance on which to perform the action + :param objmethod: The name of the action method to call + :param args: The positional arguments to the action method + :param kwargs: The keyword arguments to the action method + :returns: A tuple with the updates made to the object and + the result of the action method + """ + + oldobj = objinst.obj_clone() + result = self._object_dispatch(objinst, objmethod, context, + args, kwargs) + updates = dict() + # NOTE(danms): Diff the object with the one passed to us and + # generate a list of changes to forward back + for name, field in objinst.fields.items(): + if not objinst.obj_attr_is_set(name): + # Avoid demand-loading anything + continue + if (not oldobj.obj_attr_is_set(name) or + getattr(oldobj, name) != getattr(objinst, name)): + updates[name] = field.to_primitive(objinst, name, + getattr(objinst, name)) + # This is safe since a field named this would conflict with the + # method anyway + updates['obj_what_changed'] = objinst.obj_what_changed() + return updates, result + + def object_backport_versions(self, context, objinst, object_versions): + """Perform a backport of an object instance. + + The default behavior of the base VersionedObjectSerializer, upon + receiving an object with a version newer than what is in the local + registry, is to call this method to request a backport of the object. + + :param context: The context within which to perform the backport + :param objinst: An instance of a VersionedObject to be backported + :param object_versions: A dict of {objname: version} mappings + :returns: The downgraded instance of objinst + """ + target = object_versions[objinst.obj_name()] + LOG.debug('Backporting %(obj)s to %(ver)s with versions %(manifest)s', + {'obj': objinst.obj_name(), + 'ver': target, + 'manifest': ','.join( + ['%s=%s' % (name, ver) + for name, ver in object_versions.items()])}) + return objinst.obj_to_primitive(target_version=target, + version_manifest=object_versions) + def get_vendor_passthru_metadata(route_dict): d = {} diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index 8cb2ef205d..830743023e 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -75,11 +75,14 @@ class ConductorAPI(object): | driver_vendor_passthru to a dictionary | 1.30 - Added set_target_raid_config and | get_raid_logical_disk_properties + | 1.31 - Added Versioned Objects indirection API methods: + | object_class_action_versions, object_action and + | object_backport_versions """ # NOTE(rloo): This must be in sync with manager.ConductorManager's. - RPC_API_VERSION = '1.30' + RPC_API_VERSION = '1.31' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -589,3 +592,74 @@ class ConductorAPI(object): cctxt = self.client.prepare(topic=topic or self.topic, version='1.30') return cctxt.call(context, 'get_raid_logical_disk_properties', driver_name=driver_name) + + def object_class_action_versions(self, context, objname, objmethod, + object_versions, args, kwargs): + """Perform an action on a VersionedObject class. + + We want any conductor to handle this, so it is intentional that there + is no topic argument for this method. + + :param context: The context within which to perform the action + :param objname: The registry name of the object + :param objmethod: The name of the action method to call + :param object_versions: A dict of {objname: version} mappings + :param args: The positional arguments to the action method + :param kwargs: The keyword arguments to the action method + :raises: NotImplemented when an operator makes an error during upgrade + :returns: The result of the action method, which may (or may not) + be an instance of the implementing VersionedObject class. + """ + if not self.client.can_send_version('1.31'): + raise NotImplemented(_('Incompatible conductor version - ' + 'please upgrade ironic-conductor first')) + cctxt = self.client.prepare(topic=self.topic, version='1.31') + return cctxt.call(context, 'object_class_action_versions', + objname=objname, objmethod=objmethod, + object_versions=object_versions, + args=args, kwargs=kwargs) + + def object_action(self, context, objinst, objmethod, args, kwargs): + """Perform an action on a VersionedObject instance. + + We want any conductor to handle this, so it is intentional that there + is no topic argument for this method. + + :param context: The context within which to perform the action + :param objinst: The object instance on which to perform the action + :param objmethod: The name of the action method to call + :param args: The positional arguments to the action method + :param kwargs: The keyword arguments to the action method + :raises: NotImplemented when an operator makes an error during upgrade + :returns: A tuple with the updates made to the object and + the result of the action method + """ + if not self.client.can_send_version('1.31'): + raise NotImplemented(_('Incompatible conductor version - ' + 'please upgrade ironic-conductor first')) + cctxt = self.client.prepare(topic=self.topic, version='1.31') + return cctxt.call(context, 'object_action', objinst=objinst, + objmethod=objmethod, args=args, kwargs=kwargs) + + def object_backport_versions(self, context, objinst, object_versions): + """Perform a backport of an object instance. + + The default behavior of the base VersionedObjectSerializer, upon + receiving an object with a version newer than what is in the local + registry, is to call this method to request a backport of the object. + + We want any conductor to handle this, so it is intentional that there + is no topic argument for this method. + + :param context: The context within which to perform the backport + :param objinst: An instance of a VersionedObject to be backported + :param object_versions: A dict of {objname: version} mappings + :raises: NotImplemented when an operator makes an error during upgrade + :returns: The downgraded instance of objinst + """ + if not self.client.can_send_version('1.31'): + raise NotImplemented(_('Incompatible conductor version - ' + 'please upgrade ironic-conductor first')) + cctxt = self.client.prepare(topic=self.topic, version='1.31') + return cctxt.call(context, 'object_backport_versions', objinst=objinst, + object_versions=object_versions) diff --git a/ironic/objects/base.py b/ironic/objects/base.py index 14ef1aadfe..56bb71ed72 100644 --- a/ironic/objects/base.py +++ b/ironic/objects/base.py @@ -65,6 +65,36 @@ class IronicObject(object_base.VersionedObject): self[field] = loaded_object[field] +class IronicObjectIndirectionAPI(object_base.VersionedObjectIndirectionAPI): + def __init__(self): + super(IronicObjectIndirectionAPI, self).__init__() + # FIXME(xek): importing here due to a cyclical import error + from ironic.conductor import rpcapi as conductor_api + self._conductor = conductor_api.ConductorAPI() + + def object_action(self, context, objinst, objmethod, args, kwargs): + return self._conductor.object_action(context, objinst, objmethod, + args, kwargs) + + def object_class_action(self, context, objname, objmethod, objver, + args, kwargs): + # NOTE(xek): This method is implemented for compatibility with + # oslo.versionedobjects 0.10.0 and older. It will be replaced by + # object_class_action_versions. + versions = object_base.obj_tree_get_versions(objname) + return self.object_class_action_versions( + context, objname, objmethod, versions, args, kwargs) + + def object_class_action_versions(self, context, objname, objmethod, + object_versions, args, kwargs): + return self._conductor.object_class_action_versions( + context, objname, objmethod, object_versions, args, kwargs) + + def object_backport_versions(self, context, objinst, object_versions): + return self._conductor.object_backport_versions(context, objinst, + object_versions) + + class IronicObjectSerializer(object_base.VersionedObjectSerializer): # Base class to use for object hydration OBJ_BASE_CLASS = IronicObject diff --git a/ironic/objects/chassis.py b/ironic/objects/chassis.py index 226a5db4ae..8983ca0a7e 100644 --- a/ironic/objects/chassis.py +++ b/ironic/objects/chassis.py @@ -55,7 +55,11 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): chassis.obj_reset_changes() return chassis - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get(cls, context, chassis_id): """Find a chassis based on its id or uuid and return a Chassis object. @@ -69,7 +73,11 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): else: raise exception.InvalidIdentity(identity=chassis_id) - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_id(cls, context, chassis_id): """Find a chassis based on its integer id and return a Chassis object. @@ -80,7 +88,11 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): chassis = Chassis._from_db_object(cls(context), db_chassis) return chassis - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_uuid(cls, context, uuid): """Find a chassis based on uuid and return a :class:`Chassis` object. @@ -92,7 +104,11 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): chassis = Chassis._from_db_object(cls(context), db_chassis) return chassis - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def list(cls, context, limit=None, marker=None, sort_key=None, sort_dir=None): """Return a list of Chassis objects. @@ -112,7 +128,10 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): return [Chassis._from_db_object(cls(context), obj) for obj in db_chassis] - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def create(self, context=None): """Create a Chassis record in the DB. @@ -133,7 +152,10 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): db_chassis = self.dbapi.create_chassis(values) self._from_db_object(self, db_chassis) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def destroy(self, context=None): """Delete the Chassis from the DB. @@ -147,7 +169,10 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): self.dbapi.destroy_chassis(self.uuid) self.obj_reset_changes() - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def save(self, context=None): """Save updates to this Chassis. @@ -165,7 +190,10 @@ class Chassis(base.IronicObject, object_base.VersionedObjectDictCompat): updated_chassis = self.dbapi.update_chassis(self.uuid, updates) self._from_db_object(self, updated_chassis) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def refresh(self, context=None): """Loads and applies updates for this Chassis. diff --git a/ironic/objects/conductor.py b/ironic/objects/conductor.py index eaf98447c5..e0028e93a2 100644 --- a/ironic/objects/conductor.py +++ b/ironic/objects/conductor.py @@ -42,7 +42,11 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat): conductor.obj_reset_changes() return conductor - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_hostname(cls, context, hostname): """Get a Conductor record by its hostname. @@ -58,7 +62,10 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat): raise NotImplementedError( _('Cannot update a conductor record directly.')) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def refresh(self, context=None): """Loads and applies updates for this Conductor. @@ -77,7 +84,10 @@ class Conductor(base.IronicObject, object_base.VersionedObjectDictCompat): hostname=self.hostname) self.obj_refresh(current) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def touch(self, context): """Touch this conductor's DB record, marking it as up-to-date.""" self.dbapi.touch_conductor(self.hostname) diff --git a/ironic/objects/node.py b/ironic/objects/node.py index 483b654265..b358c3adf9 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -106,7 +106,11 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): node.obj_reset_changes() return node - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get(cls, context, node_id): """Find a node based on its id or uuid and return a Node object. @@ -120,7 +124,11 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): else: raise exception.InvalidIdentity(identity=node_id) - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_id(cls, context, node_id): """Find a node based on its integer id and return a Node object. @@ -131,7 +139,11 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): node = Node._from_db_object(cls(context), db_node) return node - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_uuid(cls, context, uuid): """Find a node based on uuid and return a Node object. @@ -142,7 +154,11 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): node = Node._from_db_object(cls(context), db_node) return node - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_name(cls, context, name): """Find a node based on name and return a Node object. @@ -153,7 +169,11 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): node = Node._from_db_object(cls(context), db_node) return node - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_instance_uuid(cls, context, instance_uuid): """Find a node based on the instance uuid and return a Node object. @@ -164,7 +184,11 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): node = Node._from_db_object(cls(context), db_node) return node - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def list(cls, context, limit=None, marker=None, sort_key=None, sort_dir=None, filters=None): """Return a list of Node objects. @@ -183,7 +207,11 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): sort_dir=sort_dir) return [Node._from_db_object(cls(context), obj) for obj in db_nodes] - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def reserve(cls, context, tag, node_id): """Get and reserve a node. @@ -201,7 +229,11 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): node = Node._from_db_object(cls(context), db_node) return node - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def release(cls, context, tag, node_id): """Release the reservation on a node. @@ -213,7 +245,10 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): """ cls.dbapi.release_node(tag, node_id) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def create(self, context=None): """Create a Node record in the DB. @@ -234,7 +269,10 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): db_node = self.dbapi.create_node(values) self._from_db_object(self, db_node) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def destroy(self, context=None): """Delete the Node from the DB. @@ -248,7 +286,10 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): self.dbapi.destroy_node(self.uuid) self.obj_reset_changes() - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def save(self, context=None): """Save updates to this Node. @@ -272,7 +313,10 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): self.dbapi.update_node(self.uuid, updates) self.obj_reset_changes() - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def refresh(self, context=None): """Refresh the object by re-fetching from the DB. @@ -286,7 +330,10 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): current = self.__class__.get_by_uuid(self._context, self.uuid) self.obj_refresh(current) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def touch_provisioning(self, context=None): """Touch the database record to mark the provisioning as alive.""" self.dbapi.touch_node_provisioning(self.id) diff --git a/ironic/objects/port.py b/ironic/objects/port.py index c145d66edc..5e9cd8cd53 100644 --- a/ironic/objects/port.py +++ b/ironic/objects/port.py @@ -58,7 +58,11 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): """Converts a list of database entities to a list of formal objects.""" return [Port._from_db_object(cls(context), obj) for obj in db_objects] - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get(cls, context, port_id): """Find a port based on its id or uuid and return a Port object. @@ -76,7 +80,11 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): else: raise exception.InvalidIdentity(identity=port_id) - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_id(cls, context, port_id): """Find a port based on its integer id and return a Port object. @@ -89,7 +97,11 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): port = Port._from_db_object(cls(context), db_port) return port - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_uuid(cls, context, uuid): """Find a port based on uuid and return a :class:`Port` object. @@ -103,7 +115,11 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): port = Port._from_db_object(cls(context), db_port) return port - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def get_by_address(cls, context, address): """Find a port based on address and return a :class:`Port` object. @@ -117,7 +133,11 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): port = Port._from_db_object(cls(context), db_port) return port - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def list(cls, context, limit=None, marker=None, sort_key=None, sort_dir=None): """Return a list of Port objects. @@ -137,7 +157,11 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): sort_dir=sort_dir) return Port._from_db_object_list(db_ports, cls, context) - @object_base.remotable_classmethod + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable_classmethod + @classmethod def list_by_node_id(cls, context, node_id, limit=None, marker=None, sort_key=None, sort_dir=None): """Return a list of Port objects associated with a given node ID. @@ -157,7 +181,10 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): sort_dir=sort_dir) return Port._from_db_object_list(db_ports, cls, context) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def create(self, context=None): """Create a Port record in the DB. @@ -175,7 +202,10 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): db_port = self.dbapi.create_port(values) self._from_db_object(self, db_port) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def destroy(self, context=None): """Delete the Port from the DB. @@ -191,7 +221,10 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): self.dbapi.destroy_port(self.uuid) self.obj_reset_changes() - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def save(self, context=None): """Save updates to this Port. @@ -212,7 +245,10 @@ class Port(base.IronicObject, object_base.VersionedObjectDictCompat): updated_port = self.dbapi.update_port(self.uuid, updates) self._from_db_object(self, updated_port) - @object_base.remotable + # NOTE(xek): We don't want to enable RPC on this call just yet. Remotable + # methods can be used in the future to replace current explicit RPC calls. + # Implications of calling new remote procedures should be thought through. + # @object_base.remotable def refresh(self, context=None): """Loads updates for this Port. diff --git a/ironic/tests/conductor/test_manager.py b/ironic/tests/conductor/test_manager.py index d49faaea30..39fb5b6890 100644 --- a/ironic/tests/conductor/test_manager.py +++ b/ironic/tests/conductor/test_manager.py @@ -27,6 +27,8 @@ from oslo_db import exception as db_exception import oslo_messaging as messaging from oslo_utils import strutils from oslo_utils import uuidutils +from oslo_versionedobjects import base as ovo_base +from oslo_versionedobjects import fields from ironic.common import boot_devices from ironic.common import driver_factory @@ -41,6 +43,7 @@ from ironic.db import api as dbapi from ironic.drivers import base as drivers_base from ironic.drivers.modules import fake from ironic import objects +from ironic.objects import base as obj_base from ironic.tests import base as tests_base from ironic.tests.conductor import utils as mgr_utils from ironic.tests.db import base as tests_db_base @@ -4354,3 +4357,105 @@ class ManagerCheckDeployingStatusTestCase(_ServiceSetUpMixin, 'provision_updated_at', callback_method=conductor_utils.cleanup_after_timeout, err_handler=manager.provisioning_error_handler) + + +class TestIndirectionApiConductor(tests_db_base.DbTestCase): + + def setUp(self): + super(TestIndirectionApiConductor, self).setUp() + self.conductor = manager.ConductorManager('test-host', 'test-topic') + + def _test_object_action(self, is_classmethod, raise_exception, + return_object=False): + @obj_base.IronicObjectRegistry.register + class TestObject(obj_base.IronicObject): + context = self.context + + def foo(self, context, raise_exception=False, return_object=False): + if raise_exception: + raise Exception('test') + elif return_object: + return obj + else: + return 'test' + + @classmethod + def bar(cls, context, raise_exception=False, return_object=False): + if raise_exception: + raise Exception('test') + elif return_object: + return obj + else: + return 'test' + + obj = TestObject(self.context) + if is_classmethod: + versions = ovo_base.obj_tree_get_versions(TestObject.obj_name()) + result = self.conductor.object_class_action_versions( + self.context, TestObject.obj_name(), 'bar', versions, + tuple(), {'raise_exception': raise_exception, + 'return_object': return_object}) + else: + updates, result = self.conductor.object_action( + self.context, obj, 'foo', tuple(), + {'raise_exception': raise_exception, + 'return_object': return_object}) + if return_object: + self.assertEqual(obj, result) + else: + self.assertEqual('test', result) + + def test_object_action(self): + self._test_object_action(False, False) + + def test_object_action_on_raise(self): + self.assertRaises(messaging.ExpectedException, + self._test_object_action, False, True) + + def test_object_action_on_object(self): + self._test_object_action(False, False, True) + + def test_object_class_action(self): + self._test_object_action(True, False) + + def test_object_class_action_on_raise(self): + self.assertRaises(messaging.ExpectedException, + self._test_object_action, True, True) + + def test_object_class_action_on_object(self): + self._test_object_action(True, False, False) + + def test_object_action_copies_object(self): + @obj_base.IronicObjectRegistry.register + class TestObject(obj_base.IronicObject): + fields = {'dict': fields.DictOfStringsField()} + + def touch_dict(self, context): + self.dict['foo'] = 'bar' + self.obj_reset_changes() + + obj = TestObject(self.context) + obj.dict = {} + obj.obj_reset_changes() + updates, result = self.conductor.object_action( + self.context, obj, 'touch_dict', tuple(), {}) + # NOTE(danms): If conductor did not properly copy the object, then + # the new and reference copies of the nested dict object will be + # the same, and thus 'dict' will not be reported as changed + self.assertIn('dict', updates) + self.assertEqual({'foo': 'bar'}, updates['dict']) + + def test_object_backport_versions(self): + fake_backported_obj = 'fake-backported-obj' + obj_name = 'fake-obj' + test_obj = mock.Mock() + test_obj.obj_name.return_value = obj_name + test_obj.obj_to_primitive.return_value = fake_backported_obj + fake_version_manifest = {obj_name: '1.0'} + + result = self.conductor.object_backport_versions( + self.context, test_obj, fake_version_manifest) + + self.assertEqual(result, fake_backported_obj) + test_obj.obj_to_primitive.assert_called_once_with( + target_version='1.0', version_manifest=fake_version_manifest) diff --git a/ironic/tests/conductor/test_rpcapi.py b/ironic/tests/conductor/test_rpcapi.py index dda8728a56..30380b8b31 100644 --- a/ironic/tests/conductor/test_rpcapi.py +++ b/ironic/tests/conductor/test_rpcapi.py @@ -22,6 +22,7 @@ import copy import mock from oslo_config import cfg +from oslo_messaging import _utils as messaging_utils from ironic.common import boot_devices from ironic.common import exception @@ -145,6 +146,10 @@ class RPCAPITestCase(base.DbTestCase): self.fake_args = None self.fake_kwargs = None + def _fake_can_send_version_method(version): + return messaging_utils.version_is_compatible( + rpcapi.RPC_API_VERSION, version) + def _fake_prepare_method(*args, **kwargs): for kwd in kwargs: self.assertEqual(kwargs[kwd], target[kwd]) @@ -156,16 +161,21 @@ class RPCAPITestCase(base.DbTestCase): if expected_retval: return expected_retval - with mock.patch.object(rpcapi.client, "prepare") as mock_prepared: - mock_prepared.side_effect = _fake_prepare_method + with mock.patch.object(rpcapi.client, + "can_send_version") as mock_can_send_version: + mock_can_send_version.side_effect = _fake_can_send_version_method + with mock.patch.object(rpcapi.client, "prepare") as mock_prepared: + mock_prepared.side_effect = _fake_prepare_method - with mock.patch.object(rpcapi.client, rpc_method) as mock_method: - mock_method.side_effect = _fake_rpc_method - retval = getattr(rpcapi, method)(self.context, **kwargs) - self.assertEqual(retval, expected_retval) - expected_args = [self.context, method, expected_msg] - for arg, expected_arg in zip(self.fake_args, expected_args): - self.assertEqual(arg, expected_arg) + with mock.patch.object(rpcapi.client, + rpc_method) as mock_method: + mock_method.side_effect = _fake_rpc_method + retval = getattr(rpcapi, method)(self.context, **kwargs) + self.assertEqual(retval, expected_retval) + expected_args = [self.context, method, expected_msg] + for arg, expected_arg in zip(self.fake_args, + expected_args): + self.assertEqual(arg, expected_arg) def test_update_node(self): self._test_rpcapi('update_node', @@ -306,3 +316,29 @@ class RPCAPITestCase(base.DbTestCase): version='1.30', node_id=self.fake_node['uuid'], target_raid_config='config') + + def test_object_action(self): + self._test_rpcapi('object_action', + 'call', + version='1.31', + objinst='fake-object', + objmethod='foo', + args=tuple(), + kwargs=dict()) + + def test_object_class_action_versions(self): + self._test_rpcapi('object_class_action_versions', + 'call', + version='1.31', + objname='fake-object', + objmethod='foo', + object_versions={'fake-object': '1.0'}, + args=tuple(), + kwargs=dict()) + + def test_object_backport_versions(self): + self._test_rpcapi('object_backport_versions', + 'call', + version='1.31', + objinst='fake-object', + object_versions={'fake-object': '1.0'}) diff --git a/ironic/tests/objects/test_objects.py b/ironic/tests/objects/test_objects.py index c001b906d0..1cb67fe262 100644 --- a/ironic/tests/objects/test_objects.py +++ b/ironic/tests/objects/test_objects.py @@ -17,6 +17,7 @@ import datetime import gettext import iso8601 +import mock from oslo_context import context from oslo_versionedobjects import base as object_base from oslo_versionedobjects import exception as object_exception @@ -439,3 +440,48 @@ class TestObjectSerializer(test_base.TestCase): self.assertEqual(1, len(thing2)) for item in thing2: self.assertIsInstance(item, MyObj) + + @mock.patch('ironic.objects.base.IronicObject.indirection_api') + def _test_deserialize_entity_newer(self, obj_version, backported_to, + mock_indirection_api, + my_version='1.6'): + ser = base.IronicObjectSerializer() + mock_indirection_api.object_backport_versions.return_value \ + = 'backported' + + @base.IronicObjectRegistry.register + class MyTestObj(MyObj): + VERSION = my_version + + obj = MyTestObj(self.context) + obj.VERSION = obj_version + primitive = obj.obj_to_primitive() + result = ser.deserialize_entity(self.context, primitive) + if backported_to is None: + self.assertFalse( + mock_indirection_api.object_backport_versions.called) + else: + self.assertEqual('backported', result) + versions = object_base.obj_tree_get_versions('MyTestObj') + mock_indirection_api.object_backport_versions.assert_called_with( + self.context, primitive, versions) + + def test_deserialize_entity_newer_version_backports(self): + "Test object with unsupported (newer) version" + self._test_deserialize_entity_newer('1.25', '1.6') + + def test_deserialize_entity_same_revision_does_not_backport(self): + "Test object with supported revision" + self._test_deserialize_entity_newer('1.6', None) + + def test_deserialize_entity_newer_revision_does_not_backport_zero(self): + "Test object with supported revision" + self._test_deserialize_entity_newer('1.6.0', None) + + def test_deserialize_entity_newer_revision_does_not_backport(self): + "Test object with supported (newer) revision" + self._test_deserialize_entity_newer('1.6.1', None) + + def test_deserialize_entity_newer_version_passes_revision(self): + "Test object with unsupported (newer) version and revision" + self._test_deserialize_entity_newer('1.7', '1.6.1', my_version='1.6.1')