Merge "Do proper cleanup if connect volume fails"

cinder/tests/unit/volume/test_connection.py

@@ -322,6 +322,15 @@ class VolumeAttachDetachTestCase(base.BaseVolumeTestCase):
                           self.context,
                           volume_id)
 
+    @mock.patch('cinder.volume.manager.LOG', mock.Mock())
+    def test_initialize_connection(self):
+        volume = mock.Mock(save=mock.Mock(side_effect=Exception))
+        with mock.patch.object(self.volume, 'driver') as driver_mock:
+            self.assertRaises(exception.ExportFailure,
+                              self.volume.initialize_connection, self.context,
+                              volume, mock.Mock())
+        driver_mock.remove_export.assert_called_once_with(mock.ANY, volume)
+
     def test_run_attach_detach_2volumes_for_instance(self):
         """Make sure volume can be attached and detached from instance."""
         # attach first volume to the instance

cinder/tests/unit/volume/test_image.py

@@ -563,6 +563,33 @@ class ImageVolumeTestCases(base.BaseVolumeTestCase):
         # We must have called detach method.
         self.assertEqual(1, mock_detach.call_count)
 
+    @mock.patch('cinder.utils.brick_get_connector_properties')
+    @mock.patch('cinder.utils.brick_get_connector')
+    @mock.patch('cinder.volume.driver.BaseVD._connect_device')
+    @mock.patch('cinder.volume.driver.BaseVD._detach_volume')
+    @mock.patch('cinder.image.image_utils.qemu_img_info')
+    def test_create_volume_from_image_unavailable_no_attach_info(
+            self, mock_qemu_info, mock_detach, mock_connect, *args):
+        """Test create volume with ImageCopyFailure
+
+        We'll raise an exception on _connect_device call to confirm that it
+        detaches the volume even if the exception doesn't have attach_info.
+        """
+        mock_connect.side_effect = NameError
+        image_info = imageutils.QemuImgInfo()
+        image_info.virtual_size = '1073741824'
+        mock_qemu_info.return_value = image_info
+
+        unbound_copy_method = cinder.volume.driver.BaseVD.copy_image_to_volume
+        bound_copy_method = unbound_copy_method.__get__(self.volume.driver)
+        with mock.patch.object(self.volume.driver, 'copy_image_to_volume',
+                               side_effect=bound_copy_method):
+            self.assertRaises(exception.ImageCopyFailure,
+                              self._create_volume_from_image,
+                              fakeout_copy_image_to_volume=False)
+        # We must have called detach method.
+        self.assertEqual(1, mock_detach.call_count)
+
     @mock.patch('cinder.image.image_utils.qemu_img_info')
     def test_create_volume_from_image_clone_image_volume(self, mock_qemu_info):
         """Test create volume from image via image volume.

cinder/volume/driver.py

@@ -434,10 +434,10 @@ class BaseVD(object):
                        force=False, remote=False):
         """Disconnect the volume from the host."""
         # Use Brick's code to do attach/detach
-        connector = attach_info['connector']
-        connector.disconnect_volume(attach_info['conn']['data'],
-                                    attach_info['device'])
-
+        if attach_info:
+            connector = attach_info['connector']
+            connector.disconnect_volume(attach_info['conn']['data'],
+                                        attach_info['device'])
         if remote:
             # Call remote manager's terminate_connection which includes
             # driver's terminate_connection and remove export
@@ -976,20 +976,20 @@ class BaseVD(object):
 
         try:
             attach_info = self._connect_device(conn)
-        except exception.DeviceUnavailable as exc:
+        except Exception as exc:
             # We may have reached a point where we have attached the volume,
             # so we have to detach it (do the cleanup).
-            attach_info = exc.kwargs.get('attach_info', None)
-            if attach_info:
-                try:
-                    LOG.debug('Device for volume %s is unavailable but did '
-                              'attach, detaching it.', volume['id'])
-                    self._detach_volume(context, attach_info, volume,
-                                        properties, force=True,
-                                        remote=remote)
-                except Exception:
-                    LOG.exception('Error detaching volume %s',
-                                  volume['id'])
+            attach_info = getattr(exc, 'kwargs', {}).get('attach_info', None)
+
+            try:
+                LOG.debug('Device for volume %s is unavailable but did '
+                          'attach, detaching it.', volume['id'])
+                self._detach_volume(context, attach_info, volume,
+                                    properties, force=True,
+                                    remote=remote)
+            except Exception:
+                LOG.exception('Error detaching volume %s',
+                              volume['id'])
             raise
 
         return (attach_info, volume)

cinder/volume/manager.py

@@ -1523,8 +1523,12 @@ class VolumeManager(manager.CleanableManager,
             if model_update:
                 volume.update(model_update)
                 volume.save()
-        except exception.CinderException as ex:
+        except Exception as ex:
             LOG.exception("Model update failed.", resource=volume)
+            try:
+                self.driver.remove_export(context.elevated(), volume)
+            except Exception:
+                LOG.exception('Could not remove export after DB model failed.')
             raise exception.ExportFailure(reason=six.text_type(ex))
 
         try: