Merge ~ltrager/maas:lp1874917 into maas:master

[Lee Trager]

LP: #1874917 - Skip network data from Controllers and Pods if duplicate MACs detected.

Controllers and Pods send commissioning information from a deployed
machine. If the machine is using a bond multiple interfaces will use
the same MAC address. Since MAAS identifies interfaces by MAC skip
updating interface information.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1874917 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/7457/console
COMMIT: 53f6e118aaa20cfb7a70375443dc757ccafd2e5a

review: Needs Fixing

5476114... by Lee Trager on 2020-04-25

Fix division by 0 error

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1874917 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 5476114b203d32a6d2e5391b98b1dfa1c616f6ad

review: Approve

00efbf6... by Lee Trager on 2020-04-27

Fix showing system_id for assoicated machine.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1874917 lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 00efbf67dd2d36e2ecd5fed61807f9271e035f10

review: Approve

[Newell Jensen (newell-jensen)]

LGTM

review: Approve

diff --git a/src/maasserver/models/bmc.py b/src/maasserver/models/bmc.py
index a3aef32..cde8004 100644
--- a/src/maasserver/models/bmc.py
+++ b/src/maasserver/models/bmc.py
@@ -642,7 +642,8 @@ class Pod(BMC):
             interface = self.ip_address.get_interface()
             if interface is not None:
                 return interface.node
-        return None
+        else:
+            return self.hints.nodes.first()
 
     def sync_hints(self, discovered_hints):
         """Sync the hints with `discovered_hints`."""
diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py
index 801a4c1..ee5bd82 100644
--- a/src/metadataserver/builtin_scripts/hooks.py
+++ b/src/metadataserver/builtin_scripts/hooks.py
@@ -35,6 +35,10 @@ from provisioningserver.utils.ipaddr import parse_ip_addr
 logger = logging.getLogger(__name__)
 
 
+class DuplicateMACs(Exception):
+    """Exception for when duplicate MAC addresses are in commissioning data."""
+
+
 SWITCH_TAG_NAME = "switch"
 SWITCH_HARDWARE = [
     # Seen on Facebook Wedge 40 switch:
@@ -143,7 +147,13 @@ def _parse_interfaces(node, data):
             link = ip_addr_info.get(interface["name"], {})
             interface["ips"] = link.get("inet", []) + link.get("inet6", [])
 
-            interfaces[mac] = interface
+            if mac in interfaces:
+                raise DuplicateMACs(
+                    f"Duplicated MAC address({mac}) on "
+                    "{interfaces[mac]['name']} and {interface['name']}"
+                )
+            else:
+                interfaces[mac] = interface
 
     return interfaces
 
@@ -248,7 +258,21 @@ def update_node_network_information(node, data, numa_nodes):
         node.save(update_fields=["skip_networking"])
         return
 
-    interfaces_info = _parse_interfaces(node, data)
+    try:
+        interfaces_info = _parse_interfaces(node, data)
+    except DuplicateMACs:
+        if node.is_controller or node.is_pod:
+            # Controllers and Pods send commissioning information from a
+            # deployed machine. If the machine is using a bond multiple
+            # interfaces will use the same MAC address. Since MAAS identifies
+            # interfaces by MAC skip updating interface information.
+            # When MAAS gains the ability to model LXD's commissioning
+            # information this can be removed.
+            return
+        else:
+            # Duplicate MACs are not expected on machines, raise the
+            # exception so this can be handled.
+            raise
     current_interfaces = set()
 
     for mac, iface in interfaces_info.items():
@@ -496,18 +520,17 @@ def _parse_cpuinfo(data):
             current_average = 0
             for socket in sockets:
                 current_average += socket.get("frequency", 0)
-            current_average /= len(sockets)
             if current_average:
                 # Fall back on the current speed, round it to
                 # the nearest hundredth as the number may be
                 # effected by CPU scaling.
+                current_average /= len(sockets)
                 cpu_speed = round(current_average / 100) * 100
 
     return cpu_count, cpu_speed, cpu_model, numa_nodes
 
 
 def _parse_memory(data, numa_nodes):
-
     total_memory = data.get("memory", {}).get("total", 0)
     default_numa_node = {"numa_node": 0, "total": total_memory}
     for memory_node in data.get("memory", {}).get(
diff --git a/src/metadataserver/builtin_scripts/tests/test_hooks.py b/src/metadataserver/builtin_scripts/tests/test_hooks.py
index 79474a2..c393f7d 100644
--- a/src/metadataserver/builtin_scripts/tests/test_hooks.py
+++ b/src/metadataserver/builtin_scripts/tests/test_hooks.py
@@ -2414,6 +2414,59 @@ class TestUpdateNodeNetworkInformation(MAASServerTestCase):
         self.assertIsNotNone(reload_object(boot_interface))
         self.assertFalse(reload_object(node).skip_networking)
 
+    def test__does_nothing_if_duplicate_mac_on_controller(self):
+        mock_iface_get = self.patch(
+            hooks_module.PhysicalInterface.objects, "get"
+        )
+        rack = factory.make_RackController()
+        create_IPADDR_OUTPUT_NAME_script(rack, IP_ADDR_OUTPUT)
+        mac = factory.make_mac_address()
+        duplicate_mac_lxd_resources = deepcopy(SAMPLE_LXD_RESOURCES)
+        for card in duplicate_mac_lxd_resources["network"]["cards"]:
+            for port in card["ports"]:
+                port["address"] = mac
+        update_node_network_information(
+            rack, duplicate_mac_lxd_resources, create_numa_nodes(rack)
+        )
+        self.assertThat(mock_iface_get, MockNotCalled())
+
+    def test__does_nothing_if_duplicate_mac_on_pod(self):
+        self.useFixture(SignalsDisabled("podhints"))
+        mock_iface_get = self.patch(
+            hooks_module.PhysicalInterface.objects, "get"
+        )
+        pod = factory.make_Pod()
+        node = factory.make_Node(
+            status=NODE_STATUS.DEPLOYED, with_empty_script_sets=True
+        )
+        create_IPADDR_OUTPUT_NAME_script(node, IP_ADDR_OUTPUT)
+        pod.hints.nodes.add(node)
+        mac = factory.make_mac_address()
+        duplicate_mac_lxd_resources = deepcopy(SAMPLE_LXD_RESOURCES)
+        for card in duplicate_mac_lxd_resources["network"]["cards"]:
+            for port in card["ports"]:
+                port["address"] = mac
+        update_node_network_information(
+            node, duplicate_mac_lxd_resources, create_numa_nodes(node)
+        )
+        self.assertThat(mock_iface_get, MockNotCalled())
+
+    def test__raises_exception_if_duplicate_mac_on_machine(self):
+        node = factory.make_Node()
+        create_IPADDR_OUTPUT_NAME_script(node, IP_ADDR_OUTPUT)
+        mac = factory.make_mac_address()
+        duplicate_mac_lxd_resources = deepcopy(SAMPLE_LXD_RESOURCES)
+        for card in duplicate_mac_lxd_resources["network"]["cards"]:
+            for port in card["ports"]:
+                port["address"] = mac
+        self.assertRaises(
+            hooks_module.DuplicateMACs,
+            update_node_network_information,
+            node,
+            duplicate_mac_lxd_resources,
+            create_numa_nodes(node),
+        )
+
     def test__add_all_interfaces(self):
         """Test a node that has no previously known interfaces on which we
         need to add a series of interfaces.