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

Proposed by Liam Young
Status: Merged
Merged at revision: 669
Proposed branch: lp:~gnuoy/charm-helpers/memcache-pkg-option
Merge into: lp:charm-helpers
Diff against target: 92 lines (+41/-9)
3 files modified
charmhelpers/contrib/openstack/context.py (+10/-1)
charmhelpers/contrib/openstack/utils.py (+13/-5)
tests/contrib/openstack/test_os_utils.py (+18/-3)
To merge this branch: bzr merge lp:~gnuoy/charm-helpers/memcache-pkg-option
Reviewer Review Type Date Requested Status
Alex Kavanagh Approve
charmers Pending
Review via email: mp+313236@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Alex Kavanagh (ajkavanagh) wrote :

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
670. By Liam Young

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

Revision history for this message
Alex Kavanagh (ajkavanagh) wrote :

LGTM. Thanks for the comment.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/openstack/context.py'
2--- charmhelpers/contrib/openstack/context.py 2016-12-09 16:52:38 +0000
3+++ charmhelpers/contrib/openstack/context.py 2016-12-14 15:46:58 +0000
4@@ -1521,9 +1521,18 @@
5 This context provides options for configuring a local memcache client and
6 server
7 """
8+
9+ def __init__(self, package=None):
10+ """
11+ @param package: Package to examine to extrapolate OpenStack release.
12+ Used when charms have no openstack-origin config
13+ option (ie subordinates)
14+ """
15+ self.package = package
16+
17 def __call__(self):
18 ctxt = {}
19- ctxt['use_memcache'] = enable_memcache(config('openstack-origin'))
20+ ctxt['use_memcache'] = enable_memcache(package=self.package)
21 if ctxt['use_memcache']:
22 # Trusty version of memcached does not support ::1 as a listen
23 # address so use host file entry instead
24
25=== modified file 'charmhelpers/contrib/openstack/utils.py'
26--- charmhelpers/contrib/openstack/utils.py 2016-12-05 13:06:55 +0000
27+++ charmhelpers/contrib/openstack/utils.py 2016-12-14 15:46:58 +0000
28@@ -1927,16 +1927,24 @@
29 application_version_set(application_version)
30
31
32-def enable_memcache(source=None, release=None):
33+def enable_memcache(source=None, release=None, package=None):
34 """Determine if memcache should be enabled on the local unit
35
36- @param source: source string for charm
37 @param release: release of OpenStack currently deployed
38+ @param package: package to derive OpenStack version deployed
39 @returns boolean Whether memcache should be enabled
40 """
41- if not release:
42- release = get_os_codename_install_source(source)
43- return release >= 'mitaka'
44+ _release = None
45+ if release:
46+ _release = release
47+ else:
48+ _release = os_release(package, base='icehouse')
49+ if not _release:
50+ _release = get_os_codename_install_source(source)
51+
52+ # TODO: this should be changed to a numeric comparison using a known list
53+ # of releases and comparing by index.
54+ return _release >= 'mitaka'
55
56
57 def token_cache_pkgs(source=None, release=None):
58
59=== modified file 'tests/contrib/openstack/test_os_utils.py'
60--- tests/contrib/openstack/test_os_utils.py 2016-11-30 10:39:48 +0000
61+++ tests/contrib/openstack/test_os_utils.py 2016-12-14 15:46:58 +0000
62@@ -155,12 +155,27 @@
63 mock_git_os_codename_install_source.assert_called_once_with(
64 'cloud-pocket')
65
66+ @mock.patch.object(utils, 'os_release')
67 @mock.patch.object(utils, 'get_os_codename_install_source')
68- def test_enable_memcache(self, _get_os_codename_install_source):
69+ def test_enable_memcache(self, _get_os_codename_install_source,
70+ _os_release):
71+ # Check call with 'release'
72+ self.assertFalse(utils.enable_memcache(release='icehouse'))
73+ self.assertTrue(utils.enable_memcache(release='zebra'))
74+ # Check call with 'source'
75+ _os_release.return_value = None
76 _get_os_codename_install_source.return_value = 'icehouse'
77- self.assertFalse(utils.enable_memcache('distro'))
78+ self.assertFalse(utils.enable_memcache(source='distro'))
79+ _os_release.return_value = None
80 _get_os_codename_install_source.return_value = 'zebra'
81- self.assertTrue(utils.enable_memcache('distro'))
82+ self.assertTrue(utils.enable_memcache(source='distro'))
83+ # Check call with 'package'
84+ _os_release.return_value = 'icehouse'
85+ _get_os_codename_install_source.return_value = None
86+ self.assertFalse(utils.enable_memcache(package='pkg1'))
87+ _os_release.return_value = 'zebra'
88+ _get_os_codename_install_source.return_value = None
89+ self.assertTrue(utils.enable_memcache(package='pkg1'))
90
91 @mock.patch.object(utils, 'enable_memcache')
92 def test_enable_token_cache_pkgs(self, _enable_memcache):

Subscribers

People subscribed via source and target branches