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 <stephane@alum.mit.edu>
Co-Authored-By: Ruby Loo <ruby.loo@intel.com>
Change-Id: I2925c081da4bb0a77edd5c579f92aaa0f5999b78
Partial-Bug: 1526231

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):

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)

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)

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)

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