Merge "Allow deleting share with invalid share server in generic driver"

manila/share/drivers/generic.py

@@ -428,6 +428,10 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
         """Deletes cinder volume."""
         volume = self._get_volume(context, share['id'])
         if volume:
+            if volume['status'] == 'in-use':
+                raise exception.ManilaException(
+                    _('Volume is still in use and '
+                      'cannot be deleted now.'))
             self.volume_api.delete(context, volume['id'])
             t = time.time()
             while (time.time() - t <
@@ -477,14 +481,25 @@ class GenericShareDriver(driver.ExecuteMixin, driver.ShareDriver):
             share['name'])
         return location
 
-    @ensure_server
+    def _is_share_server_active(self, context, share_server):
+        """Check if the share server is active."""
+        has_active_share_server = (
+            share_server and share_server.get('backend_details') and
+            self.service_instance_manager.ensure_service_instance(
+                context, share_server['backend_details']))
+        return has_active_share_server
+
     def delete_share(self, context, share, share_server=None):
         """Deletes share."""
-        self._get_helper(share).remove_export(share_server['backend_details'],
-                                              share['name'])
-        self._unmount_device(share, share_server['backend_details'])
-        self._detach_volume(self.admin_context, share,
-                            share_server['backend_details'])
+        if self._is_share_server_active(context, share_server):
+            self._get_helper(share).remove_export(
+                share_server['backend_details'], share['name'])
+            self._unmount_device(share, share_server['backend_details'])
+            self._detach_volume(self.admin_context, share,
+                                share_server['backend_details'])
+
+        # Note(jun): It is an intended breakage to deal with the cases
+        # with any reason that caused absence of Nova instances.
         self._deallocate_container(self.admin_context, share)
 
     @ensure_server

manila/tests/share/drivers/test_generic.py

@@ -678,6 +678,60 @@ class GenericShareDriverTestCase(test.TestCase):
         self._driver._deallocate_container.assert_called_once_with(
             self._driver.admin_context, self.share)
 
+    def test_delete_share_without_share_server(self):
+        self.stubs.Set(self._driver, '_unmount_device', mock.Mock())
+        self.stubs.Set(self._driver, '_detach_volume', mock.Mock())
+        self.stubs.Set(self._driver, '_deallocate_container', mock.Mock())
+
+        self._driver.delete_share(
+            self._context, self.share, share_server=None)
+
+        self.assertFalse(self._helper_nfs.remove_export.called)
+        self.assertFalse(self._driver._unmount_device.called)
+        self.assertFalse(self._driver._detach_volume.called)
+        self._driver._deallocate_container.assert_called_once_with(
+            self._driver.admin_context, self.share)
+
+    def test_delete_share_without_server_backend_details(self):
+        self.stubs.Set(self._driver, '_unmount_device', mock.Mock())
+        self.stubs.Set(self._driver, '_detach_volume', mock.Mock())
+        self.stubs.Set(self._driver, '_deallocate_container', mock.Mock())
+
+        fake_share_server = {
+            'instance_id': 'fake_instance_id',
+            'ip': 'fake_ip',
+            'username': 'fake_username',
+            'password': 'fake_password',
+            'pk_path': 'fake_pk_path',
+            'backend_details': {}
+        }
+
+        self._driver.delete_share(
+            self._context, self.share, share_server=fake_share_server)
+
+        self.assertFalse(self._helper_nfs.remove_export.called)
+        self.assertFalse(self._driver._unmount_device.called)
+        self.assertFalse(self._driver._detach_volume.called)
+        self._driver._deallocate_container.assert_called_once_with(
+            self._driver.admin_context, self.share)
+
+    def test_delete_share_without_server_availability(self):
+        self.stubs.Set(self._driver, '_unmount_device', mock.Mock())
+        self.stubs.Set(self._driver, '_detach_volume', mock.Mock())
+        self.stubs.Set(self._driver, '_deallocate_container', mock.Mock())
+
+        with mock.patch.object(self._driver.service_instance_manager,
+                               'ensure_service_instance',
+                               mock.Mock(return_value=False)):
+            self._driver.delete_share(
+                self._context, self.share, share_server=self.server)
+
+            self.assertFalse(self._helper_nfs.remove_export.called)
+            self.assertFalse(self._driver._unmount_device.called)
+            self.assertFalse(self._driver._detach_volume.called)
+            self._driver._deallocate_container.assert_called_once_with(
+                self._driver.admin_context, self.share)
+
     def test_create_snapshot(self):
         fake_vol = fake_volume.FakeVolume()
         fake_vol_snap = fake_volume.FakeVolumeSnapshot(share_id=fake_vol['id'])