Merge lp:~blake-rouse/maas/net-gateway-preseed into lp:~maas-committers/maas/trunk

[Blake Rouse]

Set only one default gateway IP address per IP family in the node networking preseed.

[Mike Pontillo (mpontillo)]

LGTM. Just one nit below.

review: Approve

[Blake Rouse (blake-rouse)]

Thanks for the review.

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2015-09-14 00:12:14 +0000
+++ src/maasserver/models/node.py	2015-09-14 00:12:14 +0000
@@ -1858,9 +1858,9 @@
             if interface.enabled:
                 interface.ensure_link_up()
 
-    def get_best_guess_for_default_gateway_ips(self):
-        """Return the best guess for the default gateway IP addresses. This is
-        either one IPv4 address, one IPv6 address, or both.
+    def get_best_guess_for_default_gateways(self):
+        """Return the best guess for the default gateways. This is
+        either one IPv4 gateway, one IPv6 gateway, or both.
 
         This is determined by looking at all interfaces on the node and
         selecting the best possible default gateway IP. The criteria below
@@ -1959,8 +1959,8 @@
                 return gateway
         return None
 
-    def get_default_gateway_ips(self):
-        """Return the default gateway IP addresses.
+    def get_default_gateways(self):
+        """Return the default gateways.
 
         :return: Return a tuple or tuples with IPv4 and IPv6 gateway
             information.
@@ -1987,7 +1987,7 @@
             return (gateway_ipv4, gateway_ipv6)
 
         # Get the best guesses for the missing IP families.
-        found_gateways = self.get_best_guess_for_default_gateway_ips()
+        found_gateways = self.get_best_guess_for_default_gateways()
         if not gateway_ipv4:
             gateway_ipv4 = self._get_gateway_tuple_by_family(
                 found_gateways, IPADDRESS_FAMILY.IPv4)

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2015-09-14 00:12:14 +0000
+++ src/maasserver/models/tests/test_node.py	2015-09-14 00:12:14 +0000
@@ -2614,7 +2614,8 @@
         self.assertItemsEqual(enabled_interfaces, observed_interfaces)
 
 
-class TestGetBestGuessForDefaultGatewayIPs(MAASServerTestCase):
+class TestGetBestGuessForDefaultGateways(MAASServerTestCase):
+    """Tests for `Node.get_best_guess_for_default_gateways`."""
 
     def test__simple(self):
         node = factory.make_Node_with_Interface_on_Subnet(
@@ -2625,7 +2626,7 @@
         gateway_ip = managed_subnet.gateway_ip
         self.assertEquals(
             [(boot_interface.id, managed_subnet.id, gateway_ip)],
-            node.get_best_guess_for_default_gateway_ips())
+            node.get_best_guess_for_default_gateways())
 
     def test__ipv4_and_ipv6(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2653,7 +2654,7 @@
         self.assertItemsEqual([
             (interface.id, subnet_v4.id, subnet_v4.gateway_ip),
             (interface.id, subnet_v6.id, subnet_v6.gateway_ip),
-            ], node.get_best_guess_for_default_gateway_ips())
+            ], node.get_best_guess_for_default_gateways())
 
     def test__only_one(self):
         node = factory.make_Node_with_Interface_on_Subnet(
@@ -2669,7 +2670,7 @@
         gateway_ip = managed_subnet.gateway_ip
         self.assertEquals(
             [(boot_interface.id, managed_subnet.id, gateway_ip)],
-            node.get_best_guess_for_default_gateway_ips())
+            node.get_best_guess_for_default_gateways())
 
     def test__managed_subnet_over_unmanaged(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2696,7 +2697,7 @@
         gateway_ip = managed_subnet.gateway_ip
         self.assertEquals(
             [(interface.id, managed_subnet.id, gateway_ip)],
-            node.get_best_guess_for_default_gateway_ips())
+            node.get_best_guess_for_default_gateways())
 
     def test__bond_over_physical_interface(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2727,7 +2728,7 @@
         gateway_ip = bond_subnet.gateway_ip
         self.assertEquals(
             [(bond_interface.id, bond_subnet.id, gateway_ip)],
-            node.get_best_guess_for_default_gateway_ips())
+            node.get_best_guess_for_default_gateways())
 
     def test__physical_over_vlan_interface(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2754,7 +2755,7 @@
         gateway_ip = physical_subnet.gateway_ip
         self.assertEquals(
             [(physical_interface.id, physical_subnet.id, gateway_ip)],
-            node.get_best_guess_for_default_gateway_ips())
+            node.get_best_guess_for_default_gateways())
 
     def test__boot_interface_over_other_interfaces(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2783,7 +2784,7 @@
         gateway_ip = boot_subnet.gateway_ip
         self.assertEquals(
             [(boot_interface.id, boot_subnet.id, gateway_ip)],
-            node.get_best_guess_for_default_gateway_ips())
+            node.get_best_guess_for_default_gateways())
 
     def test__sticky_ip_over_user_reserved(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2808,7 +2809,7 @@
         gateway_ip = sticky_subnet.gateway_ip
         self.assertEquals(
             [(interface.id, sticky_subnet.id, gateway_ip)],
-            node.get_best_guess_for_default_gateway_ips())
+            node.get_best_guess_for_default_gateways())
 
     def test__user_reserved_ip_over_auto(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2833,11 +2834,11 @@
         gateway_ip = user_reserved_subnet.gateway_ip
         self.assertEquals(
             [(interface.id, user_reserved_subnet.id, gateway_ip)],
-            node.get_best_guess_for_default_gateway_ips())
-
-
-class TestGetDefaultGatewayIP(MAASServerTestCase):
-    """Tests for `Node.get_default_gateway_ips`."""
+            node.get_best_guess_for_default_gateways())
+
+
+class TestGetDefaultGateways(MAASServerTestCase):
+    """Tests for `Node.get_default_gateways`."""
 
     def test__return_set_ipv4_and_ipv6(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2880,7 +2881,7 @@
         self.assertEquals((
             (interface.id, subnet_v4_2.id, subnet_v4_2.gateway_ip),
             (interface.id, subnet_v6_2.id, subnet_v6_2.gateway_ip),
-            ), node.get_default_gateway_ips())
+            ), node.get_default_gateways())
 
     def test__return_set_ipv4_and_guess_ipv6(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2913,7 +2914,7 @@
         self.assertEquals((
             (interface.id, subnet_v4_2.id, subnet_v4_2.gateway_ip),
             (interface.id, subnet_v6.id, subnet_v6.gateway_ip),
-            ), node.get_default_gateway_ips())
+            ), node.get_default_gateways())
 
     def test__return_set_ipv6_and_guess_ipv4(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2946,7 +2947,7 @@
         self.assertEquals((
             (interface.id, subnet_v4.id, subnet_v4.gateway_ip),
             (interface.id, subnet_v6_2.id, subnet_v6_2.gateway_ip),
-            ), node.get_default_gateway_ips())
+            ), node.get_default_gateways())
 
     def test__return_guess_ipv4_and_ipv6(self):
         nodegroup = factory.make_NodeGroup(status=NODEGROUP_STATUS.ENABLED)
@@ -2968,7 +2969,7 @@
         self.assertEquals((
             (interface.id, subnet_v4.id, subnet_v4.gateway_ip),
             (interface.id, subnet_v6.id, subnet_v6.gateway_ip),
-            ), node.get_default_gateway_ips())
+            ), node.get_default_gateways())
 
 
 class TestDeploymentStatus(MAASServerTestCase):

=== modified file 'src/maasserver/preseed_network.py'
--- src/maasserver/preseed_network.py	2015-09-10 14:00:42 +0000
+++ src/maasserver/preseed_network.py	2015-09-14 00:12:14 +0000
@@ -18,6 +18,7 @@
 from maasserver.dns.zonegenerator import get_dns_server_address
 from maasserver.enum import (
     INTERFACE_TYPE,
+    IPADDRESS_FAMILY,
     IPADDRESS_TYPE,
 )
 import yaml
@@ -28,7 +29,9 @@
 
     def __init__(self, node):
         self.node = node
-        self.boot_interface = node.get_boot_interface()
+        self.gateways = node.get_default_gateways()
+        self.gateway_ipv4_set = False
+        self.gateway_ipv6_set = False
         self.operations = {
             "physical": [],
             "vlan": [],
@@ -134,6 +137,35 @@
         else:
             return None
 
+    def _get_default_gateway(self, iface, subnet):
+        """Return True if this is the gateway that should be added to the
+        interface configuration."""
+        if subnet.gateway_ip:
+            for gateway in self.gateways:
+                if gateway is not None:
+                    iface_id, subnet_id, gateway_ip = gateway
+                    if (iface_id == iface.id and
+                            subnet_id and subnet.id and
+                            gateway_ip and subnet.gateway_ip):
+                        return subnet.gateway_ip
+        return None
+
+    def _set_default_gateway(self, iface, subnet, subnet_operation):
+        """Set the default gateway on the `subnet_operation` if it should
+        be set."""
+        ip_family = subnet.get_ipnetwork().version
+        if ip_family == IPADDRESS_FAMILY.IPv4 and self.gateway_ipv4_set:
+            return
+        elif ip_family == IPADDRESS_FAMILY.IPv6 and self.gateway_ipv6_set:
+            return
+        gateway = self._get_default_gateway(iface, subnet)
+        if gateway is not None:
+            if ip_family == IPADDRESS_FAMILY.IPv4:
+                self.gateway_ipv4_set = True
+            elif ip_family == IPADDRESS_FAMILY.IPv6:
+                self.gateway_ipv6_set = True
+            subnet_operation["gateway"] = unicode(gateway)
+
     def _generate_addresses(self, iface):
         """Generate the various addresses needed for this interface."""
         addrs = []
@@ -150,8 +182,7 @@
                     "type": "static",
                     "address": "%s/%s" % (unicode(address.ip), subnet_len)
                 }
-                if subnet.gateway_ip is not None:
-                    subnet_operation["gateway"] = unicode(subnet.gateway_ip)
+                self._set_default_gateway(iface, subnet, subnet_operation)
                 if subnet.dns_servers is not None:
                     subnet_operation["dns_nameservers"] = subnet.dns_servers
                 addrs.append(subnet_operation)

=== modified file 'src/maasserver/tests/test_preseed_network.py'
--- src/maasserver/tests/test_preseed_network.py	2015-09-10 14:00:42 +0000
+++ src/maasserver/tests/test_preseed_network.py	2015-09-14 00:12:14 +0000
@@ -19,6 +19,7 @@
 from maasserver.dns.zonegenerator import get_dns_server_address
 from maasserver.enum import (
     INTERFACE_TYPE,
+    IPADDRESS_FAMILY,
     IPADDRESS_TYPE,
 )
 from maasserver.preseed_network import compose_curtin_network_config
@@ -84,10 +85,33 @@
         expected_equals = map(Equals, expected_network)
         self.assertThat(output_network, MatchesListwise(expected_equals))
 
-    def collectInterfaceConfig(self, node, filter="physical"):
+    def collect_interface_config(self, node, filter="physical"):
         interfaces = node.interface_set.filter(enabled=True).order_by('id')
         if filter:
             interfaces = interfaces.filter(type=filter)
+
+        gateways = node.get_default_gateways()
+        ipv4_gateway_set, ipv6_gateway_set = False, False
+
+        def set_gateway_ip(iface, subnet, ret, ipv4_set, ipv6_set):
+            ip_family = subnet.get_ipnetwork().version
+            if ip_family == IPADDRESS_FAMILY.IPv4 and ipv4_set:
+                return (ret, ipv4_set, ipv6_set)
+            elif ip_family == IPADDRESS_FAMILY.IPv6 and ipv6_set:
+                return (ret, ipv4_set, ipv6_set)
+            for gateway in gateways:
+                if gateway is not None:
+                    iface_id, subnet_id, gateway_ip = gateway
+                    if (iface_id == iface.id and
+                            subnet_id == subnet.id and
+                            gateway_ip == subnet.gateway_ip):
+                        ret += "    gateway: %s\n" % gateway_ip
+                        if ip_family == IPADDRESS_FAMILY.IPv4:
+                            ipv4_set = True
+                        elif ip_family == IPADDRESS_FAMILY.IPv6:
+                            ipv6_set = True
+            return (ret, ipv4_set, ipv6_set)
+
         ret = ""
         for iface in interfaces:
             self.assertIn(iface.type, ["physical", "bond", "vlan"])
@@ -120,8 +144,10 @@
                         ret += "  - address: %s/%s\n" % (
                             unicode(address.ip), subnet_len)
                         ret += "    type: static\n"
-                        if subnet.gateway_ip is not None:
-                            ret += "    gateway: %s\n" % subnet.gateway_ip
+                        ret, ipv4_gateway_set, ipv6_gateway_set = (
+                            set_gateway_ip(
+                                iface, subnet, ret,
+                                ipv4_gateway_set, ipv6_gateway_set))
                         if subnet.dns_servers is not None:
                             ret += "    dns_nameservers:\n"
                             for dns_server in subnet.dns_servers:
@@ -164,7 +190,7 @@
         sip.subnet = None
         sip.save()
         factory.make_Interface(node=node)
-        net_config = self.collectInterfaceConfig(node)
+        net_config = self.collect_interface_config(node)
         net_config += self.collectDNSConfig(node)
         config = compose_curtin_network_config(node)
         self.assertNetworkConfig(net_config, config)
@@ -181,10 +207,10 @@
             iftype=INTERFACE_TYPE.BOND, node=node, vlan=vlan,
             parents=interfaces)
         factory.make_StaticIPAddress(
-            interface=bond_iface,
+            interface=bond_iface, alloc_type=IPADDRESS_TYPE.STICKY,
             subnet=bond_iface.vlan.subnet_set.first())
-        net_config = self.collectInterfaceConfig(node, filter="physical")
-        net_config += self.collectInterfaceConfig(node, filter="bond")
+        net_config = self.collect_interface_config(node, filter="physical")
+        net_config += self.collect_interface_config(node, filter="bond")
         net_config += self.collectDNSConfig(node)
         config = compose_curtin_network_config(node)
         self.assertNetworkConfig(net_config, config)
@@ -200,8 +226,8 @@
             iftype=INTERFACE_TYPE.VLAN, node=node, parents=interfaces)
         subnet = factory.make_Subnet(vlan=vlan_iface.vlan)
         factory.make_StaticIPAddress(interface=vlan_iface, subnet=subnet)
-        net_config = self.collectInterfaceConfig(node, filter="physical")
-        net_config += self.collectInterfaceConfig(node, filter="vlan")
+        net_config = self.collect_interface_config(node, filter="physical")
+        net_config += self.collect_interface_config(node, filter="vlan")
         net_config += self.collectDNSConfig(node)
         config = compose_curtin_network_config(node)
         self.assertNetworkConfig(net_config, config)
@@ -222,9 +248,9 @@
             iftype=INTERFACE_TYPE.VLAN, node=node, parents=[bond_iface])
         subnet = factory.make_Subnet(vlan=vlan_iface.vlan)
         factory.make_StaticIPAddress(interface=vlan_iface, subnet=subnet)
-        net_config = self.collectInterfaceConfig(node, filter="physical")
-        net_config += self.collectInterfaceConfig(node, filter="bond")
-        net_config += self.collectInterfaceConfig(node, filter="vlan")
+        net_config = self.collect_interface_config(node, filter="physical")
+        net_config += self.collect_interface_config(node, filter="bond")
+        net_config += self.collect_interface_config(node, filter="vlan")
         net_config += self.collectDNSConfig(node)
         config = compose_curtin_network_config(node)
         self.assertNetworkConfig(net_config, config)