Merge ~bjornt/maas:bug-1992330-3.2 into maas:3.2

[Björn Tillenius]

Don't return ipv6 boot addresses for ipv4 subnets.

An ipv4 subnet won't be able to use ipv6 addresses, and it would break the
DHCP config.

(cherry picked from commit 425a78cfd666e05757bf6f72e63c48bdf0f807df)

LP #1992330: Use the rack controller IP as DNS when relaying DHCP

Add relay rack ips as DNS servers
This patch aims to add the rack controll IP addresses of the relay
vlan as DNS servers.

This fixes an issue that when there are no other settings or available
IPs, MAAS adds only the IP address of the region controller, but
sometimes the region controller might not be routed to that vlan, hence
this patch, as the rack controllers should always be reachable by the
machines.

(cherry picked from commit 65a0cacf6aa37704195e0165ee69e2c516fb6936)

[Björn Tillenius (bjornt)]

Self-approve backport

review: Approve

diff --git a/src/maasserver/dhcp.py b/src/maasserver/dhcp.py
index be288ea..55f355e 100644
--- a/src/maasserver/dhcp.py
+++ b/src/maasserver/dhcp.py
@@ -23,7 +23,6 @@ from maasserver.enum import (
     INTERFACE_TYPE,
     IPADDRESS_TYPE,
     IPRANGE_TYPE,
-    NODE_TYPE,
     SERVICE_STATUS,
 )
 from maasserver.exceptions import UnresolvableHost
@@ -37,6 +36,7 @@ from maasserver.models import (
     Subnet,
     VLAN,
 )
+from maasserver.models.subnet import get_boot_rackcontroller_ips
 from maasserver.rpc import getAllClients, getClientFor, getRandomClient
 from maasserver.utils.orm import transactional
 from maasserver.utils.threads import deferToDatabase
@@ -545,37 +545,6 @@ def get_ntp_servers(ntp_servers, subnet, peer_rack):
         return ntp_servers
 
 
-def get_dns_server_addresses_for_rack(
-    rack_controller: RackController, subnet: Subnet
-) -> dict:
-    """Return a map of IP addresses suitable for DNS on subnet.
-
-    This will define a list of IP addresses across all rack controllers that
-    exist on `subnet`. The `rack_controller` will always be the first in the
-    list followed by other rack controllers that are also on the subnet.
-    """
-    addresses = StaticIPAddress.objects.filter(
-        subnet=subnet,
-        alloc_type__in={IPADDRESS_TYPE.STICKY, IPADDRESS_TYPE.USER_RESERVED},
-        interface__enabled=True,
-        interface__node_config__node__node_type__in={
-            NODE_TYPE.RACK_CONTROLLER,
-            NODE_TYPE.REGION_AND_RACK_CONTROLLER,
-        },
-    )
-    addresses = addresses.distinct()
-    addresses = addresses.values_list("ip", "interface__node_config__node_id")
-
-    def sort_key__rack__ip(record):
-        ip, node_id = record
-        return -int(node_id == rack_controller.id), IPAddress(ip)
-
-    return [
-        IPAddress(record[0])
-        for record in sorted(addresses, key=sort_key__rack__ip)
-    ]
-
-
 def get_default_dns_servers(rack_controller, subnet, use_rack_proxy=True):
     """Calculates the DNS servers on a per-subnet basis, to make sure we
     choose the best possible IP addresses for each subnet.
@@ -607,7 +576,9 @@ def get_default_dns_servers(rack_controller, subnet, use_rack_proxy=True):
     if use_rack_proxy:
         # Add the IP address for the rack controllers on the subnet before the
         # region DNS servers.
-        rack_ips = get_dns_server_addresses_for_rack(rack_controller, subnet)
+        rack_ips = [
+            IPAddress(ip) for ip in get_boot_rackcontroller_ips(subnet)
+        ]
         if dns_servers:
             dns_servers = rack_ips + [
                 server
@@ -616,7 +587,7 @@ def get_default_dns_servers(rack_controller, subnet, use_rack_proxy=True):
             ]
         elif rack_ips:
             dns_servers = rack_ips
-    elif default_region_ip in dns_servers:
+    if default_region_ip in dns_servers:
         # Make sure the region DNS server comes last
         dns_servers = [
             server for server in dns_servers if server != default_region_ip
diff --git a/src/maasserver/models/subnet.py b/src/maasserver/models/subnet.py
index d0cf030..6d9920c 100644
--- a/src/maasserver/models/subnet.py
+++ b/src/maasserver/models/subnet.py
@@ -1087,9 +1087,6 @@ def get_boot_rackcontroller_ips(subnet):
         value = 2
         if ip in network:
             value = 1
-        # Prefer addresses of the same IP version.
-        if ip.version != network.version:
-            value *= 10
         return value
 
     from maasserver.models.staticipaddress import StaticIPAddress
@@ -1107,5 +1104,13 @@ def get_boot_rackcontroller_ips(subnet):
         subnet__vlan=dhcp_vlan,
         interface__node_config__in=node_configs,
     )
-    ips = sorted((static_ip.ip for static_ip in static_ips), key=rank_ip)
+    ip_version = IPNetwork(subnet.cidr).version
+    ips = sorted(
+        (
+            static_ip.ip
+            for static_ip in static_ips
+            if IPAddress(static_ip.ip).version == ip_version
+        ),
+        key=rank_ip,
+    )
     return ips
diff --git a/src/maasserver/models/tests/test_subnet.py b/src/maasserver/models/tests/test_subnet.py
index b4d158c..a09306c 100644
--- a/src/maasserver/models/tests/test_subnet.py
+++ b/src/maasserver/models/tests/test_subnet.py
@@ -1990,7 +1990,7 @@ class TestGetBootRackcontrollerIPs(MAASServerTestCase):
         boot_ips = get_boot_rackcontroller_ips(relay_subnet)
         self.assertCountEqual(["10.10.0.2", "10.10.1.2"], boot_ips)
 
-    def test_with_relay_prefer_ipv4(self):
+    def test_with_relay_mixed_ipv4_subnet(self):
         dhcp_vlan = factory.make_VLAN(
             dhcp_on=False,
             primary_rack=None,
@@ -2017,9 +2017,9 @@ class TestGetBootRackcontrollerIPs(MAASServerTestCase):
         relay_vlan.relay_vlan = dhcp_vlan
         relay_vlan.save()
         boot_ips = get_boot_rackcontroller_ips(relay_subnet)
-        self.assertEqual(["10.10.0.2", "fd12:3456:789a::2"], boot_ips)
+        self.assertEqual(["10.10.0.2"], boot_ips)
 
-    def test_with_relay_prefer_ipv6(self):
+    def test_with_relay_prefer_mixed_ipv6_subnet(self):
         dhcp_vlan = factory.make_VLAN(
             dhcp_on=False,
             primary_rack=None,
@@ -2046,4 +2046,4 @@ class TestGetBootRackcontrollerIPs(MAASServerTestCase):
         relay_vlan.relay_vlan = dhcp_vlan
         relay_vlan.save()
         boot_ips = get_boot_rackcontroller_ips(relay_subnet)
-        self.assertEqual(["fd12:3456:789a::2", "10.10.0.2"], boot_ips)
+        self.assertEqual(["fd12:3456:789a::2"], boot_ips)
diff --git a/src/maasserver/tests/test_dhcp.py b/src/maasserver/tests/test_dhcp.py
index fa482a7..5e7f347 100644
--- a/src/maasserver/tests/test_dhcp.py
+++ b/src/maasserver/tests/test_dhcp.py
@@ -1118,9 +1118,9 @@ class TestGetDefaultDNSServers(MAASServerTestCase):
         # Regression test for LP:1881133
         mock_get_source_address = self.patch(dhcp, "get_source_address")
         mock_get_source_address.return_value = "192.168.122.209"
-        vlan = factory.make_VLAN()
-        subnet = factory.make_Subnet(vlan=vlan, cidr="192.168.200.0/24")
         rack = factory.make_RackController(interface=False)
+        vlan = factory.make_VLAN(dhcp_on=True, primary_rack=rack)
+        subnet = factory.make_Subnet(vlan=vlan, cidr="192.168.200.0/24")
         iface = factory.make_Interface(
             INTERFACE_TYPE.PHYSICAL, vlan=vlan, node=rack
         )
@@ -1134,6 +1134,46 @@ class TestGetDefaultDNSServers(MAASServerTestCase):
         servers = get_default_dns_servers(rack, subnet)
         self.assertThat(servers, Equals([IPAddress("192.168.200.1")]))
 
+    def test_dns_servers_when_relaying(self):
+        mock_get_source_address = self.patch(dhcp, "get_source_address")
+        mock_get_source_address.return_value = "10.20.30.1"
+        rack = factory.make_RackController(interface=False)
+        vlan1_primary = factory.make_VLAN(dhcp_on=True, primary_rack=rack)
+        vlan2_secondary = factory.make_VLAN(
+            primary_rack=rack, dhcp_on=False, relay_vlan=vlan1_primary
+        )
+        subnet1 = factory.make_Subnet(
+            vlan=vlan1_primary, vid=10, cidr="10.20.40.0/24"
+        )
+        subnet2 = factory.make_Subnet(
+            vlan=vlan2_secondary, vid=20, cidr="10.20.50.0/24"
+        )
+        rack_interface = factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=rack
+        )
+        rack_interface_vlan1 = factory.make_Interface(
+            INTERFACE_TYPE.VLAN,
+            vlan=vlan1_primary,
+            parents=[rack_interface],
+            node=rack,
+        )
+        factory.make_Interface(
+            INTERFACE_TYPE.VLAN,
+            vlan=vlan2_secondary,
+            parents=[rack_interface],
+            node=rack,
+        )
+        factory.make_StaticIPAddress(
+            interface=rack_interface_vlan1,
+            subnet=subnet1,
+            alloc_type=IPADDRESS_TYPE.STICKY,
+            ip="10.20.40.1",
+        )
+        dns_servers = dhcp.get_default_dns_servers(
+            rack, subnet2, use_rack_proxy=True
+        )
+        self.assertEqual(dns_servers, [IPAddress("10.20.40.1")])
+
     def test_no_default_region_ip(self):
         self.patch(dhcp, "get_source_address").return_value = None
         vlan = factory.make_VLAN()