Merge lp:~mpontillo/maas/bug-1449396-lets-not-power-query-devices into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Fix for bug #1449396: Don't try to perform power queries on non-installable nodes

[Mike Pontillo (mpontillo)]

I think this is ready for another look.

[Gavin Panella (allenap)]

Looks good!

review: Approve

=== modified file 'src/maasserver/api/nodes.py'
--- src/maasserver/api/nodes.py	2015-04-21 22:51:42 +0000
+++ src/maasserver/api/nodes.py	2015-04-30 14:48:22 +0000
@@ -651,11 +651,17 @@
         :return: a dict whose key is "state" with a value of one of
             'on' or 'off'.
 
+        Returns 400 if the node is not installable.
         Returns 404 if the node is not found.
         Returns 503 (with explanatory text) if the power state could not
         be queried.
         """
         node = get_object_or_404(Node, system_id=system_id)
+        if not node.installable:
+            raise MAASAPIBadRequest(
+                "%s: Unable to query power state; not an installable node" %
+                node.hostname)
+
         ng = node.nodegroup
 
         try:

=== modified file 'src/maasserver/api/tests/test_nodes.py'
--- src/maasserver/api/tests/test_nodes.py	2015-04-10 09:38:25 +0000
+++ src/maasserver/api/tests/test_nodes.py	2015-04-30 14:48:22 +0000
@@ -1871,6 +1871,12 @@
         self.assertResponseCode(httplib.SERVICE_UNAVAILABLE, response)
         self.assertIn(error_message, response.content)
 
+    def test__returns_400_if_device(self):
+        device = factory.make_Device()
+        response = self.client.get(
+            self.get_node_uri(device), {"op": "query_power_state"})
+        self.assertResponseCode(httplib.BAD_REQUEST, response)
+
     def test__returns_actual_state(self):
         node = factory.make_Node(power_type="ipmi")
         random_state = random.choice(["on", "off", "error"])

=== modified file 'src/maasserver/node_query.py'
--- src/maasserver/node_query.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/node_query.py	2015-04-30 14:48:22 +0000
@@ -111,6 +111,11 @@
         # abandon this task; there's no point even logging.
         return
 
+    if not node.installable:
+        # This is actually a device, not a node. We don't support power queries
+        # for devices.
+        return
+
     if power_info is None:
         # The node does not have a valid power type, so we can't query it.
         # Logging this is just spam; this problem is reported elsewhere, so

=== modified file 'src/maasserver/rpc/nodes.py'
--- src/maasserver/rpc/nodes.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/rpc/nodes.py	2015-04-30 14:48:22 +0000
@@ -74,7 +74,8 @@
     else:
         power_info_by_node = (
             (node, node.get_effective_power_info())
-            for node in nodegroup.node_set.exclude(status=NODE_STATUS.BROKEN)
+            for node in nodegroup.node_set.exclude(
+                status=NODE_STATUS.BROKEN).exclude(installable=False)
         )
         return [
             {

=== modified file 'src/maasserver/rpc/tests/test_nodes.py'
--- src/maasserver/rpc/tests/test_nodes.py	2015-03-26 00:12:12 +0000
+++ src/maasserver/rpc/tests/test_nodes.py	2015-04-30 14:48:22 +0000
@@ -332,3 +332,14 @@
 
         self.assertThat(
             returned_system_ids, Not(Contains(broken_node.system_id)))
+
+    def test_does_not_return_power_info_for_devices(self):
+        cluster = factory.make_NodeGroup()
+        device = factory.make_Device()
+
+        power_parameters = list_cluster_nodes_power_parameters(cluster.uuid)
+        returned_system_ids = [
+            power_params['system_id'] for power_params in power_parameters]
+
+        self.assertThat(
+            returned_system_ids, Not(Contains(device.system_id)))

=== modified file 'src/maasserver/websockets/handlers/node.py'
--- src/maasserver/websockets/handlers/node.py	2015-04-08 20:54:14 +0000
+++ src/maasserver/websockets/handlers/node.py	2015-04-30 14:48:22 +0000
@@ -465,6 +465,11 @@
     def check_power(self, params):
         """Check the power state of the node."""
         obj = self.get_object(params)
+        if not obj.installable:
+            raise HandlerError(
+                "%s: Unable to query power state; not an installable node" %
+                obj.hostname)
+
         ng = obj.nodegroup
 
         try: