Merge ~cgrabowski/maas:backport_glue_zone_fix into maas:3.3

Proposed by Christian Grabowski
Status: Merged
Approved by: Christian Grabowski
Approved revision: 7aa2fa996039e7f286717acfeeb107129205c18a
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~cgrabowski/maas:backport_glue_zone_fix
Merge into: maas:3.3
Diff against target: 320 lines (+150/-38)
7 files modified
src/maasserver/dns/tests/test_zonegenerator.py (+10/-1)
src/maasserver/dns/zonegenerator.py (+8/-3)
src/provisioningserver/dns/actions.py (+42/-0)
src/provisioningserver/dns/config.py (+14/-14)
src/provisioningserver/dns/tests/test_actions.py (+26/-3)
src/provisioningserver/dns/tests/test_config.py (+22/-0)
src/provisioningserver/dns/zoneconfig.py (+28/-17)
Reviewer Review Type Date Requested Status
MAAS Lander Approve
Christian Grabowski Approve
Review via email: mp+436080@code.launchpad.net

Commit message

use rndc freeze and thaw to allow BIND to handle receiving full reloads and dynamic updates more rapidly
ensure updates are only for the correct subnet
(cherry picked from commit 0eb397ff84eb81f38a4a3dedae81855de55da32e)

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 :
Revision history for this message
MAAS Lander (maas-lander) wrote :

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

STATUS: SUCCESS
COMMIT: 7aa2fa996039e7f286717acfeeb107129205c18a

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/dns/tests/test_zonegenerator.py b/src/maasserver/dns/tests/test_zonegenerator.py
2index 598074f..ee649a4 100644
3--- a/src/maasserver/dns/tests/test_zonegenerator.py
4+++ b/src/maasserver/dns/tests/test_zonegenerator.py
5@@ -514,7 +514,9 @@ class TestZoneGenerator(MAASServerTestCase):
6 def test_glue_receives_correct_dynamic_updates(self):
7 domain = factory.make_Domain()
8 subnet = factory.make_Subnet(cidr=str(IPNetwork("10/29").cidr))
9+ other_subnet = factory.make_Subnet()
10 sip = factory.make_StaticIPAddress(subnet=subnet)
11+ other_sip = factory.make_StaticIPAddress(subnet=other_subnet)
12 factory.make_Node_with_Interface_on_Subnet(
13 subnet=subnet, vlan=subnet.vlan, fabric=subnet.vlan.fabric
14 )
15@@ -528,7 +530,14 @@ class TestZoneGenerator(MAASServerTestCase):
16 zone=domain.name,
17 rectype="A",
18 answer=sip.ip,
19- )
20+ ),
21+ DynamicDNSUpdate(
22+ operation="INSERT",
23+ name=update_rec.name,
24+ zone=domain.name,
25+ rectype="A",
26+ answer=other_sip.ip,
27+ ),
28 ]
29 zones = ZoneGenerator(
30 domain,
31diff --git a/src/maasserver/dns/zonegenerator.py b/src/maasserver/dns/zonegenerator.py
32index 658147f..2a87990 100644
33--- a/src/maasserver/dns/zonegenerator.py
34+++ b/src/maasserver/dns/zonegenerator.py
35@@ -458,7 +458,7 @@ class ZoneGenerator:
36 for update in dynamic_updates
37 if update.answer
38 and update.answer_is_ip
39- and (IPAddress(update.answer) in IPNetwork(subnet.cidr))
40+ and (update.answer_as_ip in IPNetwork(subnet.cidr))
41 ]
42
43 yield DNSReverseZoneConfig(
44@@ -490,11 +490,16 @@ class ZoneGenerator:
45 if (
46 update.answer
47 and update.answer_is_ip
48- and IPAddress(update.answer) in exclude_net
49+ and update.answer_as_ip in exclude_net
50 ):
51 glue_update = False
52 break
53- if glue_update:
54+ if (
55+ glue_update
56+ and update.answer
57+ and update.answer_is_ip
58+ and update.answer_as_ip in network
59+ ):
60 domain_updates.append(
61 DynamicDNSUpdate.as_reverse_record_update(
62 update, str(network)
63diff --git a/src/provisioningserver/dns/actions.py b/src/provisioningserver/dns/actions.py
64index bafbac9..7749ff5 100644
65--- a/src/provisioningserver/dns/actions.py
66+++ b/src/provisioningserver/dns/actions.py
67@@ -42,6 +42,48 @@ def bind_reconfigure():
68 raise
69
70
71+def bind_freeze_zone(zone=None, timeout=2):
72+ cmd = ("freeze",) # freeze all zones
73+ if zone:
74+ cmd = ("freeze", zone) # freeze one zone
75+
76+ try:
77+ execute_rndc_command(cmd, timeout=timeout)
78+ except CalledProcessError as e:
79+ maaslog.error(
80+ f"Freezing {zone if zone else 'all zones'} for update failed"
81+ )
82+ ExternalProcessError.upgrade(e)
83+ raise
84+ except TimeoutExpired as e:
85+ maaslog.error(
86+ f"Freezing {zone if zone else 'all zones'} for update timed out"
87+ )
88+ ExternalProcessError.upgrade(e)
89+ raise
90+
91+
92+def bind_thaw_zone(zone=None, timeout=2):
93+ cmd = ("thaw",) # thaw all zones
94+ if zone:
95+ cmd = ("thaw", zone) # thaw one zone
96+
97+ try:
98+ execute_rndc_command(cmd, timeout=timeout)
99+ except CalledProcessError as e:
100+ maaslog.error(
101+ f"Thawing {zone if zone else 'all zones'} for update failed"
102+ )
103+ ExternalProcessError.upgrade(e)
104+ raise
105+ except TimeoutExpired as e:
106+ maaslog.error(
107+ f"Thawing {zone if zone else 'all zones'} for update timed out"
108+ )
109+ ExternalProcessError.upgrade(e)
110+ raise
111+
112+
113 def bind_reload(timeout=2):
114 """Ask BIND to reload its configuration and all zone files. This operation
115 is 'best effort' (with logging) as the server may not be running, and there
116diff --git a/src/provisioningserver/dns/config.py b/src/provisioningserver/dns/config.py
117index ef5a166..729b570 100644
118--- a/src/provisioningserver/dns/config.py
119+++ b/src/provisioningserver/dns/config.py
120@@ -9,6 +9,7 @@ from contextlib import contextmanager
121 from dataclasses import dataclass
122 from datetime import datetime
123 import errno
124+from functools import cached_property
125 import grp
126 import os
127 import os.path
128@@ -48,25 +49,17 @@ class DynamicDNSUpdate:
129 answer: Optional[str] = None
130
131 @classmethod
132- def _is_ip(cls, answer):
133- if not answer:
134- return False
135- try:
136- IPAddress(answer)
137- except AddrFormatError:
138- return False
139- else:
140- return True
141-
142- @classmethod
143 def create_from_trigger(cls, **kwargs):
144 answer = kwargs.get("answer")
145 rectype = kwargs.pop("rectype")
146 if answer:
147 del kwargs["answer"]
148 # the DB trigger is unable to figure out if an IP is v6, so we do it here instead
149- if cls._is_ip(answer):
150+ try:
151 ip = IPAddress(answer)
152+ except AddrFormatError:
153+ pass
154+ else:
155 if ip.version == 6:
156 rectype = "AAAA"
157 return cls(answer=answer, rectype=rectype, **kwargs)
158@@ -86,9 +79,16 @@ class DynamicDNSUpdate:
159 rectype="PTR",
160 )
161
162- @property
163+ @cached_property
164+ def answer_as_ip(self):
165+ try:
166+ return IPAddress(self.answer)
167+ except AddrFormatError:
168+ return None
169+
170+ @cached_property
171 def answer_is_ip(self):
172- return DynamicDNSUpdate._is_ip(self.answer)
173+ return self.answer_as_ip is not None
174
175
176 def get_dns_config_dir():
177diff --git a/src/provisioningserver/dns/tests/test_actions.py b/src/provisioningserver/dns/tests/test_actions.py
178index 1087bc0..7920448 100644
179--- a/src/provisioningserver/dns/tests/test_actions.py
180+++ b/src/provisioningserver/dns/tests/test_actions.py
181@@ -68,15 +68,38 @@ class TestReconfigure(MAASTestCase):
182 self.assertRaises(ExternalProcessError, actions.bind_reconfigure)
183
184
185+class TestFreezeZone(MAASTestCase):
186+ """Tests for :py:func:`actions.bind_freeze_zone`."""
187+
188+ def test_executes_rndc_command(self):
189+ self.patch_autospec(actions, "execute_rndc_command")
190+ zone = factory.make_name()
191+ actions.bind_freeze_zone(zone=zone)
192+ actions.execute_rndc_command.assert_called_once_with(
193+ ("freeze", zone), timeout=2
194+ )
195+
196+
197+class TestThawZone(MAASTestCase):
198+ """Tests for :py:func:`actions.bind_freeze_zone`."""
199+
200+ def test_executes_rndc_command(self):
201+ self.patch_autospec(actions, "execute_rndc_command")
202+ zone = factory.make_name()
203+ actions.bind_thaw_zone(zone=zone)
204+ actions.execute_rndc_command.assert_called_once_with(
205+ ("thaw", zone), timeout=2
206+ )
207+
208+
209 class TestReload(MAASTestCase):
210 """Tests for :py:func:`actions.bind_reload`."""
211
212 def test_executes_rndc_command(self):
213 self.patch_autospec(actions, "execute_rndc_command")
214 actions.bind_reload()
215- self.assertThat(
216- actions.execute_rndc_command,
217- MockCalledOnceWith(("reload",), timeout=2),
218+ actions.execute_rndc_command.assert_called_once_with(
219+ ("reload",), timeout=2
220 )
221
222 def test_logs_subprocess_error(self):
223diff --git a/src/provisioningserver/dns/tests/test_config.py b/src/provisioningserver/dns/tests/test_config.py
224index 60b4c58..41a1bc5 100644
225--- a/src/provisioningserver/dns/tests/test_config.py
226+++ b/src/provisioningserver/dns/tests/test_config.py
227@@ -666,6 +666,28 @@ class TestDynamicDNSUpdate(MAASTestCase):
228 )
229 self.assertEqual(update.rectype, "AAAA")
230
231+ def test_answer_as_ip_returns_ip_when_answer_is_an_ip(self):
232+ domain = factory.make_name()
233+ update = DynamicDNSUpdate(
234+ operation="INSERT",
235+ zone=domain,
236+ name=f"{factory.make_name()}.{domain}",
237+ rectype="A",
238+ answer=factory.make_ip_address(),
239+ )
240+ self.assertIsNotNone(update.answer_as_ip)
241+
242+ def test_answer_as_ip_returns_none_when_answer_is_not_an_ip(self):
243+ domain = factory.make_name()
244+ update = DynamicDNSUpdate(
245+ operation="INSERT",
246+ zone=domain,
247+ name=f"{factory.make_name()}.{domain}",
248+ rectype="CNAME",
249+ answer=factory.make_name(),
250+ )
251+ self.assertIsNone(update.answer_as_ip)
252+
253 def test_answer_is_ip_returns_true_when_answer_is_an_ip(self):
254 domain = factory.make_name()
255 update = DynamicDNSUpdate(
256diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
257index 8dec9b6..59e714b 100644
258--- a/src/provisioningserver/dns/zoneconfig.py
259+++ b/src/provisioningserver/dns/zoneconfig.py
260@@ -12,7 +12,11 @@ from pathlib import Path
261 from netaddr import IPAddress, IPNetwork, spanning_cidr
262 from netaddr.core import AddrFormatError
263
264-from provisioningserver.dns.actions import NSUpdateCommand
265+from provisioningserver.dns.actions import (
266+ bind_freeze_zone,
267+ bind_thaw_zone,
268+ NSUpdateCommand,
269+)
270 from provisioningserver.dns.config import (
271 compose_zone_file_config_path,
272 render_dns_template,
273@@ -333,24 +337,31 @@ class DNSForwardZoneConfig(DomainConfigBase):
274 else:
275 Path(f"{zi.target_path}.jnl").unlink(missing_ok=True)
276 self.requires_reload = True
277- self.write_zone_file(
278- zi.target_path,
279- self.make_parameters(),
280- {
281- "mappings": {
282- "A": self.get_A_mapping(
283- self._mapping, self._ipv4_ttl
284- ),
285- "AAAA": self.get_AAAA_mapping(
286- self._mapping, self._ipv6_ttl
287+ needs_freeze_thaw = self.zone_file_exists(zi)
288+ if needs_freeze_thaw:
289+ bind_freeze_zone(zone=self.domain)
290+ try:
291+ self.write_zone_file(
292+ zi.target_path,
293+ self.make_parameters(),
294+ {
295+ "mappings": {
296+ "A": self.get_A_mapping(
297+ self._mapping, self._ipv4_ttl
298+ ),
299+ "AAAA": self.get_AAAA_mapping(
300+ self._mapping, self._ipv6_ttl
301+ ),
302+ },
303+ "other_mapping": enumerate_rrset_mapping(
304+ self._other_mapping
305 ),
306+ "generate_directives": {"A": generate_directives},
307 },
308- "other_mapping": enumerate_rrset_mapping(
309- self._other_mapping
310- ),
311- "generate_directives": {"A": generate_directives},
312- },
313- )
314+ )
315+ finally:
316+ if needs_freeze_thaw:
317+ bind_thaw_zone(zone=self.domain)
318
319
320 class DNSReverseZoneConfig(DomainConfigBase):

Subscribers

People subscribed via source and target branches