Merge lp:~jtv/maas/set-nameserver-in-interfaces into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Add IPv6 DNS server address to network configuration on IPv6-enabled nodes.

Uses a helper that had already been written (and tested): list_dns_servers. Also, “breaks open” the extract_ip helper because part of its work was also useful in this new case.

Most of the diff will probably be boilerplate for propagating the new parameter.

Jeroen

[Graham Binns (gmb)]

review: Approve

=== modified file 'src/maasserver/clusterrpc/osystems.py'
--- src/maasserver/clusterrpc/osystems.py	2014-10-02 10:05:43 +0000
+++ src/maasserver/clusterrpc/osystems.py	2014-10-06 10:17:13 +0000
@@ -153,6 +153,7 @@
                 'aa:bb:cc:dd:ee:ff': ['10.9.1.1'],
                 '00:11:22:33:44:55': ['192.168.32.254'],
                 },
+            'nameservers': ['10.9.1.1'],
         }
 
     :param node: A `Node`.
@@ -166,6 +167,7 @@
         `gateways_mapping` maps to a dict which maps MAC addresses to lists of
         gateway IP addresses (at most one IPv4 and one IPv6) to be used by the
         corresponding network interfaces.
+        'nameservers' maps to a list of DNS servers for the node to use.
     :return: A list of preseed dicts.
     """
     client = getClientFor(node.nodegroup.uuid)

=== modified file 'src/maasserver/networking_preseed.py'
--- src/maasserver/networking_preseed.py	2014-10-02 13:17:15 +0000
+++ src/maasserver/networking_preseed.py	2014-10-06 10:17:13 +0000
@@ -334,6 +334,7 @@
         'auto_interfaces': find_macs_for_automatic_interfaces(node),
         'ips_mapping': map_static_ips(node),
         'gateways_mapping': map_gateways(node),
+        'nameservers': list_dns_servers(node),
         }
     preseed = compose_curtin_network_preseed(node, config)
     return [json.dumps(item) for item in preseed]

=== modified file 'src/maasserver/tests/test_networking_preseed.py'
--- src/maasserver/tests/test_networking_preseed.py	2014-10-02 13:17:15 +0000
+++ src/maasserver/tests/test_networking_preseed.py	2014-10-06 10:17:13 +0000
@@ -769,6 +769,8 @@
         node.nodegroup.accept()
         network = factory.make_ipv4_network(slash=24)
         router = factory.pick_ip_in_network(network)
+        dns = factory.pick_ip_in_network(network)
+        patch_dns_servers(self, dns)
         static_low = unicode(IPAddress(network.first + 1))
         static_high = unicode(IPAddress(network.first + 2))
         dyn_low = unicode(IPAddress(network.first + 3))
@@ -790,6 +792,7 @@
             'auto_interfaces': [mac.mac_address],
             'ips_mapping': {},
             'gateways_mapping': {mac.mac_address: [router]},
+            'nameservers': [dns],
             }
         self.assertThat(fake, MockCalledOnceWith(node, expected_config))
 

=== modified file 'src/provisioningserver/drivers/osystem/__init__.py'
--- src/provisioningserver/drivers/osystem/__init__.py	2014-10-01 03:58:43 +0000
+++ src/provisioningserver/drivers/osystem/__init__.py	2014-10-06 10:17:13 +0000
@@ -194,9 +194,9 @@
         """
         raise NotImplementedError()
 
-    def compose_curtin_network_preseed(interfaces, auto_interfaces,
+    def compose_curtin_network_preseed(self, interfaces, auto_interfaces,
                                        ips_mapping, gateways_mapping,
-                                       disable_ipv4=False):
+                                       disable_ipv4=False, nameservers=None):
         """Compose a Curtin preseed to configure a node's networking.
 
         :param interfaces: A list of tuples, each a pair of an interface name
@@ -217,6 +217,7 @@
             corresponding to each MAC to use the given default gateways.
         :param disable_ipv4: Should this node be installed without IPv4
             networking?
+        :param nameservers: Optional list of DNS servers.
         :return: A list of dicts that can be JSON-encoded and submitted to
             Curtin as preseeds, perhaps in combination with other preseeds.
         """

=== modified file 'src/provisioningserver/drivers/osystem/debian_networking.py'
--- src/provisioningserver/drivers/osystem/debian_networking.py	2014-10-01 04:10:51 +0000
+++ src/provisioningserver/drivers/osystem/debian_networking.py	2014-10-06 10:17:13 +0000
@@ -19,6 +19,20 @@
 from netaddr import IPAddress
 
 
+def extract_ip_from_sequence(ips, ip_version):
+    """Return the first address for the given IP version from `ips`.
+
+    :param ips: A sequence of IP address strings.
+    :param ip_version: Either 4 or 6 (for IPv4 or IPv6 respectively).  Only an
+        address for this version will be returned.
+    :return: A matching IP address, or `None`.
+    """
+    for ip in ips:
+        if IPAddress(ip).version == ip_version:
+            return ip
+    return None
+
+
 def extract_ip(mapping, mac, ip_version):
     """Extract IP address for `mac` and given IP version from `mapping`.
 
@@ -29,10 +43,7 @@
         address for this version will be returned.
     :return: A matching IP address, or `None`.
     """
-    for ip in mapping.get(mac, []):
-        if IPAddress(ip).version == ip_version:
-            return ip
-    return None
+    return extract_ip_from_sequence(mapping.get(mac, []), ip_version)
 
 
 def compose_ipv4_stanza(interface):
@@ -40,7 +51,7 @@
     return "iface %s inet dhcp" % interface
 
 
-def compose_ipv6_stanza(interface, ip, gateway=None):
+def compose_ipv6_stanza(interface, ip, gateway=None, nameserver=None):
     """Return a Debian `/etc/network/interfaces` stanza for IPv6.
 
     The stanza will configure a static address.
@@ -52,6 +63,9 @@
         ]
     if gateway is not None:
         lines.append('\tgateway %s' % gateway)
+    if nameserver is not None:
+        # Actually this keyword accepts up to 2 nameservers.
+        lines.append('\tdns-nameservers %s' % nameserver)
     return '\n'.join(lines)
 
 
@@ -69,7 +83,8 @@
 
 
 def compose_network_interfaces(interfaces, auto_interfaces, ips_mapping,
-                               gateways_mapping, disable_ipv4=False):
+                               gateways_mapping, disable_ipv4=False,
+                               nameservers=None):
     """Return contents for a node's `/etc/network/interfaces` file.
 
     :param interfaces: A list of interface/MAC pairs for the node.
@@ -80,7 +95,13 @@
     :param gateways_mapping: A `defaultdict` mapping MAC addresses to
         containers of the corresponding network interfaces' default gateways.
     :param disable_ipv4: Should this node be installed without IPv4 networking?
+    :param nameservers: Optional list of DNS servers.
     """
+    if nameservers is None:
+        ipv6_nameserver = None
+    else:
+        ipv6_nameserver = extract_ip_from_sequence(nameservers, 6)
+
     # Should we disable IPv4 on this node?  For safety's sake, we won't do this
     # if the node has no static IPv6 addresses.  Otherwise it might become
     # accidentally unaddressable: it may have IPv6 addresses, but apart from
@@ -99,5 +120,7 @@
         if static_ipv6 is not None:
             gateway = extract_ip(gateways_mapping, mac, 6)
             stanzas.append(
-                compose_ipv6_stanza(interface, static_ipv6, gateway))
+                compose_ipv6_stanza(
+                    interface, static_ipv6, gateway=gateway,
+                    nameserver=ipv6_nameserver))
     return '%s\n' % '\n\n'.join(stanzas)

=== modified file 'src/provisioningserver/drivers/osystem/tests/test_debian_networking.py'
--- src/provisioningserver/drivers/osystem/tests/test_debian_networking.py	2014-10-01 04:09:58 +0000
+++ src/provisioningserver/drivers/osystem/tests/test_debian_networking.py	2014-10-06 10:17:13 +0000
@@ -65,7 +65,21 @@
             """) % (interface, ip, gateway)
         self.assertEqual(
             expected.strip(),
-            compose_ipv6_stanza(interface, ip, gateway).strip())
+            compose_ipv6_stanza(interface, ip, gateway=gateway).strip())
+
+    def test__adds_nameserver_if_given(self):
+        ip = factory.make_ipv6_address()
+        interface = factory.make_name('eth')
+        nameserver = factory.make_ipv6_address()
+        expected = dedent("""\
+            iface %s inet6 static
+            \tnetmask 64
+            \taddress %s
+            \tdns-nameservers %s
+            """) % (interface, ip, nameserver)
+        self.assertEqual(
+            expected.strip(),
+            compose_ipv6_stanza(interface, ip, nameserver=nameserver).strip())
 
 
 class TestHasStaticIPv6Address(MAASTestCase):
@@ -159,21 +173,42 @@
                 self.make_listing(interface, mac), [],
                 self.make_mapping(mac, {ipv6}), {}, disable_ipv4=disable_ipv4))
 
-    def test__passes_ip_and_gateway_when_creating_IPv6_stanza(self):
+    def test__passes_ip_gateway_and_nameserver_when_creating_IPv6_stanza(self):
         interface = factory.make_name('eth')
         mac = factory.make_mac_address()
         ipv6 = factory.make_ipv6_address()
         gateway = factory.make_ipv6_address()
-        fake = self.patch_autospec(debian_networking, 'compose_ipv6_stanza')
-        fake.return_value = factory.make_name('stanza')
-
-        compose_network_interfaces(
-            self.make_listing(interface, mac), [],
-            self.make_mapping(mac, {ipv6}), self.make_mapping(mac, {gateway}))
-
-        self.assertThat(fake, MockCalledOnceWith(interface, ipv6, gateway))
-
-    def test__omits_gateway_if_not_set(self):
+        nameserver = factory.make_ipv6_address()
+        fake = self.patch_autospec(debian_networking, 'compose_ipv6_stanza')
+        fake.return_value = factory.make_name('stanza')
+
+        compose_network_interfaces(
+            self.make_listing(interface, mac), [],
+            self.make_mapping(mac, {ipv6}), self.make_mapping(mac, {gateway}),
+            nameservers=[nameserver])
+
+        self.assertThat(
+            fake, MockCalledOnceWith(
+                interface, ipv6, gateway=gateway, nameserver=nameserver))
+
+    def test__ignores_IPv4_nameserver_when_creating_IPv6_stanza(self):
+        interface = factory.make_name('eth')
+        mac = factory.make_mac_address()
+        ipv6 = factory.make_ipv6_address()
+        nameserver = factory.make_ipv4_address()
+        fake = self.patch_autospec(debian_networking, 'compose_ipv6_stanza')
+        fake.return_value = factory.make_name('stanza')
+
+        compose_network_interfaces(
+            self.make_listing(interface, mac), [],
+            self.make_mapping(mac, {ipv6}), gateways_mapping={},
+            nameservers=[nameserver])
+
+        self.assertThat(
+            fake, MockCalledOnceWith(
+                interface, ANY, gateway=ANY, nameserver=None))
+
+    def test__omits_gateway_and_nameserver_if_not_set(self):
         interface = factory.make_name('eth')
         mac = factory.make_mac_address()
         fake = self.patch_autospec(debian_networking, 'compose_ipv6_stanza')
@@ -184,7 +219,7 @@
 
         self.assertThat(
             fake,
-            MockCalledOnceWith(interface, ANY, None))
+            MockCalledOnceWith(interface, ANY, gateway=None, nameserver=None))
 
     def test__writes_auto_lines_for_interfaces_in_auto_interfaces(self):
         interface = factory.make_name('eth')

=== modified file 'src/provisioningserver/drivers/osystem/ubuntu.py'
--- src/provisioningserver/drivers/osystem/ubuntu.py	2014-10-01 03:58:43 +0000
+++ src/provisioningserver/drivers/osystem/ubuntu.py	2014-10-06 10:17:13 +0000
@@ -92,7 +92,7 @@
 
     def compose_curtin_network_preseed(self, interfaces, auto_interfaces,
                                        ips_mapping, gateways_mapping,
-                                       disable_ipv4=False):
+                                       disable_ipv4=False, nameservers=None):
         """As defined in `OperatingSystem`: generate networking Curtin preseed.
 
         Supports:
@@ -103,7 +103,8 @@
         """
         interfaces_file = compose_network_interfaces(
             interfaces, auto_interfaces, ips_mapping=ips_mapping,
-            gateways_mapping=gateways_mapping, disable_ipv4=disable_ipv4)
+            gateways_mapping=gateways_mapping, disable_ipv4=disable_ipv4,
+            nameservers=nameservers)
         udev_rules = compose_network_interfaces_udev_rules(interfaces)
         write_files = {
             'write_files': {

=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2014-10-03 13:04:16 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2014-10-06 10:17:13 +0000
@@ -225,10 +225,12 @@
         auto_interfaces = config.get('auto_interfaces', [])
         ips_mapping = config.get('ips_mapping', {})
         gateways_mapping = config.get('gateways_mapping', {})
+        nameservers = config.get('nameservers', [])
         return {
             'data': compose_curtin_network_preseed(
                 osystem, interfaces, auto_interfaces, ips_mapping=ips_mapping,
-                gateways_mapping=gateways_mapping, disable_ipv4=disable_ipv4),
+                gateways_mapping=gateways_mapping, disable_ipv4=disable_ipv4,
+                nameservers=nameservers),
             }
 
     @log_call(level=logging.DEBUG)

=== modified file 'src/provisioningserver/rpc/osystems.py'
--- src/provisioningserver/rpc/osystems.py	2014-10-01 03:58:43 +0000
+++ src/provisioningserver/rpc/osystems.py	2014-10-06 10:17:13 +0000
@@ -119,7 +119,7 @@
 
 def compose_curtin_network_preseed(os_name, interfaces, auto_interfaces,
                                    ips_mapping, gateways_mapping,
-                                   disable_ipv4):
+                                   disable_ipv4, nameservers):
     """Compose Curtin network preseed for a node.
 
     :param os_name: Identifying name of the operating system for which a
@@ -132,6 +132,7 @@
     :param gateways_mapping: A `defaultdict` mapping MAC addresses to
         containers of the corresponding network interfaces' default gateways.
     :param disable_ipv4: Should this node be installed without IPv4 networking?
+    :param nameservers: List of DNS servers.
     :return: Preseed data, as JSON.
     """
     try:
@@ -141,4 +142,5 @@
     else:
         return osystem.compose_curtin_network_preseed(
             interfaces, auto_interfaces, ips_mapping=ips_mapping,
-            gateways_mapping=gateways_mapping, disable_ipv4=disable_ipv4)
+            gateways_mapping=gateways_mapping, disable_ipv4=disable_ipv4,
+            nameservers=nameservers)

=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2014-10-03 13:04:16 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2014-10-06 10:17:13 +0000
@@ -1121,6 +1121,7 @@
                 'auto_interfaces': [mac],
                 'ips_mapping': {mac: [factory.make_ipv4_address()]},
                 'gateways_mapping': {mac: [factory.make_ipv4_address()]},
+                'nameservers': [],
                 },
             'disable_ipv4': factory.pick_bool(),
             }
@@ -1134,7 +1135,8 @@
     @inlineCallbacks
     def test__calls_compose_curtin_network_preseed(self):
         preseed = [factory.make_name('preseed')]
-        fake = self.patch(clusterservice, 'compose_curtin_network_preseed')
+        fake = self.patch_autospec(
+            clusterservice, 'compose_curtin_network_preseed')
         fake.return_value = preseed
         args = self.make_args()
 

=== modified file 'src/provisioningserver/rpc/tests/test_osystems.py'
--- src/provisioningserver/rpc/tests/test_osystems.py	2014-10-01 03:58:43 +0000
+++ src/provisioningserver/rpc/tests/test_osystems.py	2014-10-06 10:17:13 +0000
@@ -221,6 +221,7 @@
                     ],
                 },
             'disable_ipv4': factory.pick_bool(),
+            'nameservers': [factory.make_ipv6_address()],
             }
 
     def test__forwards_to_OS_implementation(self):