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

[James Page]

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.

[lp:~james-page/charm-helpers/configure_source_redux]

38. By James Page on 2013-06-25

Rebase against trunk

39. By James Page on 2013-06-26

Fixup failing test for cloud: type

40. By James Page on 2013-06-26

Tidy lint

[Liam Young (gnuoy)]

LGTM

review: Approve

=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2013-05-19 18:17:25 +0000
+++ charmhelpers/fetch/__init__.py	2013-06-26 13:21:26 +0000
@@ -1,15 +1,27 @@
 from yaml import safe_load
-from core.hookenv import config_get
-from subprocess import check_call
+from charmhelpers.core.hookenv import config
+from charmhelpers.core.host import (
+    apt_install, apt_update, filter_installed_packages
+)
+import subprocess
+
+CLOUD_ARCHIVE = """# Ubuntu Cloud Archive
+deb http://ubuntu-cloud.archive.canonical.com/ubuntu {} main
+"""
 
 
 def add_source(source, key=None):
     if ((source.startswith('ppa:') or
-         source.startswith('cloud:') or
          source.startswith('http:'))):
-        check_call('add-apt-repository', source)
+        subprocess.check_call(['add-apt-repository', source])
+    elif source.startswith('cloud:'):
+        apt_install(filter_installed_packages(['ubuntu-cloud-keyring']),
+                    fatal=True)
+        pocket = source.split(':')[-1]
+        with open('/etc/apt/sources.list.d/cloud-archive.list', 'w') as apt:
+            apt.write(CLOUD_ARCHIVE.format(pocket))
     if key:
-        check_call('apt-key', 'import', key)
+        subprocess.check_call(['apt-key', 'import', key])
 
 
 class SourceConfigError(Exception):
@@ -32,8 +44,8 @@
 
     Note that 'null' (a.k.a. None) should not be quoted.
     """
-    sources = safe_load(config_get(sources_var))
-    keys = safe_load(config_get(keys_var))
+    sources = safe_load(config(sources_var))
+    keys = safe_load(config(keys_var))
     if isinstance(sources, basestring) and isinstance(keys, basestring):
         add_source(sources, keys)
     else:
@@ -41,6 +53,6 @@
             msg = 'Install sources and keys lists are different lengths'
             raise SourceConfigError(msg)
         for src_num in range(len(sources)):
-            add_source(sources[src_num], sources[src_num])
+            add_source(sources[src_num], keys[src_num])
     if update:
-        check_call(('apt-get', 'update'))
+        apt_update(fatal=True)

=== added directory 'tests/fetch'
=== added file 'tests/fetch/__init__.py'
=== added file 'tests/fetch/test_fetch.py'
--- tests/fetch/test_fetch.py	1970-01-01 00:00:00 +0000
+++ tests/fetch/test_fetch.py	2013-06-26 13:21:26 +0000
@@ -0,0 +1,105 @@
+from contextlib import contextmanager
+from mock import patch, call, MagicMock
+from testtools import TestCase
+import yaml
+
+import charmhelpers.fetch as fetch
+
+
+@contextmanager
+def patch_open():
+    '''Patch open() to allow mocking both open() itself and the file that is
+    yielded.
+
+    Yields the mock for "open" and "file", respectively.'''
+    mock_open = MagicMock(spec=open)
+    mock_file = MagicMock(spec=file)
+
+    @contextmanager
+    def stub_open(*args, **kwargs):
+        mock_open(*args, **kwargs)
+        yield mock_file
+
+    with patch('__builtin__.open', stub_open):
+        yield mock_open, mock_file
+
+
+class FetchTest(TestCase):
+    @patch('subprocess.check_call')
+    def test_add_source_ppa(self, check_call):
+        source = "ppa:test-ppa"
+        fetch.add_source(source=source)
+        check_call.assert_called_with(['add-apt-repository',
+                                       source])
+
+    @patch('subprocess.check_call')
+    def test_add_source_http(self, check_call):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        fetch.add_source(source=source)
+        check_call.assert_called_with(['add-apt-repository',
+                                       source])
+
+    @patch.object(fetch, 'filter_installed_packages')
+    @patch.object(fetch, 'apt_install')
+    def test_add_source_cloud(self, apt_install, filter_pkg):
+        source = "cloud:havana-updates"
+        result = '''# Ubuntu Cloud Archive
+deb http://ubuntu-cloud.archive.canonical.com/ubuntu havana-updates main
+'''
+        with patch_open() as (mock_open, mock_file):
+            fetch.add_source(source=source)
+            mock_file.write.assert_called_with(result)
+        filter_pkg.assert_called_with(['ubuntu-cloud-keyring'])
+
+    @patch('subprocess.check_call')
+    def test_add_source_http_and_key(self, check_call):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        key = "akey"
+        fetch.add_source(source=source, key=key)
+        check_call.assert_has_calls([
+            call(['add-apt-repository', source]),
+            call(['apt-key', 'import', key])
+        ])
+
+    @patch.object(fetch, 'config')
+    @patch.object(fetch, 'add_source')
+    def test_configure_sources_single_source(self, add_source, config):
+        config.side_effect = ['source', 'key']
+        fetch.configure_sources()
+        add_source.assert_called_with('source', 'key')
+
+    @patch.object(fetch, 'config')
+    @patch.object(fetch, 'add_source')
+    def test_configure_sources_multiple_sources(self, add_source, config):
+        sources = ["sourcea", "sourceb"]
+        keys = ["keya", None]
+        config.side_effect = [
+            yaml.dump(sources),
+            yaml.dump(keys)
+        ]
+        fetch.configure_sources()
+        add_source.assert_has_calls([
+            call('sourcea', 'keya'),
+            call('sourceb', None)
+        ])
+
+    @patch.object(fetch, 'config')
+    @patch.object(fetch, 'add_source')
+    def test_configure_sources_missing_keys(self, add_source, config):
+        sources = ["sourcea", "sourceb"]
+        keys = ["keya"]  # Second key is missing
+        config.side_effect = [
+            yaml.dump(sources),
+            yaml.dump(keys)
+        ]
+        self.assertRaises(fetch.SourceConfigError, fetch.configure_sources)
+
+    @patch.object(fetch, 'apt_update')
+    @patch.object(fetch, 'config')
+    @patch.object(fetch, 'add_source')
+    def test_configure_sources_apt_update_called(self, add_source, config,
+                                                 apt_update):
+        config.side_effect = ['source', 'key']
+        fetch.configure_sources(update=True)
+        add_source.assert_called_with('source', 'key')
+        apt_update.assertCalled()