From aab505a69842beb776410761d451d13a132bd3d2 Mon Sep 17 00:00:00 2001 From: Satoru Moriya Date: Fri, 26 Feb 2016 19:52:54 +0900 Subject: [PATCH] Add RPCs to support volume target operations This patch adds the following two RPCs in order to support volume target operations in Ironic. - update_volume_target() This function is called to update the information about a volume target that is stored in the database. - destroy_volume_target() This function is called to remove a volume target from the database. Co-Authored-By: Stephane Miller Co-Authored-By: Ruby Loo Change-Id: I2925c081da4bb0a77edd5c579f92aaa0f5999b78 Partial-Bug: 1526231 --- ironic/conductor/manager.py | 58 ++++++++- ironic/conductor/rpcapi.py | 43 ++++++- ironic/tests/unit/conductor/test_manager.py | 134 ++++++++++++++++++++ ironic/tests/unit/conductor/test_rpcapi.py | 14 ++ ironic/tests/unit/objects/utils.py | 27 ++++ 5 files changed, 274 insertions(+), 2 deletions(-) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 31c2d42b50..81931727ef 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -82,7 +82,7 @@ class ConductorManager(base_manager.BaseConductorManager): """Ironic Conductor manager main class.""" # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's. - RPC_API_VERSION = '1.36' + RPC_API_VERSION = '1.37' target = messaging.Target(version=RPC_API_VERSION) @@ -1576,6 +1576,30 @@ class ConductorManager(base_manager.BaseConductorManager): '%(node)s'), {'connector': connector.uuid, 'node': task.node.uuid}) + @METRICS.timer('ConductorManager.destroy_volume_target') + @messaging.expected_exceptions(exception.NodeLocked, + exception.NodeNotFound, + exception.VolumeTargetNotFound) + def destroy_volume_target(self, context, target): + """Delete a volume target. + + :param context: request context + :param target: volume target object + :raises: NodeLocked if node is locked by another conductor + :raises: NodeNotFound if the node associated with the target does + not exist + :raises: VolumeTargetNotFound if the volume target cannot be found + """ + LOG.debug('RPC destroy_volume_target called for volume target ' + '%(target)s', + {'target': target.uuid}) + with task_manager.acquire(context, target.node_id, + purpose='volume target deletion') as task: + target.destroy() + LOG.info(_LI('Successfully deleted volume target %(target)s. ' + 'The node associated with the target was %(node)s'), + {'target': target.uuid, 'node': task.node.uuid}) + @METRICS.timer('ConductorManager.get_console_information') @messaging.expected_exceptions(exception.NodeLocked, exception.UnsupportedDriverExtension, @@ -1948,6 +1972,38 @@ class ConductorManager(base_manager.BaseConductorManager): {'connector': connector.uuid}) return connector + @METRICS.timer('ConductorManager.update_volume_target') + @messaging.expected_exceptions( + exception.InvalidParameterValue, + exception.NodeLocked, + exception.NodeNotFound, + exception.VolumeTargetNotFound, + exception.VolumeTargetBootIndexAlreadyExists) + def update_volume_target(self, context, target): + """Update a volume target. + + :param context: request context + :param target: a changed (but not saved) volume target object + :returns: an updated volume target object + :raises: InvalidParameterValue if the volume target's UUID is being + changed + :raises: NodeLocked if the node is already locked + :raises: NodeNotFound if the node associated with the volume target + does not exist + :raises: VolumeTargetNotFound if the volume target cannot be found + :raises: VolumeTargetBootIndexAlreadyExists if a volume target already + exists with the same node ID and boot index values + """ + LOG.debug("RPC update_volume_target called for target %(target)s.", + {'target': target.uuid}) + + with task_manager.acquire(context, target.node_id, + purpose='volume target update'): + target.save() + LOG.info(_LI("Successfully updated volume target %(target)s."), + {'target': target.uuid}) + return target + @METRICS.timer('ConductorManager.get_driver_properties') @messaging.expected_exceptions(exception.DriverNotFound) def get_driver_properties(self, context, driver_name): diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index 2c411e934f..b4267271f6 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -83,11 +83,12 @@ class ConductorAPI(object): | 1.34 - Added heartbeat | 1.35 - Added destroy_volume_connector and update_volume_connector | 1.36 - Added create_node + | 1.37 - Added destroy_volume_target and update_volume_target """ # NOTE(rloo): This must be in sync with manager.ConductorManager's. - RPC_API_VERSION = '1.36' + RPC_API_VERSION = '1.37' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -798,3 +799,43 @@ class ConductorAPI(object): cctxt = self.client.prepare(topic=topic or self.topic, version='1.35') return cctxt.call(context, 'update_volume_connector', connector=connector) + + def destroy_volume_target(self, context, target, topic=None): + """Delete a volume target. + + :param context: request context + :param target: volume target object + :param topic: RPC topic. Defaults to self.topic. + :raises: NodeLocked if node is locked by another conductor + :raises: NodeNotFound if the node associated with the target does + not exist + :raises: VolumeTargetNotFound if the volume target cannot be found + """ + cctxt = self.client.prepare(topic=topic or self.topic, version='1.37') + return cctxt.call(context, 'destroy_volume_target', + target=target) + + def update_volume_target(self, context, target, topic=None): + """Update the volume target's information. + + Update the volume target's information in the database and return a + volume target object. The conductor will lock the related node during + this operation. + + :param context: request context + :param target: a changed (but not saved) volume target object + :param topic: RPC topic. Defaults to self.topic. + :raises: InvalidParameterValue if the volume target's UUID is being + changed + :raises: NodeLocked if the node is already locked + :raises: NodeNotFound if the node associated with the volume target + does not exist + :raises: VolumeTargetNotFound if the volume target cannot be found + :raises: VolumeTargetBootIndexAlreadyExists if a volume target already + exists with the same node ID and boot index values + :returns: updated volume target object, including all fields + + """ + cctxt = self.client.prepare(topic=topic or self.topic, version='1.37') + return cctxt.call(context, 'update_volume_target', + target=target) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 8efd9d521f..7bf875cb56 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -5811,3 +5811,137 @@ class UpdateVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin, # Compare true exception hidden by @messaging.expected_exceptions self.assertEqual(exception.VolumeConnectorTypeAndIdAlreadyExists, exc.exc_info[0]) + + +@mgr_utils.mock_record_keepalive +class DestroyVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, + tests_db_base.DbTestCase): + def test_destroy_volume_target(self): + node = obj_utils.create_test_node(self.context, driver='fake') + + volume_target = obj_utils.create_test_volume_target(self.context, + node_id=node.id) + self.service.destroy_volume_target(self.context, volume_target) + self.assertRaises(exception.VolumeTargetNotFound, + volume_target.refresh) + self.assertRaises(exception.VolumeTargetNotFound, + self.dbapi.get_volume_target_by_uuid, + volume_target.uuid) + + def test_destroy_volume_target_node_locked(self): + node = obj_utils.create_test_node(self.context, driver='fake', + reservation='fake-reserv') + + volume_target = obj_utils.create_test_volume_target(self.context, + node_id=node.id) + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.destroy_volume_target, + self.context, volume_target) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeLocked, exc.exc_info[0]) + + def test_destroy_volume_target_node_gone(self): + node = obj_utils.create_test_node(self.context, driver='fake') + volume_target = obj_utils.create_test_volume_target(self.context, + node_id=node.id) + self.service.destroy_node(self.context, node.id) + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.destroy_volume_target, + self.context, volume_target) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeNotFound, exc.exc_info[0]) + + def test_destroy_volume_target_already_destroyed(self): + node = obj_utils.create_test_node(self.context, driver='fake') + volume_target = obj_utils.create_test_volume_target(self.context, + node_id=node.id) + self.service.destroy_volume_target(self.context, volume_target) + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.destroy_volume_target, + self.context, volume_target) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.VolumeTargetNotFound, exc.exc_info[0]) + + +@mgr_utils.mock_record_keepalive +class UpdateVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, + tests_db_base.DbTestCase): + def test_update_volume_target(self): + node = obj_utils.create_test_node(self.context, driver='fake') + + volume_target = obj_utils.create_test_volume_target( + self.context, node_id=node.id, extra={'foo': 'bar'}) + new_extra = {'foo': 'baz'} + volume_target.extra = new_extra + res = self.service.update_volume_target(self.context, volume_target) + self.assertEqual(new_extra, res.extra) + + def test_update_volume_target_node_locked(self): + node = obj_utils.create_test_node(self.context, driver='fake', + reservation='fake-reserv') + volume_target = obj_utils.create_test_volume_target(self.context, + node_id=node.id) + volume_target.extra = {'foo': 'baz'} + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_volume_target, + self.context, volume_target) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeLocked, exc.exc_info[0]) + + def test_update_volume_target_volume_type(self): + node = obj_utils.create_test_node(self.context, driver='fake') + volume_target = obj_utils.create_test_volume_target( + self.context, node_id=node.id, extra={'vol_id': 'fake-id'}) + new_volume_type = 'fibre_channel' + volume_target.volume_type = new_volume_type + res = self.service.update_volume_target(self.context, + volume_target) + self.assertEqual(new_volume_type, res.volume_type) + + def test_update_volume_target_uuid(self): + node = obj_utils.create_test_node(self.context, driver='fake') + volume_target = obj_utils.create_test_volume_target( + self.context, node_id=node.id) + volume_target.uuid = uuidutils.generate_uuid() + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_volume_target, + self.context, volume_target) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) + + def test_update_volume_target_duplicate(self): + node = obj_utils.create_test_node(self.context, driver='fake') + volume_target1 = obj_utils.create_test_volume_target( + self.context, node_id=node.id) + volume_target2 = obj_utils.create_test_volume_target( + self.context, node_id=node.id, uuid=uuidutils.generate_uuid(), + boot_index=volume_target1.boot_index + 1) + volume_target2.boot_index = volume_target1.boot_index + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_volume_target, + self.context, volume_target2) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.VolumeTargetBootIndexAlreadyExists, + exc.exc_info[0]) + + def _test_update_volume_target_exception(self, expected_exc): + node = obj_utils.create_test_node(self.context, driver='fake') + volume_target = obj_utils.create_test_volume_target( + self.context, node_id=node.id, extra={'vol_id': 'fake-id'}) + new_volume_type = 'fibre_channel' + volume_target.volume_type = new_volume_type + with mock.patch.object(objects.VolumeTarget, 'save') as mock_save: + mock_save.side_effect = expected_exc('Boo') + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_volume_target, + self.context, volume_target) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(expected_exc, exc.exc_info[0]) + + def test_update_volume_target_node_not_found(self): + self._test_update_volume_target_exception(exception.NodeNotFound) + + def test_update_volume_target_not_found(self): + self._test_update_volume_target_exception( + exception.VolumeTargetNotFound) diff --git a/ironic/tests/unit/conductor/test_rpcapi.py b/ironic/tests/unit/conductor/test_rpcapi.py index 8ba915d373..ef5cdc5d9f 100644 --- a/ironic/tests/unit/conductor/test_rpcapi.py +++ b/ironic/tests/unit/conductor/test_rpcapi.py @@ -414,3 +414,17 @@ class RPCAPITestCase(base.DbTestCase): 'call', version='1.35', connector=fake_volume_connector) + + def test_destroy_volume_target(self): + fake_volume_target = dbutils.get_test_volume_target() + self._test_rpcapi('destroy_volume_target', + 'call', + version='1.37', + target=fake_volume_target) + + def test_update_volume_target(self): + fake_volume_target = dbutils.get_test_volume_target() + self._test_rpcapi('update_volume_target', + 'call', + version='1.37', + target=fake_volume_target) diff --git a/ironic/tests/unit/objects/utils.py b/ironic/tests/unit/objects/utils.py index 50911ca16b..1f5a718c19 100644 --- a/ironic/tests/unit/objects/utils.py +++ b/ironic/tests/unit/objects/utils.py @@ -188,3 +188,30 @@ def create_test_volume_connector(ctxt, **kw): volume_connector = get_test_volume_connector(ctxt, **kw) volume_connector.create() return volume_connector + + +def get_test_volume_target(ctxt, **kw): + """Return a VolumeTarget object with appropriate attributes. + + NOTE: The object leaves the attributes marked as changed, such + that a create() could be used to commit it to the DB. + """ + db_volume_target = db_utils.get_test_volume_target(**kw) + # Let DB generate ID if it isn't specified explicitly + if 'id' not in kw: + del db_volume_target['id'] + volume_target = objects.VolumeTarget(ctxt) + for key in db_volume_target: + setattr(volume_target, key, db_volume_target[key]) + return volume_target + + +def create_test_volume_target(ctxt, **kw): + """Create and return a test volume target object. + + Create a volume target in the DB and return a VolumeTarget object with + appropriate attributes. + """ + volume_target = get_test_volume_target(ctxt, **kw) + volume_target.create() + return volume_target