Merge lp:~newell-jensen/maas/ui-power-parameter-errors into lp:maas/trunk

Proposed by Newell Jensen on 2016-04-22
Status: Merged
Approved by: Newell Jensen on 2016-04-26
Approved revision: 4931
Merged at revision: 4958
Proposed branch: lp:~newell-jensen/maas/ui-power-parameter-errors
Merge into: lp:maas/trunk
Diff against target: 346 lines (+160/-30)
8 files modified
src/maasserver/static/js/angular/controllers/node_details.js (+40/-2)
src/maasserver/static/js/angular/controllers/tests/test_node_details.js (+73/-0)
src/maasserver/static/partials/node-details.html (+3/-0)
src/provisioningserver/events.py (+5/-0)
src/provisioningserver/power/query.py (+5/-1)
src/provisioningserver/power/tests/test_query.py (+21/-8)
src/provisioningserver/rpc/clusterservice.py (+5/-1)
src/provisioningserver/rpc/tests/test_clusterservice.py (+8/-18)
To merge this branch: bzr merge lp:~newell-jensen/maas/ui-power-parameter-errors
Reviewer Review Type Date Requested Status
Blake Rouse 2016-04-22 Approve on 2016-04-26
Review via email: mp+292602@code.launchpad.net

Commit message

This branch adds a power query when the power parameters are saved in the UI and if there is an error, it will be displayed for the user. Power querying has also been added to the event log.

To post a comment you must log in.
Blake Rouse (blake-rouse) wrote :

Backend code looks good. The frontend code could use an improvement on how you find the error.

review: Needs Fixing
Newell Jensen (newell-jensen) wrote :

> Backend code looks good. The frontend code could use an improvement on how you
> find the error.

I added review fixes and updated tests. Ready for another review.

Blake Rouse (blake-rouse) wrote :

Thanks for the fixes. Looks good.

review: Approve
MAAS Lander (maas-lander) wrote :
Download full text (1.1 MiB)

The attempt to merge lp:~newell-jensen/maas/ui-power-parameter-errors into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com/ubuntu xenial-security InRelease [92.2 kB]
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [92.2 kB]
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Fetched 184 kB in 0s (403 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-coverage python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-mock python3-netaddr python3-netifaces python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-2ubuntu3).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
bind9 is already the newest version (1:9.10.3.dfsg.P4-8).
bind9utils is already the newest version (1:9.10.3.dfsg.P4-8).
build-essential is already the newest version (12.1ubuntu2).
bzr is already the newest version (2.7.0-2ubuntu1).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
dnsutils is already the newest version (1:9.10.3.dfsg.P4-8).
firefox is already the newest version (45.0.2+build1-0ubuntu1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest version (1:2.7.4-0ubuntu1).
isc-dhcp-common is already the newest version (4.3.3-5ubuntu12).
libjs-angularjs is already the newest version (1.2.28-1ubuntu2).
libjs-jquery is already the newest version (1.11.3+dfsg-4).
libj...

MAAS Lander (maas-lander) wrote :
Download full text (1.0 MiB)

The attempt to merge lp:~newell-jensen/maas/ui-power-parameter-errors into lp:maas failed. Below is the output from the failed tests.

Get:1 http://security.ubuntu.com/ubuntu xenial-security InRelease [92.2 kB]
Hit:2 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial InRelease
Get:3 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates InRelease [92.2 kB]
Hit:4 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-backports InRelease
Get:5 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/main amd64 Packages [5,400 B]
Get:6 http://prodstack-zone-2.clouds.archive.ubuntu.com/ubuntu xenial-updates/universe amd64 Packages [3,796 B]
Fetched 194 kB in 0s (411 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
    --no-install-recommends install apache2 archdetect-deb authbind bash bind9 bind9utils build-essential bzr bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm postgresql pxelinux python3-all python3-apt python3-bson python3-convoy python3-coverage python3-crochet python3-cssselect python3-curtin python3-dev python3-distro-info python3-django python3-django-nose python3-django-piston3 python3-dnspython python3-docutils python3-formencode python3-hivex python3-httplib2 python3-jinja2 python3-jsonschema python3-lxml python3-mock python3-netaddr python3-netifaces python3-oauth python3-oauthlib python3-openssl python3-paramiko python3-petname python3-pexpect python3-psycopg2 python3-pyinotify python3-pyparsing python3-pyvmomi python3-requests python3-seamicroclient python3-setuptools python3-simplestreams python3-sphinx python3-tempita python3-twisted python3-txtftp python3-tz python3-yaml python3-zope.interface python-bson python-crochet python-django python-django-piston python-djorm-ext-pgarray python-formencode python-lxml python-netaddr python-netifaces python-pocket-lint python-psycopg2 python-simplejson python-tempita python-twisted python-yaml socat syslinux-common tgt ubuntu-cloudimage-keyring wget xvfb
Reading package lists...
Building dependency tree...
Reading state information...
apache2 is already the newest version (2.4.18-2ubuntu3).
archdetect-deb is already the newest version (1.117ubuntu2).
authbind is already the newest version (2.1.1+nmu1).
bash is already the newest version (4.3-14ubuntu1).
bind9 is already the newest version (1:9.10.3.dfsg.P4-8).
bind9utils is already the newest version (1:9.10.3.dfsg.P4-8).
build-essential is already the newest version (12.1ubuntu2).
bzr is already the newest version (2.7.0-2ubuntu1).
curl is already the newest version (7.47.0-1ubuntu2).
debhelper is already the newest version (9.20160115ubuntu3).
distro-info is already the newest version (0.14build1).
dnsutils is already the newest version (1:9.10.3.dfsg.P4-8).
firefox is already the newest version (45.0.2+build1-0ubuntu1).
freeipmi-tools is already the newest version (1.4.11-1ubuntu1).
git is already the newest ve...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/static/js/angular/controllers/node_details.js'
2--- src/maasserver/static/js/angular/controllers/node_details.js 2016-04-11 16:23:26 +0000
3+++ src/maasserver/static/js/angular/controllers/node_details.js 2016-04-25 23:23:52 +0000
4@@ -405,10 +405,16 @@
5 }
6
7 // Update the node with new data on the region.
8- $scope.updateNode = function(node) {
9+ $scope.updateNode = function(node, queryPower) {
10+ if(angular.isUndefined(queryPower)) {
11+ queryPower = false;
12+ }
13 return $scope.nodesManager.updateItem(node).then(function(node) {
14 updateHeader();
15 updateSummary();
16+ if(queryPower) {
17+ $scope.checkPowerState();
18+ }
19 }, function(error) {
20 console.log(error);
21 updateHeader();
22@@ -804,7 +810,7 @@
23 node.power_parameters = angular.copy($scope.power.parameters);
24
25 // Update the node.
26- $scope.updateNode(node);
27+ $scope.updateNode(node, true);
28 };
29
30 // Return true if the "load more" events button should be available.
31@@ -836,6 +842,38 @@
32 return text;
33 };
34
35+ $scope.getPowerEventError = function() {
36+ for(i=0;i<$scope.node.events.length;i++) {
37+ var event = $scope.node.events[i];
38+ if(event.type.level === "warning" &&
39+ event.type.description === "Failed to query node's BMC") {
40+ // Latest power event is an error
41+ return event;
42+ } else if(event.type.level === "info" &&
43+ event.type.description === "Queried node's BMC") {
44+ // Latest power event is not an error
45+ return;
46+ }
47+ }
48+ // No power event found, thus no error
49+ return;
50+ };
51+
52+ $scope.hasPowerEventError = function() {
53+ var event = $scope.getPowerEventError();
54+ return angular.isObject(event);
55+ };
56+
57+ $scope.getPowerEventErrorText = function() {
58+ var event = $scope.getPowerEventError();
59+ if(angular.isObject(event)) {
60+ // Return text
61+ return event.description;
62+ } else {
63+ return "";
64+ }
65+ };
66+
67 // Called when the machine output view has changed.
68 $scope.machineOutputViewChanged = function() {
69 if(angular.isObject($scope.machine_output.selectedView) &&
70
71=== modified file 'src/maasserver/static/js/angular/controllers/tests/test_node_details.js'
72--- src/maasserver/static/js/angular/controllers/tests/test_node_details.js 2016-04-11 16:23:26 +0000
73+++ src/maasserver/static/js/angular/controllers/tests/test_node_details.js 2016-04-25 23:23:52 +0000
74@@ -1881,6 +1881,79 @@
75 });
76 });
77
78+ describe("getPowerEventError", function() {
79+
80+ it("returns event if there is a power event error", function() {
81+ var controller = makeController();
82+ var evt = makeEvent();
83+ evt.type.level = "warning";
84+ evt.type.description = "Failed to query node's BMC";
85+ $scope.node = node;
86+ $scope.node.events = [
87+ makeEvent(),
88+ evt
89+ ];
90+ expect($scope.getPowerEventError()).toBe(evt);
91+ });
92+
93+ it("returns nothing if there is no power event error", function() {
94+ var controller = makeController();
95+ var evt_info = makeEvent();
96+ var evt_error = makeEvent();
97+ evt_info.type.level = "info";
98+ evt_info.type.description = "Queried node's BMC";
99+ evt_error.type.level = "warning";
100+ evt_error.type.description = "Failed to query node's BMC";
101+ $scope.node = node;
102+ $scope.node.events = [
103+ makeEvent(),
104+ evt_info,
105+ evt_error
106+ ];
107+ expect($scope.getPowerEventError()).toBe();
108+ });
109+ });
110+
111+ describe("hasPowerEventError", function() {
112+
113+ it("returns true if last event is an error", function() {
114+ var controller = makeController();
115+ var evt = makeEvent();
116+ evt.type.level = "warning";
117+ evt.type.description = "Failed to query node's BMC";
118+ $scope.node = node;
119+ $scope.node.events = [evt];
120+ expect($scope.hasPowerEventError()).toBe(true);
121+ });
122+
123+ it("returns false if last event is not an error", function() {
124+ var controller = makeController();
125+ $scope.node = node;
126+ $scope.node.events = [makeEvent()];
127+ expect($scope.hasPowerEventError()).toBe(false);
128+ });
129+ });
130+
131+ describe("getPowerEventErrorText", function() {
132+
133+ it("returns just empty string", function() {
134+ var controller = makeController();
135+ $scope.node = node;
136+ $scope.node.events = [makeEvent()];
137+ expect($scope.getPowerEventErrorText()).toBe("");
138+ });
139+
140+ it("returns event description", function() {
141+ var controller = makeController();
142+ var evt = makeEvent();
143+ evt.type.level = "warning";
144+ evt.type.description = "Failed to query node's BMC";
145+ $scope.node = node;
146+ $scope.node.events = [evt];
147+ expect($scope.getPowerEventErrorText()).toBe(evt.description);
148+ });
149+ });
150+
151 describe("machineOutputViewChanged", function() {
152
153 it("sets showSummaryToggle to false if no selectedView", function() {
154
155=== modified file 'src/maasserver/static/partials/node-details.html'
156--- src/maasserver/static/partials/node-details.html 2016-04-12 17:36:35 +0000
157+++ src/maasserver/static/partials/node-details.html 2016-04-25 23:23:52 +0000
158@@ -353,6 +353,9 @@
159 rack controller. To proceed, install the {$ getPowerErrors() $}
160 on the rack controller.
161 </li>
162+ <li class="flash-messages__item error" data-ng-if="hasPowerEventError()">
163+ {$ getPowerEventErrorText() $}
164+ </li>
165 <li class="flash-messages__item info" data-ng-if="power.type.name == 'manual'">
166 Power control for this power type will need to be handled manually.
167 </li>
168
169=== modified file 'src/provisioningserver/events.py'
170--- src/provisioningserver/events.py 2016-03-28 13:54:47 +0000
171+++ src/provisioningserver/events.py 2016-04-25 23:23:52 +0000
172@@ -52,6 +52,7 @@
173 NODE_POWERED_OFF = 'NODE_POWERED_OFF'
174 NODE_POWER_ON_FAILED = 'NODE_POWER_ON_FAILED'
175 NODE_POWER_OFF_FAILED = 'NODE_POWER_OFF_FAILED'
176+ NODE_POWER_QUERIED = 'NODE_POWER_QUERIED'
177 NODE_POWER_QUERY_FAILED = 'NODE_POWER_QUERY_FAILED'
178 # PXE request event.
179 NODE_PXE_REQUEST = 'NODE_PXE_REQUEST'
180@@ -113,6 +114,10 @@
181 description="Failed to power off node",
182 level=ERROR,
183 ),
184+ EVENT_TYPES.NODE_POWER_QUERIED: EventDetail(
185+ description="Queried node's BMC",
186+ level=INFO,
187+ ),
188 EVENT_TYPES.NODE_POWER_QUERY_FAILED: EventDetail(
189 description="Failed to query node's BMC",
190 level=WARN,
191
192=== modified file 'src/provisioningserver/power/query.py'
193--- src/provisioningserver/power/query.py 2016-03-28 13:54:47 +0000
194+++ src/provisioningserver/power/query.py 2016-04-25 23:23:52 +0000
195@@ -106,7 +106,11 @@
196 @inlineCallbacks
197 def power_query_success(system_id, hostname, state):
198 """Report a node that for which power querying has succeeded."""
199+ message = "Power state queried: %s" % state
200 yield power.power_state_update(system_id, state)
201+ yield send_event_node(
202+ EVENT_TYPES.NODE_POWER_QUERIED,
203+ system_id, hostname, message)
204
205
206 @inlineCallbacks
207@@ -118,7 +122,7 @@
208 yield power.power_state_update(system_id, 'error')
209 yield send_event_node(
210 EVENT_TYPES.NODE_POWER_QUERY_FAILED,
211- system_id, hostname, message)
212+ system_id, hostname, failure.getErrorMessage())
213
214
215 @asynchronous
216
217=== modified file 'src/provisioningserver/power/tests/test_query.py'
218--- src/provisioningserver/power/tests/test_query.py 2016-03-28 13:54:47 +0000
219+++ src/provisioningserver/power/tests/test_query.py 2016-04-25 23:23:52 +0000
220@@ -90,9 +90,24 @@
221 protocol.SendEvent,
222 MockCalledOnceWith(
223 ANY, type_name=EVENT_TYPES.NODE_POWER_QUERY_FAILED,
224- system_id=system_id, description=(
225- "Power state could not be queried: " + message),
226- ))
227+ system_id=system_id, description=message))
228+
229+ def test_power_query_success_emits_event(self):
230+ system_id = factory.make_name('system_id')
231+ hostname = factory.make_name('hostname')
232+ state = factory.make_name('state')
233+ message = "Power state queried: %s" % state
234+ protocol, io = self.patch_rpc_methods()
235+ d = power.query.power_query_success(
236+ system_id, hostname, state)
237+ # This blocks until the deferred is complete.
238+ io.flush()
239+ self.assertIsNone(extract_result(d))
240+ self.assertThat(
241+ protocol.SendEvent,
242+ MockCalledOnceWith(
243+ ANY, type_name=EVENT_TYPES.NODE_POWER_QUERIED,
244+ system_id=system_id, description=message))
245
246
247 class TestPowerQuery(MAASTestCase):
248@@ -249,10 +264,8 @@
249 (power_type, {
250 "power_type": power_type,
251 "power_driver": power_drivers_by_name.get(power_type),
252- "func": ( # Function to invoke driver.
253- "perform_power_driver_query"
254- if power_type in PowerDriverRegistry
255- else "perform_power_query"),
256+ "func": ( # Function to invoke power driver.
257+ "perform_power_driver_query"),
258 "waits": ( # Pauses between retries.
259 [] if power_type in PowerDriverRegistry
260 else DEFAULT_WAITING_POLICY),
261@@ -322,7 +335,7 @@
262 self.assertThat(
263 send_event_node, MockCalledOnceWith(
264 EVENT_TYPES.NODE_POWER_QUERY_FAILED,
265- system_id, hostname, expected_message))
266+ system_id, hostname, exception_message))
267
268 # Nothing was logged to the Twisted log.
269 self.assertEqual("", logger_twisted.output)
270
271=== modified file 'src/provisioningserver/rpc/clusterservice.py'
272--- src/provisioningserver/rpc/clusterservice.py 2016-04-14 19:54:36 +0000
273+++ src/provisioningserver/rpc/clusterservice.py 2016-04-25 23:23:52 +0000
274@@ -35,7 +35,10 @@
275 from provisioningserver.logger.log import get_maas_logger
276 from provisioningserver.networks import get_interfaces_definition
277 from provisioningserver.power.change import maybe_change_power_state
278-from provisioningserver.power.query import get_power_state
279+from provisioningserver.power.query import (
280+ get_power_state,
281+ report_power_state,
282+)
283 from provisioningserver.refresh import (
284 get_architecture,
285 get_os_release,
286@@ -267,6 +270,7 @@
287 def power_query(self, system_id, hostname, power_type, context):
288 d = get_power_state(
289 system_id, hostname, power_type, context=context)
290+ d = report_power_state(d, system_id, hostname)
291 d.addCallback(lambda x: {'state': x})
292 return d
293
294
295=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
296--- src/provisioningserver/rpc/tests/test_clusterservice.py 2016-04-14 19:54:36 +0000
297+++ src/provisioningserver/rpc/tests/test_clusterservice.py 2016-04-25 23:23:52 +0000
298@@ -1629,19 +1629,12 @@
299 @inlineCallbacks
300 def test_returns_power_state(self):
301 state = random.choice(['on', 'off'])
302- perform_power_query = self.patch(
303- power_module.query, "perform_power_query")
304- perform_power_query.return_value = state
305-
306- # During the transition from template-based power drivers to Python
307- # drivers, alias perform_power_driver_query to perform_power_query.
308- self.patch(
309- power_module.query, "perform_power_driver_query",
310- perform_power_query)
311-
312- # Intercept calls to report the status.
313+ perform_power_driver_query = self.patch(
314+ power_module.query, "perform_power_driver_query")
315+ perform_power_driver_query.return_value = state
316 report_power_state = self.patch(
317- power_module.query, "report_power_state")
318+ clusterservice, "report_power_state")
319+ report_power_state.return_value = succeed(state)
320
321 power_type = random.choice(QUERY_POWER_TYPES)
322 arguments = {
323@@ -1653,20 +1646,17 @@
324
325 # Make sure power driver doesn't check for installed packages.
326 power_driver = power_drivers_by_name.get(power_type)
327- if power_driver:
328- self.patch_autospec(
329- power_driver, "detect_missing_packages").return_value = []
330+ self.patch_autospec(
331+ power_driver, "detect_missing_packages").return_value = []
332
333 observed = yield call_responder(
334 Cluster(), cluster.PowerQuery, arguments)
335 self.assertEqual({'state': state}, observed)
336 self.assertThat(
337- perform_power_query,
338+ perform_power_driver_query,
339 MockCalledOnceWith(
340 arguments['system_id'], arguments['hostname'],
341 arguments['power_type'], arguments['context']))
342- # The region is NOT told about the change.
343- self.assertThat(report_power_state, MockNotCalled())
344
345
346 class TestClusterProtocol_ConfigureDHCP(MAASTestCase):