Merge ~cgrabowski/maas:fix_missing_reverse_records into maas:master

[Christian Grabowski]

use zone_info.zone_name instead of domain to freeze/thaw reverse updates

fix default ttls

[MAAS Lander (maas-lander)]

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

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/1852/consoleText
COMMIT: 7a65a764010b35d286f979307292bb53079cd444

review: Needs Fixing

ccc33f1... by Christian Grabowski on 2023-01-25

move freeze/thaw into a contextmanager

[MAAS Lander (maas-lander)]

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

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/1855/consoleText
COMMIT: 7a020cbe52a0fa92d42c2a8ef8bdfc9d333821df

review: Needs Fixing

[Adam Collard (adam-collard)]

review: Approve

[MAAS Lander (maas-lander)]

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

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/1856/consoleText
COMMIT: 7baf3114bede3016c0344cb0d928b1a8aa5c3347

review: Needs Fixing

[MAAS Lander (maas-lander)]

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

STATUS: SUCCESS
COMMIT: ccc33f1d732fb47da1b69c5b1b38cd3703a85a94

review: Approve

diff --git a/src/maasserver/triggers/system.py b/src/maasserver/triggers/system.py
index bf4c51f..c6313f9 100644
--- a/src/maasserver/triggers/system.py
+++ b/src/maasserver/triggers/system.py
@@ -2143,7 +2143,7 @@ def render_dns_dynamic_update_node(op):
           domain text;
           address_ttl int;
         BEGIN
-          IF ((TG_OP = 'INSERT' OR  TG_OP = 'UPDATE') AND TG_LEVEL = 'ROW') THEN
+          IF ((TG_OP = 'INSERT' OR TG_OP = 'UPDATE') AND TG_LEVEL = 'ROW') THEN
               IF NEW.node_type <> {NODE_TYPE.DEVICE} AND NEW.node_type <> {NODE_TYPE.MACHINE} THEN
                   PERFORM pg_notify('sys_dns_updates', 'RELOAD');
               END IF;
diff --git a/src/provisioningserver/dns/actions.py b/src/provisioningserver/dns/actions.py
index 7749ff5..64fffda 100644
--- a/src/provisioningserver/dns/actions.py
+++ b/src/provisioningserver/dns/actions.py
@@ -4,6 +4,7 @@
 """Low-level actions to manage the DNS service, like reloading zones."""
 
 from collections.abc import Sequence
+from contextlib import contextmanager, nullcontext
 from subprocess import CalledProcessError, TimeoutExpired
 from time import sleep
 
@@ -84,6 +85,18 @@ def bind_thaw_zone(zone=None, timeout=2):
         raise
 
 
+@contextmanager
+def freeze_thaw_zone(required, zone=None, timeout=2):
+    if not required:
+        yield nullcontext()
+    else:
+        bind_freeze_zone(zone=zone, timeout=timeout)
+        try:
+            yield
+        finally:
+            bind_thaw_zone(zone=zone, timeout=timeout)
+
+
 def bind_reload(timeout=2):
     """Ask BIND to reload its configuration and all zone files.  This operation
     is 'best effort' (with logging) as the server may not be running, and there
diff --git a/src/provisioningserver/dns/config.py b/src/provisioningserver/dns/config.py
index 729b570..6e11f73 100644
--- a/src/provisioningserver/dns/config.py
+++ b/src/provisioningserver/dns/config.py
@@ -62,6 +62,8 @@ class DynamicDNSUpdate:
         else:
             if ip.version == 6:
                 rectype = "AAAA"
+        if kwargs.get("ttl") == 0:  # default ttl
+            kwargs["ttl"] = 30
         return cls(answer=answer, rectype=rectype, **kwargs)
 
     @classmethod
diff --git a/src/provisioningserver/dns/tests/test_zoneconfig.py b/src/provisioningserver/dns/tests/test_zoneconfig.py
index 10ee0f7..f747a53 100644
--- a/src/provisioningserver/dns/tests/test_zoneconfig.py
+++ b/src/provisioningserver/dns/tests/test_zoneconfig.py
@@ -8,6 +8,7 @@ from itertools import chain
 import os.path
 import random
 from tempfile import mktemp
+from unittest.mock import call
 
 from netaddr import IPAddress, IPNetwork, IPRange
 from testtools.matchers import (
@@ -488,6 +489,42 @@ class TestDNSForwardZoneConfig(MAASTestCase):
             stdin=expected_stdin.encode("ascii"),
         )
 
+    def test_full_reload_calls_freeze_thaw(self):
+        patch_zone_file_config_path(self)
+        execute_rndc_command = self.patch(actions, "execute_rndc_command")
+        domain = factory.make_string()
+        network = factory.make_ipv4_network()
+        ipv4_hostname = factory.make_name("host")
+        ipv4_ip = factory.pick_ip_in_network(network)
+        ipv6_hostname = factory.make_name("host")
+        ipv6_ip = factory.make_ipv6_address()
+        ipv6_network = factory.make_ipv6_network()
+        dynamic_range = IPRange(ipv6_network.first, ipv6_network.last)
+        ttl = random.randint(10, 300)
+        mapping = {
+            ipv4_hostname: HostnameIPMapping(None, ttl, {ipv4_ip}),
+            ipv6_hostname: HostnameIPMapping(None, ttl, {ipv6_ip}),
+        }
+        dns_zone_config = DNSForwardZoneConfig(
+            domain,
+            serial=random.randint(1, 100),
+            mapping=mapping,
+            default_ttl=ttl,
+            dynamic_ranges=[dynamic_range],
+        )
+        self.patch(dns_zone_config, "get_GENERATE_directives")
+        self.patch(actions, "run_command")
+        dns_zone_config.write_config()
+        dns_zone_config.force_config_write = True
+        dns_zone_config.write_config()
+        self.assertCountEqual(
+            execute_rndc_command.call_args_list,
+            [
+                call(("freeze", domain), timeout=2),
+                call(("thaw", domain), timeout=2),
+            ],
+        )
+
 
 class TestDNSReverseZoneConfig(MAASTestCase):
     """Tests for DNSReverseZoneConfig."""
@@ -1077,6 +1114,30 @@ class TestDNSReverseZoneConfig(MAASTestCase):
             stdin=expected_stdin.encode("ascii"),
         )
 
+    def test_full_reload_calls_freeze_thaw(self):
+        patch_zone_file_config_path(self)
+        execute_rndc_command = self.patch(
+            provisioningserver.dns.actions, "execute_rndc_command"
+        )
+        domain = factory.make_string()
+        network = IPNetwork("10.0.0.0/24")
+        zone = DNSReverseZoneConfig(
+            domain,
+            serial=random.randint(1, 100),
+            network=network,
+        )
+        self.patch(actions, "run_command")
+        zone.write_config()
+        zone.force_config_write = True
+        zone.write_config()
+        self.assertCountEqual(
+            execute_rndc_command.call_args_list,
+            [
+                call(("freeze", "0.0.10.in-addr.arpa"), timeout=2),
+                call(("thaw", "0.0.10.in-addr.arpa"), timeout=2),
+            ],
+        )
+
 
 class TestDNSReverseZoneConfig_GetGenerateDirectives(MAASTestCase):
     """Tests for `DNSReverseZoneConfig.get_GENERATE_directives()`."""
diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
index 3171bae..ee6c15e 100644
--- a/src/provisioningserver/dns/zoneconfig.py
+++ b/src/provisioningserver/dns/zoneconfig.py
@@ -12,11 +12,7 @@ from pathlib import Path
 from netaddr import IPAddress, IPNetwork, spanning_cidr
 from netaddr.core import AddrFormatError
 
-from provisioningserver.dns.actions import (
-    bind_freeze_zone,
-    bind_thaw_zone,
-    NSUpdateCommand,
-)
+from provisioningserver.dns.actions import freeze_thaw_zone, NSUpdateCommand
 from provisioningserver.dns.config import (
     compose_zone_file_config_path,
     render_dns_template,
@@ -344,9 +340,7 @@ class DNSForwardZoneConfig(DomainConfigBase):
                 Path(f"{zi.target_path}.jnl").unlink(missing_ok=True)
                 self.requires_reload = True
                 needs_freeze_thaw = self.zone_file_exists(zi)
-                if needs_freeze_thaw:
-                    bind_freeze_zone(zone=self.domain)
-                try:
+                with freeze_thaw_zone(needs_freeze_thaw, zone=zi.zone_name):
                     self.write_zone_file(
                         zi.target_path,
                         self.make_parameters(),
@@ -365,9 +359,6 @@ class DNSForwardZoneConfig(DomainConfigBase):
                             "generate_directives": {"A": generate_directives},
                         },
                     )
-                finally:
-                    if needs_freeze_thaw:
-                        bind_thaw_zone(zone=self.domain)
                 PROMETHEUS_METRICS.update(
                     "maas_dns_full_zonefile_write_count",
                     "inc",
@@ -650,26 +641,28 @@ class DNSReverseZoneConfig(DomainConfigBase):
             else:
                 Path(f"{zi.target_path}.jnl").unlink(missing_ok=True)
                 self.requires_reload = True
-                self.write_zone_file(
-                    zi.target_path,
-                    self.make_parameters(),
-                    {
-                        "mappings": {
-                            "PTR": self.get_PTR_mapping(
-                                self._mapping, zi.subnetwork
-                            )
-                        },
-                        "other_mapping": [],
-                        "generate_directives": {
-                            "PTR": generate_directives,
-                            "CNAME": self.get_rfc2317_GENERATE_directives(
-                                zi.subnetwork,
-                                self._rfc2317_ranges,
-                                self.domain,
-                            ),
+                needs_freeze_thaw = self.zone_file_exists(zi)
+                with freeze_thaw_zone(needs_freeze_thaw, zone=zi.zone_name):
+                    self.write_zone_file(
+                        zi.target_path,
+                        self.make_parameters(),
+                        {
+                            "mappings": {
+                                "PTR": self.get_PTR_mapping(
+                                    self._mapping, zi.subnetwork
+                                )
+                            },
+                            "other_mapping": [],
+                            "generate_directives": {
+                                "PTR": generate_directives,
+                                "CNAME": self.get_rfc2317_GENERATE_directives(
+                                    zi.subnetwork,
+                                    self._rfc2317_ranges,
+                                    self.domain,
+                                ),
+                            },
                         },
-                    },
-                )
+                    )
                 PROMETHEUS_METRICS.update(
                     "maas_dns_full_zonefile_write_count",
                     "inc",