openstack
/
manila
Code Issues Proposed changes

Delete Share Instance of unmanaged share 

Currently, for an unmanaged share, the associated share instance
stays with a status as 'unmanaged', which is not correct.
Associated share instance should be deleted.
Hence made the correction

Change-Id: Ib88205db91d69bd8fc4d98e82b479eadac85b604
Closes-Bug: #1510208
This commit is contained in:
nidhimittalhada 2015-12-23 14:35:25 +05:30
parent f178426266
commit b457fdecee
3 changed files with 19 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
manila/share/manager.py
View File

@ -939,9 +939,8 @@ class ShareManager(manager.SchedulerDependentManager):
_LE("Can not remove access rules of share: %s."), e) _LE("Can not remove access rules of share: %s."), e)
return return
self.db.share_update(context, share_id, self.db.share_instance_delete(context, share_instance['id'])
{'status': constants.STATUS_UNMANAGED, LOG.info(_LI("Share %s: unmanaged successfully."), share_id)
'deleted': True})
@add_hooks @add_hooks
@utils.require_driver_initialized @utils.require_driver_initialized

19
manila/tests/share/test_manager.py
View File

@ -1116,6 +1116,7 @@ class ShareManagerTestCase(test.TestCase):
mock_unmanage) mock_unmanage)
self.mock_object(self.share_manager.db, 'share_update') self.mock_object(self.share_manager.db, 'share_update')
self.mock_object(self.share_manager.db, 'share_instance_delete')
@ddt.data(True, False) @ddt.data(True, False)
def test_unmanage_share_invalid_driver(self, driver_handles_share_servers): def test_unmanage_share_invalid_driver(self, driver_handles_share_servers):
@ -1150,14 +1151,14 @@ class ShareManagerTestCase(test.TestCase):
mock_unmanage=mock.Mock()) mock_unmanage=mock.Mock())
share = db_utils.create_share() share = db_utils.create_share()
share_id = share['id'] share_id = share['id']
share_instance_id = share.instance['id']
self.share_manager.unmanage_share(self.context, share_id) self.share_manager.unmanage_share(self.context, share_id)
self.share_manager.driver.unmanage.\ self.share_manager.driver.unmanage.\
assert_called_once_with(mock.ANY) assert_called_once_with(mock.ANY)
self.share_manager.db.share_update.assert_called_once_with( self.share_manager.db.share_instance_delete.assert_called_once_with(
mock.ANY, share_id, mock.ANY, share_instance_id)
{'status': constants.STATUS_UNMANAGED, 'deleted': True})
def test_unmanage_share_valid_share_with_quota_error(self): def test_unmanage_share_valid_share_with_quota_error(self):
manager.CONF.set_default('driver_handles_share_servers', False) manager.CONF.set_default('driver_handles_share_servers', False)
@ -1166,14 +1167,14 @@ class ShareManagerTestCase(test.TestCase):
self.mock_object(quota.QUOTAS, 'reserve', self.mock_object(quota.QUOTAS, 'reserve',
mock.Mock(side_effect=Exception())) mock.Mock(side_effect=Exception()))
share = db_utils.create_share() share = db_utils.create_share()
share_instance_id = share.instance['id']
self.share_manager.unmanage_share(self.context, share['id']) self.share_manager.unmanage_share(self.context, share['id'])
self.share_manager.driver.unmanage.\ self.share_manager.driver.unmanage.\
assert_called_once_with(mock.ANY) assert_called_once_with(mock.ANY)
self.share_manager.db.share_update.assert_called_once_with( self.share_manager.db.share_instance_delete.assert_called_once_with(
mock.ANY, share['id'], mock.ANY, share_instance_id)
{'status': constants.STATUS_UNMANAGED, 'deleted': True})
def test_unmanage_share_remove_access_rules_error(self): def test_unmanage_share_remove_access_rules_error(self):
manager.CONF.set_default('driver_handles_share_servers', False) manager.CONF.set_default('driver_handles_share_servers', False)
@ -1199,6 +1200,7 @@ class ShareManagerTestCase(test.TestCase):
self.mock_object(quota.QUOTAS, 'reserve', mock.Mock(return_value=[])) self.mock_object(quota.QUOTAS, 'reserve', mock.Mock(return_value=[]))
share = db_utils.create_share() share = db_utils.create_share()
share_id = share['id'] share_id = share['id']
share_instance_id = share.instance['id']
self.share_manager.unmanage_share(self.context, share_id) self.share_manager.unmanage_share(self.context, share_id)
@ -1207,9 +1209,8 @@ class ShareManagerTestCase(test.TestCase):
self.share_manager._remove_share_access_rules.assert_called_once_with( self.share_manager._remove_share_access_rules.assert_called_once_with(
mock.ANY, mock.ANY, mock.ANY, mock.ANY mock.ANY, mock.ANY, mock.ANY, mock.ANY
) )
self.share_manager.db.share_update.assert_called_once_with( self.share_manager.db.share_instance_delete.assert_called_once_with(
mock.ANY, share_id, mock.ANY, share_instance_id)
{'status': constants.STATUS_UNMANAGED, 'deleted': True})
def test_remove_share_access_rules(self): def test_remove_share_access_rules(self):
self.mock_object(self.share_manager.db, self.mock_object(self.share_manager.db,

7
manila_tempest_tests/tests/api/admin/test_share_manage.py
View File

@ -147,6 +147,13 @@ class ManageNFSShareTest(base.BaseSharesAdminTest):
@test.attr(type=["gate", "smoke"]) @test.attr(type=["gate", "smoke"])
def test_manage(self): def test_manage(self):
# After 'unmanage' operation, share instance should be deleted.
# Assert not related to 'manage' test, but placed here for
# resource optimization.
share_instance_list = self.shares_v2_client.list_share_instances()
share_ids = [si['share_id'] for si in share_instance_list]
self.assertNotIn(self.shares[0]['id'], share_ids)
self._test_manage(share=self.shares[0]) self._test_manage(share=self.shares[0])
@test.attr(type=["gate", "smoke"]) @test.attr(type=["gate", "smoke"])