Implemented changes. Addressed review comments.

charmhelpers/core/host.py

@@ -30,34 +30,29 @@
 import hashlib
 import functools
 import itertools
-from contextlib import contextmanager
-from collections import OrderedDict
-
 import six
 
+from contextlib import contextmanager
+from collections import OrderedDict
 from .hookenv import log
 from .fstab import Fstab
 from charmhelpers.osplatform import get_platform
 
 __platform__ = get_platform()
 if __platform__ == "ubuntu":
     from charmhelpers.core.host_factory.ubuntu import (
-        service_available_host,
-        add_group_host,
-        lsb_release_host,
-        cmp_pkgrevno_host,
-    )
+        service_available,
+        add_new_group,
+        lsb_release,
+        cmp_pkgrevno,
+    )  # flake8: noqa -- ignore F401 for this import
 elif __platform__ == "centos":
     from charmhelpers.core.host_factory.centos import (
-        service_available_host,
-        add_group_host,
-        lsb_release_host,
-        cmp_pkgrevno_host,
-    )
-
-service_available = service_available_host
-lsb_release = lsb_release_host
-cmp_pkgrevno = cmp_pkgrevno_host
+        service_available,
+        add_new_group,
+        lsb_release,
+        cmp_pkgrevno,
+    )  # flake8: noqa -- ignore F401 for this import
 
 
 def service_start(service_name):
@@ -299,7 +294,7 @@ def add_group(group_name, system_group=False, gid=None):
             log('group with gid {0} already exists!'.format(gid))
     except KeyError:
         log('creating group {0}'.format(group_name))
-        add_group_host(group_name, system_group, gid)
+        add_new_group(group_name, system_group, gid)
         group_info = grp.getgrnam(group_name)
     return group_info
 

charmhelpers/core/host_factory/centos.py

@@ -1,15 +1,18 @@
 import subprocess
 import yum
+import os
 
-from charmhelpers.core.host import service
 
+def service_available(service_name):
+    # """Determine whether a system service is available."""
+    if os.path.isdir('/run/systemd/system'):
+        cmd = ['systemctl', 'is-enabled', service_name]
+    else:
+        cmd = ['service', service_name, 'is-enabled']
+    return subprocess.call(cmd) == 0
 
-def service_available_host(service_name):
-    """Determine whether a system service is available."""
-    return service('is-enabled', service_name)
 
-
-def add_group_host(group_name, system_group=False, gid=None):
+def add_new_group(group_name, system_group=False, gid=None):
     cmd = ['groupadd']
     if gid:
         cmd.extend(['--gid', str(gid)])
@@ -19,19 +22,19 @@ def add_group_host(group_name, system_group=False, gid=None):
     subprocess.check_call(cmd)
 
 
-def lsb_release_host():
+def lsb_release():
     """Return /etc/os-release in a dict."""
     d = {}
     with open('/etc/os-release', 'r') as lsb:
         for l in lsb:
-            if len(l.split('=')) != 2:
+            s = l.split('=')
+            if len(s) != 2:
                 continue
-            k, v = l.split('=')
-            d[k.strip()] = v.strip()
+            d[s[0].strip()] = s[1].strip()
     return d
 
 
-def cmp_pkgrevno_host(package, revno, pkgcache=None):
+def cmp_pkgrevno(package, revno, pkgcache=None):
     """Compare supplied revno with the revno of the installed package.
 
     *  1 => Installed revno is greater than supplied arg
@@ -44,10 +47,7 @@ def cmp_pkgrevno_host(package, revno, pkgcache=None):
     if not pkgcache:
         y = yum.YumBase()
         packages = y.doPackageLists()
-        pck = {}
-        for i in packages["installed"]:
-            pck[i.Name] = i.version
-            pkgcache = pck
+        pkgcache = {i.Name: i.version for i in packages['installed']}
     pkg = pkgcache[package]
     if pkg > revno:
         return 1

charmhelpers/core/host_factory/ubuntu.py

@@ -1,7 +1,7 @@
 import subprocess
 
 
-def service_available_host(service_name):
+def service_available(service_name):
     """Determine whether a system service is available"""
     try:
         subprocess.check_output(
@@ -13,7 +13,7 @@ def service_available_host(service_name):
         return True
 
 
-def add_group_host(group_name, system_group=False, gid=None):
+def add_new_group(group_name, system_group=False, gid=None):
     cmd = ['addgroup']
     if gid:
         cmd.extend(['--gid', str(gid)])
@@ -27,7 +27,7 @@ def add_group_host(group_name, system_group=False, gid=None):
     subprocess.check_call(cmd)
 
 
-def lsb_release_host():
+def lsb_release():
     """Return /etc/lsb-release in a dict"""
     d = {}
     with open('/etc/lsb-release', 'r') as lsb:
@@ -37,7 +37,7 @@ def lsb_release_host():
     return d
 
 
-def cmp_pkgrevno_host(package, revno, pkgcache=None):
+def cmp_pkgrevno(package, revno, pkgcache=None):
     """Compare supplied revno with the revno of the installed package.
 
     *  1 => Installed revno is greater than supplied arg

charmhelpers/core/kernel.py

@@ -15,8 +15,6 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-# __author__ = "Jorge Niedbalski <jorge.niedbalski@canonical.com>"
-
 import re
 import subprocess
 
@@ -29,14 +27,16 @@
 __platform__ = get_platform()
 if __platform__ == "ubuntu":
     from charmhelpers.core.kernel_factory.ubuntu import (
-        modprobe_kernel,
-        update_initramfs_kernel,
-    )
+        persistent_modprobe,
+        update_initramfs,
+    )  # flake8: noqa -- ignore F401 for this import
 elif __platform__ == "centos":
     from charmhelpers.core.kernel_factory.centos import (
-        modprobe_kernel,
-        update_initramfs_kernel,
-    )
+        persistent_modprobe,
+        update_initramfs,
+    )  # flake8: noqa -- ignore F401 for this import
+
+__author__ = "Jorge Niedbalski <jorge.niedbalski@canonical.com>"
 
 
 def modprobe(module, persist=True):
@@ -46,7 +46,8 @@ def modprobe(module, persist=True):
     log('Loading kernel module %s' % module, level=INFO)
 
     subprocess.check_call(cmd)
-    modprobe_kernel(module, persist)
+    if persist:
+        persistent_modprobe(module)
 
 
 def rmmod(module, force=False):
@@ -69,6 +70,3 @@ def is_module_loaded(module):
     """Checks if a kernel module is already loaded"""
     matches = re.findall('^%s[ ]+' % module, lsmod(), re.M)
     return len(matches) > 0
-
-
-update_initramfs = update_initramfs_kernel

charmhelpers/core/kernel_factory/centos.py

@@ -2,17 +2,16 @@
 import os
 
 
-def modprobe_kernel(module, persist=True):
+def persistent_modprobe(module):
     """Load a kernel module and configure for auto-load on reboot."""
-    if persist:
-        if not os.path.exists('/etc/rc.modules'):
-            open('/etc/rc.modules', 'a')
-            os.chmod('/etc/rc.modules', 111)
-        with open('/etc/rc.modules', 'r+') as modules:
-            if module not in modules.read():
-                modules.write('modprobe %s\n' % module)
+    if not os.path.exists('/etc/rc.modules'):
+        open('/etc/rc.modules', 'a')
+        os.chmod('/etc/rc.modules', 111)
+    with open('/etc/rc.modules', 'r+') as modules:
+        if module not in modules.read():
+            modules.write('modprobe %s\n' % module)
 
 
-def update_initramfs_kernel(version='all'):
+def update_initramfs(version='all'):
     """Updates an initramfs image."""
     return subprocess.check_call(["dracut", "-f", version])

charmhelpers/core/kernel_factory/ubuntu.py

@@ -1,14 +1,13 @@
 import subprocess
 
 
-def modprobe_kernel(module, persist=True):
+def persistent_modprobe(module):
     """Load a kernel module and configure for auto-load on reboot."""
-    if persist:
-        with open('/etc/modules', 'r+') as modules:
-            if module not in modules.read():
-                modules.write(module)
+    with open('/etc/modules', 'r+') as modules:
+        if module not in modules.read():
+            modules.write(module)
 
 
-def update_initramfs_kernel(version='all'):
+def update_initramfs(version='all'):
     """Updates an initramfs image."""
     return subprocess.check_call(["update-initramfs", "-k", version, "-u"])

charmhelpers/fetch/__init__.py

@@ -102,7 +102,7 @@ def configure_sources(update=False,
     """Configure multiple sources from charm configuration.
 
     The lists are encoded as yaml fragments in the configuration.
-    The frament needs to be included as a string. Sources and their
+    The fragment needs to be included as a string. Sources and their
     corresponding keys are of the types supported by add_source().
 
     Example config:
@@ -134,10 +134,7 @@ def configure_sources(update=False,
         for source, key in zip(sources, keys):
             add_source(source, key)
     if update:
-        def call_update():
-            global update
-            update(fatal=True)
-        call_update()
+        fetch.update(fatal=True)
 
 
 def install_remote(source, *args, **kwargs):

charmhelpers/fetch/centos.py

@@ -29,24 +29,10 @@
 def filter_installed_packages(packages):
     """Return a list of packages that require installation."""
     yb = yum.YumBase()
-    temp_cache = []
     package_list = yb.doPackageLists()
-    for package in package_list['installed']:
-        temp_cache.append(package.base_package_name)
-
-    # If each package has a candidate then return the list of packages
-    # that require installation
-    for package in packages:
-        for each in package_list['available']:
-            if package not in each.base_package_name:
-                log('Package {} has no installation '
-                    'candidate.'.format(package), level='WARNING')
-                break
-
-    _pkgs = []
-    for package in packages:
-        if package not in temp_cache:
-            _pkgs.append(package)
+    temp_cache = {p.base_package_name: 1 for p in package_list['installed']}
+
+    _pkgs = [p for p in packages if not temp_cache.get(p, False)]
     return _pkgs
 
 
@@ -104,10 +90,7 @@ def yum_search(packages):
     log("Searching for {}".format(packages))
     result = subprocess.check_output(cmd)
     for package in list(packages):
-        if package not in result:
-            output[package] = False
-        else:
-            output[package] = True
+        output[package] = package in result
     return output
 
 
@@ -128,17 +111,13 @@ def add_source(source, key=None):
         return
 
     if source.startswith('http'):
-        log("Add source: {!r}".format(source))
-
-        found = False
-        # search if already exists
         directory = '/etc/yum.repos.d/'
         for filename in os.listdir(directory):
             with open(directory + filename, 'r') as rpm_file:
-                if source in rpm_file:
-                    found = True
-
-        if not found:
+                if source in rpm_file.read():
+                    break
+        else:
+            log("Add source: {!r}".format(source))
             # write in the charms.repo
             with open(directory + 'Charms.repo', 'a') as rpm_file:
                 rpm_file.write('[%s]\n' % source[7:].replace('/', '_'))

tarmac_tests.sh

@@ -3,7 +3,7 @@
 
 set -e
 
-pkgs='python-flake8 python-shelltoolbox python-tempita python-nose python-mock python-testtools python-jinja2 python-coverage python-git python-netifaces python-netaddr python-pip zip yum'
+pkgs='python-flake8 python-shelltoolbox python-tempita python-nose python-mock python-testtools python-jinja2 python-coverage python-git python-netifaces python-netaddr python-pip zip'
 if ! dpkg -s $pkgs 2>/dev/null >/dev/null ; then
     echo "Required packages are missing. Please ensure that the missing packages are installed."
     echo "Run: sudo apt-get install $pkgs"

tests/core/__init__.py

@@ -0,0 +1,4 @@
+import sys
+import mock
+
+sys.modules['yum'] = mock.MagicMock()

tests/core/test_host.py

@@ -1649,8 +1649,7 @@ def __init__(self, current_ver):
         self.assertEqual(host.cmp_pkgrevno('python', '2.5'), -1)
 
     @patch.object(osplatform, 'get_platform')
-    @patch('yum.YumBase.doPackageLists')
-    def test_cmp_pkgrevno_revnos_centos(self, yumBase, platform):
+    def test_cmp_pkgrevno_revnos_centos(self, platform):
         platform.return_value = 'centos'
         imp.reload(host)
 
@@ -1664,7 +1663,10 @@ def __init__(self, name, version):
                 MockPackage('python', '2.4')
             }
         }
-        yumBase.return_value = yum_dict
+
+        import yum
+        yum.YumBase.return_value.doPackageLists.return_value = (
+            yum_dict)
 
         self.assertEqual(host.cmp_pkgrevno('python', '2.3'), 1)
         self.assertEqual(host.cmp_pkgrevno('python', '2.4'), 0)

tests/fetch/__init__.py

@@ -0,0 +1,4 @@
+import sys
+import mock
+
+sys.modules['yum'] = mock.MagicMock()