Merge ~ack/maas:1927667-update-interface-numanode-2.9 into maas:2.9

[Alberto Donato]

LP: #1927667 - update network interfaces numa node from resource data

[MAAS Lander (maas-lander)]

UNIT TESTS
-b 1927667-update-interface-numanode-2.9 lp:~ack/maas/+git/maas into -b 2.9 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/10085/console
COMMIT: 38ffe485e11d5365c86c18bde919b0ead6b73571

review: Needs Fixing

[MAAS Lander (maas-lander)]

UNIT TESTS
-b 1927667-update-interface-numanode-2.9 lp:~ack/maas/+git/maas into -b 2.9 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: d58f534dd9cf98e2191941478b3ff26111d07a05

review: Approve

[Lee Trager (ltrager)]

The initial caller, update_node_network_information, already has the NUMA nodes loaded in memory. I think that should be passed to update_node_interfaces which passes it to update_interface_details so MAAS doesn't have to do additional database lookups.

review: Needs Fixing

[Alberto Donato (ack)]

@Lee thanks for the review.

Passing in the NUMANode objects through those functions is quite a big change, since update_node_interfaces is used in a bunch of places.

To keep the change minimal, since this fix is only needed in 2.9 (3.0 reworks the logic and already updates the numanode elsewhere) I've made a change to pass the mapping between indexes and IDs to update_interface_details, so a single extra query is made to get those.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b 1927667-update-interface-numanode-2.9 lp:~ack/maas/+git/maas into -b 2.9 lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/10094/console
COMMIT: ef20a3be0d2663f3aacee153e3f286afafb9d874

review: Needs Fixing

[Alberto Donato (ack)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b 1927667-update-interface-numanode-2.9 lp:~ack/maas/+git/maas into -b 2.9 lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: ef20a3be0d2663f3aacee153e3f286afafb9d874

review: Approve

[Lee Trager (ltrager)]

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b 1927667-update-interface-numanode-2.9 lp:~ack/maas/+git/maas into -b 2.9 lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/10105/consoleText

[MAAS Lander (maas-lander)]

LANDING
-b 1927667-update-interface-numanode-2.9 lp:~ack/maas/+git/maas into -b 2.9 lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/10107/consoleText

[MAAS Lander (maas-lander)]

LANDING
-b 1927667-update-interface-numanode-2.9 lp:~ack/maas/+git/maas into -b 2.9 lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/10108/consoleText

diff --git a/src/maasserver/models/node.py b/src/maasserver/models/node.py
index 1d5b74f..14c3119 100644
--- a/src/maasserver/models/node.py
+++ b/src/maasserver/models/node.py
@@ -6783,12 +6783,13 @@ class Controller(Node):
             VLAN. Otherwise, creates the interfaces but does not create any
             links or VLANs.
         """
-        # Avoid circular imports
         from metadataserver.builtin_scripts.hooks import (
             parse_interfaces_details,
             update_interface_details,
         )
 
+        numa_ids_map = dict(self.numanode_set.values_list("index", "id"))
+
         # Get all of the current interfaces on this node.
         current_interfaces = {
             interface.id: interface
@@ -6825,7 +6826,9 @@ class Controller(Node):
             if interface is not None:
                 interface.update_discovery_state(discovery_mode, settings)
                 if interface.type == INTERFACE_TYPE.PHYSICAL:
-                    update_interface_details(interface, interfaces_details)
+                    update_interface_details(
+                        interface, interfaces_details, numa_ids_map
+                    )
                 if interface.id in current_interfaces:
                     del current_interfaces[interface.id]
 
diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py
index 96d7d12..d89dc75 100644
--- a/src/metadataserver/builtin_scripts/hooks.py
+++ b/src/metadataserver/builtin_scripts/hooks.py
@@ -199,11 +199,12 @@ def parse_interfaces_details(node):
     return _parse_interfaces(node, details)
 
 
-def update_interface_details(interface, details):
+def update_interface_details(interface, details, numa_ids_map):
     """Update details for an existing interface from commissioning data.
 
-    This should be passed details from the _parse_interfaces call.
-
+    :params details: details from the _parse_interfaces call
+    :params numa_ids_map: dict mapping numa node indexes to their IDs for the
+        node the interface belongs to
     """
     iface_details = details.get(interface.mac_address)
     if not iface_details:
@@ -223,10 +224,19 @@ def update_interface_details(interface, details):
             setattr(interface, field, value)
         update_fields.append(field)
 
+    numa_node_idx = iface_details["numa_node"]
+    if (
+        interface.numa_node is None
+        or interface.numa_node.index != numa_node_idx
+    ):
+        interface.numa_node_id = numa_ids_map[numa_node_idx]
+        update_fields.append("numa_node")
+
     sriov_max_vf = iface_details.get("sriov_max_vf")
     if interface.sriov_max_vf != sriov_max_vf:
         interface.sriov_max_vf = sriov_max_vf
         update_fields.append("sriov_max_vf")
+
     if update_fields:
         interface.save(update_fields=["updated", *update_fields])
 
@@ -295,8 +305,12 @@ def update_node_network_information(node, data, numa_nodes):
             # Duplicate MACs are not expected on machines, raise the
             # exception so this can be handled.
             raise
-    current_interfaces = set()
 
+    numa_ids_map = dict(
+        NUMANode.objects.filter(node=node).values_list("index", "id")
+    )
+
+    current_interfaces = set()
     for mac, iface in interfaces_info.items():
         ifname = iface.get("name")
         link_connected = iface.get("link_connected")
@@ -313,7 +327,9 @@ def update_node_network_information(node, data, numa_nodes):
             if interface.node == node:
                 # Interface already exists on this node, so just update the NIC
                 # info
-                update_interface_details(interface, interfaces_info)
+                update_interface_details(
+                    interface, interfaces_info, numa_ids_map
+                )
             else:
                 logger.warning(
                     "Interface with MAC %s moved from node %s to %s. "
diff --git a/src/metadataserver/builtin_scripts/tests/test_hooks.py b/src/metadataserver/builtin_scripts/tests/test_hooks.py
index bac7d2a..bb11f5f 100644
--- a/src/metadataserver/builtin_scripts/tests/test_hooks.py
+++ b/src/metadataserver/builtin_scripts/tests/test_hooks.py
@@ -1675,6 +1675,20 @@ class TestProcessLXDResults(MAASServerTestCase):
         self.assertEqual(0, iface3.link_speed)
         self.assertEqual(0, iface3.interface_speed)
 
+    def test_updates_interface_numa_node(self):
+        node = factory.make_Node()
+        iface = factory.make_Interface(
+            node=node,
+            mac_address="00:00:00:00:00:01",
+        )
+        create_IPADDR_OUTPUT_NAME_script(node, IP_ADDR_OUTPUT)
+
+        lxd_output = make_lxd_output()
+        lxd_output["resources"]["network"]["cards"][0]["numa_node"] = 1
+        process_lxd_results(node, json.dumps(lxd_output).encode(), 0)
+        iface1 = reload_object(iface)
+        self.assertEqual(iface1.numa_node.index, 1)
+
     def test_ipaddr_script_before(self):
         self.assertLess(
             IPADDR_OUTPUT_NAME,