Merge "Add node power state validation to volume resource update/deletion"

2017-07-04 02:44:24 +00:00
parent dc14f65696 ddcd97714c
commit 99ba58172c
2 changed files with 115 additions and 18 deletions
--- 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})
--- 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])