Merge ~mpontillo/maas:preseed-per-interface-routes--bug-1758919--2.3 into maas:2.3

[Mike Pontillo]

LP: #1758919 - Move routes to interface context within preseed.

Backports: 9fbf7eb682c3c3ff1de08b33bb967fffdf4a61bf

[Mike Pontillo (mpontillo)]

Self-approve backport.

review: Approve

diff --git a/src/maasserver/preseed_network.py b/src/maasserver/preseed_network.py
index 1be72d6..bb56765 100644
--- a/src/maasserver/preseed_network.py
+++ b/src/maasserver/preseed_network.py
@@ -52,8 +52,6 @@ def _generate_route_operation(route, version=1):
     """Generate route operation place in `network_config`."""
     if version == 1:
         route_operation = {
-            "id": route.id,
-            "type": "route",
             "destination": route.destination.cidr,
             "gateway": route.gateway_ip,
             "metric": route.metric,
@@ -99,6 +97,19 @@ class InterfaceConfiguration:
         else:
             raise ValueError("Unknown interface type: %s" % self.type)
 
+        if version == 2:
+            routes = self._generate_route_operations(
+                self.matching_routes, version=version)
+            if len(routes) > 0:
+                self.config['routes'] = routes
+
+    def _generate_route_operations(self, matching_routes, version=1):
+        """Generate all route operations."""
+        routes = []
+        for route in sorted(matching_routes, key=attrgetter("id")):
+            routes.append(_generate_route_operation(route, version=version))
+        return routes
+
     def _generate_physical_operation(self, version=1):
         """Generate physical interface operation for `interface` and place in
         `network_config`."""
@@ -241,7 +252,8 @@ class InterfaceConfiguration:
                     self._set_default_gateway(
                         subnet,
                         v1_subnet_operation if version == 1 else v2_config)
-                    if subnet.dns_servers is not None:
+                    if (subnet.dns_servers is not None and
+                            len(subnet.dns_servers) > 0):
                         v1_subnet_operation["dns_nameservers"] = (
                             subnet.dns_servers)
                         if "nameservers" not in v2_config:
@@ -251,8 +263,16 @@ class InterfaceConfiguration:
                                 v2_nameservers["addresses"] = []
                         v2_nameservers["addresses"].extend(
                             [server for server in subnet.dns_servers])
-                    self.matching_routes.update(
-                        self._get_matching_routes(subnet))
+                    matching_subnet_routes = self._get_matching_routes(subnet)
+                    if len(matching_subnet_routes) > 0 and version == 1:
+                        # For the v1 YAML, the list of routes is rendered
+                        # within the context of each subnet.
+                        routes = self._generate_route_operations(
+                            matching_subnet_routes, version=version)
+                        v1_subnet_operation['routes'] = routes
+                    # Keep track of routes which apply to the context of this
+                    # interface for rendering the v2 YAML.
+                    self.matching_routes.update(matching_subnet_routes)
             if dhcp_type:
                 v1_config.append(
                     {"type": dhcp_type}
@@ -479,7 +499,6 @@ class NodeNetworkConfiguration:
             for name in sorted(get_dns_search_paths())
             if name != self.node.domain.name
         ]
-        self._generate_route_operations(version=version)
         self.v1_config.append({
             "type": "nameserver",
             "address": default_dns_servers,
@@ -517,16 +536,6 @@ class NodeNetworkConfiguration:
             #     v2_config.update({"nameservers": nameservers})
         self.config = network_config
 
-    def _generate_route_operations(self, version=1):
-        """Generate all route operations."""
-        routes = []
-        for route in sorted(self.matching_routes, key=attrgetter("id")):
-            routes.append(_generate_route_operation(route, version=version))
-        if version == 1:
-            self.v1_config.extend(routes)
-        elif version == 2 and len(routes) > 0:
-            self.v2_config["routes"] = routes
-
 
 def compose_curtin_network_config(node, version=1):
     """Compose the network configuration for curtin."""
diff --git a/src/maasserver/tests/test_preseed_network.py b/src/maasserver/tests/test_preseed_network.py
index 8a509dc..cc66f9f 100644
--- a/src/maasserver/tests/test_preseed_network.py
+++ b/src/maasserver/tests/test_preseed_network.py
@@ -5,7 +5,6 @@
 
 __all__ = []
 
-from operator import attrgetter
 import random
 from textwrap import dedent
 
@@ -15,7 +14,6 @@ from maasserver.enum import (
     IPADDRESS_FAMILY,
     IPADDRESS_TYPE,
 )
-from maasserver.models.staticroute import StaticRoute
 from maasserver.preseed_network import (
     compose_curtin_network_config,
     NodeNetworkConfiguration,
@@ -228,31 +226,6 @@ class AssertNetworkConfigMixin:
                     ret += "  - type: dhcp6\n"
         return ret
 
-    def collectRoutesConfig(self, node):
-        routes = set()
-        interfaces = node.interface_set.filter(enabled=True).order_by('name')
-        for iface in interfaces:
-            addresses = iface.ip_addresses.exclude(
-                alloc_type__in=[
-                    IPADDRESS_TYPE.DISCOVERED,
-                    IPADDRESS_TYPE.DHCP,
-                ]).order_by('id')
-            for address in addresses:
-                subnet = address.subnet
-                if subnet is not None:
-                    routes.update(
-                        StaticRoute.objects.filter(
-                            source=subnet).order_by('id'))
-        config = ""
-        for route in sorted(routes, key=attrgetter('id')):
-            config += self.ROUTE_CONFIG % {
-                'id': route.id,
-                'destination': route.destination.cidr,
-                'gateway': route.gateway_ip,
-                'metric': route.metric,
-            }
-        return config
-
     def collectDNSConfig(self, node, ipv4=True, ipv6=True):
         config = "- type: nameserver\n  address: %s\n  search:\n" % (
             repr(node.get_default_dns_servers(ipv4=ipv4, ipv6=ipv6)))
@@ -466,29 +439,14 @@ class TestBridgeNetworkLayout(MAASServerTestCase, AssertNetworkConfigMixin):
         self.assertNetworkConfig(net_config, config)
 
 
-class TestNetworkLayoutWithRoutes(
-        MAASServerTestCase, AssertNetworkConfigMixin):
-
-    def test__renders_expected_output(self):
-        node = factory.make_Node_with_Interface_on_Subnet(
-            interface_count=2)
-        for iface in node.interface_set.filter(enabled=True):
-            subnet = iface.vlan.subnet_set.first()
-            factory.make_StaticRoute(source=subnet)
-            factory.make_StaticIPAddress(
-                interface=iface, subnet=subnet)
-        net_config = self.collect_interface_config(node)
-        net_config += self.collectRoutesConfig(node)
-        net_config += self.collectDNSConfig(node)
-        config = compose_curtin_network_config(node)
-        self.assertNetworkConfig(net_config, config)
-
-
 class TestNetplan(MAASServerTestCase):
 
     def _render_netplan_dict(self, node):
         return NodeNetworkConfiguration(node, version=2).config
 
+    def _render_v1_dict(self, node):
+        return NodeNetworkConfiguration(node, version=1).config
+
     def test__single_ethernet_interface(self):
         node = factory.make_Node()
         factory.make_Interface(
@@ -824,3 +782,208 @@ class TestNetplan(MAASServerTestCase):
             }
         }
         self.expectThat(netplan, Equals(expected_netplan))
+
+    def test__multiple_ethernet_interfaces_with_routes(self):
+        node = factory.make_Node()
+        vlan = factory.make_VLAN()
+        subnet = factory.make_Subnet(
+            cidr='10.0.0.0/24', gateway_ip='10.0.0.1',
+            dns_servers=[])
+        subnet2 = factory.make_Subnet(
+            cidr='10.0.1.0/24', gateway_ip='10.0.1.1',
+            dns_servers=[])
+        dest_subnet = factory.make_Subnet(cidr='192.168.0.0/24')
+        eth0 = factory.make_Interface(
+            node=node, name='eth0', mac_address="00:01:02:03:04:05", vlan=vlan)
+        eth1 = factory.make_Interface(
+            node=node, name='eth1', mac_address="02:01:02:03:04:05")
+        node.boot_interface = eth0
+        node.save()
+        factory.make_StaticIPAddress(
+            interface=eth0, subnet=subnet, ip='10.0.0.4',
+            alloc_type=IPADDRESS_TYPE.STICKY)
+        factory.make_StaticIPAddress(
+            interface=eth1, subnet=subnet2, ip='10.0.1.4',
+            alloc_type=IPADDRESS_TYPE.STICKY)
+        factory.make_StaticRoute(
+            source=subnet, gateway_ip='10.0.0.3', destination=dest_subnet,
+            metric=42)
+        factory.make_StaticRoute(
+            source=subnet2, gateway_ip='10.0.1.3', destination=dest_subnet,
+            metric=43)
+        # Make sure we know when and where the default DNS server will be used.
+        get_default_dns_servers_mock = self.patch(
+            node, 'get_default_dns_servers')
+        get_default_dns_servers_mock.return_value = ['127.0.0.2']
+        netplan = self._render_netplan_dict(node)
+        expected_netplan = {
+            'network': {
+                'version': 2,
+                'ethernets': {
+                    'eth0': {
+                        'gateway': '10.0.0.1',
+                        'match': {'macaddress': '00:01:02:03:04:05'},
+                        'mtu': 1500,
+                        'set-name': 'eth0',
+                        'addresses': ['10.0.0.4/24'],
+                        'routes': [{
+                            'to': '192.168.0.0/24',
+                            'via': '10.0.0.3',
+                            'metric': 42,
+                        }],
+                    },
+                    'eth1': {
+                        'match': {'macaddress': '02:01:02:03:04:05'},
+                        'mtu': 1500,
+                        'set-name': 'eth1',
+                        'addresses': ['10.0.1.4/24'],
+                        'routes': [{
+                            'to': '192.168.0.0/24',
+                            'via': '10.0.1.3',
+                            'metric': 43,
+                        }],
+                    },
+                },
+            },
+        }
+        self.expectThat(netplan, Equals(expected_netplan))
+        v1 = self._render_v1_dict(node)
+        expected_v1 = {
+            'network': {
+                'version': 1,
+                'config': [
+                    {
+                        'id': 'eth0',
+                        'mac_address': '00:01:02:03:04:05',
+                        'mtu': 1500,
+                        'name': 'eth0',
+                        'subnets': [{
+                            'address': '10.0.0.4/24',
+                            'gateway': '10.0.0.1',
+                            'type': 'static',
+                            'routes': [{
+                                'destination': '192.168.0.0/24',
+                                'gateway': '10.0.0.3',
+                                'metric': 42
+                            }],
+                        }],
+                        'type': 'physical'
+                    },
+                    {
+                        'id': 'eth1',
+                        'mac_address': '02:01:02:03:04:05',
+                        'mtu': 1500,
+                        'name': 'eth1',
+                        'subnets': [{
+                            'address': '10.0.1.4/24',
+                            'type': 'static',
+                            'routes': [{
+                                'destination': '192.168.0.0/24',
+                                'gateway': '10.0.1.3',
+                                'metric': 43,
+                            }],
+                        }],
+                        'type': 'physical'
+                    },
+                    {
+                        'address': ['127.0.0.2'],
+                        'search': ['maas'],
+                        'type': 'nameserver'
+                    }
+                ],
+            }
+        }
+        self.expectThat(v1, Equals(expected_v1))
+
+    def test__multiple_ethernet_interfaces_with_dns(self):
+        node = factory.make_Node()
+        vlan = factory.make_VLAN()
+        subnet = factory.make_Subnet(
+            cidr='10.0.0.0/24', gateway_ip='10.0.0.1',
+            dns_servers=['10.0.0.2'])
+        subnet2 = factory.make_Subnet(
+            cidr='10.0.1.0/24', gateway_ip='10.0.1.1',
+            dns_servers=['10.0.1.2'])
+        eth0 = factory.make_Interface(
+            node=node, name='eth0', mac_address="00:01:02:03:04:05", vlan=vlan)
+        eth1 = factory.make_Interface(
+            node=node, name='eth1', mac_address="02:01:02:03:04:05")
+        node.boot_interface = eth0
+        node.save()
+        factory.make_StaticIPAddress(
+            interface=eth0, subnet=subnet, ip='10.0.0.4',
+            alloc_type=IPADDRESS_TYPE.STICKY)
+        factory.make_StaticIPAddress(
+            interface=eth1, subnet=subnet2, ip='10.0.1.4',
+            alloc_type=IPADDRESS_TYPE.STICKY)
+        # Make sure we know when and where the default DNS server will be used.
+        get_default_dns_servers_mock = self.patch(
+            node, 'get_default_dns_servers')
+        get_default_dns_servers_mock.return_value = ['127.0.0.2']
+        netplan = self._render_netplan_dict(node)
+        expected_netplan = {
+            'network': {
+                'version': 2,
+                'ethernets': {
+                    'eth0': {
+                        'gateway': '10.0.0.1',
+                        'nameservers': {
+                            'addresses': ['10.0.0.2']
+                        },
+                        'match': {'macaddress': '00:01:02:03:04:05'},
+                        'mtu': 1500,
+                        'set-name': 'eth0',
+                        'addresses': ['10.0.0.4/24'],
+                    },
+                    'eth1': {
+                        'match': {'macaddress': '02:01:02:03:04:05'},
+                        'nameservers': {
+                            'addresses': ['10.0.1.2']
+                        },
+                        'mtu': 1500,
+                        'set-name': 'eth1',
+                        'addresses': ['10.0.1.4/24'],
+                    },
+                },
+            },
+        }
+        self.expectThat(netplan, Equals(expected_netplan))
+        v1 = self._render_v1_dict(node)
+        expected_v1 = {
+            'network': {
+                'version': 1,
+                'config': [
+                    {
+                        'id': 'eth0',
+                        'mac_address': '00:01:02:03:04:05',
+                        'mtu': 1500,
+                        'name': 'eth0',
+                        'subnets': [{
+                            'address': '10.0.0.4/24',
+                            'dns_nameservers': ['10.0.0.2'],
+                            'gateway': '10.0.0.1',
+                            'type': 'static',
+                        }],
+                        'type': 'physical'
+                    },
+                    {
+                        'id': 'eth1',
+                        'mac_address': '02:01:02:03:04:05',
+                        'mtu': 1500,
+                        'name': 'eth1',
+                        'subnets': [{
+                            'address': '10.0.1.4/24',
+                            'dns_nameservers': ['10.0.1.2'],
+                            'type': 'static',
+                        }],
+                        'type': 'physical'
+                    },
+                    {
+                        'address': ['127.0.0.2'],
+                        'search': ['maas'],
+                        'type': 'nameserver'
+                    }
+                ],
+            }
+        }
+        self.expectThat(v1, Equals(expected_v1))