launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1066958 into lp:maas.

Commit message:
Don't produce CNAMEs that are identical to hosts' respective auto-generated names.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1066958 in MAAS: "DNS config is invalid after a node gets enlisted."
  https://bugs.launchpad.net/maas/+bug/1066958

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1066958/+merge/129798

This implements Raphael's suggestion in bug 1066958, so that we no longer create invalid CNAME records when we enlist nodes.  I did not have a pre-imp; I just reproduced the bug in a unit test, and then changed the code to make the test pass.  No other tests were affected.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-1066958/+merge/129798
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/bug-1066958 into lp:maas.

=== modified file 'src/provisioningserver/dns/config.py'
--- src/provisioningserver/dns/config.py	2012-10-05 08:21:10 +0000
+++ src/provisioningserver/dns/config.py	2012-10-16 04:12:21 +0000
@@ -304,12 +304,16 @@
     def get_cname_mapping(self):
         """Return a generator with the mapping: hostname->generated hostname.
 
-        Note that we return a list of tuples instead of a dictionary in order
-        to be able to return a generator.
+        The mapping only contains hosts for which the two host names differ.
+
+        :return: A generator of tuples: (host name, generated host name).
         """
+        # We filter out cases where the two host names are identical: it
+        # would be wrong to define a CNAME that maps to itself.
         return (
             (hostname, generated_hostname(ip))
             for hostname, ip in self.mapping.items()
+                if generated_hostname(ip) != hostname
         )
 
     def get_static_mapping(self):

=== modified file 'src/provisioningserver/dns/tests/test_config.py'
--- src/provisioningserver/dns/tests/test_config.py	2012-10-05 08:21:10 +0000
+++ src/provisioningserver/dns/tests/test_config.py	2012-10-16 04:12:21 +0000
@@ -282,6 +282,22 @@
             MatchesAll(
                 IsInstance(Iterable), Not(IsInstance(Sequence))))
 
+    def test_forward_zone_get_cname_mapping_skips_identity(self):
+        # We don't write cname records to map host names to themselves.
+        # Without this, a node would get an invalid cname record upon
+        # enlistment.
+        zone = factory.make_name('zone')
+        network = IPNetwork('10.250.99.0/24')
+        ip = factory.getRandomIPInNetwork(network)
+        generated_name = generated_hostname(ip)
+        dns_zone_config = DNSForwardZoneConfig(
+            zone, networks=[network],
+            dns_ip=factory.getRandomIPInNetwork(network),
+            mapping={generated_name: ip})
+        self.assertNotIn(
+            generated_name,
+            dict(dns_zone_config.get_cname_mapping()))
+
     def test_get_static_mapping(self):
         name = factory.getRandomString()
         network = IPNetwork('192.12.0.1/30')