launchpad-reviewers team mailing list archive

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

Raphaël Badin has proposed merging lp:~rvb/maas/add-ns-server2 into lp:maas with lp:~rvb/maas/add-ns-server as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/add-ns-server2/+merge/116858

This branch uses the method created in the pre-req branch (get_dns_server_address) to write the right A record for the zones nameservers.

Note that I've moved to the new test test_DNSZoneConfig_writes_dns_zone_config_with_NS_record a check that was previously in another test so that this new test encapsulates all the checks related to the NS record (and its related A record).
-- 
https://code.launchpad.net/~rvb/maas/add-ns-server2/+merge/116858
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/add-ns-server2 into lp:maas.

=== modified file 'src/maasserver/dns.py'
--- src/maasserver/dns.py	2012-07-26 13:13:36 +0000
+++ src/maasserver/dns.py	2012-07-26 13:13:36 +0000
@@ -150,8 +150,9 @@
     """
     if serial is None:
         serial = next_zone_serial()
+    dns_ip = get_dns_server_address()
     return DNSZoneConfig(
-        zone_name=nodegroup.name, serial=serial,
+        zone_name=nodegroup.name, serial=serial, dns_ip=dns_ip,
         subnet_mask=nodegroup.subnet_mask, broadcast_ip=nodegroup.broadcast_ip,
         ip_range_low=nodegroup.ip_range_low,
         ip_range_high=nodegroup.ip_range_high,

=== modified file 'src/maasserver/tests/test_dns.py'
--- src/maasserver/tests/test_dns.py	2012-07-26 13:13:36 +0000
+++ src/maasserver/tests/test_dns.py	2012-07-26 13:13:36 +0000
@@ -210,6 +210,20 @@
         write_full_dns_config()
         self.assertDNSMatches(node.hostname, nodegroup.name, lease.ip)
 
+    def test_dns_config_has_NS_record(self):
+        ip = factory.getRandomIPAddress()
+        self.patch(settings, 'DEFAULT_MAAS_URL', 'http://%s/' % ip)
+        nodegroup, node, lease = self.create_nodegroup_with_lease()
+        write_full_dns_config()
+        # Get the NS record for the zone 'nodegroup.name'.
+        ns_record = dig_call(
+            port=self.bind.config.port,
+            commands=[nodegroup.name, 'NS', '+short'])
+        # Resolve that hostname.
+        ip_of_ns_record = dig_call(
+            port=self.bind.config.port, commands=[ns_record, '+short'])
+        self.assertEqual(ip, ip_of_ns_record)
+
     def test_is_dns_enabled_follows_DNS_CONNECT(self):
         rand_bool = factory.getRandomBoolean()
         self.patch(settings, "DNS_CONNECT", rand_bool)

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2012-07-25 09:36:03 +0000
+++ src/provisioningserver/dns/config.py	2012-07-26 13:13:36 +0000
@@ -208,12 +208,13 @@
 
     template_file_name = 'zone.template'
 
-    def __init__(self, zone_name, serial=None, mapping={},
+    def __init__(self, zone_name, serial=None, mapping={}, dns_ip=None,
                  subnet_mask=None, broadcast_ip=None, ip_range_low=None,
                  ip_range_high=None):
         self.zone_name = zone_name
         self.serial = serial
         self.mapping = mapping
+        self.dns_ip = dns_ip
         self.subnet_mask = subnet_mask
         self.broadcast_ip = broadcast_ip
         self.ip_range_low = ip_range_low
@@ -294,8 +295,8 @@
         """
         context = self.get_base_context()
         mapping = self.get_generated_mapping()
-        # TODO: Add NS record.
-        mapping['%s.' % self.zone_name] = '127.0.0.1'
+        # Add A record for the name server's IP.
+        mapping['%s.' % self.zone_name] = self.dns_ip
         mappings = {
             'CNAME': self.get_mapping(),
             'A': mapping,

=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py	2012-07-25 15:20:42 +0000
+++ src/provisioningserver/dns/tests/test_config.py	2012-07-26 13:13:36 +0000
@@ -302,11 +302,28 @@
             FileContains(
                 matcher=ContainsAll(
                     [
-                        'IN  NS  %s.' % zone_name,
                         '%s IN CNAME %s' % (hostname, generated_hostname(ip)),
                         '%s IN A %s' % (generated_hostname(ip), ip),
                     ])))
 
+    def test_DNSZoneConfig_writes_dns_zone_config_with_NS_record(self):
+        target_dir = self.make_dir()
+        self.patch(DNSConfig, 'target_dir', target_dir)
+        network = factory.getRandomNetwork()
+        dns_ip = factory.getRandomIPAddress()
+        dns_zone_config = DNSZoneConfig(
+            factory.getRandomString(), serial=random.randint(1, 100),
+            dns_ip=dns_ip, **network_infos(network))
+        dns_zone_config.write_config()
+        self.assertThat(
+            os.path.join(target_dir, 'zone.%s' % dns_zone_config.zone_name),
+            FileContains(
+                matcher=ContainsAll(
+                    [
+                        'IN  NS  %s.' % dns_zone_config.zone_name,
+                        '%s. IN A %s' % (dns_zone_config.zone_name, dns_ip)
+                    ])))
+
     def test_DNSZoneConfig_writes_reverse_dns_zone_config(self):
         target_dir = self.make_dir()
         self.patch(DNSConfig, 'target_dir', target_dir)