Merge ~andreserl/maas:lp1548402 into maas:master

[Andres Rodriguez]

LP: #1548402 - Handle errors in preseeds

Ensure that MAAS prevents starting the deployment of a machine if the preseeds fail to render.

68948d1... by Andres Rodriguez on 2017-07-08

Fix format

[Andres Rodriguez (andreserl)]

So I have a few questions here:

1. Should we prevent if all the curtin preseed rendering fails, or only if curtin_userdata ?
2. Raising the error via the API forms causes an issue we have discussed before, which is some errors so in json format (like {"distro_series": ["'asdfadsf' is not a valid distro_series... ']} ) but this would show as plain text. Since we discussed plain output vs structured output... how should we handle this?
3. Why do we have different actions for the API vs the UI? It would seem to me that deploying via the UI should be done exactly like the API.

[Newell Jensen (newell-jensen)]

Seems like from your comment above that you are uncertain about how to handle this but I have reviewed what you have below based on the code itself. You are missing unit tests and I also made a suggestion.

review: Needs Fixing

[Blake Rouse (blake-rouse)]

I think this is good.

The reason for doing it in 2 places is really just legacy. We have been doing it that way from the beginning as for why I don't know as that pre-dates me. I think it would be better to combine them of-course, but that could cause more bugs.

review: Approve

[Newell Jensen (newell-jensen)]

> I think this is good.
>
> The reason for doing it in 2 places is really just legacy. We have been doing
> it that way from the beginning as for why I don't know as that pre-dates me. I
> think it would be better to combine them of-course, but that could cause more
> bugs.

Still needs tests.

[Blake Rouse (blake-rouse)]

Ah true, missing tests.

review: Needs Fixing

[Andres Rodriguez (andreserl)]

This was already landed by another branch.

diff --git a/src/maasserver/api/machines.py b/src/maasserver/api/machines.py
index 58d9dfd..dfca107 100644
--- a/src/maasserver/api/machines.py
+++ b/src/maasserver/api/machines.py
@@ -443,6 +443,13 @@ class MachineHandler(NodeHandler, OwnerDataMixin, PowerMixin):
             form.save()
         else:
             raise MAASAPIValidationError(form.errors)
+        # Check that the curtin preseeds renders correctly.
+        try:
+            get_curtin_merged_config(machine)
+        except Exception as e:
+            raise MAASAPIBadRequest(
+                "Failed to render preseed: %s" % e)
+
         return self.power_on(request, system_id)
 
     @operation(idempotent=False)
diff --git a/src/maasserver/node_action.py b/src/maasserver/node_action.py
index fc48bb0..2a064b4 100644
--- a/src/maasserver/node_action.py
+++ b/src/maasserver/node_action.py
@@ -44,6 +44,7 @@ from maasserver.node_status import (
     is_failed_status,
     NON_MONITORED_STATUSES,
 )
+from maasserver.preseed import get_curtin_config
 from maasserver.utils.orm import post_commit_do
 from maasserver.utils.osystems import (
     validate_hwe_kernel,
@@ -346,6 +347,11 @@ class Deploy(NodeAction):
             raise NodeActionError(e)
 
         try:
+            get_curtin_config(self.node)
+        except Exception as e:
+            raise NodeActionError(e)
+
+        try:
             self.node.start(self.user)
         except StaticIPAddressExhaustion:
             raise NodeActionError(