Merge lp:~adam-collard/charms/trusty/swift-proxy/lib-in-python-package into lp:~openstack-charmers-archive/charms/trusty/swift-proxy/next

Proposed by Adam Collard
Status: Merged
Merged at revision: 107
Proposed branch: lp:~adam-collard/charms/trusty/swift-proxy/lib-in-python-package
Merge into: lp:~openstack-charmers-archive/charms/trusty/swift-proxy/next
Diff against target: 396 lines (+120/-52)
13 files modified
.coveragerc (+1/-0)
Makefile (+3/-3)
charm-helpers-hooks.yaml (+1/-1)
charmhelpers/contrib/openstack/utils.py (+4/-0)
charmhelpers/contrib/storage/linux/ceph.py (+2/-11)
charmhelpers/core/kernel.py (+68/-0)
hooks/swift_hooks.py (+1/-1)
lib/swift_context.py (+7/-1)
setup.cfg (+1/-1)
unit_tests/__init__.py (+0/-2)
unit_tests/test_swift_context.py (+7/-7)
unit_tests/test_swift_hooks.py (+2/-1)
unit_tests/test_swift_utils.py (+23/-24)
To merge this branch: bzr merge lp:~adam-collard/charms/trusty/swift-proxy/lib-in-python-package
Reviewer Review Type Date Requested Status
Chris Glass (community) Approve
Landscape Pending
Review via email: mp+270304@code.launchpad.net

Description of the change

With the addition of actions to the OpenStack charms, there's a need to share code between actions and hooks. This branch moves library code to a top-level ./lib/ and then symlinks that into ./hooks/lib, likewise for charm-helpers.

This should be a plain refactor with no change in behaviour, but laying the ground for an actions/lib symlink and an actions/charm-helpers symlink.

(see https://code.launchpad.net/~adam-collard/charms/precise/swift-storage/lib-in-python-package/+merge/265105 for the equivalent MP for swift-storage)

To post a comment you must log in.
110. By Adam Collard on 2015-09-07

Add explanatory comment

charm_lint_check #9540 swift-proxy-next for adam-collard mp270304
    LINT OK: passed

Build: http://10.245.162.77:8080/job/charm_lint_check/9540/

charm_unit_test #8781 swift-proxy-next for adam-collard mp270304
    UNIT OK: passed

Build: http://10.245.162.77:8080/job/charm_unit_test/8781/

Chris Glass (tribaal) wrote :

I think this looks good, small comment inline.

111. By Adam Collard on 2015-09-07

Add missing @property (tribaal's review)

Chris Glass (tribaal) wrote :

Looks good!

review: Approve

charm_amulet_test #6311 swift-proxy-next for adam-collard mp270304
    AMULET FAIL: amulet-test failed

AMULET Results (max last 2 lines):
make: *** [test] Error 1
ERROR:root:Make target returned non-zero.

Full amulet test output: http://paste.ubuntu.com/12307641/
Build: http://10.245.162.77:8080/job/charm_amulet_test/6311/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '.coveragerc'
2--- .coveragerc 2013-11-18 11:16:28 +0000
3+++ .coveragerc 2015-09-07 14:15:28 +0000
4@@ -4,3 +4,4 @@
5 if __name__ == .__main__.:
6 include=
7 hooks/swift_*
8+ lib/swift_*
9
10=== modified file 'Makefile'
11--- Makefile 2015-04-16 21:32:07 +0000
12+++ Makefile 2015-09-07 14:15:28 +0000
13@@ -1,9 +1,7 @@
14-#!/usr/bin/make
15 PYTHON := /usr/bin/env python
16
17 lint:
18- @flake8 --exclude hooks/charmhelpers --ignore=E125 hooks
19- @flake8 --exclude hooks/charmhelpers --ignore=E125 unit_tests tests
20+ @flake8 --exclude hooks unit_tests tests lib
21 @charm proof
22
23 unit_test:
24@@ -29,3 +27,5 @@
25 publish: lint unit_test
26 bzr push lp:charms/swift-proxy
27 bzr push lp:charms/trusty/swift-proxy
28+
29+.PHONY: lint unit_test test sync publish
30
31=== modified file 'charm-helpers-hooks.yaml'
32--- charm-helpers-hooks.yaml 2015-07-31 13:11:02 +0000
33+++ charm-helpers-hooks.yaml 2015-09-07 14:15:28 +0000
34@@ -1,5 +1,5 @@
35 branch: lp:charm-helpers
36-destination: hooks/charmhelpers
37+destination: charmhelpers
38 include:
39 - core
40 - cli
41
42=== renamed directory 'hooks/charmhelpers' => 'charmhelpers'
43=== modified file 'charmhelpers/contrib/openstack/utils.py'
44--- hooks/charmhelpers/contrib/openstack/utils.py 2015-09-03 09:41:22 +0000
45+++ charmhelpers/contrib/openstack/utils.py 2015-09-07 14:15:28 +0000
46@@ -114,6 +114,7 @@
47 ('2.2.1', 'kilo'),
48 ('2.2.2', 'kilo'),
49 ('2.3.0', 'liberty'),
50+ ('2.4.0', 'liberty'),
51 ])
52
53 # >= Liberty version->codename mapping
54@@ -142,6 +143,9 @@
55 'glance-common': OrderedDict([
56 ('11.0.0', 'liberty'),
57 ]),
58+ 'openstack-dashboard': OrderedDict([
59+ ('8.0.0', 'liberty'),
60+ ]),
61 }
62
63 DEFAULT_LOOPBACK_SIZE = '5G'
64
65=== modified file 'charmhelpers/contrib/storage/linux/ceph.py'
66--- hooks/charmhelpers/contrib/storage/linux/ceph.py 2015-07-29 10:47:50 +0000
67+++ charmhelpers/contrib/storage/linux/ceph.py 2015-09-07 14:15:28 +0000
68@@ -56,6 +56,8 @@
69 apt_install,
70 )
71
72+from charmhelpers.core.kernel import modprobe
73+
74 KEYRING = '/etc/ceph/ceph.client.{}.keyring'
75 KEYFILE = '/etc/ceph/ceph.client.{}.key'
76
77@@ -288,17 +290,6 @@
78 os.chown(data_src_dst, uid, gid)
79
80
81-# TODO: re-use
82-def modprobe(module):
83- """Load a kernel module and configure for auto-load on reboot."""
84- log('Loading kernel module', level=INFO)
85- cmd = ['modprobe', module]
86- check_call(cmd)
87- with open('/etc/modules', 'r+') as modules:
88- if module not in modules.read():
89- modules.write(module)
90-
91-
92 def copy_files(src, dst, symlinks=False, ignore=None):
93 """Copy files from src to dst."""
94 for item in os.listdir(src):
95
96=== added file 'charmhelpers/core/kernel.py'
97--- charmhelpers/core/kernel.py 1970-01-01 00:00:00 +0000
98+++ charmhelpers/core/kernel.py 2015-09-07 14:15:28 +0000
99@@ -0,0 +1,68 @@
100+#!/usr/bin/env python
101+# -*- coding: utf-8 -*-
102+
103+# Copyright 2014-2015 Canonical Limited.
104+#
105+# This file is part of charm-helpers.
106+#
107+# charm-helpers is free software: you can redistribute it and/or modify
108+# it under the terms of the GNU Lesser General Public License version 3 as
109+# published by the Free Software Foundation.
110+#
111+# charm-helpers is distributed in the hope that it will be useful,
112+# but WITHOUT ANY WARRANTY; without even the implied warranty of
113+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
114+# GNU Lesser General Public License for more details.
115+#
116+# You should have received a copy of the GNU Lesser General Public License
117+# along with charm-helpers. If not, see <http://www.gnu.org/licenses/>.
118+
119+__author__ = "Jorge Niedbalski <jorge.niedbalski@canonical.com>"
120+
121+from charmhelpers.core.hookenv import (
122+ log,
123+ INFO
124+)
125+
126+from subprocess import check_call, check_output
127+import re
128+
129+
130+def modprobe(module, persist=True):
131+ """Load a kernel module and configure for auto-load on reboot."""
132+ cmd = ['modprobe', module]
133+
134+ log('Loading kernel module %s' % module, level=INFO)
135+
136+ check_call(cmd)
137+ if persist:
138+ with open('/etc/modules', 'r+') as modules:
139+ if module not in modules.read():
140+ modules.write(module)
141+
142+
143+def rmmod(module, force=False):
144+ """Remove a module from the linux kernel"""
145+ cmd = ['rmmod']
146+ if force:
147+ cmd.append('-f')
148+ cmd.append(module)
149+ log('Removing kernel module %s' % module, level=INFO)
150+ return check_call(cmd)
151+
152+
153+def lsmod():
154+ """Shows what kernel modules are currently loaded"""
155+ return check_output(['lsmod'],
156+ universal_newlines=True)
157+
158+
159+def is_module_loaded(module):
160+ """Checks if a kernel module is already loaded"""
161+ matches = re.findall('^%s[ ]+' % module, lsmod(), re.M)
162+ return len(matches) > 0
163+
164+
165+def update_initramfs(version='all'):
166+ """Updates an initramfs image"""
167+ return check_call(["update-initramfs", "-k", version, "-u"])
168
169=== modified file 'hooks/swift_hooks.py'
170--- hooks/swift_hooks.py 2015-05-11 08:04:17 +0000
171+++ hooks/swift_hooks.py 2015-09-07 14:15:28 +0000
172@@ -8,7 +8,7 @@
173 CalledProcessError,
174 )
175
176-from swift_utils import (
177+from lib.swift_utils import (
178 SwiftProxyCharmException,
179 register_configs,
180 restart_map,
181
182=== added directory 'lib'
183=== added file 'lib/__init__.py'
184=== renamed file 'hooks/swift_context.py' => 'lib/swift_context.py'
185--- hooks/swift_context.py 2015-04-13 22:57:02 +0000
186+++ lib/swift_context.py 2015-09-07 14:15:28 +0000
187@@ -50,9 +50,15 @@
188
189 class ApacheSSLContext(SSLContext):
190 interfaces = ['https']
191- external_ports = [config('bind-port')]
192 service_namespace = 'swift'
193
194+ # We make this a property so that we avoid import-time
195+ # dependencies on config()
196+
197+ @property
198+ def external_ports(self):
199+ return [config('bind-port')]
200+
201
202 class SwiftRingContext(OSContextGenerator):
203
204
205=== renamed file 'hooks/swift_utils.py' => 'lib/swift_utils.py'
206=== modified file 'setup.cfg'
207--- setup.cfg 2013-11-18 11:16:28 +0000
208+++ setup.cfg 2015-09-07 14:15:28 +0000
209@@ -2,4 +2,4 @@
210 verbosity=1
211 with-coverage=1
212 cover-erase=1
213-cover-package=hooks
214+cover-package=lib,swift_hooks
215
216=== modified file 'unit_tests/__init__.py'
217--- unit_tests/__init__.py 2013-11-18 11:16:28 +0000
218+++ unit_tests/__init__.py 2015-09-07 14:15:28 +0000
219@@ -1,2 +0,0 @@
220-import sys
221-sys.path.append('hooks')
222
223=== modified file 'unit_tests/test_swift_context.py'
224--- unit_tests/test_swift_context.py 2014-12-01 23:54:03 +0000
225+++ unit_tests/test_swift_context.py 2015-09-07 14:15:28 +0000
226@@ -6,12 +6,12 @@
227
228
229 with mock.patch('charmhelpers.core.hookenv.config'):
230- import swift_context
231+ import lib.swift_context as swift_context
232
233
234 class SwiftContextTestCase(unittest.TestCase):
235
236- @mock.patch('swift_context.config')
237+ @mock.patch('lib.swift_context.config')
238 def test_get_swift_hash_file(self, mock_config):
239 expected = '##FILEHASH##'
240 with tempfile.NamedTemporaryFile() as tmpfile:
241@@ -24,7 +24,7 @@
242 self.assertFalse(mock_config.called)
243 self.assertEqual(expected, hash)
244
245- @mock.patch('swift_context.config')
246+ @mock.patch('lib.swift_context.config')
247 def test_get_swift_hash_config(self, mock_config):
248 expected = '##CFGHASH##'
249 mock_config.return_value = expected
250@@ -38,8 +38,8 @@
251 self.assertTrue(mock_config.called)
252 self.assertEqual(expected, hash)
253
254- @mock.patch('swift_context.service_name')
255- @mock.patch('swift_context.config')
256+ @mock.patch('lib.swift_context.service_name')
257+ @mock.patch('lib.swift_context.config')
258 def test_get_swift_hash_env(self, mock_config, mock_service_name):
259 mock_config.return_value = None
260 mock_service_name.return_value = "testsvc"
261@@ -47,10 +47,10 @@
262 swift_context.SWIFT_HASH_FILE = tmpfile
263 with mock.patch('swift_context.os.environ.get') as mock_env_get:
264 mock_env_get.return_value = str(uuid.uuid4())
265- hash = swift_context.get_swift_hash()
266+ hash_ = swift_context.get_swift_hash()
267 mock_env_get.assert_called_with('JUJU_ENV_UUID')
268
269 with open(tmpfile, 'r') as fd:
270- self.assertEqual(hash, fd.read())
271+ self.assertEqual(hash_, fd.read())
272
273 self.assertTrue(mock_config.called)
274
275=== modified file 'unit_tests/test_swift_hooks.py'
276--- unit_tests/test_swift_hooks.py 2015-06-11 23:22:54 +0000
277+++ unit_tests/test_swift_hooks.py 2015-09-07 14:15:28 +0000
278@@ -1,8 +1,9 @@
279 from mock import patch
280+import sys
281 import unittest
282 import uuid
283
284-
285+sys.path.append("hooks")
286 with patch('charmhelpers.core.hookenv.log'):
287 import swift_hooks
288
289
290=== modified file 'unit_tests/test_swift_utils.py'
291--- unit_tests/test_swift_utils.py 2014-12-18 11:58:28 +0000
292+++ unit_tests/test_swift_utils.py 2015-09-07 14:15:28 +0000
293@@ -5,9 +5,8 @@
294 import uuid
295 import unittest
296
297-
298 with mock.patch('charmhelpers.core.hookenv.config'):
299- import swift_utils
300+ import lib.swift_utils as swift_utils
301
302
303 def init_ring_paths(tmpdir):
304@@ -21,16 +20,16 @@
305
306 class SwiftUtilsTestCase(unittest.TestCase):
307
308- @mock.patch('swift_utils.get_broker_token')
309- @mock.patch('swift_utils.update_www_rings')
310- @mock.patch('swift_utils.get_builders_checksum')
311- @mock.patch('swift_utils.get_rings_checksum')
312- @mock.patch('swift_utils.balance_rings')
313- @mock.patch('swift_utils.log')
314- @mock.patch('swift_utils.os.path.exists')
315- @mock.patch('swift_utils.is_elected_leader')
316- @mock.patch('swift_utils.get_min_part_hours')
317- @mock.patch('swift_utils.set_min_part_hours')
318+ @mock.patch('lib.swift_utils.get_broker_token')
319+ @mock.patch('lib.swift_utils.update_www_rings')
320+ @mock.patch('lib.swift_utils.get_builders_checksum')
321+ @mock.patch('lib.swift_utils.get_rings_checksum')
322+ @mock.patch('lib.swift_utils.balance_rings')
323+ @mock.patch('lib.swift_utils.log')
324+ @mock.patch('lib.swift_utils.os.path.exists')
325+ @mock.patch('lib.swift_utils.is_elected_leader')
326+ @mock.patch('lib.swift_utils.get_min_part_hours')
327+ @mock.patch('lib.swift_utils.set_min_part_hours')
328 def test_update_rings(self, mock_set_min_hours,
329 mock_get_min_hours,
330 mock_is_elected_leader, mock_path_exists,
331@@ -76,13 +75,13 @@
332 self.assertTrue(mock_set_min_hours.called)
333 self.assertTrue(mock_balance_rings.called)
334
335- @mock.patch('swift_utils.get_broker_token')
336- @mock.patch('swift_utils.balance_rings')
337- @mock.patch('swift_utils.log')
338- @mock.patch('swift_utils.is_elected_leader')
339- @mock.patch('swift_utils.config')
340- @mock.patch('swift_utils.update_www_rings')
341- @mock.patch('swift_utils.cluster_sync_rings')
342+ @mock.patch('lib.swift_utils.get_broker_token')
343+ @mock.patch('lib.swift_utils.balance_rings')
344+ @mock.patch('lib.swift_utils.log')
345+ @mock.patch('lib.swift_utils.is_elected_leader')
346+ @mock.patch('lib.swift_utils.config')
347+ @mock.patch('lib.swift_utils.update_www_rings')
348+ @mock.patch('lib.swift_utils.cluster_sync_rings')
349 def test_sync_builders_and_rings_if_changed(self, mock_cluster_sync_rings,
350 mock_update_www_rings,
351 mock_config,
352@@ -114,7 +113,7 @@
353 self.assertTrue(mock_update_www_rings.called)
354 self.assertTrue(mock_cluster_sync_rings.called)
355
356- @mock.patch('swift_utils.get_www_dir')
357+ @mock.patch('lib.swift_utils.get_www_dir')
358 def test_mark_www_rings_deleted(self, mock_get_www_dir):
359 try:
360 tmpdir = tempfile.mkdtemp()
361@@ -123,7 +122,7 @@
362 finally:
363 shutil.rmtree(tmpdir)
364
365- @mock.patch('swift_utils.uuid')
366+ @mock.patch('lib.swift_utils.uuid')
367 def test_cluster_rpc_stop_proxy_request(self, mock_uuid):
368 mock_uuid.uuid4.return_value = 'test-uuid'
369 rpc = swift_utils.SwiftProxyClusterRPC()
370@@ -147,7 +146,7 @@
371 'stop-proxy-service-ack': None,
372 'sync-only-builders': None}, rq)
373
374- @mock.patch('swift_utils.uuid')
375+ @mock.patch('lib.swift_utils.uuid')
376 def test_cluster_rpc_stop_proxy_ack(self, mock_uuid):
377 mock_uuid.uuid4.return_value = 'token2'
378 rpc = swift_utils.SwiftProxyClusterRPC()
379@@ -161,7 +160,7 @@
380 'stop-proxy-service-ack': 'token1',
381 'sync-only-builders': None}, rq)
382
383- @mock.patch('swift_utils.uuid')
384+ @mock.patch('lib.swift_utils.uuid')
385 def test_cluster_rpc_sync_request(self, mock_uuid):
386 mock_uuid.uuid4.return_value = 'token2'
387 rpc = swift_utils.SwiftProxyClusterRPC()
388@@ -175,7 +174,7 @@
389 'stop-proxy-service-ack': None,
390 'sync-only-builders': None}, rq)
391
392- @mock.patch('swift_utils.uuid')
393+ @mock.patch('lib.swift_utils.uuid')
394 def test_cluster_rpc_notify_leader_changed(self, mock_uuid):
395 mock_uuid.uuid4.return_value = 'token1'
396 rpc = swift_utils.SwiftProxyClusterRPC()

Subscribers

People subscribed via source and target branches