Merge lp:~julian-edwards/maas/add-node-crash-bug-1305061 into lp:~maas-committers/maas/trunk

[Julian Edwards]

Ensure that validation errors are returned when adding a node over the API and its cluster controller is not contactable. Previously, the appserver thread crashed and returned a 500 to the client.

[Raphaël Badin (rvb)]

Looks good. Just one remark:

[Raphaël Badin (rvb)]

(I just tried the new inline-comments feature, see my comment in the diff.)

review: Approve

=== modified file 'src/maasserver/forms.py'
--- src/maasserver/forms.py	2014-04-25 18:03:38 +0000
+++ src/maasserver/forms.py	2014-05-14 07:17:47 +0000
@@ -425,7 +425,7 @@
                     [CLUSTER_NOT_AVAILABLE + e.args[0]])
         # If power_type is not set and power_parameters_skip_check is not
         # on, reset power_parameters (set it to the empty string).
-        no_power_type = cleaned_data['power_type'] == ''
+        no_power_type = cleaned_data.get('power_type', '') == ''
         if no_power_type and not skip_check:
             cleaned_data['power_parameters'] = ''
         return cleaned_data

=== modified file 'src/maasserver/tests/test_api_nodes.py'
--- src/maasserver/tests/test_api_nodes.py	2014-03-06 05:33:46 +0000
+++ src/maasserver/tests/test_api_nodes.py	2014-05-14 07:17:47 +0000
@@ -19,12 +19,14 @@
 import random
 
 from django.core.urlresolvers import reverse
+from maasserver import forms
 from maasserver.enum import (
     NODE_STATUS,
     NODE_STATUS_CHOICES_DICT,
     NODEGROUP_STATUS,
     NODEGROUPINTERFACE_MANAGEMENT,
     )
+from maasserver.exceptions import ClusterUnavailable
 from maasserver.fields import MAC
 from maasserver.models import Node
 from maasserver.models.user import (
@@ -175,6 +177,32 @@
             NODE_STATUS.DECLARED,
             Node.objects.get(system_id=system_id).status)
 
+    def test_POST_new_when_no_RPC_to_cluster_defaults_empty_power(self):
+        # Test for bug 1305061, if there is no cluster RPC connection
+        # then make sure that power_type is defaulted to the empty
+        # string rather than being entirely absent, which results in a
+        # crash.
+        cluster_error = factory.make_name("cluster error")
+        self.patch(forms, 'get_power_types').side_effect = (
+            ClusterUnavailable(cluster_error))
+        self.become_admin()
+        # The patching behind the scenes to avoid *real* RPC is
+        # complex and the available power types is actually a
+        # valid set, so use an invalid type to trigger the bug here.
+        power_type = factory.make_name("power_type")
+        response = self.client.post(
+            reverse('nodes_handler'),
+            {
+                'op': 'new',
+                'autodetect_nodegroup': '1',
+                'architecture': make_usable_architecture(self),
+                'mac_addresses': ['aa:bb:cc:dd:ee:ff'],
+                'power_type': power_type,
+            })
+        self.assertEqual(httplib.BAD_REQUEST, response.status_code)
+        validation_errors = json.loads(response.content)['power_type']
+        self.assertIn(cluster_error, validation_errors[0])
+
     def test_GET_list_lists_nodes(self):
         # The api allows for fetching the list of Nodes.
         node1 = factory.make_node()