From 34fff5656330c856d06b1658e513324c12de5cd7 Mon Sep 17 00:00:00 2001 From: Sun Jun Date: Thu, 25 Dec 2014 12:56:44 +0800 Subject: [PATCH] Allow deleting share with invalid share server in generic driver In generic driver, if we do not specify the "--share-network" while creating a new share(not from snapshot), the share will get error state and it cannot be deleted(will be in "error_deleting" status). Remove decorator "ensure_server" for method "delete_share" that is dedicated mostly for share creation and allow delete share with invalid share-server in Generic driver. Change-Id: Id4f2138507a1b6c6c303837b6bc09c39c86559df Closes-Bug: #1405362 --- manila/share/drivers/generic.py | 27 ++++++++--- manila/tests/share/drivers/test_generic.py | 54 ++++++++++++++++++++++ 2 files changed, 75 insertions(+), 6 deletions(-) diff --git a/manila/share/drivers/generic.py b/manila/share/drivers/generic.py index 1bd2704a49..78859e1597 100644 --- a/manila/share/drivers/generic.py +++ b/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 < @@ -490,14 +494,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 diff --git a/manila/tests/share/drivers/test_generic.py b/manila/tests/share/drivers/test_generic.py index 572682c6c1..d02d4e68c5 100644 --- a/manila/tests/share/drivers/test_generic.py +++ b/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'])