Merge ~ltrager/maas:lp1893690 into maas:master

[Lee Trager]

LP: #1893690 - Remove duplicate hardware UUIDs when detected.

Some Dell hardware uses the service number for the hardware UUID. This
value is not unqiue and causes commissioning to fail. MAAS uses the
hardware UUID to identify the machine when booting. Some hardware like
IBM Z series LPARs only identify themselves this way while PXE booting
tries the UUID first and then falls back on using a MAC address. If a
duplicate is detected remove the UUID from both machines.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1893690 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/8210/console
COMMIT: f56a2bf05ee8dd2dc9c42607f7dd622f440a60b1

review: Needs Fixing

[Dougal Matthews (d0ugal)]

review: Needs Information

[Dougal Matthews (d0ugal)]

Lee explained there is a unique constraint, so the potential issue I raised can't happen.

LGTM

review: Approve

0fe11d1... by Lee Trager on 2020-09-01

Validate UUID

[MAAS Lander (maas-lander)]

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

STATUS: SUCCESS
COMMIT: 0fe11d17f823c94a1236fe77a60ecb55471e8cf0

review: Approve

794cf00... by Lee Trager on 2020-09-01

Check duplicate UUIDs atomically

[MAAS Lander (maas-lander)]

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

STATUS: SUCCESS
COMMIT: 794cf008a4327f161461bdc475c6f994950c3863

review: Approve

diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py
index 26c7e11..4538dce 100644
--- a/src/metadataserver/builtin_scripts/hooks.py
+++ b/src/metadataserver/builtin_scripts/hooks.py
@@ -10,8 +10,10 @@ import json
 import logging
 import re
 
+from django.core.exceptions import ValidationError
+
 from maasserver.enum import NODE_METADATA, NODE_STATUS
-from maasserver.models import Fabric, NUMANode, Subnet
+from maasserver.models import Fabric, Node, NUMANode, Subnet
 from maasserver.models.blockdevice import MIN_BLOCK_DEVICE_SIZE
 from maasserver.models.interface import Interface, PhysicalInterface
 from maasserver.models.nodemetadata import NodeMetadata
@@ -414,8 +416,29 @@ def _process_system_information(node, system_data):
             NodeMetadata.objects.filter(node=node, key=key).delete()
 
     uuid = system_data.get("uuid")
-    # Convert "" to None, so that the unique check isn't triggered.
-    node.hardware_uuid = None if uuid == "" else uuid
+    if not uuid or not re.search(
+        r"^[\da-f]{8}[\-]?([\da-f]{4}[\-]?){3}[\da-f]{12}$", uuid, re.I
+    ):
+        # Convert "" to None, so that the unique check isn't triggered.
+        # Some vendors store the service tag as the UUID which is not unique,
+        # if the UUID isn't a valid UUID ignore it.
+        node.hardware_uuid = None
+    else:
+        # LP:1893690 - If the UUID is valid check that it isn't duplicated
+        # with save so the check is atomic.
+        node.hardware_uuid = uuid
+        try:
+            node.save()
+        except ValidationError as e:
+            # Check that the ValidationError is due to the hardware_uuid
+            # other errors will be caught and logged later.
+            if "hardware_uuid" in e.error_dict:
+                node.hardware_uuid = None
+                # If the UUID isn't really unique make sure it isn't stored on
+                # any Node.
+                Node.objects.filter(hardware_uuid=uuid).update(
+                    hardware_uuid=None
+                )
 
     # Gather system information. Custom built machines and some Supermicro
     # servers do not provide this information.
diff --git a/src/metadataserver/builtin_scripts/tests/test_hooks.py b/src/metadataserver/builtin_scripts/tests/test_hooks.py
index 29d0857..2bfa72d 100644
--- a/src/metadataserver/builtin_scripts/tests/test_hooks.py
+++ b/src/metadataserver/builtin_scripts/tests/test_hooks.py
@@ -1273,6 +1273,22 @@ class TestProcessLXDResults(MAASServerTestCase):
         # check for unique UUIDs isn't triggered.
         self.assertIsNone(node.hardware_uuid)
 
+    def test_removes_duplicate_uuid(self):
+        uuid = factory.make_UUID()
+        duplicate_uuid_node = factory.make_Node(hardware_uuid=uuid)
+        node = factory.make_Node()
+        create_IPADDR_OUTPUT_NAME_script(node, IP_ADDR_OUTPUT)
+        process_lxd_results(node, make_lxd_output_json(uuid=uuid), 0)
+        self.assertIsNone(reload_object(node).hardware_uuid)
+        self.assertIsNone(reload_object(duplicate_uuid_node).hardware_uuid)
+
+    def test_ignores_invalid_uuid(self):
+        uuid = factory.make_name("invalid_uuid")
+        node = factory.make_Node()
+        create_IPADDR_OUTPUT_NAME_script(node, IP_ADDR_OUTPUT)
+        process_lxd_results(node, make_lxd_output_json(uuid=uuid), 0)
+        self.assertIsNone(reload_object(node).hardware_uuid)
+
     def test_sets_os_hostname_for_controller(self):
         rack = factory.make_RackController()
         create_IPADDR_OUTPUT_NAME_script(rack, IP_ADDR_OUTPUT)