openstack
/
manila
Code Issues Proposed changes

Remove retry logic from manage API 

Lets say, we try to manage a share and share for some reason
goes in 'manage_error' state. Now, if we try manage command again
with the same parameters as earlier, then the logic in API,
tries to reuse old record in DB.

That is unsettling with the REST way of doing things.

Share Manage API is admin-facing by default, Admin should
delete the share before attempting to manage again.

Hence dropping the retry logic.

APIImpact

Change-Id: I0eee6e108fc263e0d2e092eeaa6d0cba47b53dc4
Closes-Bug: #1566504
This commit is contained in:
nidhimittalhada 2016-05-13 12:59:54 +05:30
parent b771466cba
commit 348fde021f
2 changed files with 4 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
manila/share/api.py
View File

@ -532,16 +532,10 @@ class API(base.Base):
LOG.debug("Manage: Found shares %s.", len(shares))
retry_states = (constants.STATUS_MANAGE_ERROR,)
export_location = share_data.pop('export_location')
if len(shares) == 0:
share = self.db.share_create(context, share_data)
# NOTE(u_glide): Case when administrator have fixed some problems and
# tries to manage share again
elif len(shares) == 1 and shares[0]['status'] in retry_states:
share = self.db.share_update(context, shares[0]['id'], share_data)
else:
msg = _("Share already exists.")
raise exception.InvalidShare(reason=msg)

54
manila/tests/share/test_api.py
View File

@ -826,8 +826,8 @@ class ShareAPITestCase(test.TestCase):
self.scheduler_rpcapi.manage_share.assert_called_once_with(
self.context, share['id'], driver_options, expected_request_spec)
@ddt.data([{'id': 'fake', 'status': constants.STATUS_MANAGE_ERROR}])
def test_manage_retry(self, shares):
@ddt.data(constants.STATUS_MANAGE_ERROR, constants.STATUS_AVAILABLE)
def test_manage_duplicate(self, status):
share_data = {
'host': 'fake',
'export_location': 'fake',
@ -835,64 +835,18 @@ class ShareAPITestCase(test.TestCase):
'share_type_id': 'fake',
}
driver_options = {}
fake_share_data = {'id': 'fakeid'}
fake_type = {
'id': 'fake_type_id',
'extra_specs': {
'snapshot_support': False,
'replication_type': 'writable',
},
}
share = db_api.share_create(self.context, fake_share_data)
self.mock_object(db_api, 'share_update',
mock.Mock(return_value=share))
self.mock_object(db_api, 'share_get',
mock.Mock(return_value=share))
self.mock_object(share_types, 'get_share_type',
mock.Mock(return_value=fake_type))
self.mock_object(db_api, 'share_export_locations_update')
shares = [{'id': 'fake', 'status': status}]
self.mock_object(self.api, 'get_all',
mock.Mock(return_value=shares))
self.api.manage(self.context,
copy.deepcopy(share_data),
driver_options)
expected_request_spec = self._get_request_spec_dict(
share, fake_type, size=0, share_proto=share_data['share_proto'],
host=share_data['host'])
db_api.share_update.assert_called_once_with(
self.context, 'fake', mock.ANY)
self.scheduler_rpcapi.manage_share.assert_called_once_with(
self.context, share['id'], driver_options, expected_request_spec)
db_api.share_export_locations_update.assert_called_once_with(
self.context, share.instance['id'], mock.ANY
)
def test_manage_duplicate(self):
share_data = {
'host': 'fake',
'export_location': 'fake',
'share_proto': 'fake',
'share_type_id': 'fake',
}
driver_options = {}
fake_type = {
'id': 'fake_type_id',
'extra_specs': {
'snapshot_support': False,
},
}
self.mock_object(self.api, 'get_all',
mock.Mock(return_value=['fake', 'fake2']))
self.mock_object(share_types, 'get_share_type',
mock.Mock(return_value=fake_type))
self.assertRaises(exception.ManilaException, self.api.manage,
self.assertRaises(exception.InvalidShare, self.api.manage,
self.context, share_data, driver_options)
def _get_request_spec_dict(self, share, share_type, **kwargs):