Merge lp:~mew/charm-helpers/lp1202804 into lp:charm-helpers

Proposed by Matthew Wedgwood
Status: Merged
Merged at revision: 69
Proposed branch: lp:~mew/charm-helpers/lp1202804
Merge into: lp:charm-helpers
Diff against target: 542 lines (+126/-125)
11 files modified
charmhelpers/contrib/ansible/__init__.py (+2/-2)
charmhelpers/contrib/hahelpers/ceph.py (+4/-1)
charmhelpers/contrib/openstack/templating.py (+1/-1)
charmhelpers/contrib/openstack/utils.py (+3/-0)
charmhelpers/contrib/saltstack/__init__.py (+1/-1)
charmhelpers/core/host.py (+0/-44)
charmhelpers/fetch/__init__.py (+44/-3)
tests/contrib/ansible/test_ansible.py (+3/-5)
tests/contrib/saltstack/test_saltstates.py (+8/-9)
tests/core/test_host.py (+10/-56)
tests/fetch/test_fetch.py (+50/-3)
To merge this branch: bzr merge lp:~mew/charm-helpers/lp1202804
Reviewer Review Type Date Requested Status
Jacek Nykis (community) Approve
Charm Helper Maintainers Pending
Review via email: mp+179066@code.launchpad.net

Description of the change

Per lp:1202804, I've moved the apt-specific helpers out of core.host and into fetch as core.host is intended to be platform-agnostic.

To post a comment you must log in.
Revision history for this message
Jacek Nykis (jacekn) wrote :

Looks fine

review: Approve
Revision history for this message
Jacek Nykis (jacekn) wrote :

Looks fine

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/contrib/ansible/__init__.py'
2--- charmhelpers/contrib/ansible/__init__.py 2013-07-25 08:02:54 +0000
3+++ charmhelpers/contrib/ansible/__init__.py 2013-08-07 23:16:26 +0000
4@@ -89,8 +89,8 @@
5 """
6 if from_ppa:
7 charmhelpers.fetch.add_source('ppa:rquillo/ansible')
8- charmhelpers.core.host.apt_update(fatal=True)
9- charmhelpers.core.host.apt_install('ansible')
10+ charmhelpers.fetch.apt_update(fatal=True)
11+ charmhelpers.fetch.apt_install('ansible')
12 with open(ansible_hosts_path, 'w+') as hosts_file:
13 hosts_file.write('localhost ansible_connection=local')
14
15
16=== modified file 'charmhelpers/contrib/hahelpers/ceph.py'
17--- charmhelpers/contrib/hahelpers/ceph.py 2013-07-11 18:32:05 +0000
18+++ charmhelpers/contrib/hahelpers/ceph.py 2013-08-07 23:16:26 +0000
19@@ -26,8 +26,11 @@
20 INFO,
21 )
22
23+from charmhelpers.fetch import (
24+ apt_install,
25+)
26+
27 from charmhelpers.core.host import (
28- apt_install,
29 mount,
30 mounts,
31 service_start,
32
33=== modified file 'charmhelpers/contrib/openstack/templating.py'
34--- charmhelpers/contrib/openstack/templating.py 2013-07-12 08:18:11 +0000
35+++ charmhelpers/contrib/openstack/templating.py 2013-08-07 23:16:26 +0000
36@@ -1,6 +1,6 @@
37 import os
38
39-from charmhelpers.core.host import apt_install
40+from charmhelpers.fetch import apt_install
41
42 from charmhelpers.core.hookenv import (
43 log,
44
45=== modified file 'charmhelpers/contrib/openstack/utils.py'
46--- charmhelpers/contrib/openstack/utils.py 2013-07-18 11:13:34 +0000
47+++ charmhelpers/contrib/openstack/utils.py 2013-08-07 23:16:26 +0000
48@@ -17,6 +17,9 @@
49
50 from charmhelpers.core.host import (
51 lsb_release,
52+)
53+
54+from charmhelpers.fetch import (
55 apt_install,
56 )
57
58
59=== modified file 'charmhelpers/contrib/saltstack/__init__.py'
60--- charmhelpers/contrib/saltstack/__init__.py 2013-07-25 06:22:19 +0000
61+++ charmhelpers/contrib/saltstack/__init__.py 2013-08-07 23:16:26 +0000
62@@ -89,7 +89,7 @@
63 subprocess.check_call(['/usr/bin/apt-get', 'update'])
64 # We install salt-common as salt-minion would run the salt-minion
65 # daemon.
66- charmhelpers.core.host.apt_install('salt-common')
67+ charmhelpers.fetch.apt_install('salt-common')
68
69
70 def update_machine_state(state_path):
71
72=== modified file 'charmhelpers/core/host.py'
73--- charmhelpers/core/host.py 2013-07-30 12:24:47 +0000
74+++ charmhelpers/core/host.py 2013-08-07 23:16:26 +0000
75@@ -5,7 +5,6 @@
76 # Nick Moffitt <nick.moffitt@canonical.com>
77 # Matthew Wedgwood <matthew.wedgwood@canonical.com>
78
79-import apt_pkg
80 import os
81 import pwd
82 import grp
83@@ -136,49 +135,6 @@
84 target.write(content)
85
86
87-def filter_installed_packages(packages):
88- """Returns a list of packages that require installation"""
89- apt_pkg.init()
90- cache = apt_pkg.Cache()
91- _pkgs = []
92- for package in packages:
93- try:
94- p = cache[package]
95- p.current_ver or _pkgs.append(package)
96- except KeyError:
97- log('Package {} has no installation candidate.'.format(package),
98- level='WARNING')
99- _pkgs.append(package)
100- return _pkgs
101-
102-
103-def apt_install(packages, options=None, fatal=False):
104- """Install one or more packages"""
105- options = options or []
106- cmd = ['apt-get', '-y']
107- cmd.extend(options)
108- cmd.append('install')
109- if isinstance(packages, basestring):
110- cmd.append(packages)
111- else:
112- cmd.extend(packages)
113- log("Installing {} with options: {}".format(packages,
114- options))
115- if fatal:
116- subprocess.check_call(cmd)
117- else:
118- subprocess.call(cmd)
119-
120-
121-def apt_update(fatal=False):
122- """Update local apt cache"""
123- cmd = ['apt-get', 'update']
124- if fatal:
125- subprocess.check_call(cmd)
126- else:
127- subprocess.call(cmd)
128-
129-
130 def mount(device, mountpoint, options=None, persist=False):
131 '''Mount a filesystem'''
132 cmd_args = ['mount']
133
134=== modified file 'charmhelpers/fetch/__init__.py'
135--- charmhelpers/fetch/__init__.py 2013-07-23 07:58:41 +0000
136+++ charmhelpers/fetch/__init__.py 2013-08-07 23:16:26 +0000
137@@ -1,9 +1,6 @@
138 import importlib
139 from yaml import safe_load
140 from charmhelpers.core.host import (
141- apt_install,
142- apt_update,
143- filter_installed_packages,
144 lsb_release
145 )
146 from urlparse import (
147@@ -15,6 +12,7 @@
148 config,
149 log,
150 )
151+import apt_pkg
152
153 CLOUD_ARCHIVE = """# Ubuntu Cloud Archive
154 deb http://ubuntu-cloud.archive.canonical.com/ubuntu {} main
155@@ -24,6 +22,49 @@
156 """
157
158
159+def filter_installed_packages(packages):
160+ """Returns a list of packages that require installation"""
161+ apt_pkg.init()
162+ cache = apt_pkg.Cache()
163+ _pkgs = []
164+ for package in packages:
165+ try:
166+ p = cache[package]
167+ p.current_ver or _pkgs.append(package)
168+ except KeyError:
169+ log('Package {} has no installation candidate.'.format(package),
170+ level='WARNING')
171+ _pkgs.append(package)
172+ return _pkgs
173+
174+
175+def apt_install(packages, options=None, fatal=False):
176+ """Install one or more packages"""
177+ options = options or []
178+ cmd = ['apt-get', '-y']
179+ cmd.extend(options)
180+ cmd.append('install')
181+ if isinstance(packages, basestring):
182+ cmd.append(packages)
183+ else:
184+ cmd.extend(packages)
185+ log("Installing {} with options: {}".format(packages,
186+ options))
187+ if fatal:
188+ subprocess.check_call(cmd)
189+ else:
190+ subprocess.call(cmd)
191+
192+
193+def apt_update(fatal=False):
194+ """Update local apt cache"""
195+ cmd = ['apt-get', 'update']
196+ if fatal:
197+ subprocess.check_call(cmd)
198+ else:
199+ subprocess.call(cmd)
200+
201+
202 def add_source(source, key=None):
203 if ((source.startswith('ppa:') or
204 source.startswith('http:'))):
205
206=== modified file 'tests/contrib/ansible/test_ansible.py'
207--- tests/contrib/ansible/test_ansible.py 2013-07-25 06:22:19 +0000
208+++ tests/contrib/ansible/test_ansible.py 2013-08-07 23:16:26 +0000
209@@ -26,7 +26,6 @@
210 self.mock_core = patcher.start()
211 self.addCleanup(patcher.stop)
212
213-
214 hosts_file = tempfile.NamedTemporaryFile()
215 self.ansible_hosts_path = hosts_file.name
216 self.addCleanup(hosts_file.close)
217@@ -41,8 +40,8 @@
218
219 self.mock_fetch.add_source.assert_called_once_with(
220 'ppa:rquillo/ansible')
221- self.mock_core.host.apt_update.assert_called_once_with(fatal=True)
222- self.mock_core.host.apt_install.assert_called_once_with(
223+ self.mock_fetch.apt_update.assert_called_once_with(fatal=True)
224+ self.mock_fetch.apt_install.assert_called_once_with(
225 'ansible')
226
227 def test_no_ppa(self):
228@@ -50,7 +49,7 @@
229 from_ppa=False)
230
231 self.assertEqual(self.mock_fetch.add_source.call_count, 0)
232- self.mock_core.host.apt_install.assert_called_once_with(
233+ self.mock_fetch.apt_install.assert_called_once_with(
234 'ansible')
235
236 def test_writes_ansible_hosts(self):
237@@ -100,7 +99,6 @@
238 patcher.start()
239 self.addCleanup(patcher.stop)
240
241-
242 def test_calls_ansible_playbook(self):
243 charmhelpers.contrib.ansible.apply_playbook(
244 'playbooks/dependencies.yaml')
245
246=== modified file 'tests/contrib/saltstack/test_saltstates.py'
247--- tests/contrib/saltstack/test_saltstates.py 2013-07-25 06:22:19 +0000
248+++ tests/contrib/saltstack/test_saltstates.py 2013-08-07 23:16:26 +0000
249@@ -21,8 +21,8 @@
250 self.mock_subprocess = patcher.start()
251 self.addCleanup(patcher.stop)
252
253- patcher = mock.patch('charmhelpers.core')
254- self.mock_charmhelpers_core = patcher.start()
255+ patcher = mock.patch('charmhelpers.fetch')
256+ self.mock_charmhelpers_fetch = patcher.start()
257 self.addCleanup(patcher.stop)
258
259 def test_adds_ppa_by_default(self):
260@@ -30,16 +30,16 @@
261
262 self.assertEqual(self.mock_subprocess.check_call.call_count, 2)
263 self.assertEqual([(([
264- '/usr/bin/add-apt-repository',
265- '--yes',
266- 'ppa:saltstack/salt',
267- ],), {}),
268+ '/usr/bin/add-apt-repository',
269+ '--yes',
270+ 'ppa:saltstack/salt',
271+ ],), {}),
272 (([
273 '/usr/bin/apt-get',
274 'update',
275 ],), {})
276 ], self.mock_subprocess.check_call.call_args_list)
277- self.mock_charmhelpers_core.host.apt_install.assert_called_once_with(
278+ self.mock_charmhelpers_fetch.apt_install.assert_called_once_with(
279 'salt-common')
280
281 def test_no_ppa(self):
282@@ -47,7 +47,7 @@
283 from_ppa=False)
284
285 self.assertEqual(self.mock_subprocess.check_call.call_count, 0)
286- self.mock_charmhelpers_core.host.apt_install.assert_called_once_with(
287+ self.mock_charmhelpers_fetch.apt_install.assert_called_once_with(
288 'salt-common')
289
290
291@@ -150,7 +150,6 @@
292 "local_unit": "click-index/3",
293 }, result)
294
295-
296 def test_output_with_relation(self):
297 self.mock_config.return_value = {
298 'group_code_owner': 'webops_deploy',
299
300=== modified file 'tests/core/test_host.py'
301--- tests/core/test_host.py 2013-07-30 12:24:47 +0000
302+++ tests/core/test_host.py 2013-08-07 23:16:26 +0000
303@@ -7,6 +7,7 @@
304 from testtools import TestCase
305
306 from charmhelpers.core import host
307+from charmhelpers import fetch
308
309
310 MOUNT_LINES = ("""
311@@ -18,16 +19,6 @@
312 """rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0
313 """).strip().split('\n')
314
315-FAKE_APT_CACHE = {
316- # an installed package
317- 'vim': {
318- 'current_ver': '2:7.3.547-6ubuntu5'
319- },
320- # a uninstalled installation candidate
321- 'emacs': {
322- }
323-}
324-
325 LSB_RELEASE = u'''DISTRIB_ID=Ubuntu
326 DISTRIB_RELEASE=13.10
327 DISTRIB_CODENAME=saucy
328@@ -35,22 +26,6 @@
329 '''
330
331
332-def fake_apt_cache():
333- def _get(package):
334- pkg = MagicMock()
335- if package not in FAKE_APT_CACHE:
336- raise KeyError
337- pkg.name = package
338- if 'current_ver' in FAKE_APT_CACHE[package]:
339- pkg.current_ver = FAKE_APT_CACHE[package]['current_ver']
340- else:
341- pkg.current_ver = None
342- return pkg
343- cache = MagicMock()
344- cache.__getitem__.side_effect = _get
345- return cache
346-
347-
348 @contextmanager
349 def patch_open():
350 '''Patch open() to allow mocking both open() itself and the file that is
351@@ -435,7 +410,7 @@
352 packages = ['foo', 'bar']
353 options = ['--foo', '--bar']
354
355- host.apt_install(packages, options)
356+ fetch.apt_install(packages, options)
357
358 mock_call.assert_called_with(['apt-get', '-y', '--foo', '--bar',
359 'install', 'foo', 'bar'])
360@@ -445,7 +420,7 @@
361 def test_installs_apt_packages_without_options(self, log, mock_call):
362 packages = ['foo', 'bar']
363
364- host.apt_install(packages)
365+ fetch.apt_install(packages)
366
367 mock_call.assert_called_with(['apt-get', '-y', 'install', 'foo',
368 'bar'])
369@@ -456,7 +431,7 @@
370 packages = 'foo bar'
371 options = ['--foo', '--bar']
372
373- host.apt_install(packages, options)
374+ fetch.apt_install(packages, options)
375
376 mock_call.assert_called_with(['apt-get', '-y', '--foo', '--bar',
377 'install', 'foo bar'])
378@@ -467,42 +442,21 @@
379 packages = ['foo', 'bar']
380 options = ['--foo', '--bar']
381
382- host.apt_install(packages, options, fatal=True)
383+ fetch.apt_install(packages, options, fatal=True)
384
385 check_call.assert_called_with(['apt-get', '-y', '--foo', '--bar',
386 'install', 'foo', 'bar'])
387
388 @patch('subprocess.check_call')
389 def test_apt_update_fatal(self, check_call):
390- host.apt_update(fatal=True)
391+ fetch.apt_update(fatal=True)
392 check_call.assert_called_with(['apt-get', 'update'])
393
394 @patch('subprocess.call')
395 def test_apt_update_nonfatal(self, call):
396- host.apt_update()
397+ fetch.apt_update()
398 call.assert_called_with(['apt-get', 'update'])
399
400- @patch('apt_pkg.Cache')
401- def test_filter_packages_missing(self, cache):
402- cache.side_effect = fake_apt_cache
403- result = host.filter_installed_packages(['vim', 'emacs'])
404- self.assertEquals(result, ['emacs'])
405-
406- @patch('apt_pkg.Cache')
407- def test_filter_packages_none_missing(self, cache):
408- cache.side_effect = fake_apt_cache
409- result = host.filter_installed_packages(['vim'])
410- self.assertEquals(result, [])
411-
412- @patch.object(host, 'log')
413- @patch('apt_pkg.Cache')
414- def test_filter_packages_not_available(self, cache, log):
415- cache.side_effect = fake_apt_cache
416- result = host.filter_installed_packages(['vim', 'joe'])
417- self.assertEquals(result, ['joe'])
418- log.assert_called_with('Package joe has no installation candidate.',
419- level='WARNING')
420-
421 @patch('subprocess.check_output')
422 @patch.object(host, 'log')
423 def test_mounts_a_device(self, log, check_output):
424@@ -612,7 +566,7 @@
425 file_name = '/etc/missing.conf'
426 restart_map = {
427 file_name: ['test-service']
428- }
429+ }
430 exists.side_effect = [False, False]
431
432 @host.restart_on_change(restart_map)
433@@ -633,7 +587,7 @@
434 file_name = '/etc/missing.conf'
435 restart_map = {
436 file_name: ['test-service']
437- }
438+ }
439 exists.side_effect = [False, True]
440
441 @host.restart_on_change(restart_map)
442@@ -658,7 +612,7 @@
443 restart_map = {
444 file_name_one: ['test-service'],
445 file_name_two: ['test-service', 'test-service2']
446- }
447+ }
448 exists.side_effect = [False, True, True, True]
449
450 @host.restart_on_change(restart_map)
451
452=== modified file 'tests/fetch/test_fetch.py'
453--- tests/fetch/test_fetch.py 2013-07-23 07:58:41 +0000
454+++ tests/fetch/test_fetch.py 2013-08-07 23:16:26 +0000
455@@ -9,6 +9,32 @@
456 from charmhelpers import fetch
457 import yaml
458
459+FAKE_APT_CACHE = {
460+ # an installed package
461+ 'vim': {
462+ 'current_ver': '2:7.3.547-6ubuntu5'
463+ },
464+ # a uninstalled installation candidate
465+ 'emacs': {
466+ }
467+}
468+
469+
470+def fake_apt_cache():
471+ def _get(package):
472+ pkg = MagicMock()
473+ if package not in FAKE_APT_CACHE:
474+ raise KeyError
475+ pkg.name = package
476+ if 'current_ver' in FAKE_APT_CACHE[package]:
477+ pkg.current_ver = FAKE_APT_CACHE[package]['current_ver']
478+ else:
479+ pkg.current_ver = None
480+ return pkg
481+ cache = MagicMock()
482+ cache.__getitem__.side_effect = _get
483+ return cache
484+
485
486 @contextmanager
487 def patch_open():
488@@ -29,6 +55,27 @@
489
490
491 class FetchTest(TestCase):
492+ @patch('apt_pkg.Cache')
493+ def test_filter_packages_missing(self, cache):
494+ cache.side_effect = fake_apt_cache
495+ result = fetch.filter_installed_packages(['vim', 'emacs'])
496+ self.assertEquals(result, ['emacs'])
497+
498+ @patch('apt_pkg.Cache')
499+ def test_filter_packages_none_missing(self, cache):
500+ cache.side_effect = fake_apt_cache
501+ result = fetch.filter_installed_packages(['vim'])
502+ self.assertEquals(result, [])
503+
504+ @patch.object(fetch, 'log')
505+ @patch('apt_pkg.Cache')
506+ def test_filter_packages_not_available(self, cache, log):
507+ cache.side_effect = fake_apt_cache
508+ result = fetch.filter_installed_packages(['vim', 'joe'])
509+ self.assertEquals(result, ['joe'])
510+ log.assert_called_with('Package joe has no installation candidate.',
511+ level='WARNING')
512+
513 @patch('subprocess.check_call')
514 def test_add_source_ppa(self, check_call):
515 source = "ppa:test-ppa"
516@@ -136,7 +183,7 @@
517 "ftp://example.com/foo.tar.gz",
518 "https://example.com/foo.tgz",
519 "file://example.com/foo.tar.bz2",
520- )
521+ )
522 self.invalid_urls = (
523 "git://example.com/foo.tar.gz",
524 "http://example.com/foo",
525@@ -146,7 +193,7 @@
526 "lp:example/foo.tgz",
527 "file//example.com/foo.tar.bz2",
528 "garbage",
529- )
530+ )
531
532 @patch('charmhelpers.fetch.plugins')
533 def test_installs_remote(self, _plugins):
534@@ -235,7 +282,7 @@
535 "bzr+ssh://bazaar.launchpad.net/foo/bar",
536 "bzr+http://bazaar.launchpad.net/foo/bar",
537 "garbage",
538- )
539+ )
540 self.fh = fetch.BaseFetchHandler()
541
542 def test_handles_nothing(self):

Subscribers

People subscribed via source and target branches