Repository: jamesoff/simplemonitor

Issue:
[suggestion] recover_command when the status is up again
It would be great to have the ability to run a command once the status of a monitored entity is up again, similar to `recover_command` ran when the status is failed.

A typical usage is for active-passive environments: once the normal state is back, some actions are done so that consumers of these services point again to them.

As a practical example, I have WiFi power switches (Sonoff Basic) which can connect to a set of WiFi networks. When one network is down, the next one is tried - but upon successful connection the switch stays on the network. In order to kick back the switches to the primary network, I would monitor the Access Point, and when it is back from failure - reboot the switches (or otherwise force then to rescan the networks). 

Another example is an active-passive home automation software( Home Assistant in my case) where in case of loss of the primary one, the secondary one is started  - and then switched off when the primary is back.

Discussion:

Patch:
diff --git a/.gitignore b/.gitignore
index c8d4272..ad8e536 100644
--- a/.gitignore
+++ b/.gitignore
@@ -27,3 +27,4 @@ docs/vendor
 monitor.log
 monitor.pid
 output.json
+did_recover*
diff --git a/.python-version b/.python-version
new file mode 100644
index 0000000..c1e43e6
--- /dev/null
+++ b/.python-version
@@ -0,0 +1,1 @@
+3.7.3
diff --git a/docs/_includes/monitors.html b/docs/_includes/monitors.html
index 4439cff..dab1044 100644
--- a/docs/_includes/monitors.html
+++ b/docs/_includes/monitors.html
@@ -18,6 +18,7 @@ All monitor types share the following configuration options:
 |gap| The number of seconds gap between polls for this monitor. Setting this lower than the global interval will have no effect. Use it to make a monitor poll only once an hour, for example.|no|0|
 |remote_alert| This monitor wants a remote host to handle alerting instead of the local host. Set to 1 to enable. This is a good candidate for putting in defaults if you want to use remote alerting for all your monitors.|no|0|
 |recover_command| A command to execute once when this monitor fails. It could, for example, restart a service if an HTTP check fails.|no| |
+|recovered_command| A command to execute once when this monitor succeeds the first time after being failed.|no| |
 |group|The group the monitor belongs to. Alerters will only fire for monitors which appear in their groups.|no|`default`|
 |notify|If the monitor should alert at all|no|1|
 {% for monitor in m %}
diff --git a/simplemonitor/Monitors/monitor.py b/simplemonitor/Monitors/monitor.py
index b4e8570..7ea2d89 100644
--- a/simplemonitor/Monitors/monitor.py
+++ b/simplemonitor/Monitors/monitor.py
@@ -75,7 +75,11 @@ class Monitor:
         self._recover_command = Monitor.get_config_option(
             config_options, "recover_command"
         )
+        self._recovered_command = Monitor.get_config_option(
+            config_options, "recovered_command"
+        )
         self.recover_info = ""
+        self.recovered_info = ""
         self.minimum_gap = Monitor.get_config_option(
             config_options, "gap", required_type="int", minimum=0, default=0
         )
@@ -123,10 +127,10 @@ class Monitor:
         return max(vfs, 0)
 
     def test_success(self) -> bool:
-        """Returns false if the test has failed."""
-        if self.error_count > self._tolerance:
-            return False
-        return True
+        """Returns false if the monitor has failed.
+
+        This means that enough tests have failed in a row to exceed the tolerance."""
+        return not bool(self.virtual_fail_count())
 
     def first_failure(self) -> bool:
         """Check if this is our first failure (past tolerance)."""
@@ -135,7 +139,10 @@ class Monitor:
         return False
 
     def state(self) -> bool:
-        """Returns false if the last test failed."""
+        """Returns false if the last test failed.
+
+        The monitor may not be in a failed state if the number of failed
+        tests are less than the tolerance."""
         if self.error_count > 0:
             return False
         return True
@@ -258,7 +265,7 @@ class Monitor:
     def all_better_now(self) -> bool:
         """Check if we've just recovered."""
         if (
-            self.last_error_count >= self._tolerance
+            self.last_virtual_fail_count()
             and self.success_count == 1
             and not self.was_skipped
         ):
@@ -323,9 +330,8 @@ class Monitor:
         return False
 
     def last_virtual_fail_count(self) -> int:
-        if (self.last_error_count - self._tolerance) < 0:
-            return 0
-        return self.last_error_count - self._tolerance
+        value = self.last_error_count - self._tolerance
+        return max(0, value)
 
     def attempt_recover(self) -> None:
         if self._recover_command is None:
@@ -335,12 +341,26 @@ class Monitor:
             return
 
         try:
+            self.monitor_logger.info("Attempting recovery command")
             p = subprocess.Popen(self._recover_command.split(" "))  # nosec
             p.wait()
             self.recover_info = "Command executed and returned %d" % p.returncode
         except Exception as e:
             self.recover_info = "Unable to run command: %s" % e
 
+    def run_recovered(self) -> None:
+        if self._recovered_command is None:
+            self.recovered_info = ""
+            return
+        if self.all_better_now():
+            self.monitor_logger.info("Attempting recovered command")
+            try:
+                p = subprocess.Popen(self._recovered_command.split(" "))  # nosec
+                p.wait()
+                self.recovered_info = "Command executed and returned %d" % p.returncode
+            except Exception as e:
+                self.recovered_info = "Unable to run command: %s" % e
+
     def post_config_setup(self) -> None:
         """ any post config setup needed """
         pass
@@ -413,9 +433,9 @@ class Monitor:
 
 @register
 class MonitorFail(Monitor):
-    """A monitor which always fails.
+    """A monitor which fails a fixed number of times then succeeds once, and repeats.
 
-    Use for testing alerters etc."""
+    Use for testing alerters etc. The default interval for successes is 5."""
 
     type = "fail"
 
@@ -438,11 +458,9 @@ class MonitorFail(Monitor):
             or (self.error_count == 0)
             or (self.error_count % self.interval != 0)
         ):
-            self.record_fail("This monitor always fails.")
-            return False
+            return self.record_fail("This monitor always fails.")
         else:
-            self.record_success()
-            return True
+            return self.record_success()
 
     def describe(self) -> str:
         return "A monitor which always fails."
diff --git a/simplemonitor/simplemonitor.py b/simplemonitor/simplemonitor.py
index 413d11f..038f24b 100644
--- a/simplemonitor/simplemonitor.py
+++ b/simplemonitor/simplemonitor.py
@@ -290,6 +290,10 @@ class SimpleMonitor:
         for key in list(self.monitors.keys()):
             self.monitors[key].attempt_recover()
 
+    def do_recovered(self) -> None:
+        for key in list(self.monitors.keys()):
+            self.monitors[key].run_recovered()
+
     def hup_loggers(self) -> None:
         for logger in self.loggers:
             self.loggers[logger].hup()
@@ -329,6 +333,7 @@ class SimpleMonitor:
         self.run_tests()
         module_logger.debug("Running recovery")
         self.do_recovery()
+        self.do_recovered()
         module_logger.debug("Running alerts")
         self.do_alerts()
         module_logger.debug("Running logs")


Test patch:
diff --git a/tests/test_monitor.py b/tests/test_monitor.py
index 9ab4610..2befb85 100644
--- a/tests/test_monitor.py
+++ b/tests/test_monitor.py
@@ -1,5 +1,6 @@
 # type: ignore
 import datetime
+import os
 import time
 import unittest
 
@@ -26,6 +27,7 @@ class TestMonitor(unittest.TestCase):
                 "tolerance": 2,
                 "remote_alert": 1,
                 "recover_command": "true",
+                "recovered_command": "true",
             }
         )
         self.assertEqual(m.name, "unnamed", "Monitor did not set name")
@@ -35,6 +37,9 @@ class TestMonitor(unittest.TestCase):
         self.assertEqual(
             m._recover_command, "true", "Monitor did not set recover_command"
         )
+        self.assertEqual(
+            m._recovered_command, "true", "Monitor did not set recovered_command"
+        )
         with self.assertRaises(ValueError):
             m.minimum_gap = -1
         with self.assertRaises(TypeError):
@@ -351,3 +356,25 @@ class TestMonitor(unittest.TestCase):
             "2 of 2 services failed. Fail after: 2",
             "compound monitor did not report failures properly",
         )
+
+    def test_recovery(self):
+        m = MonitorFail("fail1", {"recover_command": "touch did_recovery"})
+        try:
+            os.unlink("did_recovery")
+        except FileNotFoundError:
+            pass
+        m.run_test()
+        m.attempt_recover()
+        #  throws an exception if the file isn't there
+        os.stat("did_recovery")
+
+    def test_recovered(self):
+        m = MonitorFail("fail9", {"recovered_command": "touch did_recovered"})
+        try:
+            os.unlink("did_recovered")
+        except FileNotFoundError:
+            pass
+        for i in range(0, 6):
+            m.run_test()
+        m.run_recovered()
+        os.stat("did_recovered")