Merge lp:~hloeung/ubuntu-repository-cache/only-reload-apache2-when-needed-3 into lp:ubuntu-repository-cache

[Haw Loeung]

Only reload/graceful apache2 when required - LP:1916084

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Unable to determine commit message from repository - please click "Set commit message" and enter the commit message manually.

[Joel Sing (jsing)]

LGTM, see minor comment inline.

review: Approve (+1)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Merge proposal is approved, but source revision has changed, setting status to needs review.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change has no commit message, setting status to needs review.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 311

=== modified file 'lib/ubuntu_repository_cache/apache.py'
--- lib/ubuntu_repository_cache/apache.py	2021-02-19 05:25:57 +0000
+++ lib/ubuntu_repository_cache/apache.py	2021-02-19 09:01:14 +0000
@@ -208,18 +208,27 @@
         start()
 
 
-def configure_health_check():
-    config = hookenv.config()
-
-    LOG('Apache enable_healthcheck changed')
-    if not config['enable_healthcheck']:
-        subprocess.check_call(['a2dismod', 'cgi'])
-        LOG('Disabled health check endpoint')
-        return
-
-    subprocess.check_call(['touch', HEALTH_CHECK_LOG])
-    subprocess.check_call(['chown', HEALTH_CHECK_LOG_OWNER, HEALTH_CHECK_LOG])
-    subprocess.check_call(['a2enmod', 'cgi'])
+def configure_health_check(config=None):
+    if not config:
+        config = hookenv.config()
+
+    restart_required = False
+
+    if config.changed('enable_healthcheck'):
+        LOG('Apache enable_healthcheck changed')
+        if config['enable_healthcheck']:
+            cmd = 'a2enmod'
+            action = 'Enabled'
+        else:
+            cmd = 'a2dismod'
+            action = 'Disabled'
+
+        subprocess.check_call([cmd, 'cgi'])
+        LOG('{} health check endpoint'.format(action))
+        restart_required = True
+
+    subprocess.call(['touch', HEALTH_CHECK_LOG])
+    subprocess.call(['chown', HEALTH_CHECK_LOG_OWNER, HEALTH_CHECK_LOG])
 
     apache_path = unitdata.kv().get('apache-root')
     healthcheck_path = os.path.join(apache_path, 'health-check')
@@ -228,8 +237,8 @@
     with open('files/health_check.py', 'r') as f:
         content = f.read()
     host.write_file(path=healthcheck_script, content=content, perms=0o755)
-    LOG('Enabled health check endpoint')
-    start()
+
+    return restart_required
 
 
 @util.run_as_user('root')

=== modified file 'tests/unit/test_apache.py'
--- tests/unit/test_apache.py	2021-02-19 05:23:14 +0000
+++ tests/unit/test_apache.py	2021-02-19 09:01:14 +0000
@@ -10,7 +10,7 @@
 sys.modules['charms.layer'] = mock.MagicMock()
 
 from charms.layer import status  # NOQA: E402
-from charmhelpers.core import unitdata  # NOQA: E402
+from charmhelpers.core import hookenv, unitdata  # NOQA: E402
 
 # Add path to where our reactive layer lives and import.
 sys.path.append(os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__)))))
@@ -58,25 +58,26 @@
         status.maintenance.reset_mock()
 
     @mock.patch('charmhelpers.core.host.write_file')
-    @mock.patch('lib.ubuntu_repository_cache.apache.start')
     @mock.patch('os.chown')
     @mock.patch('os.setegid')
     @mock.patch('os.seteuid')
+    @mock.patch('subprocess.call')
     @mock.patch('subprocess.check_call')
-    def test_configure_health_check(self, check_call, seteuid, setegid, chown, start, write_file):
-        self.mock_config.return_value = {
-            'enable_healthcheck': False,
-        }
-        apache.configure_health_check()
+    def test_configure_health_check(self, check_call, call, seteuid, setegid, chown, write_file):
+        config = hookenv.Config({'enable_healthcheck': True})
+        self.assertEqual(apache.configure_health_check(config), True)
+        check_call.assert_called_with(['a2enmod', 'cgi'])
+
+        check_call.reset_mock()
+        config = hookenv.Config({'enable_healthcheck': False})
+        self.assertEqual(apache.configure_health_check(config), True)
         check_call.assert_called_with(['a2dismod', 'cgi'])
 
-        os.mkdir(os.path.join(self.tmpdir, 'apache'))
-
-        self.mock_config.return_value = {
-            'enable_healthcheck': True,
-        }
-        apache.configure_health_check()
-        check_call.assert_called_with(['a2enmod', 'cgi'])
+        check_call.reset_mock()
+        config = hookenv.Config({'enable_healthcheck': True})
+        config.save()
+        config = hookenv.Config({'enable_healthcheck': True})
+        self.assertEqual(apache.configure_health_check(config), False)
 
     @mock.patch('charmhelpers.core.templating.render')
     @mock.patch('lib.ubuntu_repository_cache.apache.start')