Merge lp:~james-page/charm-helpers/configure_source_redux into lp:charm-helpers

Proposed by James Page
Status: Merged
Merged at revision: 36
Proposed branch: lp:~james-page/charm-helpers/configure_source_redux
Merge into: lp:charm-helpers
Diff against target: 168 lines (+126/-9)
2 files modified
charmhelpers/fetch/__init__.py (+21/-9)
tests/fetch/test_fetch.py (+105/-0)
To merge this branch: bzr merge lp:~james-page/charm-helpers/configure_source_redux
Reviewer Review Type Date Requested Status
Liam Young (community) Approve
Review via email: mp+171079@code.launchpad.net

Description of the change

Redux of configure_source code in fetch helper

1) Fixed up a number of bugs and issues in the original code

2) Implement correct cloud: handling

3) Added tests for everything.

To post a comment you must log in.
38. By James Page

Rebase against trunk

39. By James Page

Fixup failing test for cloud: type

40. By James Page

Tidy lint

Revision history for this message
Liam Young (gnuoy) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'charmhelpers/fetch/__init__.py'
2--- charmhelpers/fetch/__init__.py 2013-05-19 18:17:25 +0000
3+++ charmhelpers/fetch/__init__.py 2013-06-26 13:21:26 +0000
4@@ -1,15 +1,27 @@
5 from yaml import safe_load
6-from core.hookenv import config_get
7-from subprocess import check_call
8+from charmhelpers.core.hookenv import config
9+from charmhelpers.core.host import (
10+ apt_install, apt_update, filter_installed_packages
11+)
12+import subprocess
13+
14+CLOUD_ARCHIVE = """# Ubuntu Cloud Archive
15+deb http://ubuntu-cloud.archive.canonical.com/ubuntu {} main
16+"""
17
18
19 def add_source(source, key=None):
20 if ((source.startswith('ppa:') or
21- source.startswith('cloud:') or
22 source.startswith('http:'))):
23- check_call('add-apt-repository', source)
24+ subprocess.check_call(['add-apt-repository', source])
25+ elif source.startswith('cloud:'):
26+ apt_install(filter_installed_packages(['ubuntu-cloud-keyring']),
27+ fatal=True)
28+ pocket = source.split(':')[-1]
29+ with open('/etc/apt/sources.list.d/cloud-archive.list', 'w') as apt:
30+ apt.write(CLOUD_ARCHIVE.format(pocket))
31 if key:
32- check_call('apt-key', 'import', key)
33+ subprocess.check_call(['apt-key', 'import', key])
34
35
36 class SourceConfigError(Exception):
37@@ -32,8 +44,8 @@
38
39 Note that 'null' (a.k.a. None) should not be quoted.
40 """
41- sources = safe_load(config_get(sources_var))
42- keys = safe_load(config_get(keys_var))
43+ sources = safe_load(config(sources_var))
44+ keys = safe_load(config(keys_var))
45 if isinstance(sources, basestring) and isinstance(keys, basestring):
46 add_source(sources, keys)
47 else:
48@@ -41,6 +53,6 @@
49 msg = 'Install sources and keys lists are different lengths'
50 raise SourceConfigError(msg)
51 for src_num in range(len(sources)):
52- add_source(sources[src_num], sources[src_num])
53+ add_source(sources[src_num], keys[src_num])
54 if update:
55- check_call(('apt-get', 'update'))
56+ apt_update(fatal=True)
57
58=== added directory 'tests/fetch'
59=== added file 'tests/fetch/__init__.py'
60=== added file 'tests/fetch/test_fetch.py'
61--- tests/fetch/test_fetch.py 1970-01-01 00:00:00 +0000
62+++ tests/fetch/test_fetch.py 2013-06-26 13:21:26 +0000
63@@ -0,0 +1,105 @@
64+from contextlib import contextmanager
65+from mock import patch, call, MagicMock
66+from testtools import TestCase
67+import yaml
68+
69+import charmhelpers.fetch as fetch
70+
71+
72+@contextmanager
73+def patch_open():
74+ '''Patch open() to allow mocking both open() itself and the file that is
75+ yielded.
76+
77+ Yields the mock for "open" and "file", respectively.'''
78+ mock_open = MagicMock(spec=open)
79+ mock_file = MagicMock(spec=file)
80+
81+ @contextmanager
82+ def stub_open(*args, **kwargs):
83+ mock_open(*args, **kwargs)
84+ yield mock_file
85+
86+ with patch('__builtin__.open', stub_open):
87+ yield mock_open, mock_file
88+
89+
90+class FetchTest(TestCase):
91+ @patch('subprocess.check_call')
92+ def test_add_source_ppa(self, check_call):
93+ source = "ppa:test-ppa"
94+ fetch.add_source(source=source)
95+ check_call.assert_called_with(['add-apt-repository',
96+ source])
97+
98+ @patch('subprocess.check_call')
99+ def test_add_source_http(self, check_call):
100+ source = "http://archive.ubuntu.com/ubuntu raring-backports main"
101+ fetch.add_source(source=source)
102+ check_call.assert_called_with(['add-apt-repository',
103+ source])
104+
105+ @patch.object(fetch, 'filter_installed_packages')
106+ @patch.object(fetch, 'apt_install')
107+ def test_add_source_cloud(self, apt_install, filter_pkg):
108+ source = "cloud:havana-updates"
109+ result = '''# Ubuntu Cloud Archive
110+deb http://ubuntu-cloud.archive.canonical.com/ubuntu havana-updates main
111+'''
112+ with patch_open() as (mock_open, mock_file):
113+ fetch.add_source(source=source)
114+ mock_file.write.assert_called_with(result)
115+ filter_pkg.assert_called_with(['ubuntu-cloud-keyring'])
116+
117+ @patch('subprocess.check_call')
118+ def test_add_source_http_and_key(self, check_call):
119+ source = "http://archive.ubuntu.com/ubuntu raring-backports main"
120+ key = "akey"
121+ fetch.add_source(source=source, key=key)
122+ check_call.assert_has_calls([
123+ call(['add-apt-repository', source]),
124+ call(['apt-key', 'import', key])
125+ ])
126+
127+ @patch.object(fetch, 'config')
128+ @patch.object(fetch, 'add_source')
129+ def test_configure_sources_single_source(self, add_source, config):
130+ config.side_effect = ['source', 'key']
131+ fetch.configure_sources()
132+ add_source.assert_called_with('source', 'key')
133+
134+ @patch.object(fetch, 'config')
135+ @patch.object(fetch, 'add_source')
136+ def test_configure_sources_multiple_sources(self, add_source, config):
137+ sources = ["sourcea", "sourceb"]
138+ keys = ["keya", None]
139+ config.side_effect = [
140+ yaml.dump(sources),
141+ yaml.dump(keys)
142+ ]
143+ fetch.configure_sources()
144+ add_source.assert_has_calls([
145+ call('sourcea', 'keya'),
146+ call('sourceb', None)
147+ ])
148+
149+ @patch.object(fetch, 'config')
150+ @patch.object(fetch, 'add_source')
151+ def test_configure_sources_missing_keys(self, add_source, config):
152+ sources = ["sourcea", "sourceb"]
153+ keys = ["keya"] # Second key is missing
154+ config.side_effect = [
155+ yaml.dump(sources),
156+ yaml.dump(keys)
157+ ]
158+ self.assertRaises(fetch.SourceConfigError, fetch.configure_sources)
159+
160+ @patch.object(fetch, 'apt_update')
161+ @patch.object(fetch, 'config')
162+ @patch.object(fetch, 'add_source')
163+ def test_configure_sources_apt_update_called(self, add_source, config,
164+ apt_update):
165+ config.side_effect = ['source', 'key']
166+ fetch.configure_sources(update=True)
167+ add_source.assert_called_with('source', 'key')
168+ apt_update.assertCalled()

Subscribers

People subscribed via source and target branches