Merge lp:~free.ekanayaka/landscape-charm/pull-python-2-in-install-hook into lp:~landscape/landscape-charm/trunk

[Free Ekanayaka]

This branch modifies the landscape-server charm minimally to get it
working on xenial units.

The change ports the install hook (and related tests) to python 3, so
we can install python 2 along with our regular dependencies.

This branch modifies the landscape-server charm minimally to get it
working on xenial units.

The change ports the install hook (and related tests) to python 3, so
we can install python 2 along with our regular dependencies.

I've tested this modified charm both with trusty and xenial targets
and it worked okay [0].

The diff is a bit large mainly because it contains a charmhelper upgrade,
the bare diff is here:

https://pastebin.canonical.com/155935/

[0] we have no bundle for xenial yet but you can test it by using
    something like this https://pastebin.canonical.com/155933/
    (replace placeholder values as appropriate)

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Fail
Revno: 349
Branch: lp:~free.ekanayaka/landscape-charm/pull-python-2-in-install-hook
Jenkins: https://ci.lscape.net/job/latch-test/4531/

review: Needs Fixing (test results)

[lp:~free.ekanayaka/landscape-charm/pull-python-2-in-install-hook]

350. By Free Ekanayaka on 2016-05-06

Fix tests

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Fail
Revno: 350
Branch: lp:~free.ekanayaka/landscape-charm/pull-python-2-in-install-hook
Jenkins: https://ci.lscape.net/job/latch-test/4532/

review: Needs Fixing (test results)

[lp:~free.ekanayaka/landscape-charm/pull-python-2-in-install-hook]

351. By Free Ekanayaka on 2016-05-06

Fix import

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Fail
Revno: 351
Branch: lp:~free.ekanayaka/landscape-charm/pull-python-2-in-install-hook
Jenkins: https://ci.lscape.net/job/latch-test/4533/

review: Needs Fixing (test results)

[lp:~free.ekanayaka/landscape-charm/pull-python-2-in-install-hook]

352. By Free Ekanayaka on 2016-05-06

Fix import for real

[🤖 Landscape Builder (landscape-builder)]

review: Abstain (executing tests)

[🤖 Landscape Builder (landscape-builder)]

Command: make ci-test
Result: Success
Revno: 352
Branch: lp:~free.ekanayaka/landscape-charm/pull-python-2-in-install-hook
Jenkins: https://ci.lscape.net/job/latch-test/4534/

review: Approve (test results)

[Chris Glass (tribaal)]

Looks good, +1.

review: Approve

[Benji York (benji)]

This branch looks good. I commented on a few small things inline.

review: Approve

[Данило Шеган (danilo)]

make lint complains:

$ make lint
flake8 --filename='*' hooks
flake8 lib tests
lib/action.py:36:27: E901 SyntaxError: invalid syntax
Makefile:64: recipe for target 'lint' failed
make: *** [lint] Error 1

I'll do a run with xenial just in case (since I've already started looking at this)

[Данило Шеган (danilo)]

I am having trouble with my local env and xenial, so I won't be doing a test run right now: please don't block on me (you've already got enough reviews).

review: Abstain

[Free Ekanayaka (free.ekanayaka)]

> make lint complains:
>
> $ make lint
> flake8 --filename='*' hooks
> flake8 lib tests
> lib/action.py:36:27: E901 SyntaxError: invalid syntax
> Makefile:64: recipe for target 'lint' failed
> make: *** [lint] Error 1

Fixed.

=== modified file 'Makefile'
--- Makefile	2015-11-17 04:32:24 +0000
+++ Makefile	2016-05-06 14:04:41 +0000
@@ -2,6 +2,8 @@
 
 test:
 	trial lib
+	# For now only the install hook runs against python3
+	trial3 lib/tests/test_apt.py lib/tests/test_install.py
 
 ci-test:
 	./dev/ubuntu-deps

=== modified file 'charmhelpers/core/hookenv.py'
--- charmhelpers/core/hookenv.py	2015-12-11 15:23:38 +0000
+++ charmhelpers/core/hookenv.py	2016-05-06 14:04:41 +0000
@@ -912,6 +912,24 @@
     subprocess.check_call(cmd)
 
 
+@translate_exc(from_exc=OSError, to_exc=NotImplementedError)
+def resource_get(name):
+    """used to fetch the resource path of the given name.
+
+    <name> must match a name of defined resource in metadata.yaml
+
+    returns either a path or False if resource not available
+    """
+    if not name:
+        return False
+
+    cmd = ['resource-get', name]
+    try:
+        return subprocess.check_output(cmd).decode('UTF-8')
+    except subprocess.CalledProcessError:
+        return False
+
+
 @cached
 def juju_version():
     """Full version string (eg. '1.23.3.1-trusty-amd64')"""
@@ -976,3 +994,16 @@
     for callback, args, kwargs in reversed(_atexit):
         callback(*args, **kwargs)
     del _atexit[:]
+
+
+@translate_exc(from_exc=OSError, to_exc=NotImplementedError)
+def network_get_primary_address(binding):
+    '''
+    Retrieve the primary network address for a named binding
+
+    :param binding: string. The name of a relation of extra-binding
+    :return: string. The primary IP address for the named binding
+    :raise: NotImplementedError if run on Juju < 2.0
+    '''
+    cmd = ['network-get', '--primary-address', binding]
+    return subprocess.check_output(cmd).strip()

=== modified file 'charmhelpers/core/host.py'
--- charmhelpers/core/host.py	2015-12-11 15:23:38 +0000
+++ charmhelpers/core/host.py	2016-05-06 14:04:41 +0000
@@ -30,6 +30,8 @@
 import string
 import subprocess
 import hashlib
+import functools
+import itertools
 from contextlib import contextmanager
 from collections import OrderedDict
 
@@ -72,7 +74,9 @@
         stopped = service_stop(service_name)
     upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
     sysv_file = os.path.join(initd_dir, service_name)
-    if os.path.exists(upstart_file):
+    if init_is_systemd():
+        service('disable', service_name)
+    elif os.path.exists(upstart_file):
         override_path = os.path.join(
             init_dir, '{}.override'.format(service_name))
         with open(override_path, 'w') as fh:
@@ -80,9 +84,9 @@
     elif os.path.exists(sysv_file):
         subprocess.check_call(["update-rc.d", service_name, "disable"])
     else:
-        # XXX: Support SystemD too
         raise ValueError(
-            "Unable to detect {0} as either Upstart {1} or SysV {2}".format(
+            "Unable to detect {0} as SystemD, Upstart {1} or"
+            " SysV {2}".format(
                 service_name, upstart_file, sysv_file))
     return stopped
 
@@ -94,7 +98,9 @@
     Reenable starting again at boot. Start the service"""
     upstart_file = os.path.join(init_dir, "{}.conf".format(service_name))
     sysv_file = os.path.join(initd_dir, service_name)
-    if os.path.exists(upstart_file):
+    if init_is_systemd():
+        service('enable', service_name)
+    elif os.path.exists(upstart_file):
         override_path = os.path.join(
             init_dir, '{}.override'.format(service_name))
         if os.path.exists(override_path):
@@ -102,9 +108,9 @@
     elif os.path.exists(sysv_file):
         subprocess.check_call(["update-rc.d", service_name, "enable"])
     else:
-        # XXX: Support SystemD too
         raise ValueError(
-            "Unable to detect {0} as either Upstart {1} or SysV {2}".format(
+            "Unable to detect {0} as SystemD, Upstart {1} or"
+            " SysV {2}".format(
                 service_name, upstart_file, sysv_file))
 
     started = service_running(service_name)
@@ -115,22 +121,40 @@
 
 def service(action, service_name):
     """Control a system service"""
-    cmd = ['service', service_name, action]
+    if init_is_systemd():
+        cmd = ['systemctl', action, service_name]
+    else:
+        cmd = ['service', service_name, action]
     return subprocess.call(cmd) == 0
 
 
-def service_running(service):
+def systemv_services_running():
+    output = subprocess.check_output(
+        ['service', '--status-all'],
+        stderr=subprocess.STDOUT).decode('UTF-8')
+    return [row.split()[-1] for row in output.split('\n') if '[ + ]' in row]
+
+
+def service_running(service_name):
     """Determine whether a system service is running"""
-    try:
-        output = subprocess.check_output(
-            ['service', service, 'status'],
-            stderr=subprocess.STDOUT).decode('UTF-8')
-    except subprocess.CalledProcessError:
-        return False
+    if init_is_systemd():
+        return service('is-active', service_name)
     else:
-        if ("start/running" in output or "is running" in output):
-            return True
+        try:
+            output = subprocess.check_output(
+                ['service', service_name, 'status'],
+                stderr=subprocess.STDOUT).decode('UTF-8')
+        except subprocess.CalledProcessError:
+            return False
         else:
+            # This works for upstart scripts where the 'service' command
+            # returns a consistent string to represent running 'start/running'
+            if ("start/running" in output or "is running" in output or
+                    "up and running" in output):
+                return True
+            # Check System V scripts init script return codes
+            if service_name in systemv_services_running():
+                return True
             return False
 
 
@@ -146,10 +170,17 @@
         return True
 
 
+SYSTEMD_SYSTEM = '/run/systemd/system'
+
+
+def init_is_systemd():
+    """Return True if the host system uses systemd, False otherwise."""
+    return os.path.isdir(SYSTEMD_SYSTEM)
+
+
 def adduser(username, password=None, shell='/bin/bash', system_user=False,
             primary_group=None, secondary_groups=None):
-    """
-    Add a user to the system.
+    """Add a user to the system.
 
     Will log but otherwise succeed if the user already exists.
 
@@ -157,7 +188,7 @@
     :param str password: Password for user; if ``None``, create a system user
     :param str shell: The default shell for the user
     :param bool system_user: Whether to create a login or system user
-    :param str primary_group: Primary group for user; defaults to their username
+    :param str primary_group: Primary group for user; defaults to username
     :param list secondary_groups: Optional list of additional groups
 
     :returns: The password database entry struct, as returned by `pwd.getpwnam`
@@ -283,14 +314,12 @@
 
 
 def fstab_remove(mp):
-    """Remove the given mountpoint entry from /etc/fstab
-    """
+    """Remove the given mountpoint entry from /etc/fstab"""
     return Fstab.remove_by_mountpoint(mp)
 
 
 def fstab_add(dev, mp, fs, options=None):
-    """Adds the given device entry to the /etc/fstab file
-    """
+    """Adds the given device entry to the /etc/fstab file"""
     return Fstab.add(dev, mp, fs, options=options)
 
 
@@ -346,8 +375,7 @@
 
 
 def file_hash(path, hash_type='md5'):
-    """
-    Generate a hash checksum of the contents of 'path' or None if not found.
+    """Generate a hash checksum of the contents of 'path' or None if not found.
 
     :param str hash_type: Any hash alrgorithm supported by :mod:`hashlib`,
                           such as md5, sha1, sha256, sha512, etc.
@@ -362,10 +390,9 @@
 
 
 def path_hash(path):
-    """
-    Generate a hash checksum of all files matching 'path'. Standard wildcards
-    like '*' and '?' are supported, see documentation for the 'glob' module for
-    more information.
+    """Generate a hash checksum of all files matching 'path'. Standard
+    wildcards like '*' and '?' are supported, see documentation for the 'glob'
+    module for more information.
 
     :return: dict: A { filename: hash } dictionary for all matched files.
                    Empty if none found.
@@ -377,8 +404,7 @@
 
 
 def check_hash(path, checksum, hash_type='md5'):
-    """
-    Validate a file using a cryptographic checksum.
+    """Validate a file using a cryptographic checksum.
 
     :param str checksum: Value of the checksum used to validate the file.
     :param str hash_type: Hash algorithm used to generate `checksum`.
@@ -393,10 +419,11 @@
 
 
 class ChecksumError(ValueError):
+    """A class derived from Value error to indicate the checksum failed."""
     pass
 
 
-def restart_on_change(restart_map, stopstart=False):
+def restart_on_change(restart_map, stopstart=False, restart_functions=None):
     """Restart services based on configuration files changing
 
     This function is used a decorator, for example::
@@ -414,27 +441,58 @@
     restarted if any file matching the pattern got changed, created
     or removed. Standard wildcards are supported, see documentation
     for the 'glob' module for more information.
+
+    @param restart_map: {path_file_name: [service_name, ...]
+    @param stopstart: DEFAULT false; whether to stop, start OR restart
+    @param restart_functions: nonstandard functions to use to restart services
+                              {svc: func, ...}
+    @returns result from decorated function
     """
     def wrap(f):
+        @functools.wraps(f)
         def wrapped_f(*args, **kwargs):
-            checksums = {path: path_hash(path) for path in restart_map}
-            f(*args, **kwargs)
-            restarts = []
-            for path in restart_map:
-                if path_hash(path) != checksums[path]:
-                    restarts += restart_map[path]
-            services_list = list(OrderedDict.fromkeys(restarts))
-            if not stopstart:
-                for service_name in services_list:
-                    service('restart', service_name)
-            else:
-                for action in ['stop', 'start']:
-                    for service_name in services_list:
-                        service(action, service_name)
+            return restart_on_change_helper(
+                (lambda: f(*args, **kwargs)), restart_map, stopstart,
+                restart_functions)
         return wrapped_f
     return wrap
 
 
+def restart_on_change_helper(lambda_f, restart_map, stopstart=False,
+                             restart_functions=None):
+    """Helper function to perform the restart_on_change function.
+
+    This is provided for decorators to restart services if files described
+    in the restart_map have changed after an invocation of lambda_f().
+
+    @param lambda_f: function to call.
+    @param restart_map: {file: [service, ...]}
+    @param stopstart: whether to stop, start or restart a service
+    @param restart_functions: nonstandard functions to use to restart services
+                              {svc: func, ...}
+    @returns result of lambda_f()
+    """
+    if restart_functions is None:
+        restart_functions = {}
+    checksums = {path: path_hash(path) for path in restart_map}
+    r = lambda_f()
+    # create a list of lists of the services to restart
+    restarts = [restart_map[path]
+                for path in restart_map
+                if path_hash(path) != checksums[path]]
+    # create a flat list of ordered services without duplicates from lists
+    services_list = list(OrderedDict.fromkeys(itertools.chain(*restarts)))
+    if services_list:
+        actions = ('stop', 'start') if stopstart else ('restart',)
+        for service_name in services_list:
+            if service_name in restart_functions:
+                restart_functions[service_name](service_name)
+            else:
+                for action in actions:
+                    service(action, service_name)
+    return r
+
+
 def lsb_release():
     """Return /etc/lsb-release in a dict"""
     d = {}
@@ -498,7 +556,7 @@
 
 
 def list_nics(nic_type=None):
-    '''Return a list of nics of given type(s)'''
+    """Return a list of nics of given type(s)"""
     if isinstance(nic_type, six.string_types):
         int_types = [nic_type]
     else:
@@ -540,12 +598,13 @@
 
 
 def set_nic_mtu(nic, mtu):
-    '''Set MTU on a network interface'''
+    """Set the Maximum Transmission Unit (MTU) on a network interface."""
     cmd = ['ip', 'link', 'set', nic, 'mtu', mtu]
     subprocess.check_call(cmd)
 
 
 def get_nic_mtu(nic):
+    """Return the Maximum Transmission Unit (MTU) for a network interface."""
     cmd = ['ip', 'addr', 'show', nic]
     ip_output = subprocess.check_output(cmd).decode('UTF-8').split('\n')
     mtu = ""
@@ -557,6 +616,7 @@
 
 
 def get_nic_hwaddr(nic):
+    """Return the Media Access Control (MAC) for a network interface."""
     cmd = ['ip', '-o', '-0', 'addr', 'show', nic]
     ip_output = subprocess.check_output(cmd).decode('UTF-8')
     hwaddr = ""
@@ -567,7 +627,7 @@
 
 
 def cmp_pkgrevno(package, revno, pkgcache=None):
-    '''Compare supplied revno with the revno of the installed package
+    """Compare supplied revno with the revno of the installed package
 
     *  1 => Installed revno is greater than supplied arg
     *  0 => Installed revno is the same as supplied arg
@@ -576,7 +636,7 @@
     This function imports apt_cache function from charmhelpers.fetch if
     the pkgcache argument is None. Be sure to add charmhelpers.fetch if
     you call this function, or pass an apt_pkg.Cache() instance.
-    '''
+    """
     import apt_pkg
     if not pkgcache:
         from charmhelpers.fetch import apt_cache
@@ -586,19 +646,27 @@
 
 
 @contextmanager
-def chdir(d):
+def chdir(directory):
+    """Change the current working directory to a different directory for a code
+    block and return the previous directory after the block exits. Useful to
+    run commands from a specificed directory.
+
+    :param str directory: The directory path to change to for this context.
+    """
     cur = os.getcwd()
     try:
-        yield os.chdir(d)
+        yield os.chdir(directory)
     finally:
         os.chdir(cur)
 
 
 def chownr(path, owner, group, follow_links=True, chowntopdir=False):
-    """
-    Recursively change user and group ownership of files and directories
+    """Recursively change user and group ownership of files and directories
     in given path. Doesn't chown path itself by default, only its children.
 
+    :param str path: The string path to start changing ownership.
+    :param str owner: The owner string to use when looking up the uid.
+    :param str group: The group string to use when looking up the gid.
     :param bool follow_links: Also Chown links if True
     :param bool chowntopdir: Also chown path itself if True
     """
@@ -622,15 +690,23 @@
 
 
 def lchownr(path, owner, group):
+    """Recursively change user and group ownership of files and directories
+    in a given path, not following symbolic links. See the documentation for
+    'os.lchown' for more information.
+
+    :param str path: The string path to start changing ownership.
+    :param str owner: The owner string to use when looking up the uid.
+    :param str group: The group string to use when looking up the gid.
+    """
     chownr(path, owner, group, follow_links=False)
 
 
 def get_total_ram():
-    '''The total amount of system RAM in bytes.
+    """The total amount of system RAM in bytes.
 
     This is what is reported by the OS, and may be overcommitted when
     there are multiple containers hosted on the same machine.
-    '''
+    """
     with open('/proc/meminfo', 'r') as f:
         for line in f.readlines():
             if line:

=== modified file 'charmhelpers/fetch/__init__.py'
--- charmhelpers/fetch/__init__.py	2015-12-11 15:23:38 +0000
+++ charmhelpers/fetch/__init__.py	2016-05-06 14:04:41 +0000
@@ -98,6 +98,14 @@
     'liberty/proposed': 'trusty-proposed/liberty',
     'trusty-liberty/proposed': 'trusty-proposed/liberty',
     'trusty-proposed/liberty': 'trusty-proposed/liberty',
+    # Mitaka
+    'mitaka': 'trusty-updates/mitaka',
+    'trusty-mitaka': 'trusty-updates/mitaka',
+    'trusty-mitaka/updates': 'trusty-updates/mitaka',
+    'trusty-updates/mitaka': 'trusty-updates/mitaka',
+    'mitaka/proposed': 'trusty-proposed/mitaka',
+    'trusty-mitaka/proposed': 'trusty-proposed/mitaka',
+    'trusty-proposed/mitaka': 'trusty-proposed/mitaka',
 }
 
 # The order of this list is very important. Handlers should be listed in from
@@ -411,7 +419,7 @@
                 importlib.import_module(package),
                 classname)
             plugin_list.append(handler_class())
-        except (ImportError, AttributeError):
+        except NotImplementedError:
             # Skip missing plugins so that they can be ommitted from
             # installation if desired
             log("FetchHandler {} not found, skipping plugin".format(

=== modified file 'charmhelpers/fetch/bzrurl.py'
--- charmhelpers/fetch/bzrurl.py	2015-12-11 15:23:38 +0000
+++ charmhelpers/fetch/bzrurl.py	2016-05-06 14:04:41 +0000
@@ -15,54 +15,40 @@
 # along with charm-helpers.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+from subprocess import check_call
 from charmhelpers.fetch import (
     BaseFetchHandler,
-    UnhandledSource
+    UnhandledSource,
+    filter_installed_packages,
+    apt_install,
 )
 from charmhelpers.core.host import mkdir
 
-import six
-if six.PY3:
-    raise ImportError('bzrlib does not support Python3')
 
-try:
-    from bzrlib.branch import Branch
-    from bzrlib import bzrdir, workingtree, errors
-except ImportError:
-    from charmhelpers.fetch import apt_install
-    apt_install("python-bzrlib")
-    from bzrlib.branch import Branch
-    from bzrlib import bzrdir, workingtree, errors
+if filter_installed_packages(['bzr']) != []:
+    apt_install(['bzr'])
+    if filter_installed_packages(['bzr']) != []:
+        raise NotImplementedError('Unable to install bzr')
 
 
 class BzrUrlFetchHandler(BaseFetchHandler):
     """Handler for bazaar branches via generic and lp URLs"""
     def can_handle(self, source):
         url_parts = self.parse_url(source)
-        if url_parts.scheme not in ('bzr+ssh', 'lp'):
+        if url_parts.scheme not in ('bzr+ssh', 'lp', ''):
             return False
+        elif not url_parts.scheme:
+            return os.path.exists(os.path.join(source, '.bzr'))
         else:
             return True
 
     def branch(self, source, dest):
-        url_parts = self.parse_url(source)
-        # If we use lp:branchname scheme we need to load plugins
         if not self.can_handle(source):
             raise UnhandledSource("Cannot handle {}".format(source))
-        if url_parts.scheme == "lp":
-            from bzrlib.plugin import load_plugins
-            load_plugins()
-        try:
-            local_branch = bzrdir.BzrDir.create_branch_convenience(dest)
-        except errors.AlreadyControlDirError:
-            local_branch = Branch.open(dest)
-        try:
-            remote_branch = Branch.open(source)
-            remote_branch.push(local_branch)
-            tree = workingtree.WorkingTree.open(dest)
-            tree.update()
-        except Exception as e:
-            raise e
+        if os.path.exists(dest):
+            check_call(['bzr', 'pull', '--overwrite', '-d', dest, source])
+        else:
+            check_call(['bzr', 'branch', source, dest])
 
     def install(self, source, dest=None):
         url_parts = self.parse_url(source)

=== modified file 'charmhelpers/fetch/giturl.py'
--- charmhelpers/fetch/giturl.py	2015-12-11 15:23:38 +0000
+++ charmhelpers/fetch/giturl.py	2016-05-06 14:04:41 +0000
@@ -15,20 +15,18 @@
 # along with charm-helpers.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+from subprocess import check_call, CalledProcessError
 from charmhelpers.fetch import (
     BaseFetchHandler,
-    UnhandledSource
+    UnhandledSource,
+    filter_installed_packages,
+    apt_install,
 )
-from charmhelpers.core.host import mkdir
-
-try:
-    from git import Repo
-except ImportError:
-    from charmhelpers.fetch import apt_install
-    apt_install("python-git")
-    from git import Repo
-
-from git.exc import GitCommandError  # noqa E402
+
+if filter_installed_packages(['git']) != []:
+    apt_install(['git'])
+    if filter_installed_packages(['git']) != []:
+        raise NotImplementedError('Unable to install git')
 
 
 class GitUrlFetchHandler(BaseFetchHandler):
@@ -36,19 +34,24 @@
     def can_handle(self, source):
         url_parts = self.parse_url(source)
         # TODO (mattyw) no support for ssh git@ yet
-        if url_parts.scheme not in ('http', 'https', 'git'):
+        if url_parts.scheme not in ('http', 'https', 'git', ''):
             return False
+        elif not url_parts.scheme:
+            return os.path.exists(os.path.join(source, '.git'))
         else:
             return True
 
-    def clone(self, source, dest, branch, depth=None):
+    def clone(self, source, dest, branch="master", depth=None):
         if not self.can_handle(source):
             raise UnhandledSource("Cannot handle {}".format(source))
 
-        if depth:
-            Repo.clone_from(source, dest, branch=branch, depth=depth)
+        if os.path.exists(dest):
+            cmd = ['git', '-C', dest, 'pull', source, branch]
         else:
-            Repo.clone_from(source, dest, branch=branch)
+            cmd = ['git', 'clone', source, dest, '--branch', branch]
+            if depth:
+                cmd.extend(['--depth', depth])
+        check_call(cmd)
 
     def install(self, source, branch="master", dest=None, depth=None):
         url_parts = self.parse_url(source)
@@ -58,11 +61,9 @@
         else:
             dest_dir = os.path.join(os.environ.get('CHARM_DIR'), "fetched",
                                     branch_name)
-        if not os.path.exists(dest_dir):
-            mkdir(dest_dir, perms=0o755)
         try:
             self.clone(source, dest_dir, branch, depth)
-        except GitCommandError as e:
+        except CalledProcessError as e:
             raise UnhandledSource(e)
         except OSError as e:
             raise UnhandledSource(e.strerror)

=== modified file 'dev/ubuntu-deps'
--- dev/ubuntu-deps	2016-02-03 01:41:18 +0000
+++ dev/ubuntu-deps	2016-05-06 14:04:41 +0000
@@ -3,20 +3,15 @@
 # It will ask your password a lot.
 
 # Install add-apt-repository (packaging differs across releases).
-lsb_release -r | grep -q 12.04 \
-    && sudo apt-get -y install python-software-properties \
-    || sudo apt-get -y install software-properties-common
+sudo apt-get -y install python-software-properties
 
 # Add the juju stable ppa, install charm-tools (juju-test plugin) and other deps
 sudo add-apt-repository -y ppa:juju/stable
 sudo apt-get update
 sudo apt-get -y install juju-deployer juju-core charm-tools python3 \
     python3-yaml curl python3-zope.testrunner \
-    python3-amulet python3-fixtures python3-jinja2
-
-# python3-flake8 was introduced after 12.04. Releases prior to that are not
-# supported.
-lsb_release -r | grep -q 12.04 || sudo apt-get -y install python3-flake8
+    python3-amulet python3-fixtures python3-jinja2 python3-fixtures\
+    python3-psutil python3-twisted python3-flake8
 
 sudo apt-get -y install python-fixtures python-six python-yaml dpkg-dev \
     pbuilder dh-make python-jinja2 python-psutil

=== modified file 'hooks/install'
--- hooks/install	2015-01-28 09:10:07 +0000
+++ hooks/install	2016-05-06 14:04:41 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 import sys
 
 from lib.install import InstallHook

=== modified file 'lib/apt.py'
--- lib/apt.py	2016-03-17 12:36:55 +0000
+++ lib/apt.py	2016-05-06 14:04:41 +0000
@@ -12,7 +12,7 @@
 from lib.paths import default_paths
 
 LANDSCAPE_PACKAGES = ("landscape-server", "landscape-hashids")
-INSTALL_PACKAGES = LANDSCAPE_PACKAGES + ("python-psutil",)
+INSTALL_PACKAGES = LANDSCAPE_PACKAGES + ("python-minimal", "python-psutil")
 PACKAGES_DEV = ("dpkg-dev", "pbuilder")
 TARBALL = "landscape-server_*.tar.gz"
 
@@ -255,8 +255,9 @@
 
     def _is_tarball_new(self, tarball):
         """Check if this is a new tarball and we need to build it."""
-        with open(tarball, "r") as fd:
-            digest = hashlib.md5(fd.read()).hexdigest()
+        with open(tarball, "rb") as fd:
+            data = fd.read()
+        digest = hashlib.md5(data).hexdigest()
 
         md5sum = tarball + ".md5sum"
         if os.path.exists(md5sum):

=== modified file 'lib/tests/rootdir.py'
--- lib/tests/rootdir.py	2015-05-28 12:25:22 +0000
+++ lib/tests/rootdir.py	2016-05-06 14:04:41 +0000
@@ -19,6 +19,6 @@
         os.makedirs(self.paths.config_dir())
         os.makedirs(os.path.dirname(self.paths.ssl_certificate()))
         os.makedirs(self.paths.offline_dir())
-        for path in ERRORFILES_MAP.itervalues():
+        for path in ERRORFILES_MAP.values():
             with open(os.path.join(self.paths.offline_dir(), path), "w") as fd:
                 fd.write("Fake %s" % path)

=== modified file 'lib/tests/test_apt.py'
--- lib/tests/test_apt.py	2016-03-17 12:36:55 +0000
+++ lib/tests/test_apt.py	2016-05-06 14:04:41 +0000
@@ -126,7 +126,7 @@
         """
         self.hookenv.config()["source"] = "15.11, ppa:juju/devel"
         self.apt.set_sources()
-        self.assertItemsEqual(
+        self.assertEqual(
             [("ppa:landscape/15.11", None), ("ppa:juju/devel", None)],
             self.fetch.sources)
 
@@ -138,7 +138,7 @@
         self.hookenv.config()["source"] = "15.11, deb http://host/ ./"
         self.hookenv.config()["key"] = "null, xyz"
         self.apt.set_sources()
-        self.assertItemsEqual(
+        self.assertEqual(
             [("ppa:landscape/15.11", None), ("deb http://host/ ./", "xyz")],
             self.fetch.sources)
 
@@ -235,7 +235,8 @@
         installed.
         """
         self.assertEqual(
-            ("landscape-server", "landscape-hashids", "python-psutil"),
+            ("landscape-server", "landscape-hashids", "python-minimal",
+             "python-psutil"),
             INSTALL_PACKAGES)
 
     def test_install(self):

=== modified file 'lib/tests/test_install.py'
--- lib/tests/test_install.py	2016-01-12 12:11:20 +0000
+++ lib/tests/test_install.py	2016-05-06 14:04:41 +0000
@@ -36,7 +36,7 @@
         os.makedirs(os.path.dirname(hook))
         with open(hook, "w") as fd:
             fd.write("#!/bin/sh\ntouch %s\n" % flag)
-        os.chmod(hook, 0755)
+        os.chmod(hook, 0o755)
         self.hookenv.config()["source"] = "ppa:landscape/14.10"
         self.hook()
         self.assertTrue(os.path.exists(flag))
@@ -50,7 +50,7 @@
         os.makedirs(os.path.dirname(hook))
         with open(hook, "w") as fd:
             fd.write("#!/bin/sh\ntexit 127\n")
-        os.chmod(hook, 0755)
+        os.chmod(hook, 0o755)
         self.hookenv.config()["source"] = "ppa:landscape/14.10"
         self.assertEqual(1, self.hook())
         self.assertEqual(
@@ -71,7 +71,7 @@
             fd.write("#!/bin/sh\ntouch %s\n" % flag)
         with open(hook2, "w") as fd:
             fd.write("#!/bin/sh\ntouch %s\n" % flag)
-        os.chmod(hook2, 0755)
+        os.chmod(hook2, 0o755)
         self.hookenv.config()["source"] = "ppa:landscape/14.10"
         self.hook()
         self.assertFalse(os.path.exists(flag))

=== modified file 'lib/tests/test_upgrade.py'
--- lib/tests/test_upgrade.py	2016-01-12 12:11:20 +0000
+++ lib/tests/test_upgrade.py	2016-05-06 14:04:41 +0000
@@ -2,6 +2,7 @@
 from lib.tests.rootdir import RootDir
 from lib.tests.stubs import FetchStub, SubprocessStub
 from lib.upgrade import UpgradeAction
+from lib.apt import INSTALL_PACKAGES
 
 
 class UpgradeActionTest(HookenvTest):
@@ -33,7 +34,7 @@
         self.assertEqual([True], self.fetch.updates)
         # And one apt_install with appropriate options.
         self.assertEqual(
-            [(("landscape-server", "landscape-hashids", "python-psutil"),
+            [(INSTALL_PACKAGES,
               ["--option=Dpkg::Options::=--force-confdef",
                "--option=Dpkg::Options::=--force-confold"],
               True)], self.fetch.installed)