sts-sponsors team mailing list archive
-
sts-sponsors team
-
Mailing list archive
-
Message #04691
[Merge] ~cgrabowski/maas:backport_glue_zone_fix into maas:3.3
Christian Grabowski has proposed merging ~cgrabowski/maas:backport_glue_zone_fix into maas:3.3.
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)
Requested reviews:
Christian Grabowski (cgrabowski)
For more details, see:
https://code.launchpad.net/~cgrabowski/maas/+git/maas/+merge/436080
--
Your team MAAS Committers is subscribed to branch maas:3.3.
diff --git a/src/maasserver/dns/tests/test_zonegenerator.py b/src/maasserver/dns/tests/test_zonegenerator.py
index 598074f..ee649a4 100644
--- a/src/maasserver/dns/tests/test_zonegenerator.py
+++ b/src/maasserver/dns/tests/test_zonegenerator.py
@@ -514,7 +514,9 @@ class TestZoneGenerator(MAASServerTestCase):
def test_glue_receives_correct_dynamic_updates(self):
domain = factory.make_Domain()
subnet = factory.make_Subnet(cidr=str(IPNetwork("10/29").cidr))
+ other_subnet = factory.make_Subnet()
sip = factory.make_StaticIPAddress(subnet=subnet)
+ other_sip = factory.make_StaticIPAddress(subnet=other_subnet)
factory.make_Node_with_Interface_on_Subnet(
subnet=subnet, vlan=subnet.vlan, fabric=subnet.vlan.fabric
)
@@ -528,7 +530,14 @@ class TestZoneGenerator(MAASServerTestCase):
zone=domain.name,
rectype="A",
answer=sip.ip,
- )
+ ),
+ DynamicDNSUpdate(
+ operation="INSERT",
+ name=update_rec.name,
+ zone=domain.name,
+ rectype="A",
+ answer=other_sip.ip,
+ ),
]
zones = ZoneGenerator(
domain,
diff --git a/src/maasserver/dns/zonegenerator.py b/src/maasserver/dns/zonegenerator.py
index 658147f..2a87990 100644
--- a/src/maasserver/dns/zonegenerator.py
+++ b/src/maasserver/dns/zonegenerator.py
@@ -458,7 +458,7 @@ class ZoneGenerator:
for update in dynamic_updates
if update.answer
and update.answer_is_ip
- and (IPAddress(update.answer) in IPNetwork(subnet.cidr))
+ and (update.answer_as_ip in IPNetwork(subnet.cidr))
]
yield DNSReverseZoneConfig(
@@ -490,11 +490,16 @@ class ZoneGenerator:
if (
update.answer
and update.answer_is_ip
- and IPAddress(update.answer) in exclude_net
+ and update.answer_as_ip in exclude_net
):
glue_update = False
break
- if glue_update:
+ if (
+ glue_update
+ and update.answer
+ and update.answer_is_ip
+ and update.answer_as_ip in network
+ ):
domain_updates.append(
DynamicDNSUpdate.as_reverse_record_update(
update, str(network)
diff --git a/src/provisioningserver/dns/actions.py b/src/provisioningserver/dns/actions.py
index bafbac9..7749ff5 100644
--- a/src/provisioningserver/dns/actions.py
+++ b/src/provisioningserver/dns/actions.py
@@ -42,6 +42,48 @@ def bind_reconfigure():
raise
+def bind_freeze_zone(zone=None, timeout=2):
+ cmd = ("freeze",) # freeze all zones
+ if zone:
+ cmd = ("freeze", zone) # freeze one zone
+
+ try:
+ execute_rndc_command(cmd, timeout=timeout)
+ except CalledProcessError as e:
+ maaslog.error(
+ f"Freezing {zone if zone else 'all zones'} for update failed"
+ )
+ ExternalProcessError.upgrade(e)
+ raise
+ except TimeoutExpired as e:
+ maaslog.error(
+ f"Freezing {zone if zone else 'all zones'} for update timed out"
+ )
+ ExternalProcessError.upgrade(e)
+ raise
+
+
+def bind_thaw_zone(zone=None, timeout=2):
+ cmd = ("thaw",) # thaw all zones
+ if zone:
+ cmd = ("thaw", zone) # thaw one zone
+
+ try:
+ execute_rndc_command(cmd, timeout=timeout)
+ except CalledProcessError as e:
+ maaslog.error(
+ f"Thawing {zone if zone else 'all zones'} for update failed"
+ )
+ ExternalProcessError.upgrade(e)
+ raise
+ except TimeoutExpired as e:
+ maaslog.error(
+ f"Thawing {zone if zone else 'all zones'} for update timed out"
+ )
+ ExternalProcessError.upgrade(e)
+ raise
+
+
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 ef5a166..729b570 100644
--- a/src/provisioningserver/dns/config.py
+++ b/src/provisioningserver/dns/config.py
@@ -9,6 +9,7 @@ from contextlib import contextmanager
from dataclasses import dataclass
from datetime import datetime
import errno
+from functools import cached_property
import grp
import os
import os.path
@@ -48,25 +49,17 @@ class DynamicDNSUpdate:
answer: Optional[str] = None
@classmethod
- def _is_ip(cls, answer):
- if not answer:
- return False
- try:
- IPAddress(answer)
- except AddrFormatError:
- return False
- else:
- return True
-
- @classmethod
def create_from_trigger(cls, **kwargs):
answer = kwargs.get("answer")
rectype = kwargs.pop("rectype")
if answer:
del kwargs["answer"]
# the DB trigger is unable to figure out if an IP is v6, so we do it here instead
- if cls._is_ip(answer):
+ try:
ip = IPAddress(answer)
+ except AddrFormatError:
+ pass
+ else:
if ip.version == 6:
rectype = "AAAA"
return cls(answer=answer, rectype=rectype, **kwargs)
@@ -86,9 +79,16 @@ class DynamicDNSUpdate:
rectype="PTR",
)
- @property
+ @cached_property
+ def answer_as_ip(self):
+ try:
+ return IPAddress(self.answer)
+ except AddrFormatError:
+ return None
+
+ @cached_property
def answer_is_ip(self):
- return DynamicDNSUpdate._is_ip(self.answer)
+ return self.answer_as_ip is not None
def get_dns_config_dir():
diff --git a/src/provisioningserver/dns/tests/test_actions.py b/src/provisioningserver/dns/tests/test_actions.py
index 1087bc0..7920448 100644
--- a/src/provisioningserver/dns/tests/test_actions.py
+++ b/src/provisioningserver/dns/tests/test_actions.py
@@ -68,15 +68,38 @@ class TestReconfigure(MAASTestCase):
self.assertRaises(ExternalProcessError, actions.bind_reconfigure)
+class TestFreezeZone(MAASTestCase):
+ """Tests for :py:func:`actions.bind_freeze_zone`."""
+
+ def test_executes_rndc_command(self):
+ self.patch_autospec(actions, "execute_rndc_command")
+ zone = factory.make_name()
+ actions.bind_freeze_zone(zone=zone)
+ actions.execute_rndc_command.assert_called_once_with(
+ ("freeze", zone), timeout=2
+ )
+
+
+class TestThawZone(MAASTestCase):
+ """Tests for :py:func:`actions.bind_freeze_zone`."""
+
+ def test_executes_rndc_command(self):
+ self.patch_autospec(actions, "execute_rndc_command")
+ zone = factory.make_name()
+ actions.bind_thaw_zone(zone=zone)
+ actions.execute_rndc_command.assert_called_once_with(
+ ("thaw", zone), timeout=2
+ )
+
+
class TestReload(MAASTestCase):
"""Tests for :py:func:`actions.bind_reload`."""
def test_executes_rndc_command(self):
self.patch_autospec(actions, "execute_rndc_command")
actions.bind_reload()
- self.assertThat(
- actions.execute_rndc_command,
- MockCalledOnceWith(("reload",), timeout=2),
+ actions.execute_rndc_command.assert_called_once_with(
+ ("reload",), timeout=2
)
def test_logs_subprocess_error(self):
diff --git a/src/provisioningserver/dns/tests/test_config.py b/src/provisioningserver/dns/tests/test_config.py
index 60b4c58..41a1bc5 100644
--- a/src/provisioningserver/dns/tests/test_config.py
+++ b/src/provisioningserver/dns/tests/test_config.py
@@ -666,6 +666,28 @@ class TestDynamicDNSUpdate(MAASTestCase):
)
self.assertEqual(update.rectype, "AAAA")
+ def test_answer_as_ip_returns_ip_when_answer_is_an_ip(self):
+ domain = factory.make_name()
+ update = DynamicDNSUpdate(
+ operation="INSERT",
+ zone=domain,
+ name=f"{factory.make_name()}.{domain}",
+ rectype="A",
+ answer=factory.make_ip_address(),
+ )
+ self.assertIsNotNone(update.answer_as_ip)
+
+ def test_answer_as_ip_returns_none_when_answer_is_not_an_ip(self):
+ domain = factory.make_name()
+ update = DynamicDNSUpdate(
+ operation="INSERT",
+ zone=domain,
+ name=f"{factory.make_name()}.{domain}",
+ rectype="CNAME",
+ answer=factory.make_name(),
+ )
+ self.assertIsNone(update.answer_as_ip)
+
def test_answer_is_ip_returns_true_when_answer_is_an_ip(self):
domain = factory.make_name()
update = DynamicDNSUpdate(
diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
index 8dec9b6..59e714b 100644
--- a/src/provisioningserver/dns/zoneconfig.py
+++ b/src/provisioningserver/dns/zoneconfig.py
@@ -12,7 +12,11 @@ from pathlib import Path
from netaddr import IPAddress, IPNetwork, spanning_cidr
from netaddr.core import AddrFormatError
-from provisioningserver.dns.actions import NSUpdateCommand
+from provisioningserver.dns.actions import (
+ bind_freeze_zone,
+ bind_thaw_zone,
+ NSUpdateCommand,
+)
from provisioningserver.dns.config import (
compose_zone_file_config_path,
render_dns_template,
@@ -333,24 +337,31 @@ class DNSForwardZoneConfig(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": {
- "A": self.get_A_mapping(
- self._mapping, self._ipv4_ttl
- ),
- "AAAA": self.get_AAAA_mapping(
- self._mapping, self._ipv6_ttl
+ needs_freeze_thaw = self.zone_file_exists(zi)
+ if needs_freeze_thaw:
+ bind_freeze_zone(zone=self.domain)
+ try:
+ self.write_zone_file(
+ zi.target_path,
+ self.make_parameters(),
+ {
+ "mappings": {
+ "A": self.get_A_mapping(
+ self._mapping, self._ipv4_ttl
+ ),
+ "AAAA": self.get_AAAA_mapping(
+ self._mapping, self._ipv6_ttl
+ ),
+ },
+ "other_mapping": enumerate_rrset_mapping(
+ self._other_mapping
),
+ "generate_directives": {"A": generate_directives},
},
- "other_mapping": enumerate_rrset_mapping(
- self._other_mapping
- ),
- "generate_directives": {"A": generate_directives},
- },
- )
+ )
+ finally:
+ if needs_freeze_thaw:
+ bind_thaw_zone(zone=self.domain)
class DNSReverseZoneConfig(DomainConfigBase):
References
