Merge "Update detach to use V3 Cinder API"

nova/exception.py

@@ -571,6 +571,10 @@ class AgentBuildExists(NovaException):
                 "architecture %(architecture)s exists.")
 
 
+class VolumeAttachmentNotFound(NotFound):
+    msg_fmt = _("Volume attachment %(attachment_id)s could not be found.")
+
+
 class VolumeNotFound(NotFound):
     msg_fmt = _("Volume %(volume_id)s could not be found.")
 

nova/tests/unit/virt/test_block_device.py

@@ -101,6 +101,7 @@ class TestDriverBlockDevice(test.NoDBTestCase):
          'boot_index': 0})
 
     volume_driver_bdm = {
+        'attachment_id': None,
         'mount_device': '/dev/sda1',
         'connection_info': {"fake": "connection_info"},
         'delete_on_termination': False,
@@ -129,6 +130,7 @@ class TestDriverBlockDevice(test.NoDBTestCase):
          'boot_index': -1})
 
     snapshot_driver_bdm = {
+        'attachment_id': None,
         'mount_device': '/dev/sda2',
         'connection_info': {"fake": "connection_info"},
         'delete_on_termination': True,
@@ -157,6 +159,7 @@ class TestDriverBlockDevice(test.NoDBTestCase):
          'boot_index': -1})
 
     image_driver_bdm = {
+        'attachment_id': None,
         'mount_device': '/dev/sda2',
         'connection_info': {"fake": "connection_info"},
         'delete_on_termination': True,
@@ -185,6 +188,7 @@ class TestDriverBlockDevice(test.NoDBTestCase):
          'boot_index': -1})
 
     blank_driver_bdm = {
+        'attachment_id': None,
         'mount_device': '/dev/sda2',
         'connection_info': {"fake": "connection_info"},
         'delete_on_termination': True,
@@ -383,6 +387,31 @@ class TestDriverBlockDevice(test.NoDBTestCase):
     def test_call_wait_no_delete_volume(self):
         self._test_call_wait_func(False)
 
+    def test_volume_delete_attachment(self):
+        attachment_id = uuids.attachment
+        driver_bdm = self.driver_classes['volume'](self.volume_bdm)
+        driver_bdm['attachment_id'] = attachment_id
+
+        elevated_context = self.context.elevated()
+        instance_detail = {'id': '123', 'uuid': uuids.uuid,
+                           'availability_zone': None}
+        instance = fake_instance.fake_instance_obj(self.context,
+                                                   **instance_detail)
+        connector = {'ip': 'fake_ip', 'host': 'fake_host'}
+
+        with test.nested(
+            mock.patch.object(self.virt_driver, 'get_volume_connector',
+                              return_value=connector),
+            mock.patch.object(self.volume_api, 'attachment_delete'),
+        ) as (_, vapi_attach_del):
+
+            driver_bdm.detach(elevated_context, instance,
+                              self.volume_api, self.virt_driver,
+                              attachment_id=attachment_id)
+
+            vapi_attach_del.assert_called_once_with(elevated_context,
+                                                    attachment_id)
+
     def _test_volume_attach(self, driver_bdm, bdm_dict,
                             fake_volume, fail_check_av_zone=False,
                             driver_attach=False, fail_driver_attach=False,

nova/tests/unit/volume/test_cinder.py

@@ -280,6 +280,33 @@ class CinderApiTestCase(test.NoDBTestCase):
         mock_volumes.attach.assert_called_once_with('id1', 'uuid', 'point',
                                                     mode='ro')
 
+    @mock.patch('nova.volume.cinder.cinderclient')
+    def test_attachment_delete(self, mock_cinderclient):
+        mock_attachments = mock.MagicMock()
+        mock_cinderclient.return_value = \
+            mock.MagicMock(attachments=mock_attachments)
+
+        attachment_id = uuids.attachment
+        self.api.attachment_delete(self.ctx, attachment_id)
+
+        mock_cinderclient.assert_called_once_with(self.ctx)
+        mock_attachments.delete.assert_called_once_with(attachment_id)
+
+    @mock.patch('nova.volume.cinder.LOG')
+    @mock.patch('nova.volume.cinder.cinderclient')
+    def test_attachment_delete_failed(self, mock_cinderclient, mock_log):
+        mock_cinderclient.return_value.attachments.delete.side_effect = (
+                cinder_exception.NotFound(404, '404'))
+
+        attachment_id = uuids.attachment
+        ex = self.assertRaises(exception.VolumeAttachmentNotFound,
+                               self.api.attachment_delete,
+                               self.ctx,
+                               attachment_id)
+
+        self.assertEqual(404, ex.code)
+        self.assertIn(attachment_id, ex.message)
+
     @mock.patch('nova.volume.cinder.cinderclient')
     def test_detach(self, mock_cinderclient):
         mock_volumes = mock.MagicMock()

nova/virt/block_device.py

@@ -224,7 +224,8 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
     _legacy_fields = set(['connection_info', 'mount_device',
                           'delete_on_termination'])
     _new_fields = set(['guest_format', 'device_type',
-                       'disk_bus', 'boot_index'])
+                       'disk_bus', 'boot_index',
+                       'attachment_id'])
     _fields = _legacy_fields | _new_fields
 
     _valid_source = 'volume'
@@ -349,9 +350,16 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
                            'schost': stashed_connector.get('host')})
                 connector = stashed_connector
 
-        volume_api.terminate_connection(context, volume_id, connector)
-        volume_api.detach(context.elevated(), volume_id, instance.uuid,
-                          attachment_id)
+        # NOTE(jdg): For now we need to actually inspect the bdm for an
+        # attachment_id as opposed to relying on what may have been passed
+        # in, we want to force usage of the old detach flow for now and only
+        # use the new flow when we explicitly used it for the attach.
+        if not self['attachment_id']:
+            volume_api.terminate_connection(context, volume_id, connector)
+            volume_api.detach(context.elevated(), volume_id, instance.uuid,
+                              attachment_id)
+        else:
+            volume_api.attachment_delete(context, self['attachment_id'])
 
     @update_db
     def attach(self, context, instance, volume_api, virt_driver,

nova/volume/cinder.py

@@ -205,6 +205,19 @@ def translate_volume_exception(method):
     return translate_cinder_exception(wrapper)
 
 
+def translate_attachment_exception(method):
+    """Transforms the exception for the attachment but keeps its traceback intact.
+    """
+    def wrapper(self, ctx, attachment_id, *args, **kwargs):
+        try:
+            res = method(self, ctx, attachment_id, *args, **kwargs)
+        except (keystone_exception.NotFound, cinder_exception.NotFound):
+            _reraise(exception.VolumeAttachmentNotFound(
+                attachment_id=attachment_id))
+        return res
+    return translate_cinder_exception(wrapper)
+
+
 def translate_snapshot_exception(method):
     """Transforms the exception for the snapshot but keeps its traceback
        intact.
@@ -480,3 +493,16 @@ class API(object):
             {'status': status,
              'progress': '90%'}
         )
+
+    @translate_attachment_exception
+    def attachment_delete(self, context, attachment_id):
+        try:
+            cinderclient(
+                context).attachments.delete(attachment_id)
+        except cinder_exception.ClientException as ex:
+            with excutils.save_and_reraise_exception():
+                LOG.error(('Delete attachment failed for attachment '
+                           '%(id)s. Error: %(msg)s Code: %(code)s'),
+                          {'id': attachment_id,
+                           'msg': six.text_type(ex),
+                           'code': getattr(ex, 'code', None)})