Merge lp:~gnuoy/charm-helpers/memcache into lp:charm-helpers

[Liam Young]

[Felipe Reyes (freyes)]

Hi Liam,

what's rationale to have a locahost only memcached process per unit?, is this to provide some sort of resilience during restart of openstack daemons?

Best,

[James Page (james-page)]

Felipe

This is to support use of memcache for token auth caching via the keystoneauth middleware that all projects use - the in-process cache is officially deprecated and support will be dropped in Ocata - using a local only memcache instance allows token auth caching across processes improving memory efficiency.

We could use a centralized memcache deployment, but I'd rather keep the failure domain unit centric for this feature.

[James Page (james-page)]

A few minor comments - other than those looks OK

review: Needs Fixing

[lp:~gnuoy/charm-helpers/memcache]

665. By Liam Young on 2016-11-30

Add header to memcached.conf to show it is Juju managed
Install python-memcache incase the OpenStack service does not have it down as a dependancy
Enable memcache on Mitaka+ for consistancy

[James Page (james-page)]

review: Approve

=== modified file 'charmhelpers/contrib/openstack/context.py'
--- charmhelpers/contrib/openstack/context.py	2016-11-26 15:31:11 +0000
+++ charmhelpers/contrib/openstack/context.py	2016-11-30 10:40:30 +0000
@@ -90,6 +90,7 @@
 from charmhelpers.contrib.openstack.utils import (
     config_flags_parser,
     get_host_ip,
+    enable_memcache,
 )
 from charmhelpers.core.unitdata import kv
 
@@ -1512,3 +1513,23 @@
                                   "".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']:
+            ctxt['memcache_server'] = '::1'
+            ctxt['memcache_server_formatted'] = '[::1]'
+            ctxt['memcache_port'] = '11211'
+            ctxt['memcache_url'] = 'inet6:{}:{}'.format(
+                ctxt['memcache_server_formatted'],
+                ctxt['memcache_port'])
+
+        return ctxt

=== added file 'charmhelpers/contrib/openstack/templates/memcached.conf'
--- charmhelpers/contrib/openstack/templates/memcached.conf	1970-01-01 00:00:00 +0000
+++ charmhelpers/contrib/openstack/templates/memcached.conf	2016-11-30 10:40:30 +0000
@@ -0,0 +1,53 @@
+###############################################################################
+# [ WARNING ]
+# memcached configuration file maintained by Juju
+# local changes may be overwritten.
+###############################################################################
+
+# memcached default config file
+# 2003 - Jay Bonci <[e-mail]>
+# 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

=== modified file 'charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka'
--- charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka	2016-11-11 18:38:30 +0000
+++ charmhelpers/contrib/openstack/templates/section-keystone-authtoken-mitaka	2016-11-30 10:40:30 +0000
@@ -14,4 +14,7 @@
 username = {{ admin_user }}
 password = {{ admin_password }}
 signing_dir = {{ signing_dir }}
+{% if use_memcache == true %}
+memcached_servers = {{ memcache_url }}
+{% endif -%}
 {% endif -%}

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2016-11-28 16:54:19 +0000
+++ charmhelpers/contrib/openstack/utils.py	2016-11-30 10:40:30 +0000
@@ -1925,3 +1925,28 @@
         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

=== modified file 'tests/contrib/openstack/test_os_contexts.py'
--- tests/contrib/openstack/test_os_contexts.py	2016-11-26 15:31:11 +0000
+++ tests/contrib/openstack/test_os_contexts.py	2016-11-30 10:40:30 +0000
@@ -3002,3 +3002,27 @@
         AA.setup_aa_profile()
         self.check_call.assert_called_with(['aa-disable', 'fake-aa-profile'])
         AA.manually_disable_aa_profile.assert_called_with()
+
+    @patch.object(context, 'enable_memcache')
+    def test_memcache_context(self, _enable_memcache):
+        _enable_memcache.return_value = True
+        config = {'openstack-origin': 'distro'}
+        self.config.side_effect = fake_config(config)
+        ctxt = context.MemcacheContext()
+        self.assertTrue(ctxt()['use_memcache'])
+        expect = {
+            'memcache_port': '11211',
+            'memcache_server': '::1',
+            'memcache_server_formatted': '[::1]',
+            'memcache_url': 'inet6:[::1]:11211',
+            'use_memcache': True}
+        self.assertEqual(ctxt(), expect)
+
+    @patch.object(context, 'enable_memcache')
+    def test_memcache_off_context(self, _enable_memcache):
+        _enable_memcache.return_value = False
+        config = {'openstack-origin': 'distro'}
+        self.config.side_effect = fake_config(config)
+        ctxt = context.MemcacheContext()
+        self.assertFalse(ctxt()['use_memcache'])
+        self.assertEqual(ctxt(), {'use_memcache': False})

=== modified file 'tests/contrib/openstack/test_os_utils.py'
--- tests/contrib/openstack/test_os_utils.py	2016-10-10 12:06:28 +0000
+++ tests/contrib/openstack/test_os_utils.py	2016-11-30 10:40:30 +0000
@@ -154,3 +154,18 @@
             'my-pkg', fatal=False)
         mock_git_os_codename_install_source.assert_called_once_with(
             'cloud-pocket')
+
+    @mock.patch.object(utils, 'get_os_codename_install_source')
+    def test_enable_memcache(self, _get_os_codename_install_source):
+        _get_os_codename_install_source.return_value = 'icehouse'
+        self.assertFalse(utils.enable_memcache('distro'))
+        _get_os_codename_install_source.return_value = 'zebra'
+        self.assertTrue(utils.enable_memcache('distro'))
+
+    @mock.patch.object(utils, 'enable_memcache')
+    def test_enable_token_cache_pkgs(self, _enable_memcache):
+        _enable_memcache.return_value = False
+        self.assertEqual(utils.token_cache_pkgs(source='distro'), [])
+        _enable_memcache.return_value = True
+        self.assertEqual(utils.token_cache_pkgs(source='distro'),
+                         ['memcached', 'python-memcache'])