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)

2 files changed, 112 insertions, 0 deletions

diff --git a/designate/central/service.py b/designate/central/service.py
index eda8a955..69af869a 100644
--- a/designate/central/service.py
+++ b/designate/central/service.py
@@ -2747,6 +2747,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 7649856a..22ac3db4 100644
--- a/designate/tests/test_central/test_service.py
+++ b/designate/tests/test_central/test_service.py
@@ -3409,6 +3409,117 @@ class CentralServiceTest(CentralTestCase):
 
         self.wait_for_import(zone_import.id)
 
+    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_create_zone_import_overquota(self):
         self.config(
             quota_zone_records=5,