Merge ~cgrabowski/maas:backport_lp2062107_fix_3.4 into maas:3.4

[Christian Grabowski]

fix: handle BIND failures when notifies are delayed
(cherry picked from commit 19ac75f3d5e2bcb7d4934579aa94717dc1823104)

[Christian Grabowski (cgrabowski)]

self-approving backport

review: Approve

diff --git a/src/maasserver/region_controller.py b/src/maasserver/region_controller.py
index 7cc6428..590ca1d 100644
--- a/src/maasserver/region_controller.py
+++ b/src/maasserver/region_controller.py
@@ -329,10 +329,6 @@ class RegionControllerService(Service):
             return None
         serial, reloaded, domain_names = result
 
-        # check that there is not a newer serial we should query instead
-        if self._dns_latest_serial and self._dns_latest_serial > serial:
-            return result
-
         if not reloaded:
             raise DNSReloadError(
                 "Failed to reload DNS; timeout or rdnc command failed."
@@ -340,6 +336,10 @@ class RegionControllerService(Service):
         not_matching_domains = set(domain_names)
         loop = 0
         while len(not_matching_domains) > 0 and loop != 30:
+            # check that there is not a newer serial we should query instead
+            if self._dns_latest_serial and self._dns_latest_serial > serial:
+                return result
+
             for domain in list(not_matching_domains):
                 try:
                     answers, _, _ = yield self.dnsResolver.lookupAuthority(
@@ -401,7 +401,7 @@ class RegionControllerService(Service):
     def _onDNSReloadFailure(self, failure):
         """Force kill and restart bind9."""
         failure.trap(DNSReloadError)
-        if not self.retryOnFailure:
+        if not self.retryOnFailure or self._dns_update_in_progress:
             return failure
         log.err(failure, "Failed configuring DNS; killing and restarting")
         d = service_monitor.killService("bind9")
diff --git a/src/maasserver/tests/test_region_controller.py b/src/maasserver/tests/test_region_controller.py
index 0b45912..bdcd75f 100644
--- a/src/maasserver/tests/test_region_controller.py
+++ b/src/maasserver/tests/test_region_controller.py
@@ -24,6 +24,7 @@ from maasserver.region_controller import (
     RegionControllerService,
 )
 from maasserver.secrets import SecretManager
+from maasserver.service_monitor import service_monitor
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import (
     MAASServerTestCase,
@@ -346,6 +347,28 @@ class TestRegionControllerService(MAASServerTestCase):
 
     @wait_for_reactor
     @inlineCallbacks
+    def test_process_waits_for_in_progress_update_for_dns_failure_restart(
+        self,
+    ):
+        service = self.make_service(sentinel.listener, sentinel.dbtasks)
+        service.needsDNSUpdate = True
+        mock_kill_service = self.patch(service_monitor, "killService")
+        self.patch(region_controller, "dns_update_all_zones")
+
+        def set_in_progress(*args):
+            service._dns_update_in_progress = True
+
+        self.patch(
+            region_controller, "_clear_dynamic_dns_update"
+        ).side_effect = set_in_progress
+        mock_dns_check_serial = self.patch(region_controller, "_checkSerial")
+        mock_dns_check_serial.side_effect = fail(DNSReloadError())
+        service.startProcessing()
+        yield service.processingDefer
+        mock_kill_service.assert_not_called()
+
+    @wait_for_reactor
+    @inlineCallbacks
     def test_process_updates_proxy_logs_failure(self):
         service = self.make_service(sentinel.listener, sentinel.dbtasks)
         service.needsProxyUpdate = True
@@ -512,6 +535,38 @@ class TestRegionControllerService(MAASServerTestCase):
         with ExpectedException(DNSReloadError):
             yield service._checkSerial((formatted_serial, True, dns_names))
 
+    @wait_for_reactor
+    @inlineCallbacks
+    def test_check_serial_returns_early_if_newer_serial_exists(self):
+        service = self.make_service(sentinel.listern, sentinel.dbtasks)
+        first_serial = random.randint(1, 1000)
+        second_serial = first_serial + 1
+        formatted_first_serial = f"{first_serial:10d}"
+        formatted_second_serial = f"{second_serial:10d}"
+        dns_names = [factory.make_name("domain") for _ in range(3)]
+        mock_lookup = self.patch(service.dnsResolver, "lookupAuthority")
+        mock_lookup.side_effect = [
+            # First pass no results.
+            succeed(([], [], [])),
+            succeed(([], [], [])),
+            succeed(([], [], [])),
+            # First domain valid result.
+            succeed(([self.make_soa_result(first_serial)], [], [])),
+            succeed(([], [], [])),
+            succeed(([], [], [])),
+            # Second domain wrong serial.
+            succeed(([self.make_soa_result(first_serial)], [], [])),
+            succeed(([], [], [])),
+            # Third domain correct serial.
+            succeed(([], [], [])),
+            succeed(([self.make_soa_result(second_serial)], [], [])),
+            # Second domain correct serial.
+            succeed(([self.make_soa_result(second_serial)], [], [])),
+        ]
+        d = service._checkSerial((formatted_first_serial, True, dns_names))
+        service._dns_latest_serial = formatted_second_serial
+        yield d
+
     def test_getRBACClient_returns_None_when_no_url(self):
         service = self.make_service(sentinel.listener, sentinel.dbtasks)
         service.rbacClient = sentinel.client
diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
index cf8a8ef..2a80cbe 100644
--- a/src/provisioningserver/dns/zoneconfig.py
+++ b/src/provisioningserver/dns/zoneconfig.py
@@ -226,7 +226,6 @@ class DomainConfigBase:
                     uid=os.getuid(),
                     gid=os.getgid(),
                 )
-        pass
 
 
 class DNSForwardZoneConfig(DomainConfigBase):