From c7752936541e277c7a5948ebc55df44ddc37695f Mon Sep 17 00:00:00 2001 From: Liam Young Date: Fri, 9 Dec 2016 14:01:14 +0000 Subject: [PATCH] Enable Memcache service for Token Caching With the release of 4.2.0 of keystonemiddleware using the in-process token cache is no longer recommended. It is recommended that a memcache backend to store tokens is used instead, This installs and configures memcache and configures ceilometer agent to use memcache for token caching. http://docs.openstack.org/releasenotes/keystonemiddleware/mitaka.html#id2 Change-Id: If7dd4c5472a7842f06d0fd44c1b2012c9c9d1075 --- hooks/ceilometer_hooks.py | 9 +++ hooks/ceilometer_utils.py | 25 ++++++- .../contrib/openstack/amulet/utils.py | 67 +++++++++++++++++++ .../charmhelpers/contrib/openstack/context.py | 25 +++++++ .../openstack/templates/memcached.conf | 53 +++++++++++++++ .../section-keystone-authtoken-mitaka | 3 + hooks/charmhelpers/contrib/openstack/utils.py | 25 +++++++ hooks/charmhelpers/core/host.py | 6 +- tests/basic_deployment.py | 6 ++ tests/charmhelpers/contrib/amulet/utils.py | 3 +- .../contrib/openstack/amulet/utils.py | 67 +++++++++++++++++++ unit_tests/test_actions_openstack_upgrade.py | 3 +- unit_tests/test_ceilometer_hooks.py | 8 +-- unit_tests/test_ceilometer_utils.py | 29 ++++++-- 14 files changed, 312 insertions(+), 17 deletions(-) create mode 100644 hooks/charmhelpers/contrib/openstack/templates/memcached.conf diff --git a/hooks/ceilometer_hooks.py b/hooks/ceilometer_hooks.py index d41e190..34c0de6 100755 --- a/hooks/ceilometer_hooks.py +++ b/hooks/ceilometer_hooks.py @@ -41,6 +41,7 @@ from ceilometer_utils import ( NOVA_SETTINGS, do_openstack_upgrade, assess_status, + get_packages, ) from charmhelpers.contrib.charmsupport import nrpe @@ -57,6 +58,14 @@ def install(): apt_install( filter_installed_packages(CEILOMETER_AGENT_PACKAGES), fatal=True) + # Call apt_install a 2nd time to allow packages which are enabled + # for specific OpenStack version to be installed . This is because + # Openstack version for a subordinate should be derived from the + # version of an installed package rather than relying on + # openstack-origin which would not be present in a subordinate. + apt_install( + filter_installed_packages(get_packages()), + fatal=True) @hooks.hook('nova-ceilometer-relation-joined') diff --git a/hooks/ceilometer_utils.py b/hooks/ceilometer_utils.py index 617c17d..0f7c4fe 100644 --- a/hooks/ceilometer_utils.py +++ b/hooks/ceilometer_utils.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from copy import deepcopy from charmhelpers.contrib.openstack import ( context, templating, @@ -27,6 +28,8 @@ from charmhelpers.contrib.openstack.utils import ( pause_unit, resume_unit, os_application_version_set, + token_cache_pkgs, + enable_memcache, ) from charmhelpers.core.hookenv import ( config, @@ -47,6 +50,7 @@ CEILOMETER_AGENT_PACKAGES = [ VERSION_PACKAGE = 'ceilometer-common' NOVA_CONF = "/etc/nova/nova.conf" +MEMCACHED_CONF = '/etc/memcached.conf' NOVA_SETTINGS = { "nova": { @@ -70,11 +74,11 @@ CONFIG_FILES = { CEILOMETER_CONF: { 'hook_contexts': [ CeilometerServiceContext(ssl_dir=CEILOMETER_CONF_DIR), - context.InternalEndpointContext()], + context.InternalEndpointContext(), + context.MemcacheContext()], 'services': CEILOMETER_AGENT_SERVICES - } + }, } - TEMPLATES = 'templates' REQUIRED_INTERFACES = { @@ -99,9 +103,20 @@ def register_configs(): for conf in CONFIG_FILES: configs.register(conf, CONFIG_FILES[conf]['hook_contexts']) + if enable_memcache(release=release): + configs.register(MEMCACHED_CONF, [context.MemcacheContext()]) + return configs +def get_packages(): + release = (get_os_codename_package('ceilometer-common', fatal=False) or + 'icehouse') + packages = deepcopy(CEILOMETER_AGENT_PACKAGES) + packages.extend(token_cache_pkgs(release=release)) + return packages + + def restart_map(): ''' Determine the correct resource map to be passed to @@ -110,6 +125,8 @@ def restart_map(): :returns: dict: A dictionary mapping config file to lists of services that should be restarted when file changes. ''' + release = (get_os_codename_package('ceilometer-common', fatal=False) or + 'icehouse') _map = {} for f, ctxt in CONFIG_FILES.iteritems(): svcs = [] @@ -117,6 +134,8 @@ def restart_map(): svcs.append(svc) if svcs: _map[f] = svcs + if enable_memcache(release=release): + _map[MEMCACHED_CONF] = ['memcached'] return _map diff --git a/hooks/charmhelpers/contrib/openstack/amulet/utils.py b/hooks/charmhelpers/contrib/openstack/amulet/utils.py index 6a0ba83..2b0a562 100644 --- a/hooks/charmhelpers/contrib/openstack/amulet/utils.py +++ b/hooks/charmhelpers/contrib/openstack/amulet/utils.py @@ -1133,3 +1133,70 @@ class OpenStackAmuletUtils(AmuletUtils): else: msg = 'No message retrieved.' amulet.raise_status(amulet.FAIL, msg) + + def validate_memcache(self, sentry_unit, conf, os_release, + earliest_release=5, section='keystone_authtoken', + check_kvs=None): + """Check Memcache is running and is configured to be used + + Example call from Amulet test: + + def test_110_memcache(self): + u.validate_memcache(self.neutron_api_sentry, + '/etc/neutron/neutron.conf', + self._get_openstack_release()) + + :param sentry_unit: sentry unit + :param conf: OpenStack config file to check memcache settings + :param os_release: Current OpenStack release int code + :param earliest_release: Earliest Openstack release to check int code + :param section: OpenStack config file section to check + :param check_kvs: Dict of settings to check in config file + :returns: None + """ + if os_release < earliest_release: + self.log.debug('Skipping memcache checks for deployment. {} <' + 'mitaka'.format(os_release)) + return + _kvs = check_kvs or {'memcached_servers': 'inet6:[::1]:11211'} + self.log.debug('Checking memcached is running') + ret = self.validate_services_by_name({sentry_unit: ['memcached']}) + if ret: + amulet.raise_status(amulet.FAIL, msg='Memcache running check' + 'failed {}'.format(ret)) + else: + self.log.debug('OK') + self.log.debug('Checking memcache url is configured in {}'.format( + conf)) + if self.validate_config_data(sentry_unit, conf, section, _kvs): + message = "Memcache config error in: {}".format(conf) + amulet.raise_status(amulet.FAIL, msg=message) + else: + self.log.debug('OK') + self.log.debug('Checking memcache configuration in ' + '/etc/memcached.conf') + contents = self.file_contents_safe(sentry_unit, '/etc/memcached.conf', + fatal=True) + ubuntu_release, _ = self.run_cmd_unit(sentry_unit, 'lsb_release -cs') + if ubuntu_release <= 'trusty': + memcache_listen_addr = 'ip6-localhost' + else: + memcache_listen_addr = '::1' + expected = { + '-p': '11211', + '-l': memcache_listen_addr} + found = [] + for key, value in expected.items(): + for line in contents.split('\n'): + if line.startswith(key): + self.log.debug('Checking {} is set to {}'.format( + key, + value)) + assert value == line.split()[-1] + self.log.debug(line.split()[-1]) + found.append(key) + if sorted(found) == sorted(expected.keys()): + self.log.debug('OK') + else: + message = "Memcache config error in: /etc/memcached.conf" + amulet.raise_status(amulet.FAIL, msg=message) diff --git a/hooks/charmhelpers/contrib/openstack/context.py b/hooks/charmhelpers/contrib/openstack/context.py index d5b3a33..91ddcec 100644 --- a/hooks/charmhelpers/contrib/openstack/context.py +++ b/hooks/charmhelpers/contrib/openstack/context.py @@ -90,6 +90,7 @@ from charmhelpers.contrib.network.ip import ( from charmhelpers.contrib.openstack.utils import ( config_flags_parser, get_host_ip, + enable_memcache, ) from charmhelpers.core.unitdata import kv @@ -1512,3 +1513,27 @@ class AppArmorContext(OSContextGenerator): "".format(self.ctxt['aa_profile'], self.ctxt['aa_profile_mode'])) raise e + + +class MemcacheContext(OSContextGenerator): + """Memcache context + + This context provides options for configuring a local memcache client and + server + """ + def __call__(self): + ctxt = {} + ctxt['use_memcache'] = enable_memcache(config('openstack-origin')) + if ctxt['use_memcache']: + # Trusty version of memcached does not support ::1 as a listen + # address so use host file entry instead + if lsb_release()['DISTRIB_CODENAME'].lower() > 'trusty': + ctxt['memcache_server'] = '::1' + else: + ctxt['memcache_server'] = 'ip6-localhost' + ctxt['memcache_server_formatted'] = '[::1]' + ctxt['memcache_port'] = '11211' + ctxt['memcache_url'] = 'inet6:{}:{}'.format( + ctxt['memcache_server_formatted'], + ctxt['memcache_port']) + return ctxt diff --git a/hooks/charmhelpers/contrib/openstack/templates/memcached.conf b/hooks/charmhelpers/contrib/openstack/templates/memcached.conf new file mode 100644 index 0000000..26cb037 --- /dev/null +++ b/hooks/charmhelpers/contrib/openstack/templates/memcached.conf @@ -0,0 +1,53 @@ +############################################################################### +# [ WARNING ] +# memcached configuration file maintained by Juju +# local changes may be overwritten. +############################################################################### + +# memcached default config file +# 2003 - Jay Bonci +# This configuration file is read by the start-memcached script provided as +# part of the Debian GNU/Linux distribution. + +# Run memcached as a daemon. This command is implied, and is not needed for the +# daemon to run. See the README.Debian that comes with this package for more +# information. +-d + +# Log memcached's output to /var/log/memcached +logfile /var/log/memcached.log + +# Be verbose +# -v + +# Be even more verbose (print client commands as well) +# -vv + +# Start with a cap of 64 megs of memory. It's reasonable, and the daemon default +# Note that the daemon will grow to this size, but does not start out holding this much +# memory +-m 64 + +# Default connection port is 11211 +-p {{ memcache_port }} + +# Run the daemon as root. The start-memcached will default to running as root if no +# -u command is present in this config file +-u memcache + +# Specify which IP address to listen on. The default is to listen on all IP addresses +# This parameter is one of the only security measures that memcached has, so make sure +# it's listening on a firewalled interface. +-l {{ memcache_server }} + +# Limit the number of simultaneous incoming connections. The daemon default is 1024 +# -c 1024 + +# Lock down all paged memory. Consult with the README and homepage before you do this +# -k + +# Return error when memory is exhausted (rather than removing items) +# -M + +# Maximize core file limit +# -r diff --git a/hooks/charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka b/hooks/charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka index 7c6f0c3..8e6889e 100644 --- a/hooks/charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka +++ b/hooks/charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka @@ -14,4 +14,7 @@ project_name = {{ admin_tenant_name }} username = {{ admin_user }} password = {{ admin_password }} signing_dir = {{ signing_dir }} +{% if use_memcache == true %} +memcached_servers = {{ memcache_url }} +{% endif -%} {% endif -%} diff --git a/hooks/charmhelpers/contrib/openstack/utils.py b/hooks/charmhelpers/contrib/openstack/utils.py index 553ff8f..59f9f51 100644 --- a/hooks/charmhelpers/contrib/openstack/utils.py +++ b/hooks/charmhelpers/contrib/openstack/utils.py @@ -1925,3 +1925,28 @@ def os_application_version_set(package): application_version_set(os_release(package)) else: application_version_set(application_version) + + +def enable_memcache(source=None, release=None): + """Determine if memcache should be enabled on the local unit + + @param source: source string for charm + @param release: release of OpenStack currently deployed + @returns boolean Whether memcache should be enabled + """ + if not release: + release = get_os_codename_install_source(source) + return release >= 'mitaka' + + +def token_cache_pkgs(source=None, release=None): + """Determine additional packages needed for token caching + + @param source: source string for charm + @param release: release of OpenStack currently deployed + @returns List of package to enable token caching + """ + packages = [] + if enable_memcache(source=source, release=release): + packages.extend(['memcached', 'python-memcache']) + return packages diff --git a/hooks/charmhelpers/core/host.py b/hooks/charmhelpers/core/host.py index 04cadb3..af5bd64 100644 --- a/hooks/charmhelpers/core/host.py +++ b/hooks/charmhelpers/core/host.py @@ -306,15 +306,17 @@ def add_user_to_group(username, group): subprocess.check_call(cmd) -def rsync(from_path, to_path, flags='-r', options=None): +def rsync(from_path, to_path, flags='-r', options=None, timeout=None): """Replicate the contents of a path""" options = options or ['--delete', '--executability'] cmd = ['/usr/bin/rsync', flags] + if timeout: + cmd = ['timeout', str(timeout)] + cmd cmd.extend(options) cmd.append(from_path) cmd.append(to_path) log(" ".join(cmd)) - return subprocess.check_output(cmd).decode('UTF-8').strip() + return subprocess.check_output(cmd, stderr=subprocess.STDOUT).decode('UTF-8').strip() def symlink(source, destination): diff --git a/tests/basic_deployment.py b/tests/basic_deployment.py index 24f185b..7b8f70c 100644 --- a/tests/basic_deployment.py +++ b/tests/basic_deployment.py @@ -259,6 +259,12 @@ class CeiloAgentBasicDeployment(OpenStackAmuletDeployment): u.log.debug('OK') + def test_130_memcache(self): + u.validate_memcache(self.ceil_agent_sentry, + '/etc/ceilometer/ceilometer.conf', + self._get_openstack_release(), + earliest_release=self.trusty_mitaka) + def test_200_ceilometer_identity_relation(self): """Verify the ceilometer to keystone identity-service relation data""" u.log.debug('Checking ceilometer to keystone identity-service ' diff --git a/tests/charmhelpers/contrib/amulet/utils.py b/tests/charmhelpers/contrib/amulet/utils.py index 8e13ab1..f9e4c3a 100644 --- a/tests/charmhelpers/contrib/amulet/utils.py +++ b/tests/charmhelpers/contrib/amulet/utils.py @@ -148,7 +148,8 @@ class AmuletUtils(object): for service_name in services_list: if (self.ubuntu_releases.index(release) >= systemd_switch or - service_name in ['rabbitmq-server', 'apache2']): + service_name in ['rabbitmq-server', 'apache2', + 'memcached']): # init is systemd (or regular sysv) cmd = 'sudo service {} status'.format(service_name) output, code = sentry_unit.run(cmd) diff --git a/tests/charmhelpers/contrib/openstack/amulet/utils.py b/tests/charmhelpers/contrib/openstack/amulet/utils.py index 6a0ba83..2b0a562 100644 --- a/tests/charmhelpers/contrib/openstack/amulet/utils.py +++ b/tests/charmhelpers/contrib/openstack/amulet/utils.py @@ -1133,3 +1133,70 @@ class OpenStackAmuletUtils(AmuletUtils): else: msg = 'No message retrieved.' amulet.raise_status(amulet.FAIL, msg) + + def validate_memcache(self, sentry_unit, conf, os_release, + earliest_release=5, section='keystone_authtoken', + check_kvs=None): + """Check Memcache is running and is configured to be used + + Example call from Amulet test: + + def test_110_memcache(self): + u.validate_memcache(self.neutron_api_sentry, + '/etc/neutron/neutron.conf', + self._get_openstack_release()) + + :param sentry_unit: sentry unit + :param conf: OpenStack config file to check memcache settings + :param os_release: Current OpenStack release int code + :param earliest_release: Earliest Openstack release to check int code + :param section: OpenStack config file section to check + :param check_kvs: Dict of settings to check in config file + :returns: None + """ + if os_release < earliest_release: + self.log.debug('Skipping memcache checks for deployment. {} <' + 'mitaka'.format(os_release)) + return + _kvs = check_kvs or {'memcached_servers': 'inet6:[::1]:11211'} + self.log.debug('Checking memcached is running') + ret = self.validate_services_by_name({sentry_unit: ['memcached']}) + if ret: + amulet.raise_status(amulet.FAIL, msg='Memcache running check' + 'failed {}'.format(ret)) + else: + self.log.debug('OK') + self.log.debug('Checking memcache url is configured in {}'.format( + conf)) + if self.validate_config_data(sentry_unit, conf, section, _kvs): + message = "Memcache config error in: {}".format(conf) + amulet.raise_status(amulet.FAIL, msg=message) + else: + self.log.debug('OK') + self.log.debug('Checking memcache configuration in ' + '/etc/memcached.conf') + contents = self.file_contents_safe(sentry_unit, '/etc/memcached.conf', + fatal=True) + ubuntu_release, _ = self.run_cmd_unit(sentry_unit, 'lsb_release -cs') + if ubuntu_release <= 'trusty': + memcache_listen_addr = 'ip6-localhost' + else: + memcache_listen_addr = '::1' + expected = { + '-p': '11211', + '-l': memcache_listen_addr} + found = [] + for key, value in expected.items(): + for line in contents.split('\n'): + if line.startswith(key): + self.log.debug('Checking {} is set to {}'.format( + key, + value)) + assert value == line.split()[-1] + self.log.debug(line.split()[-1]) + found.append(key) + if sorted(found) == sorted(expected.keys()): + self.log.debug('OK') + else: + message = "Memcache config error in: /etc/memcached.conf" + amulet.raise_status(amulet.FAIL, msg=message) diff --git a/unit_tests/test_actions_openstack_upgrade.py b/unit_tests/test_actions_openstack_upgrade.py index 7628232..07b369e 100644 --- a/unit_tests/test_actions_openstack_upgrade.py +++ b/unit_tests/test_actions_openstack_upgrade.py @@ -18,7 +18,8 @@ import os os.environ['JUJU_UNIT_NAME'] = 'ceilometer' with patch('ceilometer_utils.register_configs') as register_configs: - import openstack_upgrade + with patch('ceilometer_utils.restart_map') as restart_map: + import openstack_upgrade from test_utils import ( CharmTestCase diff --git a/unit_tests/test_ceilometer_hooks.py b/unit_tests/test_ceilometer_hooks.py index 67a933b..21be451 100644 --- a/unit_tests/test_ceilometer_hooks.py +++ b/unit_tests/test_ceilometer_hooks.py @@ -39,6 +39,7 @@ TO_PATCH = [ 'do_openstack_upgrade', 'update_nrpe_config', 'is_relation_made', + 'get_packages', ] @@ -57,13 +58,12 @@ class CeilometerHooksTest(CharmTestCase): @patch('charmhelpers.core.hookenv.config') def test_install_hook(self, mock_config): - self.filter_installed_packages.return_value = \ - hooks.CEILOMETER_AGENT_PACKAGES + ceil_pkgs = ['pkg1', 'pkg2'] + self.filter_installed_packages.return_value = ceil_pkgs hooks.hooks.execute(['hooks/install']) self.assertTrue(self.configure_installation_source.called) self.apt_update.assert_called_with(fatal=True) - self.apt_install.assert_called_with(hooks.CEILOMETER_AGENT_PACKAGES, - fatal=True) + self.apt_install.assert_called_with(ceil_pkgs, fatal=True) @patch('charmhelpers.core.hookenv.config') def test_ceilometer_changed(self, mock_config): diff --git a/unit_tests/test_ceilometer_utils.py b/unit_tests/test_ceilometer_utils.py index fe41b00..ec560ef 100644 --- a/unit_tests/test_ceilometer_utils.py +++ b/unit_tests/test_ceilometer_utils.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from mock import call, MagicMock, patch +from mock import MagicMock, patch import ceilometer_utils as utils @@ -30,6 +30,8 @@ TO_PATCH = [ 'apt_upgrade', 'log', 'os_application_version_set', + 'token_cache_pkgs', + 'enable_memcache', ] @@ -42,19 +44,34 @@ class CeilometerUtilsTest(CharmTestCase): super(CeilometerUtilsTest, self).tearDown() def test_register_configs(self): + self.enable_memcache.return_value = False configs = utils.register_configs() - calls = [] - for conf in utils.CONFIG_FILES: - calls.append(call(conf, - utils.CONFIG_FILES[conf]['hook_contexts'])) - configs.register.assert_has_calls(calls, any_order=True) + registered_configs = [c[0][0] for c in configs.register.call_args_list] + self.assertTrue(utils.CEILOMETER_CONF in registered_configs) + self.assertFalse(utils.MEMCACHED_CONF in registered_configs) + + def test_register_configs_newton(self): + self.enable_memcache.return_value = True + configs = utils.register_configs() + registered_configs = [c[0][0] for c in configs.register.call_args_list] + for config in utils.CONFIG_FILES.keys(): + self.assertTrue(config in registered_configs) def test_restart_map(self): + self.enable_memcache.return_value = False restart_map = utils.restart_map() self.assertEquals(restart_map, {'/etc/ceilometer/ceilometer.conf': [ 'ceilometer-agent-compute']}) + def test_restart_map_newton(self): + self.enable_memcache.return_value = True + restart_map = utils.restart_map() + expect = { + '/etc/ceilometer/ceilometer.conf': ['ceilometer-agent-compute'], + '/etc/memcached.conf': ['memcached']} + self.assertEquals(restart_map, expect) + def test_do_openstack_upgrade(self): self.config.side_effect = self.test_config.get self.test_config.set('openstack-origin', 'cloud:precise-havana')