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

[Newell Jensen]

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.

[Blake Rouse (blake-rouse)]

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

review: Needs Fixing

[Newell Jensen (newell-jensen)]

> 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)]

Thanks for the fixes. Looks good.

review: Approve

[MAAS Lander (maas-lander)]

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)]

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

=== modified file 'src/maasserver/static/js/angular/controllers/node_details.js'
--- src/maasserver/static/js/angular/controllers/node_details.js	2016-04-11 16:23:26 +0000
+++ src/maasserver/static/js/angular/controllers/node_details.js	2016-04-25 23:23:52 +0000
@@ -405,10 +405,16 @@
         }
 
         // Update the node with new data on the region.
-        $scope.updateNode = function(node) {
+        $scope.updateNode = function(node, queryPower) {
+            if(angular.isUndefined(queryPower)) {
+                queryPower = false;
+            }
             return $scope.nodesManager.updateItem(node).then(function(node) {
                 updateHeader();
                 updateSummary();
+                if(queryPower) {
+                    $scope.checkPowerState();
+                }
             }, function(error) {
                 console.log(error);
                 updateHeader();
@@ -804,7 +810,7 @@
             node.power_parameters = angular.copy($scope.power.parameters);
 
             // Update the node.
-            $scope.updateNode(node);
+            $scope.updateNode(node, true);
         };
 
         // Return true if the "load more" events button should be available.
@@ -836,6 +842,38 @@
             return text;
         };
 
+        $scope.getPowerEventError = function() {
+            for(i=0;i<$scope.node.events.length;i++) {
+                var event = $scope.node.events[i];
+                if(event.type.level === "warning" &&
+                   event.type.description === "Failed to query node's BMC") {
+                    // Latest power event is an error
+                    return event;
+                } else if(event.type.level === "info" &&
+                          event.type.description === "Queried node's BMC") {
+                    // Latest power event is not an error
+                    return;
+                }
+            }
+            // No power event found, thus no error
+            return;
+        };
+
+        $scope.hasPowerEventError = function() {
+            var event = $scope.getPowerEventError();
+            return angular.isObject(event);
+        };
+
+        $scope.getPowerEventErrorText = function() {
+            var event = $scope.getPowerEventError();
+            if(angular.isObject(event)) {
+                // Return text
+                return event.description;
+            } else {
+                return "";
+            }
+        };
+
         // Called when the machine output view has changed.
         $scope.machineOutputViewChanged = function() {
             if(angular.isObject($scope.machine_output.selectedView) &&
 
=== modified file 'src/maasserver/static/js/angular/controllers/tests/test_node_details.js'
--- src/maasserver/static/js/angular/controllers/tests/test_node_details.js	2016-04-11 16:23:26 +0000
+++ src/maasserver/static/js/angular/controllers/tests/test_node_details.js	2016-04-25 23:23:52 +0000
@@ -1881,6 +1881,79 @@
             });
     });
 
+    describe("getPowerEventError", function() {
+
+        it("returns event if there is a power event error", function() {
+            var controller = makeController();
+            var evt = makeEvent();
+            evt.type.level = "warning";
+            evt.type.description = "Failed to query node's BMC";
+            $scope.node = node;
+            $scope.node.events = [
+                makeEvent(),
+                evt
+            ];
+            expect($scope.getPowerEventError()).toBe(evt);
+        });
+
+        it("returns nothing if there is no power event error", function() {
+            var controller = makeController();
+            var evt_info = makeEvent();
+            var evt_error = makeEvent();
+            evt_info.type.level = "info";
+            evt_info.type.description = "Queried node's BMC";
+            evt_error.type.level = "warning";
+            evt_error.type.description = "Failed to query node's BMC";
+            $scope.node = node;
+            $scope.node.events = [
+                makeEvent(),
+                evt_info,
+                evt_error
+            ];
+            expect($scope.getPowerEventError()).toBe();
+        });
+    });
+
+    describe("hasPowerEventError", function() {
+
+        it("returns true if last event is an error", function() {
+            var controller = makeController();
+            var evt = makeEvent();
+            evt.type.level = "warning";
+            evt.type.description = "Failed to query node's BMC";
+            $scope.node = node;
+            $scope.node.events = [evt];
+            expect($scope.hasPowerEventError()).toBe(true);
+        });
+
+        it("returns false if last event is not an error", function() {
+            var controller = makeController();
+            $scope.node = node;
+            $scope.node.events = [makeEvent()];
+            expect($scope.hasPowerEventError()).toBe(false);
+            });
+    });
+
+    describe("getPowerEventErrorText", function() {
+
+        it("returns just empty string", function() {
+            var controller = makeController();
+            $scope.node = node;
+            $scope.node.events = [makeEvent()];
+            expect($scope.getPowerEventErrorText()).toBe("");
+        });
+
+        it("returns event description", function() {
+            var controller = makeController();
+            var evt = makeEvent();
+            evt.type.level = "warning";
+            evt.type.description = "Failed to query node's BMC";
+            $scope.node = node;
+            $scope.node.events = [evt];
+            expect($scope.getPowerEventErrorText()).toBe(evt.description);
+            });
+    });
+
     describe("machineOutputViewChanged", function() {
 
         it("sets showSummaryToggle to false if no selectedView", function() {
 
=== modified file 'src/maasserver/static/partials/node-details.html'
--- src/maasserver/static/partials/node-details.html	2016-04-12 17:36:35 +0000
+++ src/maasserver/static/partials/node-details.html	2016-04-25 23:23:52 +0000
@@ -353,6 +353,9 @@
                             rack controller.  To proceed, install the {$ getPowerErrors() $}
                             on the rack controller.
                         </li>
+                        <li class="flash-messages__item error" data-ng-if="hasPowerEventError()">
+			    {$ getPowerEventErrorText() $}
+                        </li>
                         <li class="flash-messages__item info" data-ng-if="power.type.name == 'manual'">
                             Power control for this power type will need to be handled manually.
                         </li>
 
=== modified file 'src/provisioningserver/events.py'
--- src/provisioningserver/events.py	2016-03-28 13:54:47 +0000
+++ src/provisioningserver/events.py	2016-04-25 23:23:52 +0000
@@ -52,6 +52,7 @@
     NODE_POWERED_OFF = 'NODE_POWERED_OFF'
     NODE_POWER_ON_FAILED = 'NODE_POWER_ON_FAILED'
     NODE_POWER_OFF_FAILED = 'NODE_POWER_OFF_FAILED'
+    NODE_POWER_QUERIED = 'NODE_POWER_QUERIED'
     NODE_POWER_QUERY_FAILED = 'NODE_POWER_QUERY_FAILED'
     # PXE request event.
     NODE_PXE_REQUEST = 'NODE_PXE_REQUEST'
@@ -113,6 +114,10 @@
         description="Failed to power off node",
         level=ERROR,
     ),
+    EVENT_TYPES.NODE_POWER_QUERIED: EventDetail(
+        description="Queried node's BMC",
+        level=INFO,
+    ),
     EVENT_TYPES.NODE_POWER_QUERY_FAILED: EventDetail(
         description="Failed to query node's BMC",
         level=WARN,
 
=== modified file 'src/provisioningserver/power/query.py'
--- src/provisioningserver/power/query.py	2016-03-28 13:54:47 +0000
+++ src/provisioningserver/power/query.py	2016-04-25 23:23:52 +0000
@@ -106,7 +106,11 @@
 @inlineCallbacks
 def power_query_success(system_id, hostname, state):
     """Report a node that for which power querying has succeeded."""
+    message = "Power state queried: %s" % state
     yield power.power_state_update(system_id, state)
+    yield send_event_node(
+        EVENT_TYPES.NODE_POWER_QUERIED,
+        system_id, hostname, message)
 
 
 @inlineCallbacks
@@ -118,7 +122,7 @@
     yield power.power_state_update(system_id, 'error')
     yield send_event_node(
         EVENT_TYPES.NODE_POWER_QUERY_FAILED,
-        system_id, hostname, message)
+        system_id, hostname, failure.getErrorMessage())
 
 
 @asynchronous
 
=== modified file 'src/provisioningserver/power/tests/test_query.py'
--- src/provisioningserver/power/tests/test_query.py	2016-03-28 13:54:47 +0000
+++ src/provisioningserver/power/tests/test_query.py	2016-04-25 23:23:52 +0000
@@ -90,9 +90,24 @@
             protocol.SendEvent,
             MockCalledOnceWith(
                 ANY, type_name=EVENT_TYPES.NODE_POWER_QUERY_FAILED,
-                system_id=system_id, description=(
-                    "Power state could not be queried: " + message),
-            ))
+                system_id=system_id, description=message))
+
+    def test_power_query_success_emits_event(self):
+        system_id = factory.make_name('system_id')
+        hostname = factory.make_name('hostname')
+        state = factory.make_name('state')
+        message = "Power state queried: %s" % state
+        protocol, io = self.patch_rpc_methods()
+        d = power.query.power_query_success(
+            system_id, hostname, state)
+        # This blocks until the deferred is complete.
+        io.flush()
+        self.assertIsNone(extract_result(d))
+        self.assertThat(
+            protocol.SendEvent,
+            MockCalledOnceWith(
+                ANY, type_name=EVENT_TYPES.NODE_POWER_QUERIED,
+                system_id=system_id, description=message))
 
 
 class TestPowerQuery(MAASTestCase):
@@ -249,10 +264,8 @@
         (power_type, {
             "power_type": power_type,
             "power_driver": power_drivers_by_name.get(power_type),
-            "func": (  # Function to invoke driver.
-                "perform_power_driver_query"
-                if power_type in PowerDriverRegistry
-                else "perform_power_query"),
+            "func": (  # Function to invoke power driver.
+                "perform_power_driver_query"),
             "waits": (  # Pauses between retries.
                 [] if power_type in PowerDriverRegistry
                 else DEFAULT_WAITING_POLICY),
@@ -322,7 +335,7 @@
         self.assertThat(
             send_event_node, MockCalledOnceWith(
                 EVENT_TYPES.NODE_POWER_QUERY_FAILED,
-                system_id, hostname, expected_message))
+                system_id, hostname, exception_message))
 
         # Nothing was logged to the Twisted log.
         self.assertEqual("", logger_twisted.output)
 
=== modified file 'src/provisioningserver/rpc/clusterservice.py'
--- src/provisioningserver/rpc/clusterservice.py	2016-04-14 19:54:36 +0000
+++ src/provisioningserver/rpc/clusterservice.py	2016-04-25 23:23:52 +0000
@@ -35,7 +35,10 @@
 from provisioningserver.logger.log import get_maas_logger
 from provisioningserver.networks import get_interfaces_definition
 from provisioningserver.power.change import maybe_change_power_state
-from provisioningserver.power.query import get_power_state
+from provisioningserver.power.query import (
+    get_power_state,
+    report_power_state,
+)
 from provisioningserver.refresh import (
     get_architecture,
     get_os_release,
@@ -267,6 +270,7 @@
     def power_query(self, system_id, hostname, power_type, context):
         d = get_power_state(
             system_id, hostname, power_type, context=context)
+        d = report_power_state(d, system_id, hostname)
         d.addCallback(lambda x: {'state': x})
         return d
 
 
=== modified file 'src/provisioningserver/rpc/tests/test_clusterservice.py'
--- src/provisioningserver/rpc/tests/test_clusterservice.py	2016-04-14 19:54:36 +0000
+++ src/provisioningserver/rpc/tests/test_clusterservice.py	2016-04-25 23:23:52 +0000
@@ -1629,19 +1629,12 @@
     @inlineCallbacks
     def test_returns_power_state(self):
         state = random.choice(['on', 'off'])
-        perform_power_query = self.patch(
-            power_module.query, "perform_power_query")
-        perform_power_query.return_value = state
-
-        # During the transition from template-based power drivers to Python
-        # drivers, alias perform_power_driver_query to perform_power_query.
-        self.patch(
-            power_module.query, "perform_power_driver_query",
-            perform_power_query)
-
-        # Intercept calls to report the status.
+        perform_power_driver_query = self.patch(
+            power_module.query, "perform_power_driver_query")
+        perform_power_driver_query.return_value = state
         report_power_state = self.patch(
-            power_module.query, "report_power_state")
+            clusterservice, "report_power_state")
+        report_power_state.return_value = succeed(state)
 
         power_type = random.choice(QUERY_POWER_TYPES)
         arguments = {
@@ -1653,20 +1646,17 @@
 
         # Make sure power driver doesn't check for installed packages.
         power_driver = power_drivers_by_name.get(power_type)
-        if power_driver:
-            self.patch_autospec(
-                power_driver, "detect_missing_packages").return_value = []
+        self.patch_autospec(
+            power_driver, "detect_missing_packages").return_value = []
 
         observed = yield call_responder(
             Cluster(), cluster.PowerQuery, arguments)
         self.assertEqual({'state': state}, observed)
         self.assertThat(
-            perform_power_query,
+            perform_power_driver_query,
             MockCalledOnceWith(
                 arguments['system_id'], arguments['hostname'],
                 arguments['power_type'], arguments['context']))
-        # The region is NOT told about the change.
-        self.assertThat(report_power_state, MockNotCalled())
 
 
 class TestClusterProtocol_ConfigureDHCP(MAASTestCase):