modify monit to use 'status' output instead of 'summary' output

The summary output is a fixed width table which truncates the
contents and prevents us from parsing the actual status of the
program.

plugins/modules/monitoring/monit.py

@@ -7,6 +7,7 @@
 from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
+from collections import namedtuple
 
 DOCUMENTATION = '''
 ---
@@ -45,7 +46,6 @@ import re
 
 from ansible.module_utils.basic import AnsibleModule
 
-
 STATE_COMMAND_MAP = {
     'stopped': 'stop',
     'started': 'start',
@@ -56,6 +56,36 @@ STATE_COMMAND_MAP = {
 
 MIN_VERSION = (5, 21)
 
+ALL_STATUS = [
+    'missing', 'ok', 'not_monitored', 'initializing', 'does_not_exist'
+]
+
+
+class StatusValue(namedtuple("Status", "status, is_pending")):
+    MISSING = 0
+    OK = 1
+    NOT_MONITORED = 2
+    INITIALIZING = 3
+    DOES_NOT_EXIST = 4
+
+    def __new__(cls, status, is_pending=False):
+        return super(StatusValue, cls).__new__(cls, status, is_pending)
+
+    def pending(self):
+        return StatusValue(self.status, True)
+
+    def __getattr__(self, item):
+        if item in ('is_%s' % status for status in ALL_STATUS):
+            return self.status == getattr(self, item[3:].upper())
+
+
+class Status(object):
+    MISSING = StatusValue(StatusValue.MISSING)
+    OK = StatusValue(StatusValue.OK)
+    NOT_MONITORED = StatusValue(StatusValue.NOT_MONITORED)
+    INITIALIZING = StatusValue(StatusValue.INITIALIZING)
+    DOES_NOT_EXIST = StatusValue(StatusValue.DOES_NOT_EXIST)
+
 
 class Monit(object):
     def __init__(self, module, monit_bin_path, service_name, timeout):
@@ -81,30 +111,40 @@ class Monit(object):
             min_version = '.'.join(str(v) for v in MIN_VERSION)
             self.module.fail_json(msg='Monit version not compatible with module. Install version >= %s' % min_version)
 
-    def parse(self, parts):
-        if len(parts) > 2 and parts[2].lower() == 'process' and parts[0] == self.process_name:
-            return ''.join(parts[1]).lower()
-        else:
-            return ''
-
     def get_status(self):
-        """Return the status of the process in monit, or the empty string if not present."""
+        """Return the status of the process in monit."""
-        rc, out, err = self.module.run_command('%s %s' % (self.monit_bin_path, 'summary -B'), check_rc=True)
+        command = '%s status %s -B' % (self.monit_bin_path, self.process_name)
-        for line in out.split('\n'):
+        rc, out, err = self.module.run_command(command, check_rc=True)
-            # Sample output lines:
+        return self._parse_status(out)
-            # Process 'name'    Running
-            # Process 'name'    Running - restart pending
-            parts = self.parse(line.split())
-            if parts != '':
-                return parts
 
-        return ''
+    def _parse_status(self, output):
+        if "Process '%s'" % self.process_name not in output:
+            return Status.MISSING
+
+        status_val = re.findall(r"^\s*status\s*([\w\- ]+)", output, re.MULTILINE)
+        if status_val:
+            status_val = status_val[0].strip().upper()
+            if ' - ' not in status_val:
+                status_val.replace(' ', '_')
+                return getattr(Status, status_val)
+            else:
+                status_val, substatus = status_val.split(' - ')
+                action, state = substatus.split()
+                if action in ['START', 'INITIALIZING', 'RESTART', 'MONITOR']:
+                    status = Status.OK
+                else:
+                    status = Status.NOT_MONITORED
+
+                if state == 'pending':
+                    status = status.pending()
+                return status
 
     def is_process_present(self):
-        return self.get_status() != ''
+        rc, out, err = self.module.run_command('%s summary -B' % (self.monit_bin_path), check_rc=True)
+        return bool(re.findall(r'\b%s\b' % self.process_name, out))
 
     def is_process_running(self):
-        return 'running' in self.get_status()
+        return self.get_status().is_ok
 
     def run_command(self, command):
         """Runs a monit command, and returns the new status."""
@@ -115,7 +155,7 @@ class Monit(object):
         timeout_time = time.time() + self.timeout
 
         running_status = self.get_status()
-        while running_status == '' or 'pending' in running_status or 'initializing' in running_status:
+        while running_status.is_missing or running_status.is_pending or running_status.is_initializing:
             if time.time() >= timeout_time:
                 self.module.fail_json(
                     msg='waited too long for "pending", or "initiating" status to go away ({0})'.format(
@@ -141,30 +181,30 @@ class Monit(object):
             self.wait_for_monit_to_stop_pending('present')
         self.module.exit_json(changed=True, name=self.process_name, state='present')
 
-    def change_state(self, state, expected_status, status_contains=None, invert_expected=None):
+    def change_state(self, state, expected_status, invert_expected=None):
         self.run_command(STATE_COMMAND_MAP[state])
         status = self.get_status()
-        status_match = status in expected_status
+        status_match = status.status == expected_status.status
         if invert_expected:
             status_match = not status_match
-        if status_match or (status_contains and status_contains in status):
+        if status_match:
             self.module.exit_json(changed=True, name=self.process_name, state=state)
         self.module.fail_json(msg='%s process not %s' % (self.process_name, state), status=status)
 
     def stop(self):
-        self.change_state('stopped', ['not monitored'], 'stop pending')
+        self.change_state('stopped', Status.NOT_MONITORED)
 
     def unmonitor(self):
-        self.change_state('unmonitored', ['not monitored'], 'unmonitor pending')
+        self.change_state('unmonitored', Status.NOT_MONITORED)
 
     def restart(self):
-        self.change_state('restarted', ['initializing', 'running'], 'restart pending')
+        self.change_state('restarted', Status.OK)
 
     def start(self):
-        self.change_state('started', ['initializing', 'running'], 'start pending')
+        self.change_state('started', Status.OK)
 
     def monitor(self):
-        self.change_state('monitored', ['not monitored'], invert_expected=True)
+        self.change_state('monitored', Status.NOT_MONITORED, invert_expected=True)
 
 
 def main():

tests/unit/plugins/modules/monitoring/test_monit.py

@@ -4,11 +4,21 @@ from __future__ import (absolute_import, division, print_function)
 __metaclass__ = type
 
 import mock
+import pytest
+
 from ansible_collections.community.general.tests.unit.compat import unittest
 from ansible_collections.community.general.plugins.modules.monitoring import monit
 from ansible_collections.community.general.tests.unit.plugins.modules.utils import AnsibleExitJson, AnsibleFailJson
 
 
+TEST_OUTPUT = """
+Process '%s'
+  status                       %s
+  monitoring status            Not monitored
+  monitoring mode              active
+"""
+
+
 class MonitTest(unittest.TestCase):
     def setUp(self):
         self.module = mock.MagicMock()
@@ -17,6 +27,8 @@ class MonitTest(unittest.TestCase):
         self.monit = monit.Monit(self.module, 'monit', 'processX', 1)
 
     def patch_status(self, side_effect):
+        if not isinstance(side_effect, list):
+            side_effect = [side_effect]
         return mock.patch.object(self.monit, 'get_status', side_effect=side_effect)
 
     def test_min_version(self):
@@ -25,43 +37,75 @@ class MonitTest(unittest.TestCase):
                 self.monit.check_version()
 
     def test_change_state_success(self):
-        with self.patch_status(['not monitored']):
+        with self.patch_status(monit.Status.NOT_MONITORED):
             with self.assertRaises(AnsibleExitJson):
                 self.monit.stop()
         self.module.fail_json.assert_not_called()
         self.module.run_command.assert_called_with('monit stop processX', check_rc=True)
 
     def test_change_state_fail(self):
-        with self.patch_status(['monitored']):
+        with self.patch_status(monit.Status.OK):
             with self.assertRaises(AnsibleFailJson):
                 self.monit.stop()
 
     def test_reload_fail(self):
         self.module.run_command.return_value = (1, 'stdout', 'stderr')
-        self.module.fail_json.side_effect = AnsibleFailJson(Exception)
         with self.assertRaises(AnsibleFailJson):
             self.monit.reload()
 
     def test_reload(self):
         self.module.run_command.return_value = (0, '', '')
         self.monit._sleep_time = 0
-        with self.patch_status(['', 'pending', 'running']):
+        status = [
+            monit.Status.MISSING,
+            monit.Status.INITIALIZING,
+            monit.Status.OK.pending(),
+            monit.Status.OK
+        ]
+        with self.patch_status(status) as get_status:
             with self.assertRaises(AnsibleExitJson):
                 self.monit.reload()
+            self.assertEqual(get_status.call_count, len(status))
 
     def test_monitor(self):
-        with self.patch_status(['not monitored - start pending']):
+        with self.patch_status(monit.Status.OK.pending()):
             with self.assertRaises(AnsibleExitJson):
                 self.monit.monitor()
 
     def test_monitor_fail(self):
-        with self.patch_status(['not monitored']):
+        with self.patch_status(monit.Status.NOT_MONITORED):
             with self.assertRaises(AnsibleFailJson):
                 self.monit.monitor()
 
     def test_timeout(self):
         self.monit.timeout = 0
-        self.module.fail_json.side_effect = AnsibleFailJson(Exception)
+        with self.patch_status(monit.Status.NOT_MONITORED.pending()):
-        with self.patch_status(['stop pending']):
             with self.assertRaises(AnsibleFailJson):
                 self.monit.wait_for_monit_to_stop_pending('stopped')
+
+
+@pytest.mark.parametrize('status_name', [name for name in monit.ALL_STATUS])
+def test_status_value(status_name):
+    value = getattr(monit.StatusValue, status_name.upper())
+    status = monit.StatusValue(value)
+    assert getattr(status, 'is_%s' % status_name)
+    assert not all(getattr(status, 'is_%s' % name) for name in monit.ALL_STATUS if name != status_name)
+
+
+BASIC_OUTPUT_CASES = [
+    (TEST_OUTPUT % ('processX', name), getattr(monit.Status, name.upper()))
+    for name in monit.ALL_STATUS
+]
+
+
+@pytest.mark.parametrize('output, expected', BASIC_OUTPUT_CASES + [
+    ('', monit.Status.MISSING),
+    (TEST_OUTPUT % ('processY', 'OK'), monit.Status.MISSING),
+    (TEST_OUTPUT % ('processX', 'Not Monitored - start pending'), monit.Status.OK),
+    (TEST_OUTPUT % ('processX', 'Monitored - stop pending'), monit.Status.NOT_MONITORED),
+    (TEST_OUTPUT % ('processX', 'Monitored - restart pending'), monit.Status.OK),
+    (TEST_OUTPUT % ('processX', 'Not Monitored - monitor pending'), monit.Status.OK),
+])
+def test_parse_status(output, expected):
+    status = monit.Monit(None, '', 'processX', 0)._parse_status(output)
+    assert status == expected