Merge ~cgrabowski/maas:backport_fix_missing_reverse_records into maas:3.3

Proposed by Christian Grabowski
Status: Merged
Approved by: Christian Grabowski
Approved revision: 0c9c3136e4e1548e83c7ae068b2c8ec7545c08c5
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~cgrabowski/maas:backport_fix_missing_reverse_records
Merge into: maas:3.3
Diff against target: 226 lines (+100/-31)
5 files modified
src/maasserver/triggers/system.py (+1/-1)
src/provisioningserver/dns/actions.py (+13/-0)
src/provisioningserver/dns/config.py (+2/-0)
src/provisioningserver/dns/tests/test_zoneconfig.py (+61/-0)
src/provisioningserver/dns/zoneconfig.py (+23/-30)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Christian Grabowski Approve
Review via email: mp+436339@code.launchpad.net

Commit message

use zone_info.zone_name instead of domain to freeze/thaw reverse updates
fix default ttls
(cherry picked from commit d4b96129a725fda571a05a67efecde937cb55da2)

To post a comment you must log in.
Revision history for this message
Christian Grabowski (cgrabowski) wrote :

self-approving backport

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b backport_fix_missing_reverse_records lp:~cgrabowski/maas/+git/maas into -b 3.3 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas-tester/1860/consoleText
COMMIT: 0c9c3136e4e1548e83c7ae068b2c8ec7545c08c5

review: Needs Fixing
Revision history for this message
Christian Grabowski (cgrabowski) wrote :

jenkins: !test

Revision history for this message
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b backport_fix_missing_reverse_records lp:~cgrabowski/maas/+git/maas into -b 3.3 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 0c9c3136e4e1548e83c7ae068b2c8ec7545c08c5

review: Approve
Revision history for this message
MAAS Lander (maas-lander) wrote :

LANDING
-b backport_fix_missing_reverse_records lp:~cgrabowski/maas/+git/maas into -b 3.3 lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas-tester/1863/consoleText

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/triggers/system.py b/src/maasserver/triggers/system.py
2index 8e5ba5a..9aa7e41 100644
3--- a/src/maasserver/triggers/system.py
4+++ b/src/maasserver/triggers/system.py
5@@ -2143,7 +2143,7 @@ def render_dns_dynamic_update_node(op):
6 domain text;
7 address_ttl int;
8 BEGIN
9- IF ((TG_OP = 'INSERT' OR TG_OP = 'UPDATE') AND TG_LEVEL = 'ROW') THEN
10+ IF ((TG_OP = 'INSERT' OR TG_OP = 'UPDATE') AND TG_LEVEL = 'ROW') THEN
11 IF NEW.node_type <> {NODE_TYPE.DEVICE} AND NEW.node_type <> {NODE_TYPE.MACHINE} THEN
12 PERFORM pg_notify('sys_dns_updates', 'RELOAD');
13 END IF;
14diff --git a/src/provisioningserver/dns/actions.py b/src/provisioningserver/dns/actions.py
15index 7749ff5..64fffda 100644
16--- a/src/provisioningserver/dns/actions.py
17+++ b/src/provisioningserver/dns/actions.py
18@@ -4,6 +4,7 @@
19 """Low-level actions to manage the DNS service, like reloading zones."""
20
21 from collections.abc import Sequence
22+from contextlib import contextmanager, nullcontext
23 from subprocess import CalledProcessError, TimeoutExpired
24 from time import sleep
25
26@@ -84,6 +85,18 @@ def bind_thaw_zone(zone=None, timeout=2):
27 raise
28
29
30+@contextmanager
31+def freeze_thaw_zone(required, zone=None, timeout=2):
32+ if not required:
33+ yield nullcontext()
34+ else:
35+ bind_freeze_zone(zone=zone, timeout=timeout)
36+ try:
37+ yield
38+ finally:
39+ bind_thaw_zone(zone=zone, timeout=timeout)
40+
41+
42 def bind_reload(timeout=2):
43 """Ask BIND to reload its configuration and all zone files. This operation
44 is 'best effort' (with logging) as the server may not be running, and there
45diff --git a/src/provisioningserver/dns/config.py b/src/provisioningserver/dns/config.py
46index 729b570..6e11f73 100644
47--- a/src/provisioningserver/dns/config.py
48+++ b/src/provisioningserver/dns/config.py
49@@ -62,6 +62,8 @@ class DynamicDNSUpdate:
50 else:
51 if ip.version == 6:
52 rectype = "AAAA"
53+ if kwargs.get("ttl") == 0: # default ttl
54+ kwargs["ttl"] = 30
55 return cls(answer=answer, rectype=rectype, **kwargs)
56
57 @classmethod
58diff --git a/src/provisioningserver/dns/tests/test_zoneconfig.py b/src/provisioningserver/dns/tests/test_zoneconfig.py
59index 10ee0f7..f747a53 100644
60--- a/src/provisioningserver/dns/tests/test_zoneconfig.py
61+++ b/src/provisioningserver/dns/tests/test_zoneconfig.py
62@@ -8,6 +8,7 @@ from itertools import chain
63 import os.path
64 import random
65 from tempfile import mktemp
66+from unittest.mock import call
67
68 from netaddr import IPAddress, IPNetwork, IPRange
69 from testtools.matchers import (
70@@ -488,6 +489,42 @@ class TestDNSForwardZoneConfig(MAASTestCase):
71 stdin=expected_stdin.encode("ascii"),
72 )
73
74+ def test_full_reload_calls_freeze_thaw(self):
75+ patch_zone_file_config_path(self)
76+ execute_rndc_command = self.patch(actions, "execute_rndc_command")
77+ domain = factory.make_string()
78+ network = factory.make_ipv4_network()
79+ ipv4_hostname = factory.make_name("host")
80+ ipv4_ip = factory.pick_ip_in_network(network)
81+ ipv6_hostname = factory.make_name("host")
82+ ipv6_ip = factory.make_ipv6_address()
83+ ipv6_network = factory.make_ipv6_network()
84+ dynamic_range = IPRange(ipv6_network.first, ipv6_network.last)
85+ ttl = random.randint(10, 300)
86+ mapping = {
87+ ipv4_hostname: HostnameIPMapping(None, ttl, {ipv4_ip}),
88+ ipv6_hostname: HostnameIPMapping(None, ttl, {ipv6_ip}),
89+ }
90+ dns_zone_config = DNSForwardZoneConfig(
91+ domain,
92+ serial=random.randint(1, 100),
93+ mapping=mapping,
94+ default_ttl=ttl,
95+ dynamic_ranges=[dynamic_range],
96+ )
97+ self.patch(dns_zone_config, "get_GENERATE_directives")
98+ self.patch(actions, "run_command")
99+ dns_zone_config.write_config()
100+ dns_zone_config.force_config_write = True
101+ dns_zone_config.write_config()
102+ self.assertCountEqual(
103+ execute_rndc_command.call_args_list,
104+ [
105+ call(("freeze", domain), timeout=2),
106+ call(("thaw", domain), timeout=2),
107+ ],
108+ )
109+
110
111 class TestDNSReverseZoneConfig(MAASTestCase):
112 """Tests for DNSReverseZoneConfig."""
113@@ -1077,6 +1114,30 @@ class TestDNSReverseZoneConfig(MAASTestCase):
114 stdin=expected_stdin.encode("ascii"),
115 )
116
117+ def test_full_reload_calls_freeze_thaw(self):
118+ patch_zone_file_config_path(self)
119+ execute_rndc_command = self.patch(
120+ provisioningserver.dns.actions, "execute_rndc_command"
121+ )
122+ domain = factory.make_string()
123+ network = IPNetwork("10.0.0.0/24")
124+ zone = DNSReverseZoneConfig(
125+ domain,
126+ serial=random.randint(1, 100),
127+ network=network,
128+ )
129+ self.patch(actions, "run_command")
130+ zone.write_config()
131+ zone.force_config_write = True
132+ zone.write_config()
133+ self.assertCountEqual(
134+ execute_rndc_command.call_args_list,
135+ [
136+ call(("freeze", "0.0.10.in-addr.arpa"), timeout=2),
137+ call(("thaw", "0.0.10.in-addr.arpa"), timeout=2),
138+ ],
139+ )
140+
141
142 class TestDNSReverseZoneConfig_GetGenerateDirectives(MAASTestCase):
143 """Tests for `DNSReverseZoneConfig.get_GENERATE_directives()`."""
144diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
145index 59e714b..93f2504 100644
146--- a/src/provisioningserver/dns/zoneconfig.py
147+++ b/src/provisioningserver/dns/zoneconfig.py
148@@ -12,11 +12,7 @@ from pathlib import Path
149 from netaddr import IPAddress, IPNetwork, spanning_cidr
150 from netaddr.core import AddrFormatError
151
152-from provisioningserver.dns.actions import (
153- bind_freeze_zone,
154- bind_thaw_zone,
155- NSUpdateCommand,
156-)
157+from provisioningserver.dns.actions import freeze_thaw_zone, NSUpdateCommand
158 from provisioningserver.dns.config import (
159 compose_zone_file_config_path,
160 render_dns_template,
161@@ -338,9 +334,7 @@ class DNSForwardZoneConfig(DomainConfigBase):
162 Path(f"{zi.target_path}.jnl").unlink(missing_ok=True)
163 self.requires_reload = True
164 needs_freeze_thaw = self.zone_file_exists(zi)
165- if needs_freeze_thaw:
166- bind_freeze_zone(zone=self.domain)
167- try:
168+ with freeze_thaw_zone(needs_freeze_thaw, zone=zi.zone_name):
169 self.write_zone_file(
170 zi.target_path,
171 self.make_parameters(),
172@@ -359,9 +353,6 @@ class DNSForwardZoneConfig(DomainConfigBase):
173 "generate_directives": {"A": generate_directives},
174 },
175 )
176- finally:
177- if needs_freeze_thaw:
178- bind_thaw_zone(zone=self.domain)
179
180
181 class DNSReverseZoneConfig(DomainConfigBase):
182@@ -634,23 +625,25 @@ class DNSReverseZoneConfig(DomainConfigBase):
183 else:
184 Path(f"{zi.target_path}.jnl").unlink(missing_ok=True)
185 self.requires_reload = True
186- self.write_zone_file(
187- zi.target_path,
188- self.make_parameters(),
189- {
190- "mappings": {
191- "PTR": self.get_PTR_mapping(
192- self._mapping, zi.subnetwork
193- )
194- },
195- "other_mapping": [],
196- "generate_directives": {
197- "PTR": generate_directives,
198- "CNAME": self.get_rfc2317_GENERATE_directives(
199- zi.subnetwork,
200- self._rfc2317_ranges,
201- self.domain,
202- ),
203+ needs_freeze_thaw = self.zone_file_exists(zi)
204+ with freeze_thaw_zone(needs_freeze_thaw, zone=zi.zone_name):
205+ self.write_zone_file(
206+ zi.target_path,
207+ self.make_parameters(),
208+ {
209+ "mappings": {
210+ "PTR": self.get_PTR_mapping(
211+ self._mapping, zi.subnetwork
212+ )
213+ },
214+ "other_mapping": [],
215+ "generate_directives": {
216+ "PTR": generate_directives,
217+ "CNAME": self.get_rfc2317_GENERATE_directives(
218+ zi.subnetwork,
219+ self._rfc2317_ranges,
220+ self.domain,
221+ ),
222+ },
223 },
224- },
225- )
226+ )

Subscribers

People subscribed via source and target branches