Merge lp:~lamont/maas/dns-2.0 into lp:~maas-committers/maas/trunk

[LaMont Jones]

Correct the management of reverse DNS zones in MAAS. (a /22 means that we create 4 /24 reverse zones, not one /16 reverse zone.)

Correct the management of reverse DNS zones in MAAS. (a /22 means that we create 4 /24 reverse zones, not one /16 reverse zone.)

[LaMont Jones (lamont)]

general overview: DNSZoneConfigBase.{zone_name,target_path} get replaced with zone_info: a list of 1-128 DNSZoneInfo elements (which contain the subnet and reverse zone name, with target_path being derived as needed.)

The zones that we are authoritative for are limited to only the reverse zones for the subnets we
own.

[Mike Pontillo (mpontillo)]

Nice work, LaMont.

I commented on a few minor issues below; in my mind, the only thing keeping this branch from landing is the 'assert', which we should gracefully log and skip instead, rather than crashing regiond.

See comments below.

review: Needs Fixing

[Mike Pontillo (mpontillo)]

Looks good. Thanks for the fixes!

review: Approve

[Andres Rodriguez (andreserl)]

Hi Guys,

Just keep in mind that fixing this means that the behavior of DNS completely changes, and as such, it is something that's not back portable.

[LaMont Jones (lamont)]

> Hi Guys,
>
> Just keep in mind that fixing this means that the behavior of DNS completely
> changes, and as such, it is something that's not back portable.

Acknowledged.

[lp:~lamont/maas/dns-2.0]

4528. By LaMont Jones on 2015-11-30

[r=mpontillo][bug=1356012][author=lamont] Correct the management of reverse DNS zones in MAAS. (a /22 means that we create 4 /24 reverse zones, not one /16 reverse zone.)

=== modified file 'etc/maas/templates/dns/named.conf.template'
--- etc/maas/templates/dns/named.conf.template	2014-11-14 21:50:42 +0000
+++ etc/maas/templates/dns/named.conf.template	2015-11-28 05:29:35 +0000
@@ -2,11 +2,13 @@
 
 # Zone declarations.
 {{for zone in zones}}
-zone "{{zone.zone_name}}" {
+{{for zoneinfo in zone.zone_info}}
+zone "{{zoneinfo.zone_name}}" {
     type master;
-    file "{{zone.target_path}}";
+    file "{{zoneinfo.target_path}}";
 };
 {{endfor}}
+{{endfor}}
 
 # Access control for recursive queries.  See named.conf.options.inside.maas
 # for the directives used on this ACL.

=== modified file 'src/maasserver/dns/config.py'
--- src/maasserver/dns/config.py	2015-09-24 16:22:12 +0000
+++ src/maasserver/dns/config.py	2015-11-28 05:29:35 +0000
@@ -50,7 +50,7 @@
     bind_reconfigure,
     bind_reload,
     bind_reload_with_retries,
-    bind_reload_zone,
+    bind_reload_zones,
     bind_write_configuration,
     bind_write_options,
     bind_write_zones,
@@ -144,9 +144,10 @@
 
     serial = next_zone_serial()
     for zone in ZoneGenerator(clusters, serial):
-        maaslog.info("Generating new DNS zone file for %s", zone.zone_name)
+        names = [zi.zone_name for zi in zone.zone_info]
+        maaslog.info("Generating new DNS zone file for %s", " ".join(names))
         bind_write_zones([zone])
-        bind_reload_zone(zone.zone_name)
+        bind_reload_zones(names)
 
 
 def dns_update_zones(clusters):

=== modified file 'src/provisioningserver/dns/actions.py'
--- src/provisioningserver/dns/actions.py	2015-10-22 00:06:43 +0000
+++ src/provisioningserver/dns/actions.py	2015-11-28 05:29:35 +0000
@@ -15,7 +15,7 @@
 __all__ = [
     "bind_reconfigure",
     "bind_reload",
-    "bind_reload_zone",
+    "bind_reload_zones",
     "bind_write_configuration",
     "bind_write_options",
     "bind_write_zones",
@@ -87,21 +87,26 @@
             sleep(interval)
 
 
-def bind_reload_zone(zone_name):
+def bind_reload_zones(zone_list):
     """Ask BIND to reload the zone file for the given zone.
 
-    :param zone_name: The name of the zone to reload.
+    :param zone_list: A list of zone names to reload, or a single name as a
+        string.
     :return: True if success, False otherwise.
     """
-    try:
-        execute_rndc_command(("reload", zone_name))
-        return True
-    except CalledProcessError as exc:
-        maaslog.error(
-            "Reloading BIND zone %r failed (is it running?): %s",
-            zone_name,
-            exc)
-        return False
+    ret = True
+    if not isinstance(zone_list, list):
+        zone_list = [zone_list]
+    for name in zone_list:
+        try:
+            execute_rndc_command(("reload", name))
+        except CalledProcessError as exc:
+            maaslog.error(
+                "Reloading BIND zone %r failed (is it running?): %s",
+                name,
+                exc)
+            ret = False
+    return ret
 
 
 def bind_write_configuration(zones, trusted_networks):

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2015-07-09 22:25:27 +0000
+++ src/provisioningserver/dns/config.py	2015-11-28 05:29:35 +0000
@@ -29,6 +29,7 @@
 import re
 import sys
 
+from provisioningserver.logger import get_maas_logger
 from provisioningserver.utils import locate_config
 from provisioningserver.utils.fs import atomic_write
 from provisioningserver.utils.isc import read_isc_file
@@ -36,6 +37,7 @@
 import tempita
 
 
+maaslog = get_maas_logger("dns")
 NAMED_CONF_OPTIONS = 'named.conf.options'
 MAAS_NAMED_CONF_NAME = 'named.conf.maas'
 MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME = 'named.conf.options.inside.maas'
@@ -319,7 +321,16 @@
 
     @classmethod
     def get_include_snippet(cls):
-        target_path = compose_config_path(cls.target_file_name)
-        assert '"' not in target_path, (
-            "DNS config path contains quote: %s." % target_path)
-        return 'include "%s";\n' % target_path
+        snippet = ""
+        if isinstance(cls.target_file_name, list):
+            target_file_names = cls.target_file_name
+        else:
+            target_file_names = [cls.target_file_name]
+        for target_file_name in target_file_names:
+            target_path = compose_config_path(target_file_name)
+            if '"' in target_path:
+                maaslog.error(
+                    "DNS config path contains quote: %s." % target_path)
+            else:
+                snippet += 'include "%s";\n' % target_path
+        return snippet

=== modified file 'src/provisioningserver/dns/tests/test_actions.py'
--- src/provisioningserver/dns/tests/test_actions.py	2015-10-22 00:06:43 +0000
+++ src/provisioningserver/dns/tests/test_actions.py	2015-11-28 05:29:35 +0000
@@ -144,11 +144,11 @@
 
 
 class TestReloadZone(MAASTestCase):
-    """Tests for :py:func:`actions.bind_reload_zone`."""
+    """Tests for :py:func:`actions.bind_reload_zones`."""
 
     def test__executes_rndc_command(self):
         self.patch_autospec(actions, "execute_rndc_command")
-        self.assertTrue(actions.bind_reload_zone(sentinel.zone))
+        self.assertTrue(actions.bind_reload_zones(sentinel.zone))
         self.assertThat(
             actions.execute_rndc_command,
             MockCalledOnceWith(("reload", sentinel.zone)))
@@ -157,7 +157,7 @@
         erc = self.patch_autospec(actions, "execute_rndc_command")
         erc.side_effect = factory.make_CalledProcessError()
         with FakeLogger("maas") as logger:
-            self.assertFalse(actions.bind_reload_zone(sentinel.zone))
+            self.assertFalse(actions.bind_reload_zones(sentinel.zone))
         self.assertDocTestMatches(
             "Reloading BIND zone ... failed (is it running?): "
             "Command ... returned non-zero exit status ...",
@@ -166,7 +166,7 @@
     def test__false_on_subprocess_error(self):
         erc = self.patch_autospec(actions, "execute_rndc_command")
         erc.side_effect = factory.make_CalledProcessError()
-        self.assertFalse(actions.bind_reload_zone(sentinel.zone))
+        self.assertFalse(actions.bind_reload_zones(sentinel.zone))
 
 
 class TestConfiguration(PservTestCase):

=== modified file 'src/provisioningserver/dns/tests/test_zoneconfig.py'
--- src/provisioningserver/dns/tests/test_zoneconfig.py	2015-05-07 18:14:38 +0000
+++ src/provisioningserver/dns/tests/test_zoneconfig.py	2015-11-28 05:29:35 +0000
@@ -37,6 +37,7 @@
 from provisioningserver.dns.zoneconfig import (
     DNSForwardZoneConfig,
     DNSReverseZoneConfig,
+    DNSZoneInfo,
 )
 from testtools.matchers import (
     Contains,
@@ -102,7 +103,7 @@
         dns_zone_config = DNSForwardZoneConfig(domain)
         self.assertEqual(
             os.path.join(get_dns_config_dir(), 'zone.%s' % domain),
-            dns_zone_config.target_path)
+            dns_zone_config.zone_info[0].target_path)
 
     def test_get_a_mapping_returns_ipv4_mapping(self):
         name = factory.make_string()
@@ -266,7 +267,7 @@
             factory.make_string(), serial=random.randint(1, 100),
             dns_ip=factory.make_ipv4_address())
         dns_zone_config.write_config()
-        filepath = FilePath(dns_zone_config.target_path)
+        filepath = FilePath(dns_zone_config.zone_info[0].target_path)
         self.assertTrue(filepath.getPermissions().other.read)
 
 
@@ -290,34 +291,109 @@
 
     def test_computes_dns_config_file_paths(self):
         domain = factory.make_name('zone')
-        reverse_file_name = 'zone.168.192.in-addr.arpa'
+        reverse_file_name = [
+            'zone.%d.168.192.in-addr.arpa' % i for i in range(4)]
         dns_zone_config = DNSReverseZoneConfig(
             domain, network=IPNetwork("192.168.0.0/22"))
+        for i in range(4):
+            self.assertEqual(
+                os.path.join(get_dns_config_dir(), reverse_file_name[i]),
+                dns_zone_config.zone_info[i].target_path)
+
+    def test_computes_dns_config_file_paths_for_small_network(self):
+        domain = factory.make_name('zone')
+        reverse_file_name = 'zone.192-27.0.168.192.in-addr.arpa'
+        dns_zone_config = DNSReverseZoneConfig(
+            domain, network=IPNetwork("192.168.0.192/27"))
+        self.assertEqual(1, len(dns_zone_config.zone_info))
         self.assertEqual(
             os.path.join(get_dns_config_dir(), reverse_file_name),
-            dns_zone_config.target_path)
+            dns_zone_config.zone_info[0].target_path)
 
     def test_reverse_zone_file(self):
         # DNSReverseZoneConfig calculates the reverse zone file name
         # correctly for IPv4 and IPv6 networks.
+        # As long as the network size ends on a "nice" boundary (multiple of
+        # 8 for IPv4, multiple of 4 for IPv6), then there will be one reverse
+        # zone for the subnet.  When it isn't, then there will be 2-128
+        # individual reverse zones for the subnet.
+        # A special case is the small subnet (less than 256 hosts for IPv4,
+        # less than 16 hosts for IPv6), in which case, we follow RFC2317 with
+        # the modern adjustment of using '-' instead of '/'.
+        zn = "%d.0.0.0.0.0.0.0.0.0.0.0.4.0.1.f.1.0.8.a.b.0.1.0.0.2.ip6.arpa"
         expected = [
             # IPv4 networks.
-            (IPNetwork('192.168.0.1/22'), '168.192.in-addr.arpa'),
-            (IPNetwork('192.168.0.1/24'), '0.168.192.in-addr.arpa'),
+            # /22 ==> 4 /24 reverse zones
+            (
+                IPNetwork('192.168.0.1/22'),
+                [DNSZoneInfo(
+                    IPNetwork('192.168.%d.0/24' % i),
+                    '%d.168.192.in-addr.arpa' % i) for i in range(4)]
+            ),
+            # /24 ==> 1 reverse zone
+            (
+                IPNetwork('192.168.0.1/24'),
+                [DNSZoneInfo(
+                    IPNetwork('192.168.0.0/24'),
+                    '0.168.192.in-addr.arpa')]
+            ),
+            # /29 ==> 1 reverse zones, per RFC2317
+            (
+                IPNetwork('192.168.0.241/29'),
+                [DNSZoneInfo(
+                    IPNetwork('192.168.0.240/29'),
+                    '240-29.0.168.192.in-addr.arpa')]
+            ),
             # IPv6 networks.
-            (IPNetwork('3ffe:801::/32'), '1.0.8.0.e.f.f.3.ip6.arpa'),
-            (IPNetwork('2001:db8:0::/48'), '0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa'),
+            # /32, 48, 56, 64 ==> 1 reverse zones
+            (
+                IPNetwork('3ffe:801::/32'),
+                [DNSZoneInfo(
+                    IPNetwork('3ffe:801::32'),
+                    '1.0.8.0.e.f.f.3.ip6.arpa')]),
+            (
+                IPNetwork('2001:db8:0::/48'),
+                [DNSZoneInfo(
+                    IPNetwork('2001:db8:0::/48'),
+                    '0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa')]
+            ),
             (
                 IPNetwork('2001:ba8:1f1:400::/56'),
-                '4.0.1.f.1.0.8.a.b.0.1.0.0.2.ip6.arpa'
+                [DNSZoneInfo(
+                    IPNetwork('2001:ba8:1f1:400::/56'),
+                    '4.0.1.f.1.0.8.a.b.0.1.0.0.2.ip6.arpa')],
             ),
             (
                 IPNetwork('2610:8:6800:1::/64'),
-                '1.0.0.0.0.0.8.6.8.0.0.0.0.1.6.2.ip6.arpa',
-            ),
+                [DNSZoneInfo(
+                    IPNetwork('2610:8:6800:1::/64'),
+                    '1.0.0.0.0.0.8.6.8.0.0.0.0.1.6.2.ip6.arpa')],
+            ),
+            # /2 with hex digits ==> 4 /4 reverse zones
+            (
+                IPNetwork('8000::/2'),
+                [
+                    DNSZoneInfo(IPNetwork('8000::/4'), '8.ip6.arpa'),
+                    DNSZoneInfo(IPNetwork('9000::/4'), '9.ip6.arpa'),
+                    DNSZoneInfo(IPNetwork('a000::/4'), 'a.ip6.arpa'),
+                    DNSZoneInfo(IPNetwork('b000::/4'), 'b.ip6.arpa'),
+                ],
+            ),
+            # /103 ==> 2 /104 reverse zones
             (
                 IPNetwork('2001:ba8:1f1:400::/103'),
-                '0.0.0.0.0.0.0.0.0.0.0.4.0.1.f.1.0.8.a.b.0.1.0.0.2.ip6.arpa',
+                [DNSZoneInfo(
+                    IPNetwork('2001:ba8:1f1:400:0:0:%d00:0000/104' % i),
+                    zn % i) for i in range(2)],
+            ),
+            # /125 ==> 1 reverse zone, based on RFC2317
+            (
+                IPNetwork('2001:ba8:1f1:400::/125'),
+                [DNSZoneInfo(
+                    IPNetwork('2001:ba8:1f1:400::/125'),
+                    "0-125.%s" % (
+                        IPAddress('2001:ba8:1f1:400::').reverse_dns[2:-1]))],
+
             ),
 
         ]
@@ -325,8 +401,15 @@
         for network, _ in expected:
             domain = factory.make_name('zone')
             dns_zone_config = DNSReverseZoneConfig(domain, network=network)
-            results.append((network, dns_zone_config.zone_name))
-        self.assertEqual(expected, results)
+            results.append((network, dns_zone_config.zone_info))
+        # Make sure we have the right number of elements.
+        self.assertEqual(len(expected), len(results))
+        # And that the zone names chosen for each element are correct.
+        for net in range(len(expected)):
+            for zi in range(len(expected[net][1])):
+                self.assertItemsEqual(
+                    expected[net][1][zi].zone_name,
+                    results[net][1][zi].zone_name)
 
     def test_get_ptr_mapping(self):
         name = factory.make_string()
@@ -378,11 +461,12 @@
             factory.make_string(), serial=random.randint(1, 100),
             network=network)
         dns_zone_config.write_config()
-        self.assertThat(
-            os.path.join(
-                target_dir, 'zone.%s' % dns_zone_config.zone_name),
-            FileContains(
-                matcher=Contains('IN  NS  %s.' % dns_zone_config.domain)))
+        for zone_name in [zi.zone_name for zi in dns_zone_config.zone_info]:
+            self.assertThat(
+                os.path.join(
+                    target_dir, 'zone.%s' % zone_name),
+                FileContains(
+                    matcher=Contains('IN  NS  %s.' % dns_zone_config.domain)))
 
     def test_writes_reverse_dns_zone_config(self):
         target_dir = patch_dns_config_path(self)
@@ -394,9 +478,41 @@
             dynamic_ranges=[
                 IPRange(dynamic_network.first, dynamic_network.last)])
         dns_zone_config.write_config()
-        reverse_file_name = 'zone.168.192.in-addr.arpa'
-        expected_generate_directives = dns_zone_config.get_GENERATE_directives(
-            dynamic_network, domain)
+        for sub in range(4):
+            reverse_file_name = 'zone.%d.168.192.in-addr.arpa' % sub
+            expected_GEN_direct = dns_zone_config.get_GENERATE_directives(
+                dynamic_network, domain,
+                DNSZoneInfo(
+                    IPNetwork('192.168.%d.0/24' % sub),
+                    "%d.168.192.in-addr.arpa" % sub))
+            expected = ContainsAll(
+                [
+                    'IN  NS  %s' % domain
+                ] +
+                [
+                    '$GENERATE %s %s IN PTR %s' % (
+                        iterator_values, reverse_dns, hostname)
+                    for iterator_values, reverse_dns, hostname in
+                    expected_GEN_direct
+                ])
+            self.assertThat(
+                os.path.join(target_dir, reverse_file_name),
+                FileContains(matcher=expected))
+
+    def test_writes_reverse_dns_zone_config_for_small_network(self):
+        target_dir = patch_dns_config_path(self)
+        domain = factory.make_string()
+        network = IPNetwork('192.168.0.1/26')
+        dynamic_network = IPNetwork('192.168.0.1/28')
+        dns_zone_config = DNSReverseZoneConfig(
+            domain, serial=random.randint(1, 100), network=network,
+            dynamic_ranges=[
+                IPRange(dynamic_network.first, dynamic_network.last)])
+        dns_zone_config.write_config()
+        reverse_zone_name = '0-26.0.168.192.in-addr.arpa'
+        reverse_file_name = 'zone.0-26.0.168.192.in-addr.arpa'
+        expected_GEN_direct = dns_zone_config.get_GENERATE_directives(
+            dynamic_network, domain, DNSZoneInfo(network, reverse_zone_name))
         expected = ContainsAll(
             [
                 'IN  NS  %s' % domain
@@ -405,7 +521,7 @@
                 '$GENERATE %s %s IN PTR %s' % (
                     iterator_values, reverse_dns, hostname)
                 for iterator_values, reverse_dns, hostname in
-                expected_generate_directives
+                expected_GEN_direct
             ])
         self.assertThat(
             os.path.join(target_dir, reverse_file_name),
@@ -431,8 +547,9 @@
             factory.make_string(), serial=random.randint(1, 100),
             network=factory.make_ipv4_network())
         dns_zone_config.write_config()
-        filepath = FilePath(dns_zone_config.target_path)
-        self.assertTrue(filepath.getPermissions().other.read)
+        for tgt in [zi.target_path for zi in dns_zone_config.zone_info]:
+            filepath = FilePath(tgt)
+            self.assertTrue(filepath.getPermissions().other.read)
 
 
 class TestDNSReverseZoneConfig_GetGenerateDirectives(MAASTestCase):
@@ -451,7 +568,11 @@
         self.assertItemsEqual(
             expected_directives,
             DNSReverseZoneConfig.get_GENERATE_directives(
-                ip_range, domain="domain"))
+                ip_range,
+                domain="domain",
+                zone_info=DNSZoneInfo(
+                    IPNetwork('192.168.0.0/16'),
+                    "168.192.in-addr.arpa")))
 
     def get_expected_generate_directives(self, network, domain):
         ip_parts = network.network.format().split('.')
@@ -481,8 +602,11 @@
             relevant_ip_parts.reverse()
             expected_rdns_base = (
                 "%s.%s.in-addr.arpa." % tuple(relevant_ip_parts))
-            expected_rdns_template = "$.%s.%s" % (
-                num + second_octet_offset, expected_rdns_base)
+            if network.size >= 256:
+                expected_rdns_template = "$.%s.%s" % (
+                    num + second_octet_offset, expected_rdns_base)
+            else:
+                expected_rdns_template = "$"
             expected_generate_directives.append(
                 (
                     "%s-%s" % (iterator_low, iterator_high),
@@ -494,22 +618,25 @@
 
     def test_returns_single_entry_for_slash_24_network(self):
         network = IPNetwork("%s/24" % factory.make_ipv4_address())
+        reverse = ".".join(IPAddress(network).reverse_dns.split('.')[1:-1])
         domain = factory.make_string()
         expected_generate_directives = self.get_expected_generate_directives(
             network, domain)
         directives = DNSReverseZoneConfig.get_GENERATE_directives(
-            network, domain)
+            network, domain, DNSZoneInfo(network, reverse))
         self.expectThat(directives, HasLength(1))
         self.assertItemsEqual(expected_generate_directives, directives)
 
     def test_returns_single_entry_for_tiny_network(self):
         network = IPNetwork("%s/28" % factory.make_ipv4_address())
+        reverse = IPAddress(network).reverse_dns.split('.')
+        reverse = ".".join(["%s-28" % reverse[0]] + reverse[1:-1])
         domain = factory.make_string()
 
         expected_generate_directives = self.get_expected_generate_directives(
             network, domain)
         directives = DNSReverseZoneConfig.get_GENERATE_directives(
-            network, domain)
+            network, domain, DNSZoneInfo(network, reverse))
         self.expectThat(directives, HasLength(1))
         self.assertItemsEqual(expected_generate_directives, directives)
 
@@ -517,31 +644,22 @@
         ip_range = IPRange('10.0.0.1', '10.0.0.255')
         domain = factory.make_string()
         directives = DNSReverseZoneConfig.get_GENERATE_directives(
-            ip_range, domain)
+            ip_range, domain,
+            DNSZoneInfo(IPNetwork('10.0.0.0/24'), '0.0.10.in-addr.arpa'))
         self.expectThat(directives, HasLength(1))
 
-    def test_dtrt_for_larger_networks(self):
-        # For every other network size that we're not explicitly
-        # testing here,
-        # DNSReverseZoneConfig.get_GENERATE_directives() will return
-        # one GENERATE directive for every 255 addresses in the network.
-        for prefixlen in range(23, 17):
-            network = IPNetwork(
-                "%s/%s" % (factory.make_ipv4_address(), prefixlen))
-            domain = factory.make_string()
-            directives = DNSReverseZoneConfig.get_GENERATE_directives(
-                network, domain)
-            self.expectThat(directives, HasLength(network.size / 256))
-
-    def test_returns_two_entries_for_slash_23_network(self):
-        network = IPNetwork(factory.make_ipv4_network(slash=23))
+    # generate 2 zones, rather than 1 zone with 2 GENERATEs.
+    def test_returns_256_entries_for_slash_16_network(self):
+        network = IPNetwork(factory.make_ipv4_network(slash=16))
+        reverse = IPAddress(network.first).reverse_dns.split('.')[2:-1]
+        reverse = ".".join(reverse)
         domain = factory.make_string()
 
         expected_generate_directives = self.get_expected_generate_directives(
             network, domain)
         directives = DNSReverseZoneConfig.get_GENERATE_directives(
-            network, domain)
-        self.expectThat(directives, HasLength(2))
+            network, domain, DNSZoneInfo(network, reverse))
+        self.expectThat(directives, HasLength(256))
         self.assertItemsEqual(expected_generate_directives, directives)
 
     def test_ignores_network_larger_than_slash_16(self):
@@ -549,7 +667,8 @@
         self.assertEqual(
             [],
             DNSReverseZoneConfig.get_GENERATE_directives(
-                network, factory.make_string()))
+                network, factory.make_string(),
+                DNSZoneInfo(network, "do not care")))
 
     def test_ignores_networks_that_span_slash_16s(self):
         # If the upper and lower bounds of a range span two /16 networks
@@ -557,7 +676,8 @@
         # get_GENERATE_directives() will return early
         ip_range = IPRange('10.0.0.55', '10.1.0.54')
         directives = DNSReverseZoneConfig.get_GENERATE_directives(
-            ip_range, factory.make_string())
+            ip_range, factory.make_string(),
+            DNSZoneInfo(IPNetwork('10.0.0.0/15'), "do not care"))
         self.assertEqual([], directives)
 
     def test_sorts_output_by_hostname(self):
@@ -568,12 +688,19 @@
         expected_rdns = "$.%s.0.10.in-addr.arpa."
 
         directives = list(DNSReverseZoneConfig.get_GENERATE_directives(
-            network, domain))
+            network, domain,
+            DNSZoneInfo(IPNetwork('10.0.0.0/24'), '0.0.10.in-addr.arpa')))
         self.expectThat(
             directives[0], Equals(
                 ("0-255", expected_rdns % "0", expected_hostname % "0")))
+
+        expected_hostname = "10-0-%s-$." + domain + "."
+        expected_rdns = "$.%s.0.10.in-addr.arpa."
+        directives = list(DNSReverseZoneConfig.get_GENERATE_directives(
+            network, domain,
+            DNSZoneInfo(IPNetwork('10.0.1.0/24'), '1.0.10.in-addr.arpa')))
         self.expectThat(
-            directives[1], Equals(
+            directives[0], Equals(
                 ("0-255", expected_rdns % "1", expected_hostname % "1")))
 
 

=== modified file 'src/provisioningserver/dns/zoneconfig.py'
--- src/provisioningserver/dns/zoneconfig.py	2015-05-07 18:14:38 +0000
+++ src/provisioningserver/dns/zoneconfig.py	2015-11-28 05:29:35 +0000
@@ -15,13 +15,13 @@
 __all__ = [
     'DNSForwardZoneConfig',
     'DNSReverseZoneConfig',
+    'DNSZoneInfo',
     ]
 
 
 from abc import ABCMeta
 from datetime import datetime
 from itertools import chain
-import math
 
 from netaddr import (
     IPAddress,
@@ -106,6 +106,24 @@
     return intersecting_subnets, prefix, rdns_suffix
 
 
+class DNSZoneInfo:
+    """Information about a DNS zone"""
+
+    def __init__(self, subnetwork, zone_name, target_path=None):
+        """
+        :param subnetwork: IPNetwork that this zone (chunk) is for.  None
+            for forward zones.
+        :param zone_name: Fully-qualified zone name
+        :param target_path: Optional, can be used to override the target path.
+        """
+        self.subnetwork = subnetwork
+        self.zone_name = zone_name
+        if target_path is None:
+            self.target_path = compose_config_path('zone.%s' % zone_name)
+        else:
+            self.target_path = target_path
+
+
 class DNSZoneConfigBase:
     """Base class for zone writers."""
 
@@ -113,17 +131,17 @@
 
     template_file_name = 'zone.template'
 
-    def __init__(self, domain, zone_name, serial=None):
+    def __init__(self, domain, zone_info, serial=None):
         """
         :param domain: The domain name of the forward zone.
-        :param zone_name: Fully-qualified zone name.
+        :param zone_info: list of DNSZoneInfo entries.
         :param serial: The serial to use in the zone file. This must increment
             on each change.
         """
         self.domain = domain
-        self.zone_name = zone_name
         self.serial = serial
-        self.target_path = compose_config_path('zone.%s' % self.zone_name)
+        self.zone_info = zone_info
+        self.target_base = compose_config_path('zone')
 
     def make_parameters(self):
         """Return a dict of the common template parameters."""
@@ -142,9 +160,12 @@
         support a resolution of one second, and so this method may set an
         unexpected modification time in order to maintain that property.
         """
-        content = render_dns_template(cls.template_file_name, *parameters)
-        with report_missing_config_dir():
-            incremental_write(content, output_file, mode=0644)
+        if not isinstance(output_file, list):
+            output_file = [output_file]
+        for outfile in output_file:
+            content = render_dns_template(cls.template_file_name, *parameters)
+            with report_missing_config_dir():
+                incremental_write(content, outfile, mode=0644)
 
 
 class DNSForwardZoneConfig(DNSZoneConfigBase):
@@ -175,7 +196,7 @@
         self._dynamic_ranges = kwargs.pop('dynamic_ranges', [])
         self._srv_mapping = kwargs.pop('srv_mapping', [])
         super(DNSForwardZoneConfig, self).__init__(
-            domain, zone_name=domain, **kwargs)
+            domain, zone_info=[DNSZoneInfo(None, domain)], **kwargs)
 
     @classmethod
     def get_mapping(cls, mapping, domain, dns_ip):
@@ -255,7 +276,7 @@
             return []
 
         generate_directives = set()
-        subnets, prefix, _ = get_details_for_ip_range(dynamic_range)
+        subnets, prefix, rdns_suffix = get_details_for_ip_range(dynamic_range)
         for subnet in subnets:
             iterator = "%d-%d" % (
                 (subnet.first & 0x000000ff),
@@ -279,27 +300,28 @@
     def write_config(self):
         """Write the zone file."""
         # Create GENERATE directives for IPv4 ranges.
-        generate_directives = list(
-            chain.from_iterable(
-                self.get_GENERATE_directives(dynamic_range)
-                for dynamic_range in self._dynamic_ranges
-                if dynamic_range.version == 4
-            ))
-        self.write_zone_file(
-            self.target_path, self.make_parameters(),
-            {
-                'mappings': {
-                    'SRV': self.get_srv_mapping(
-                        self._srv_mapping),
-                    'A': self.get_A_mapping(
-                        self._mapping, self.domain, self._dns_ip),
-                    'AAAA': self.get_AAAA_mapping(
-                        self._mapping, self.domain, self._dns_ip),
-                },
-                'generate_directives': {
-                    'A': generate_directives,
-                }
-            })
+        for zi in self.zone_info:
+            generate_directives = list(
+                chain.from_iterable(
+                    self.get_GENERATE_directives(dynamic_range)
+                    for dynamic_range in self._dynamic_ranges
+                    if dynamic_range.version == 4
+                ))
+            self.write_zone_file(
+                zi.target_path, self.make_parameters(),
+                {
+                    'mappings': {
+                        'SRV': self.get_srv_mapping(
+                            self._srv_mapping),
+                        'A': self.get_A_mapping(
+                            self._mapping, self.domain, self._dns_ip),
+                        'AAAA': self.get_AAAA_mapping(
+                            self._mapping, self.domain, self._dns_ip),
+                    },
+                    'generate_directives': {
+                        'A': generate_directives,
+                    }
+                })
 
 
 class DNSReverseZoneConfig(DNSZoneConfigBase):
@@ -325,32 +347,88 @@
         self._mapping = kwargs.pop('mapping', {})
         self._network = kwargs.pop("network", None)
         self._dynamic_ranges = kwargs.pop('dynamic_ranges', [])
-        zone_name = self.compose_zone_name(self._network)
+        zone_info = self.compose_zone_info(self._network)
         super(DNSReverseZoneConfig, self).__init__(
-            domain, zone_name=zone_name, **kwargs)
+            domain, zone_info=zone_info, **kwargs)
 
     @classmethod
-    def compose_zone_name(cls, network):
-        """Return the name of the reverse zone."""
+    def compose_zone_info(cls, network):
+        """Return the names of the reverse zones."""
         # Generate the name of the reverse zone file:
         # Use netaddr's reverse_dns() to get the reverse IP name
         # of the first IP address in the network and then drop the first
         # octets of that name (i.e. drop the octets that will be specified in
         # the zone file).
+        # returns a list of (IPNetwork, zone_name, zonefile_path) tuples
+        info = []
         first = IPAddress(network.first)
+        last = IPAddress(network.last)
         if first.version == 6:
             # IPv6.
-            # Use float division and ceil to cope with network sizes that
-            # are not divisible by 4.
-            rest_limit = int(math.ceil((128 - network.prefixlen) / 4.))
+            # 2001:89ab::/19 yields 8.1.0.0.2.ip6.arpa, and the full list
+            # is 8.1.0.0.2.ip6.arpa, 9.1.0.0.2.ip6.arpa
+            # The ipv6 reverse dns form is 32 elements of 1 hex digit each.
+            # How many elements of the reverse DNS name to we throw away?
+            # Prefixlen of 0-3 gives us 1, 4-7 gives us 2, etc.
+            # While this seems wrong, we always _add_ a base label back in,
+            # so it's correct.
+            rest_limit = (132 - network.prefixlen) / 4
+            # What is the prefix for each inner subnet (It will be the next
+            # smaller multiple of 4.)  If it's the smallest one, then RFC2317
+            # tells us that we're adding an extra blob to the front of the
+            # reverse zone name, and we want the entire prefixlen.
+            subnet_prefix = (network.prefixlen + 3) / 4 * 4
+            if subnet_prefix == 128:
+                subnet_prefix = network.prefixlen
+            # How big is the step between subnets?  Again, special case for
+            # extra small subnets.
+            step = 1 << ((128 - network.prefixlen) / 4 * 4)
+            if step < 16:
+                step = 16
+            # Grab the base (hex) and trailing labels for our reverse zone.
+            split_zone = first.reverse_dns.split('.')
+            zone_rest = ".".join(split_zone[rest_limit:-1])
+            base = int(split_zone[rest_limit - 1], 16)
         else:
             # IPv4.
-            # Use float division and ceil to cope with splits not done on
-            # octets boundaries.
-            rest_limit = int(math.ceil((32 - network.prefixlen) / 8.))
-        reverse_name = first.reverse_dns.split('.', rest_limit)[-1]
-        # Strip off trailing '.'.
-        return reverse_name[:-1]
+            # The logic here is the same as for IPv6, but with 8 instead of 4.
+            rest_limit = (40 - network.prefixlen) / 8
+            subnet_prefix = (network.prefixlen + 7) / 8 * 8
+            if subnet_prefix == 32:
+                subnet_prefix = network.prefixlen
+            step = 1 << ((32 - network.prefixlen) / 8 * 8)
+            if step < 256:
+                step = 256
+            # Grab the base (decimal) and trailing labels for our reverse
+            # zone.
+            split_zone = first.reverse_dns.split('.')
+            zone_rest = ".".join(split_zone[rest_limit:-1])
+            base = int(split_zone[rest_limit - 1])
+        while first <= last:
+            # Rest_limit has bounds of 1..labelcount+1 (5 or 33).
+            # If we're stripping any elements, then we just want base.name.
+            if rest_limit > 1:
+                if first.version == 6:
+                    new_zone = "%x.%s" % (base, zone_rest)
+                else:
+                    new_zone = "%d.%s" % (base, zone_rest)
+            # We didn't actually strip any elemnts, so base goes back with
+            # the prefixlen attached.
+            elif first.version == 6:
+                new_zone = "%x-%d.%s" % (base, network.prefixlen, zone_rest)
+            else:
+                new_zone = "%d-%d.%s" % (base, network.prefixlen, zone_rest)
+            info.append(DNSZoneInfo(
+                IPNetwork("%s/%d" % (first, subnet_prefix)),
+                new_zone))
+            base += 1
+            try:
+                first += step
+            except IndexError:
+                # IndexError occurs when we go from 255.255.255.255 to
+                # 0.0.0.0.  If we hit that, we're all fine and done.
+                break
+        return info
 
     @classmethod
     def get_PTR_mapping(cls, mapping, domain, network):
@@ -366,7 +444,7 @@
         :param mapping: A hostname:ip-addresses mapping for all known hosts in
             the reverse zone.
         :param domain: Zone's domain name.
-        :param network: Zone's network.
+        :param network: DNS Zone's network.
         :type network: :class:`netaddr.IPNetwork`
         """
         return (
@@ -380,7 +458,7 @@
         )
 
     @classmethod
-    def get_GENERATE_directives(cls, dynamic_range, domain):
+    def get_GENERATE_directives(cls, dynamic_range, domain, zone_info):
         """Return the GENERATE directives for the reverse zone of a network."""
         slash_16 = IPNetwork("%s/16" % IPAddress(dynamic_range.first))
         if (dynamic_range.size > 256 ** 2 or
@@ -391,19 +469,29 @@
             return []
 
         generate_directives = set()
+        # The largest subnet returned is a /24.
         subnets, prefix, rdns_suffix = get_details_for_ip_range(dynamic_range)
         for subnet in subnets:
-            iterator = "%d-%d" % (
-                (subnet.first & 0x000000ff),
-                (subnet.last & 0x000000ff))
-            hostname = "%s-%d-$" % (
-                prefix.replace('.', '-'),
-                (subnet.first & 0x0000ff00) >> 8)
-            rdns = "$.%d.%s" % (
-                (subnet.first & 0x0000ff00) >> 8,
-                rdns_suffix)
-            generate_directives.add(
-                (iterator, rdns, "%s.%s." % (hostname, domain)))
+            if (IPAddress(subnet.first) in zone_info.subnetwork):
+                iterator = "%d-%d" % (
+                    (subnet.first & 0x000000ff),
+                    (subnet.last & 0x000000ff))
+                hostname = "%s-%d-$" % (
+                    prefix.replace('.', '-'),
+                    (subnet.first & 0x0000ff00) >> 8)
+                # If we're at least a /24, then fully specify the name,
+                # rather than trying to figure out how much of the name
+                # is in the zone name.
+                if zone_info.subnetwork.prefixlen <= 24:
+                    rdns = "$.%d.%s" % (
+                        (subnet.first & 0x0000ff00) >> 8,
+                        rdns_suffix)
+                else:
+                    # Let the zone declaration provide the suffix.
+                    # rather than trying to calculate it.
+                    rdns = "$"
+                generate_directives.add(
+                    (iterator, rdns, "%s.%s." % (hostname, domain)))
 
         return sorted(
             generate_directives, key=lambda directive: directive[2])
@@ -411,21 +499,25 @@
     def write_config(self):
         """Write the zone file."""
         # Create GENERATE directives for IPv4 ranges.
-        generate_directives = list(
-            chain.from_iterable(
-                self.get_GENERATE_directives(dynamic_range, self.domain)
-                for dynamic_range in self._dynamic_ranges
-                if dynamic_range.version == 4
-            ))
-        self.write_zone_file(
-            self.target_path, self.make_parameters(),
-            {
-                'mappings': {
-                    'PTR': self.get_PTR_mapping(
-                        self._mapping, self.domain, self._network),
-                },
-                'generate_directives': {
-                    'PTR': generate_directives,
+        for zi in self.zone_info:
+            generate_directives = list(
+                chain.from_iterable(
+                    self.get_GENERATE_directives(
+                        dynamic_range,
+                        self.domain,
+                        zi)
+                    for dynamic_range in self._dynamic_ranges
+                    if dynamic_range.version == 4
+                ))
+            self.write_zone_file(
+                zi.target_path, self.make_parameters(),
+                {
+                    'mappings': {
+                        'PTR': self.get_PTR_mapping(
+                            self._mapping, self.domain, zi.subnetwork),
+                    },
+                    'generate_directives': {
+                        'PTR': generate_directives,
+                    }
                 }
-            }
-        )
+            )