Detach and terminate conn if Cinder attach fails

Driver's detach volume is not called while handling
failure during Cinder attach. This can result in data
loss in the case of VMware driver since the instance
VM is not re-configured to remove volume's vmdk. Also,
the connection won't be terminated if attach fails.
This patch fixes the above issues with cleanup during
Cinder attach failure.

Closes-bug: #1460044
Change-Id: I62c0800f68d8fd89fed98eea9e3f95d1adbe9d7f

nova/tests/unit/virt/test_block_device.py

@@ -431,6 +431,17 @@ class TestDriverBlockDevice(test.NoDBTestCase):
                                        'fake_uuid', bdm_dict['device_name'],
                                         mode=access_mode).AndRaise(
                                             test.TestingException)
+                if driver_attach:
+                    self.virt_driver.detach_volume(
+                            expected_conn_info, instance,
+                            bdm_dict['device_name'],
+                            encryption=enc_data).AndReturn(None)
+                self.volume_api.terminate_connection(
+                        elevated_context, fake_volume['id'],
+                        connector).AndReturn(None)
+                self.volume_api.detach(elevated_context,
+                                       fake_volume['id']).AndReturn(None)
+
         driver_bdm._bdm_obj.save().AndReturn(None)
         return instance, expected_conn_info
 
@@ -559,6 +570,20 @@ class TestDriverBlockDevice(test.NoDBTestCase):
                          instance, self.volume_api, self.virt_driver,
                          do_driver_attach=True)
 
+    def test_volume_attach_no_driver_attach_volume_attach_fails(self):
+        test_bdm = self.driver_classes['volume'](
+            self.volume_bdm)
+        volume = {'id': 'fake-volume-id-1',
+                  'attach_status': 'detached'}
+
+        instance, _ = self._test_volume_attach(
+                test_bdm, self.volume_bdm, volume, fail_volume_attach=True)
+        self.mox.ReplayAll()
+
+        self.assertRaises(test.TestingException, test_bdm.attach, self.context,
+                         instance, self.volume_api, self.virt_driver,
+                         do_driver_attach=False)
+
     def test_refresh_connection(self):
         test_bdm = self.driver_classes['snapshot'](
             self.snapshot_bdm)

nova/virt/block_device.py

@@ -276,8 +276,31 @@ class DriverVolumeBlockDevice(DriverBlockDevice):
             # after that we can detach and connection_info is required for
             # detach.
             self.save()
-            volume_api.attach(context, volume_id, instance.uuid,
-                              self['mount_device'], mode=mode)
+            try:
+                volume_api.attach(context, volume_id, instance.uuid,
+                                  self['mount_device'], mode=mode)
+            except Exception:
+                with excutils.save_and_reraise_exception():
+                    if do_driver_attach:
+                        try:
+                            virt_driver.detach_volume(connection_info,
+                                                      instance,
+                                                      self['mount_device'],
+                                                      encryption=encryption)
+                        except Exception:
+                            LOG.warn(_LW("Driver failed to detach volume "
+                                         "%(volume_id)s at %(mount_point)s."),
+                                     {'volume_id': volume_id,
+                                      'mount_point': self['mount_device']},
+                                     exc_info=True, context=context,
+                                     instance=instance)
+                    volume_api.terminate_connection(context, volume_id,
+                                                    connector)
+
+                    # Cinder-volume might have completed volume attach. So
+                    # we should detach the volume. If the attach did not
+                    # happen, the detach request will be ignored.
+                    volume_api.detach(context, volume_id)
 
     @update_db
     def refresh_connection_info(self, context, instance,