From 2862a55057411037c45d065cd59d091ac24d2dc6 Mon Sep 17 00:00:00 2001 From: dekehn Date: Tue, 29 Mar 2022 15:09:29 +0000 Subject: Clarifies the zone import error message This patch defines the situation where the import_zone exception handler produces the message 'An undefined error occurred' to the exception during the zone import where an underlying exception occurred from the rpc dispatcher at the same time as a duplicate zone exception. Kiall Mac Innes explains this in https://opendev.org/openstack/designate/commit/2c9460505d07d0e46765a552b637e5a3296b667b the problem here is in the way the _import_zone was written where https://github.com/openstack/designate/commit/9b809a11b3068552274340606eb76d2217411b85 was written with threading.local, so it only works if it stays within the same thread. Since _import_zone is created on a separate thread, the necessity to add the decorator @rpc_expected_exceptions() at the top. Closes-bug: #1950118 Closes-bug: #1964323 Change-Id: If7d50cbd4fa0ce86e0ddf03068da36acd7d72cb4 (cherry picked from commit d705c5d6b861f319096afdd3ad50a2100651b064) --- designate/central/service.py | 1 + designate/tests/test_central/test_service.py | 111 +++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) diff --git a/designate/central/service.py b/designate/central/service.py index 92be0afa..6c37a8a0 100644 --- a/designate/central/service.py +++ b/designate/central/service.py @@ -3072,6 +3072,7 @@ class Service(service.RPCService): return created_zone_import + @rpc.expected_exceptions() def _import_zone(self, context, zone_import, request_body): zone = None try: diff --git a/designate/tests/test_central/test_service.py b/designate/tests/test_central/test_service.py index 448cf06c..163a396e 100644 --- a/designate/tests/test_central/test_service.py +++ b/designate/tests/test_central/test_service.py @@ -3570,6 +3570,117 @@ class CentralServiceTest(CentralTestCase): zone_import.message) self.assertEqual('ERROR', zone_import.status) + def test_create_zone_import_duplicate_threading(self): + context = self.get_context(project_id=utils.generate_uuid()) + request_body = self.get_zonefile_fixture() + zone_import = self.central_service.create_zone_import(context, + request_body) + self.wait_for_import(zone_import.id) + + def create_zone_import(): + context = self.get_context(project_id=utils.generate_uuid()) + request_body = self.get_zonefile_fixture() + zone_import = self.central_service.create_zone_import(context, + request_body) + return self.wait_for_import(zone_import.id, error_is_ok=True) + + with futurist.GreenThreadPoolExecutor() as executor: + results = [] + for fixture in [0, 2, 3, 4, 5]: + results.append(executor.submit(create_zone_import,)) + for future in futures.as_completed(results): + result = future.result() + self.assertEqual('Duplicate zone.', result.message) + + @mock.patch('dns.zone.from_text') + @mock.patch('designate.central.service.Service.update_zone_import') + def test_create_zone_import_from_text_exceptions( + self, mock_update_zone_import, mock_dnszone_from_text): + + # the second set of exceptions for the create_zone + context = self.get_context(project_id=utils.generate_uuid()) + request_body = self.get_zonefile_fixture() + values = { + 'status': 'PENDING', + 'message': None, + 'zone_id': None, + 'tenant_id': context.project_id, + 'task_type': 'IMPORT' + } + zone_import = objects.ZoneImport(**values) + + mock_dnszone_from_text.side_effect = [exceptions.BadRequest(), + Exception('boom')] + + LOG.debug("Testing zone import exceptions: BadRequest") + zone_import = objects.ZoneImport(**values) + self.central_service._import_zone(context, zone_import, request_body) + self.assertEqual('ERROR', zone_import.status) + self.assertIsNotNone(zone_import.message) + self.assertEqual('An SOA record is required.', zone_import.message) + + LOG.debug("Testing zone import exceptions: Undefined") + zone_import = objects.ZoneImport(**values) + self.central_service._import_zone(context, zone_import, request_body) + self.assertEqual('ERROR', zone_import.status) + self.assertIsNotNone(zone_import.message) + self.assertEqual('An undefined error occurred. boom', + zone_import.message) + + @mock.patch('designate.central.service.Service.create_zone') + @mock.patch('designate.central.service.Service.update_zone_import') + def test_create_zone_import_create_import_exceptions( + self, mock_update_zone_import, mock_create_zone): + + # setup to test the create_zone exceptions from _import_zone method + context = self.get_context(project_id=utils.generate_uuid()) + request_body = self.get_zonefile_fixture() + values = { + 'status': 'PENDING', + 'message': None, + 'zone_id': None, + 'tenant_id': context.project_id, + 'task_type': 'IMPORT' + } + zone_import = objects.ZoneImport(**values) + + mock_create_zone.side_effect = [mock.DEFAULT, + exceptions.DuplicateZone(), + exceptions.InvalidTTL(), + exceptions.OverQuota(), + Exception('boom')] + + LOG.debug("Testing zone import exceptions: No exception case") + self.central_service._import_zone(context, zone_import, request_body) + self.assertEqual('COMPLETE', zone_import.status) + self.assertIn('imported', zone_import.message) + + LOG.debug("Testing zone import exceptions: DuplicateZone") + zone_import = objects.ZoneImport(**values) + self.central_service._import_zone(context, zone_import, request_body) + self.assertEqual('ERROR', zone_import.status) + self.assertEqual('Duplicate zone.', zone_import.message) + + LOG.debug("Testing zone import exceptions: InvalidTTL") + zone_import = objects.ZoneImport(**values) + self.central_service._import_zone(context, zone_import, request_body) + self.assertEqual('ERROR', zone_import.status) + self.assertIsNotNone(zone_import.message) + + LOG.debug("Testing zone import exceptions: OverQuota") + zone_import = objects.ZoneImport(**values) + self.central_service._import_zone(context, zone_import, request_body) + self.assertEqual('ERROR', zone_import.status) + self.assertEqual('Quota exceeded during zone import.', + zone_import.message) + + LOG.debug("Testing zone import exceptions: Undefined") + zone_import = objects.ZoneImport(**values) + self.central_service._import_zone(context, zone_import, request_body) + self.assertEqual('ERROR', zone_import.status) + self.assertEqual('An undefined error occurred. boom', + zone_import.message) + def test_find_zone_imports(self): context = self.get_context(project_id=utils.generate_uuid()) -- cgit v1.2.1