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

Proposed by Julian Edwards
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 3045
Proposed branch: lp:~julian-edwards/maas/api-for-power-query
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 297 lines (+160/-5)
6 files modified
src/maasserver/api/nodes.py (+61/-0)
src/maasserver/api/tests/test_doc.py (+1/-0)
src/maasserver/api/tests/test_nodes.py (+84/-0)
src/maasserver/exceptions.py (+11/-0)
src/maasserver/models/node.py (+1/-4)
src/maasserver/models/tests/test_node.py (+2/-1)
To merge this branch: bzr merge lp:~julian-edwards/maas/api-for-power-query
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Approve
Review via email: mp+235422@code.launchpad.net

Commit message

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.

Description of the change

I also removed a duplicate UnknownPowerAction exception...

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

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
Revision history for this message
Julian Edwards (julian-edwards) wrote :

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.

Revision history for this message
MAAS Lander (maas-lander) wrote :
Download full text (19.5 KiB)

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...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/api/nodes.py'
2--- src/maasserver/api/nodes.py 2014-09-16 15:46:23 +0000
3+++ src/maasserver/api/nodes.py 2014-09-22 06:29:36 +0000
4@@ -20,6 +20,7 @@
5 from base64 import b64decode
6
7 import bson
8+import crochet
9 from django.conf import settings
10 from django.core.exceptions import (
11 PermissionDenied,
12@@ -50,6 +51,7 @@
13 MAASAPIBadRequest,
14 NodesNotAvailable,
15 NodeStateViolation,
16+ PowerProblem,
17 StaticIPAddressExhaustion,
18 Unauthorized,
19 )
20@@ -69,10 +71,17 @@
21 from maasserver.models.nodeprobeddetails import get_single_probed_details
22 from maasserver.node_action import Commission
23 from maasserver.node_constraint_filter_forms import AcquireNodeForm
24+from maasserver.rpc import getClientFor
25 from maasserver.utils import find_nodegroup
26 from maasserver.utils.orm import get_first
27 from piston.utils import rc
28+from provisioningserver.power.poweraction import (
29+ PowerActionFail,
30+ UnknownPowerType,
31+ )
32 from provisioningserver.power_schema import UNKNOWN_POWER_TYPE
33+from provisioningserver.rpc.cluster import PowerQuery
34+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
35 import simplejson as json
36
37 # Node's fields exposed on the API.
38@@ -454,6 +463,58 @@
39 node = get_object_or_404(Node, system_id=system_id)
40 return node.power_parameters
41
42+ @operation(idempotent=True)
43+ def query_power_state(self, request, system_id):
44+ """Query the power state of a node.
45+
46+ Send a request to the node's power controller which asks it about
47+ the node's state. The reply to this could be delayed by up to
48+ 30 seconds while waiting for the power controller to respond.
49+ Use this method sparingly as it ties up an appserver thread
50+ while waiting.
51+
52+ If there is a problem, SERVICE_UNAVAILABLE is returned with text
53+ explaining why.
54+
55+ :param system_id: The node to query.
56+ :return: a dict whose key is "state" with a value of one of
57+ 'on' or 'off'.
58+ """
59+ node = get_object_or_404(Node, system_id=system_id)
60+ ng = node.nodegroup
61+
62+ try:
63+ client = getClientFor(ng.uuid)
64+ except NoConnectionsAvailable:
65+ maaslog.error(
66+ "Unable to get RPC connection for cluster '%s'", ng.name)
67+ raise PowerProblem("Unable to connect to cluster controller")
68+
69+ try:
70+ power_info = node.get_effective_power_info()
71+ except UnknownPowerType as e:
72+ raise PowerProblem(e)
73+ if not power_info.can_be_started:
74+ raise PowerProblem("Power state is not queryable")
75+
76+ call = client(
77+ PowerQuery, system_id=system_id, hostname=node.hostname,
78+ power_type=power_info.power_type,
79+ context=power_info.power_parameters)
80+ try:
81+ # Allow 30 seconds for the power query max as we're holding
82+ # up an appserver thread here.
83+ state = call.wait(30)
84+ except crochet.TimeoutError:
85+ maaslog.error(
86+ "%s: Timed out waiting for power response in Node.power_state",
87+ node.hostname)
88+ raise PowerProblem("Timed out waiting for power response")
89+ except (NotImplementedError, PowerActionFail) as e:
90+ raise PowerProblem(e)
91+
92+ return state
93+
94
95 def create_node(request):
96 """Service an http request to create a node.
97
98=== modified file 'src/maasserver/api/tests/test_doc.py'
99--- src/maasserver/api/tests/test_doc.py 2014-09-04 14:51:02 +0000
100+++ src/maasserver/api/tests/test_doc.py 2014-09-22 06:29:36 +0000
101@@ -268,6 +268,7 @@
102 "GET read op=None restful=True",
103 "GET details op=details restful=False",
104 "GET power_parameters op=power_parameters restful=False",
105+ "GET query_power_state op=query_power_state restful=False",
106 "POST start op=start restful=False",
107 "POST stop op=stop restful=False",
108 "POST release op=release restful=False",
109
110=== modified file 'src/maasserver/api/tests/test_nodes.py'
111--- src/maasserver/api/tests/test_nodes.py 2014-09-18 12:44:38 +0000
112+++ src/maasserver/api/tests/test_nodes.py 2014-09-22 06:29:36 +0000
113@@ -18,8 +18,10 @@
114 import json
115 import random
116
117+import crochet
118 from django.core.urlresolvers import reverse
119 from maasserver import forms
120+from maasserver.api import nodes as nodes_module
121 from maasserver.enum import (
122 NODE_STATUS,
123 NODE_STATUS_CHOICES_DICT,
124@@ -34,17 +36,27 @@
125 create_auth_token,
126 get_auth_tokens,
127 )
128+from maasserver.rpc.testing.fixtures import MockLiveRegionToClusterRPCFixture
129 from maasserver.testing.api import (
130 APITestCase,
131 MultipleUsersScenarios,
132 )
133 from maasserver.testing.architecture import make_usable_architecture
134+from maasserver.testing.eventloop import (
135+ RegionEventLoopFixture,
136+ RunningEventLoopFixture,
137+ )
138 from maasserver.testing.factory import factory
139 from maasserver.testing.orm import reload_object
140 from maasserver.testing.testcase import MAASServerTestCase
141 from maasserver.utils import ignore_unused
142 from maasserver.utils.orm import get_one
143 from maastesting.djangotestcase import count_queries
144+from mock import Mock
145+from provisioningserver.power.poweraction import PowerActionFail
146+from provisioningserver.rpc import cluster as cluster_module
147+from provisioningserver.rpc.exceptions import NoConnectionsAvailable
148+from provisioningserver.rpc.testing import always_succeed_with
149 from provisioningserver.utils.enum import map_enum
150 from testtools.matchers import (
151 Contains,
152@@ -1360,3 +1372,75 @@
153
154 self.assertEqual(httplib.OK, response.status_code)
155 self.assertEqual(self.old_allocated_status, parsed_result['status'])
156+
157+
158+class TestPowerState(APITestCase):
159+
160+ def get_node_uri(self, node):
161+ """Get the API URI for a node."""
162+ return reverse('node_handler', args=[node.system_id])
163+
164+ def prepare_rpc(self, nodegroup, side_effect=None):
165+ self.useFixture(RegionEventLoopFixture("rpc"))
166+ self.useFixture(RunningEventLoopFixture())
167+ self.rpc_fixture = self.useFixture(MockLiveRegionToClusterRPCFixture())
168+ protocol = self.rpc_fixture.makeCluster(
169+ nodegroup, cluster_module.PowerQuery)
170+ if side_effect is None:
171+ protocol.PowerQuery.side_effect = always_succeed_with({})
172+ else:
173+ protocol.PowerQuery.side_effect = side_effect
174+
175+ def test__catches_no_connection_error(self):
176+ self.patch(
177+ nodes_module,
178+ 'getClientFor').side_effect = NoConnectionsAvailable()
179+ node = factory.make_Node()
180+ response = self.client.get(
181+ self.get_node_uri(node), {"op": "query_power_state"})
182+ self.assertResponseCode(httplib.SERVICE_UNAVAILABLE, response)
183+ self.assertIn(
184+ "Unable to connect to cluster controller", response.content)
185+
186+ def test__catches_timeout_error(self):
187+ mock_client = Mock()
188+ self.patch(
189+ nodes_module, 'getClientFor').return_value = mock_client
190+ mock_client().wait.side_effect = crochet.TimeoutError("error")
191+ node = factory.make_Node(power_type="ipmi")
192+ response = self.client.get(
193+ self.get_node_uri(node), {"op": "query_power_state"})
194+ self.assertResponseCode(httplib.SERVICE_UNAVAILABLE, response)
195+ self.assertIn("Timed out waiting for power response", response.content)
196+
197+ def test__catches_unknown_power_type(self):
198+ self.patch(nodes_module, 'getClientFor')
199+ node = factory.make_Node(power_type="")
200+ response = self.client.get(
201+ self.get_node_uri(node), {"op": "query_power_state"})
202+ self.assertResponseCode(httplib.SERVICE_UNAVAILABLE, response)
203+ self.assertIn("Power state is not queryable", response.content)
204+
205+ def test__catches_poweraction_fail(self):
206+ node = factory.make_Node(power_type="ipmi")
207+ error_message = factory.make_name("error")
208+ self.prepare_rpc(
209+ node.nodegroup, side_effect=PowerActionFail(error_message))
210+ response = self.client.get(
211+ self.get_node_uri(node), {"op": "query_power_state"})
212+ self.assertResponseCode(httplib.SERVICE_UNAVAILABLE, response)
213+ self.assertIn(error_message, response.content)
214+
215+ def test__returns_actual_state(self):
216+ node = factory.make_Node(power_type="ipmi")
217+ random_state = random.choice(["on", "off", "error"])
218+ self.prepare_rpc(
219+ node.nodegroup,
220+ side_effect=always_succeed_with({"state": random_state}))
221+ response = self.client.get(
222+ self.get_node_uri(node), {"op": "query_power_state"})
223+ self.assertResponseCode(httplib.OK, response)
224+ response = json.loads(response.content)
225+ self.assertEqual(
226+ {"state": random_state},
227+ response)
228
229=== modified file 'src/maasserver/exceptions.py'
230--- src/maasserver/exceptions.py 2014-09-10 15:52:27 +0000
231+++ src/maasserver/exceptions.py 2014-09-22 06:29:36 +0000
232@@ -22,6 +22,7 @@
233 "NodeStateViolation",
234 "NodeGroupMisconfiguration",
235 "IteratorReusedError",
236+ "PowerProblem",
237 "StaticIPAddressExhaustion",
238 "StaticIPAddressTypeClash",
239 "UnresolvableHost",
240@@ -157,3 +158,13 @@
241
242 class PreseedError(MAASException):
243 """Raised when issue generating the preseed."""
244+
245+
246+class PowerProblem(MAASAPIException):
247+ """Raised when there's a problem with a power operation.
248+
249+ This could be a problem with parameters, a problem with the power
250+ controller, or something else. The exception text will contain more
251+ information.
252+ """
253+ api_error = httplib.SERVICE_UNAVAILABLE
254
255=== modified file 'src/maasserver/models/node.py'
256--- src/maasserver/models/node.py 2014-09-19 04:36:06 +0000
257+++ src/maasserver/models/node.py 2014-09-22 06:29:36 +0000
258@@ -106,6 +106,7 @@
259 from piston.models import Token
260 from provisioningserver.drivers.osystem import OperatingSystemRegistry
261 from provisioningserver.logger import get_maas_logger
262+from provisioningserver.power.poweraction import UnknownPowerType
263 from provisioningserver.rpc.cluster import (
264 CancelMonitor,
265 StartMonitors,
266@@ -124,10 +125,6 @@
267 return 'node-%s' % uuid1()
268
269
270-class UnknownPowerType(Exception):
271- """Raised when a node has an unknown power type."""
272-
273-
274 def validate_hostname(hostname):
275 """Validator for hostnames.
276
277
278=== modified file 'src/maasserver/models/tests/test_node.py'
279--- src/maasserver/models/tests/test_node.py 2014-09-19 01:47:01 +0000
280+++ src/maasserver/models/tests/test_node.py 2014-09-22 06:29:36 +0000
281@@ -88,6 +88,7 @@
282 Mock,
283 sentinel,
284 )
285+from provisioningserver.power.poweraction import UnknownPowerType
286 from provisioningserver.rpc import cluster as cluster_module
287 from provisioningserver.rpc.cluster import StartMonitors
288 from provisioningserver.rpc.exceptions import MultipleFailures
289@@ -439,7 +440,7 @@
290 def test_get_effective_power_type_raises_if_not_set(self):
291 node = factory.make_Node(power_type='')
292 self.assertRaises(
293- node_module.UnknownPowerType, node.get_effective_power_type)
294+ UnknownPowerType, node.get_effective_power_type)
295
296 def test_get_effective_power_type_reads_node_field(self):
297 power_types = list(get_power_types().keys()) # Python3 proof.