Merge lp:~bcsaller/pyjuju/lxc-omega into lp:pyjuju

[Benjamin Saller]

LXC driven local development story

This branch includes the merge of many preceding branches related to the lxc local dev story. In its current state it can be used to do local lxc deployemnt and testing.

Add the following to your ~/.juju/environments.yaml

juju: environments
default: laptop

laptop
    type: lxc
    dmin-secret: whoknows
    data-dir: /tmp

I recommend adding dmsmasq to your /etc/resolv.conf with:

nameserver 192.168.122.1

However a subsequent branch makes status and ssh find the proper VM for you.

Machines are named via a pattern <username>-<env name>-<servicename>-<unitid>

So for me a wordpress deployment from the local repo would result in a machine at

http://bcsaller-laptop-wordpress-0/

[Gustavo Niemeyer (niemeyer)]

What is the pre-req of this branch? I've already reviewed content in
this branch before.

review: Needs Information

[Gustavo Niemeyer (niemeyer)]

There's still no information on how to review this branch without looking at
changes already reviewed in previous branches.

Please do mention what are the pre-requisite branches in the summary, when it
contains multiple branches.

One detail I could already figure on it:

[1]

[e-mail]
+def lock_manager(path):
+ """Use a file to represent a lock.
(...)
+ # This will raise if the file exists
+ fd = os.open(path, os.O_CREAT | os.O_RDWR | os.O_EXCL)

Please use the existing filesystem lock in Twisted. This version
will explode badly when the second process tries to get into the
mutex region.

review: Needs Fixing

[Gustavo Niemeyer (niemeyer)]

Kapil mentioned the file-lock branch that is missing from the kanban. I've moved this review there.

[Benjamin Saller (bcsaller)]

Uses twisted filelock impl
Uses latest lxc-lib
Examples based around IP address rather than hostname to avoid need to include resolv.conf change on host to address machines (the containers can address each other fine either way)

[Kapil Thangavelu (hazmat)]

Looks good to me, some minors

[0] Please use self.juju_home as the base for the lock

762 + lockfile = FilesystemLock(os.path.join(
763 + "/tmp/", "%s.lock" % container_template_name))

[1] I'd prefer the master template to be namespace'd to the environment, it makes its cleanup automatic (if it gets broken), and allows divergence in series across environments.

[2] There's a few minor style violations, mostly of the over 80 col variety, ie.

+ if self._public_key:
+ args.insert(1, "JUJU_PUBLIC_KEY=%s" % pipes.quote(self._public_key))

review: Approve

[Gustavo Niemeyer (niemeyer)]

There are a few details, but this looks generally nice, thanks.

[2]

+ self.master_template = LXCContainer(
+ container_template_name,
+ origin="ppa",

Shouldn't this be dependent on the source of juju itself?

Please check out the env-origin branch that hopefully Jim is landing tomorrow.
Should be trivial to hook on it.

[3]

-hostname=`curl http://169.254.169.254/latest/meta-data/local-hostname`
-
+hostname=`ip -f inet addr |grep 192.168| grep -v '192\.168\.1\.' | awk '{print $2;}'|cut -f 1 -d '/'`

Please drop the example changes for now. It break the examples in EC2, and consequently our waterfall.

[4]

I haven't followed carefully, but my gut feeling is that the branch is
a bit shallow on testing. Can you please verify if the following logic
is well covered:

- get_master_template
- start using get_master_template
- JUJU_PUBLIC_KEY handling in agent.py
- etc

[5]

+ log.debug("Created master container %s" %
+ container_template_name)

Indentation is off.

[6]

+ try:
+ lockfile.lock()

The locking should be outside the try.. if it fails for whatever
reason, it shouldn't be unlocked.

[7]

Considering that this is using Twisted, shouldn't the lock and unlock
be yielding? I suspect they might be a NOOP at the moment.

[8]

+ if public_key.startswith("'"):
+ public_key = public_key[1:-1]

Please strip it rather than assuming the position blindly. This logic
breaks if e.g. there are spaces at the end of the line.

[9]

+ lockfile = FilesystemLock(os.path.join(
+ "/tmp/", "%s.lock" % container_template_name))

That's a security bug. One should never write any files to a publicly
writable location with a known name.

[10]

After you do these changes, can you please ask for another review from
Kapil. I'll be traveling tomorrow, but if he's happy with the branch
I'm +1 on it too. Thank you!

review: Approve

[Benjamin Saller (bcsaller)]

> [2]
>
> +        self.master_template = LXCContainer(
> +            container_template_name,
> +            origin="ppa",
>
> Shouldn't this be dependent on the source of juju itself?
>
> Please check out the env-origin branch that hopefully Jim is landing tomorrow.
> Should be trivial to hook on it.

I'd like to push this to a later branch, for today this will work with
the ppa and enables local development.

>
> [3]
>
>
> -hostname=`curl http://169.254.169.254/latest/meta-data/local-hostname`
> -
> +hostname=`ip -f inet addr |grep 192.168| grep -v '192\.168\.1\.' | awk '{print $2;}'|cut -f 1 -d '/'`
>
> Please drop the example changes for now. It break the examples in EC2, and consequently our waterfall.
>

Changed.

>
> [4]
>
> I haven't followed carefully, but my gut feeling is that the branch is
> a bit shallow on testing.  Can you please verify if the following logic
> is well covered:
>
> - get_master_template
> - start using get_master_template
> - JUJU_PUBLIC_KEY handling in agent.py
> - etc
>

Ådded tests around _get_container which encapsulates
get_master_template, now _get_master_template and private. This change
marks those as internal and the interface made them a bit simpler to
mock in the tests.

[6,7,9] I've removed the lock file, it was a theoretical issue and
wouldn't prevent manual lxc commands from interfering anyway. Instead
we better namespace the template name and check to see if containers
are built to attempt to avoid other conflicts. This new behavior
should aid in restarts of the agent down the line.

> +        if public_key.startswith("'"):
> +            public_key = public_key[1:-1]
>
> Please strip it rather than assuming the position blindly. This logic
> breaks if e.g. there are spaces at the end of the line.

Done.

>
>
>
> [10]
>
> After you do these changes, can you please ask for another review from
> Kapil.  I'll be traveling tomorrow, but if he's happy with the branch
> I'm +1 on it too.  Thank you!

[Kapil Thangavelu (hazmat)]

post commit review..

[0]
        if not master_template.is_constructed():
            yield self.master_template.create()
            log.debug("Created master container %s" % container_template_name)

That's a bug, there is no instance attribute self.master_template

[1] Lots of over 80 lines, diff to fix (also includes fix for 0)
http://paste.ubuntu.com/699361/

[2] test_containers should be subsumed along with the implmeentation into the
 lib/lxc package.

=== modified file 'examples/oneiric/mysql/hooks/db-relation-joined'
--- examples/oneiric/mysql/hooks/db-relation-joined	2011-09-15 18:56:08 +0000
+++ examples/oneiric/mysql/hooks/db-relation-joined	2011-09-29 06:40:31 +0000
@@ -2,11 +2,7 @@
 
 set -eu # -x for verbose logging to juju debug-log
 
-# XXX The local hostname should be provided by juju itself in a
-# future release, so as to remove the dependency of the charm on the
-# cloud provider (EC2 in this case)
-hostname=`curl http://169.254.169.254/latest/meta-data/local-hostname`
-
+hostname=`hostname`
 # Get the mysql password that was generated by the install hook
 password=`cat /var/lib/juju/mysql.passwd`
 

=== modified file 'examples/oneiric/wordpress/hooks/config-changed'
--- examples/oneiric/wordpress/hooks/config-changed	2011-09-15 18:56:08 +0000
+++ examples/oneiric/wordpress/hooks/config-changed	2011-09-29 06:40:31 +0000
@@ -1,6 +1,6 @@
 #!/bin/bash
 
-hostname=`curl -s http://169.254.169.254/latest/meta-data/public-hostname`
+hostname=`hostname`
 plugin_dir="/var/www/$hostname/wp-content/plugins"
 plugin_url=`config-get plugin`
 

=== modified file 'examples/oneiric/wordpress/hooks/db-relation-changed'
--- examples/oneiric/wordpress/hooks/db-relation-changed	2011-09-15 18:56:08 +0000
+++ examples/oneiric/wordpress/hooks/db-relation-changed	2011-09-29 06:40:31 +0000
@@ -2,12 +2,12 @@
 
 set -eu # -x for verbose logging to juju debug-log
 
+hostname=`hostname`
 UPLOAD_PATH="/var/www/wp-uploads"
 
 # XXX The public hostname should be provided by juju itself in a
 # future release, so as to remove the dependency of the charm on the
 # cloud provider (EC2 in this case)
-hostname=`curl http://169.254.169.254/latest/meta-data/public-hostname`
 juju-log "Retrieved hostname: $hostname"
 
 # Check we haven't already been setup.

=== modified file 'examples/oneiric/wordpress/hooks/install'
--- examples/oneiric/wordpress/hooks/install	2011-09-15 18:56:08 +0000
+++ examples/oneiric/wordpress/hooks/install	2011-09-29 06:40:31 +0000
@@ -3,4 +3,4 @@
 juju-log "Installing wordpress, pwgen via apt-get"
 set -eu # -x for verbose logging to juju debug-log
 
-apt-get -y install wordpress pwgen unzip
+apt-get -y install wordpress pwgen unzip curl

=== modified file 'juju/agents/machine.py'
--- juju/agents/machine.py	2011-09-28 09:48:30 +0000
+++ juju/agents/machine.py	2011-09-29 06:40:31 +0000
@@ -96,7 +96,7 @@
         same charm.
         """
 
-        log.debug("Downloading charm %s...", charm_state.id)
+        log.debug("Downloading charm %s to %s", charm_state.id, self.charms_directory)
         return download_charm(
             self.client, charm_state.id, self.charms_directory)
 
@@ -151,6 +151,7 @@
 
         running = yield deployment.is_running()
         if not running:
+            log.debug("Starting service unit %s", service_unit_name)
             yield deployment.start(
                 self.get_machine_id(), self.client.servers, bundle)
             log.info("Started service unit %s", service_unit_name)

=== modified file 'juju/control/add_unit.py'
--- juju/control/add_unit.py	2011-09-27 02:14:56 +0000
+++ juju/control/add_unit.py	2011-09-29 06:40:31 +0000
@@ -3,6 +3,7 @@
 from twisted.internet.defer import inlineCallbacks
 
 from juju.control.utils import get_environment
+from juju.state.machine import MachineStateManager
 from juju.state.placement import place_unit
 from juju.state.service import ServiceStateManager
 

=== modified file 'juju/lib/lxc/data/juju-create'
--- juju/lib/lxc/data/juju-create	2011-09-27 03:42:49 +0000
+++ juju/lib/lxc/data/juju-create	2011-09-29 06:40:31 +0000
@@ -19,6 +19,7 @@
 
     # By listing the container name first we can expect hostname and
     # hostname -f to return the routable name
+    # note that lxc-clone will overwrite this
     cat <<EOF >/etc/hosts
 127.0.0.1 $JUJU_CONTAINER_NAME localhost
 EOF

=== modified file 'juju/machine/tests/test_unit_deployment.py'
--- juju/machine/tests/test_unit_deployment.py	2011-09-28 09:48:30 +0000
+++ juju/machine/tests/test_unit_deployment.py	2011-09-29 06:40:31 +0000
@@ -3,18 +3,17 @@
 """
 import logging
 import os
-import subprocess
 import sys
 
 from twisted.internet.protocol import ProcessProtocol
 from twisted.internet.defer import inlineCallbacks, succeed
 
 import juju
-
 from juju.charm import get_charm_from_path
 from juju.charm.tests import local_charm_id
 from juju.charm.tests.test_repository import RepositoryTestBase
-from juju.lib.mocker import MATCH
+from juju.lib.lxc import LXCContainer
+from juju.lib.mocker import MATCH, ANY
 from juju.lib.twistutils import get_module_directory
 from juju.machine.unit import UnitMachineDeployment, UnitContainerDeployment
 from juju.machine.errors import UnitDeploymentError
@@ -328,9 +327,11 @@
 upstart_job_sample = '''\
 description "Unit agent for riak/0"
 author "Juju Team <[e-mail]>"
-start on (runlevel [2345] and stopped networking RESULT=ok)
+start on start on filesystem or runlevel [2345]
 stop on runlevel [!2345]
+
 respawn
+
 env JUJU_MACHINE_ID="0"
 env JUJU_HOME="/var/lib/juju"
 env JUJU_ZOOKEEPER="127.0.1.1:2181"
@@ -368,33 +369,17 @@
     def test_get_upstart_job(self):
         upstart_job = self.unit_deploy.get_upstart_unit_job(
             0, "127.0.1.1:2181")
-        self.assertEqual(self.get_normalized(upstart_job), upstart_job_sample)
-
-    @inlineCallbacks
-    def test_is_running(self):
-        name = self.unit_deploy.container_name
-        mock_lxc_ls = self.mocker.mock()
-
-        self.patch(subprocess, "check_output", mock_lxc_ls)
-        mock_lxc_ls(["lxc-ls"])
-        self.mocker.result("%s\n%s" % (name, name))
-
-        mock_lxc_ls(["lxc-ls"])
-        self.mocker.result("%s\n" % name)
-
-        mock_lxc_ls(["lxc-ls"])
-        self.mocker.result("abc-0\n")
-        self.mocker.replay()
-
-        self.assertTrue((yield self.unit_deploy.is_running()))
-        self.assertFalse((yield self.unit_deploy.is_running()))
-        self.assertFalse((yield self.unit_deploy.is_running()))
+        job = self.get_normalized(upstart_job)
+        self.assertIn('JUJU_ZOOKEEPER="127.0.1.1:2181"', job)
+        self.assertIn('JUJU_MACHINE_ID="0"', job)
+        self.assertIn('JUJU_UNIT_NAME="riak/0"', job)
 
     @inlineCallbacks
     def test_destroy(self):
         mock_container = self.mocker.patch(self.unit_deploy.container)
         mock_container.destroy()
         self.mocker.replay()
+
         yield self.unit_deploy.destroy()
 
         output = self.output.getvalue()
@@ -403,19 +388,36 @@
 
     @inlineCallbacks
     def test_start(self):
-        self.unit_deploy.directory = rootfs = self.makeDir()
+        container = LXCContainer(self.unit_name, None, None, None)
+        rootfs = self.makeDir()
+        env = dict(os.environ)
+        env["JUJU_PUBLIC_KEY"] = "dsa ..."
+        self.change_environment(**env)
+
+        mock_deploy = self.mocker.patch(self.unit_deploy)
+        # this minimally validates that we are also called with the
+        # expect public key
+        mock_deploy._get_container(ANY, ANY, ANY, env["JUJU_PUBLIC_KEY"])
+        self.mocker.result((container, rootfs))
+
+        mock_container = self.mocker.patch(container)
+        mock_container.run()
+
+        self.mocker.replay()
+
+        self.unit_deploy.directory = rootfs
         os.makedirs(os.path.join(rootfs, "etc", "init"))
-        mock_container = self.mocker.patch(self.unit_deploy.container)
-        mock_container.run()
-        self.mocker.replay()
 
         yield self.unit_deploy.start("0", "127.0.1.1:2181", self.bundle)
 
         # Verify the upstart job
-        upstart_agent_name = "%s-unit-agent" % self.unit_name.replace("/", "-")
+        upstart_agent_name = "%s-unit-agent.conf" % self.unit_name.replace("/", "-")
         content = open(
             os.path.join(rootfs, "etc", "init", upstart_agent_name)).read()
-        self.assertEqual(self.get_normalized(content), upstart_job_sample)
+        job = self.get_normalized(content)
+        self.assertIn('JUJU_ZOOKEEPER="127.0.1.1:2181"', job)
+        self.assertIn('JUJU_MACHINE_ID="0"', job)
+        self.assertIn('JUJU_UNIT_NAME="riak/0"', job)
 
         # Verify the symlink exists
         self.assertTrue(os.path.lexists(os.path.join(
@@ -432,3 +434,24 @@
         self.assertIn("Charm extracted into container", output)
         self.assertIn("Started container for %s" % self.unit_deploy.unit_name,
                       output)
+
+    @inlineCallbacks
+    def test_get_container(self):
+        rootfs = self.makeDir()
+        container = LXCContainer(self.unit_name, None, None, None)
+
+        mock_deploy = self.mocker.patch(self.unit_deploy)
+        mock_deploy._get_master_template(ANY, ANY, ANY)
+        self.mocker.result(container)
+
+        mock_container = self.mocker.patch(container)
+        mock_container.clone(ANY)
+        self.mocker.result(container)
+
+        self.mocker.replay()
+
+        container, rootfs = yield self.unit_deploy._get_container("0", "127.0.0.1:2181", None, "dsa...")
+
+        output = self.output.getvalue()
+        self.assertIn("Container created for %s" % self.unit_deploy.unit_name , output)
+

=== modified file 'juju/machine/unit.py'
--- juju/machine/unit.py	2011-09-23 20:32:51 +0000
+++ juju/machine/unit.py	2011-09-29 06:40:31 +0000
@@ -13,8 +13,7 @@
 
 from juju.charm.bundle import CharmBundle
 from juju.lib.twistutils import get_module_directory
-from juju.lib.lxc import LXCContainer
-from juju.providers.lxc.container import get_containers
+from juju.lib.lxc import LXCContainer, get_containers, LXCError
 
 from .errors import UnitDeploymentError
 
@@ -155,7 +154,7 @@
 description "Unit agent for %(JUJU_UNIT_NAME)s"
 author "Juju Team <[e-mail]>"
 
-start on (runlevel [2345] and stopped networking RESULT=ok)
+start on start on filesystem or runlevel [2345]
 stop on runlevel [!2345]
 
 respawn
@@ -191,15 +190,8 @@
         self._unit_namespace = os.environ.get("JUJU_UNIT_NS")
         assert self._unit_namespace is not None, "Required unit ns not found"
 
-        self.container = LXCContainer(
-            self.container_name,
-            {"JUJU_ORIGIN": "ppa",
-             "JUJU_SOURCE": "ppa:juju/ppa"},
-            #console_log=os.path.join(self.directory, "console.log"),
-            #debug_log=os.path.join(self.directory, "container.log"))
-            )
-        self.directory = self.container.rootfs
         self.pid_file = None
+        self.container = LXCContainer(self.container_name, None, None, None)
 
     @property
     def container_name(self):
@@ -251,32 +243,63 @@
             os.makedirs(host_unit_dir)
 
     @inlineCallbacks
+    def _get_master_template(self, machine_id, zookeeper_hosts, public_key):
+        container_template_name = "%s-%s-template" % (self._unit_namespace, machine_id)
+
+        master_template = LXCContainer(container_template_name, origin="ppa", public_key=public_key)
+
+        if not master_template.is_constructed():
+            yield self.master_template.create()
+            log.debug("Created master container %s" % container_template_name)
+
+        # it wasn't constructed and we couldn't construct it
+        if not master_template.is_constructed():
+            raise LXCError("Unable to create master container")
+
+        returnValue(master_template)
+
+    @inlineCallbacks
+    def _get_container(self, machine_id, zookeeper_hosts, bundle, public_key):
+        master_template = yield self._get_master_template(machine_id, zookeeper_hosts, public_key)
+        log.info(
+            "Creating container %s...", os.path.basename(self.directory))
+
+        container = yield master_template.clone(self.container_name)
+        directory = container.rootfs
+        log.info("Container created for %s" % self.unit_name)
+        returnValue((container, directory))
+
+    @inlineCallbacks
     def start(self, machine_id, zookeeper_hosts, bundle):
         """Start the unit.
 
         Creates and starts an lxc container for the unit.
         """
-        if not os.path.exists(self.directory):
-            self.container.configuration["JUJU_ZOOKEEPER"] = zookeeper_hosts
-            log.info(
-                "Creating container %s...", os.path.basename(self.directory))
-            yield self.container.create()
-            log.info("Container created for %s" % self.unit_name)
+        # remove any quoting around the key
+        public_key = os.environ.get("JUJU_PUBLIC_KEY", "")
+        public_key = public_key.strip("'\"")
 
+        # Build a template container that can be cloned in deploy
+        # we leave the loosely initialized self.container in place for
+        # the class as thats all we need for methods other than start.
+        container, directory = yield self._get_container(machine_id,
+                                                         zookeeper_hosts,
+                                                         bundle,
+                                                         public_key)
         # Create state directories for unit in the container
         self.setup_directories()
 
         # Extract the charm bundle
         charm_path = os.path.join(
-            self.directory, "var", "lib", "juju", "units",
+            directory, "var", "lib", "juju", "units",
             self.unit_path_name, "charm")
         bundle.extract_to(charm_path)
         log.debug("Charm extracted into container")
 
         # Write upstart file for the agent into the container
         upstart_path = os.path.join(
-            self.directory, "etc", "init",
-            "%s-unit-agent" % self.unit_path_name)
+            directory, "etc", "init",
+            "%s-unit-agent.conf" % self.unit_path_name)
         with open(upstart_path, "w") as fh:
             fh.write(self.get_upstart_unit_job(machine_id, zookeeper_hosts))
 
@@ -285,12 +308,12 @@
             self.juju_home, "units", self.unit_path_name, "unit.log")
         if not os.path.lexists(unit_log_path_host):
             os.symlink(
-                os.path.join(self.directory, "var", "log", "juju",
+                os.path.join(directory, "var", "log", "juju",
                              "unit-%s.log" % self.unit_path_name),
                 unit_log_path_host)
 
         log.debug("Starting container...")
-        yield self.container.run()
+        yield container.run()
         log.info("Started container for %s" % self.unit_name)
 
     @inlineCallbacks
@@ -308,7 +331,10 @@
         # TODO: container running may not imply agent running.  the
         # pid file has the pid from the container, we need a container
         # pid -> host pid mapping to query status from the machine agent.
-        # alternatively querying zookeeper for the unit agent presence node.
+        # alternatively querying zookeeper for the unit agent presence
+        # node.
+        if not self.container:
+            returnValue(False)
         container_map = yield get_containers(
             prefix=self.container.container_name)
         returnValue(container_map.get(self.container.container_name, False))

=== modified file 'juju/providers/lxc/__init__.py'
--- juju/providers/lxc/__init__.py	2011-09-27 02:51:59 +0000
+++ juju/providers/lxc/__init__.py	2011-09-29 06:40:31 +0000
@@ -6,13 +6,13 @@
 from txzookeeper import ZookeeperClient
 
 from juju.errors import ProviderError, EnvironmentNotFound
-from juju.lib.lxc import LXCContainer
+from juju.lib.lxc import LXCContainer, get_containers
 from juju.lib.zk import Zookeeper
 from juju.providers.common.base import MachineProviderBase
 from juju.providers.common.files import FileStorage
 from juju.providers.common.connect import ZookeeperConnect
+from juju.providers.common.utils import get_user_authorized_keys
 from juju.providers.lxc.agent import ManagedMachineAgent
-from juju.providers.lxc.container import get_containers
 from juju.providers.lxc.machine import LocalMachine
 from juju.providers.lxc.network import Network
 from juju.providers.lxc.pkg import check_packages
@@ -107,6 +107,13 @@
         hierarchy = StateHierarchy(client, admin_identity, "local", "lxc")
         yield hierarchy.initialize()
 
+        # Store user credentials from the running user
+        try:
+            public_key = get_user_authorized_keys(self.config)
+            public_key = public_key.strip()
+        except LookupError, e:
+            raise ProviderError(str(e))
+
         # Startup the machine agent
         log.info("Starting machine agent...")
         pid_file = os.path.join(self._directory, "machine-agent.pid")
@@ -115,7 +122,9 @@
                                     zookeeper_hosts=zookeeper.address,
                                     machine_id="0",
                                     juju_directory=self._directory,
-                                    log_file=log_file)
+                                    log_file=log_file,
+                                    juju_unit_namespace=self._qualified_name,
+                                    public_key=public_key)
         yield agent.start()
 
         log.info("Environment bootstrapped")
@@ -150,7 +159,7 @@
     def _destroy_containers(self):
         container_map = yield get_containers(self._qualified_name)
         for container_name in container_map:
-            container = LXCContainer(container_name)
+            container = LXCContainer(container_name, None, None, None)
             if container_map[container.container_name]:
                 yield container.stop()
             yield container.destroy()

=== modified file 'juju/providers/lxc/agent.py'
--- juju/providers/lxc/agent.py	2011-09-23 20:32:51 +0000
+++ juju/providers/lxc/agent.py	2011-09-29 06:40:31 +0000
@@ -1,5 +1,6 @@
 import errno
 import os
+import pipes
 import subprocess
 import sys
 
@@ -13,7 +14,9 @@
 
     def __init__(
         self, pid_file, zookeeper_hosts=None, machine_id="0",
-        log_file=None, juju_directory="/var/lib/juju", juju_unit_namespace=""):
+        log_file=None, juju_directory="/var/lib/juju",
+        juju_unit_namespace="",
+        public_key=None):
         """
         :param pid_file: Path to file used to store process id.
         :param machine_id: machine id for the machine agent (0 in lxc provider)
@@ -24,6 +27,8 @@
                a known a prefix to allow for multiple users and multiple
                environments to create containers. The namespace should be
                unique per user and per environment.
+        :param public_key: An SSH public key (string) that will be
+               used in the container for access.
         """
         self._pid_file = pid_file
         self._machine_id = machine_id
@@ -31,6 +36,7 @@
         self._juju_directory = juju_directory
         self._juju_unit_namespace = juju_unit_namespace
         self._log_file = log_file
+        self._public_key = public_key
 
     @inlineCallbacks
     def start(self):
@@ -52,6 +58,10 @@
                 sys.executable, "-m", self.agent_module,
                 "-n", "--pidfile", self._pid_file,
                 "--logfile", self._log_file]
+
+        if self._public_key:
+            args.insert(1, "JUJU_PUBLIC_KEY=%s" % pipes.quote(self._public_key))
+
         yield deferToThread(subprocess.check_call, args)
 
     @inlineCallbacks

=== removed file 'juju/providers/lxc/container.py'
--- juju/providers/lxc/container.py	2011-09-23 17:25:37 +0000
+++ juju/providers/lxc/container.py	1970-01-01 00:00:00 +0000
@@ -1,31 +0,0 @@
-from twisted.internet.threads import deferToThread
-
-import subprocess
-
-
-def get_containers(prefix=None):
-    """Return a dictionary of containers key names to boolean values.
-
-    A True value indicates the container is currently running. A False value
-    indicates the container is defined but in the stopped state.
-
-    :param prefix: Optionally specify a prefix that the container should
-          match any returned containers.
-    """
-    return deferToThread(_list_containers, prefix)
-
-
-def _list_containers(prefix):
-    output = subprocess.check_output(["lxc-ls"])
-    containers = {}
-    for i in filter(None, output.split("\n")):
-        if i in containers:
-            containers[i] = True
-        else:
-            containers[i] = False
-
-    if prefix:
-        remove = [k for k in containers.keys() if not k.startswith(prefix)]
-        map(containers.pop, remove)
-
-    return containers

=== modified file 'juju/providers/lxc/tests/test_container.py'
--- juju/providers/lxc/tests/test_container.py	2011-09-18 18:58:46 +0000
+++ juju/providers/lxc/tests/test_container.py	2011-09-29 06:40:31 +0000
@@ -3,8 +3,8 @@
 from twisted.internet.defer import inlineCallbacks
 
 from juju.lib.testing import TestCase
-from juju.providers.lxc.container import get_containers
-
+from juju.lib.lxc import get_containers
+from juju.lib import lxc
 lxc_output_sample = "\
 calendarserver\ncaprica\ngemini\nreconnoiter\nvirgo\ncalendarserver\n"
 
@@ -14,12 +14,12 @@
     @inlineCallbacks
     def test_get_containers(self):
         lxc_ls_mock = self.mocker.mock()
-        self.patch(subprocess, "check_output", lxc_ls_mock)
+        self.patch(lxc, "_cmd", lxc_ls_mock)
         lxc_ls_mock(["lxc-ls"])
-        self.mocker.result(lxc_output_sample)
+        self.mocker.result((0, lxc_output_sample))
         self.mocker.replay()
 
-        container_map = yield get_containers()
+        container_map = yield get_containers(None)
         self.assertEqual(
             dict(caprica=False, gemini=False, reconnoiter=False, virgo=False,
                  calendarserver=True),
@@ -28,9 +28,9 @@
     @inlineCallbacks
     def test_get_containers_with_prefix(self):
         lxc_ls_mock = self.mocker.mock()
-        self.patch(subprocess, "check_output", lxc_ls_mock)
+        self.patch(lxc, "_cmd", lxc_ls_mock)
         lxc_ls_mock(["lxc-ls"])
-        self.mocker.result(lxc_output_sample)
+        self.mocker.result((0, lxc_output_sample))
         self.mocker.replay()
 
         container_map = yield get_containers("ca")

=== modified file 'juju/providers/lxc/tests/test_provider.py'
--- juju/providers/lxc/tests/test_provider.py	2011-09-27 02:51:59 +0000
+++ juju/providers/lxc/tests/test_provider.py	2011-09-29 06:40:31 +0000
@@ -11,6 +11,7 @@
 
 from juju.errors import ProviderError, EnvironmentNotFound
 from juju.lib.lxc import LXCContainer
+from juju.lib import lxc as lxc_lib
 from juju.lib.testing import TestCase
 from juju.tests.common import get_test_zookeeper_address
 from juju.lib.zk import Zookeeper
@@ -43,8 +44,7 @@
 
     def test_get_placement_policy(self):
         """Lxc provider only supports local placement."""
-        self.assertEqual(
-            self.provider.get_placement_policy(), "local")
+        self.assertEqual(self.provider.get_placement_policy(), "local")
         provider = MachineProvider(
             "test", {"placement": "unassigned",
                      "data-dir": self.makeDir()})
@@ -122,12 +122,12 @@
         user_name = pwd.getpwuid(os.getuid()).pw_name
         # Mock container destruction, including stopping running contianers.
         lxc_ls_mock = self.mocker.mock()
-        self.patch(subprocess, "check_output", lxc_ls_mock)
+        self.patch(lxc_lib, "_cmd", lxc_ls_mock)
         lxc_ls_mock(["lxc-ls"])
-        self.mocker.result(
-            "%(name)s-local-test-unit-1\n%(name)s-local-test-unit-2\n"
-            "%(name)s-local-test-unit-3\n%(name)s-local-test-unit-1\n"
-            "%(name)s-local-test-unit-2\n" % dict(name=user_name))
+        self.mocker.result((0,
+                            "%(name)s-local-test-unit-1\n%(name)s-local-test-unit-2\n"
+                            "%(name)s-local-test-unit-3\n%(name)s-local-test-unit-1\n"
+                            "%(name)s-local-test-unit-2\n" % dict(name=user_name)))
 
         mock_container = self.mocker.patch(LXCContainer)
         mock_container.stop()

=== modified file 'misc/bash_completion.d/juju'
--- misc/bash_completion.d/juju	2011-09-15 18:58:54 +0000
+++ misc/bash_completion.d/juju	2011-09-29 06:40:31 +0000
@@ -6,7 +6,7 @@
 
     COMPREPLY=()
     cur=${COMP_WORDS[COMP_CWORD]}
-    cmds='add-relation add-unit bootstrap debug-hooks debug-log deploy destroy-service expose open-tunnel remove-relation remove-unit resolved set shutdown ssh status terminate-machine unexpose upgrade-charm'
+    cmds='add-relation add-unit bootstrap debug-hooks debug-log deploy destroy-environment destroy-service expose open-tunnel remove-relation remove-unit resolved set ssh status terminate-machine unexpose upgrade-charm'
     globalOpts=( -h --verbose -v --log-file)
 
     # do ordinary expansion if we are anywhere after a -- argument
@@ -47,7 +47,7 @@
     optEnums=( )
     fixedWords=( )
     case "$cmd" in
-        add-relation|debug-hooks|destory-service|expose-service|unexpose-service|open-tunnel|remove-relation|remove-unit|set|ssh|terminate-machine)
+        add-relation|debug-hooks|destroy-environment|destory-service|expose-service|unexpose-service|open-tunnel|remove-relation|remove-unit|set|ssh|terminate-machine)
             cmdOpts=( --environment )
             ;;
         bootstrap)

=== modified file 'setup.py'
--- setup.py	2011-09-15 19:07:58 +0000
+++ setup.py	2011-09-29 06:40:31 +0000
@@ -30,6 +30,7 @@
     url="https://launchpad.net/juju",
     license="GPL",
     packages=find_packages(),
+    package_data={"juju.lib.lxc": ["data/juju-create", "data/lxc.conf"]},
     scripts=glob("./bin/*"),
     classifiers=[
         "Development Status :: 4 - Beta",