Merge pull request #2992 from certbot/post-hook

Only run post-hook if pre-hook was (or would have been, if it existed)

certbot/cli.py

@@ -761,7 +761,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         "renew", "--post-hook",
         help="Command to be run in a shell after attempting to obtain/renew "
         " certificates. Can be used to deploy renewed certificates, or to restart"
-        " any servers that were stopped by --pre-hook.")
+        " any servers that were stopped by --pre-hook. This is only run if"
+        " an attempt was made to obtain/renew a certificate.")
     helpful.add(
         "renew", "--renew-hook",
         help="Command to be run in a shell once for each successfully renewed certificate."

certbot/hooks.py

@@ -39,7 +39,7 @@ def pre_hook(config):
     if config.pre_hook and not pre_hook.already:
         logger.info("Running pre-hook command: %s", config.pre_hook)
         _run_hook(config.pre_hook)
-        pre_hook.already = True
+    pre_hook.already = True
 
 pre_hook.already = False
 
@@ -50,6 +50,11 @@ def post_hook(config, final=False):
     we're called with final=True before actually doing anything.
     """
     if config.post_hook:
+        if not pre_hook.already:
+            logger.info("No renewals attempted, so not running post-hook")
+            if config.verb != "renew":
+                logger.warn("Sanity failure in renewal hooks")
+            return
         if final or config.verb != "renew":
             logger.info("Running post-hook command: %s", config.post_hook)
             _run_hook(config.post_hook)

certbot/main.py

@@ -94,7 +94,7 @@ def _auth_from_domains(le_client, config, domains, lineage=None):
             if lineage is False:
                 raise errors.Error("Certificate could not be obtained")
     finally:
-        hooks.post_hook(config)
+        hooks.post_hook(config, final=False)
 
     if not config.dry_run and not config.verb == "renew":
         _report_new_cert(config, lineage.cert, lineage.fullchain)

certbot/tests/hook_test.py

@@ -56,14 +56,22 @@ class HookTest(unittest.TestCase):
             return mock_logger.warning
 
     def test_pre_hook(self):
+        hooks.pre_hook.already = False
         config = mock.MagicMock(pre_hook="true")
         self._test_a_hook(config, hooks.pre_hook, 1)
         config = mock.MagicMock(pre_hook="")
         self._test_a_hook(config, hooks.pre_hook, 0)
 
     def test_post_hook(self):
+        hooks.pre_hook.already = False
+        # if pre-hook isn't called, post-hook shouldn't be
         config = mock.MagicMock(post_hook="true", verb="splonk")
+        self._test_a_hook(config, hooks.post_hook, 0)
+
+        config = mock.MagicMock(post_hook="true", verb="splonk")
+        self._test_a_hook(config, hooks.pre_hook, 1)
         self._test_a_hook(config, hooks.post_hook, 2)
+
         config = mock.MagicMock(post_hook="true", verb="renew")
         self._test_a_hook(config, hooks.post_hook, 0)