Merge lp:~julian-edwards/maas/api-for-power-query into lp:~maas-committers/maas/trunk

[Julian Edwards]

Add an API call Node.power_state() which asks the cluster controller to query the state of the Node's power and returns the result. This is mainly intended for use in the web UI as a "test power parameters" button. It will hold up the appserver thread for up to 30 seconds waiting for the power query to finish, so use sparingly.

I also removed a duplicate UnknownPowerAction exception...

[Jeroen T. Vermeulen (jtv)]

Thorough work. I do have a few questions:

.

Half a minute seems generous... Are we really likely to need that long? It's an awfully long time to hold up an app server thread.

And if calling this method is likely to have a noticeable impact on the system, I wouldn't name it as if it were just a getter. Why not call it query_power_state to make it clear that it's “doing things” and not just “returning data”?

.

Not sure it matters but it looks to me as if 503: Service Unavailable was meant to signify that the web service as such is down, not just the resource that's being accessed.

.

What does “Power state is not queryable” mean? Is there anything more that can be said about that kind of error?

review: Approve

[Julian Edwards (julian-edwards)]

On Monday 22 September 2014 05:25:47 you wrote:
> Review: Approve
>
> Thorough work. I do have a few questions:
>
> .
>
> Half a minute seems generous... Are we really likely to need that long?
> It's an awfully long time to hold up an app server thread.

Remember that this is the *maximum*. If the power method is quicker then it
won't last that long. 30 seconds is backstop.

> And if calling this method is likely to have a noticeable impact on the
> system, I wouldn't name it as if it were just a getter. Why not call it
> query_power_state to make it clear that it's “doing things” and not just
> “returning data”?

Good point, I'll change it, thanks.

> Not sure it matters but it looks to me as if 503: Service Unavailable was
> meant to signify that the web service as such is down, not just the
> resource that's being accessed.

I used 503 when we're out of IPs as well. I'm not sure there's a better one.
Here's the blurb for 503:

"The Web server (running the Web site) is currently unable to handle the HTTP
request due to a temporary overloading or maintenance of the server. The
implication is that this is a temporary condition which will be alleviated
after some delay."

I didn't want to use BAD_REQUEST as that implies you must not send the request
again without modifying it.

> What does “Power state is not queryable” mean? Is there anything more that
> can be said about that kind of error?

That's a tough one. At the moment, it means that the power type is WoL, but
that is abstracted away through the method it's calling. I'm not sure what
else to say in the error.

[MAAS Lander (maas-lander)]

The attempt to merge lp:~julian-edwards/maas/api-for-power-query into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Hit http://security.ubuntu.com trusty-security Release.gpg
Hit http://security.ubuntu.com trusty-security Release
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:1 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:2 http://nova.clouds.archive.ubuntu.com trusty-updates Release [59.7 kB]
Hit http://security.ubuntu.com trusty-security/main Sources
Hit http://security.ubuntu.com trusty-security/universe Sources
Hit http://security.ubuntu.com trusty-security/main amd64 Packages
Hit http://security.ubuntu.com trusty-security/universe amd64 Packages
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:3 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [120 kB]
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [84.7 kB]
Get:5 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [320 kB]
Get:6 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [203 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 788 kB in 0s (1,876 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb curl daemontools debhelper dh-apport distro-info dnsutils firefox freeipmi-tools ipython isc-dhcp-common libjs-raphael libjs-yui3-full libjs-yui3-min libpq-dev make pep8 postgresql pyflakes python-amqplib python-bzrlib python-celery python-convoy python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mimeparse python-mock python-netaddr python-netifaces python-nose python-oauth python-oops python-oops-amqp python-oops-datedir-repo python-oops-twisted python-oops-wsgi python-openssl python-paramiko python-pexpect python-pip python-pocket-lint pyt...

=== modified file 'src/maasserver/api/nodes.py'
--- src/maasserver/api/nodes.py	2014-09-16 15:46:23 +0000
+++ src/maasserver/api/nodes.py	2014-09-22 06:29:36 +0000
@@ -20,6 +20,7 @@
 from base64 import b64decode
 
 import bson
+import crochet
 from django.conf import settings
 from django.core.exceptions import (
     PermissionDenied,
@@ -50,6 +51,7 @@
     MAASAPIBadRequest,
     NodesNotAvailable,
     NodeStateViolation,
+    PowerProblem,
     StaticIPAddressExhaustion,
     Unauthorized,
     )
@@ -69,10 +71,17 @@
 from maasserver.models.nodeprobeddetails import get_single_probed_details
 from maasserver.node_action import Commission
 from maasserver.node_constraint_filter_forms import AcquireNodeForm
+from maasserver.rpc import getClientFor
 from maasserver.utils import find_nodegroup
 from maasserver.utils.orm import get_first
 from piston.utils import rc
+from provisioningserver.power.poweraction import (
+    PowerActionFail,
+    UnknownPowerType,
+    )
 from provisioningserver.power_schema import UNKNOWN_POWER_TYPE
+from provisioningserver.rpc.cluster import PowerQuery
+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
 import simplejson as json
 
 # Node's fields exposed on the API.
@@ -454,6 +463,58 @@
         node = get_object_or_404(Node, system_id=system_id)
         return node.power_parameters
 
+    @operation(idempotent=True)
+    def query_power_state(self, request, system_id):
+        """Query the power state of a node.
+
+        Send a request to the node's power controller which asks it about
+        the node's state.  The reply to this could be delayed by up to
+        30 seconds while waiting for the power controller to respond.
+        Use this method sparingly as it ties up an appserver thread
+        while waiting.
+
+        If there is a problem, SERVICE_UNAVAILABLE is returned with text
+        explaining why.
+
+        :param system_id: The node to query.
+        :return: a dict whose key is "state" with a value of one of
+            'on' or 'off'.
+        """
+        node = get_object_or_404(Node, system_id=system_id)
+        ng = node.nodegroup
+
+        try:
+            client = getClientFor(ng.uuid)
+        except NoConnectionsAvailable:
+            maaslog.error(
+                "Unable to get RPC connection for cluster '%s'", ng.name)
+            raise PowerProblem("Unable to connect to cluster controller")
+
+        try:
+            power_info = node.get_effective_power_info()
+        except UnknownPowerType as e:
+            raise PowerProblem(e)
+        if not power_info.can_be_started:
+            raise PowerProblem("Power state is not queryable")
+
+        call = client(
+            PowerQuery, system_id=system_id, hostname=node.hostname,
+            power_type=power_info.power_type,
+            context=power_info.power_parameters)
+        try:
+            # Allow 30 seconds for the power query max as we're holding
+            # up an appserver thread here.
+            state = call.wait(30)
+        except crochet.TimeoutError:
+            maaslog.error(
+                "%s: Timed out waiting for power response in Node.power_state",
+                node.hostname)
+            raise PowerProblem("Timed out waiting for power response")
+        except (NotImplementedError, PowerActionFail) as e:
+            raise PowerProblem(e)
+
+        return state
+
 
 def create_node(request):
     """Service an http request to create a node.

=== modified file 'src/maasserver/api/tests/test_doc.py'
--- src/maasserver/api/tests/test_doc.py	2014-09-04 14:51:02 +0000
+++ src/maasserver/api/tests/test_doc.py	2014-09-22 06:29:36 +0000
@@ -268,6 +268,7 @@
             "GET read op=None restful=True",
             "GET details op=details restful=False",
             "GET power_parameters op=power_parameters restful=False",
+            "GET query_power_state op=query_power_state restful=False",
             "POST start op=start restful=False",
             "POST stop op=stop restful=False",
             "POST release op=release restful=False",

=== modified file 'src/maasserver/api/tests/test_nodes.py'
--- src/maasserver/api/tests/test_nodes.py	2014-09-18 12:44:38 +0000
+++ src/maasserver/api/tests/test_nodes.py	2014-09-22 06:29:36 +0000
@@ -18,8 +18,10 @@
 import json
 import random
 
+import crochet
 from django.core.urlresolvers import reverse
 from maasserver import forms
+from maasserver.api import nodes as nodes_module
 from maasserver.enum import (
     NODE_STATUS,
     NODE_STATUS_CHOICES_DICT,
@@ -34,17 +36,27 @@
     create_auth_token,
     get_auth_tokens,
     )
+from maasserver.rpc.testing.fixtures import MockLiveRegionToClusterRPCFixture
 from maasserver.testing.api import (
     APITestCase,
     MultipleUsersScenarios,
     )
 from maasserver.testing.architecture import make_usable_architecture
+from maasserver.testing.eventloop import (
+    RegionEventLoopFixture,
+    RunningEventLoopFixture,
+    )
 from maasserver.testing.factory import factory
 from maasserver.testing.orm import reload_object
 from maasserver.testing.testcase import MAASServerTestCase
 from maasserver.utils import ignore_unused
 from maasserver.utils.orm import get_one
 from maastesting.djangotestcase import count_queries
+from mock import Mock
+from provisioningserver.power.poweraction import PowerActionFail
+from provisioningserver.rpc import cluster as cluster_module
+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
+from provisioningserver.rpc.testing import always_succeed_with
 from provisioningserver.utils.enum import map_enum
 from testtools.matchers import (
     Contains,
@@ -1360,3 +1372,75 @@
 
         self.assertEqual(httplib.OK, response.status_code)
         self.assertEqual(self.old_allocated_status, parsed_result['status'])
+
+
+class TestPowerState(APITestCase):
+
+    def get_node_uri(self, node):
+        """Get the API URI for a node."""
+        return reverse('node_handler', args=[node.system_id])
+
+    def prepare_rpc(self, nodegroup, side_effect=None):
+        self.useFixture(RegionEventLoopFixture("rpc"))
+        self.useFixture(RunningEventLoopFixture())
+        self.rpc_fixture = self.useFixture(MockLiveRegionToClusterRPCFixture())
+        protocol = self.rpc_fixture.makeCluster(
+            nodegroup, cluster_module.PowerQuery)
+        if side_effect is None:
+            protocol.PowerQuery.side_effect = always_succeed_with({})
+        else:
+            protocol.PowerQuery.side_effect = side_effect
+
+    def test__catches_no_connection_error(self):
+        self.patch(
+            nodes_module,
+            'getClientFor').side_effect = NoConnectionsAvailable()
+        node = factory.make_Node()
+        response = self.client.get(
+            self.get_node_uri(node), {"op": "query_power_state"})
+        self.assertResponseCode(httplib.SERVICE_UNAVAILABLE, response)
+        self.assertIn(
+            "Unable to connect to cluster controller", response.content)
+
+    def test__catches_timeout_error(self):
+        mock_client = Mock()
+        self.patch(
+            nodes_module, 'getClientFor').return_value = mock_client
+        mock_client().wait.side_effect = crochet.TimeoutError("error")
+        node = factory.make_Node(power_type="ipmi")
+        response = self.client.get(
+            self.get_node_uri(node), {"op": "query_power_state"})
+        self.assertResponseCode(httplib.SERVICE_UNAVAILABLE, response)
+        self.assertIn("Timed out waiting for power response", response.content)
+
+    def test__catches_unknown_power_type(self):
+        self.patch(nodes_module, 'getClientFor')
+        node = factory.make_Node(power_type="")
+        response = self.client.get(
+            self.get_node_uri(node), {"op": "query_power_state"})
+        self.assertResponseCode(httplib.SERVICE_UNAVAILABLE, response)
+        self.assertIn("Power state is not queryable", response.content)
+
+    def test__catches_poweraction_fail(self):
+        node = factory.make_Node(power_type="ipmi")
+        error_message = factory.make_name("error")
+        self.prepare_rpc(
+            node.nodegroup, side_effect=PowerActionFail(error_message))
+        response = self.client.get(
+            self.get_node_uri(node), {"op": "query_power_state"})
+        self.assertResponseCode(httplib.SERVICE_UNAVAILABLE, response)
+        self.assertIn(error_message, response.content)
+
+    def test__returns_actual_state(self):
+        node = factory.make_Node(power_type="ipmi")
+        random_state = random.choice(["on", "off", "error"])
+        self.prepare_rpc(
+            node.nodegroup,
+            side_effect=always_succeed_with({"state": random_state}))
+        response = self.client.get(
+            self.get_node_uri(node), {"op": "query_power_state"})
+        self.assertResponseCode(httplib.OK, response)
+        response = json.loads(response.content)
+        self.assertEqual(
+            {"state": random_state},
+            response)

=== modified file 'src/maasserver/exceptions.py'
--- src/maasserver/exceptions.py	2014-09-10 15:52:27 +0000
+++ src/maasserver/exceptions.py	2014-09-22 06:29:36 +0000
@@ -22,6 +22,7 @@
     "NodeStateViolation",
     "NodeGroupMisconfiguration",
     "IteratorReusedError",
+    "PowerProblem",
     "StaticIPAddressExhaustion",
     "StaticIPAddressTypeClash",
     "UnresolvableHost",
@@ -157,3 +158,13 @@
 
 class PreseedError(MAASException):
     """Raised when issue generating the preseed."""
+
+
+class PowerProblem(MAASAPIException):
+    """Raised when there's a problem with a power operation.
+
+    This could be a problem with parameters, a problem with the power
+    controller, or something else.  The exception text will contain more
+    information.
+    """
+    api_error = httplib.SERVICE_UNAVAILABLE

=== modified file 'src/maasserver/models/node.py'
--- src/maasserver/models/node.py	2014-09-19 04:36:06 +0000
+++ src/maasserver/models/node.py	2014-09-22 06:29:36 +0000
@@ -106,6 +106,7 @@
 from piston.models import Token
 from provisioningserver.drivers.osystem import OperatingSystemRegistry
 from provisioningserver.logger import get_maas_logger
+from provisioningserver.power.poweraction import UnknownPowerType
 from provisioningserver.rpc.cluster import (
     CancelMonitor,
     StartMonitors,
@@ -124,10 +125,6 @@
     return 'node-%s' % uuid1()
 
 
-class UnknownPowerType(Exception):
-    """Raised when a node has an unknown power type."""
-
-
 def validate_hostname(hostname):
     """Validator for hostnames.
 

=== modified file 'src/maasserver/models/tests/test_node.py'
--- src/maasserver/models/tests/test_node.py	2014-09-19 01:47:01 +0000
+++ src/maasserver/models/tests/test_node.py	2014-09-22 06:29:36 +0000
@@ -88,6 +88,7 @@
     Mock,
     sentinel,
     )
+from provisioningserver.power.poweraction import UnknownPowerType
 from provisioningserver.rpc import cluster as cluster_module
 from provisioningserver.rpc.cluster import StartMonitors
 from provisioningserver.rpc.exceptions import MultipleFailures
@@ -439,7 +440,7 @@
     def test_get_effective_power_type_raises_if_not_set(self):
         node = factory.make_Node(power_type='')
         self.assertRaises(
-            node_module.UnknownPowerType, node.get_effective_power_type)
+            UnknownPowerType, node.get_effective_power_type)
 
     def test_get_effective_power_type_reads_node_field(self):
         power_types = list(get_power_types().keys())  # Python3 proof.