From ddcd97714cd5b27ac43ac0ef1138c13392223b90 Mon Sep 17 00:00:00 2001 From: Hironori Shiina Date: Fri, 30 Jun 2017 13:46:23 +0900 Subject: [PATCH] Add node power state validation to volume resource update/deletion This patch validate the power state of a node when the following actions regarding volume resources associated with the node are requested. - update a volume connector - delete a volume connector - update a volume target - delete a volume target These actions should allowed only when the node is tuned off as designed in the SPEC. Change-Id: I5d0465c6ac2d2c6ddac03385e6ed0ccb37556306 Partial-Bug: 1526231 --- ironic/conductor/manager.py | 48 ++++++++++-- ironic/tests/unit/conductor/test_manager.py | 85 ++++++++++++++++++--- 2 files changed, 115 insertions(+), 18 deletions(-) diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 70256f88d3..c2cea5fa74 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1682,7 +1682,8 @@ class ConductorManager(base_manager.BaseConductorManager): @METRICS.timer('ConductorManager.destroy_volume_connector') @messaging.expected_exceptions(exception.NodeLocked, exception.NodeNotFound, - exception.VolumeConnectorNotFound) + exception.VolumeConnectorNotFound, + exception.InvalidStateRequested) def destroy_volume_connector(self, context, connector): """Delete a volume connector. @@ -1693,12 +1694,20 @@ class ConductorManager(base_manager.BaseConductorManager): not exist :raises: VolumeConnectorNotFound if the volume connector cannot be found + :raises: InvalidStateRequested if the node associated with the + connector is not powered off. """ LOG.debug('RPC destroy_volume_connector called for volume connector ' '%(connector)s', {'connector': connector.uuid}) with task_manager.acquire(context, connector.node_id, purpose='volume connector deletion') as task: + node = task.node + if node.power_state != states.POWER_OFF: + raise exception.InvalidStateRequested( + action='volume connector deletion', + node=node.uuid, + state=node.power_state) connector.destroy() LOG.info('Successfully deleted volume connector %(connector)s. ' 'The node associated with the connector was %(node)s', @@ -1707,7 +1716,8 @@ class ConductorManager(base_manager.BaseConductorManager): @METRICS.timer('ConductorManager.destroy_volume_target') @messaging.expected_exceptions(exception.NodeLocked, exception.NodeNotFound, - exception.VolumeTargetNotFound) + exception.VolumeTargetNotFound, + exception.InvalidStateRequested) def destroy_volume_target(self, context, target): """Delete a volume target. @@ -1717,12 +1727,20 @@ class ConductorManager(base_manager.BaseConductorManager): :raises: NodeNotFound if the node associated with the target does not exist :raises: VolumeTargetNotFound if the volume target cannot be found + :raises: InvalidStateRequested if the node associated with the target + is not powered off. """ 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: + node = task.node + if node.power_state != states.POWER_OFF: + raise exception.InvalidStateRequested( + action='volume target deletion', + node=node.uuid, + state=node.power_state) target.destroy() LOG.info('Successfully deleted volume target %(target)s. ' 'The node associated with the target was %(node)s', @@ -2030,7 +2048,8 @@ class ConductorManager(base_manager.BaseConductorManager): exception.NodeLocked, exception.NodeNotFound, exception.VolumeConnectorNotFound, - exception.VolumeConnectorTypeAndIdAlreadyExists) + exception.VolumeConnectorTypeAndIdAlreadyExists, + exception.InvalidStateRequested) def update_volume_connector(self, context, connector): """Update a volume connector. @@ -2047,13 +2066,21 @@ class ConductorManager(base_manager.BaseConductorManager): :raises: VolumeConnectorTypeAndIdAlreadyExists if another connector already exists with the same values for type and connector_id fields + :raises: InvalidStateRequested if the node associated with the + connector is not powered off. """ LOG.debug("RPC update_volume_connector called for connector " "%(connector)s.", {'connector': connector.uuid}) with task_manager.acquire(context, connector.node_id, - purpose='volume connector update'): + purpose='volume connector update') as task: + node = task.node + if node.power_state != states.POWER_OFF: + raise exception.InvalidStateRequested( + action='volume connector update', + node=node.uuid, + state=node.power_state) connector.save() LOG.info("Successfully updated volume connector %(connector)s.", {'connector': connector.uuid}) @@ -2065,7 +2092,8 @@ class ConductorManager(base_manager.BaseConductorManager): exception.NodeLocked, exception.NodeNotFound, exception.VolumeTargetNotFound, - exception.VolumeTargetBootIndexAlreadyExists) + exception.VolumeTargetBootIndexAlreadyExists, + exception.InvalidStateRequested) def update_volume_target(self, context, target): """Update a volume target. @@ -2080,12 +2108,20 @@ class ConductorManager(base_manager.BaseConductorManager): :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 + :raises: InvalidStateRequested if the node associated with the target + is not powered off. """ 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'): + purpose='volume target update') as task: + node = task.node + if node.power_state != states.POWER_OFF: + raise exception.InvalidStateRequested( + action='volume target update', + node=node.uuid, + state=node.power_state) target.save() LOG.info("Successfully updated volume target %(target)s.", {'target': target.uuid}) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 50ad3ab494..47224a3f25 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -6041,7 +6041,8 @@ class DoNodeAdoptionTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): class DestroyVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): def test_destroy_volume_connector(self): - node = obj_utils.create_test_node(self.context, driver='fake') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_connector = obj_utils.create_test_volume_connector( self.context, node_id=node.id) @@ -6064,12 +6065,25 @@ class DestroyVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin, # Compare true exception hidden by @messaging.expected_exceptions self.assertEqual(exception.NodeLocked, exc.exc_info[0]) + def test_destroy_volume_connector_node_power_on(self): + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_ON) + + volume_connector = obj_utils.create_test_volume_connector( + self.context, node_id=node.id) + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.destroy_volume_connector, + self.context, volume_connector) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.InvalidStateRequested, exc.exc_info[0]) + @mgr_utils.mock_record_keepalive class UpdateVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): def test_update_volume_connector(self): - node = obj_utils.create_test_node(self.context, driver='fake') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_connector = obj_utils.create_test_volume_connector( self.context, node_id=node.id, extra={'foo': 'bar'}) @@ -6093,7 +6107,8 @@ class UpdateVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(exception.NodeLocked, exc.exc_info[0]) def test_update_volume_connector_type(self): - node = obj_utils.create_test_node(self.context, driver='fake') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_connector = obj_utils.create_test_volume_connector( self.context, node_id=node.id, extra={'vol_id': 'fake-id'}) new_type = 'wwnn' @@ -6103,7 +6118,8 @@ class UpdateVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(new_type, res.type) def test_update_volume_connector_uuid(self): - node = obj_utils.create_test_node(self.context, driver='fake') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_connector = obj_utils.create_test_volume_connector( self.context, node_id=node.id) volume_connector.uuid = uuidutils.generate_uuid() @@ -6114,7 +6130,8 @@ class UpdateVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) def test_update_volume_connector_duplicate(self): - node = obj_utils.create_test_node(self.context, driver='fake') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_connector1 = obj_utils.create_test_volume_connector( self.context, node_id=node.id) volume_connector2 = obj_utils.create_test_volume_connector( @@ -6128,12 +6145,26 @@ class UpdateVolumeConnectorTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(exception.VolumeConnectorTypeAndIdAlreadyExists, exc.exc_info[0]) + def test_update_volume_connector_node_power_on(self): + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_ON) + + volume_connector = obj_utils.create_test_volume_connector( + self.context, node_id=node.id) + volume_connector.extra = {'foo': 'baz'} + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_volume_connector, + self.context, volume_connector) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.InvalidStateRequested, exc.exc_info[0]) + @mgr_utils.mock_record_keepalive class DestroyVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): def test_destroy_volume_target(self): - node = obj_utils.create_test_node(self.context, driver='fake') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_target = obj_utils.create_test_volume_target(self.context, node_id=node.id) @@ -6169,7 +6200,8 @@ class DestroyVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, 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') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_target = obj_utils.create_test_volume_target(self.context, node_id=node.id) self.service.destroy_volume_target(self.context, volume_target) @@ -6179,12 +6211,25 @@ class DestroyVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, # Compare true exception hidden by @messaging.expected_exceptions self.assertEqual(exception.VolumeTargetNotFound, exc.exc_info[0]) + def test_destroy_volume_target_node_power_on(self): + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_ON) + + 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.InvalidStateRequested, exc.exc_info[0]) + @mgr_utils.mock_record_keepalive class UpdateVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): def test_update_volume_target(self): - node = obj_utils.create_test_node(self.context, driver='fake') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_target = obj_utils.create_test_volume_target( self.context, node_id=node.id, extra={'foo': 'bar'}) @@ -6206,7 +6251,8 @@ class UpdateVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, 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') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_target = obj_utils.create_test_volume_target( self.context, node_id=node.id, extra={'vol_id': 'fake-id'}) new_volume_type = 'fibre_channel' @@ -6216,7 +6262,8 @@ class UpdateVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, 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') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_target = obj_utils.create_test_volume_target( self.context, node_id=node.id) volume_target.uuid = uuidutils.generate_uuid() @@ -6227,7 +6274,8 @@ class UpdateVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, 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') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_target1 = obj_utils.create_test_volume_target( self.context, node_id=node.id) volume_target2 = obj_utils.create_test_volume_target( @@ -6242,7 +6290,8 @@ class UpdateVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, exc.exc_info[0]) def _test_update_volume_target_exception(self, expected_exc): - node = obj_utils.create_test_node(self.context, driver='fake') + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_OFF) volume_target = obj_utils.create_test_volume_target( self.context, node_id=node.id, extra={'vol_id': 'fake-id'}) new_volume_type = 'fibre_channel' @@ -6261,3 +6310,15 @@ class UpdateVolumeTargetTestCase(mgr_utils.ServiceSetUpMixin, def test_update_volume_target_not_found(self): self._test_update_volume_target_exception( exception.VolumeTargetNotFound) + + def test_update_volume_target_node_power_on(self): + node = obj_utils.create_test_node(self.context, driver='fake', + power_state=states.POWER_ON) + 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.InvalidStateRequested, exc.exc_info[0])