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

[Matthew Wedgwood]

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.

[Jacek Nykis (jacekn)]

Looks fine

review: Approve

[Jacek Nykis (jacekn)]

Looks fine

review: Approve

=== modified file 'charmhelpers/contrib/ansible/__init__.py'
--- charmhelpers/contrib/ansible/__init__.py	2013-07-25 08:02:54 +0000
+++ charmhelpers/contrib/ansible/__init__.py	2013-08-07 23:16:26 +0000
@@ -89,8 +89,8 @@
     """
     if from_ppa:
         charmhelpers.fetch.add_source('ppa:rquillo/ansible')
-        charmhelpers.core.host.apt_update(fatal=True)
-    charmhelpers.core.host.apt_install('ansible')
+        charmhelpers.fetch.apt_update(fatal=True)
+    charmhelpers.fetch.apt_install('ansible')
     with open(ansible_hosts_path, 'w+') as hosts_file:
         hosts_file.write('localhost ansible_connection=local')
 

=== modified file 'charmhelpers/contrib/hahelpers/ceph.py'
--- charmhelpers/contrib/hahelpers/ceph.py	2013-07-11 18:32:05 +0000
+++ charmhelpers/contrib/hahelpers/ceph.py	2013-08-07 23:16:26 +0000
@@ -26,8 +26,11 @@
     INFO,
 )
 
+from charmhelpers.fetch import (
+    apt_install,
+)
+
 from charmhelpers.core.host import (
-    apt_install,
     mount,
     mounts,
     service_start,

=== modified file 'charmhelpers/contrib/openstack/templating.py'
--- charmhelpers/contrib/openstack/templating.py	2013-07-12 08:18:11 +0000
+++ charmhelpers/contrib/openstack/templating.py	2013-08-07 23:16:26 +0000
@@ -1,6 +1,6 @@
 import os
 
-from charmhelpers.core.host import apt_install
+from charmhelpers.fetch import apt_install
 
 from charmhelpers.core.hookenv import (
     log,

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2013-07-18 11:13:34 +0000
+++ charmhelpers/contrib/openstack/utils.py	2013-08-07 23:16:26 +0000
@@ -17,6 +17,9 @@
 
 from charmhelpers.core.host import (
     lsb_release,
+)
+
+from charmhelpers.fetch import (
     apt_install,
 )
 

=== modified file 'charmhelpers/contrib/saltstack/__init__.py'
--- charmhelpers/contrib/saltstack/__init__.py	2013-07-25 06:22:19 +0000
+++ charmhelpers/contrib/saltstack/__init__.py	2013-08-07 23:16:26 +0000
@@ -89,7 +89,7 @@
         subprocess.check_call(['/usr/bin/apt-get', 'update'])
     # We install salt-common as salt-minion would run the salt-minion
     # daemon.
-    charmhelpers.core.host.apt_install('salt-common')
+    charmhelpers.fetch.apt_install('salt-common')
 
 
 def update_machine_state(state_path):

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2013-07-30 12:24:47 +0000
+++ charmhelpers/core/host.py	2013-08-07 23:16:26 +0000
@@ -5,7 +5,6 @@
 #  Nick Moffitt <[e-mail]>
 #  Matthew Wedgwood <[e-mail]>
 
-import apt_pkg
 import os
 import pwd
 import grp
@@ -136,49 +135,6 @@
         target.write(content)
 
 
-def filter_installed_packages(packages):
-    """Returns a list of packages that require installation"""
-    apt_pkg.init()
-    cache = apt_pkg.Cache()
-    _pkgs = []
-    for package in packages:
-        try:
-            p = cache[package]
-            p.current_ver or _pkgs.append(package)
-        except KeyError:
-            log('Package {} has no installation candidate.'.format(package),
-                level='WARNING')
-            _pkgs.append(package)
-    return _pkgs
-
-
-def apt_install(packages, options=None, fatal=False):
-    """Install one or more packages"""
-    options = options or []
-    cmd = ['apt-get', '-y']
-    cmd.extend(options)
-    cmd.append('install')
-    if isinstance(packages, basestring):
-        cmd.append(packages)
-    else:
-        cmd.extend(packages)
-    log("Installing {} with options: {}".format(packages,
-                                                options))
-    if fatal:
-        subprocess.check_call(cmd)
-    else:
-        subprocess.call(cmd)
-
-
-def apt_update(fatal=False):
-    """Update local apt cache"""
-    cmd = ['apt-get', 'update']
-    if fatal:
-        subprocess.check_call(cmd)
-    else:
-        subprocess.call(cmd)
-
-
 def mount(device, mountpoint, options=None, persist=False):
     '''Mount a filesystem'''
     cmd_args = ['mount']

=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2013-07-23 07:58:41 +0000
+++ charmhelpers/fetch/__init__.py	2013-08-07 23:16:26 +0000
@@ -1,9 +1,6 @@
 import importlib
 from yaml import safe_load
 from charmhelpers.core.host import (
-    apt_install,
-    apt_update,
-    filter_installed_packages,
     lsb_release
 )
 from urlparse import (
@@ -15,6 +12,7 @@
     config,
     log,
 )
+import apt_pkg
 
 CLOUD_ARCHIVE = """# Ubuntu Cloud Archive
 deb http://ubuntu-cloud.archive.canonical.com/ubuntu {} main
@@ -24,6 +22,49 @@
 """
 
 
+def filter_installed_packages(packages):
+    """Returns a list of packages that require installation"""
+    apt_pkg.init()
+    cache = apt_pkg.Cache()
+    _pkgs = []
+    for package in packages:
+        try:
+            p = cache[package]
+            p.current_ver or _pkgs.append(package)
+        except KeyError:
+            log('Package {} has no installation candidate.'.format(package),
+                level='WARNING')
+            _pkgs.append(package)
+    return _pkgs
+
+
+def apt_install(packages, options=None, fatal=False):
+    """Install one or more packages"""
+    options = options or []
+    cmd = ['apt-get', '-y']
+    cmd.extend(options)
+    cmd.append('install')
+    if isinstance(packages, basestring):
+        cmd.append(packages)
+    else:
+        cmd.extend(packages)
+    log("Installing {} with options: {}".format(packages,
+                                                options))
+    if fatal:
+        subprocess.check_call(cmd)
+    else:
+        subprocess.call(cmd)
+
+
+def apt_update(fatal=False):
+    """Update local apt cache"""
+    cmd = ['apt-get', 'update']
+    if fatal:
+        subprocess.check_call(cmd)
+    else:
+        subprocess.call(cmd)
+
+
 def add_source(source, key=None):
     if ((source.startswith('ppa:') or
          source.startswith('http:'))):

=== modified file 'tests/contrib/ansible/test_ansible.py'
--- tests/contrib/ansible/test_ansible.py	2013-07-25 06:22:19 +0000
+++ tests/contrib/ansible/test_ansible.py	2013-08-07 23:16:26 +0000
@@ -26,7 +26,6 @@
         self.mock_core = patcher.start()
         self.addCleanup(patcher.stop)
 
-
         hosts_file = tempfile.NamedTemporaryFile()
         self.ansible_hosts_path = hosts_file.name
         self.addCleanup(hosts_file.close)
@@ -41,8 +40,8 @@
 
         self.mock_fetch.add_source.assert_called_once_with(
             'ppa:rquillo/ansible')
-        self.mock_core.host.apt_update.assert_called_once_with(fatal=True)
-        self.mock_core.host.apt_install.assert_called_once_with(
+        self.mock_fetch.apt_update.assert_called_once_with(fatal=True)
+        self.mock_fetch.apt_install.assert_called_once_with(
             'ansible')
 
     def test_no_ppa(self):
@@ -50,7 +49,7 @@
             from_ppa=False)
 
         self.assertEqual(self.mock_fetch.add_source.call_count, 0)
-        self.mock_core.host.apt_install.assert_called_once_with(
+        self.mock_fetch.apt_install.assert_called_once_with(
             'ansible')
 
     def test_writes_ansible_hosts(self):
@@ -100,7 +99,6 @@
         patcher.start()
         self.addCleanup(patcher.stop)
 
-
     def test_calls_ansible_playbook(self):
         charmhelpers.contrib.ansible.apply_playbook(
             'playbooks/dependencies.yaml')

=== modified file 'tests/contrib/saltstack/test_saltstates.py'
--- tests/contrib/saltstack/test_saltstates.py	2013-07-25 06:22:19 +0000
+++ tests/contrib/saltstack/test_saltstates.py	2013-08-07 23:16:26 +0000
@@ -21,8 +21,8 @@
         self.mock_subprocess = patcher.start()
         self.addCleanup(patcher.stop)
 
-        patcher = mock.patch('charmhelpers.core')
-        self.mock_charmhelpers_core = patcher.start()
+        patcher = mock.patch('charmhelpers.fetch')
+        self.mock_charmhelpers_fetch = patcher.start()
         self.addCleanup(patcher.stop)
 
     def test_adds_ppa_by_default(self):
@@ -30,16 +30,16 @@
 
         self.assertEqual(self.mock_subprocess.check_call.call_count, 2)
         self.assertEqual([(([
-                '/usr/bin/add-apt-repository',
-                '--yes',
-                'ppa:saltstack/salt',
-            ],), {}),
+            '/usr/bin/add-apt-repository',
+            '--yes',
+            'ppa:saltstack/salt',
+        ],), {}),
             (([
                 '/usr/bin/apt-get',
                 'update',
             ],), {})
         ], self.mock_subprocess.check_call.call_args_list)
-        self.mock_charmhelpers_core.host.apt_install.assert_called_once_with(
+        self.mock_charmhelpers_fetch.apt_install.assert_called_once_with(
             'salt-common')
 
     def test_no_ppa(self):
@@ -47,7 +47,7 @@
             from_ppa=False)
 
         self.assertEqual(self.mock_subprocess.check_call.call_count, 0)
-        self.mock_charmhelpers_core.host.apt_install.assert_called_once_with(
+        self.mock_charmhelpers_fetch.apt_install.assert_called_once_with(
             'salt-common')
 
 
@@ -150,7 +150,6 @@
                 "local_unit": "click-index/3",
             }, result)
 
-
     def test_output_with_relation(self):
         self.mock_config.return_value = {
             'group_code_owner': 'webops_deploy',

=== modified file 'tests/core/test_host.py'
--- tests/core/test_host.py	2013-07-30 12:24:47 +0000
+++ tests/core/test_host.py	2013-08-07 23:16:26 +0000
@@ -7,6 +7,7 @@
 from testtools import TestCase
 
 from charmhelpers.core import host
+from charmhelpers import fetch
 
 
 MOUNT_LINES = ("""
@@ -18,16 +19,6 @@
                """rw,nosuid,noexec,relatime,gid=5,mode=620,ptmxmode=000 0 0
 """).strip().split('\n')
 
-FAKE_APT_CACHE = {
-    # an installed package
-    'vim': {
-        'current_ver': '2:7.3.547-6ubuntu5'
-    },
-    # a uninstalled installation candidate
-    'emacs': {
-    }
-}
-
 LSB_RELEASE = u'''DISTRIB_ID=Ubuntu
 DISTRIB_RELEASE=13.10
 DISTRIB_CODENAME=saucy
@@ -35,22 +26,6 @@
 '''
 
 
-def fake_apt_cache():
-    def _get(package):
-        pkg = MagicMock()
-        if package not in FAKE_APT_CACHE:
-            raise KeyError
-        pkg.name = package
-        if 'current_ver' in FAKE_APT_CACHE[package]:
-            pkg.current_ver = FAKE_APT_CACHE[package]['current_ver']
-        else:
-            pkg.current_ver = None
-        return pkg
-    cache = MagicMock()
-    cache.__getitem__.side_effect = _get
-    return cache
-
-
 @contextmanager
 def patch_open():
     '''Patch open() to allow mocking both open() itself and the file that is
@@ -435,7 +410,7 @@
         packages = ['foo', 'bar']
         options = ['--foo', '--bar']
 
-        host.apt_install(packages, options)
+        fetch.apt_install(packages, options)
 
         mock_call.assert_called_with(['apt-get', '-y', '--foo', '--bar',
                                       'install', 'foo', 'bar'])
@@ -445,7 +420,7 @@
     def test_installs_apt_packages_without_options(self, log, mock_call):
         packages = ['foo', 'bar']
 
-        host.apt_install(packages)
+        fetch.apt_install(packages)
 
         mock_call.assert_called_with(['apt-get', '-y', 'install', 'foo',
                                       'bar'])
@@ -456,7 +431,7 @@
         packages = 'foo bar'
         options = ['--foo', '--bar']
 
-        host.apt_install(packages, options)
+        fetch.apt_install(packages, options)
 
         mock_call.assert_called_with(['apt-get', '-y', '--foo', '--bar',
                                       'install', 'foo bar'])
@@ -467,42 +442,21 @@
         packages = ['foo', 'bar']
         options = ['--foo', '--bar']
 
-        host.apt_install(packages, options, fatal=True)
+        fetch.apt_install(packages, options, fatal=True)
 
         check_call.assert_called_with(['apt-get', '-y', '--foo', '--bar',
                                        'install', 'foo', 'bar'])
 
     @patch('subprocess.check_call')
     def test_apt_update_fatal(self, check_call):
-        host.apt_update(fatal=True)
+        fetch.apt_update(fatal=True)
         check_call.assert_called_with(['apt-get', 'update'])
 
     @patch('subprocess.call')
     def test_apt_update_nonfatal(self, call):
-        host.apt_update()
+        fetch.apt_update()
         call.assert_called_with(['apt-get', 'update'])
 
-    @patch('apt_pkg.Cache')
-    def test_filter_packages_missing(self, cache):
-        cache.side_effect = fake_apt_cache
-        result = host.filter_installed_packages(['vim', 'emacs'])
-        self.assertEquals(result, ['emacs'])
-
-    @patch('apt_pkg.Cache')
-    def test_filter_packages_none_missing(self, cache):
-        cache.side_effect = fake_apt_cache
-        result = host.filter_installed_packages(['vim'])
-        self.assertEquals(result, [])
-
-    @patch.object(host, 'log')
-    @patch('apt_pkg.Cache')
-    def test_filter_packages_not_available(self, cache, log):
-        cache.side_effect = fake_apt_cache
-        result = host.filter_installed_packages(['vim', 'joe'])
-        self.assertEquals(result, ['joe'])
-        log.assert_called_with('Package joe has no installation candidate.',
-                               level='WARNING')
-
     @patch('subprocess.check_output')
     @patch.object(host, 'log')
     def test_mounts_a_device(self, log, check_output):
@@ -612,7 +566,7 @@
         file_name = '/etc/missing.conf'
         restart_map = {
             file_name: ['test-service']
-            }
+        }
         exists.side_effect = [False, False]
 
         @host.restart_on_change(restart_map)
@@ -633,7 +587,7 @@
         file_name = '/etc/missing.conf'
         restart_map = {
             file_name: ['test-service']
-            }
+        }
         exists.side_effect = [False, True]
 
         @host.restart_on_change(restart_map)
@@ -658,7 +612,7 @@
         restart_map = {
             file_name_one: ['test-service'],
             file_name_two: ['test-service', 'test-service2']
-            }
+        }
         exists.side_effect = [False, True, True, True]
 
         @host.restart_on_change(restart_map)

=== modified file 'tests/fetch/test_fetch.py'
--- tests/fetch/test_fetch.py	2013-07-23 07:58:41 +0000
+++ tests/fetch/test_fetch.py	2013-08-07 23:16:26 +0000
@@ -9,6 +9,32 @@
 from charmhelpers import fetch
 import yaml
 
+FAKE_APT_CACHE = {
+    # an installed package
+    'vim': {
+        'current_ver': '2:7.3.547-6ubuntu5'
+    },
+    # a uninstalled installation candidate
+    'emacs': {
+    }
+}
+
+
+def fake_apt_cache():
+    def _get(package):
+        pkg = MagicMock()
+        if package not in FAKE_APT_CACHE:
+            raise KeyError
+        pkg.name = package
+        if 'current_ver' in FAKE_APT_CACHE[package]:
+            pkg.current_ver = FAKE_APT_CACHE[package]['current_ver']
+        else:
+            pkg.current_ver = None
+        return pkg
+    cache = MagicMock()
+    cache.__getitem__.side_effect = _get
+    return cache
+
 
 @contextmanager
 def patch_open():
@@ -29,6 +55,27 @@
 
 
 class FetchTest(TestCase):
+    @patch('apt_pkg.Cache')
+    def test_filter_packages_missing(self, cache):
+        cache.side_effect = fake_apt_cache
+        result = fetch.filter_installed_packages(['vim', 'emacs'])
+        self.assertEquals(result, ['emacs'])
+
+    @patch('apt_pkg.Cache')
+    def test_filter_packages_none_missing(self, cache):
+        cache.side_effect = fake_apt_cache
+        result = fetch.filter_installed_packages(['vim'])
+        self.assertEquals(result, [])
+
+    @patch.object(fetch, 'log')
+    @patch('apt_pkg.Cache')
+    def test_filter_packages_not_available(self, cache, log):
+        cache.side_effect = fake_apt_cache
+        result = fetch.filter_installed_packages(['vim', 'joe'])
+        self.assertEquals(result, ['joe'])
+        log.assert_called_with('Package joe has no installation candidate.',
+                               level='WARNING')
+
     @patch('subprocess.check_call')
     def test_add_source_ppa(self, check_call):
         source = "ppa:test-ppa"
@@ -136,7 +183,7 @@
             "ftp://example.com/foo.tar.gz",
             "https://example.com/foo.tgz",
             "file://example.com/foo.tar.bz2",
-            )
+        )
         self.invalid_urls = (
             "git://example.com/foo.tar.gz",
             "http://example.com/foo",
@@ -146,7 +193,7 @@
             "lp:example/foo.tgz",
             "file//example.com/foo.tar.bz2",
             "garbage",
-            )
+        )
 
     @patch('charmhelpers.fetch.plugins')
     def test_installs_remote(self, _plugins):
@@ -235,7 +282,7 @@
             "bzr+ssh://bazaar.launchpad.net/foo/bar",
             "bzr+http://bazaar.launchpad.net/foo/bar",
             "garbage",
-            )
+        )
         self.fh = fetch.BaseFetchHandler()
 
     def test_handles_nothing(self):