Merge lp:~mpontillo/maas/fix-1469305-service-monitor-ignore-sudo-output into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Ignore sudo output inside service monitor. (This fixes upstart service status parsing errors that occur when, for example, an invalid hostname is set.)

[Raphaël Badin (rvb)]

Looks good. Thanks for fixing this.

review: Approve

=== modified file 'src/provisioningserver/service_monitor.py'
--- src/provisioningserver/service_monitor.py	2015-06-02 09:36:59 +0000
+++ src/provisioningserver/service_monitor.py	2015-06-27 00:30:22 +0000
@@ -269,6 +269,20 @@
         if exit_code != 0:
             raise UnknownServiceError("'%s' is unknown to upstart." % (
                 service_name))
+        for line in output.splitlines():
+            if not line.startswith('sudo:'):
+                active_state, process_state = self._parse_upstart_status_line(
+                    line, service_name)
+                break
+        active_state_enum = self.UPSTART_TO_STATE.get(active_state)
+        if active_state_enum is None:
+            raise ServiceParsingError(
+                "Unable to parse the active state from upstart for "
+                "service '%s', active state reported as '%s'." % (
+                    service_name, active_state))
+        return active_state_enum, process_state
+
+    def _parse_upstart_status_line(self, output, service_name):
         # output looks like:
         #    tgt start/running, process 29993
         # split to get the active_state/process_state
@@ -279,13 +293,7 @@
             raise ServiceParsingError(
                 "Unable to parse the output from upstart for service '%s'." % (
                     service_name))
-        active_state_enum = self.UPSTART_TO_STATE.get(active_state)
-        if active_state_enum is None:
-            raise ServiceParsingError(
-                "Unable to parse the active state from upstart for "
-                "service '%s', active state reported as '%s'." % (
-                    service_name, active_state))
-        return active_state_enum, process_state
+        return active_state, process_state
 
     def _get_expected_process_state(self, active_state):
         """Return the expected process state for the `active_state` based

=== modified file 'src/provisioningserver/tests/test_service_monitor.py'
--- src/provisioningserver/tests/test_service_monitor.py	2015-06-02 09:36:59 +0000
+++ src/provisioningserver/tests/test_service_monitor.py	2015-06-27 00:30:22 +0000
@@ -436,6 +436,24 @@
         self.assertEquals(SERVICE_STATE.ON, active_state)
         self.assertEquals("running", process_state)
 
+    def test__get_systemd_service_status_ignores_sudo_output(self):
+        systemd_status_output = dedent("""\
+            sudo: unable to resolve host sub-etha-sens-o-matic
+            tgt.service - LSB: iscsi target daemon
+                Loaded: loaded (/etc/init.d/tgt)
+                Active: active (running) since Fri 2015-05-15 15:08:26 UTC;
+                Docs: man:systemd-sysv-generator(8)
+            """)
+
+        service_monitor = ServiceMonitor()
+        mock_exec_service_action = self.patch(
+            service_monitor, "_exec_service_action")
+        mock_exec_service_action.return_value = (0, systemd_status_output)
+        active_state, process_state = (
+            service_monitor._get_systemd_service_status("tgt"))
+        self.assertEquals(SERVICE_STATE.ON, active_state)
+        self.assertEquals("running", process_state)
+
     def test__get_systemd_service_status_raise_error_for_invalid_active(self):
         systemd_status_output = dedent("""\
             tgt.service - LSB: iscsi target daemon
@@ -494,6 +512,18 @@
         self.assertEquals(SERVICE_STATE.ON, active_state)
         self.assertEquals("running", process_state)
 
+    def test__get_upstart_service_status_parsing_ignores_sudo_output(self):
+        service_monitor = ServiceMonitor()
+        mock_exec_service_action = self.patch(
+            service_monitor, "_exec_service_action")
+        mock_exec_service_action.return_value = (0, dedent("""\
+                sudo: unable to resolve host infinite-improbability
+                tgt start/running, process 23239"""))
+        active_state, process_state = (
+            service_monitor._get_upstart_service_status("tgt"))
+        self.assertEquals(SERVICE_STATE.ON, active_state)
+        self.assertEquals("running", process_state)
+
     def test__get_upstart_service_status_raise_error_for_invalid_active(self):
         service_monitor = ServiceMonitor()
         mock_exec_service_action = self.patch(