Prevent hanging share server in 'creating' state

Currently, share server can hang in 'creating' state
because except block does not take into account possibility
of error occurring in share_server_backend_details_set() method.

- Write server details one by one with individual error handling
- Move call of share_server_update() to 'finally' block to
prevent hanging share server in 'creating' state

Change-Id: I77bdf62fbaa66a226944fb0c07fb2b8dfaf11a0e
Closes-Bug: #1434563
This commit is contained in:
Igor Malinovskiy 2015-04-10 11:04:03 +03:00
parent cdcf2e7671
commit 815e2c95ac
2 changed files with 84 additions and 15 deletions

View File

@ -699,24 +699,46 @@ class ShareManager(manager.SchedulerDependentManager):
{'status': constants.STATUS_ACTIVE})
except Exception as e:
with excutils.save_and_reraise_exception():
detail_data = getattr(e, 'detail_data', {})
if (type(detail_data) is dict and
detail_data.get('server_details')):
try:
details = getattr(e, 'detail_data', {})
if not isinstance(details, dict):
msg = (_("Invalid detail_data '%s'")
% six.text_type(details))
raise exception.Invalid(msg)
server_details = detail_data['server_details']
server_details = details.get('server_details')
if isinstance(server_details, dict):
self.db.share_server_backend_details_set(
context, share_server['id'], server_details)
else:
LOG.warning(_LW('Server Information in '
'exception can not be written to db '
'because it contains %s and it is not '
'a dictionary.'), server_details)
if not isinstance(server_details, dict):
msg = (_("Invalid server_details '%s'")
% six.text_type(server_details))
raise exception.Invalid(msg)
self.db.share_server_update(context, share_server['id'],
{'status': constants.STATUS_ERROR})
self.driver.deallocate_network(context, share_server['id'])
invalid_details = []
for key, value in server_details.items():
try:
self.db.share_server_backend_details_set(
context, share_server['id'], {key: value})
except Exception:
invalid_details.append("%(key)s: %(value)s" % {
'key': six.text_type(key),
'value': six.text_type(value)
})
if invalid_details:
msg = (_("Following server details are not valid:\n%s")
% six.text_type('\n'.join(invalid_details)))
raise exception.Invalid(msg)
except Exception as e:
LOG.warning(_LW('Server Information in '
'exception can not be written to db : %s '
), six.text_type(e))
finally:
self.db.share_server_update(
context, share_server['id'],
{'status': constants.STATUS_ERROR})
self.driver.deallocate_network(context, share_server['id'])
def _validate_segmentation_id(self, network_info):
"""Raises exception if the segmentation type is incorrect."""

View File

@ -1313,6 +1313,53 @@ class ShareManagerTestCase(test.TestCase):
def test_setup_server_exception_in_driver(self):
self.setup_server_raise_exception(detail_data_proper=True)
@ddt.data({},
{'detail_data': 'fake'},
{'detail_data': {'server_details': 'fake'}},
{'detail_data': {'server_details': {'fake': 'fake'}}},
{'detail_data': {
'server_details': {'fake': 'fake', 'fake2': 'fake2'}}},)
def test_setup_server_exception_in_cleanup_after_error(self, data):
def get_server_details_from_data(data):
d = data.get('detail_data')
if not isinstance(d, dict):
return {}
d = d.get('server_details')
if not isinstance(d, dict):
return {}
return d
share_server = {'id': 'fake', 'share_network_id': 'fake'}
details = get_server_details_from_data(data)
exc_mock = mock.Mock(side_effect=exception.ManilaException(**data))
details_mock = mock.Mock(side_effect=exception.ManilaException())
self.mock_object(self.share_manager.db, 'share_network_get', exc_mock)
self.mock_object(self.share_manager.db,
'share_server_backend_details_set', details_mock)
self.mock_object(self.share_manager.db, 'share_server_update')
self.mock_object(self.share_manager.driver, 'deallocate_network')
self.assertRaises(
exception.ManilaException,
self.share_manager._setup_server,
self.context,
share_server,
)
self.assertTrue(self.share_manager.db.share_network_get.called)
if details:
self.assertEqual(len(details), details_mock.call_count)
expected = [mock.call(mock.ANY, share_server['id'], {k: v})
for k, v in details.items()]
self.assertEqual(expected, details_mock.call_args_list)
self.share_manager.db.share_server_update.assert_called_once_with(
self.context, share_server['id'], {'status': 'ERROR'})
self.share_manager.driver.deallocate_network.assert_called_once_with(
self.context, share_server['id']
)
def test_ensure_share_has_pool_with_only_host(self):
fake_share = {'status': 'available', 'host': 'host1', 'id': 1}
host = self.share_manager._ensure_share_has_pool(context.