From c1cce7eb452c228dc2633e80f2c98fd142574fa9 Mon Sep 17 00:00:00 2001 From: Mark Goddard Date: Thu, 18 Jan 2018 11:11:16 +0000 Subject: [PATCH] Add RPC API and conductor manager for traits Adds two new methods to the conductor RPC API, add_node_traits and remove_node_traits, and provides implementations for them in the conductor manager. add_node_traits can be used to add one or more traits to a node, or to replace all the traits for a node. remove_node_traits can be used to remove one or more traits from a node, or to remove all traits from a node. The conductor RPC API version is bumped to 1.44. Change-Id: I0181df6a41e603874677246066d84bf4ac4f433c Partial-Bug: #1722194 --- ironic/common/release_mappings.py | 2 +- ironic/conductor/manager.py | 60 ++++++++++++- ironic/conductor/rpcapi.py | 37 +++++++- ironic/tests/unit/conductor/test_manager.py | 97 +++++++++++++++++++++ ironic/tests/unit/conductor/test_rpcapi.py | 29 ++++++ 5 files changed, 222 insertions(+), 3 deletions(-) diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index e6ecfefb09..276a04c81d 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -122,7 +122,7 @@ RELEASE_MAPPING = { }, 'master': { 'api': '1.36', - 'rpc': '1.43', + 'rpc': '1.44', 'objects': { 'Node': ['1.23'], 'Conductor': ['1.2'], diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index ffba6b3dad..e178179dd0 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -94,7 +94,7 @@ class ConductorManager(base_manager.BaseConductorManager): # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's. # NOTE(pas-ha): This also must be in sync with # ironic.common.release_mappings.RELEASE_MAPPING['master'] - RPC_API_VERSION = '1.43' + RPC_API_VERSION = '1.44' target = messaging.Target(version=RPC_API_VERSION) @@ -3060,6 +3060,64 @@ class ConductorManager(base_manager.BaseConductorManager): return objinst.obj_to_primitive(target_version=target, version_manifest=object_versions) + @METRICS.timer('ConductorManager.add_node_traits') + @messaging.expected_exceptions(exception.InvalidParameterValue, + exception.NodeLocked, + exception.NodeNotFound) + def add_node_traits(self, context, node_id, traits, replace=False): + """Add or replace traits for a node. + + :param context: request context. + :param node_id: node ID or UUID. + :param traits: a list of traits to add to the node. + :param replace: True to replace all of the node's traits. + :raises: InvalidParameterValue if adding the traits would exceed the + per-node traits limit. Traits added prior to reaching the limit + will not be removed. + :raises: NodeLocked if node is locked by another conductor. + :raises: NodeNotFound if the node does not exist. + """ + LOG.debug("RPC add_node_traits called for the node %(node_id)s with " + "traits %(traits)s", {'node_id': node_id, 'traits': traits}) + with task_manager.acquire(context, node_id, + purpose='add node traits'): + if replace: + objects.TraitList.create(context, node_id=node_id, + traits=traits) + else: + for trait in traits: + trait = objects.Trait(context, node_id=node_id, + trait=trait) + trait.create() + + @METRICS.timer('ConductorManager.remove_node_traits') + @messaging.expected_exceptions(exception.NodeLocked, + exception.NodeNotFound, + exception.NodeTraitNotFound) + def remove_node_traits(self, context, node_id, traits): + """Remove some or all traits from a node. + + :param context: request context. + :param node_id: node ID or UUID. + :param traits: a list of traits to remove from the node, or None. If + None, all traits will be removed from the node. + :raises: NodeLocked if node is locked by another conductor. + :raises: NodeNotFound if the node does not exist. + :raises: NodeTraitNotFound if one of the traits is not found. Traits + removed prior to the non-existent trait will not be replaced. + """ + LOG.debug("RPC remove_node_traits called for the node %(node_id)s " + "with traits %(traits)s", + {'node_id': node_id, 'traits': traits}) + with task_manager.acquire(context, node_id, + purpose='remove node traits'): + if traits is None: + objects.TraitList.destroy(context, node_id=node_id) + else: + for trait in traits: + objects.Trait.destroy(context, node_id=node_id, + trait=trait) + @METRICS.timer('get_vendor_passthru_metadata') def get_vendor_passthru_metadata(route_dict): diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index e3ebcce809..8b1262dc7b 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -92,13 +92,14 @@ class ConductorAPI(object): | 1.41 - Added create_port | 1.42 - Added optional agent_version to heartbeat | 1.43 - Added do_node_rescue, do_node_unrescue and can_send_rescue + | 1.44 - Added add_node_traits and remove_node_traits. """ # NOTE(rloo): This must be in sync with manager.ConductorManager's. # NOTE(pas-ha): This also must be in sync with # ironic.common.release_mappings.RELEASE_MAPPING['master'] - RPC_API_VERSION = '1.43' + RPC_API_VERSION = '1.44' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -1017,3 +1018,37 @@ class ConductorAPI(object): """ cctxt = self.client.prepare(topic=topic or self.topic, version='1.43') return cctxt.call(context, 'do_node_unrescue', node_id=node_id) + + def add_node_traits(self, context, node_id, traits, replace=False, + topic=None): + """Add or replace traits for a node. + + :param context: request context. + :param node_id: node ID or UUID. + :param traits: a list of traits to add to the node. + :param replace: True to replace all of the node's traits. + :param topic: RPC topic. Defaults to self.topic. + :raises: InvalidParameterValue if adding the traits would exceed the + per-node traits limit. + :raises: NodeLocked if node is locked by another conductor. + :raises: NodeNotFound if the node does not exist. + """ + cctxt = self.client.prepare(topic=topic or self.topic, version='1.44') + return cctxt.call(context, 'add_node_traits', node_id=node_id, + traits=traits, replace=replace) + + def remove_node_traits(self, context, node_id, traits, topic=None): + """Remove some or all traits from a node. + + :param context: request context. + :param node_id: node ID or UUID. + :param traits: a list of traits to remove from the node, or None. If + None, all traits will be removed from the node. + :param topic: RPC topic. Defaults to self.topic. + :raises: NodeLocked if node is locked by another conductor. + :raises: NodeNotFound if the node does not exist. + :raises: NodeTraitNotFound if one of the traits is not found. + """ + cctxt = self.client.prepare(topic=topic or self.topic, version='1.44') + return cctxt.call(context, 'remove_node_traits', node_id=node_id, + traits=traits) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 431282cbcd..d513cead21 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -6903,3 +6903,100 @@ class UpdateVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, self.context, volume_target) # Compare true exception hidden by @messaging.expected_exceptions self.assertEqual(exception.InvalidStateRequested, exc.exc_info[0]) + + +@mgr_utils.mock_record_keepalive +class NodeTraitsTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): + + def setUp(self): + super(NodeTraitsTestCase, self).setUp() + self.traits = ['trait1', 'trait2'] + self.node = obj_utils.create_test_node(self.context, + driver='fake-hardware') + + def test_add_node_traits(self): + self.service.add_node_traits(self.context, self.node.id, + self.traits[:1]) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual([trait.trait for trait in traits], self.traits[:1]) + + self.service.add_node_traits(self.context, self.node.id, + self.traits[1:]) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual([trait.trait for trait in traits], self.traits) + + def test_add_node_traits_replace(self): + self.service.add_node_traits(self.context, self.node.id, + self.traits[:1], replace=True) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual([trait.trait for trait in traits], self.traits[:1]) + + self.service.add_node_traits(self.context, self.node.id, + self.traits[1:], replace=True) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual([trait.trait for trait in traits], self.traits[1:]) + + def _test_add_node_traits_exception(self, expected_exc): + with mock.patch.object(objects.Trait, 'create') as mock_create: + mock_create.side_effect = expected_exc('Boo') + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.add_node_traits, self.context, + self.node.id, self.traits) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(expected_exc, exc.exc_info[0]) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual(traits.objects, []) + + def test_add_node_traits_invalid_parameter_value(self): + self._test_add_node_traits_exception(exception.InvalidParameterValue) + + def test_add_node_traits_node_locked(self): + self._test_add_node_traits_exception(exception.NodeLocked) + + def test_add_node_traits_node_not_found(self): + self._test_add_node_traits_exception(exception.NodeNotFound) + + def test_remove_node_traits(self): + objects.TraitList.create(self.context, self.node.id, self.traits) + self.service.remove_node_traits(self.context, self.node.id, + self.traits[:1]) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual([trait.trait for trait in traits], self.traits[1:]) + + self.service.remove_node_traits(self.context, self.node.id, + self.traits[1:]) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual(traits.objects, []) + + def test_remove_node_traits_all(self): + objects.TraitList.create(self.context, self.node.id, self.traits) + self.service.remove_node_traits(self.context, self.node.id, None) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual(traits.objects, []) + + def test_remove_node_traits_empty(self): + objects.TraitList.create(self.context, self.node.id, self.traits) + self.service.remove_node_traits(self.context, self.node.id, []) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual([trait.trait for trait in traits], self.traits) + + def _test_remove_node_traits_exception(self, expected_exc): + objects.TraitList.create(self.context, self.node.id, self.traits) + with mock.patch.object(objects.Trait, 'destroy') as mock_destroy: + mock_destroy.side_effect = expected_exc('Boo') + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.remove_node_traits, + self.context, self.node.id, self.traits) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(expected_exc, exc.exc_info[0]) + traits = objects.TraitList.get_by_node_id(self.context, self.node.id) + self.assertEqual([trait.trait for trait in traits], self.traits) + + def test_remove_node_traits_node_locked(self): + self._test_remove_node_traits_exception(exception.NodeLocked) + + def test_remove_node_traits_node_not_found(self): + self._test_remove_node_traits_exception(exception.NodeNotFound) + + def test_remove_node_traits_node_trait_not_found(self): + self._test_remove_node_traits_exception(exception.NodeTraitNotFound) diff --git a/ironic/tests/unit/conductor/test_rpcapi.py b/ironic/tests/unit/conductor/test_rpcapi.py index f384ae52ce..92e74313d4 100644 --- a/ironic/tests/unit/conductor/test_rpcapi.py +++ b/ironic/tests/unit/conductor/test_rpcapi.py @@ -527,3 +527,32 @@ class RPCAPITestCase(db_base.DbTestCase): def test_can_send_rescue_false(self): self._test_can_send_rescue(False) + + def test_add_node_traits(self): + self._test_rpcapi('add_node_traits', + 'call', + node_id='fake-node', + traits=['trait1'], + version='1.44') + + def test_add_node_traits_replace(self): + self._test_rpcapi('add_node_traits', + 'call', + node_id='fake-node', + traits=['trait1'], + replace=True, + version='1.44') + + def test_remove_node_traits(self): + self._test_rpcapi('remove_node_traits', + 'call', + node_id='fake-node', + traits=['trait1'], + version='1.44') + + def test_remove_node_traits_all(self): + self._test_rpcapi('remove_node_traits', + 'call', + node_id='fake-node', + traits=None, + version='1.44')