diff --git a/manila/share/manager.py b/manila/share/manager.py index 942100f7a6..5be779cb7d 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -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.""" diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index ed321efab5..ec37fd29c4 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -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.