Merge lp:~ajkavanagh/charm-helpers/ppc64le_proposed_fix_plus_refactor into lp:charm-helpers

[Alex Kavanagh]

  Refactor configure_installation_source() and add_source() + fix bug 1611134

  Bug#1611134: fetch helper assumes amd64 arch (cannot use with ports such as ppc64el)

  The refactor is because the two functions do almost exactly the same thing in two
  different places, and the core function does OpenStack sources, which ought to be in
  the contrib/openstack module set.

  After the refactor, the bug fix was implemented on the 'proposed' source to pick the
  correct ports specification for ppc64le:

  deb http://ports.ubuntu.com/ubuntu-ports {}-proposed main universe multiverse restricted

  where {} is the ubuntu release.

[lp:~ajkavanagh/charm-helpers/ppc64le_proposed_fix_plus_refactor]

622. By Alex Kavanagh on 2016-12-15

Merge in r619 in prep for r620 which is very different

623. By Alex Kavanagh on 2016-12-20

Reworking of the ports_fetch code and the dbuliga CentOS support

The CentOS support changed the same code as the fetch code for
ports (i.e. other architectures that x86_64). This merge changes
the way the testing works for the CentOS/Ubuntu dbuliga work by
moving the specific tests for each distro into their own files
and keeping a common file. This has simplified the testing, and
also fixed a few tests.

Also, this merge tidies up quite a few PEP8 issues with the library.

624. By Alex Kavanagh on 2016-12-21

Merge main branch to 626

625. By Alex Kavanagh on 2016-12-21

Merge/resolve conflicts with main 627

626. By Alex Kavanagh on 2016-12-21

Add back in DEBUG to c/o/utils.py

627. By Alex Kavanagh on 2016-12-21

Merge/resolve conflicts to main branch 628

628. By Alex Kavanagh on 2016-12-21

Merge to main branch r630

629. By Alex Kavanagh on 2016-12-21

Merge/resolve conflicts with main branch r631

630. By Alex Kavanagh on 2016-12-21

Merge to main branch r647

631. By Alex Kavanagh on 2016-12-21

Merge/resolve conflicts with main branch r648

632. By Alex Kavanagh on 2016-12-21

Merge up to main branch r657

633. By Alex Kavanagh on 2016-12-21

Merge/resolve conflicts for main branch r658

The issue is that the different architecture ports has changed where the
cloud pockets are represented in the code, and simplified them slightly.
This merge resolves the conflicts with r658 and also fixes a bug with
'zesty' in the list of OpenStack releases (which confuses them with
Ubuntu release names).

634. By Alex Kavanagh on 2016-12-21

Merge to main branch r660

635. By Alex Kavanagh on 2016-12-21

Merge r661 from main branch + resolve conflicts

636. By Alex Kavanagh on 2016-12-21

Merge & resolve conflicts up to main branch r663

This is because the ports refactor has removed one set of (obsolete)
cloud pockets.

637. By Alex Kavanagh on 2016-12-21

Merge up to r671 from main devel branch

[Ryan Beisner (1chb1n)]

As you might expect, there are a number of (what seem to be small) merge conflicts with Trunk by now. But I merged this into a local copy of Trunk, took a stab at resolving those conflicts, and found the following issues:

1. Some of the added tests appear to assume that the tests will be executed from a Xenial machine, which will not always be the case. It might be best to base the tests on Xenial (LTS) regardless of the release of the developer's machine which is executing the unit tests.

2. There are a number of "local variable 'env' referenced before assignment" errors in the tests which cover _run_apt_command. This may have been missed with the rebase against that big centos change. This is causing unit and lint check failures.
| charmhelpers/fetch/ubuntu.py:642:57: F821 undefined name 'env'

3. There has been a recent change in the apt retry logic which needs to be reconciled on a rebase. I didn't fully (or carefully) resolve those conflicts in my WIP as I needed to plow on for a separate test case.

http://bazaar.launchpad.net/~charm-helpers/charm-helpers/devel/revision/705

This is causing:
| charmhelpers/fetch/ubuntu.py:645:34: F821 undefined name 'APT_NO_LOCK_RETRY_COUNT'
| charmhelpers/fetch/ubuntu.py:649:29: F821 undefined name 'APT_NO_LOCK_RETRY_DELAY'
| charmhelpers/fetch/ubuntu.py:650:28: F821 undefined name 'APT_NO_LOCK_RETRY_DELAY'

See nosetests results:
 - http://paste.ubuntu.com/24278293/

My temporary trunk merge branch is here (NOT intended to land or to be proposed as my merge conflict resolution approach was quite fast and furious):
 - https://code.launchpad.net/~1chb1n/charm-helpers/lp1611134

I synced it into the nova-compute charm as a WIP TEST here (also NOT for reals):
 - https://github.com/ryan-beisner/charm-nova-compute-wip

And that is what I'm using on arm64 at the moment (!), because I need to consume arm64 ports. I'll chime back in here with how that goes. :-)

review: Needs Fixing

[Ryan Beisner (1chb1n)]

A test deployment on arm64 xenial-mitaka, with openstack-origin set to "distro-proposed" failed:

2017-03-30 03:05:04 DEBUG juju-log No hardening applied to 'install'
2017-03-30 03:05:05 INFO juju-log DEPRECATION WARNING: Function configure_installation_source is being removed on/around 2017-07 : use charmhelpers.fetch.add_source() instead.
2017-03-30 03:05:05 ERROR juju-log FATAL ERROR: Arch aarch64 not supported for (distro-)proposed
2017-03-30 03:05:05 ERROR juju.worker.uniter.operation runhook.go:107 hook "install" failed: exit status 1

review: Needs Fixing

[Ryan Beisner (1chb1n)]

FYI, this (in my WIP temp branch) resolved the failure in my previous comment:
 - http://bazaar.launchpad.net/~1chb1n/charm-helpers/lp1611134/revision/729

[Alex Kavanagh (ajkavanagh)]

Hi Ryan

Thanks for taking a look at it. I'm going to swing back to this shortly and have another go at bring it up to date. I'll also test it against the glance charm to check that it does indeed work.

Thanks
Alex.

[lp:~ajkavanagh/charm-helpers/ppc64le_proposed_fix_plus_refactor]

638. By Alex Kavanagh on 2017-04-07

Remove redundant plugins() funct from c.f.ubuntu.py

639. By Alex Kavanagh on 2017-04-07

Remove duplicated code in fetch module(s)

640. By Alex Kavanagh on 2017-04-07

bring upto date to r704

641. By Alex Kavanagh on 2017-04-10

Merge in r705 (retry-add-apt-repository)

This needed a little work as the fetch code in this branch is very different
and reorganised from the dev branch.

642. By Alex Kavanagh on 2017-04-10

Merge up to r731

643. By Alex Kavanagh on 2017-04-11

Add ARM 64 arch to proposed pockets for OpenStack

Unmerged revisions

=== modified file 'Makefile'
--- Makefile	2017-01-25 17:11:37 +0000
+++ Makefile	2017-04-10 18:12:16 +0000
@@ -27,6 +27,7 @@
 	-python setup.py clean
 	rm -rf build/ MANIFEST
 	find . -name '*.pyc' -delete
+	find . -name '__pycache__' -delete
 	rm -rf dist/*
 	rm -rf .venv
 	rm -rf .venv3

=== modified file 'charmhelpers/__init__.py'
--- charmhelpers/__init__.py	2016-07-06 14:41:05 +0000
+++ charmhelpers/__init__.py	2017-04-10 18:12:16 +0000
@@ -14,6 +14,11 @@
 
 # Bootstrap charm-helpers, installing its dependencies if necessary using
 # only standard libraries.
+from __future__ import print_function
+from __future__ import absolute_import
+
+import functools
+import inspect
 import subprocess
 import sys
 
@@ -34,3 +39,59 @@
     else:
         subprocess.check_call(['apt-get', 'install', '-y', 'python3-yaml'])
     import yaml  # flake8: noqa
+
+
+# Holds a list of mapping of mangled function names that have been deprecated
+# using the @deprecate decorator below.  This is so that the warning is only
+# printed once for each usage of the function.
+__deprecated_functions = {}
+
+
+def deprecate(warning, date=None, log=None):
+    """Add a deprecation warning the first time the function is used.
+    The date, which is a string in semi-ISO8660 format indicate the year-month
+    that the function is officially going to be removed.
+
+    usage:
+
+    @deprecate('use core/fetch/add_source() instead', '2017-04')
+    def contributed_add_source_thing(...):
+        ...
+
+    And it then prints to the log ONCE that the function is deprecated.
+    The reason for passing the logging function (log) is so that hookenv.log
+    can be used for a charm if needed.
+
+    :param warning:  String to indicat where it has moved ot.
+    :param date: optional sting, in YYYY-MM format to indicate when the
+                 function will definitely (probably) be removed.
+    :param log: The log function to call to log.  If not, logs to stdout
+    """
+    def wrap(f):
+
+        @functools.wraps(f)
+        def wrapped_f(*args, **kwargs):
+            try:
+                module = inspect.getmodule(f)
+                file = inspect.getsourcefile(f)
+                lines = inspect.getsourcelines(f)
+                f_name = "{}-{}-{}..{}-{}".format(
+                    module.__name__, file, lines[0], lines[-1], f.__name__)
+            except (IOError, TypeError):
+                # assume it was local, so just use the name of the function
+                f_name = f.__name__
+            if f_name not in __deprecated_functions:
+                __deprecated_functions[f_name] = True
+                s = "DEPRECATION WARNING: Function {} is being removed".format(
+                    f.__name__)
+                if date:
+                    s = "{} on/around {}".format(s, date)
+                if warning:
+                    s = "{} : {}".format(s, warning)
+                if log:
+                    log(s)
+                else:
+                    print(s)
+            return f(*args, **kwargs)
+        return wrapped_f
+    return wrap

=== modified file 'charmhelpers/contrib/openstack/utils.py'
--- charmhelpers/contrib/openstack/utils.py	2017-03-24 16:50:31 +0000
+++ charmhelpers/contrib/openstack/utils.py	2017-04-10 18:12:16 +0000
@@ -26,11 +26,12 @@
 import shutil
 
 import six
-import tempfile
 import traceback
 import uuid
 import yaml
 
+from charmhelpers import deprecate
+
 from charmhelpers.contrib.network import ip
 
 from charmhelpers.core import unitdata
@@ -41,7 +42,6 @@
     config,
     log as juju_log,
     charm_dir,
-    DEBUG,
     INFO,
     ERROR,
     related_units,
@@ -82,9 +82,12 @@
     restart_on_change_helper,
 )
 from charmhelpers.fetch import (
-    apt_install,
     apt_cache,
     install_remote,
+    import_key as fetch_import_key,
+    add_source as fetch_add_source,
+    SourceConfigError,
+    GPGKeyError,
     get_upstream_version
 )
 from charmhelpers.contrib.storage.linux.utils import is_block_device, zap_disk
@@ -436,13 +439,14 @@
     # error_out(e)
 
 
-os_rel = None
+# Module local cache variable for the os_release.
+_os_rel = None
 
 
 def reset_os_release():
     '''Unset the cached os_release version'''
-    global os_rel
-    os_rel = None
+    global _os_rel
+    _os_rel = None
 
 
 def os_release(package, base='essex', reset_cache=False):
@@ -456,144 +460,77 @@
     the installation source, the earliest release supported by the charm should
     be returned.
     '''
-    global os_rel
+    global _os_rel
     if reset_cache:
         reset_os_release()
-    if os_rel:
-        return os_rel
-    os_rel = (git_os_codename_install_source(config('openstack-origin-git')) or
-              get_os_codename_package(package, fatal=False) or
-              get_os_codename_install_source(config('openstack-origin')) or
-              base)
-    return os_rel
-
-
+    if _os_rel:
+        return _os_rel
+    _os_rel = (
+        git_os_codename_install_source(config('openstack-origin-git')) or
+        get_os_codename_package(package, fatal=False) or
+        get_os_codename_install_source(config('openstack-origin')) or
+        base)
+    return _os_rel
+
+
+@deprecate("moved to charmhelpers.fetch.import_key()", "2017-07", log=juju_log)
 def import_key(keyid):
-    key = keyid.strip()
-    if (key.startswith('-----BEGIN PGP PUBLIC KEY BLOCK-----') and
-            key.endswith('-----END PGP PUBLIC KEY BLOCK-----')):
-        juju_log("PGP key found (looks like ASCII Armor format)", level=DEBUG)
-        juju_log("Importing ASCII Armor PGP key", level=DEBUG)
-        with tempfile.NamedTemporaryFile() as keyfile:
-            with open(keyfile.name, 'w') as fd:
-                fd.write(key)
-                fd.write("\n")
-
-            cmd = ['apt-key', 'add', keyfile.name]
-            try:
-                subprocess.check_call(cmd)
-            except subprocess.CalledProcessError:
-                error_out("Error importing PGP key '%s'" % key)
-    else:
-        juju_log("PGP key found (looks like Radix64 format)", level=DEBUG)
-        juju_log("Importing PGP key from keyserver", level=DEBUG)
-        cmd = ['apt-key', 'adv', '--keyserver',
-               'hkp://keyserver.ubuntu.com:80', '--recv-keys', key]
-        try:
-            subprocess.check_call(cmd)
-        except subprocess.CalledProcessError:
-            error_out("Error importing PGP key '%s'" % key)
-
-
-def get_source_and_pgp_key(input):
-    """Look for a pgp key ID or ascii-armor key in the given input."""
-    index = input.strip()
-    index = input.rfind('|')
-    if index < 0:
-        return input, None
-
-    key = input[index + 1:].strip('|')
-    source = input[:index]
-    return source, key
-
-
-def configure_installation_source(rel):
-    '''Configure apt installation source.'''
-    if rel == 'distro':
-        return
-    elif rel == 'distro-proposed':
-        ubuntu_rel = lsb_release()['DISTRIB_CODENAME']
-        with open('/etc/apt/sources.list.d/juju_deb.list', 'w') as f:
-            f.write(DISTRO_PROPOSED % ubuntu_rel)
-    elif rel[:4] == "ppa:":
-        src, key = get_source_and_pgp_key(rel)
-        if key:
-            import_key(key)
-
-        subprocess.check_call(["add-apt-repository", "-y", src])
-    elif rel[:3] == "deb":
-        src, key = get_source_and_pgp_key(rel)
-        if key:
-            import_key(key)
-
-        with open('/etc/apt/sources.list.d/juju_deb.list', 'w') as f:
-            f.write(src)
-    elif rel[:6] == 'cloud:':
-        ubuntu_rel = lsb_release()['DISTRIB_CODENAME']
-        rel = rel.split(':')[1]
-        u_rel = rel.split('-')[0]
-        ca_rel = rel.split('-')[1]
-
-        if u_rel != ubuntu_rel:
-            e = 'Cannot install from Cloud Archive pocket %s on this Ubuntu '\
-                'version (%s)' % (ca_rel, ubuntu_rel)
-            error_out(e)
-
-        if 'staging' in ca_rel:
-            # staging is just a regular PPA.
-            os_rel = ca_rel.split('/')[0]
-            ppa = 'ppa:ubuntu-cloud-archive/%s-staging' % os_rel
-            cmd = 'add-apt-repository -y %s' % ppa
-            subprocess.check_call(cmd.split(' '))
-            return
-
-        # map charm config options to actual archive pockets.
-        pockets = {
-            'folsom': 'precise-updates/folsom',
-            'folsom/updates': 'precise-updates/folsom',
-            'folsom/proposed': 'precise-proposed/folsom',
-            'grizzly': 'precise-updates/grizzly',
-            'grizzly/updates': 'precise-updates/grizzly',
-            'grizzly/proposed': 'precise-proposed/grizzly',
-            'havana': 'precise-updates/havana',
-            'havana/updates': 'precise-updates/havana',
-            'havana/proposed': 'precise-proposed/havana',
-            'icehouse': 'precise-updates/icehouse',
-            'icehouse/updates': 'precise-updates/icehouse',
-            'icehouse/proposed': 'precise-proposed/icehouse',
-            'juno': 'trusty-updates/juno',
-            'juno/updates': 'trusty-updates/juno',
-            'juno/proposed': 'trusty-proposed/juno',
-            'kilo': 'trusty-updates/kilo',
-            'kilo/updates': 'trusty-updates/kilo',
-            'kilo/proposed': 'trusty-proposed/kilo',
-            'liberty': 'trusty-updates/liberty',
-            'liberty/updates': 'trusty-updates/liberty',
-            'liberty/proposed': 'trusty-proposed/liberty',
-            'mitaka': 'trusty-updates/mitaka',
-            'mitaka/updates': 'trusty-updates/mitaka',
-            'mitaka/proposed': 'trusty-proposed/mitaka',
-            'newton': 'xenial-updates/newton',
-            'newton/updates': 'xenial-updates/newton',
-            'newton/proposed': 'xenial-proposed/newton',
-            'ocata': 'xenial-updates/ocata',
-            'ocata/updates': 'xenial-updates/ocata',
-            'ocata/proposed': 'xenial-proposed/ocata',
-        }
-
-        try:
-            pocket = pockets[ca_rel]
-        except KeyError:
-            e = 'Invalid Cloud Archive release specified: %s' % rel
-            error_out(e)
-
-        src = "deb %s %s main" % (CLOUD_ARCHIVE_URL, pocket)
-        apt_install('ubuntu-cloud-keyring', fatal=True)
-
-        with open('/etc/apt/sources.list.d/cloud-archive.list', 'w') as f:
-            f.write(src)
-    else:
-        error_out("Invalid openstack-release specified: %s" % rel)
+    """Import a key, either ASCII armored, or a GPG key id.
+
+    @param keyid: the key in ASCII armor format, or a GPG key id.
+    @raises SystemExit() via sys.exit() on failure.
+    """
+    try:
+        return fetch_import_key(keyid)
+    except GPGKeyError as e:
+        error_out("Could not import key: {}".format(str(e)))
+
+
+def get_source_and_pgp_key(source_and_key):
+    """Look for a pgp key ID or ascii-armor key in the given input.
+
+    :param source_and_key: Sting, "source_spec|keyid" where '|keyid' is
+        optional.
+    :returns (source_spec, key_id OR None) as a tuple.  Returns None for key_id
+        if there was no '|' in the source_and_key string.
+    """
+    try:
+        source, key = source_and_key.split('|', 2)
+        return source, key or None
+    except ValueError:
+        return source_and_key, None
+
+
+@deprecate("use charmhelpers.fetch.add_source() instead.",
+           "2017-07", log=juju_log)
+def configure_installation_source(source_plus_key):
+    """Configure an installation source.
+
+    The functionality is provided by charmhelpers.fetch.add_source()
+    The difference between the two functions is that add_source() signature
+    requires the key to be passed directly, whereas this function passes an
+    optional key by appending '|<key>' to the end of the source specificiation
+    'source'.
+
+    Another difference from add_source() is that the function calls sys.exit(1)
+    if the configuration fails, whereas add_source() raises
+    SourceConfigurationError().  Another difference, is that add_source()
+    silently fails (with a juju_log command) if there is no matching source to
+    configure, whereas this function fails with a sys.exit(1)
+
+    :param source: String_plus_key -- see above for details.
+
+    Note that the behaviour on error is to log the error to the juju log and
+    then call sys.exit(1).
+    """
+    # extract the key if there is one, denoted by a '|' in the rel
+    source, key = get_source_and_pgp_key(source_plus_key)
+
+    # handle the ordinary sources via add_source
+    try:
+        fetch_add_source(source, key, fail_invalid=True)
+    except SourceConfigError as se:
+        error_out(str(se))
 
 
 def config_value_changed(option):
@@ -638,7 +575,6 @@
 
     :returns: bool:    : Returns True if configured installation source offers
                          a newer version of package.
-
     """
 
     import apt_pkg as apt

=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2016-09-20 17:07:51 +0000
+++ charmhelpers/fetch/__init__.py	2017-04-10 18:12:16 +0000
@@ -48,6 +48,13 @@
     pass
 
 
+class GPGKeyError(Exception):
+    """Exception occurs when a GPG key cannot be fetched or used.  The message
+    indicates what the problem is.
+    """
+    pass
+
+
 class BaseFetchHandler(object):
 
     """Base class for FetchHandler implementations in fetch plugins"""
@@ -77,21 +84,22 @@
 fetch = importlib.import_module(module)
 
 filter_installed_packages = fetch.filter_installed_packages
-install = fetch.install
-upgrade = fetch.upgrade
-update = fetch.update
-purge = fetch.purge
+install = fetch.apt_install
+upgrade = fetch.apt_upgrade
+update = _fetch_update = fetch.apt_update
+purge = fetch.apt_purge
 add_source = fetch.add_source
 
 if __platform__ == "ubuntu":
     apt_cache = fetch.apt_cache
-    apt_install = fetch.install
-    apt_update = fetch.update
-    apt_upgrade = fetch.upgrade
-    apt_purge = fetch.purge
+    apt_install = fetch.apt_install
+    apt_update = fetch.apt_update
+    apt_upgrade = fetch.apt_upgrade
+    apt_purge = fetch.apt_purge
     apt_mark = fetch.apt_mark
     apt_hold = fetch.apt_hold
     apt_unhold = fetch.apt_unhold
+    import_key = fetch.import_key
     get_upstream_version = fetch.get_upstream_version
 elif __platform__ == "centos":
     yum_search = fetch.yum_search
@@ -135,7 +143,7 @@
         for source, key in zip(sources, keys):
             add_source(source, key)
     if update:
-        fetch.update(fatal=True)
+        _fetch_update(fatal=True)
 
 
 def install_remote(source, *args, **kwargs):

=== modified file 'charmhelpers/fetch/centos.py'
--- charmhelpers/fetch/centos.py	2016-08-11 15:47:44 +0000
+++ charmhelpers/fetch/centos.py	2017-04-10 18:12:16 +0000
@@ -132,7 +132,7 @@
                 key_file.write(key)
                 key_file.flush()
                 key_file.seek(0)
-            subprocess.check_call(['rpm', '--import', key_file])
+                subprocess.check_call(['rpm', '--import', key_file.name])
         else:
             subprocess.check_call(['rpm', '--import', key])
 

=== modified file 'charmhelpers/fetch/ubuntu.py'
--- charmhelpers/fetch/ubuntu.py	2017-03-03 20:37:37 +0000
+++ charmhelpers/fetch/ubuntu.py	2017-04-10 18:12:16 +0000
@@ -12,29 +12,46 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from collections import OrderedDict
 import os
+import platform
+import re
 import six
 import time
 import subprocess
-
 from tempfile import NamedTemporaryFile
+
 from charmhelpers.core.host import (
     lsb_release
 )
-from charmhelpers.core.hookenv import log
-from charmhelpers.fetch import SourceConfigError
+from charmhelpers.core.hookenv import (
+    log,
+    DEBUG,
+)
+from charmhelpers.fetch import SourceConfigError, GPGKeyError
 
+PROPOSED_POCKET = (
+    "# Proposed\n"
+    "deb http://archive.ubuntu.com/ubuntu {}-proposed main universe "
+    "multiverse restricted\n")
+PROPOSED_PORTS_POCKET = (
+    "# Proposed\n"
+    "deb http://ports.ubuntu.com/ubuntu-ports {}-proposed main universe "
+    "multiverse restricted\n")
+# Only supports 64bit and ppc64 at the moment.
+ARCH_TO_PROPOSED_POCKET = {
+    'x86_64': PROPOSED_POCKET,
+    'ppc64le': PROPOSED_PORTS_POCKET,
+}
+CLOUD_ARCHIVE_URL = "http://ubuntu-cloud.archive.canonical.com/ubuntu"
+CLOUD_ARCHIVE_KEY_ID = '5EDB1B62EC4926EA'
 CLOUD_ARCHIVE = """# Ubuntu Cloud Archive
 deb http://ubuntu-cloud.archive.canonical.com/ubuntu {} main
 """
-
-PROPOSED_POCKET = """# Proposed
-deb http://archive.ubuntu.com/ubuntu {}-proposed main universe multiverse restricted
-"""
-
 CLOUD_ARCHIVE_POCKETS = {
     # Folsom
     'folsom': 'precise-updates/folsom',
+    'folsom/updates': 'precise-updates/folsom',
     'precise-folsom': 'precise-updates/folsom',
     'precise-folsom/updates': 'precise-updates/folsom',
     'precise-updates/folsom': 'precise-updates/folsom',
@@ -43,6 +60,7 @@
     'precise-proposed/folsom': 'precise-proposed/folsom',
     # Grizzly
     'grizzly': 'precise-updates/grizzly',
+    'grizzly/updates': 'precise-updates/grizzly',
     'precise-grizzly': 'precise-updates/grizzly',
     'precise-grizzly/updates': 'precise-updates/grizzly',
     'precise-updates/grizzly': 'precise-updates/grizzly',
@@ -51,6 +69,7 @@
     'precise-proposed/grizzly': 'precise-proposed/grizzly',
     # Havana
     'havana': 'precise-updates/havana',
+    'havana/updates': 'precise-updates/havana',
     'precise-havana': 'precise-updates/havana',
     'precise-havana/updates': 'precise-updates/havana',
     'precise-updates/havana': 'precise-updates/havana',
@@ -59,6 +78,7 @@
     'precise-proposed/havana': 'precise-proposed/havana',
     # Icehouse
     'icehouse': 'precise-updates/icehouse',
+    'icehouse/updates': 'precise-updates/icehouse',
     'precise-icehouse': 'precise-updates/icehouse',
     'precise-icehouse/updates': 'precise-updates/icehouse',
     'precise-updates/icehouse': 'precise-updates/icehouse',
@@ -67,6 +87,7 @@
     'precise-proposed/icehouse': 'precise-proposed/icehouse',
     # Juno
     'juno': 'trusty-updates/juno',
+    'juno/updates': 'trusty-updates/juno',
     'trusty-juno': 'trusty-updates/juno',
     'trusty-juno/updates': 'trusty-updates/juno',
     'trusty-updates/juno': 'trusty-updates/juno',
@@ -75,6 +96,7 @@
     'trusty-proposed/juno': 'trusty-proposed/juno',
     # Kilo
     'kilo': 'trusty-updates/kilo',
+    'kilo/updates': 'trusty-updates/kilo',
     'trusty-kilo': 'trusty-updates/kilo',
     'trusty-kilo/updates': 'trusty-updates/kilo',
     'trusty-updates/kilo': 'trusty-updates/kilo',
@@ -83,6 +105,7 @@
     'trusty-proposed/kilo': 'trusty-proposed/kilo',
     # Liberty
     'liberty': 'trusty-updates/liberty',
+    'liberty/updates': 'trusty-updates/liberty',
     'trusty-liberty': 'trusty-updates/liberty',
     'trusty-liberty/updates': 'trusty-updates/liberty',
     'trusty-updates/liberty': 'trusty-updates/liberty',
@@ -91,6 +114,7 @@
     'trusty-proposed/liberty': 'trusty-proposed/liberty',
     # Mitaka
     'mitaka': 'trusty-updates/mitaka',
+    'mitaka/updates': 'trusty-updates/mitaka',
     'trusty-mitaka': 'trusty-updates/mitaka',
     'trusty-mitaka/updates': 'trusty-updates/mitaka',
     'trusty-updates/mitaka': 'trusty-updates/mitaka',
@@ -99,6 +123,7 @@
     'trusty-proposed/mitaka': 'trusty-proposed/mitaka',
     # Newton
     'newton': 'xenial-updates/newton',
+    'newton/updates': 'xenial-updates/newton',
     'xenial-newton': 'xenial-updates/newton',
     'xenial-newton/updates': 'xenial-updates/newton',
     'xenial-updates/newton': 'xenial-updates/newton',
@@ -107,6 +132,7 @@
     'xenial-proposed/newton': 'xenial-proposed/newton',
     # Ocata
     'ocata': 'xenial-updates/ocata',
+    'ocata/updates': 'xenial-updates/ocata',
     'xenial-ocata': 'xenial-updates/ocata',
     'xenial-ocata/updates': 'xenial-updates/ocata',
     'xenial-updates/ocata': 'xenial-updates/ocata',
@@ -115,6 +141,7 @@
     'xenial-ocata/newton': 'xenial-proposed/ocata',
 }
 
+
 APT_NO_LOCK = 100  # The return code for "couldn't acquire lock" in APT.
 CMD_RETRY_DELAY = 10  # Wait 10 seconds between command retries.
 CMD_RETRY_COUNT = 30  # Retry a failing fatal command X times.
@@ -145,7 +172,7 @@
     return apt_pkg.Cache(progress)
 
 
-def install(packages, options=None, fatal=False):
+def apt_install(packages, options=None, fatal=False):
     """Install one or more packages."""
     if options is None:
         options = ['--option=Dpkg::Options::=--force-confold']
@@ -162,7 +189,7 @@
     _run_apt_command(cmd, fatal)
 
 
-def upgrade(options=None, fatal=False, dist=False):
+def apt_upgrade(options=None, fatal=False, dist=False):
     """Upgrade all packages."""
     if options is None:
         options = ['--option=Dpkg::Options::=--force-confold']
@@ -177,13 +204,13 @@
     _run_apt_command(cmd, fatal)
 
 
-def update(fatal=False):
+def apt_update(fatal=False):
     """Update local apt cache."""
     cmd = ['apt-get', 'update']
     _run_apt_command(cmd, fatal)
 
 
-def purge(packages, fatal=False):
+def apt_purge(packages, fatal=False):
     """Purge one or more packages."""
     cmd = ['apt-get', '--assume-yes', 'purge']
     if isinstance(packages, six.string_types):
@@ -217,7 +244,45 @@
     return apt_mark(packages, 'unhold', fatal=fatal)
 
 
-def add_source(source, key=None):
+def import_key(keyid):
+    """Import a key in either ASCII Armor or Radix64 format.
+
+    `keyid` is either the keyid to fetch from a PGP server, or
+    the key in ASCII armor foramt.
+
+    :param keyid: String of key (or key id).
+    :raises: GPGKeyError if the key could not be imported
+    """
+    key = keyid.strip()
+    if (key.startswith('-----BEGIN PGP PUBLIC KEY BLOCK-----') and
+            key.endswith('-----END PGP PUBLIC KEY BLOCK-----')):
+        log("PGP key found (looks like ASCII Armor format)", level=DEBUG)
+        log("Importing ASCII Armor PGP key", level=DEBUG)
+        with NamedTemporaryFile() as keyfile:
+            with open(keyfile.name, 'w') as fd:
+                fd.write(key)
+                fd.write("\n")
+            cmd = ['apt-key', 'add', keyfile.name]
+            try:
+                subprocess.check_call(cmd)
+            except subprocess.CalledProcessError:
+                error = "Error importing PGP key '{}'".format(key)
+                log(error)
+                raise GPGKeyError(error)
+    else:
+        log("PGP key found (looks like Radix64 format)", level=DEBUG)
+        log("Importing PGP key from keyserver", level=DEBUG)
+        cmd = ['apt-key', 'adv', '--keyserver',
+               'hkp://keyserver.ubuntu.com:80', '--recv-keys', key]
+        try:
+            subprocess.check_call(cmd)
+        except subprocess.CalledProcessError:
+            error = "Error importing PGP key '{}'".format(key)
+            log(error)
+            raise GPGKeyError(error)
+
+
+def add_source(source, key=None, fail_invalid=False):
     """Add a package source to this system.
 
     @param source: a URL or sources.list entry, as supported by
@@ -233,6 +298,33 @@
         such as 'cloud:icehouse'
         'distro' may be used as a noop
 
+    Full list of source specifications supported by the function are:
+
+    'distro': A NOP; i.e. it has no effect.
+    'proposed': the proposed deb spec [2] is wrtten to
+      /etc/apt/sources.list/proposed
+    'distro-proposed': adds <version>-proposed to the debs [2]
+    'ppa:<ppa-name>': add-apt-repository --yes <ppa_name>
+    'deb <deb-spec>': add-apt-repository --yes deb <deb-spec>
+    'http://....': add-apt-repository --yes http://...
+    'cloud-archive:<spec>': add-apt-repository -yes cloud-archive:<spec>
+    'cloud:<release>[-staging]': specify a Cloud Archive pocket <release> with
+      optional staging version.  If staging is used then the staging PPA [2]
+      with be used.  If staging is NOT used then the cloud archive [3] will be
+      added, and the 'ubuntu-cloud-keyring' package will be added for the
+      current distro.
+
+    Otherwise the source is not recognised and this is logged to the juju log.
+    However, no error is raised, unless sys_error_on_exit is True.
+
+    [1] deb http://ubuntu-cloud.archive.canonical.com/ubuntu {} main
+        where {} is replaced with the derived pocket name.
+    [2] deb http://archive.ubuntu.com/ubuntu {}-proposed \
+        main universe multiverse restricted
+        where {} is replaced with the lsb_release codename (e.g. xenial)
+    [3] deb http://ubuntu-cloud.archive.canonical.com/ubuntu <pocket>
+        to /etc/apt/sources.list.d/cloud-archive-list
+
     @param key: A key to be added to the system's APT keyring and used
     to verify the signatures on packages. Ideally, this should be an
     ASCII format GPG public key including the block headers. A GPG key
@@ -240,51 +332,141 @@
     available to retrieve the actual public key from a public keyserver
     placing your Juju environment at risk. ppa and cloud archive keys
     are securely added automtically, so sould not be provided.
+
+    @param fail_invalid: (boolean) if True, then the function raises a
+    SourceConfigError is there is no matching installation source.
+
+    @raises SourceConfigError() if for cloud:<pocket>, the <pocket> is not a
+    valid pocket in CLOUD_ARCHIVE_POCKETS
     """
+    _mapping = OrderedDict([
+        (r"^distro$", lambda: None),  # This is a NOP
+        (r"^(?:proposed|distro-proposed)$", _add_proposed),
+        (r"^cloud-archive:(.*)$", _add_apt_repository),
+        (r"^((?:deb |http:|https:|ppa:).*)$", _add_apt_repository),
+        (r"^cloud:(.*)-(.*)\/staging$", _add_cloud_staging),
+        (r"^cloud:(.*)-(.*)$", _add_cloud_distro_check),
+        (r"^cloud:(.*)$", _add_cloud_pocket),
+    ])
     if source is None:
-        log('Source is not present. Skipping')
-        return
-
-    if (source.startswith('ppa:') or
-        source.startswith('http') or
-        source.startswith('deb ') or
-            source.startswith('cloud-archive:')):
-        cmd = ['add-apt-repository', '--yes', source]
-        _run_with_retries(cmd)
-    elif source.startswith('cloud:'):
-        install(filter_installed_packages(['ubuntu-cloud-keyring']),
+        source = ''
+    for r, fn in six.iteritems(_mapping):
+        m = re.match(r, source)
+        if m:
+            # call the assoicated function with the captured groups
+            # raises SourceConfigError on error.
+            fn(*m.groups())
+            if key:
+                try:
+                    import_key(key)
+                except GPGKeyError as e:
+                    raise SourceConfigError(str(e))
+            break
+    else:
+        # nothing matched.  log an error and maybe sys.exit
+        err = "Unknown source: {!r}".format(source)
+        log(err)
+        if fail_invalid:
+            raise SourceConfigError(err)
+
+
+def _add_proposed():
+    """Add the PROPOSED_POCKET as /etc/apt/source.list.d/proposed.list
+
+    Uses lsb_release()['DISTRIB_CODENAME'] to determine the correct staza for
+    the deb line.
+
+    For intel architecutres PROPOSED_POCKET is used for the release, but for
+    other architectures PROPOSED_PORTS_POCKET is used for the release.
+    """
+    release = lsb_release()['DISTRIB_CODENAME']
+    arch = platform.machine()
+    if arch not in six.iterkeys(ARCH_TO_PROPOSED_POCKET):
+        raise SourceConfigError("Arch {} not supported for (distro-)proposed"
+                                .format(arch))
+    with open('/etc/apt/sources.list.d/proposed.list', 'w') as apt:
+        apt.write(ARCH_TO_PROPOSED_POCKET[arch].format(release))
+
+
+def _add_apt_repository(spec):
+    """Add the spec using add_apt_repository
+
+    :param spec: the parameter to pass to add_apt_repository
+    """
+    _run_with_retries(['add-apt-repository', '--yes', spec])
+
+
+def _add_cloud_pocket(pocket):
+    """Add a cloud pocket as /etc/apt/sources.d/cloud-archive.list
+
+    Note that this overwrites the existing file if there is one.
+
+    This function also converts the simple pocket in to the actual pocket using
+    the CLOUD_ARCHIVE_POCKETS mapping.
+
+    :param pocket: string representing the pocket to add a deb spec for.
+    :raises: SourceConfigError if the cloud pocket doesn't exist or the
+        requested release doesn't match the current distro version.
+    """
+    apt_install(filter_installed_packages(['ubuntu-cloud-keyring']),
                 fatal=True)
-        pocket = source.split(':')[-1]
-        if pocket not in CLOUD_ARCHIVE_POCKETS:
-            raise SourceConfigError(
-                'Unsupported cloud: source option %s' %
-                pocket)
-        actual_pocket = CLOUD_ARCHIVE_POCKETS[pocket]
-        with open('/etc/apt/sources.list.d/cloud-archive.list', 'w') as apt:
-            apt.write(CLOUD_ARCHIVE.format(actual_pocket))
-    elif source == 'proposed':
-        release = lsb_release()['DISTRIB_CODENAME']
-        with open('/etc/apt/sources.list.d/proposed.list', 'w') as apt:
-            apt.write(PROPOSED_POCKET.format(release))
-    elif source == 'distro':
-        pass
-    else:
-        log("Unknown source: {!r}".format(source))
-
-    if key:
-        if '-----BEGIN PGP PUBLIC KEY BLOCK-----' in key:
-            with NamedTemporaryFile('w+') as key_file:
-                key_file.write(key)
-                key_file.flush()
-                key_file.seek(0)
-                subprocess.check_call(['apt-key', 'add', '-'], stdin=key_file)
-        else:
-            # Note that hkp: is in no way a secure protocol. Using a
-            # GPG key id is pointless from a security POV unless you
-            # absolutely trust your network and DNS.
-            subprocess.check_call(['apt-key', 'adv', '--keyserver',
-                                   'hkp://keyserver.ubuntu.com:80', '--recv',
-                                   key])
+    if pocket not in CLOUD_ARCHIVE_POCKETS:
+        raise SourceConfigError(
+            'Unsupported cloud: source option %s' %
+            pocket)
+    actual_pocket = CLOUD_ARCHIVE_POCKETS[pocket]
+    with open('/etc/apt/sources.list.d/cloud-archive.list', 'w') as apt:
+        apt.write(CLOUD_ARCHIVE.format(actual_pocket))
+
+
+def _add_cloud_staging(cloud_archive_release, openstack_release):
+    """Add the cloud staging repository which is in
+    ppa:ubuntu-cloud-archive/<openstack_release>-staging
+
+    This function checks that the cloud_archive_release matches the current
+    codename for the distro that charm is being installed on.
+
+    :param cloud_archive_release: string, codename for the release.
+    :param openstack_release: String, codename for the openstack release.
+    :raises: SourceConfigError if the cloud_archive_release doesn't match the
+        current version of the os.
+    """
+    _verify_is_ubuntu_rel(cloud_archive_release, openstack_release)
+    ppa = 'ppa:ubuntu-cloud-archive/{}-staging'.format(openstack_release)
+    cmd = 'add-apt-repository -y {}'.format(ppa)
+    _run_with_retries(cmd.split(' '))
+
+
+def _add_cloud_distro_check(cloud_archive_release, openstack_release):
+    """Add the cloud pocket, but also check the cloud_archive_release against
+    the current distro, and use the openstack_release as the full lookup.
+
+    This just calls _add_cloud_pocket() with the openstack_release as pocket
+    to get the correct cloud-archive.list for dpkg to work with.
+
+    :param cloud_archive_release:String, codename for the distro release.
+    :param openstack_release: String, spec for the release to look up in the
+        CLOUD_ARCHIVE_POCKETS
+    :raises: SourceConfigError if this is the wrong distro, or the pocket spec
+        doesn't exist.
+    """
+    _verify_is_ubuntu_rel(cloud_archive_release, openstack_release)
+    _add_cloud_pocket("{}-{}".format(cloud_archive_release, openstack_release))
+
+
+def _verify_is_ubuntu_rel(release, os_release):
+    """Verify that the release is in the same as the current ubuntu release.
+
+    :param release: String, lowercase for the release.
+    :param os_release: String, the os_release being asked for
+    :raises: SourceConfigError if the release is not the same as the ubuntu
+        release.
+    """
+    ubuntu_rel = lsb_release()['DISTRIB_CODENAME']
+    if release != ubuntu_rel:
+        raise SourceConfigError(
+            'Invalid Cloud Archive release specified: {}-{} on this Ubuntu'
+            'version ({})'.format(release, os_release, ubuntu_rel))
 
 
 def _run_with_retries(cmd, max_retries=CMD_RETRY_COUNT, retry_exitcodes=(1,),
@@ -300,9 +482,12 @@
     :param: cmd_env: dict: Environment variables to add to the command run.
     """
 
-    env = os.environ.copy()
+    env = None
+    kwargs = {}
     if cmd_env:
+        env = os.environ.copy()
         env.update(cmd_env)
+        kwargs['env'] = env
 
     if not retry_message:
         retry_message = "Failed executing '{}'".format(" ".join(cmd))
@@ -314,7 +499,8 @@
     retry_results = (None,) + retry_exitcodes
     while result in retry_results:
         try:
-            result = subprocess.check_call(cmd, env=env)
+            # result = subprocess.check_call(cmd, env=env)
+            result = subprocess.check_call(cmd, **kwargs)
         except subprocess.CalledProcessError as e:
             retry_count = retry_count + 1
             if retry_count > max_retries:
@@ -327,6 +513,7 @@
 def _run_apt_command(cmd, fatal=False):
     """Run an apt command with optional retries.
 
+    :param: cmd: str: The apt command to run.
     :param: fatal: bool: Whether the command's output should be checked and
         retried.
     """

=== modified file 'tests/contrib/openstack/test_openstack_utils.py'
--- tests/contrib/openstack/test_openstack_utils.py	2016-12-14 18:51:15 +0000
+++ tests/contrib/openstack/test_openstack_utils.py	2017-04-10 18:12:16 +0000
@@ -1,13 +1,13 @@
 import io
 import os
-import subprocess
-import tempfile
 import contextlib
 import unittest
 from copy import copy
+from tests.helpers import patch_open
 from testtools import TestCase
 from mock import MagicMock, patch, call
 
+from charmhelpers.fetch import ubuntu as fetch
 import charmhelpers.contrib.openstack.utils as openstack
 
 import six
@@ -123,23 +123,9 @@
 
 # Mock python-dnspython resolver used by get_host_ip()
 
-PGP_KEY_ASCII_ARMOR = """-----BEGIN PGP PUBLIC KEY BLOCK-----
-Version: SKS 1.1.5
-Comment: Hostname: keyserver.ubuntu.com
-
-mI0EUCEyTAEEAMuUxyfiegCCwn4J/c0nw5PUTSJdn5FqiUTq6iMfij65xf1vl0g/Mxqw0gfg
-AJIsCDvO9N9dloLAwF6FUBMg5My7WyhRPTAKF505TKJboyX3Pp4J1fU1LV8QFVOp87vUh1Rz
-B6GU7cSglhnbL85gmbJTllkzkb3h4Yw7W+edjcQ/ABEBAAG0K0xhdW5jaHBhZCBQUEEgZm9y
-IFVidW50dSBDbG91ZCBBcmNoaXZlIFRlYW2IuAQTAQIAIgUCUCEyTAIbAwYLCQgHAwIGFQgC
-CQoLBBYCAwECHgECF4AACgkQimhEop9oEE7kJAP/eTBgq3Mhbvo0d8elMOuqZx3nmU7gSyPh
-ep0zYIRZ5TJWl/7PRtvp0CJA6N6ZywYTQ/4ANHhpibcHZkh8K0AzUvsGXnJRSFoJeqyDbD91
-EhoO+4ZfHs2HvRBQEDZILMa2OyuB497E5Mmyua3HDEOrG2cVLllsUZzpTFCx8NgeMHk=
-=jLBm
------END PGP PUBLIC KEY BLOCK-----
-"""
-
 
 class FakeAnswer(object):
+
     def __init__(self, ip):
         self.ip = ip
 
@@ -148,6 +134,7 @@
 
 
 class FakeResolver(object):
+
     def __init__(self, ip):
         self.ip = ip
 
@@ -159,16 +146,19 @@
 
 
 class FakeReverse(object):
+
     def from_address(self, address):
         return '156.94.189.91.in-addr.arpa'
 
 
 class FakeDNSName(object):
+
     def __init__(self, dnsname):
         pass
 
 
 class FakeDNS(object):
+
     def __init__(self, ip):
         self.resolver = FakeResolver(ip)
         self.reversename = FakeReverse()
@@ -177,6 +167,7 @@
 
 
 class OpenStackHelpersTestCase(TestCase):
+
     def _apt_cache(self):
         # mocks out the apt cache
         def cache_get(package):
@@ -197,7 +188,7 @@
 
     @patch('charmhelpers.contrib.openstack.utils.lsb_release')
     def test_os_codename_from_install_source(self, mocked_lsb):
-        '''Test mapping install source to OpenStack release name'''
+        """Test mapping install source to OpenStack release name"""
         mocked_lsb.return_value = FAKE_RELEASE
 
         # the openstack release shipped with respective ubuntu/lsb release.
@@ -236,7 +227,7 @@
 
     @patch('charmhelpers.contrib.openstack.utils.lsb_release')
     def test_os_codename_from_bad_install_source(self, mocked_lsb):
-        '''Test mapping install source to OpenStack release name'''
+        """Test mapping install source to OpenStack release name"""
         _fake_release = copy(FAKE_RELEASE)
         _fake_release['DISTRIB_CODENAME'] = 'natty'
 
@@ -249,32 +240,32 @@
             mocked_err.assert_called_with(_er)
 
     def test_os_codename_from_version(self):
-        '''Test mapping OpenStack numerical versions to code name'''
+        """Test mapping OpenStack numerical versions to code name"""
         self.assertEquals(openstack.get_os_codename_version('2013.1'),
                           'grizzly')
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_os_codename_from_bad_version(self, mocked_error):
-        '''Test mapping a bad OpenStack numerical versions to code name'''
+        """Test mapping a bad OpenStack numerical versions to code name"""
         openstack.get_os_codename_version('2014.5.5')
         expected_err = ('Could not determine OpenStack codename for '
                         'version 2014.5.5')
         mocked_error.assert_called_with(expected_err)
 
     def test_os_version_from_codename(self):
-        '''Test mapping a OpenStack codename to numerical version'''
+        """Test mapping a OpenStack codename to numerical version"""
         self.assertEquals(openstack.get_os_version_codename('folsom'),
                           '2012.2')
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_os_version_from_bad_codename(self, mocked_error):
-        '''Test mapping a bad OpenStack codename to numerical version'''
+        """Test mapping a bad OpenStack codename to numerical version"""
         openstack.get_os_version_codename('foo')
         expected_err = 'Could not derive OpenStack version for codename: foo'
         mocked_error.assert_called_with(expected_err)
 
     def test_os_version_swift_from_codename(self):
-        '''Test mapping a swift codename to numerical version'''
+        """Test mapping a swift codename to numerical version"""
         self.assertEquals(openstack.get_os_version_codename_swift('liberty'),
                           '2.5.0')
 
@@ -283,7 +274,7 @@
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_os_version_swift_from_bad_codename(self, mocked_error):
-        '''Test mapping a bad swift codename to numerical version'''
+        """Test mapping a bad swift codename to numerical version"""
         openstack.get_os_version_codename_swift('foo')
         expected_err = 'Could not derive swift version for codename: foo'
         mocked_error.assert_called_with(expected_err)
@@ -302,7 +293,7 @@
         self.assertEquals(openstack.get_swift_codename('1.2.3'), None)
 
     def test_os_codename_from_package(self):
-        '''Test deriving OpenStack codename from an installed package'''
+        """Test deriving OpenStack codename from an installed package"""
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
             for pkg, vers in six.iteritems(FAKE_REPO):
@@ -316,7 +307,7 @@
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_os_codename_from_bad_package_version(self, mocked_error):
-        '''Test deriving OpenStack codename for a poorly versioned package'''
+        """Test deriving OpenStack codename for a poorly versioned package"""
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
             openstack.get_os_codename_package('bad-version')
@@ -325,7 +316,7 @@
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_os_codename_from_bad_package(self, mocked_error):
-        '''Test deriving OpenStack codename from an unavailable package'''
+        """Test deriving OpenStack codename from an unavailable package"""
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
             try:
@@ -339,7 +330,7 @@
             mocked_error.assert_called_with(e)
 
     def test_os_codename_from_bad_package_nonfatal(self):
-        '''Test OpenStack codename from an unavailable package is non-fatal'''
+        """Test OpenStack codename from an unavailable package is non-fatal"""
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
             self.assertEquals(
@@ -349,7 +340,7 @@
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_os_codename_from_uninstalled_package(self, mock_error):
-        '''Test OpenStack codename from an available but uninstalled pkg'''
+        """Test OpenStack codename from an available but uninstalled pkg"""
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
             try:
@@ -361,7 +352,7 @@
             mock_error.assert_called_with(e)
 
     def test_os_codename_from_uninstalled_package_nonfatal(self):
-        '''Test OpenStack codename from avail uninstalled pkg is non fatal'''
+        """Test OpenStack codename from avail uninstalled pkg is non fatal"""
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
             self.assertEquals(
@@ -371,7 +362,7 @@
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_os_version_from_package(self, mocked_error):
-        '''Test deriving OpenStack version from an installed package'''
+        """Test deriving OpenStack version from an installed package"""
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
             for pkg, vers in six.iteritems(FAKE_REPO):
@@ -384,7 +375,7 @@
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_os_version_from_bad_package(self, mocked_error):
-        '''Test deriving OpenStack version from an uninstalled package'''
+        """Test deriving OpenStack version from an uninstalled package"""
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
             try:
@@ -398,7 +389,7 @@
             mocked_error.assert_called_with(e)
 
     def test_os_version_from_bad_package_nonfatal(self):
-        '''Test OpenStack version from an uninstalled package is non-fatal'''
+        """Test OpenStack version from an uninstalled package is non-fatal"""
         with patch('apt_pkg.Cache') as cache:
             cache.return_value = self._apt_cache()
             self.assertEquals(
@@ -410,7 +401,7 @@
     @patch.object(openstack, 'git_os_codename_install_source')
     @patch('charmhelpers.contrib.openstack.utils.config')
     def test_os_release_uncached(self, config, git_cn, get_cn):
-        openstack.os_rel = None
+        openstack._os_rel = None
         get_cn.return_value = 'folsom'
         git_cn.return_value = None
 
@@ -420,69 +411,142 @@
         self.assertEquals('folsom', openstack.os_release('nova-common'))
 
     def test_os_release_cached(self):
-        openstack.os_rel = 'foo'
+        openstack._os_rel = 'foo'
         self.assertEquals('foo', openstack.os_release('nova-common'))
 
     @patch.object(openstack, 'juju_log')
     @patch('sys.exit')
     def test_error_out(self, mocked_exit, juju_log):
-        '''Test erroring out'''
+        """Test erroring out"""
         openstack.error_out('Everything broke.')
         _log = 'FATAL ERROR: Everything broke.'
         juju_log.assert_called_with(_log, level='ERROR')
         mocked_exit.assert_called_with(1)
 
+    def test_get_source_and_pgp_key(self):
+        tests = {
+            "source|key": ('source', 'key'),
+            "source|": ('source', None),
+            "|key": ('', 'key'),
+            "source": ('source', None),
+        }
+        for k, v in six.iteritems(tests):
+            self.assertEqual(openstack.get_source_and_pgp_key(k), v)
+
+    # These should still work, even though the bulk of the functionality has
+    # moved to charmhelpers.fetch.add_source()
     def test_configure_install_source_distro(self):
-        '''Test configuring installation from distro'''
+        """Test configuring installation from distro"""
         self.assertIsNone(openstack.configure_installation_source('distro'))
 
     def test_configure_install_source_ppa(self):
-        '''Test configuring installation source from PPA'''
+        """Test configuring installation source from PPA"""
         with patch('subprocess.check_call') as mock:
             src = 'ppa:gandelman-a/openstack'
             openstack.configure_installation_source(src)
-            ex_cmd = ['add-apt-repository', '-y', 'ppa:gandelman-a/openstack']
+            ex_cmd = [
+                'add-apt-repository', '--yes', 'ppa:gandelman-a/openstack']
             mock.assert_called_with(ex_cmd)
 
-    @patch(builtin_open)
-    @patch('charmhelpers.contrib.openstack.utils.juju_log')
-    @patch('charmhelpers.contrib.openstack.utils.import_key')
-    def test_configure_install_source_deb_url(self, _import, _log, _open):
-        '''Test configuring installation source from deb repo url'''
-        _file = MagicMock(spec=io.FileIO)
-        _open.return_value = _file
+    @patch('subprocess.check_call')
+    @patch.object(fetch, 'import_key')
+    def test_configure_install_source_deb_url(self, _import, _spcc):
+        """Test configuring installation source from deb repo url"""
         src = ('deb http://ubuntu-cloud.archive.canonical.com/ubuntu '
                'precise-havana main|KEYID')
         openstack.configure_installation_source(src)
         _import.assert_called_with('KEYID')
-        _file.__enter__().write.assert_called_with(src.split('|')[0])
-        src = ('deb http://ubuntu-cloud.archive.canonical.com/ubuntu '
-               'precise-havana main')
-        openstack.configure_installation_source(src)
-        _file.__enter__().write.assert_called_with(src)
+        _spcc.assert_called_once_with(
+            ['add-apt-repository', '--yes',
+             'deb http://ubuntu-cloud.archive.canonical.com/ubuntu '
+             'precise-havana main'])
 
-    @patch('charmhelpers.contrib.openstack.utils.lsb_release')
+    @patch.object(fetch, 'lsb_release')
     @patch(builtin_open)
-    @patch('charmhelpers.contrib.openstack.utils.juju_log')
-    def test_configure_install_source_distro_proposed(self, _log, _open, _lsb):
-        '''Test configuring installation source from deb repo url'''
+    @patch('subprocess.check_call')
+    def test_configure_install_source_distro_proposed(
+            self, _spcc, _open, _lsb):
+        """Test configuring installation source from deb repo url"""
         _lsb.return_value = FAKE_RELEASE
         _file = MagicMock(spec=io.FileIO)
         _open.return_value = _file
         openstack.configure_installation_source('distro-proposed')
+        _file.__enter__().write.assert_called_once_with(
+            '# Proposed\ndeb http://archive.ubuntu.com/ubuntu '
+            'precise-proposed main universe multiverse restricted\n')
         src = ('deb http://archive.ubuntu.com/ubuntu/ precise-proposed '
                'restricted main multiverse universe')
         openstack.configure_installation_source(src)
-        _file.__enter__().write.assert_called_with(src)
+        _spcc.assert_called_once_with(
+            ['add-apt-repository', '--yes',
+             'deb http://archive.ubuntu.com/ubuntu/ precise-proposed '
+             'restricted main multiverse universe'])
+
+    @patch('charmhelpers.fetch.filter_installed_packages')
+    @patch('charmhelpers.fetch.apt_install')
+    @patch.object(openstack, 'error_out')
+    @patch.object(openstack, 'juju_log')
+    def test_add_source_cloud_invalid_pocket(self, _log, _out,
+                                             apt_install, filter_pkg):
+        openstack.configure_installation_source("cloud:havana-updates")
+        _e = ('Invalid Cloud Archive release specified: '
+              'havana-updates on this Ubuntuversion')
+        _s = _out.call_args[0][0]
+        self.assertTrue(_s.startswith(_e))
+
+    @patch.object(fetch, 'filter_installed_packages')
+    @patch.object(fetch, 'apt_install')
+    @patch.object(fetch, 'lsb_release')
+    def test_add_source_cloud_pocket_style(self, lsb_release,
+                                           apt_install, filter_pkg):
+        source = "cloud:precise-updates/havana"
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
+        result = (
+            "# Ubuntu Cloud Archive\n"
+            "deb http://ubuntu-cloud.archive.canonical.com/ubuntu "
+            "precise-updates/havana main\n")
+        with patch_open() as (mock_open, mock_file):
+            openstack.configure_installation_source(source)
+            mock_file.write.assert_called_with(result)
+        filter_pkg.assert_called_with(['ubuntu-cloud-keyring'])
+
+    @patch.object(fetch, 'filter_installed_packages')
+    @patch.object(fetch, 'apt_install')
+    @patch.object(fetch, 'lsb_release')
+    def test_add_source_cloud_os_style(self, lsb_release,
+                                       apt_install, filter_pkg):
+        source = "cloud:precise-havana"
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
+        result = (
+            "# Ubuntu Cloud Archive\n"
+            "deb http://ubuntu-cloud.archive.canonical.com/ubuntu "
+            "precise-updates/havana main\n")
+        with patch_open() as (mock_open, mock_file):
+            openstack.configure_installation_source(source)
+            mock_file.write.assert_called_with(result)
+        filter_pkg.assert_called_with(['ubuntu-cloud-keyring'])
+
+    @patch.object(fetch, 'filter_installed_packages')
+    @patch.object(fetch, 'apt_install')
+    def test_add_source_cloud_distroless_style(self, apt_install, filter_pkg):
+        source = "cloud:havana"
+        result = (
+            "# Ubuntu Cloud Archive\n"
+            "deb http://ubuntu-cloud.archive.canonical.com/ubuntu "
+            "precise-updates/havana main\n")
+        with patch_open() as (mock_open, mock_file):
+            openstack.configure_installation_source(source)
+            mock_file.write.assert_called_with(result)
+        filter_pkg.assert_called_with(['ubuntu-cloud-keyring'])
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_configure_bad_install_source(self, _error):
         openstack.configure_installation_source('foo')
-        _error.assert_called_with('Invalid openstack-release specified: foo')
+        _error.assert_called_with("Unknown source: 'foo'")
 
-    @patch('charmhelpers.contrib.openstack.utils.lsb_release')
+    @patch.object(fetch, 'lsb_release')
     def test_configure_install_source_uca_staging(self, _lsb):
-        '''Test configuring installation source from UCA staging sources'''
+        """Test configuring installation source from UCA staging sources"""
         _lsb.return_value = FAKE_RELEASE
         # staging pockets are configured as PPAs
         with patch('subprocess.check_call') as _subp:
@@ -493,76 +557,63 @@
             _subp.assert_called_with(cmd)
 
     @patch(builtin_open)
-    @patch('charmhelpers.contrib.openstack.utils.apt_install')
-    @patch('charmhelpers.contrib.openstack.utils.lsb_release')
-    def test_configure_install_source_uca_repos(self, _lsb, _install, _open):
-        '''Test configuring installation source from UCA sources'''
+    @patch.object(fetch, 'apt_install')
+    @patch.object(fetch, 'lsb_release')
+    @patch.object(fetch, 'filter_installed_packages')
+    def test_configure_install_source_uca_repos(
+            self, _fip, _lsb, _install, _open):
+        """Test configuring installation source from UCA sources"""
         _lsb.return_value = FAKE_RELEASE
         _file = MagicMock(spec=io.FileIO)
         _open.return_value = _file
+        _fip.side_effect = lambda x: x
         for src, url in UCA_SOURCES:
+            actual_url = "# Ubuntu Cloud Archive\n{}\n".format(url)
             openstack.configure_installation_source(src)
-            _install.assert_called_with('ubuntu-cloud-keyring',
+            _install.assert_called_with(['ubuntu-cloud-keyring'],
                                         fatal=True)
             _open.assert_called_with(
                 '/etc/apt/sources.list.d/cloud-archive.list',
                 'w'
             )
-            _file.__enter__().write.assert_called_with(url)
+            _file.__enter__().write.assert_called_with(actual_url)
 
     @patch('charmhelpers.contrib.openstack.utils.error_out')
     def test_configure_install_source_bad_uca(self, mocked_error):
-        '''Test configuring installation source from bad UCA source'''
+        """Test configuring installation source from bad UCA source"""
         try:
             openstack.configure_installation_source('cloud:foo-bar')
         except:
             # ignore exceptions that raise when error_out is mocked
             # and doesn't sys.exit(1)
             pass
-        _e = 'Invalid Cloud Archive release specified: foo-bar'
-        mocked_error.assert_called_with(_e)
-
-    @patch.object(openstack, 'juju_log', lambda *args, **kwargs: None)
-    def test_import_apt_key_radix(self):
-        '''Ensure shell out apt-key during key import'''
-        with patch('subprocess.check_call') as _subp:
-            openstack.import_key('foo')
-            cmd = ['apt-key', 'adv', '--keyserver',
-                   'hkp://keyserver.ubuntu.com:80', '--recv-keys', 'foo']
-            _subp.assert_called_with(cmd)
-
-    @patch.object(openstack, 'juju_log', lambda *args, **kwargs: None)
-    def test_import_apt_key_ascii_armor(self):
-        with tempfile.NamedTemporaryFile() as tmp:
-            with patch.object(openstack, 'tempfile') as \
-                    mock_tmpfile:
-                tmpfile = mock_tmpfile.NamedTemporaryFile.return_value
-                tmpfile.__enter__.return_value = tmpfile
-                tmpfile.name = tmp.name
-                with patch('subprocess.check_call') as _subp:
-                    openstack.import_key(PGP_KEY_ASCII_ARMOR)
-                    cmd = ['apt-key', 'add', tmp.name]
-                    _subp.assert_called_with(cmd)
-
-    @patch.object(openstack, 'juju_log', lambda *args, **kwargs: None)
-    @patch('charmhelpers.contrib.openstack.utils.error_out')
-    def test_import_bad_apt_key(self, mocked_error):
-        '''Ensure error when importing apt key fails'''
-        with patch('subprocess.check_call') as _subp:
-            cmd = ['apt-key', 'adv', '--keyserver',
-                   'hkp://keyserver.ubuntu.com:80', '--recv-keys', 'foo']
-            _subp.side_effect = subprocess.CalledProcessError(1, cmd, '')
-            openstack.import_key('foo')
-            cmd = ['apt-key', 'adv', '--keyserver',
-                   'hkp://keyserver.ubuntu.com:80', '--recv-keys', 'foo']
-        mocked_error.assert_called_with("Error importing PGP key 'foo'")
+        _e = ('Invalid Cloud Archive release specified: foo-bar'
+              ' on this Ubuntuversion')
+        _s = mocked_error.call_args[0][0]
+        self.assertTrue(_s.startswith(_e))
+
+    @patch.object(openstack, 'fetch_import_key')
+    def test_import_key_calls_fetch_import_key(self, fetch_import_key):
+        openstack.import_key('random-string')
+        fetch_import_key.assert_called_once_with('random-string')
+
+    @patch.object(openstack, 'fetch_import_key')
+    @patch.object(openstack, 'sys')
+    def test_import_key_calls_sys_exit_on_error(self, mock_sys,
+                                                fetch_import_key):
+
+        def raiser(_):
+            raise openstack.GPGKeyError("an error occurred")
+        fetch_import_key.side_effect = raiser
+        openstack.import_key('random failure')
+        mock_sys.exit.assert_called_once_with(1)
 
     @patch('os.mkdir')
     @patch('os.path.exists')
     @patch('charmhelpers.contrib.openstack.utils.charm_dir')
     @patch(builtin_open)
     def test_save_scriptrc(self, _open, _charm_dir, _exists, _mkdir):
-        '''Test generation of scriptrc from environment'''
+        """Test generation of scriptrc from environment"""
         scriptrc = ['#!/bin/bash\n',
                     'export setting1=foo\n',
                     'export setting2=bar\n']
@@ -618,14 +669,14 @@
     @patch.object(openstack, 'is_block_device')
     @patch.object(openstack, 'error_out')
     def test_ensure_block_device_bad_config(self, err, is_bd):
-        '''Test it doesn't prepare storage with bad config'''
+        """Test it doesn't prepare storage with bad config"""
         openstack.ensure_block_device(block_device='none')
         self.assertTrue(err.called)
 
     @patch.object(openstack, 'is_block_device')
     @patch.object(openstack, 'ensure_loopback_device')
     def test_ensure_block_device_loopback(self, ensure_loopback, is_bd):
-        '''Test it ensures loopback device when checking block device'''
+        """Test it ensures loopback device when checking block device"""
         defsize = openstack.DEFAULT_LOOPBACK_SIZE
         is_bd.return_value = True
 
@@ -641,7 +692,7 @@
 
     @patch.object(openstack, 'is_block_device')
     def test_ensure_standard_block_device(self, is_bd):
-        '''Test it looks for storage at both relative and full device path'''
+        """Test it looks for storage at both relative and full device path"""
         for dev in ['vdb', '/dev/vdb']:
             openstack.ensure_block_device(dev)
             is_bd.assert_called_with('/dev/vdb')
@@ -649,7 +700,7 @@
     @patch.object(openstack, 'is_block_device')
     @patch.object(openstack, 'error_out')
     def test_ensure_nonexistent_block_device(self, error_out, is_bd):
-        '''Test it will not ensure a non-existant block device'''
+        """Test it will not ensure a non-existant block device"""
         is_bd.return_value = False
         openstack.ensure_block_device(block_device='foo')
         self.assertTrue(error_out.called)
@@ -660,7 +711,7 @@
     @patch.object(openstack, 'zap_disk')
     @patch.object(openstack, 'is_lvm_physical_volume')
     def test_clean_storage_unmount(self, is_pv, zap_disk, mounts, umount, log):
-        '''Test it unmounts block device when cleaning storage'''
+        """Test it unmounts block device when cleaning storage"""
         is_pv.return_value = False
         zap_disk.return_value = True
         mounts.return_value = MOUNTS
@@ -673,7 +724,7 @@
     @patch.object(openstack, 'mounts')
     @patch.object(openstack, 'is_lvm_physical_volume')
     def test_clean_storage_lvm_wipe(self, is_pv, mounts, rm_lv, rm_vg, log):
-        '''Test it removes traces of LVM when cleaning storage'''
+        """Test it removes traces of LVM when cleaning storage"""
         mounts.return_value = []
         is_pv.return_value = True
         openstack.clean_storage('/dev/vdb')
@@ -684,7 +735,7 @@
     @patch.object(openstack, 'is_lvm_physical_volume')
     @patch.object(openstack, 'mounts')
     def test_clean_storage_zap_disk(self, mounts, is_pv, zap_disk):
-        '''It removes traces of LVM when cleaning storage'''
+        """It removes traces of LVM when cleaning storage"""
         mounts.return_value = []
         is_pv.return_value = False
         openstack.clean_storage('/dev/vdb')

=== modified file 'tests/contrib/openstack/test_os_utils.py'
--- tests/contrib/openstack/test_os_utils.py	2017-03-24 16:50:31 +0000
+++ tests/contrib/openstack/test_os_utils.py	2017-04-10 18:12:16 +0000
@@ -126,7 +126,7 @@
                         mock_git_os_codename_install_source,
                         mock_config):
         # Wipe the modules cached os_rel
-        utils.os_rel = None
+        utils._os_rel = None
         mock_get_os_codename_install_source.return_value = None
         mock_get_os_codename_package.return_value = None
         mock_git_os_codename_install_source.return_value = None

=== modified file 'tests/fetch/test_fetch.py'
--- tests/fetch/test_fetch.py	2017-03-03 20:18:47 +0000
+++ tests/fetch/test_fetch.py	2017-04-10 18:12:16 +0000
@@ -1,25 +1,22 @@
-import subprocess
 import six
 import os
 import yaml
-import imp
-import tempfile
 
-from charmhelpers import osplatform
-from tests.helpers import patch_open
 from testtools import TestCase
 from mock import (
     patch,
     MagicMock,
     call,
-    sentinel,
 )
+
 from charmhelpers import fetch
-from six.moves import StringIO
+
 if six.PY3:
     from urllib.parse import urlparse
+    builtin_open = 'builtins.open'
 else:
     from urlparse import urlparse
+    builtin_open = '__builtin__.open'
 
 
 FAKE_APT_CACHE = {
@@ -60,479 +57,7 @@
 
 class FetchTest(TestCase):
 
-    @patch("charmhelpers.fetch.ubuntu.log")
-    @patch.object(osplatform, 'get_platform')
-    @patch('apt_pkg.Cache')
-    def test_filter_packages_missing_ubuntu(self, cache, platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        cache.side_effect = fake_apt_cache
-        result = fetch.filter_installed_packages(['vim', 'emacs'])
-        self.assertEquals(result, ['emacs'])
-
-    @patch("charmhelpers.fetch.centos.log")
-    @patch('yum.YumBase.doPackageLists')
-    @patch.object(osplatform, 'get_platform')
-    def test_filter_packages_missing_centos(self, platform, yumBase, log):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        class MockPackage:
-            def __init__(self, name):
-                self.base_package_name = name
-
-        yum_dict = {
-            'installed': {
-                MockPackage('vim')
-            },
-            'available': {
-                MockPackage('vim')
-            }
-        }
-        import yum
-        yum.YumBase.return_value.doPackageLists.return_value = yum_dict
-        result = fetch.filter_installed_packages(['vim', 'emacs'])
-        self.assertEquals(result, ['emacs'])
-
-    @patch("charmhelpers.fetch.ubuntu.log")
-    @patch.object(osplatform, 'get_platform')
-    @patch('apt_pkg.Cache')
-    def test_filter_packages_none_missing_ubuntu(self, cache, platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        cache.side_effect = fake_apt_cache
-        result = fetch.filter_installed_packages(['vim'])
-        self.assertEquals(result, [])
-
-    @patch("charmhelpers.fetch.centos.log")
-    @patch.object(osplatform, 'get_platform')
-    def test_filter_packages_none_missing_centos(self, platform, log):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        class MockPackage:
-            def __init__(self, name):
-                self.base_package_name = name
-
-        yum_dict = {
-            'installed': {
-                MockPackage('vim')
-            },
-            'available': {
-                MockPackage('vim')
-            }
-        }
-        import yum
-        yum.yumBase.return_value.doPackageLists.return_value = yum_dict
-        result = fetch.filter_installed_packages(['vim'])
-        self.assertEquals(result, [])
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    @patch('apt_pkg.Cache')
-    def test_filter_packages_not_available_ubuntu(self, cache, log, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        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.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.centos.log')
-    @patch('yum.YumBase.doPackageLists')
-    def test_filter_packages_not_available_centos(self, yumBase,
-                                                  log, platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        class MockPackage:
-            def __init__(self, name):
-                self.base_package_name = name
-
-        yum_dict = {
-            'installed': {
-                MockPackage('vim')
-            }
-        }
-        import yum
-        yum.YumBase.return_value.doPackageLists.return_value = yum_dict
-
-        result = fetch.filter_installed_packages(['vim', 'joe'])
-        self.assertEquals(result, ['joe'])
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_add_source_none_ubuntu(self, log, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        fetch.add_source(source=None)
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_add_source_none_centos(self, log, platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        fetch.add_source(source=None)
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_ppa(self, check_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "ppa:test-ppa"
-        fetch.add_source(source=source)
-        check_call.assert_called_with(
-            ['add-apt-repository', '--yes', source], env=getenv())
-
-    @patch("charmhelpers.fetch.ubuntu.log")
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('time.sleep')
-    def test_add_source_ppa_retries_30_times(self, sleep, check_call,
-                                             platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        self.call_count = 0
-
-        def side_effect(*args, **kwargs):
-            """Raise an 3 times, then return 0 """
-            self.call_count += 1
-            if self.call_count <= fetch.ubuntu.CMD_RETRY_COUNT:
-                raise subprocess.CalledProcessError(
-                    returncode=1, cmd="some add-apt-repository command")
-            else:
-                return 0
-        check_call.side_effect = side_effect
-
-        source = "ppa:test-ppa"
-        fetch.add_source(source=source)
-        check_call.assert_called_with(
-            ['add-apt-repository', '--yes', source], env=getenv())
-        sleep.assert_called_with(10)
-        self.assertTrue(fetch.ubuntu.CMD_RETRY_COUNT, sleep.call_count)
-
-    @patch('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_http_ubuntu(self, check_call, platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
-        fetch.add_source(source=source)
-        check_call.assert_called_with(
-            ['add-apt-repository', '--yes', source], env=getenv())
-
-    @patch('charmhelpers.fetch.centos.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch('os.listdir')
-    def test_add_source_http_centos(self, listdir, platform, log):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
-        with patch_open() as (mock_open, mock_file):
-            fetch.add_source(source=source)
-            listdir.assert_called_with('/etc/yum.repos.d/')
-            mock_file.write.assert_has_calls([
-                call("[archive.ubuntu.com_ubuntu raring-backports main]\n"),
-                call("name=archive.ubuntu.com/ubuntu raring-backports main\n"),
-                call("baseurl=http://archive.ubuntu.com/ubuntu raring"
-                     "-backports main\n\n")])
-
-    @patch('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_https(self, check_call, platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "https://example.com"
-        fetch.add_source(source=source)
-        check_call.assert_called_with(
-            ['add-apt-repository', '--yes', source], env=getenv())
-
-    @patch('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_deb(self, check_call, platform, log):
-        """add-apt-repository behaves differently when using the deb prefix.
-
-        $ add-apt-repository --yes "http://special.example.com/ubuntu
-        precise-special main"
-        $ grep special /etc/apt/sources.list
-        deb http://special.example.com/ubuntu precise precise-special main
-        deb-src http://special.example.com/ubuntu precise precise-special main
-
-        $ add-apt-repository --yes "deb http://special.example.com/ubuntu
-        precise-special main"
-        $ grep special /etc/apt/sources.list
-        deb http://special.example.com/ubuntu precise precise-special main
-        deb-src http://special.example.com/ubuntu precise precise-special main
-        deb http://special.example.com/ubuntu precise-special main
-        deb-src http://special.example.com/ubuntu precise-special main
-        """
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "deb http://archive.ubuntu.com/ubuntu raring-backports main"
-        fetch.add_source(source=source)
-        check_call.assert_called_with(
-            ['add-apt-repository', '--yes', source], env=getenv())
-
-    @patch.object(osplatform, 'get_platform')
-    @patch.object(fetch.ubuntu, 'filter_installed_packages')
-    @patch.object(fetch.ubuntu, 'install')
-    def test_add_source_cloud_invalid_pocket(self, install,
-                                             filter_pkg, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "cloud:havana-updates"
-        self.assertRaises(fetch.ubuntu.SourceConfigError,
-                          fetch.add_source, source)
-        filter_pkg.assert_called_with(['ubuntu-cloud-keyring'])
-
-    @patch('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch.object(fetch.ubuntu, 'filter_installed_packages')
-    @patch.object(fetch.ubuntu, 'install')
-    def test_add_source_cloud_pocket_style(self, install, filter_pkg,
-                                           platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "cloud:precise-updates/havana"
-        result = ('# Ubuntu Cloud Archive\n'
-                  'deb http://ubuntu-cloud.archive.canonical.com/ubuntu'
-                  ' precise-updates/havana main\n')
-
-        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('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch.object(fetch.ubuntu, 'filter_installed_packages')
-    @patch.object(fetch.ubuntu, 'install')
-    def test_add_source_cloud_os_style(self, install, filter_pkg,
-                                       platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "cloud:precise-havana"
-        result = ('# Ubuntu Cloud Archive\n'
-                  'deb http://ubuntu-cloud.archive.canonical.com/ubuntu'
-                  ' precise-updates/havana main\n')
-        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('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch.object(fetch.ubuntu, 'filter_installed_packages')
-    @patch.object(fetch.ubuntu, 'install')
-    def test_add_source_cloud_distroless_style(self, install, filter_pkg,
-                                               platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "cloud:havana"
-        result = ('# Ubuntu Cloud Archive\n'
-                  'deb http://ubuntu-cloud.archive.canonical.com/ubuntu'
-                  ' precise-updates/havana main\n')
-        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('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch.object(fetch.ubuntu, 'lsb_release')
-    def test_add_source_proposed(self, lsb_release, platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "proposed"
-        result = ('# Proposed\n'
-                  'deb http://archive.ubuntu.com/ubuntu precise-proposed'
-                  ' main universe multiverse restricted\n')
-        lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
-        with patch_open() as (mock_open, mock_file):
-            fetch.add_source(source=source)
-            mock_file.write.assert_called_with(result)
-
-    @patch('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_http_and_key_id_ubuntu(self, check_call,
-                                               platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
-        key_id = "akey"
-        check_call.return_value = 0  # Successful exit code
-        fetch.add_source(source=source, key=key_id)
-        check_call.assert_has_calls([
-            call(['add-apt-repository', '--yes', source], env=getenv()),
-            call(['apt-key', 'adv', '--keyserver',
-                  'hkp://keyserver.ubuntu.com:80', '--recv', key_id])
-        ])
-
-    @patch('charmhelpers.fetch.centos.log')
-    @patch('os.listdir')
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_http_and_key_id_centos(self, check_call,
-                                               platform, listdir, log):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
-        key_id = "akey"
-        with patch_open() as (mock_open, mock_file):
-            fetch.add_source(source=source, key=key_id)
-            listdir.assert_called_with('/etc/yum.repos.d/')
-            mock_file.write.assert_has_calls([
-                call("[archive.ubuntu.com_ubuntu raring-backports main]\n"),
-                call("name=archive.ubuntu.com/ubuntu raring-backports main\n"),
-                call("baseurl=http://archive.ubuntu.com/ubuntu raring"
-                     "-backports main\n\n")])
-        check_call.assert_called_with(['rpm', '--import', key_id])
-
-    @patch('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_https_and_key_id_ubuntu(self, check_call,
-                                                platform, log):
-        check_call.return_value = 0  # Success from both calls
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "https://USER:[e-mail]/project/awesome"
-        key_id = "GPGPGP"
-        fetch.add_source(source=source, key=key_id)
-        check_call.assert_has_calls([
-            call(['add-apt-repository', '--yes', source], env=getenv()),
-            call(['apt-key', 'adv', '--keyserver',
-                  'hkp://keyserver.ubuntu.com:80', '--recv', key_id])
-        ])
-
-    @patch('charmhelpers.fetch.centos.log')
-    @patch('os.listdir')
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_https_and_key_id_centos(self, check_call,
-                                                platform, listdir, log):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        source = "https://USER:[e-mail]/project/awesome"
-        key_id = "GPGPGP"
-        with patch_open() as (mock_open, mock_file):
-            fetch.add_source(source=source, key=key_id)
-            listdir.assert_called_with('/etc/yum.repos.d/')
-            mock_file.write.assert_has_calls([
-                call("[_USER:[e-mail]"
-                     ".net_project_awesome]\n"),
-                call("name=/USER:[e-mail]"
-                     "/project/awesome\n"),
-                call("baseurl=https://USER:[e-mail]"
-                     "/project/awesome\n\n")])
-        check_call.assert_called_with(['rpm', '--import', key_id])
-
-    @patch('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_http_and_key_ubuntu(self, check_call, platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
-        key = '''
-            -----BEGIN PGP PUBLIC KEY BLOCK-----
-            [...]
-            -----END PGP PUBLIC KEY BLOCK-----
-            '''
-
-        received_args = []
-        received_key = StringIO()
-
-        def _check_call(arg, stdin=None, env=None):
-            '''side_effect to store the stdin passed to check_call process.'''
-            if stdin is not None:
-                received_args.extend(arg)
-                received_key.write(stdin.read())
-            return 0  # Successful return code checked by add-apt-repository
-
-        with patch('subprocess.check_call',
-                   side_effect=_check_call) as check_call:
-            fetch.add_source(source=source, key=key)
-            check_call.assert_any_call(
-                ['add-apt-repository', '--yes', source], env=getenv())
-            self.assertEqual(['apt-key', 'add', '-'], received_args)
-            self.assertEqual(key, received_key.getvalue())
-
-    @patch('charmhelpers.fetch.centos.log')
-    @patch.object(tempfile, 'NamedTemporaryFile')
-    @patch('os.listdir')
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_add_source_http_and_key_centos(self, check_call, platform,
-                                            listdir, temp_file, log):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
-        key = '''
-            -----BEGIN PGP PUBLIC KEY BLOCK-----
-            [...]
-            -----END PGP PUBLIC KEY BLOCK-----
-            '''
-        temp_file.return_value.__enter__.return_value = key
-        temp_file.return_value.__exit__.return_value = key
-
-        with patch_open() as (mock_open, mock_file):
-            fetch.add_source(source=source, key=key)
-            listdir.assert_called_with('/etc/yum.repos.d/')
-            self.assertTrue(log.called)
-        check_call.assert_called_with(['rpm', '--import', key])
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_add_unparsable_source(self, log_, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "propsed"  # Minor typo
-        fetch.add_source(source=source)
-        self.assertEqual(1, log_.call_count)
-
-    @patch('charmhelpers.fetch.ubuntu.log')
-    @patch.object(osplatform, 'get_platform')
-    def test_add_distro_source(self, platform, log):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        source = "distro"
-        # distro is a noop but test validate no exception is thrown
-        fetch.add_source(source=source)
-
-    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('charmhelpers.fetch.log')
     @patch.object(fetch, 'config')
     @patch.object(fetch, 'add_source')
     def test_configure_sources_single_source(self, add_source, config, log):
@@ -587,12 +112,11 @@
         ]
         self.assertRaises(fetch.SourceConfigError, fetch.configure_sources)
 
-    @patch.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.ubuntu.update')
+    @patch.object(fetch, '_fetch_update')
     @patch.object(fetch, 'config')
     @patch.object(fetch, 'add_source')
     def test_configure_sources_update_called_ubuntu(self, add_source, config,
-                                                    update, platform):
+                                                    update):
         config.side_effect = ['source', 'key']
         fetch.configure_sources(update=True)
         add_source.assert_called_with('source', 'key')
@@ -703,7 +227,8 @@
         self.assertEqual(2, log_.call_count)
 
     @patch('charmhelpers.fetch.log')
-    def test_plugins_are_valid(self, log_):
+    @patch.object(fetch.importlib, 'import_module')
+    def test_plugins_are_valid(self, import_module, log_):
         plugins = fetch.plugins()
         self.assertEqual(len(fetch.FETCH_HANDLERS), len(plugins))
 
@@ -742,572 +267,3 @@
         expected_url = "http://example.com/foo"
         u = self.fh.base_url(sample_url)
         self.assertEqual(u, expected_url)
-
-
-class AptTests(TestCase):
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_apt_upgrade_non_fatal(self, log, mock_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        options = ['--foo', '--bar']
-        fetch.apt_upgrade(options)
-
-        mock_call.assert_called_with(
-            ['apt-get', '--assume-yes',
-             '--foo', '--bar', 'upgrade'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_apt_upgrade_fatal(self, log, mock_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        options = ['--foo', '--bar']
-        fetch.apt_upgrade(options, fatal=True)
-
-        mock_call.assert_called_with(
-            ['apt-get', '--assume-yes',
-             '--foo', '--bar', 'upgrade'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_apt_dist_upgrade_fatal(self, log, mock_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        options = ['--foo', '--bar']
-        fetch.apt_upgrade(options, fatal=True, dist=True)
-
-        mock_call.assert_called_with(
-            ['apt-get', '--assume-yes',
-             '--foo', '--bar', 'dist-upgrade'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_installs_apt_packages(self, log, mock_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-        options = ['--foo', '--bar']
-
-        fetch.apt_install(packages, options)
-
-        mock_call.assert_called_with(
-            ['apt-get', '--assume-yes',
-             '--foo', '--bar', 'install', 'foo', 'bar'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_installs_apt_packages_without_options(self, log, mock_call,
-                                                   platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-
-        fetch.apt_install(packages)
-
-        mock_call.assert_called_with(
-            ['apt-get', '--assume-yes',
-             '--option=Dpkg::Options::=--force-confold',
-             'install', 'foo', 'bar'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_installs_apt_packages_as_string(self, log, mock_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = 'foo bar'
-        options = ['--foo', '--bar']
-
-        fetch.apt_install(packages, options)
-
-        mock_call.assert_called_with(
-            ['apt-get', '--assume-yes',
-             '--foo', '--bar', 'install', 'foo bar'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_installs_apt_packages_with_possible_errors(self, log,
-                                                        check_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-        options = ['--foo', '--bar']
-
-        fetch.apt_install(packages, options, fatal=True)
-
-        check_call.assert_called_with(
-            ['apt-get', '--assume-yes',
-             '--foo', '--bar', 'install', 'foo', 'bar'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_purges_apt_packages_as_string_fatal(self, log, mock_call,
-                                                 platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = 'irrelevant names'
-        mock_call.side_effect = OSError('fail')
-
-        self.assertRaises(OSError, fetch.apt_purge, packages, fatal=True)
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_purges_apt_packages_fatal(self, log, mock_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = ['irrelevant', 'names']
-        mock_call.side_effect = OSError('fail')
-
-        self.assertRaises(OSError, fetch.apt_purge, packages, fatal=True)
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_purges_apt_packages_as_string_nofatal(self, log, mock_call,
-                                                   platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = 'foo bar'
-
-        fetch.apt_purge(packages)
-
-        self.assertTrue(log.called)
-        mock_call.assert_called_with(
-            ['apt-get', '--assume-yes', 'purge', 'foo bar'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_purges_apt_packages_nofatal(self, log, mock_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-
-        fetch.apt_purge(packages)
-
-        self.assertTrue(log.called)
-        mock_call.assert_called_with(
-            ['apt-get', '--assume-yes', 'purge', 'foo', 'bar'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_mark_apt_packages_as_string_fatal(self, log, mock_call,
-                                               platform):
-        packages = 'irrelevant names'
-        mock_call.side_effect = OSError('fail')
-
-        self.assertRaises(OSError, fetch.apt_mark, packages, sentinel.mark,
-                          fatal=True)
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_mark_apt_packages_fatal(self, log, mock_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = ['irrelevant', 'names']
-        mock_call.side_effect = OSError('fail')
-
-        self.assertRaises(OSError, fetch.apt_mark, packages, sentinel.mark,
-                          fatal=True)
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_mark_apt_packages_as_string_nofatal(self, log, mock_call,
-                                                 platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = 'foo bar'
-
-        fetch.apt_mark(packages, sentinel.mark)
-
-        self.assertTrue(log.called)
-        mock_call.assert_called_with(
-            ['apt-mark', sentinel.mark, 'foo bar'],
-            universal_newlines=True)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_mark_apt_packages_nofatal(self, log, mock_call,
-                                       platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-
-        fetch.apt_mark(packages, sentinel.mark)
-
-        self.assertTrue(log.called)
-        mock_call.assert_called_with(
-            ['apt-mark', sentinel.mark, 'foo', 'bar'],
-            universal_newlines=True)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.ubuntu.log')
-    def test_mark_apt_packages_nofatal_abortonfatal(self, log, mock_call,
-                                                    platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-
-        fetch.apt_mark(packages, sentinel.mark, fatal=True)
-
-        self.assertTrue(log.called)
-        mock_call.assert_called_with(
-            ['apt-mark', sentinel.mark, 'foo', 'bar'],
-            universal_newlines=True)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.ubuntu.apt_mark')
-    def test_apt_hold(self, apt_mark, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        fetch.apt_hold(sentinel.packages)
-        apt_mark.assert_called_once_with(sentinel.packages, 'hold',
-                                         fatal=False)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.ubuntu.apt_mark')
-    def test_apt_hold_fatal(self, apt_mark, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        fetch.apt_hold(sentinel.packages, fatal=sentinel.fatal)
-        apt_mark.assert_called_once_with(sentinel.packages, 'hold',
-                                         fatal=sentinel.fatal)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.ubuntu.apt_mark')
-    def test_apt_unhold(self, apt_mark, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        fetch.apt_unhold(sentinel.packages)
-        apt_mark.assert_called_once_with(sentinel.packages, 'unhold',
-                                         fatal=False)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('charmhelpers.fetch.ubuntu.apt_mark')
-    def test_apt_unhold_fatal(self, apt_mark, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        fetch.apt_unhold(sentinel.packages, fatal=sentinel.fatal)
-        apt_mark.assert_called_once_with(sentinel.packages, 'unhold',
-                                         fatal=sentinel.fatal)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    def test_apt_update_fatal(self, check_call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        fetch.apt_update(fatal=True)
-        check_call.assert_called_with(
-            ['apt-get', 'update'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    def test_apt_update_nonfatal(self, call, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        fetch.apt_update()
-        call.assert_called_with(
-            ['apt-get', 'update'],
-            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('time.sleep')
-    def test_run_apt_command_retries_if_fatal(self, check_call,
-                                              sleep, platform):
-        """The _run_apt_command function retries the command if it can't get
-        the APT lock."""
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-
-        self.called = False
-
-        def side_effect(*args, **kwargs):
-            """
-            First, raise an exception (can't acquire lock), then return 0
-            (the lock is grabbed).
-            """
-            if not self.called:
-                self.called = True
-                raise subprocess.CalledProcessError(
-                    returncode=100, cmd="some command")
-            else:
-                return 0
-
-        check_call.side_effect = side_effect
-        check_call.return_value = 0
-
-        from charmhelpers.fetch.ubuntu import _run_apt_command
-        _run_apt_command(["some", "command"], fatal=True)
-        self.assertTrue(sleep.called)
-
-
-class YumTests(TestCase):
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_yum_upgrade_non_fatal(self, log, mock_call, platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        options = ['--foo', '--bar']
-        fetch.upgrade(options)
-
-        mock_call.assert_called_with(['yum', '--assumeyes',
-                                      '--foo', '--bar', 'upgrade'],
-                                     env=getenv())
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_yum_upgrade_fatal(self, log, mock_call, platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        options = ['--foo', '--bar']
-        fetch.upgrade(options, fatal=True)
-
-        mock_call.assert_called_with(['yum', '--assumeyes',
-                                      '--foo', '--bar', 'upgrade'],
-                                     env=getenv())
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_installs_yum_packages(self, log, mock_call, platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-        options = ['--foo', '--bar']
-
-        fetch.install(packages, options)
-
-        mock_call.assert_called_with(['yum', '--assumeyes',
-                                      '--foo', '--bar', 'install',
-                                      'foo', 'bar'],
-                                     env=getenv())
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_installs_yum_packages_without_options(self, log, mock_call,
-                                                   platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-        fetch.install(packages)
-
-        mock_call.assert_called_with(['yum', '--assumeyes',
-                                      'install', 'foo', 'bar'],
-                                     env=getenv())
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_installs_yum_packages_as_string(self, log, mock_call,
-                                             platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        packages = 'foo bar'
-        fetch.install(packages)
-
-        mock_call.assert_called_with(['yum', '--assumeyes',
-                                      'install', 'foo bar'],
-                                     env=getenv())
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_installs_yum_packages_with_possible_errors(self, log, mock_call,
-                                                        platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-        options = ['--foo', '--bar']
-
-        fetch.install(packages, options, fatal=True)
-
-        mock_call.assert_called_with(['yum', '--assumeyes',
-                                      '--foo', '--bar',
-                                      'install', 'foo', 'bar'],
-                                     env=getenv())
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_purges_yum_packages_as_string_fatal(self, log, mock_call,
-                                                 platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        packages = 'irrelevant names'
-        mock_call.side_effect = OSError('fail')
-
-        self.assertRaises(OSError, fetch.purge, packages, fatal=True)
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_purges_yum_packages_fatal(self, log, mock_call, platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        packages = ['irrelevant', 'names']
-        mock_call.side_effect = OSError('fail')
-
-        self.assertRaises(OSError, fetch.purge, packages, fatal=True)
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_purges_yum_packages_as_string_nofatal(self, log, mock_call,
-                                                   platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        packages = 'foo bar'
-        fetch.purge(packages)
-
-        self.assertTrue(log.called)
-        mock_call.assert_called_with(['yum', '--assumeyes',
-                                      'remove', 'foo bar'],
-                                     env=getenv())
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_purges_yum_packages_nofatal(self, log, mock_call,
-                                         platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        packages = ['foo', 'bar']
-        fetch.purge(packages)
-
-        self.assertTrue(log.called)
-        mock_call.assert_called_with(['yum', '--assumeyes',
-                                      'remove', 'foo', 'bar'],
-                                     env=getenv())
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_yum_update_fatal(self, log, check_call, platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        fetch.update(fatal=True)
-        check_call.assert_called_with(['yum', '--assumeyes', 'update'],
-                                      env=getenv())
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_output')
-    @patch('charmhelpers.fetch.centos.log')
-    def test_yum_search(self, log, check_output, platform):
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        package = ['irrelevant']
-
-        from charmhelpers.fetch.centos import yum_search
-        yum_search(package)
-        check_output.assert_called_with(['yum', 'search', 'irrelevant'])
-        self.assertTrue(log.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('subprocess.check_call')
-    @patch('time.sleep')
-    def test_run_yum_command_retries_if_fatal(self, check_call,
-                                              sleep, platform):
-        """The _run_yum_command function retries the command if it can't get
-        the YUM lock."""
-        platform.return_value = 'centos'
-        imp.reload(fetch)
-
-        self.called = False
-
-        def side_effect(*args, **kwargs):
-            """
-            First, raise an exception (can't acquire lock), then return 0
-            (the lock is grabbed).
-            """
-            if not self.called:
-                self.called = True
-                raise subprocess.CalledProcessError(
-                    returncode=1, cmd="some command")
-            else:
-                return 0
-
-        check_call.side_effect = side_effect
-        check_call.return_value = 0
-        from charmhelpers.fetch.centos import _run_yum_command
-        _run_yum_command(["some", "command"], fatal=True)
-        self.assertTrue(sleep.called)
-
-    @patch.object(osplatform, 'get_platform')
-    @patch('apt_pkg.Cache')
-    def test_get_upstream_version(self, cache, platform):
-        platform.return_value = 'ubuntu'
-        imp.reload(fetch)
-        cache.side_effect = fake_apt_cache
-        self.assertEqual(fetch.get_upstream_version('vim'), '7.3.547')
-        self.assertEqual(fetch.get_upstream_version('emacs'), None)
-        self.assertEqual(fetch.get_upstream_version('unknown'), None)

=== added file 'tests/fetch/test_fetch_centos.py'
--- tests/fetch/test_fetch_centos.py	1970-01-01 00:00:00 +0000
+++ tests/fetch/test_fetch_centos.py	2017-04-10 18:12:16 +0000
@@ -0,0 +1,315 @@
+import subprocess
+import os
+
+from tests.helpers import patch_open
+from testtools import TestCase
+from mock import (
+    patch,
+    MagicMock,
+    call,
+)
+from charmhelpers.fetch import centos as fetch
+
+
+def getenv(update=None):
+    # return a copy of os.environ with update applied.
+    # this was necessary because some modules modify os.environment directly
+    copy = os.environ.copy()
+    if update is not None:
+        copy.update(update)
+    return copy
+
+
+class FetchTest(TestCase):
+
+    @patch("charmhelpers.fetch.log")
+    @patch('yum.YumBase.doPackageLists')
+    def test_filter_packages_missing_centos(self, yumBase, log):
+
+        class MockPackage:
+            def __init__(self, name):
+                self.base_package_name = name
+
+        yum_dict = {
+            'installed': {
+                MockPackage('vim')
+            },
+            'available': {
+                MockPackage('vim')
+            }
+        }
+        import yum
+        yum.YumBase.return_value.doPackageLists.return_value = yum_dict
+        result = fetch.filter_installed_packages(['vim', 'emacs'])
+        self.assertEquals(result, ['emacs'])
+
+    @patch("charmhelpers.fetch.log")
+    def test_filter_packages_none_missing_centos(self, log):
+
+        class MockPackage:
+            def __init__(self, name):
+                self.base_package_name = name
+
+        yum_dict = {
+            'installed': {
+                MockPackage('vim')
+            },
+            'available': {
+                MockPackage('vim')
+            }
+        }
+        import yum
+        yum.yumBase.return_value.doPackageLists.return_value = yum_dict
+        result = fetch.filter_installed_packages(['vim'])
+        self.assertEquals(result, [])
+
+    @patch('charmhelpers.fetch.centos.log')
+    @patch('yum.YumBase.doPackageLists')
+    def test_filter_packages_not_available_centos(self, yumBase, log):
+
+        class MockPackage:
+            def __init__(self, name):
+                self.base_package_name = name
+
+        yum_dict = {
+            'installed': {
+                MockPackage('vim')
+            }
+        }
+        import yum
+        yum.YumBase.return_value.doPackageLists.return_value = yum_dict
+
+        result = fetch.filter_installed_packages(['vim', 'joe'])
+        self.assertEquals(result, ['joe'])
+
+    @patch('charmhelpers.fetch.centos.log')
+    def test_add_source_none_centos(self, log):
+        fetch.add_source(source=None)
+        self.assertTrue(log.called)
+
+    @patch('charmhelpers.fetch.centos.log')
+    @patch('os.listdir')
+    def test_add_source_http_centos(self, listdir, log):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        with patch_open() as (mock_open, mock_file):
+            fetch.add_source(source=source)
+            listdir.assert_called_with('/etc/yum.repos.d/')
+            mock_file.write.assert_has_calls([
+                call("[archive.ubuntu.com_ubuntu raring-backports main]\n"),
+                call("name=archive.ubuntu.com/ubuntu raring-backports main\n"),
+                call("baseurl=http://archive.ubuntu.com/ubuntu raring"
+                     "-backports main\n\n")])
+
+    @patch('charmhelpers.fetch.centos.log')
+    @patch('os.listdir')
+    @patch('subprocess.check_call')
+    def test_add_source_http_and_key_id_centos(self, check_call,
+                                               listdir, log):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        key_id = "akey"
+        with patch_open() as (mock_open, mock_file):
+            fetch.add_source(source=source, key=key_id)
+            listdir.assert_called_with('/etc/yum.repos.d/')
+            mock_file.write.assert_has_calls([
+                call("[archive.ubuntu.com_ubuntu raring-backports main]\n"),
+                call("name=archive.ubuntu.com/ubuntu raring-backports main\n"),
+                call("baseurl=http://archive.ubuntu.com/ubuntu raring"
+                     "-backports main\n\n")])
+        check_call.assert_called_with(['rpm', '--import', key_id])
+
+    @patch('charmhelpers.fetch.centos.log')
+    @patch('os.listdir')
+    @patch('subprocess.check_call')
+    def test_add_source_https_and_key_id_centos(self, check_call,
+                                                listdir, log):
+        source = "https://USER:[e-mail]/project/awesome"
+        key_id = "GPGPGP"
+        with patch_open() as (mock_open, mock_file):
+            fetch.add_source(source=source, key=key_id)
+            listdir.assert_called_with('/etc/yum.repos.d/')
+            mock_file.write.assert_has_calls([
+                call("[_USER:[e-mail]"
+                     ".net_project_awesome]\n"),
+                call("name=/USER:[e-mail]"
+                     "/project/awesome\n"),
+                call("baseurl=https://USER:[e-mail]"
+                     "/project/awesome\n\n")])
+        check_call.assert_called_with(['rpm', '--import', key_id])
+
+    @patch('charmhelpers.fetch.centos.log')
+    @patch.object(fetch, 'NamedTemporaryFile')
+    @patch('os.listdir')
+    @patch('subprocess.check_call')
+    def test_add_source_http_and_key_centos(self, check_call,
+                                            listdir, temp_file, log):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        key = '''
+            -----BEGIN PGP PUBLIC KEY BLOCK-----
+            [...]
+            -----END PGP PUBLIC KEY BLOCK-----
+            '''
+        file_mock = MagicMock()
+        file_mock.name = 'temporary_file'
+        temp_file.return_value.__enter__.return_value = file_mock
+        listdir.return_value = []
+
+        with patch_open() as (mock_open, mock_file):
+            fetch.add_source(source=source, key=key)
+            listdir.assert_called_with('/etc/yum.repos.d/')
+            self.assertTrue(log.called)
+        check_call.assert_called_with(['rpm', '--import', file_mock.name])
+        file_mock.write.assert_called_once_with(key)
+        file_mock.flush.assert_called_once_with()
+        file_mock.seek.assert_called_once_with(0)
+
+
+class YumTests(TestCase):
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_yum_upgrade_non_fatal(self, log, mock_call):
+        options = ['--foo', '--bar']
+        fetch.upgrade(options)
+
+        mock_call.assert_called_with(['yum', '--assumeyes',
+                                      '--foo', '--bar', 'upgrade'],
+                                     env=getenv())
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_yum_upgrade_fatal(self, log, mock_call):
+        options = ['--foo', '--bar']
+        fetch.upgrade(options, fatal=True)
+
+        mock_call.assert_called_with(['yum', '--assumeyes',
+                                      '--foo', '--bar', 'upgrade'],
+                                     env=getenv())
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_installs_yum_packages(self, log, mock_call):
+        packages = ['foo', 'bar']
+        options = ['--foo', '--bar']
+
+        fetch.install(packages, options)
+
+        mock_call.assert_called_with(['yum', '--assumeyes',
+                                      '--foo', '--bar', 'install',
+                                      'foo', 'bar'],
+                                     env=getenv())
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_installs_yum_packages_without_options(self, log, mock_call):
+        packages = ['foo', 'bar']
+        fetch.install(packages)
+
+        mock_call.assert_called_with(['yum', '--assumeyes',
+                                      'install', 'foo', 'bar'],
+                                     env=getenv())
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_installs_yum_packages_as_string(self, log, mock_call):
+        packages = 'foo bar'
+        fetch.install(packages)
+
+        mock_call.assert_called_with(['yum', '--assumeyes',
+                                      'install', 'foo bar'],
+                                     env=getenv())
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_installs_yum_packages_with_possible_errors(self, log, mock_call):
+        packages = ['foo', 'bar']
+        options = ['--foo', '--bar']
+
+        fetch.install(packages, options, fatal=True)
+
+        mock_call.assert_called_with(['yum', '--assumeyes',
+                                      '--foo', '--bar',
+                                      'install', 'foo', 'bar'],
+                                     env=getenv())
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_purges_yum_packages_as_string_fatal(self, log, mock_call):
+        packages = 'irrelevant names'
+        mock_call.side_effect = OSError('fail')
+
+        self.assertRaises(OSError, fetch.purge, packages, fatal=True)
+        self.assertTrue(log.called)
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_purges_yum_packages_fatal(self, log, mock_call):
+        packages = ['irrelevant', 'names']
+        mock_call.side_effect = OSError('fail')
+
+        self.assertRaises(OSError, fetch.purge, packages, fatal=True)
+        self.assertTrue(log.called)
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_purges_yum_packages_as_string_nofatal(self, log, mock_call):
+        packages = 'foo bar'
+        fetch.purge(packages)
+
+        self.assertTrue(log.called)
+        mock_call.assert_called_with(['yum', '--assumeyes',
+                                      'remove', 'foo bar'],
+                                     env=getenv())
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_purges_yum_packages_nofatal(self, log, mock_call):
+        packages = ['foo', 'bar']
+        fetch.purge(packages)
+
+        self.assertTrue(log.called)
+        mock_call.assert_called_with(['yum', '--assumeyes',
+                                      'remove', 'foo', 'bar'],
+                                     env=getenv())
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_yum_update_fatal(self, log, check_call):
+        fetch.update(fatal=True)
+        check_call.assert_called_with(['yum', '--assumeyes', 'update'],
+                                      env=getenv())
+        self.assertTrue(log.called)
+
+    @patch('subprocess.check_output')
+    @patch('charmhelpers.fetch.centos.log')
+    def test_yum_search(self, log, check_output):
+        package = ['irrelevant']
+
+        from charmhelpers.fetch.centos import yum_search
+        yum_search(package)
+        check_output.assert_called_with(['yum', 'search', 'irrelevant'])
+        self.assertTrue(log.called)
+
+    @patch('subprocess.check_call')
+    @patch('time.sleep')
+    def test_run_yum_command_retries_if_fatal(self, check_call, sleep):
+        """The _run_yum_command function retries the command if it can't get
+        the YUM lock."""
+        self.called = False
+
+        def side_effect(*args, **kwargs):
+            """
+            First, raise an exception (can't acquire lock), then return 0
+            (the lock is grabbed).
+            """
+            if not self.called:
+                self.called = True
+                raise subprocess.CalledProcessError(
+                    returncode=1, cmd="some command")
+            else:
+                return 0
+
+        check_call.side_effect = side_effect
+        check_call.return_value = 0
+        from charmhelpers.fetch.centos import _run_yum_command
+        _run_yum_command(["some", "command"], fatal=True)
+        self.assertTrue(sleep.called)

=== added file 'tests/fetch/test_fetch_ubuntu.py'
--- tests/fetch/test_fetch_ubuntu.py	1970-01-01 00:00:00 +0000
+++ tests/fetch/test_fetch_ubuntu.py	2017-04-10 18:12:16 +0000
@@ -0,0 +1,712 @@
+import six
+import subprocess
+import io
+import os
+import tempfile
+
+from tests.helpers import patch_open
+from testtools import TestCase
+from mock import (
+    patch,
+    MagicMock,
+    call,
+    sentinel,
+    ANY,
+)
+from charmhelpers.fetch import ubuntu as fetch
+
+if six.PY3:
+    builtin_open = 'builtins.open'
+else:
+    builtin_open = '__builtin__.open'
+
+# mocked return of openstack.lsb_release()
+FAKE_RELEASE = {
+    'DISTRIB_CODENAME': 'precise',
+    'DISTRIB_RELEASE': '12.04',
+    'DISTRIB_ID': 'Ubuntu',
+    'DISTRIB_DESCRIPTION': '"Ubuntu 12.04"'
+}
+
+url = 'deb ' + fetch.CLOUD_ARCHIVE_URL
+UCA_SOURCES = [
+    ('cloud:precise-folsom/proposed', url + ' precise-proposed/folsom main'),
+    ('cloud:precise-folsom', url + ' precise-updates/folsom main'),
+    ('cloud:precise-folsom/updates', url + ' precise-updates/folsom main'),
+    ('cloud:precise-grizzly/proposed', url + ' precise-proposed/grizzly main'),
+    ('cloud:precise-grizzly', url + ' precise-updates/grizzly main'),
+    ('cloud:precise-grizzly/updates', url + ' precise-updates/grizzly main'),
+    ('cloud:precise-havana/proposed', url + ' precise-proposed/havana main'),
+    ('cloud:precise-havana', url + ' precise-updates/havana main'),
+    ('cloud:precise-havana/updates', url + ' precise-updates/havana main'),
+    ('cloud:precise-icehouse/proposed',
+     url + ' precise-proposed/icehouse main'),
+    ('cloud:precise-icehouse', url + ' precise-updates/icehouse main'),
+    ('cloud:precise-icehouse/updates', url + ' precise-updates/icehouse main'),
+]
+
+PGP_KEY_ASCII_ARMOR = """-----BEGIN PGP PUBLIC KEY BLOCK-----
+Version: SKS 1.1.5
+Comment: Hostname: keyserver.ubuntu.com
+
+mI0EUCEyTAEEAMuUxyfiegCCwn4J/c0nw5PUTSJdn5FqiUTq6iMfij65xf1vl0g/Mxqw0gfg
+AJIsCDvO9N9dloLAwF6FUBMg5My7WyhRPTAKF505TKJboyX3Pp4J1fU1LV8QFVOp87vUh1Rz
+B6GU7cSglhnbL85gmbJTllkzkb3h4Yw7W+edjcQ/ABEBAAG0K0xhdW5jaHBhZCBQUEEgZm9y
+IFVidW50dSBDbG91ZCBBcmNoaXZlIFRlYW2IuAQTAQIAIgUCUCEyTAIbAwYLCQgHAwIGFQgC
+CQoLBBYCAwECHgECF4AACgkQimhEop9oEE7kJAP/eTBgq3Mhbvo0d8elMOuqZx3nmU7gSyPh
+ep0zYIRZ5TJWl/7PRtvp0CJA6N6ZywYTQ/4ANHhpibcHZkh8K0AzUvsGXnJRSFoJeqyDbD91
+EhoO+4ZfHs2HvRBQEDZILMa2OyuB497E5Mmyua3HDEOrG2cVLllsUZzpTFCx8NgeMHk=
+=jLBm
+-----END PGP PUBLIC KEY BLOCK-----
+"""
+
+FAKE_APT_CACHE = {
+    # an installed package
+    'vim': {
+        'current_ver': '2:7.3.547-6ubuntu5'
+    },
+    # a uninstalled installation candidate
+    'emacs': {
+    }
+}
+
+
+def fake_apt_cache(in_memory=True, progress=None):
+    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.ver_str = FAKE_APT_CACHE[package]['current_ver']
+        else:
+            pkg.current_ver = None
+        return pkg
+    cache = MagicMock()
+    cache.__getitem__.side_effect = _get
+    return cache
+
+
+def getenv(update=None):
+    # return a copy of os.environ with update applied.
+    # this was necessary because some modules modify os.environment directly
+    copy = os.environ.copy()
+    if update is not None:
+        copy.update(update)
+    return copy
+
+
+class FetchTest(TestCase):
+
+    @patch("charmhelpers.fetch.ubuntu.log")
+    @patch('apt_pkg.Cache')
+    def test_filter_packages_missing_ubuntu(self, cache, log):
+        cache.side_effect = fake_apt_cache
+        result = fetch.filter_installed_packages(['vim', 'emacs'])
+        self.assertEquals(result, ['emacs'])
+
+    @patch("charmhelpers.fetch.ubuntu.log")
+    @patch('apt_pkg.Cache')
+    def test_filter_packages_none_missing_ubuntu(self, cache, log):
+        cache.side_effect = fake_apt_cache
+        result = fetch.filter_installed_packages(['vim'])
+        self.assertEquals(result, [])
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('apt_pkg.Cache')
+    def test_filter_packages_not_available_ubuntu(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.object(fetch, 'log', lambda *args, **kwargs: None)
+    def test_import_apt_key_radix(self):
+        """Ensure shell out apt-key during key import"""
+        with patch('subprocess.check_call') as _subp:
+            fetch.import_key('foo')
+            cmd = ['apt-key', 'adv', '--keyserver',
+                   'hkp://keyserver.ubuntu.com:80', '--recv-keys', 'foo']
+            _subp.assert_called_with(cmd)
+
+    @patch.object(fetch, 'log', lambda *args, **kwargs: None)
+    def test_import_apt_key_ascii_armor(self):
+        with tempfile.NamedTemporaryFile() as tmp:
+            with patch.object(fetch, 'NamedTemporaryFile') as mock_tmpfile:
+                tmpfile = mock_tmpfile.return_value
+                tmpfile.__enter__.return_value = tmpfile
+                tmpfile.name = tmp.name
+                with patch('subprocess.check_call') as _subp:
+                    fetch.import_key(PGP_KEY_ASCII_ARMOR)
+                    cmd = ['apt-key', 'add', tmp.name]
+                    _subp.assert_called_with(cmd)
+                with open(tmp.name, 'r') as f:
+                    self.assertEqual(PGP_KEY_ASCII_ARMOR, f.read())
+
+    @patch.object(fetch, 'log', lambda *args, **kwargs: None)
+    def test_import_bad_apt_key(self):
+        """Ensure error when importing apt key fails"""
+        with patch('subprocess.check_call') as _subp:
+            cmd = ['apt-key', 'adv', '--keyserver',
+                   'hkp://keyserver.ubuntu.com:80', '--recv-keys', 'foo']
+            _subp.side_effect = subprocess.CalledProcessError(1, cmd, '')
+            try:
+                fetch.import_key('foo')
+                assert False
+            except fetch.GPGKeyError as e:
+                self.assertEqual(str(e), "Error importing PGP key 'foo'")
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_add_source_none_ubuntu(self, log):
+        fetch.add_source(source=None)
+        self.assertTrue(log.called)
+
+    @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', '--yes', source])
+
+    @patch("charmhelpers.fetch.ubuntu.log")
+    @patch('subprocess.check_call')
+    @patch('time.sleep')
+    def test_add_source_ppa_retries_30_times(self, sleep, check_call, log):
+        self.call_count = 0
+
+        def side_effect(*args, **kwargs):
+            """Raise an 3 times, then return 0 """
+            self.call_count += 1
+            if self.call_count <= fetch.CMD_RETRY_COUNT:
+                raise subprocess.CalledProcessError(
+                    returncode=1, cmd="some add-apt-repository command")
+            else:
+                return 0
+        check_call.side_effect = side_effect
+
+        source = "ppa:test-ppa"
+        fetch.add_source(source=source)
+        check_call.assert_called_with(
+            ['add-apt-repository', '--yes', source])
+        sleep.assert_called_with(10)
+        self.assertTrue(fetch.CMD_RETRY_COUNT, sleep.call_count)
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('subprocess.check_call')
+    def test_add_source_http_ubuntu(self, check_call, log):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        fetch.add_source(source=source)
+        check_call.assert_called_with(
+            ['add-apt-repository', '--yes', source])
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('subprocess.check_call')
+    def test_add_source_https(self, check_call, log):
+        source = "https://example.com"
+        fetch.add_source(source=source)
+        check_call.assert_called_with(
+            ['add-apt-repository', '--yes', source])
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('subprocess.check_call')
+    def test_add_source_deb(self, check_call, log):
+        """add-apt-repository behaves differently when using the deb prefix.
+
+        $ add-apt-repository --yes \
+            "http://special.example.com/ubuntu precise-special main"
+        $ grep special /etc/apt/sources.list
+        deb http://special.example.com/ubuntu precise precise-special main
+        deb-src http://special.example.com/ubuntu precise precise-special main
+
+        $ add-apt-repository --yes \
+            "deb http://special.example.com/ubuntu precise-special main"
+        $ grep special /etc/apt/sources.list
+        deb http://special.example.com/ubuntu precise precise-special main
+        deb-src http://special.example.com/ubuntu precise precise-special main
+        deb http://special.example.com/ubuntu precise-special main
+        deb-src http://special.example.com/ubuntu precise-special main
+        """
+        source = "deb http://archive.ubuntu.com/ubuntu raring-backports main"
+        fetch.add_source(source=source)
+        check_call.assert_called_with(
+            ['add-apt-repository', '--yes', source])
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('subprocess.check_call')
+    def test_add_source_http_and_key_id(self, check_call, log):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        key_id = "akey"
+        check_call.return_value = 0  # Successful exit code
+        fetch.add_source(source=source, key=key_id)
+        check_call.assert_has_calls([
+            call(['add-apt-repository', '--yes', source]),
+            call(['apt-key', 'adv', '--keyserver',
+                  'hkp://keyserver.ubuntu.com:80', '--recv-keys', key_id])
+        ])
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('subprocess.check_call')
+    def test_add_source_https_and_key_id(self, check_call, log):
+        source = "https://USER:[e-mail]/project/awesome"
+        key_id = "GPGPGP"
+        check_call.return_value = 0  # Success from both calls
+        fetch.add_source(source=source, key=key_id)
+        check_call.assert_has_calls([
+            call(['add-apt-repository', '--yes', source]),
+            call(['apt-key', 'adv', '--keyserver',
+                  'hkp://keyserver.ubuntu.com:80', '--recv-keys', key_id])
+        ])
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('subprocess.check_call')
+    def test_add_source_http_and_key(self, check_call, log):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        key = '''
+            -----BEGIN PGP PUBLIC KEY BLOCK-----
+            [...]
+            -----END PGP PUBLIC KEY BLOCK-----
+            '''
+        with patch('subprocess.check_call') as check_call:
+            check_call.return_value = 0
+            fetch.add_source(source=source, key=key)
+            check_call.assert_any_call(['add-apt-repository', '--yes', source])
+            check_call.assert_any_call(['apt-key', 'add', ANY])
+
+    def test_add_source_cloud_invalid_pocket(self):
+        source = "cloud:havana-updates"
+        self.assertRaises(fetch.SourceConfigError,
+                          fetch.add_source, source)
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch.object(fetch, 'filter_installed_packages')
+    @patch.object(fetch, 'apt_install')
+    @patch.object(fetch, 'lsb_release')
+    def test_add_source_cloud_pocket_style(self, lsb_release, apt_install,
+                                           filter_pkg, log):
+        source = "cloud:precise-updates/havana"
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
+        result = ('# Ubuntu Cloud Archive\n'
+                  'deb http://ubuntu-cloud.archive.canonical.com/ubuntu'
+                  ' precise-updates/havana main\n')
+
+        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('charmhelpers.fetch.ubuntu.log')
+    @patch.object(fetch, 'filter_installed_packages')
+    @patch.object(fetch, 'apt_install')
+    @patch.object(fetch, 'lsb_release')
+    def test_add_source_cloud_os_style(self, lsb_release, apt_install,
+                                       filter_pkg, log):
+        source = "cloud:precise-havana"
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
+        result = ('# Ubuntu Cloud Archive\n'
+                  'deb http://ubuntu-cloud.archive.canonical.com/ubuntu'
+                  ' precise-updates/havana main\n')
+        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('charmhelpers.fetch.ubuntu.log')
+    @patch.object(fetch, 'filter_installed_packages')
+    @patch.object(fetch, 'apt_install')
+    def test_add_source_cloud_distroless_style(self, apt_install,
+                                               filter_pkg, log):
+        source = "cloud:havana"
+        result = ('# Ubuntu Cloud Archive\n'
+                  'deb http://ubuntu-cloud.archive.canonical.com/ubuntu'
+                  ' precise-updates/havana main\n')
+        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('charmhelpers.fetch.ubuntu.log')
+    @patch.object(fetch, 'lsb_release')
+    @patch('platform.machine')
+    def test_add_source_proposed_x86_64(self, _machine, lsb_release, log):
+        source = "proposed"
+        result = ('# Proposed\n'
+                  'deb http://archive.ubuntu.com/ubuntu precise-proposed'
+                  ' main universe multiverse restricted\n')
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
+        _machine.return_value = 'x86_64'
+        with patch_open() as (mock_open, mock_file):
+            fetch.add_source(source=source)
+            mock_file.write.assert_called_with(result)
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch.object(fetch, 'lsb_release')
+    @patch('platform.machine')
+    def test_add_source_proposed_ppc64le(self, _machine, lsb_release, log):
+        source = "proposed"
+        result = (
+            "# Proposed\n"
+            "deb http://ports.ubuntu.com/ubuntu-ports precise-proposed main "
+            "universe multiverse restricted\n")
+        lsb_release.return_value = {'DISTRIB_CODENAME': 'precise'}
+        _machine.return_value = 'ppc64le'
+        with patch_open() as (mock_open, mock_file):
+            fetch.add_source(source=source)
+            mock_file.write.assert_called_with(result)
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('subprocess.check_call')
+    def test_add_source_http_and_key_id_ubuntu(self, check_call, log):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        key_id = "akey"
+        fetch.add_source(source=source, key=key_id)
+        check_call.assert_any_call(['add-apt-repository', '--yes', source]),
+        check_call.assert_any_call([
+            'apt-key', 'adv', '--keyserver',
+            'hkp://keyserver.ubuntu.com:80', '--recv-keys', key_id])
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('subprocess.check_call')
+    def test_add_source_https_and_key_id_ubuntu(self, check_call, log):
+        source = "https://USER:[e-mail]/project/awesome"
+        key_id = "GPGPGP"
+        fetch.add_source(source=source, key=key_id)
+        check_call.assert_any_call(['add-apt-repository', '--yes', source]),
+        check_call.assert_any_call([
+            'apt-key', 'adv', '--keyserver',
+            'hkp://keyserver.ubuntu.com:80', '--recv-keys', key_id])
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    @patch('subprocess.check_call')
+    def test_add_source_http_and_key_ubuntu(self, check_call, log):
+        source = "http://archive.ubuntu.com/ubuntu raring-backports main"
+        key = '''
+            -----BEGIN PGP PUBLIC KEY BLOCK-----
+            [...]
+            -----END PGP PUBLIC KEY BLOCK-----
+            '''
+        fetch.add_source(source=source, key=key)
+        check_call.assert_any_call(['add-apt-repository', '--yes', source])
+        check_call.assert_any_call(['apt-key', 'add', ANY])
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_configure_bad_install_source(self, log):
+        try:
+            fetch.add_source('foo', fail_invalid=True)
+            self.fail("Calling add_source('foo') should fail")
+        except fetch.SourceConfigError as e:
+            self.assertEqual(str(e), "Unknown source: 'foo'")
+
+    @patch('charmhelpers.fetch.ubuntu.lsb_release')
+    def test_configure_install_source_uca_staging(self, _lsb):
+        """Test configuring installation source from UCA staging sources"""
+        _lsb.return_value = FAKE_RELEASE
+        # staging pockets are configured as PPAs
+        with patch('subprocess.check_call') as _subp:
+            src = 'cloud:precise-folsom/staging'
+            fetch.add_source(src)
+            cmd = ['add-apt-repository', '-y',
+                   'ppa:ubuntu-cloud-archive/folsom-staging']
+            _subp.assert_called_with(cmd)
+
+    @patch(builtin_open)
+    @patch('charmhelpers.fetch.ubuntu.apt_install')
+    @patch('charmhelpers.fetch.ubuntu.lsb_release')
+    @patch('charmhelpers.fetch.ubuntu.filter_installed_packages')
+    def test_configure_install_source_uca_repos(
+            self, _fip, _lsb, _install, _open):
+        """Test configuring installation source from UCA sources"""
+        _lsb.return_value = FAKE_RELEASE
+        _file = MagicMock(spec=io.FileIO)
+        _open.return_value = _file
+        _fip.side_effect = lambda x: x
+        for src, url in UCA_SOURCES:
+            actual_url = "# Ubuntu Cloud Archive\n{}\n".format(url)
+            fetch.add_source(src)
+            _install.assert_called_with(['ubuntu-cloud-keyring'],
+                                        fatal=True)
+            _open.assert_called_with(
+                '/etc/apt/sources.list.d/cloud-archive.list',
+                'w'
+            )
+            _file.__enter__().write.assert_called_with(actual_url)
+
+    def test_configure_install_source_bad_uca(self):
+        """Test configuring installation source from bad UCA source"""
+        try:
+            fetch.add_source('cloud:foo-bar', fail_invalid=True)
+            self.fail("add_source('cloud:foo-bar') should fail")
+        except fetch.SourceConfigError as e:
+            _e = ('Invalid Cloud Archive release specified: foo-bar'
+                  ' on this Ubuntuversion')
+            self.assertTrue(str(e).startswith(_e))
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_add_unparsable_source(self, log_):
+        source = "propsed"  # Minor typo
+        fetch.add_source(source=source)
+        self.assertEqual(1, log_.call_count)
+
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_add_distro_source(self, log):
+        source = "distro"
+        # distro is a noop but test validate no exception is thrown
+        fetch.add_source(source=source)
+
+
+class AptTests(TestCase):
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_apt_upgrade_non_fatal(self, log, mock_call):
+        options = ['--foo', '--bar']
+        fetch.apt_upgrade(options)
+
+        mock_call.assert_called_with(
+            ['apt-get', '--assume-yes',
+             '--foo', '--bar', 'upgrade'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_apt_upgrade_fatal(self, log, mock_call):
+        options = ['--foo', '--bar']
+        fetch.apt_upgrade(options, fatal=True)
+
+        mock_call.assert_called_with(
+            ['apt-get', '--assume-yes',
+             '--foo', '--bar', 'upgrade'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_apt_dist_upgrade_fatal(self, log, mock_call):
+        options = ['--foo', '--bar']
+        fetch.apt_upgrade(options, fatal=True, dist=True)
+
+        mock_call.assert_called_with(
+            ['apt-get', '--assume-yes',
+             '--foo', '--bar', 'dist-upgrade'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_installs_apt_packages(self, log, mock_call):
+        packages = ['foo', 'bar']
+        options = ['--foo', '--bar']
+
+        fetch.apt_install(packages, options)
+
+        mock_call.assert_called_with(
+            ['apt-get', '--assume-yes',
+             '--foo', '--bar', 'install', 'foo', 'bar'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_installs_apt_packages_without_options(self, log, mock_call):
+        packages = ['foo', 'bar']
+
+        fetch.apt_install(packages)
+
+        mock_call.assert_called_with(
+            ['apt-get', '--assume-yes',
+             '--option=Dpkg::Options::=--force-confold',
+             'install', 'foo', 'bar'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_installs_apt_packages_as_string(self, log, mock_call):
+        packages = 'foo bar'
+        options = ['--foo', '--bar']
+
+        fetch.apt_install(packages, options)
+
+        mock_call.assert_called_with(
+            ['apt-get', '--assume-yes',
+             '--foo', '--bar', 'install', 'foo bar'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_installs_apt_packages_with_possible_errors(self, log,
+                                                        check_call):
+        packages = ['foo', 'bar']
+        options = ['--foo', '--bar']
+
+        fetch.apt_install(packages, options, fatal=True)
+
+        check_call.assert_called_with(
+            ['apt-get', '--assume-yes',
+             '--foo', '--bar', 'install', 'foo', 'bar'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_purges_apt_packages_as_string_fatal(self, log, mock_call):
+        packages = 'irrelevant names'
+        mock_call.side_effect = OSError('fail')
+
+        self.assertRaises(OSError, fetch.apt_purge, packages, fatal=True)
+        self.assertTrue(log.called)
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_purges_apt_packages_fatal(self, log, mock_call):
+        packages = ['irrelevant', 'names']
+        mock_call.side_effect = OSError('fail')
+
+        self.assertRaises(OSError, fetch.apt_purge, packages, fatal=True)
+        self.assertTrue(log.called)
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_purges_apt_packages_as_string_nofatal(self, log, mock_call):
+        packages = 'foo bar'
+
+        fetch.apt_purge(packages)
+
+        self.assertTrue(log.called)
+        mock_call.assert_called_with(
+            ['apt-get', '--assume-yes', 'purge', 'foo bar'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_purges_apt_packages_nofatal(self, log, mock_call):
+        packages = ['foo', 'bar']
+
+        fetch.apt_purge(packages)
+
+        self.assertTrue(log.called)
+        mock_call.assert_called_with(
+            ['apt-get', '--assume-yes', 'purge', 'foo', 'bar'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_mark_apt_packages_as_string_fatal(self, log, mock_call):
+        packages = 'irrelevant names'
+        mock_call.side_effect = OSError('fail')
+
+        self.assertRaises(OSError, fetch.apt_mark, packages, sentinel.mark,
+                          fatal=True)
+        self.assertTrue(log.called)
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_mark_apt_packages_fatal(self, log, mock_call):
+        packages = ['irrelevant', 'names']
+        mock_call.side_effect = OSError('fail')
+
+        self.assertRaises(OSError, fetch.apt_mark, packages, sentinel.mark,
+                          fatal=True)
+        self.assertTrue(log.called)
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_mark_apt_packages_as_string_nofatal(self, log, mock_call):
+        packages = 'foo bar'
+
+        fetch.apt_mark(packages, sentinel.mark)
+
+        self.assertTrue(log.called)
+        mock_call.assert_called_with(
+            ['apt-mark', sentinel.mark, 'foo bar'],
+            universal_newlines=True)
+
+    @patch('subprocess.call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_mark_apt_packages_nofatal(self, log, mock_call):
+        packages = ['foo', 'bar']
+
+        fetch.apt_mark(packages, sentinel.mark)
+
+        self.assertTrue(log.called)
+        mock_call.assert_called_with(
+            ['apt-mark', sentinel.mark, 'foo', 'bar'],
+            universal_newlines=True)
+
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_mark_apt_packages_nofatal_abortonfatal(self, log, mock_call):
+        packages = ['foo', 'bar']
+
+        fetch.apt_mark(packages, sentinel.mark, fatal=True)
+
+        self.assertTrue(log.called)
+        mock_call.assert_called_with(
+            ['apt-mark', sentinel.mark, 'foo', 'bar'],
+            universal_newlines=True)
+
+    @patch('charmhelpers.fetch.ubuntu.apt_mark')
+    def test_apt_hold(self, apt_mark):
+        fetch.apt_hold(sentinel.packages)
+        apt_mark.assert_called_once_with(sentinel.packages, 'hold',
+                                         fatal=False)
+
+    @patch('charmhelpers.fetch.ubuntu.apt_mark')
+    def test_apt_hold_fatal(self, apt_mark):
+        fetch.apt_hold(sentinel.packages, fatal=sentinel.fatal)
+        apt_mark.assert_called_once_with(sentinel.packages, 'hold',
+                                         fatal=sentinel.fatal)
+
+    @patch('charmhelpers.fetch.ubuntu.apt_mark')
+    def test_apt_unhold(self, apt_mark):
+        fetch.apt_unhold(sentinel.packages)
+        apt_mark.assert_called_once_with(sentinel.packages, 'unhold',
+                                         fatal=False)
+
+    @patch('charmhelpers.fetch.ubuntu.apt_mark')
+    def test_apt_unhold_fatal(self, apt_mark):
+        fetch.apt_unhold(sentinel.packages, fatal=sentinel.fatal)
+        apt_mark.assert_called_once_with(sentinel.packages, 'unhold',
+                                         fatal=sentinel.fatal)
+
+    @patch('subprocess.check_call')
+    def test_apt_update_fatal(self, check_call):
+        fetch.apt_update(fatal=True)
+        check_call.assert_called_with(
+            ['apt-get', 'update'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.call')
+    def test_apt_update_nonfatal(self, call):
+        fetch.apt_update()
+        call.assert_called_with(
+            ['apt-get', 'update'],
+            env=getenv({'DEBIAN_FRONTEND': 'noninteractive'}))
+
+    @patch('subprocess.check_call')
+    @patch('time.sleep')
+    def test_run_apt_command_retries_if_fatal(self, check_call, sleep):
+        """The _run_apt_command function retries the command if it can't get
+        the APT lock."""
+        self.called = False
+
+        def side_effect(*args, **kwargs):
+            """
+            First, raise an exception (can't acquire lock), then return 0
+            (the lock is grabbed).
+            """
+            if not self.called:
+                self.called = True
+                raise subprocess.CalledProcessError(
+                    returncode=100, cmd="some command")
+            else:
+                return 0
+
+        check_call.side_effect = side_effect
+        check_call.return_value = 0
+
+        from charmhelpers.fetch.ubuntu import _run_apt_command
+        _run_apt_command(["some", "command"], fatal=True)
+        self.assertTrue(sleep.called)
+
+    @patch('apt_pkg.Cache')
+    def test_get_upstream_version(self, cache):
+        cache.side_effect = fake_apt_cache
+        self.assertEqual(fetch.get_upstream_version('vim'), '7.3.547')
+        self.assertEqual(fetch.get_upstream_version('emacs'), None)
+        self.assertEqual(fetch.get_upstream_version('unknown'), None)