From 9996730fb1a3eded162b54c6ca97731a145e9169 Mon Sep 17 00:00:00 2001
From: Joona Hoikkala <joohoi@users.noreply.github.com>
Date: Wed, 4 Apr 2018 00:05:37 +0300
Subject: [PATCH] If restart fails, try alternative restart command if
 available (#5500)

* Use alternative restart command if available in distro overrides
---
 certbot-apache/certbot_apache/configurator.py | 19 ++++++++++++++++++-
 .../certbot_apache/override_centos.py         |  1 +
 .../certbot_apache/override_gentoo.py         |  1 +
 .../certbot_apache/tests/centos_test.py       | 14 ++++++++++++++
 .../certbot_apache/tests/gentoo_test.py       |  8 ++++++++
 5 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/certbot-apache/certbot_apache/configurator.py b/certbot-apache/certbot_apache/configurator.py
index 8b996c675..722e94e18 100644
--- a/certbot-apache/certbot_apache/configurator.py
+++ b/certbot-apache/certbot_apache/configurator.py
@@ -2000,10 +2000,27 @@ class ApacheConfigurator(augeas_configurator.AugeasConfigurator):
         :raises .errors.MisconfigurationError: If reload fails
 
         """
+        error = ""
         try:
             util.run_script(self.constant("restart_cmd"))
         except errors.SubprocessError as err:
-            raise errors.MisconfigurationError(str(err))
+            logger.info("Unable to restart apache using %s",
+                        self.constant("restart_cmd"))
+            alt_restart = self.constant("restart_cmd_alt")
+            if alt_restart:
+                logger.debug("Trying alternative restart command: %s",
+                             alt_restart)
+                # There is an alternative restart command available
+                # This usually is "restart" verb while original is "graceful"
+                try:
+                    util.run_script(self.constant(
+                        "restart_cmd_alt"))
+                    return
+                except errors.SubprocessError as secerr:
+                    error = str(secerr)
+            else:
+                error = str(err)
+            raise errors.MisconfigurationError(error)
 
     def config_test(self):  # pylint: disable=no-self-use
         """Check the configuration of Apache for errors.
diff --git a/certbot-apache/certbot_apache/override_centos.py b/certbot-apache/certbot_apache/override_centos.py
index db6cd6fba..6e75e361d 100644
--- a/certbot-apache/certbot_apache/override_centos.py
+++ b/certbot-apache/certbot_apache/override_centos.py
@@ -21,6 +21,7 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         version_cmd=['apachectl', '-v'],
         apache_cmd="apachectl",
         restart_cmd=['apachectl', 'graceful'],
+        restart_cmd_alt=['apachectl', 'restart'],
         conftest_cmd=['apachectl', 'configtest'],
         enmod=None,
         dismod=None,
diff --git a/certbot-apache/certbot_apache/override_gentoo.py b/certbot-apache/certbot_apache/override_gentoo.py
index 92f1d4a20..165e44c96 100644
--- a/certbot-apache/certbot_apache/override_gentoo.py
+++ b/certbot-apache/certbot_apache/override_gentoo.py
@@ -21,6 +21,7 @@ class GentooConfigurator(configurator.ApacheConfigurator):
         version_cmd=['/usr/sbin/apache2', '-v'],
         apache_cmd="apache2ctl",
         restart_cmd=['apache2ctl', 'graceful'],
+        restart_cmd_alt=['apache2ctl', 'restart'],
         conftest_cmd=['apache2ctl', 'configtest'],
         enmod=None,
         dismod=None,
diff --git a/certbot-apache/certbot_apache/tests/centos_test.py b/certbot-apache/certbot_apache/tests/centos_test.py
index d7a2a2fd9..4ee8b5dcf 100644
--- a/certbot-apache/certbot_apache/tests/centos_test.py
+++ b/certbot-apache/certbot_apache/tests/centos_test.py
@@ -4,6 +4,8 @@ import unittest
 
 import mock
 
+from certbot import errors
+
 from certbot_apache import obj
 from certbot_apache import override_centos
 from certbot_apache.tests import util
@@ -121,5 +123,17 @@ class MultipleVhostsTestCentOS(util.ApacheTest):
         self.assertTrue("MOCK_NOSEP" in self.config.parser.variables.keys())
         self.assertEqual("NOSEP_VAL", self.config.parser.variables["NOSEP_TWO"])
 
+    @mock.patch("certbot_apache.configurator.util.run_script")
+    def test_alt_restart_works(self, mock_run_script):
+        mock_run_script.side_effect = [None, errors.SubprocessError, None]
+        self.config.restart()
+        self.assertEquals(mock_run_script.call_count, 3)
+
+    @mock.patch("certbot_apache.configurator.util.run_script")
+    def test_alt_restart_errors(self, mock_run_script):
+        mock_run_script.side_effect = [None,
+                                       errors.SubprocessError,
+                                       errors.SubprocessError]
+        self.assertRaises(errors.MisconfigurationError, self.config.restart)
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover
diff --git a/certbot-apache/certbot_apache/tests/gentoo_test.py b/certbot-apache/certbot_apache/tests/gentoo_test.py
index cfbaffac7..d32551267 100644
--- a/certbot-apache/certbot_apache/tests/gentoo_test.py
+++ b/certbot-apache/certbot_apache/tests/gentoo_test.py
@@ -4,6 +4,8 @@ import unittest
 
 import mock
 
+from certbot import errors
+
 from certbot_apache import override_gentoo
 from certbot_apache import obj
 from certbot_apache.tests import util
@@ -123,5 +125,11 @@ class MultipleVhostsTestGentoo(util.ApacheTest):
         self.assertEquals(len(self.config.parser.modules), 4)
         self.assertTrue("mod_another.c" in self.config.parser.modules)
 
+    @mock.patch("certbot_apache.configurator.util.run_script")
+    def test_alt_restart_works(self, mock_run_script):
+        mock_run_script.side_effect = [None, errors.SubprocessError, None]
+        self.config.restart()
+        self.assertEquals(mock_run_script.call_count, 3)
+
 if __name__ == "__main__":
     unittest.main()  # pragma: no cover