Merge ~adam-collard/maas:clone-errors-system-id into maas:master

[Adam Collard]

Add system_id to clone errors

Annotate JSON from Django form errors with system_id where we have it.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b clone-errors-system-id lp:~adam-collard/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: edf98d2291446c57933473c6183748c71c2b98e2

review: Approve

[Alberto Donato (ack)]

nice! +1

small nit inline

review: Approve

diff --git a/src/maasserver/node_action.py b/src/maasserver/node_action.py
index 2162065..af16a5f 100644
--- a/src/maasserver/node_action.py
+++ b/src/maasserver/node_action.py
@@ -15,6 +15,7 @@ order as they do in `ACTION_CLASSES`.
 
 from abc import ABCMeta, abstractmethod, abstractproperty
 from collections import OrderedDict
+import json
 
 from crochet import TimeoutError
 from django.core.exceptions import ValidationError
@@ -969,6 +970,25 @@ class Clone(NodeAction):
     def get_node_action_audit_description(self, action):
         return self.audit_description % action.node.hostname
 
+    def _format_field_errors_as_json(self, field_errors):
+        rich_json_data = []
+        base_data = field_errors.as_data()
+        field_json = field_errors.get_json_data()
+        for error, error_json in zip(base_data, field_json):
+            # Annotate base JSON data with specifics
+            if error.params and "system_id" in error.params:
+                error_json["system_id"] = error.params["system_id"]
+            rich_json_data.append(error_json)
+        return rich_json_data
+
+    def _format_errors_as_json(self, error_dict):
+        return json.dumps(
+            {
+                field: self._format_field_errors_as_json(error)
+                for field, error in error_dict.items()
+            }
+        )
+
     def _execute(
         self,
         destinations=None,
@@ -996,18 +1016,18 @@ class Clone(NodeAction):
                     _error_data=form.errors,
                 )
         if not form.is_valid():
-            raise NodeActionError(form.errors.as_json())
+            raise NodeActionError(self._format_errors_as_json(form.errors))
         try:
             form.save()
         except ValidationError as exc:
-            raise NodeActionError(exc.errors.as_json())
+            raise NodeActionError(self._format_errors_as_json(exc.errors))
         if _error_data:
             for name, error_list in _error_data.items():
                 if name in form.errors:
                     form.errors[name].extend(error_list)
                 else:
                     form.errors[name] = error_list
-            raise NodeActionError(form.errors.as_json())
+            raise NodeActionError(self._format_errors_as_json(form.errors))
 
 
 ACTION_CLASSES = (
diff --git a/src/maasserver/tests/test_node_action.py b/src/maasserver/tests/test_node_action.py
index a47d046..897d5f9 100644
--- a/src/maasserver/tests/test_node_action.py
+++ b/src/maasserver/tests/test_node_action.py
@@ -1860,6 +1860,41 @@ class TestCloneAction(MAASServerTestCase):
             audit_event.description, f"Cloning from '{source.hostname}'."
         )
 
+    def test_clone_errors_include_system_id(self):
+        user = factory.make_admin()
+        request = factory.make_fake_request("/")
+        request.user = user
+        source = factory.make_Machine(
+            status=NODE_STATUS.READY, with_boot_disk=False
+        )
+        factory.make_PhysicalBlockDevice(
+            node=source, size=8 * 1024 ** 3, name="sda"
+        )
+        destination = factory.make_Machine(
+            status=NODE_STATUS.READY, with_boot_disk=False
+        )
+        factory.make_PhysicalBlockDevice(
+            node=destination, size=4 * 1024 ** 3, name="sda"
+        )
+        action = Clone(source, user, request)
+        exception = self.assertRaises(
+            NodeActionError,
+            action.execute,
+            destinations=[destination.system_id],
+            storage=True,
+        )
+        (error,) = exception.args
+        self.assertEqual(
+            json.loads(error)["destinations"],
+            [
+                {
+                    "code": "storage",
+                    "message": f"{destination} is invalid: destination boot disk(sda) is smaller than source boot disk(sda)",
+                    "system_id": destination.system_id,
+                }
+            ],
+        )
+
 
 class TestActionsErrorHandling(MAASServerTestCase):
     """Tests for error handling in actions.
diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py
index c871158..fb32526 100644
--- a/src/maasserver/websockets/handlers/tests/test_machine.py
+++ b/src/maasserver/websockets/handlers/tests/test_machine.py
@@ -3705,7 +3705,7 @@ class TestMachineHandler(MAASServerTestCase):
             node.distro_series, Equals(osystem["releases"][0]["name"])
         )
 
-    def test_action_clone_errors_bundled(self):
+    def test_clone_errors_bundled(self):
         user = factory.make_admin()
         node = factory.make_Node()
 
@@ -3742,7 +3742,7 @@ class TestMachineHandler(MAASServerTestCase):
             },
         )
 
-    def test_action_clone_errors_storage(self):
+    def test_clone_errors_storage(self):
         user = factory.make_admin()
         request = HttpRequest()
         request.user = user
@@ -3789,10 +3789,12 @@ class TestMachineHandler(MAASServerTestCase):
                     {
                         "message": f"{destination1} is invalid: destination boot disk(sda) is smaller than source boot disk(sda)",
                         "code": "storage",
+                        "system_id": destination1.system_id,
                     },
                     {
                         "message": f"{destination2} is invalid: destination boot disk(sda) is smaller than source boot disk(sda)",
                         "code": "storage",
+                        "system_id": destination2.system_id,
                     },
                 ],
             },
@@ -3860,6 +3862,7 @@ class TestMachineHandler(MAASServerTestCase):
                     {
                         "message": f"{destination1} is invalid: destination boot disk(sda) is smaller than source boot disk(sda)",
                         "code": "storage",
+                        "system_id": destination1.system_id,
                     },
                 ],
             },