Merge lp:~julian-edwards/maas/api-for-power-query into lp:~maas-committers/maas/trunk
- api-for-power-query
- Merge into trunk
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jeroen T. Vermeulen (community) | Approve | ||
Review via email:
|
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...
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
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.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
MAAS Lander (maas-lander) wrote : | # |
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://
Hit http://
Hit http://
Ign http://
Ign http://
Hit http://
Get:1 http://
Hit http://
Get:2 http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Get:3 http://
Get:4 http://
Get:5 http://
Get:6 http://
Hit http://
Hit http://
Ign http://
Ign http://
Fetched 788 kB in 0s (1,876 kB/s)
Reading package lists...
sudo DEBIAN_
--
Preview Diff
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. |
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?