Merge ~cgrabowski/maas:fix_dns_update_race_condition into maas:master

[Christian Grabowski]

store updates in a separate list while updating BIND

no longer remove BIND jnl

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_dns_update_race_condition lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: a2e3f48b6d1b268f2bc9c4935df538227aa117d0

review: Approve

[Alexsander de Souza (alexsander-souza)]

+1

review: Approve

diff --git a/src/maasserver/region_controller.py b/src/maasserver/region_controller.py
index a7fa69c..a8cb828 100644
--- a/src/maasserver/region_controller.py
+++ b/src/maasserver/region_controller.py
@@ -98,6 +98,8 @@ class RegionControllerService(Service):
         self.needsProxyUpdate = False
         self.needsRBACUpdate = False
         self._dns_updates = []
+        self._queued_updates = []
+        self._dns_update_in_progress = False
         self._dns_requires_full_reload = True
         self.postgresListener = postgresListener
         self.dnsResolver = Resolver(
@@ -185,7 +187,10 @@ class RegionControllerService(Service):
         self._dns_requires_full_reload = (
             self._dns_requires_full_reload or need_reload
         )
-        self._dns_updates += new_updates
+        if self._dns_update_in_progress:
+            self._queued_updates += new_updates
+        else:
+            self._dns_updates += new_updates
 
     def startProcessing(self):
         """Start the process looping call."""
@@ -217,13 +222,19 @@ class RegionControllerService(Service):
                 return pause(delay)
 
         def _clear_dynamic_dns_updates(d):
-            self._dns_updates = []
+            if len(self._queued_updates) > 0:
+                self._dns_updates = self._queued_updates
+                self._queued_updates = []
+            else:
+                self._dns_updates = []
             self._dns_requires_full_reload = False
+            self._dns_update_in_progress = False
             return d
 
         defers = []
         if self.needsDNSUpdate:
             self.needsDNSUpdate = False
+            self._dns_update_in_progress = True
             d = deferToDatabase(
                 transactional(
                     dns_update_all_zones,
diff --git a/src/maasserver/tests/test_region_controller.py b/src/maasserver/tests/test_region_controller.py
index 4e97cdd..072d3b3 100644
--- a/src/maasserver/tests/test_region_controller.py
+++ b/src/maasserver/tests/test_region_controller.py
@@ -36,6 +36,7 @@ from maastesting.matchers import (
     MockCallsMatch,
     MockNotCalled,
 )
+from provisioningserver.dns.config import DynamicDNSUpdate
 from provisioningserver.utils.events import Event
 
 wait_for_reactor = wait_for()
@@ -841,3 +842,54 @@ class TestRegionControllerServiceTransactional(MAASTransactionServerTestCase):
         self.assertEqual(rbac_sync.id, last_sync.id)
         self.assertEqual(last_sync.resource_type, "resource-pool")
         self.assertEqual(last_sync.sync_id, "x-y-z")
+
+    @wait_for_reactor
+    @inlineCallbacks
+    def test_queueDynamicDNSUpdate_queues_in_separate_list_while_update_in_progress(
+        self,
+    ):
+        domain = yield deferToDatabase(factory.make_Domain)
+        update_result = (random.randint(0, 10), True, [domain.name])
+        record = yield deferToDatabase(factory.make_DNSResource, domain=domain)
+        service = RegionControllerService(sentinel.listener)
+
+        update_zones = self.patch(region_controller, "dns_update_all_zones")
+        update_zones.return_value = update_result
+        check_serial = self.patch(service, "_checkSerial")
+        check_serial.return_value = succeed(update_result)
+
+        service._dns_update_in_progress = True
+        service.queueDynamicDNSUpdate(
+            factory.make_name(),
+            f"INSERT {domain.name} {record.name} A 30 10.10.10.10",
+        )
+        self.assertCountEqual(service._dns_updates, [])
+        self.assertCountEqual(
+            service._queued_updates,
+            [
+                DynamicDNSUpdate(
+                    operation="INSERT",
+                    name=f"{record.name}.{domain.name}",
+                    zone=domain.name,
+                    rectype="A",
+                    ttl=30,
+                    answer="10.10.10.10",
+                )
+            ],
+        )
+        service.needsDNSUpdate = True
+        yield service.process()
+        self.assertCountEqual(
+            service._dns_updates,
+            [
+                DynamicDNSUpdate(
+                    operation="INSERT",
+                    name=f"{record.name}.{domain.name}",
+                    zone=domain.name,
+                    rectype="A",
+                    ttl=30,
+                    answer="10.10.10.10",
+                )
+            ],
+        )
+        self.assertCountEqual(service._queued_updates, [])
diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
index ec5da14..2992337 100644
--- a/src/provisioningserver/dns/zoneconfig.py
+++ b/src/provisioningserver/dns/zoneconfig.py
@@ -7,7 +7,6 @@
 from datetime import datetime
 from itertools import chain
 import os
-from pathlib import Path
 
 from netaddr import IPAddress, IPNetwork, spanning_cidr
 from netaddr.core import AddrFormatError
@@ -347,7 +346,6 @@ class DNSForwardZoneConfig(DomainConfigBase):
                     labels={"zone": self.domain},
                 )
             else:
-                Path(f"{zi.target_path}.jnl").unlink(missing_ok=True)
                 self.requires_reload = True
                 needs_freeze_thaw = self.zone_file_exists(zi)
                 with freeze_thaw_zone(needs_freeze_thaw, zone=zi.zone_name):
@@ -649,7 +647,6 @@ class DNSReverseZoneConfig(DomainConfigBase):
                     labels={"zone": self.domain},
                 )
             else:
-                Path(f"{zi.target_path}.jnl").unlink(missing_ok=True)
                 self.requires_reload = True
                 needs_freeze_thaw = self.zone_file_exists(zi)
                 with freeze_thaw_zone(needs_freeze_thaw, zone=zi.zone_name):