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

[Liam Young]

[Alex Kavanagh (ajkavanagh)]

Generally good. Just think we should add a comment as a TODO to fix the alpha comparison on the release as it's prone to breakage in the future.

review: Needs Fixing

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

670. By Liam Young on 2016-12-14

Add TODO as a reminder to switch to using a more rebust method for determining OpenStack release order

[Alex Kavanagh (ajkavanagh)]

LGTM. Thanks for the comment.

review: Approve

=== modified file 'charmhelpers/contrib/openstack/context.py'
--- charmhelpers/contrib/openstack/context.py	2016-12-09 16:52:38 +0000
+++ charmhelpers/contrib/openstack/context.py	2016-12-14 15:46:58 +0000
@@ -1521,9 +1521,18 @@
     This context provides options for configuring a local memcache client and
     server
     """
+
+    def __init__(self, package=None):
+        """
+        @param package: Package to examine to extrapolate OpenStack release.
+                        Used when charms have no openstack-origin config
+                        option (ie subordinates)
+        """
+        self.package = package
+
     def __call__(self):
         ctxt = {}
-        ctxt['use_memcache'] = enable_memcache(config('openstack-origin'))
+        ctxt['use_memcache'] = enable_memcache(package=self.package)
         if ctxt['use_memcache']:
             # Trusty version of memcached does not support ::1 as a listen
             # address so use host file entry instead

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2016-12-05 13:06:55 +0000
+++ charmhelpers/contrib/openstack/utils.py	2016-12-14 15:46:58 +0000
@@ -1927,16 +1927,24 @@
         application_version_set(application_version)
 
 
-def enable_memcache(source=None, release=None):
+def enable_memcache(source=None, release=None, package=None):
     """Determine if memcache should be enabled on the local unit
 
-    @param source: source string for charm
     @param release: release of OpenStack currently deployed
+    @param package: package to derive OpenStack version deployed
     @returns boolean Whether memcache should be enabled
     """
-    if not release:
-        release = get_os_codename_install_source(source)
-    return release >= 'mitaka'
+    _release = None
+    if release:
+        _release = release
+    else:
+        _release = os_release(package, base='icehouse')
+    if not _release:
+        _release = get_os_codename_install_source(source)
+
+    # TODO: this should be changed to a numeric comparison using a known list
+    # of releases and comparing by index.
+    return _release >= 'mitaka'
 
 
 def token_cache_pkgs(source=None, release=None):

=== modified file 'tests/contrib/openstack/test_os_utils.py'
--- tests/contrib/openstack/test_os_utils.py	2016-11-30 10:39:48 +0000
+++ tests/contrib/openstack/test_os_utils.py	2016-12-14 15:46:58 +0000
@@ -155,12 +155,27 @@
         mock_git_os_codename_install_source.assert_called_once_with(
             'cloud-pocket')
 
+    @mock.patch.object(utils, 'os_release')
     @mock.patch.object(utils, 'get_os_codename_install_source')
-    def test_enable_memcache(self, _get_os_codename_install_source):
+    def test_enable_memcache(self, _get_os_codename_install_source,
+                             _os_release):
+        # Check call with 'release'
+        self.assertFalse(utils.enable_memcache(release='icehouse'))
+        self.assertTrue(utils.enable_memcache(release='zebra'))
+        # Check call with 'source'
+        _os_release.return_value = None
         _get_os_codename_install_source.return_value = 'icehouse'
-        self.assertFalse(utils.enable_memcache('distro'))
+        self.assertFalse(utils.enable_memcache(source='distro'))
+        _os_release.return_value = None
         _get_os_codename_install_source.return_value = 'zebra'
-        self.assertTrue(utils.enable_memcache('distro'))
+        self.assertTrue(utils.enable_memcache(source='distro'))
+        # Check call with 'package'
+        _os_release.return_value = 'icehouse'
+        _get_os_codename_install_source.return_value = None
+        self.assertFalse(utils.enable_memcache(package='pkg1'))
+        _os_release.return_value = 'zebra'
+        _get_os_codename_install_source.return_value = None
+        self.assertTrue(utils.enable_memcache(package='pkg1'))
 
     @mock.patch.object(utils, 'enable_memcache')
     def test_enable_token_cache_pkgs(self, _enable_memcache):