From 91298e4e3592cd3f3df4e8dc719644f551ff8c19 Mon Sep 17 00:00:00 2001
From: Joona Hoikkala <joona@kuori.org>
Date: Sat, 31 Mar 2018 22:00:20 +0300
Subject: [PATCH] Use alternative restart command if available in distro
 overrides

---
 certbot-apache/certbot_apache/configurator.py | 19 ++++++++++++++++++-
 .../certbot_apache/override_centos.py         |  3 ++-
 .../certbot_apache/override_gentoo.py         |  3 ++-
 .../certbot_apache/tests/centos_test.py       | 14 ++++++++++++++
 .../certbot_apache/tests/gentoo_test.py       |  8 ++++++++
 5 files changed, 44 insertions(+), 3 deletions(-)

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 8a20cfd72..6e75e361d 100644
--- a/certbot-apache/certbot_apache/override_centos.py
+++ b/certbot-apache/certbot_apache/override_centos.py
@@ -20,7 +20,8 @@ class CentOSConfigurator(configurator.ApacheConfigurator):
         logs_root="/var/log/httpd",
         version_cmd=['apachectl', '-v'],
         apache_cmd="apachectl",
-        restart_cmd=['apachectl', 'restart'],
+        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 ed340493a..b35e51865 100644
--- a/certbot-apache/certbot_apache/override_gentoo.py
+++ b/certbot-apache/certbot_apache/override_gentoo.py
@@ -20,7 +20,8 @@ class GentooConfigurator(configurator.ApacheConfigurator):
         logs_root="/var/log/apache2",
         version_cmd=['/usr/sbin/apache2', '-v'],
         apache_cmd="apache2ctl",
-        restart_cmd=['apache2ctl', 'restart'],
+        restart_cmd=['apache2ctl', 'graceful'],
+        restart_cmd_alt=['apachectl', '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