Merge lp:~bloodearnest/charms/trusty/apache2/fix-mpm-type into lp:charms/trusty/apache2

[Simon Davy]

Fixes configuration of mpm_type is broken on trusty.

Fixes configuration of mpm_type is broken on trusty, which is broken completely.

Apache 2.4 on trusty defaults to mpm_event, and all mpm modules are compiled in, and require explicit a2en/dismodding. The charm was written for precise, and just purged/installed apache2-mpm-{type} package to a) install the module and b) switch the worker type.

In trusty, these packages are just transitional packages that install nothing and do not switch modules.

This change refactors the mpm management to work on trusty, and adds amulet test coverage for *all three* worker types (previously, mpm_event was *not* supported).

[lp:~bloodearnest/charms/trusty/apache2/fix-mpm-type]

68. By Simon Davy on 2015-03-13

make trusty only

[Tom Haddon (mthaddon)]

Looks good. I've confirmed the issue with the current charm, and that upgrading to this version fixes it so that setting the mpm_type config variable works as expected.

review: Approve

=== modified file 'Makefile'
--- Makefile	2015-03-09 16:32:43 +0000
+++ Makefile	2015-03-13 16:05:28 +0000
@@ -28,7 +28,8 @@
 
 lint:
 	@echo Checking for Python syntax...
-	@flake8 $(HOOKS_DIR) --ignore=E123 --exclude=$(HOOKS_DIR)/charmhelpers && echo OK
+	@flake8 $(HOOKS_DIR) --ignore=E123 --exclude=$(HOOKS_DIR)/charmhelpers && echo hooks OK
+	@flake8 tests --ignore=E123 && echo tests OK
 
 sourcedeps: $(PWD)/config-manager.txt
 	@echo Updating source dependencies...

=== modified file 'hooks/hooks.py'
--- hooks/hooks.py	2015-03-10 19:10:28 +0000
+++ hooks/hooks.py	2015-03-13 16:05:28 +0000
@@ -542,6 +542,45 @@
     return True
 
 
+MPM_TYPES = ['mpm_worker', 'mpm_prefork', 'mpm_event']
+
+
+def enable_mpm(config):
+    """Enables a particular mpm module.
+
+    Different from simply enabling a module, as one and only one mpm module
+    *must* be enabled.
+    """
+    # only do anything if value has changed, to avoid a needless restart
+    if not config.changed('mpm_type'):
+        return
+
+    mpm_type = config.get('mpm_type', '')
+    name = 'mpm_' + mpm_type
+    if name not in MPM_TYPES:
+        log('bad mpm_type: %s. Falling back to mpm_worker' % mpm_type)
+        name = 'mpm_worker'
+
+    # disable all other mpm modules
+    for mpm in MPM_TYPES:
+        if mpm != name:
+            return_value = subprocess.call(['/usr/sbin/a2dismod', mpm])
+            if return_value != 0:
+                return False
+
+    return_value = subprocess.call(['/usr/sbin/a2enmod', name])
+    if return_value != 0:
+        return False
+
+    if service_apache2("check"):
+        log("Switching mpm module to {}".format(name))
+        service_apache2("restart")  # must be a restart to switch mpm
+        return True
+    else:
+        log("Failed to switch mpm module to {}".format(name))
+        return False
+
+
 def config_changed():
     relationship_data = {}
     config_data = config_get()
@@ -567,12 +606,8 @@
         for module in module_list:
             disable_module(module)
 
-    if config_data['mpm_type'] == 'worker':
-        apt_get_install('apache2-mpm-worker')
-        apt_get_purge('apache2-mpm-prefork')
-    elif config_data['mpm_type'] == 'prefork':
-        apt_get_install('apache2-mpm-prefork')
-        apt_get_purge('apache2-mpm-worker')
+    enable_mpm(config_data)
+    # XXX we only configure the worker mpm?
     create_mpm_workerfile()
     create_security()
 

=== modified file 'hooks/tests/test_config_changed.py'
--- hooks/tests/test_config_changed.py	2015-03-10 19:10:28 +0000
+++ hooks/tests/test_config_changed.py	2015-03-13 16:05:28 +0000
@@ -36,9 +36,10 @@
     @patch('hooks.get_reverseproxy_data')
     @patch('hooks.service_apache2')
     @patch('hooks.relation_set')
+    @patch('hooks.enable_mpm')
     def test_config_changed_ensure_empty_site_dir(
-            self, mock_relation_set, mock_service_apache2,
-            mock_reverseproxy, mock_config_get,
+            self, mock_enable_mpm, mock_relation_set,
+            mock_service_apache2, mock_reverseproxy, mock_config_get,
             mock_relations_of_type, mock_relation_ids, mock_log,
             mock_create_mpm_workerfile, mock_create_security, mock_run,
             mock_update_nrpe_checks, mock_ship_logrotate_conf,

=== modified file 'tests/10-bundles-test.py'
--- tests/10-bundles-test.py	2014-10-29 00:10:14 +0000
+++ tests/10-bundles-test.py	2015-03-13 16:05:28 +0000
@@ -30,4 +30,4 @@
 
 
 if __name__ == '__main__':
-    unittest.main()
\ No newline at end of file
+    unittest.main()

=== added file 'tests/20-mpm-test.py'
--- tests/20-mpm-test.py	1970-01-01 00:00:00 +0000
+++ tests/20-mpm-test.py	2015-03-13 16:05:28 +0000
@@ -0,0 +1,51 @@
+#!/usr/bin/env python3
+import os
+import time
+import unittest
+import amulet
+
+
+class BundleTest(unittest.TestCase):
+    """ Create a class for testing the charm in the unit test framework. """
+    @classmethod
+    def setUpClass(cls):
+        """ Set up an amulet deployment using the bundle. """
+        d = amulet.Deployment(series='trusty')
+        d.add('apache2', os.path.join(os.path.dirname(__file__), os.pardir))
+        d.setup(180)
+        cls.d = d
+        cls.unit = d.sentry.unit['apache2/0']
+        output, code = cls.unit.run('curl localhost')
+
+    def assert_mpm(self, mpm):
+        cmd = (". /etc/apache2/envvars && apache2 -V 2>/dev/null "
+               "| grep MPM | awk -F: '{print $2}' | xargs")
+        self.d.configure('apache2', {'mpm_type': mpm})
+        self.d.sentry.wait()
+        # the above doesn't seem to work
+        time.sleep(10)
+        # enable default web site so we can check for a valid config
+        output, code = self.unit.run(
+            'a2ensite 000-default.conf && service apache2 reload')
+        time.sleep(10)
+        # enable default web site so we can check for a valid config
+        output, code = self.unit.run(cmd)
+        self.assertEqual(code, 0)
+        self.assertIn(mpm, output)
+        output, code = self.unit.run('curl localhost')
+        if code != 0:
+            raise Exception(output)
+        self.assertEqual(code, 0)
+
+    def test_mpm_worker(self):
+        self.assert_mpm('worker')
+
+    def test_mpm_prefork(self):
+        self.assert_mpm('prefork')
+
+    def test_mpm_event(self):
+        self.assert_mpm('event')
+
+
+if __name__ == '__main__':
+    unittest.main()