Merge lp:~dooferlad/juju-ci-tools/juju-ci-tools-addressable-containers into lp:juju-ci-tools

[James Tunnicliffe]

This adds tests to check container networking as set up by the Juju addressable containers feature.
It contains comprehensive unit tests and passes make lint.

An unusual but I hope useful feature that, if you like it, should end up in a library is the request_machines / clean_environment pair of functions. They allow you to use an existing environment and existing machines to run tests on. This is particularly useful during development of tests because bringing up machines and containers is time consuming and when some of your tests are "ping google.com" then you don't want to wait 10 minutes to see if your fix for a dumb mistake works. Obviously you don't want to use it in production, but it sure speeds up test development!

I realise this is about 1000 lines (about half are tests) and I am happy to go through them with the reviewer.

[Martin Packman (gz)]

Thanks for the proposal!

I have a number of specific comments in line, but my general concern is this proposal adds a lot of new code just to this assess file where extending existing mechanisms would be preferable.

In particular, you could add methods in jujupy to Status for machine/container handling, and to EnvJujuClient for waiting. Using the existing bootstrap and log collection from deploy_stack would save a lot of code, and don't seem to need much modification for what you want to do here.

review: Needs Fixing

[Aaron Bentley (abentley)]

This is a follow-on to Martin's review.

Please don't apologize for the length of your diff. Instead, please split your work into separate chunks, as described in https://github.com/juju/juju/wiki/ci-tests#creating-a-new-ci-test

Please merge trunk and fix conflicts.

I haven't reviewed the unit tests, because I think the code being tested will change a lot.

See comments inline.

[James Tunnicliffe (dooferlad)]

Thanks for your comments. I clearly wasn't looking at the right tests for example code and should have asked to pair on this!

[James Tunnicliffe (dooferlad)]

I will be returning to this in a few days after we hit feature freeze - I haven't forgotten it!

[Aaron Bentley (abentley)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2015-08-17 05:19 AM, James Tunnicliffe wrote:
>> + while new_machine_count > 0: +
>> new_machine_count -= 1 + with
>> client.juju_async('add-machine', ''): + pass
>
> If memory serves it was because client.juju is blocking and I
> wanted to add multiple machines or containers in parallel. Will
> look for a nicer solution.

The point of the juju_async is that you do something in the 'with'
block while the process is executing. When you exit the block, we
call subprocess.Popen.wait(). So the effect of doing nothing in the
'with block' is the same as doing a blocking call.

Assuming 2 machines, here's what you've done:
1. For machine 1, client.juju_async('add-machine', '').__enter__() is
called. The process starts.
2. For machine 1, client.juju_async('add-machine', '').__exit__() is
called. This blocks until the process exits.
3. For machine 2, client.juju_async('add-machine', '').__enter__() is
called. The process starts.
4. For machine 2, client.juju_async('add-machine', '').__exit__() is
called. This blocks until the process exits.

What you want is something like:
1. For machine 1, client.juju_async('add-machine', '').__enter__() is
called. The process starts.
2. For machine 2, client.juju_async('add-machine', '').__enter__() is
called. The process starts.
3. For machine 1, client.juju_async('add-machine', '').__exit__() is
called. This blocks until the process exits.
4. For machine 2, client.juju_async('add-machine', '').__exit__() is
called. This blocks until the process exits.

The simplest way of achieving this that I can see is recursion:

def add_many_machines(machine_count):
    if machine_count == 0:
        return
    with client.juju_async('add-machine', ''):
        add_many_machines(machine_count - 1)

Of course, Python limits recursion. So another option would be
itertools.nested:

machine_contexts = [client.juju_async('add-machine', '')
                    for x in range(new_machine_count)]
with itertools.nested(*machine_contexts):
    pass

Unfortunately, itertools.nested is deprecated. There is syntax to
replace it, but AFAICT, it does not support variable-length lists.

Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1

iQEcBAEBAgAGBQJV0fx4AAoJEK84cMOcf+9hsqQH/2vK9Tt4Jusx9+ttQyKdzQBX
Bva8Y/8oe5y2uBTpQt9RGtSb+mAwJgZAVJ09uet7kXrXIolZ7pw1fzV/gWzQiK10
MoxhCz4hn083Wfy1c/PfHQyc6jVAbE0KRh76NqaqaIqj7MKqfLqs+NZ8grAH3iOS
OzChtvoJLO8b2WWzGT//ZKkjTaumPeFac4Um5cnBqqUK0mnqMMkPjZBDMZXR2Tq3
WsxXmCcqTnfwIP0HpyvgjPsj0HxeXptikc9PnMMZXoygzYl/m1whfva7mg/qY+yH
umKArOJVkQ6vLJ8vkPiIU2uYKv7SQuHf+Kjmu9HjTwaJKnZtIXVwvcLYFm3DU3Y=
=02h5
-----END PGP SIGNATURE-----

[Aaron Bentley (abentley)]

> Unfortunately, itertools.nested is deprecated. There is syntax to
> replace it, but AFAICT, it does not support variable-length lists.

According to StackOverflow, contextlib2.ExitStack is one way to do this. I would probably just use nested, though.

http://stackoverflow.com/questions/16083791/alternative-to-contextlib-nested-with-variable-number-of-context-managers
http://contextlib2.readthedocs.org/en/latest/

[lp:~dooferlad/juju-ci-tools/juju-ci-tools-addressable-containers]

1003. By James Tunnicliffe on 2015-09-15

Initial post-review updates.

1004. By James Tunnicliffe on 2015-09-21

Refactored assess_networking.py and associated tests. Now far more modular and shorter.

1005. By James Tunnicliffe on 2015-09-21

changed 'lxc' and 'kvm' into constants.
removed extra_env from bootstrap.

Unmerged revisions

1005. By James Tunnicliffe on 2015-09-21

changed 'lxc' and 'kvm' into constants.
removed extra_env from bootstrap.

1004. By James Tunnicliffe on 2015-09-21

Refactored assess_networking.py and associated tests. Now far more modular and shorter.

1003. By James Tunnicliffe on 2015-09-15

Initial post-review updates.

1002. By James Tunnicliffe on 2015-08-10

Some pre-review cleanup

1001. By James Tunnicliffe on 2015-08-10

Finished testing

1000. By James Tunnicliffe on 2015-08-04

Added the first few tests

999. By James Tunnicliffe on 2015-06-30

Fetch logs and configs on failure

998. By James Tunnicliffe on 2015-06-29

WIP container and VM networking tests

=== added file 'assess_container_networking.py'
--- assess_container_networking.py	1970-01-01 00:00:00 +0000
+++ assess_container_networking.py	2015-09-21 15:30:36 +0000
@@ -0,0 +1,427 @@
+#!/usr/bin/env python
+from __future__ import print_function
+
+__metaclass__ = type
+
+import logging
+import re
+import tempfile
+import os
+import socket
+from textwrap import dedent
+
+from jujuconfig import (
+    get_juju_home,
+)
+from jujupy import (
+    make_client,
+    parse_new_state_server_from_error,
+    temp_bootstrap_env,
+    SimpleEnvironment,
+    EnvJujuClient,
+)
+from utility import (
+    print_now,
+    add_basic_testing_arguments,
+)
+from deploy_stack import (
+    update_env,
+    dump_env_logs,
+)
+
+from argparse import ArgumentParser
+
+
+KVM_MACHINE = 'kvm'
+LXC_MACHINE = 'lxc'
+
+
+def parse_args(argv=None):
+    """Parse all arguments."""
+
+    description = dedent("""\
+    Test container address allocation.
+    For LXC and KVM, create machines of each type and test the network
+    between LXC <--> LXC, KVM <--> KVM and LXC <--> KVM. Also test machine
+    to outside world, DNS and that these tests still pass after a reboot. In
+    case of failure pull logs and configuration files from the machine that
+    we detected a problem on for later analysis.
+    """)
+    parser = add_basic_testing_arguments(ArgumentParser(
+        description=description
+    ))
+    parser.add_argument(
+        '--machine-type',
+        help='Which virtual machine/container type to test. Defaults to all.',
+        choices=[KVM_MACHINE, LXC_MACHINE])
+    parser.add_argument(
+        '--clean-environment', action='store_true', help=dedent("""\
+        Attempts to re-use an existing environment rather than destroying it
+        and creating a new one.
+
+        On launch, if an environment exists, clean out services and machines
+        from it rather than destroying it. If an environment doesn't exist,
+        create one and use it.
+
+        At termination, clean out services and machines from the environment
+        rather than destroying it."""))
+    return parser.parse_args(argv)
+
+
+def ssh(client, machine, cmd):
+    """Convenience function: run a juju ssh command and get back the output
+    :param client: A Juju client
+    :param machine: ID of the machine on which to run a command
+    :param cmd: the command to run
+    :return: text output of the command
+    """
+    return client.get_juju_output('ssh', machine, cmd)
+
+
+def clean_environment(client, services_only=False):
+    """Remove all the services and, optionally, machines from an environment.
+
+    Use as an alternative to destroying an environment and creating a new one
+    to save a bit of time.
+
+    :param client: a Juju client
+    """
+    status = client.get_status()
+    for service in status.status['services']:
+        client.juju('remove-service', service)
+
+    if not services_only:
+        # First remove all containers; we can't remove a machine that is
+        # hosting containers.
+        for m, _ in status.iter_machines(containers=True, machines=False):
+            client.juju('remove-machine', m)
+
+        client.wait_for('containers', 'none')
+
+        for m, _ in status.iter_machines(containers=False, machines=True):
+            if m != '0':
+                client.juju('remove-machine', m)
+
+        client.wait_for('machines-not-0', 'none')
+
+    client.wait_for_started()
+
+
+class MachineGetter:
+    def __init__(self, client):
+        """Get or allocate machines in this Juju environment
+        :param client: Juju client
+        """
+        self.client = client
+        self.count = None
+        self.requested_machines = []
+        self.allocated_machines = []
+
+    def get(self, container_type=None, machine=None, container=None, count=1):
+        """Get one or more machines or containers that match the given spec.
+        :param container_type: machine type ('machine', KVM_MACHINE,
+                LXC_MACHINE)
+        :param machine: obtain a specific machine if available
+        :param container: obtain a specific container on the given machine if
+                available
+        :param count: number of machines to allocate/create
+        :return: list of machine IDs
+        """
+
+        # Allow user to send integers for ease of calling
+        if machine is not None:
+            machine = str(machine)
+        if container is not None:
+            container = str(container)
+
+        self.count = count
+        self.requested_machines = []
+        self.allocated_machines = []
+
+        status = self.client.get_status().status
+
+        for s in status['services'].values():
+            for u in s['units'].values():
+                self.allocated_machines.append(u['machine'])
+
+        if container_type in [KVM_MACHINE, LXC_MACHINE]:
+            self._get_container(status, machine, container, container_type)
+        elif container_type is None:
+            self._get_machine(status, machine)
+        else:
+            raise ValueError("Unrecognised container type %r" % container_type)
+
+        return self.requested_machines
+
+    def _add_new_machines(self, machine_spec):
+        req_machines = [machine_spec for _ in range(self._new_machine_count())]
+        self._add_machines_worker(req_machines)
+        self.client.wait_for_started(60 * 10)
+
+    def _add_machines_worker(self, machines):
+        if len(machines) > 0:
+            with self.client.juju_async('add-machine', machines[0]):
+                if len(machines) > 1:
+                    self._add_machines_worker(machines[1:])
+
+    def _new_machine_count(self):
+        return self.count - len(self.requested_machines)
+
+    def _get_machine(self, status, machine):
+        # If we have the requested machine, return it if it exists
+        if machine:
+            if machine in status['machines']:
+                self.requested_machines.append(machine)
+            return
+
+        self._allocate_free_machines()
+        self._add_new_machines('')
+        self._allocate_free_machines()
+
+    def _get_container(self, status, machine, container, container_type):
+        if machine is not None and machine in status['machines']:
+            hosts = [machine]
+
+            if container is not None:
+                name = '/'.join([machine, container_type, container])
+                m = status['machines'][machine]
+                if 'containers' in m and name in m['containers']:
+                    self.requested_machines.append(name)
+                return
+        else:
+            hosts = sorted(status['machines'].keys())
+
+        self._allocate_free_containers(hosts, container_type)
+        req = container_type + ':' + (machine or '0')
+        self._add_new_machines(req)
+        self._allocate_free_containers(hosts, container_type)
+
+    def _allocate_free_containers(self, hosts, container_type):
+        status = self.client.get_status().status
+        for m in hosts:
+            if 'containers' in status['machines'][m]:
+                for c in status['machines'][m]['containers']:
+                    self._try_allocation(c, container_type)
+
+    def _allocate_free_machines(self):
+        status = self.client.get_status().status
+        for m in status['machines']:
+            self._try_allocation(m)
+
+    def _try_allocation(self, machine, container_type=None):
+        if (machine not in self.requested_machines and
+                machine not in self.allocated_machines):
+            if container_type is not None:
+                if container_type != machine.split('/')[1]:
+                    return
+
+            self.requested_machines.append(machine)
+            if self._new_machine_count() == 0:
+                return
+
+
+def find_network(client, machine, addr):
+    """Find a connected subnet containing the given address.
+
+    When using this to find the subnet of a container, don't use the container
+    as the machine to run the ip route show command on ("machine"), use a real
+    box because lxc will just send everything to its host machine, so it is on
+    a subnet containing itself. Not much use.
+    :param client: A Juju client
+    :param machine: ID of the machine on which to run a command
+    :param addr: find the connected subnet containing this address
+    :return: CIDR containing the address if found, else, None
+    """
+    ip_cmd = ' '.join(['ip', 'route', 'show', 'to', 'match', addr])
+    routes = ssh(client, machine, ip_cmd)
+
+    for route in re.findall(r'^(\S+).*[\d\.]+/\d+', routes, re.MULTILINE):
+        if route != 'default':
+            return route
+
+    raise ValueError("Unable to find route to %r" % addr)
+
+
+def assess_network_traffic(client, targets):
+    """Test that all containers in target can talk to target[0]
+    :param client: Juju client
+    :param targets: machine IDs of machines to test
+    :return: None;
+    """
+    status = client.wait_for_started().status
+    source = targets[0]
+    dests = targets[1:]
+
+    with tempfile.NamedTemporaryFile(delete=False) as f:
+        f.write('tmux new-session -d -s test "nc -l 6778 > nc_listen.out"')
+    client.juju('scp', (f.name, source + ':/home/ubuntu/listen.sh'))
+    os.remove(f.name)
+
+    # Containers are named 'x/type/y' where x is the host of the container. We
+    host = source.split('/')[0]
+    address = status['machines'][host]['containers'][source]['dns-name']
+
+    for dest in dests:
+        ssh(client, source, 'rm nc_listen.out; bash ./listen.sh')
+        ssh(client, dest, 'echo "hello" | nc ' + address + ' 6778')
+        result = ssh(client, source, 'more nc_listen.out')
+        if result.rstrip() != 'hello':
+            raise ValueError("Wrong or missing message: %r" % result.rstrip())
+
+
+def assess_address_range(client, targets):
+    """Test that two containers are in the same subnet as their host
+    :param client: Juju client
+    :param targets: machine IDs of machines to test
+    :return: None; will assert failures
+    """
+    status = client.wait_for_started().status
+
+    host = targets[0].split('/')[0]
+    host_address = socket.gethostbyname(status['machines'][host]['dns-name'])
+    host_subnet = find_network(client, host, host_address)
+
+    for target in targets:
+        vm_host = target.split('/')[0]
+        addr = status['machines'][vm_host]['containers'][target]['dns-name']
+        subnet = find_network(client, host, addr)
+        assert host_subnet == subnet, \
+            '{} ({}) not on the same subnet as {} ({})'.format(
+                target, subnet, host, host_subnet)
+
+
+def assess_internet_connection(client, targets):
+    """Test that targets can ping Google's DNS server, google.com
+    :param client: Juju client
+    :param targets: machine IDs of machines to test
+    :return: None; will assert failures
+    """
+
+    for target in targets:
+        routes = ssh(client, target, 'ip route show')
+
+        d = re.search(r'^default\s+via\s+([\d\.]+)\s+', routes, re.MULTILINE)
+        if d:
+            rc = client.juju('ssh', (target, 'ping -c1 -q ' + d.group(1)),
+                             check=False)
+            if rc != 0:
+                raise ValueError('%s unable to ping default route' % target)
+        else:
+            raise ValueError("Default route not found")
+
+
+def _assessment_iteration(client, containers):
+    """Run the network tests on this collection of machines and containers
+    :param client: Juju client
+    :param hosts: list of hosts of containers
+    :param containers: list of containers to run tests between
+    :return: None
+    """
+    assess_internet_connection(client, containers)
+    assess_address_range(client, containers)
+    assess_network_traffic(client, containers)
+
+
+def _assess_container_networking(client, args):
+    """Run _assessment_iteration on all useful combinations of containers
+    :param client: Juju client
+    :param args: Parsed command line arguments
+    :return: None
+    """
+    # Only test the containers we were asked to test
+    if args.machine_type:
+        types = [args.machine_type]
+    else:
+        types = [KVM_MACHINE, LXC_MACHINE]
+
+    mg = MachineGetter(client)
+    hosts = mg.get(count=2)
+
+    for container_type in types:
+        # Test with two containers on the same host
+        containers = mg.get(container_type, count=2)
+        _assessment_iteration(client, containers)
+
+        # Now test with two containers on two different hosts
+        containers = [mg.get(container_type, machine=hosts[0])[0],
+                      mg.get(container_type, machine=hosts[1])[0]]
+        _assessment_iteration(client, containers)
+
+    if args.machine_type is None:
+        # Test with an LXC and a KVM on the same machine
+        containers = [mg.get(LXC_MACHINE, machine=hosts[0])[0],
+                      mg.get(KVM_MACHINE, machine=hosts[0])[0]]
+        _assessment_iteration(client, containers)
+
+        # Test with an LXC and a KVM on different machines
+        containers = [mg.get(LXC_MACHINE, machine=hosts[0])[0],
+                      mg.get(KVM_MACHINE, machine=hosts[1])[0]]
+        _assessment_iteration(client, containers)
+
+
+def assess_container_networking(client, args):
+    """Runs _assess_address_allocation, reboots hosts, repeat.
+    :param client: Juju client
+    :param args: Parsed command line arguments
+    :return: None
+    """
+    _assess_container_networking(client, args)
+    mg = MachineGetter(client)
+    hosts = mg.get(count=2)
+    for host in hosts:
+        ssh(client, host, 'sudo reboot')
+    client.wait_for_started()
+    _assess_container_networking(client, args)
+
+
+def main():
+    args = parse_args()
+    client = EnvJujuClient.by_version(
+        SimpleEnvironment.from_config(args.env),
+        os.path.join(args.juju_bin, 'juju'), args.debug)
+    client.enable_container_address_allocation()
+    update_env(client.env, args.temp_env_name)
+    juju_home = get_juju_home()
+    bootstrap_host = None
+    try:
+        if args.clean_environment:
+            try:
+                clean_environment(client, services_only=True)
+            except Exception as e:
+                logging.exception(e)
+                with temp_bootstrap_env(juju_home, client):
+                    client.bootstrap(args.upload_tools)
+        else:
+            client.destroy_environment()
+            client = make_client(
+                args.juju_bin, args.debug, args.env, args.temp_env_name)
+            with temp_bootstrap_env(juju_home, client):
+                client.bootstrap(args.upload_tools)
+
+        logging.info('Waiting for the bootstrap machine agent to start.')
+        status = client.wait_for_started()
+        mid, data = list(status.iter_machines())[0]
+        bootstrap_host = data['dns-name']
+
+        assess_container_networking(client, args)
+
+    except Exception as e:
+        logging.exception(e)
+        try:
+            if bootstrap_host is None:
+                parse_new_state_server_from_error(e)
+            else:
+                dump_env_logs(client, bootstrap_host, args.logs)
+        except Exception as e:
+            print_now('exception while dumping logs:\n')
+            logging.exception(e)
+        exit(1)
+    finally:
+        if args.clean_environment:
+            clean_environment(client, services_only=True)
+        else:
+            client.destroy_environment()
+
+
+if __name__ == '__main__':
+    main()

=== modified file 'jujupy.py'
--- jujupy.py	2015-09-15 21:51:42 +0000
+++ jujupy.py	2015-09-21 15:30:36 +0000
@@ -173,6 +173,10 @@
             prefix = get_timeout_prefix(timeout, self._timeout_path)
         logging = '--debug' if self.debug else '--show-log'
 
+        # If args is a string, make it a tuple. This makes writing commands
+        # with one argument a bit nicer.
+        if not isinstance(args, tuple) and isinstance(args, basestring):
+            args = (args,)
         # we split the command here so that the caller can control where the -e
         # <env> flag goes.  Everything in the command string is put before the
         # -e flag.
@@ -518,6 +522,34 @@
         else:
             raise Exception('Timed out waiting for services to start.')
 
+    def wait_for(self, thing, search_type, timeout=300):
+        for status in self.status_until(timeout):
+            hit = False
+            miss = False
+
+            for machine, details in status.status['machines'].iteritems():
+                if thing == 'containers':
+                    if 'containers' in details:
+                        hit = True
+                    else:
+                        miss = True
+
+                if thing == 'not-machine-0':
+                    if machine != '0':
+                        hit = True
+                    else:
+                        miss = True
+
+            if search_type == 'none':
+                if not hit:
+                    return
+            elif search_type == 'some':
+                if hit:
+                    return
+            elif search_type == 'all':
+                if not miss:
+                    return
+
     def get_matching_agent_version(self, no_build=False):
         # strip the series and srch from the built version.
         version_parts = self.version.split('-')
@@ -629,6 +661,7 @@
     def __init__(self, *args, **kwargs):
         super(EnvJujuClient26, self).__init__(*args, **kwargs)
         self._use_jes = False
+        self._use_container_address_allocation = False
 
     def enable_jes(self):
         if self._use_jes:
@@ -640,11 +673,16 @@
             self._use_jes = False
             raise JESNotSupported()
 
+    def enable_container_address_allocation(self):
+        self._use_container_address_allocation = True
+
     def _get_feature_flags(self):
         if self.env.config.get('type') == 'cloudsigma':
             yield 'cloudsigma'
         if self._use_jes is True:
             yield 'jes'
+        if self._use_container_address_allocation:
+            yield 'address-allocation'
 
     def _shell_environ(self):
         """Generate a suitable shell environment.
@@ -845,9 +883,10 @@
         status_yaml = yaml_loads(text)
         return cls(status_yaml, text)
 
-    def iter_machines(self, containers=False):
+    def iter_machines(self, containers=False, machines=True):
         for machine_name, machine in sorted(self.status['machines'].items()):
-            yield machine_name, machine
+            if machines:
+                yield machine_name, machine
             if containers:
                 for contained, unit in machine.get('containers', {}).items():
                     yield contained, unit

=== added file 'test_container_networking.py'
--- test_container_networking.py	1970-01-01 00:00:00 +0000
+++ test_container_networking.py	2015-09-21 15:30:36 +0000
@@ -0,0 +1,410 @@
+from mock import (
+    patch,
+    Mock,
+)
+from unittest import TestCase
+from jujupy import (
+    EnvJujuClient,
+    SimpleEnvironment,
+    Status,
+)
+
+import sys
+from StringIO import StringIO
+import assess_container_networking as jcnet
+from copy import deepcopy
+from contextlib import contextmanager
+
+
+class JujuMock(object):
+    """A mock of the parts of the Juju command that the tests hit."""
+
+    def __init__(self):
+        self._call_n = 0
+        self._status = {'services': {},
+                        'machines': {'0': {}}}
+        self.commands = []
+        self.next_machine = 1
+        self._ssh_output = []
+
+    def add_machine(self, name):
+        if name == '':
+            name = str(self.next_machine)
+            self.next_machine += 1
+
+        bits = name.split(':')
+        if len(bits) > 1:
+            # is a container
+            machine = bits[1]
+            container_type = bits[0]
+            if machine not in self._status['machines']:
+                self._status['machines'][machine] = {}
+            if 'containers' not in self._status['machines'][machine]:
+                self._status['machines'][machine]['containers'] = {}
+
+            n = 0
+            c_name = machine + '/' + container_type + '/' + str(n)
+            while c_name in self._status['machines'][machine]['containers']:
+                n += 1
+                c_name = machine + '/' + container_type + '/' + str(n)
+            self._status['machines'][machine]['containers'][c_name] = {}
+        else:
+            # normal machine
+            self._status['machines'][name] = {}
+
+    def add_service(self, name, machine=0, instance_number=1):
+        # We just add a hunk of data captured from a real Juju run and don't
+        # worry about how it doesn't match reality. It is enough to exercise
+        # the code under test.
+        new_service = {
+            'units': {
+                name + '/' + str(instance_number): {
+                    'machine': str(machine),
+                    'public-address': 'noxious-disgust.maas',
+                    'workload-status': {
+                        'current': 'unknown',
+                        'since': '06 Aug 2015 11:39:29+01:00'},
+                    'agent-status': {
+                        'current': 'idle',
+                        'since': '06 Aug 2015 11:39:33+01:00',
+                        'version': '1.25-alpha1.1'},
+                    'agent-state': 'started',
+                    'agent-version': '1.25-alpha1.1'}
+            },
+            'service-status': {
+                'current': 'unknown',
+                'since': '06 Aug 2015 11:39:29+01:00'
+            },
+            'charm': 'cs:trusty/{}-26'.format(name),
+            'relations': {'cluster': [name]},
+            'exposed': False}
+        self._status['services'][name] = deepcopy(new_service)
+
+    def juju(self, cmd, *args, **kwargs):
+        if len(args) == 1:
+            args = args[0]
+        self.commands.append((cmd, args))
+        if cmd == 'remove-service' and args in self._status['services']:
+            del self._status['services'][args]
+
+        elif cmd == 'remove-machine':
+            if args in self._status['machines']:
+                del self._status['machines'][args]
+            else:
+                machine = args.split('/')[0]
+                del self._status['machines'][machine]['containers'][args]
+
+                if len(self._status['machines'][machine]['containers']) == 0:
+                    del self._status['machines'][machine]['containers']
+
+        elif cmd == 'add-machine':
+            self.add_machine(args)
+
+        elif cmd == 'ssh':
+            if len(self._ssh_output) == 0:
+                return ""
+
+            try:
+                return self._ssh_output[self._call_number()]
+            except IndexError:
+                # If we ran out of values, just return the last one
+                return self._ssh_output[-1]
+
+    @contextmanager
+    def juju_async(self, cmd, args):
+        self.juju(cmd, args)
+        yield
+        pass
+
+    def _call_number(self):
+        call_number = self._call_n
+        self._call_n += 1
+        return call_number
+
+    def get_status(self, machine_id=None):
+        return Status(deepcopy(self._status), "")
+
+    def set_status(self, status):
+        self._status = deepcopy(status)
+
+    def set_ssh_output(self, ssh_output):
+        self._ssh_output = deepcopy(ssh_output)
+
+    def reset_calls(self):
+        self._call_n = 0
+
+
+class TestContainerNetworking(TestCase):
+    def setUp(self):
+        self.client = EnvJujuClient(
+            SimpleEnvironment('foo', {'type': 'local'}), '1.234-76', None)
+
+        nil_func = lambda *args, **kw: None
+        self.juju_mock = JujuMock()
+        self.ssh_mock = Mock()
+
+        patches = [
+            patch.object(self.client, 'juju', self.juju_mock.juju),
+            patch.object(self.client, 'get_status', self.juju_mock.get_status),
+            patch.object(self.client, 'juju_async', self.juju_mock.juju_async),
+            patch.object(self.client, 'wait_for', nil_func),
+            patch.object(self.client, 'wait_for_started',
+                         self.juju_mock.get_status),
+            patch.object(self.client, 'get_juju_output', self.juju_mock.juju),
+        ]
+
+        for patcher in patches:
+            patcher.start()
+            self.addCleanup(patcher.stop)
+
+    def assert_ssh(self, args, machine, cmd):
+        self.assertEqual(args, [('ssh', machine, cmd), ])
+
+    def test_parse_args(self):
+        # Test a simple command line that should work
+        cmdline = ['env', 'juju_bin', 'logs', 'ten']
+        args = jcnet.parse_args(cmdline)
+        self.assertEqual(args.machine_type, None)
+        self.assertEqual(args.juju_bin, 'juju_bin')
+        self.assertEqual(args.env, 'env')
+        self.assertEqual(args.logs, 'logs')
+        self.assertEqual(args.temp_env_name, 'ten')
+        self.assertEqual(args.debug, False)
+        self.assertEqual(args.upload_tools, False)
+        self.assertEqual(args.clean_environment, False)
+
+        # check the optional arguments
+        opts = ['--machine-type', jcnet.KVM_MACHINE, '--debug',
+                '--upload-tools', '--clean-environment']
+        args = jcnet.parse_args(cmdline + opts)
+        self.assertEqual(args.machine_type, jcnet.KVM_MACHINE)
+        self.assertEqual(args.debug, True)
+        self.assertEqual(args.upload_tools, True)
+        self.assertEqual(args.clean_environment, True)
+
+        # Now check that we can only set machine_type to kvm or lxc
+        opts = ['--machine-type', jcnet.LXC_MACHINE]
+        args = jcnet.parse_args(cmdline + opts)
+        self.assertEqual(args.machine_type, jcnet.LXC_MACHINE)
+
+        # Set up an error (bob is invalid)
+        opts = ['--machine-type', 'bob']
+
+        # Kill stderr so the test doesn't print anything (calling the tests
+        # with buffer=True would also do this, but I am trying not to be
+        # invasive)
+        sys.stderr = StringIO()
+        self.assertRaises(SystemExit, jcnet.parse_args, cmdline + opts)
+
+        # Restore stderr
+        sys.stderr = sys.__stderr__
+
+    def test_ssh(self):
+        machine, addr = '0', 'foobar'
+        with patch.object(self.client, 'get_juju_output',
+                          autospec=True) as ssh_mock:
+            jcnet.ssh(self.client, machine, addr)
+            self.assertEqual(1, ssh_mock.call_count)
+            self.assert_ssh(ssh_mock.call_args, machine, addr)
+
+    def test_find_network(self):
+        machine, addr = '0', '1.1.1.1'
+        self.assertRaises(ValueError,
+                          jcnet.find_network, self.client, machine, addr)
+
+        self.juju_mock.set_ssh_output([
+            'default via 192.168.0.1 dev eth3\n'
+            '1.1.1.0/24 dev eth3  proto kernel  scope link  src 1.1.1.22',
+        ])
+        self.juju_mock.commands = []
+        jcnet.find_network(self.client, machine, addr)
+        self.assertItemsEqual(self.juju_mock.commands,
+                              [('ssh', (
+                               machine, 'ip route show to match ' + addr))])
+
+    def test_clean_environment(self):
+        self.juju_mock.add_machine('1')
+        self.juju_mock.add_machine('2')
+        self.juju_mock.add_service('name')
+
+        jcnet.clean_environment(self.client)
+        self.assertEqual(3, len(self.juju_mock.commands))
+        self.assertItemsEqual(self.juju_mock.commands, [
+            ('remove-service', 'name'),
+            ('remove-machine', '1'),
+            ('remove-machine', '2'),
+        ])
+
+    def test_clean_environment_with_containers(self):
+        self.juju_mock.add_machine('1')
+        self.juju_mock.add_machine('2')
+        self.juju_mock.add_machine('lxc:0')
+        self.juju_mock.add_machine('kvm:0')
+
+        jcnet.clean_environment(self.client)
+        self.assertEqual(4, len(self.juju_mock.commands))
+        self.assertItemsEqual(self.juju_mock.commands, [
+            ('remove-machine', '0/lxc/0'),
+            ('remove-machine', '0/kvm/0'),
+            ('remove-machine', '1'),
+            ('remove-machine', '2')
+        ])
+
+    def test_clean_environment_just_services(self):
+        self.juju_mock.add_machine('1')
+        self.juju_mock.add_machine('2')
+        self.juju_mock.add_machine('lxc:0')
+        self.juju_mock.add_machine('kvm:0')
+        self.juju_mock.add_service('name')
+
+        jcnet.clean_environment(self.client, True)
+        self.assertEqual(1, len(self.juju_mock.commands))
+        self.assertEqual(self.juju_mock.commands, [
+            ('remove-service', 'name'),
+        ])
+
+    def test_request_machines(self):
+        mg = jcnet.MachineGetter(self.client)
+        # Requesting a machine with none have been allocated will always
+        # return machine 0, which always exists if we have an environment.
+        self.assertEqual(mg.get(), ['0'])
+
+        # Since we haven't done anything with machine 0, we will still get
+        # it back
+        self.assertEqual(mg.get(), ['0'])
+
+        # Adding a service will remove the machine it is on from the
+        # available pool. Requesting a machine will allocate a new one.
+        self.juju_mock.add_service('name', 0)
+        self.assertEqual(mg.get(), ['1'])
+
+        # Request a specific machine without services allocated to it.
+        self.assertEqual(mg.get(machine=1), ['1'])
+
+        # Request a specific machine with services allocated to it
+        self.assertEqual(mg.get(machine=0), ['0'])
+
+        # Requesting a specific machine that doesn't exist returns nothing
+        self.assertEqual(mg.get(machine=2), [])
+
+        # Request more machines than are allocated - a new one appears
+        self.assertEqual(mg.get(count=2), ['1', '2'])
+
+        # Still there!
+        self.assertEqual(mg.get(count=2), ['1', '2'])
+
+        # Now containers (kvm). Ask for a kvm
+        self.assertEqual(mg.get(jcnet.KVM_MACHINE), ['0/kvm/0'])
+
+        # Ask for a kvm on machine 1
+        self.assertEqual(mg.get(jcnet.KVM_MACHINE, '1'), ['1/kvm/0'])
+
+        # Ask for two kvms on machine 1
+        machines = mg.get(jcnet.KVM_MACHINE, '1', count=2)
+        self.assertEqual(machines, ['1/kvm/0', '1/kvm/1'])
+
+        # Ask for a specific kvm
+        self.assertEqual(mg.get(jcnet.KVM_MACHINE, '1', '1'), ['1/kvm/1'])
+
+        # Ask for a specific kvm that doesn't exist
+        self.assertEqual(mg.get(jcnet.KVM_MACHINE, '1', '2'), [])
+
+        # Now containers (lxc). Ask for an lxc
+        self.assertEqual(mg.get(jcnet.LXC_MACHINE), ['0/lxc/0'])
+
+        # Ask for a lxc on machine 1
+        self.assertEqual(mg.get(jcnet.LXC_MACHINE, '1'), ['1/lxc/0'])
+
+        # Ask for two lxcs on machine 1
+        machines = mg.get(jcnet.LXC_MACHINE, '1', count=2)
+        self.assertEqual(machines, ['1/lxc/0', '1/lxc/1'])
+
+        # Ask for a specific lxc
+        self.assertEqual(mg.get(jcnet.LXC_MACHINE, '1', '1'), ['1/lxc/1'])
+
+        # Ask for a specific lxc that doesn't exist
+        self.assertEqual(mg.get(jcnet.LXC_MACHINE, '1', '2'), [])
+
+    def test_test_network_traffic(self):
+        targets = ['0/lxc/0', '0/lxc/1']
+        self.juju_mock.set_status({'machines': {'0': {
+            'containers': {targets[0]: {'dns-name': '0-dns-name'}}}}})
+
+        self.juju_mock.set_ssh_output(['', '', 'hello'])
+        jcnet.assess_network_traffic(self.client, targets)
+
+        self.juju_mock.reset_calls()
+        self.juju_mock.set_ssh_output(['', '', 'fail'])
+        self.assertRaises(ValueError,
+                          jcnet.assess_network_traffic, self.client, targets)
+
+    def test_test_address_range(self):
+        rtn = Mock(**{'r.side_effect': [
+            '192.168.0.2', '192.168.0.3', '192.168.0.4', '192.168.0.5']})
+        with patch('assess_container_networking.socket.gethostbyname',
+                   rtn.r):
+            targets = ['0/lxc/0', '0/lxc/1']
+            self.juju_mock.set_status({'machines': {'0': {
+                'containers': {
+                    targets[0]: {'dns-name': 'lxc0-dns-name'},
+                    targets[1]: {'dns-name': 'lxc1-dns-name'},
+                },
+                'dns-name': '0-dns-name',
+            }}})
+            self.juju_mock.set_ssh_output(['192.168.0.0/24'])
+
+            jcnet.assess_address_range(self.client, targets)
+
+    def test_test_address_range_fail(self):
+        rtn = Mock(**{'r.side_effect': [
+            '192.168.0.2', '192.168.0.3', '192.168.0.4', '192.168.0.5']})
+        with patch('assess_container_networking.socket.gethostbyname',
+                   rtn.r):
+            targets = ['0/lxc/0', '0/lxc/1']
+            self.juju_mock.set_status({'machines': {'0': {
+                'containers': {
+                    targets[0]: {'dns-name': 'lxc0-dns-name'},
+                    targets[1]: {'dns-name': 'lxc1-dns-name'},
+                },
+                'dns-name': '0-dns-name',
+            }}})
+            self.juju_mock.set_ssh_output([
+                '192.168.0.0/24',
+                '192.168.1.0/24',
+                '192.168.2.0/24',
+                '192.168.3.0/24',
+            ])
+
+            self.assertRaises(AssertionError,
+                              jcnet.assess_address_range, self.client, targets)
+
+    def test_test_internet_connection(self):
+        targets = ['0/lxc/0', '0/lxc/1']
+        self.juju_mock.set_status({'machines': {'0': {
+            'containers': {
+                targets[0]: {'dns-name': 'lxc0-dns-name'},
+                targets[1]: {'dns-name': 'lxc1-dns-name'},
+            },
+            'dns-name': '0-dns-name',
+        }}})
+
+        # Can ping default route
+        self.juju_mock.set_ssh_output([
+            'default via 192.168.0.1 dev eth3', 0,
+            'default via 192.168.0.1 dev eth3', 0])
+        jcnet.assess_internet_connection(self.client, targets)
+
+        # Can't ping default route
+        self.juju_mock.set_ssh_output([
+            'default via 192.168.0.1 dev eth3', 1])
+        self.juju_mock.reset_calls()
+        self.assertRaisesRegexp(
+            ValueError, "0/lxc/0 unable to ping default route",
+            jcnet.assess_internet_connection, self.client, targets)
+
+        # Can't find default route
+        self.juju_mock.set_ssh_output(['', 1])
+        self.juju_mock.reset_calls()
+        self.assertRaisesRegexp(
+            ValueError, "Default route not found",
+            jcnet.assess_internet_connection, self.client, targets)