launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/rename-rev-zone-file into lp:maas with lp:~rvb/maas/fix-buildout-netaddr as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/rename-rev-zone-file/+merge/116611

This branch is a small cleanup branch: it fixes the zone file name so that it now follows the standard and includes the actual name of the reverse zone in its name.  For instance, for a 192.168.0.1/22 network, the reverse zone file will be named 'zone.rev.168.192.in-addr.arpa'.  Right now, this doesn't change anything really but this will only be useful when we will tackle the problem of the colliding network and implement RFC2317 but this is a first step towards a proper solution.
-- 
https://code.launchpad.net/~rvb/maas/rename-rev-zone-file/+merge/116611
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/rename-rev-zone-file into lp:maas.

=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py	2012-07-25 08:20:23 +0000
+++ src/maasserver/dns.py	2012-07-25 09:45:26 +0000
@@ -59,7 +59,7 @@
 def change_dns_zone(nodegroup):
     """Update the zone configurtion for the given `nodegroup`.
 
-    :param nodegroup: The nodegroup for which the zone should be cupdated.
+    :param nodegroup: The nodegroup for which the zone should be updated.
     :type nodegroup: :class:`NodeGroup`
     """
     zone = get_zone(nodegroup)

=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py	2012-07-24 17:47:48 +0000
+++ src/maasserver/tests/test_dns.py	2012-07-25 09:45:26 +0000
@@ -13,6 +13,8 @@
 __all__ = []
 
 
+from itertools import islice
+
 from django.core.management import call_command
 from maasserver.dns import (
     add_zone,
@@ -26,7 +28,10 @@
 from maastesting.bindfixture import BINDServer
 from maastesting.celery import CeleryFixture
 from maastesting.tests.test_bindfixture import dig_call
-from netaddr import IPNetwork
+from netaddr import (
+    IPNetwork,
+    IPRange,
+    )
 from provisioningserver.dns.config import conf
 from provisioningserver.dns.utils import generated_hostname
 from testresources import FixtureResource
@@ -82,9 +87,10 @@
         node = factory.make_node(
             nodegroup=nodegroup, set_hostname=True)
         mac = factory.make_mac_address(node=node)
+        ips = IPRange(nodegroup.ip_range_low, nodegroup.ip_range_high)
+        lease_ip = str(islice(ips, lease_number, lease_number + 1).next())
         lease = factory.make_dhcp_lease(
-            nodegroup=nodegroup, mac=mac.mac_address,
-            ip='192.168.0.%d' % lease_number)
+            nodegroup=nodegroup, mac=mac.mac_address, ip=lease_ip)
         return nodegroup, node, lease
 
     def dig_resolve(self, fqdn):

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2012-07-25 08:20:23 +0000
+++ src/provisioningserver/dns/config.py	2012-07-25 09:45:26 +0000
@@ -277,7 +277,7 @@
     def target_reverse_path(self):
         """Return the full path of the DNS reverse zone config file."""
         return os.path.join(
-            self.target_dir, 'zone.rev.%s' % self.zone_name)
+            self.target_dir, 'zone.rev.%s' % self.reverse_zone_name)
 
     def get_base_context(self):
         """Return the dict used to render both zone files."""

=== modified file 'src/provisioningserver/dns/templates/named.conf.template'
--- src/provisioningserver/dns/templates/named.conf.template	2012-07-17 16:43:28 +0000
+++ src/provisioningserver/dns/templates/named.conf.template	2012-07-25 09:45:26 +0000
@@ -1,6 +1,6 @@
 include "{{named_rndc_conf_path}}";
 
-# Zone declarations
+# Zone declarations.
 {{for zone in zones}}
 zone "{{zone.zone_name}}" {
     type master;
@@ -8,6 +8,7 @@
 };
 {{endfor}}
 
+# Reverse zone declarations.
 {{for zone in zones}}
 zone "{{zone.reverse_zone_name}}" {
     type master;

=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py	2012-07-25 08:20:23 +0000
+++ src/provisioningserver/dns/tests/test_config.py	2012-07-25 09:45:26 +0000
@@ -121,7 +121,7 @@
         target_dir = self.make_dir()
         self.patch(DNSConfig, 'target_dir', target_dir)
         zone_name = factory.getRandomString()
-        network = factory.getRandomNetwork()
+        network = IPNetwork('192.168.0.3/24')
         ip = factory.getRandomIPInNetwork(network)
         zone = DNSZoneConfig(
             zone_name, mapping={factory.getRandomString(): ip},
@@ -134,7 +134,7 @@
                 matcher=ContainsAll(
                     [
                         'zone.%s' % zone_name,
-                        'zone.rev.%s' % zone_name,
+                        'zone.rev.0.168.192.in-addr.arpa',
                         MAAS_NAMED_RNDC_CONF_NAME,
                     ])))
 
@@ -197,12 +197,16 @@
 
     def test_DNSZoneConfig_computes_dns_config_file_paths(self):
         zone_name = factory.getRandomString()
-        dns_zone_config = DNSZoneConfig(zone_name)
+        reverse_file_name = 'zone.rev.168.192.in-addr.arpa'
+        dns_zone_config = DNSZoneConfig(
+            zone_name, broadcast_ip='192.168.0.255',
+            subnet_mask='255.255.252.0',
+            )
         self.assertEqual(
             (
                 os.path.join(TEMPLATES_PATH, 'zone.template'),
                 os.path.join(conf.DNS_CONFIG_DIR, 'zone.%s' % zone_name),
-                os.path.join(conf.DNS_CONFIG_DIR, 'zone.rev.%s' % zone_name)
+                os.path.join(conf.DNS_CONFIG_DIR, reverse_file_name)
             ),
             (
                 dns_zone_config.template_path,
@@ -311,8 +315,9 @@
         dns_zone_config = DNSZoneConfig(
             zone_name, serial=random.randint(1, 100), **network_infos(network))
         dns_zone_config.write_reverse_config()
+        reverse_file_name = 'zone.rev.168.192.in-addr.arpa'
         self.assertThat(
-            os.path.join(target_dir, 'zone.rev.%s' % zone_name),
+            os.path.join(target_dir, reverse_file_name),
             FileContains(
                 matcher=ContainsAll(
                     ['%s IN PTR %s' % (

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-07-25 08:20:23 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-07-25 09:45:26 +0000
@@ -21,6 +21,7 @@
 from maastesting.fakemethod import FakeMethod
 from maastesting.matchers import ContainsAll
 from maastesting.testcase import TestCase
+from netaddr import IPNetwork
 from provisioningserver import tasks
 from provisioningserver.dns.config import (
     conf,
@@ -153,7 +154,7 @@
     def test_write_dns_zone_config_writes_file(self):
         command = factory.getRandomString()
         zone_name = factory.getRandomString()
-        network = factory.getRandomNetwork()
+        network = IPNetwork('192.168.0.3/24')
         ip = factory.getRandomIPInNetwork(network)
         zone = DNSZoneConfig(
             zone_name, serial=random.randint(1, 100),
@@ -161,11 +162,12 @@
         result = write_dns_zone_config.delay(
             zone=zone, callback=rndc_command.subtask(args=[command]))
 
+        reverse_file_name = 'zone.rev.0.168.192.in-addr.arpa'
         self.assertThat(
             (
                 result.successful(),
                 os.path.join(self.dns_conf_dir, 'zone.%s' % zone_name),
-                os.path.join(self.dns_conf_dir, 'zone.rev.%s' % zone_name),
+                os.path.join(self.dns_conf_dir, reverse_file_name),
                 self.rndc_recorder.calls,
             ),
             MatchesListwise(
@@ -215,7 +217,7 @@
         # write_full_dns_config writes the config file, writes
         # the zone files, and reloads the dns service.
         zone_name = factory.getRandomString()
-        network = factory.getRandomNetwork()
+        network = IPNetwork('192.168.0.3/24')
         ip = factory.getRandomIPInNetwork(network)
         zones = [DNSZoneConfig(
             zone_name, serial=random.randint(1, 100),
@@ -225,16 +227,14 @@
             zones=zones,
             callback=rndc_command.subtask(args=[command]))
 
+        reverse_file_name = 'zone.rev.0.168.192.in-addr.arpa'
         self.assertThat(
             (
                 result.successful(),
                 self.rndc_recorder.calls,
-                os.path.join(
-                    self.dns_conf_dir, 'zone.%s' % zone_name),
-                os.path.join(
-                    self.dns_conf_dir, 'zone.rev.%s' % zone_name),
-                os.path.join(
-                    self.dns_conf_dir, MAAS_NAMED_CONF_NAME),
+                os.path.join(self.dns_conf_dir, 'zone.%s' % zone_name),
+                os.path.join(self.dns_conf_dir, reverse_file_name),
+                os.path.join(self.dns_conf_dir, MAAS_NAMED_CONF_NAME),
             ),
             MatchesListwise(
                 (