Merge lp:~newell-jensen/maas/fix-1617591 into lp:~maas-committers/maas/trunk

[Newell Jensen]

Clean up power query messaging on provisioningserver so that all messages will have the hostname of the node in the message to help identify which node is experiencing the issue. Additionally catches PowerError's from the power drivers when doing a power query, which was missing before.

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

=== modified file 'src/provisioningserver/power/query.py'
--- src/provisioningserver/power/query.py	2016-07-30 01:17:54 +0000
+++ src/provisioningserver/power/query.py	2016-09-09 18:37:28 +0000
@@ -15,6 +15,7 @@
 from provisioningserver.drivers.power import (
     power_drivers_by_name,
     PowerDriverRegistry,
+    PowerError,
 )
 from provisioningserver.events import (
     EVENT_TYPES,
@@ -109,9 +110,8 @@
 @inlineCallbacks
 def power_query_failure(system_id, hostname, failure):
     """Report a node that for which power querying has failed."""
-    message = "Power state could not be queried: %s"
-    message %= failure.getErrorMessage()
-    maaslog.error(message)
+    maaslog.error("%s: Power state could not be queried: %s" % (
+        hostname, failure.getErrorMessage()))
     yield power.power_state_update(system_id, 'error')
     yield send_event_node(
         EVENT_TYPES.NODE_POWER_QUERY_FAILED,
@@ -150,7 +150,7 @@
 
 def maaslog_report_failure(node, failure):
     """Log failure to query node."""
-    if failure.check(PowerActionFail):
+    if failure.check(PowerActionFail, PowerError):
         maaslog.error(
             "%s: Could not query power state: %s.",
             node['hostname'], failure.getErrorMessage())

=== modified file 'src/provisioningserver/power/tests/test_query.py'
--- src/provisioningserver/power/tests/test_query.py	2016-08-19 10:22:29 +0000
+++ src/provisioningserver/power/tests/test_query.py	2016-09-09 18:37:28 +0000
@@ -32,6 +32,7 @@
     DEFAULT_WAITING_POLICY,
     power_drivers_by_name,
     PowerDriverRegistry,
+    PowerError,
 )
 from provisioningserver.events import EVENT_TYPES
 from provisioningserver.rpc import (
@@ -308,7 +309,8 @@
         self.assertThat(query, MockCallsMatch(*expected_calls))
 
         expected_message = (
-            "Power state could not be queried: %s" % exception_message)
+            "%s: Power state could not be queried: %s" % (
+                hostname, exception_message))
 
         # An attempt was made to report the failure to the region.
         self.assertThat(
@@ -470,6 +472,29 @@
             maaslog.output)
 
     @inlineCallbacks
+    def test_query_all_nodes_swallows_PowerError(self):
+        node1, node2 = self.make_nodes(2)
+        new_state_2 = self.pick_alternate_state(node2['power_state'])
+        get_power_state = self.patch(power.query, 'get_power_state')
+        error_msg = factory.make_name("error")
+        get_power_state.side_effect = [
+            fail(PowerError(error_msg)),
+            succeed(new_state_2),
+        ]
+        suppress_reporting(self)
+
+        with FakeLogger("maas.power", level=logging.DEBUG) as maaslog:
+            yield power.query.query_all_nodes([node1, node2])
+
+        self.assertDocTestMatches(
+            """\
+            %s: Could not query power state: %s.
+            %s: Power state has changed from %s to %s.
+            """ % (node1['hostname'], error_msg,
+                   node2['hostname'], node2['power_state'], new_state_2),
+            maaslog.output)
+
+    @inlineCallbacks
     def test_query_all_nodes_swallows_NoSuchNode(self):
         node1, node2 = self.make_nodes(2)
         new_state_2 = self.pick_alternate_state(node2['power_state'])