sts-sponsors team mailing list archive

[Adam Collard <mp+436300@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/src/provisioningserver/dns/tests/test_zoneconfig.py b/src/provisioningserver/dns/tests/test_zoneconfig.py
> index 10ee0f7..1db577e 100644
> --- a/src/provisioningserver/dns/tests/test_zoneconfig.py
> +++ b/src/provisioningserver/dns/tests/test_zoneconfig.py
> @@ -24,6 +25,7 @@ from maastesting.factory import factory
>  from maastesting.matchers import MockNotCalled
>  from maastesting.testcase import MAASTestCase
>  from provisioningserver.dns import actions
> +import provisioningserver.dns.actions

unnecessary import duplicating the one above

>  from provisioningserver.dns.config import (
>      DynamicDNSUpdate,
>      get_nsupdate_key_path,
> @@ -488,6 +490,44 @@ 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(
> +            provisioningserver.dns.actions, "execute_rndc_command"

s/provisioningserver.dns.actions/actions/

> +        )
> +        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")
> +        run_command = 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."""
> diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py
> index 3171bae..bc02651 100644
> --- a/src/provisioningserver/dns/zoneconfig.py
> +++ b/src/provisioningserver/dns/zoneconfig.py
> @@ -650,26 +650,33 @@ 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)
> +                if needs_freeze_thaw:
> +                    bind_freeze_zone(zone=zi.zone_name)
> +                try:

this is ok as is, but could also be written as a context manager, using a nullcontext if we don't need to freeze/thaw

https://docs.python.org/3/library/contextlib.html#contextlib.nullcontext

> +                    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,
> +                                ),
> +                            },
>                          },
> -                    },
> -                )
> +                    )
> +                finally:
> +                    if needs_freeze_thaw:
> +                        bind_thaw_zone(zone=zi.zone_name)
>                  PROMETHEUS_METRICS.update(
>                      "maas_dns_full_zonefile_write_count",
>                      "inc",


-- 
https://code.launchpad.net/~cgrabowski/maas/+git/maas/+merge/436300
Your team MAAS Committers is subscribed to branch maas:master.