Merge ~r00ta/maas:lp-2064281-3.4 into maas:3.4

[Jacopo Rota]

fix: lp-2064281. The mac address that the rack controller communicates to the region when the machine's configuration is requested must be normalised

(cherry-picked from 5c528c657e916ec6dce9e231b54a26205a243885)

[Jacopo Rota (r00ta)]

self-approving backport

review: Approve

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp-2064281-3.4 lp:~r00ta/maas/+git/maas into -b 3.4 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 433acdc23b481edfa969a442e46cf66e41b3cb98

review: Approve

diff --git a/src/maasserver/rpc/boot.py b/src/maasserver/rpc/boot.py
index 931a7e0..940b453 100644
--- a/src/maasserver/rpc/boot.py
+++ b/src/maasserver/rpc/boot.py
@@ -363,6 +363,18 @@ def get_quirks_kernel_opts(
     return None
 
 
+def update_boot_interface_vlan(machine: Node, local_ip: str):
+    subnet = Subnet.objects.get_best_subnet_for_ip(local_ip)
+    boot_vlan = getattr(machine.boot_interface, "vlan", None)
+    if subnet and subnet.vlan != boot_vlan:
+        # This might choose the wrong interface, but we don't
+        # have enough information to decide which interface is
+        # the boot one.
+        machine.boot_interface = machine.current_config.interface_set.filter(
+            vlan=subnet.vlan
+        ).first()
+
+
 @synchronous
 @transactional
 def get_config(
@@ -446,50 +458,47 @@ def get_config(
         if machine.bios_boot_method != bios_boot_method:
             machine.bios_boot_method = bios_boot_method
 
-        try:
-            machine.boot_interface = machine.current_config.interface_set.get(
-                type=INTERFACE_TYPE.PHYSICAL, mac_address=mac
-            )
-        except ObjectDoesNotExist:
-            # MAC is unknown or wasn't sent. Determine the boot_interface using
-            # the boot_cluster_ip.
-            subnet = Subnet.objects.get_best_subnet_for_ip(local_ip)
-            boot_vlan = getattr(machine.boot_interface, "vlan", None)
-            if subnet and subnet.vlan != boot_vlan:
-                # This might choose the wrong interface, but we don't
-                # have enough information to decide which interface is
-                # the boot one.
+        if not mac:
+            # MAC was not sent. Determine the boot_interface using the boot_cluster_ip.
+            update_boot_interface_vlan(machine, local_ip)
+        else:
+            try:
                 machine.boot_interface = (
-                    machine.current_config.interface_set.filter(
-                        vlan=subnet.vlan
-                    ).first()
+                    machine.current_config.interface_set.get(
+                        type=INTERFACE_TYPE.PHYSICAL,
+                        mac_address=normalise_macaddress(mac),
+                    )
                 )
-        else:
-            # Update the VLAN of the boot interface to be the same VLAN for the
-            # interface on the rack controller that the machine communicated
-            # with, unless the VLAN is being relayed.
-            rack_interface = (
-                rack_controller.current_config.interface_set.filter(
-                    ip_addresses__ip=local_ip
+            except ObjectDoesNotExist:
+                # MAC is unknown. Determine the boot_interface using the boot_cluster_ip.
+                update_boot_interface_vlan(machine, local_ip)
+            else:
+                # Update the VLAN of the boot interface to be the same VLAN for the
+                # interface on the rack controller that the machine communicated
+                # with, unless the VLAN is being relayed.
+                rack_interface = (
+                    rack_controller.current_config.interface_set.filter(
+                        ip_addresses__ip=local_ip
+                    )
+                    .select_related("vlan")
+                    .first()
                 )
-                .select_related("vlan")
-                .first()
-            )
-            if (
-                rack_interface is not None
-                and machine.boot_interface.vlan_id != rack_interface.vlan_id
-            ):
-                # Rack controller and machine is not on the same VLAN, with
-                # DHCP relay this is possible. Lets ensure that the VLAN on the
-                # interface is setup to relay through the identified VLAN.
-                if not VLAN.objects.filter(
-                    id=machine.boot_interface.vlan_id,
-                    relay_vlan=rack_interface.vlan_id,
-                ).exists():
-                    # DHCP relay is not being performed for that VLAN. Set the
-                    # VLAN to the VLAN of the rack controller.
-                    machine.boot_interface.vlan = rack_interface.vlan
-                    machine.boot_interface.save()
+                if (
+                    rack_interface is not None
+                    and machine.boot_interface.vlan_id
+                    != rack_interface.vlan_id
+                ):
+                    # Rack controller and machine is not on the same VLAN, with
+                    # DHCP relay this is possible. Lets ensure that the VLAN on the
+                    # interface is setup to relay through the identified VLAN.
+                    if not VLAN.objects.filter(
+                        id=machine.boot_interface.vlan_id,
+                        relay_vlan=rack_interface.vlan_id,
+                    ).exists():
+                        # DHCP relay is not being performed for that VLAN. Set the
+                        # VLAN to the VLAN of the rack controller.
+                        machine.boot_interface.vlan = rack_interface.vlan
+                        machine.boot_interface.save()
 
         # Reset the machine's status_expires whenever the boot_config is called
         # on a known machine. This allows a machine to take up to the maximum
diff --git a/src/maasserver/rpc/tests/test_boot.py b/src/maasserver/rpc/tests/test_boot.py
index 6928a67..10f8c32 100644
--- a/src/maasserver/rpc/tests/test_boot.py
+++ b/src/maasserver/rpc/tests/test_boot.py
@@ -18,7 +18,7 @@ from maasserver.enum import (
     NODE_STATUS,
     NODE_TYPE,
 )
-from maasserver.models import Config, Event
+from maasserver.models import Config, Event, Machine
 from maasserver.models.timestampedmodel import now
 from maasserver.node_status import get_node_timeout, MONITORED_STATUSES
 from maasserver.preseed import compose_enlistment_preseed_url
@@ -1193,6 +1193,100 @@ class TestGetConfig(MAASServerTestCase):
         )
         self.assertEqual("hwe-20.04", observed_config["subarch"])
 
+    def test_machine_boots_from_another_vlan_and_gets_updated_when_mac_is_unknown(
+        self,
+    ):
+        make_usable_architecture(self)
+
+        vlan_10 = factory.make_VLAN(vid=10)
+        subnet_10 = factory.make_Subnet(cidr="10.0.0.0/24", vlan=vlan_10)
+
+        vlan_20 = factory.make_VLAN(vid=20)
+        subnet_20 = factory.make_Subnet(cidr="20.0.0.0/24", vlan=vlan_20)
+
+        rack = factory.make_rack_with_interfaces(
+            eth0=[subnet_10.cidr],
+            eth1=[subnet_20.cidr],
+        )
+
+        # The machine is configured to use the subnet_10
+        machine = factory.make_Machine_with_Interface_on_Subnet(
+            status=NODE_STATUS.DEPLOYING, subnet=subnet_10
+        )
+        factory.make_Interface(
+            INTERFACE_TYPE.PHYSICAL, node=machine, subnet=subnet_20
+        )
+
+        # But actually the machine boots from the other subnet
+        local_ip = (
+            rack.current_config.interface_set.filter(vlan=vlan_20)
+            .first()
+            .ip_addresses.first()
+            .ip
+        )
+        remote_ip = factory.pick_ip_in_Subnet(subnet_20)
+
+        get_config(
+            rack.system_id,
+            local_ip,
+            remote_ip,
+            hardware_uuid=machine.hardware_uuid,
+            mac=None,
+        )
+
+        reloaded_machine = Machine.objects.filter(
+            system_id=machine.system_id
+        ).first()
+
+        # The second NIC is the new boot interface
+        self.assertEqual(reloaded_machine.boot_interface.vlan.id, vlan_20.id)
+
+    def test_machine_boots_from_another_vlan_and_gets_updated(self):
+        vlan_10 = factory.make_VLAN(vid=10)
+        subnet_10 = factory.make_Subnet(cidr="10.0.0.0/24", vlan=vlan_10)
+
+        vlan_20 = factory.make_VLAN(vid=20)
+        subnet_20 = factory.make_Subnet(cidr="20.0.0.0/24", vlan=vlan_20)
+
+        rack = factory.make_rack_with_interfaces(
+            eth0=[subnet_10.cidr],
+            eth1=[subnet_20.cidr],
+        )
+
+        # The machine is configured to use the subnet_10
+        machine = factory.make_Machine_with_Interface_on_Subnet(
+            status=NODE_STATUS.DEPLOYING, subnet=subnet_10
+        )
+
+        # But actually the machine boots from the other subnet
+        local_ip = (
+            rack.current_config.interface_set.filter(vlan=vlan_20)
+            .first()
+            .ip_addresses.first()
+            .ip
+        )
+        remote_ip = factory.pick_ip_in_Subnet(subnet_20)
+
+        # The MAC address coming from the rack is always in the format XX-XX-XX-XX-XX-XX
+        unformatted_mac_address = machine.boot_interface.mac_address.replace(
+            ":", "-"
+        )
+
+        get_config(
+            rack.system_id,
+            local_ip,
+            remote_ip,
+            mac=unformatted_mac_address,
+            query_count=50,
+        )
+
+        reloaded_machine = Machine.objects.filter(
+            system_id=machine.system_id
+        ).first()
+
+        # Expect that the boot interface vlan is moved to the same rack controller's vlan
+        self.assertEqual(reloaded_machine.boot_interface.vlan.id, vlan_20.id)
+
     def test_commissioning_node_uses_min_hwe_kernel_reports_missing(self):
         factory.make_BootSourceCache(
             release="20.10",