Merge ~ltrager/maas:lp1881133 into maas:master

[Lee Trager]

LP: #1881133 - Always include rack controller IPs for DNS servers.

MAAS was only including the configured URL for controllers which did not
always include all rack controllers the VLAN is configured to use. This
would also sometimes include the region controller's IP even when the
machine is unable to directly access the rack controller.

diff --git a/src/maasserver/preseed_network.py b/src/maasserver/preseed_network.py
index f8a782f..c13f2a3 100644
--- a/src/maasserver/preseed_network.py
+++ b/src/maasserver/preseed_network.py
@@ -8,7 +8,7 @@ __all__ = []
 from collections import defaultdict, OrderedDict
 from operator import attrgetter
 
-from netaddr import IPNetwork
+from netaddr import IPAddress, IPNetwork
 import yaml
 
 from maasserver.dns.zonegenerator import (
@@ -22,7 +22,7 @@ from maasserver.enum import (
     IPADDRESS_TYPE,
     NODE_STATUS,
 )
-from maasserver.models import Interface
+from maasserver.models import Interface, StaticIPAddress
 from maasserver.models.staticroute import StaticRoute
 from provisioningserver.utils.netplan import (
     get_netplan_bond_parameters,
@@ -362,40 +362,42 @@ class InterfaceConfiguration:
                         v1_subnet_operation if version == 1 else v2_config,
                         version,
                     )
-                    if (
-                        subnet.dns_servers is not None
-                        and len(subnet.dns_servers) > 0
-                    ):
+
+                    if "dns_nameservers" not in v1_subnet_operation:
                         v1_subnet_operation["dns_nameservers"] = []
                         v1_subnet_operation[
                             "dns_search"
                         ] = self.node_config.default_search_list
-                        if "nameservers" not in v2_config:
-                            v2_config["nameservers"] = v2_nameservers
-                            v2_config["nameservers"][
-                                "search"
-                            ] = self.node_config.default_search_list
-                            if "addresses" not in v2_nameservers:
-                                v2_nameservers["addresses"] = []
-                        for rack in {
+                    if "nameservers" not in v2_config:
+                        v2_config["nameservers"] = v2_nameservers
+                        v2_config["nameservers"][
+                            "search"
+                        ] = self.node_config.default_search_list
+                        if "addresses" not in v2_nameservers:
+                            v2_nameservers["addresses"] = []
+
+                    for ip in StaticIPAddress.objects.filter(
+                        interface__node__in=[
                             subnet.vlan.primary_rack,
                             subnet.vlan.secondary_rack,
-                        }:
-                            if rack is None:
-                                continue
-                            for ip in get_dns_server_addresses(
-                                rack_controller=rack,
-                                ipv4=(subnet.get_ip_version() == 4),
-                                ipv6=(subnet.get_ip_version() == 6),
-                                include_alternates=True,
-                            ):
-                                if ip.is_loopback():
-                                    continue
-                                ip_str = str(ip)
-                                v1_subnet_operation["dns_nameservers"].append(
-                                    ip_str
-                                )
-                                v2_nameservers["addresses"].append(ip_str)
+                        ],
+                        subnet__vlan=subnet.vlan,
+                        alloc_type__in=[
+                            IPADDRESS_TYPE.AUTO,
+                            IPADDRESS_TYPE.STICKY,
+                        ],
+                    ).exclude(ip=None):
+                        if (
+                            IPAddress(ip.get_ip()).version
+                            == subnet.get_ip_version()
+                            and ip.get_ip() not in v2_nameservers["addresses"]
+                        ):
+                            v1_subnet_operation["dns_nameservers"].append(
+                                ip.ip
+                            )
+                            v2_nameservers["addresses"].append(ip.ip)
+
+                    if subnet.dns_servers:
                         v1_subnet_operation[
                             "dns_nameservers"
                         ] += subnet.dns_servers
@@ -408,6 +410,14 @@ class InterfaceConfiguration:
                             matching_subnet_routes, version=version
                         )
                         v1_subnet_operation["routes"] = routes
+
+                    # Delete if no DNS servers were added.
+                    if len(v1_subnet_operation["dns_nameservers"]) == 0:
+                        del v1_subnet_operation["dns_nameservers"]
+                        del v1_subnet_operation["dns_search"]
+                    if len(v2_nameservers["addresses"]) == 0:
+                        del v2_config["nameservers"]
+
             if dhcp_type:
                 v1_config.append({"type": dhcp_type})
                 if dhcp_type == "dhcp":
@@ -416,6 +426,7 @@ class InterfaceConfiguration:
                     v2_config.update({"dhcp4": True})
                 elif dhcp_type == "dhcp6":
                     v2_config.update({"dhcp6": True})
+
         if version == 1:
             return v1_config
         elif version == 2:
diff --git a/src/maasserver/tests/test_preseed_network.py b/src/maasserver/tests/test_preseed_network.py
index bc49c79..c19283a 100644
--- a/src/maasserver/tests/test_preseed_network.py
+++ b/src/maasserver/tests/test_preseed_network.py
@@ -1739,6 +1739,28 @@ class TestNetplan(MAASServerTestCase):
         }
         self.expectThat(v1, Equals(expected_v1))
 
+    def test_dns_includes_rack_controllers(self):
+        # Regression test for LP:1881133
+        vlan = factory.make_VLAN()
+        subnet = factory.make_Subnet(dns_servers=[], vlan=vlan)
+        rack = factory.make_RackController(subnet=subnet)
+        rack_iface = rack.interface_set.first()
+        rack_ip = factory.make_StaticIPAddress(
+            subnet=subnet, interface=rack_iface
+        )
+        vlan.primary_rack = rack
+        vlan.save()
+        node = factory.make_Node_with_Interface_on_Subnet(
+            status=NODE_STATUS.DEPLOYING, subnet=subnet
+        )
+        iface = node.interface_set.first()
+        factory.make_StaticIPAddress(subnet=subnet, interface=iface)
+        v2 = self._render_netplan_dict(node)
+        self.assertDictEqual(
+            {"search": ["maas"], "addresses": [rack_ip.ip]},
+            v2["network"]["ethernets"][iface.name]["nameservers"],
+        )
+
     def test_commissioning_dhcp_config(self):
         # Verifies dhcp config is given when commissioning has run
         # or just run and no AUTOIP has been acquired.