Merge lp:~jimbaker/pyjuju/env-origin into lp:pyjuju

[Jim Baker]

Implements optional environment settings juju-origin (which replaces juju-branch).

Does not implement juju-version or support for deb repositories, per attached bug.

Functionally tested for not setting juju-origin, setting it to the standard PPA (the only one supported now), and LP branches.

[Gustavo Niemeyer (niemeyer)]

[1]

The documentation must be changed accordingly.

[2]

Please drop the version support. Without pinning, this doesn't guarantee that
the version will stay as suggested, and we shouldn't be looking into pinning
right now.

[3]

238 + # XXX needed for python-txzookeeper dependency
239 return ["ppa:juju/pkgs"]

That's a *major* one. Why? Please synchronize with SpamapS and ensure that's not
needed anymore.

[4]

213 + # Classify origins so the proper install script is later selected
214 + if origin is None:
215 + self._origin_type = _DISTRO
216 + elif (origin.startswith("deb ") or \
217 + origin.startswith("ppa:") or \
218 + origin.startswith("http:") or \
219 + origin.startswith("https:")):
220 + self._origin_type = _DEBIAN
221 + elif origin.startswith("lp:"):
222 + self._origin_type = _BRANCH

Why is this changing? The original version of this logic was significantly
simpler and cleaner, and seems to support all the cases we're trying to
support right now.

[5]

I'm _very_ concerned about your summary. The support for other package sources
besides branches is precisely what this branch is introducing, and you seem to
claim this wasn't tested at all.

Please do make sure this stuff is tested.

review: Needs Fixing

[William Reade (fwereade)]

[0]

+ :type branch: str or None

two of these, should be "origin" and "version" instead of "branch"

[1]

+ version=str(config.get("juju-version")))

Won't that lead to version being "None" rather than None? If so, maybe add a test for that path.

Otherwise looks very nice, +1.

review: Approve

[Gustavo Niemeyer (niemeyer)]

[6]

+ PPA (use a `ppa:` prefix, eg `ppa:juju/pkgs`). Otherwise, the
+ utilized source will be based on the machine running the
+ bootstrap command (distribution, PPA, or branch).

s/eg/e.g./

In the following sentence, please use this wording:

  Otherwise, juju will attempt to detect the correct origin based on
  its run location and the installed juju package.

Also this should be fixed according to the rest of this review.

[7]

+ def _parse_policy(self, output):
(...)
+ def get_default_origin(self):
(...)

Neither of these methods seem to need any information from "self",
which means they are actually independent functions rather than
methods.

[8]

+ if k != "juju":
+ # Sanity check: no record at all, the only possibility is that
+ # this being run as a branch
+ return "lp:juju"
+ for k, v in parse:
+ if k == "Installed" and v == "(none)":
+ return _PPA
+ if k == "Version table":
+ break
+ for _, version_info in parse:
+ if "ppa.launchpad.net" in version_info:
+ return _PPA

As we talked yesterday online, all of this logic is idempotent, and should
be part of a single function, called parse_juju_origin(data) or similar,
and not part of the same function that interacts with the system (calling
"apt-cache policy juju").

[9]

+cat <<EOF
+juju:
+ Installed: (none)
+ Candidate: good-magic-1.0
+ Version table:
+ *** good-magic-1.0
+ 500 http://us.archive.ubuntu.com/ubuntu/ natty/main amd64 Packages
+ 100 /var/lib/dpkg/status
+EOF

This is testing the idempotent parse_juju_origin(data) function. Put the
data inside the test file, and test the function trivially:

    def test_foo(self):
        origin, source = parse_juju_policy(data)
 self.assertEquals(origin, "whatever")
 self.assertEquals(source, "whatever else")

It's really that simple! :-)

Kill most of these apt-cache dummies, please. You only need one test
of these, to ensure that the system interaction is really working as
it should. All the variations should be tested in the above fashion.

[10]

+ elif origin.startswith("ppa:"):
+ cloud_init.set_juju_source(ppa=True)

This is bogus. This should be `if origin == "ppa":`, otherwise you're
promising something and delivering something else. Please fix the
branch and documentation.

[11]

+class OriginPolicyTestMixin(object):
+
+ def setup_origin_policy(self, name, is_branch=False):
(...)
+class EC2MachineLaunchTest(EC2TestMixin, EC2MachineLaunchMixin,
+ OriginPolicyTestMixin, TestCase):

Ouch!

Please drop the mixin.. this is a single simple function,
so let's handle it as such.

[12]

--- juju/providers/ec2/tests/data/launch_cloud_init_branch 1970-01-01 00:00:00 +0000

The number of these files and the way we copy them blindly on every
minor detail we want to test is pretty sad. That's fine for this
branch, but it'd be awesome to figure a better way to test this in
the future.

review: Needs Fixing

[Gustavo Niemeyer (niemeyer)]

[13]

+ juju-origin:
+ The origin of juju deployed to machines within an environment
+ is set by this option. This option can be set to a bzr branch
+ pushed to Launchpad (use a `lp:` prefix, e.g., `lp:juju`); to
+ the PPA by using `ppa`; or to the juju distribution by using
+ `distro`. If this option cannot be parsed, the distribution is
+ used.

This sentence is not reading correctly in a few ways (e.g. the
juju _origin_ is not really deployed, and there's no such thing
as a juju distribution.. it's the Ubuntu distribution).

Please use this:

    juju-origin:
        Defines where juju should be obtained for installing in
 machines. Can be set to a "lp..." branch url, to "ppa" for
 getting packages from the official juju PPA, or to "distro"
 for using packages from the official Ubuntu repositories.

[14]

Jim, please read this excerpt from our conversation again, *carefully*:

<niemeyer> 1) Grab the installed version from the "Installed:" line
<niemeyer> 2) If this is "(none)" return "branch"
<niemeyer> 3) Otherwise, store the version in a variable, and keep parsing
<niemeyer> 4) Find the version table, and find the installed version
<niemeyer> 5) For each line in the given version, if it contains "ppa.launchpad.net/juju/pkgs", return "ppa"
<niemeyer> 6) If you reach the end of the installed version, return "distro"

Compare to what is in the branch right now.

[15]

+ if k == "Installed" and v == "(none)":
+ return "ppa", None

Compare with the summary above.

If there's no juju package installed in the system, it means the
user is not using the ppa.

[16]

+ for _, version_info in parse:
+ if "ppa.launchpad.net" in version_info:
+ return "ppa", None

Compare with the summary above.

Which version is this? The existence of _any_ version that comes from
a PPA in the apt cache is not an indication that the _installed_
version comes from a PPA. The indication that it comes from _a_ ppa
is not an indication that it comes from _our_ ppa either.

[17]

+ def test_distro_not_installed(self):
+ data = (
+ "juju:\n"
+ " Installed: (none)\n"
+ " Candidate: good-magic-1.0\n"

Much better, thank you.

review: Needs Fixing

[Jim Baker (jimbaker)]

I do not see my reply that I emailed to the lp gateway last night, so I'm reposting:

I have made the desired changes, merged trunk, and retested against EC2 for
the various combinations (other than distro, of course).

Please note that some of the branches that were recently merged into trunk
have broken it, as indicated here: http://wtf.labix.org/. I will certainly
remerge trunk as soon as that's fixed.

- Jim

On Wed, Sep 28, 2011 at 7:18 PM, Gustavo Niemeyer <email address hidden>wrote:

> Review: Needs Fixing
>
> [13]
>
> + juju-origin:
> + The origin of juju deployed to machines within an environment
> + is set by this option. This option can be set to a bzr branch
> + pushed to Launchpad (use a `lp:` prefix, e.g., `lp:juju`); to
> + the PPA by using `ppa`; or to the juju distribution by using
> + `distro`. If this option cannot be parsed, the distribution is
> + used.
>
> This sentence is not reading correctly in a few ways (e.g. the
> juju _origin_ is not really deployed, and there's no such thing
> as a juju distribution.. it's the Ubuntu distribution).
>
> Please use this:
>
> juju-origin:
> Defines where juju should be obtained for installing in
> machines. Can be set to a "lp..." branch url, to "ppa" for
> getting packages from the official juju PPA, or to "distro"
> for using packages from the official Ubuntu repositories.
>
>
Used this text as suggested.

>
> [14]
>
> Jim, please read this excerpt from our conversation again, *carefully*:
>
> <niemeyer> 1) Grab the installed version from the "Installed:" line
> <niemeyer> 2) If this is "(none)" return "branch"
> <niemeyer> 3) Otherwise, store the version in a variable, and keep parsing
> <niemeyer> 4) Find the version table, and find the installed version
> <niemeyer> 5) For each line in the given version, if it contains "
> ppa.launchpad.net/juju/pkgs", return "ppa"
> <niemeyer> 6) If you reach the end of the installed version, return
> "distro"
>
> Compare to what is in the branch right now.
>
>
> [15]
>
> + if k == "Installed" and v == "(none)":
> + return "ppa", None
>
> Compare with the summary above.
>
> If there's no juju package installed in the system, it means the
> user is not using the ppa.
>

Changed the logic above and the impacted test

>
>
> [16]
>
> + for _, version_info in parse:
> + if "ppa.launchpad.net" in version_info:
> + return "ppa", None
>
> Compare with the summary above.
>
> Which version is this? The existence of _any_ version that comes from
> a PPA in the apt cache is not an indication that the _installed_
> version comes from a PPA. The indication that it comes from _a_ ppa
> is not an indication that it comes from _our_ ppa either.
>
>
Got it. It's now using the precise test as you specified earlier. Sorry
about that.

>
> [17]
>
> + def test_distro_not_installed(self):
> + data = (
> + "juju:\n"
> + " Installed: (none)\n"
> + " Candidate: good-magic-1.0\n"
>
> Much better, thank you.
>
>
Thanks!

[Gustavo Niemeyer (niemeyer)]

<jimbaker> niemeyer, hi
<niemeyer> jimbaker: Hi
<niemeyer> jimbaker: Please re-read points 3, 4 and 5
<niemeyer> jimbaker: and compare to the implementation
<jimbaker> niemeyer, ok
<niemeyer> jimbaker: Is there anything missing there?
<niemeyer> jimbaker: Maybe I'm just missing.. where's that: 3) Otherwise, store the version in a variable, and keep parsing
<jimbaker> niemeyer, #3 - python-txzookeeper dependency is no longer relevant; #4, it uses the original implementation; #5, there is no attempt to test deb repositories, since they are no longer supported
<niemeyer> jimbaker: 4) Find the version table, and find the installed version
<niemeyer> ?
* ejat has quit (Remote host closed the connection)
<jimbaker> niemeyer, ok, i'm referring to the review points. you are referring to a list from irc
<niemeyer> jimbaker: yeah
<jimbaker> looking at that now
<jimbaker> niemeyer, ok, i believe what you are saying here is that 6) refers to the installed version, which was stored earlier in 3), is that correct?
<niemeyer> 1) Grab the installed version from the "Installed:" line
<niemeyer> 2) If this is "(none)" return "branch"
<niemeyer> 3) Otherwise, store the version in a variable, and keep parsing
<niemeyer> 4) Find the version table, and find the installed version
<niemeyer> jimbaker: Is there any ambiguity here?
<jimbaker> niemeyer, correct, there is no ambiguity. the version table has more information than i have assumed, and this parse needs to take it in account

review: Needs Fixing

[Jim Baker (jimbaker)]

> <niemeyer> 1) Grab the installed version from the "Installed:" line
> <niemeyer> 2) If this is "(none)" return "branch"
> <niemeyer> 3) Otherwise, store the version in a variable, and keep parsing
> <niemeyer> 4) Find the version table, and find the installed version
> <niemeyer> jimbaker: Is there any ambiguity here?
> <jimbaker> niemeyer, correct, there is no ambiguity. the version table has
> more information than i have assumed, and this parse needs to take it in
> account

The algorithm in parse_juju_origin, and its helper function, has been reworked as described.

[Gustavo Niemeyer (niemeyer)]

[18]

345 + " *** 0.5+bzr366-1juju1~natty1 0\n"
346 + " 500 http://ppa.launchpad.net/juju/pkgs/ubuntu/ "
347 + "natty/main amd64 Packages\n"
348 + " *** 0.5+bzr356-1juju1~natty1 0\n"
349 + " 500 http://ppa.launchpad.net/juju/pkgs/ubuntu/ "

Your multiple versions test has the PPA both in the installed and in the
non-installed version, so it's not really verifying the bug that you have
just fixed.

Change the URL in the non-installed version in this test so it doesn't come
from the PPA so that we ensure that it's really testing the installed version.
Also add a non-installed version that comes from the ppa in the distro test,
to ensure it's not being considered.

Please ensure tests pass with this and that a real interaction passes as well
before merging.

review: Approve

[Gustavo Niemeyer (niemeyer)]

I'm taking this over after a conversation on IRC.

Pushed the following changes onto lp:~niemeyer/juju/env-origin, reviewed by Kapil:

- Implementation redone completely.
- Do not crash on missing apt-cache.
- Exported and tested get_default_origin.
- Tests tweaked to explore edge cases.

=== modified file 'docs/source/provider-configuration-ec2.rst'
--- docs/source/provider-configuration-ec2.rst	2011-09-15 17:41:55 +0000
+++ docs/source/provider-configuration-ec2.rst	2011-09-29 23:50:28 +0000
@@ -24,9 +24,15 @@
         An S3 bucket unique to the environment, where some runtime metadata and
         charms are stored.
 
-    juju-branch:
-        Allow a juju bzr branch to be utilized for all machines within an
-        environment.
+    juju-origin:
+        Defines where juju should be obtained for installing in
+        machines. Can be set to a "lp:..." branch url, to "ppa" for
+        getting packages from the official juju PPA, or to "distro"
+        for using packages from the official Ubuntu repositories.
+
+        If this option is not set, juju will attempt to detect the
+        correct origin based on its run location and the installed
+        juju package.
 
     default-instance-type:
         The instance type to be used for machines launched within the juju

=== modified file 'juju/providers/common/cloudinit.py'
--- juju/providers/common/cloudinit.py	2011-09-23 20:35:02 +0000
+++ juju/providers/common/cloudinit.py	2011-09-29 23:50:28 +0000
@@ -1,3 +1,5 @@
+from subprocess import Popen, PIPE
+
 from juju.errors import CloudInitError
 from juju.providers.common.utils import format_cloud_init
 from juju.state.auth import make_identity
@@ -54,6 +56,80 @@
         % zookeeper_hosts]
 
 
+def _get_apt_cache_policy():
+    return Popen(["apt-cache", "policy", "juju"], stdout=PIPE).\
+        communicate()[0]
+
+
+def _parse_apt_cache_policy(data):
+    """Flatten apt-cache policy output to generate (key, value) pairs."""
+
+    # Need to parse in two different phases, so set up iterator now
+    data = iter(data.split("\n"))
+
+    # Parse preamble
+    for row in data:
+        split = row.split(":", 1)
+        if len(split) != 2:
+            yield None, None
+        k, v = split[0].strip(), split[1].strip()
+        yield k, v
+        if k == "Version table":
+            break
+    
+    # Parse version table into tag, priorities
+    tag = None
+    for row in data:
+        row = row.strip()
+        if row.startswith("***"):
+            tag = row
+        elif tag:
+            yield tag, row
+
+
+def parse_juju_origin(data):
+    """Parse output of apt-cache to determine how juju was installed."""
+    parse = _parse_apt_cache_policy(data)
+    k, _ = parse.next()
+    if k != "juju":
+        # Sanity check: no record at all, the only possibility is that
+        # this being run as a branch
+        return "branch", "lp:juju"
+
+    version = None
+    for k, v in parse:
+        if k == "Installed":
+            if v == "(none)":
+                return "branch", "lp:juju"
+            version = v
+        if k == "Version table":
+            break
+
+    for tag, priority in parse:
+        if (version in tag and 
+            "http://ppa.launchpad.net/juju/pkgs/" in priority):
+            return "ppa", None
+
+    return "distro", None
+
+
+def _get_default_origin():
+    """Select the best fit for running juju on cloudinit.
+
+    Used if not otherwise specified by juju-origin.
+    """
+    import juju
+    if not juju.__file__.startswith("/usr/"):
+        return "lp:juju"
+    origin, source = parse_juju_origin(_get_apt_cache_policy())
+    if origin == "branch":
+        return source
+    elif origin == "ppa":
+        return _PPA
+    else:
+        return _DISTRO
+
+
 class CloudInit(object):
     """Encapsulates juju-specific machine initialisation.
 
@@ -65,7 +141,7 @@
         self._machine_id = None
         self._instance_id = None
         self._provider_type = None
-        self._source = _PPA
+        self._source = _get_default_origin()
         self._ssh_keys = []
         self._provision = False
         self._zookeeper = False
@@ -103,7 +179,7 @@
             or fewer than one options, are specified.
 
         Note that you don't need to call this method; the juju source
-        defaults to the juju PPA.
+        defaults to what is returned by `get_default_origin`.
         """
         if len(filter(None, (branch, ppa, distro))) != 1:
             raise CloudInitError("Please specify one source")

=== modified file 'juju/providers/common/launch.py'
--- juju/providers/common/launch.py	2011-09-22 13:23:00 +0000
+++ juju/providers/common/launch.py	2011-09-29 23:50:28 +0000
@@ -76,8 +76,15 @@
         cloud_init.add_ssh_key(get_user_authorized_keys(config))
         cloud_init.set_machine_id(machine_id)
         cloud_init.set_zookeeper_machines(zookeepers)
-        if config.get("juju-branch"):
-            cloud_init.set_juju_source(config["juju-branch"])
+        origin = config.get("juju-origin")
+        if origin:
+            if origin.startswith("lp:"):
+                cloud_init.set_juju_source(branch=origin)
+            elif origin == "ppa":
+                cloud_init.set_juju_source(ppa=True)
+            else:
+                # Ignore other values, just use the distro for sanity
+                cloud_init.set_juju_source(distro=True)
         if self._master:
             cloud_init.enable_bootstrap()
             cloud_init.set_zookeeper_secret(config["admin-secret"])

=== modified file 'juju/providers/common/tests/data/cloud_init_bootstrap'
--- juju/providers/common/tests/data/cloud_init_bootstrap	2011-09-23 20:35:02 +0000
+++ juju/providers/common/tests/data/cloud_init_bootstrap	2011-09-29 23:50:28 +0000
@@ -1,8 +1,6 @@
 #cloud-config
 apt-update: true
 apt-upgrade: true
-apt_sources:
-- {source: 'ppa:juju/pkgs'}
 machine-data: {juju-provider-type: dummy, juju-zookeeper-hosts: 'localhost:2181',
   machine-id: passport}
 output: {all: '| tee -a /var/log/cloud-init-output.log'}

=== modified file 'juju/providers/common/tests/data/cloud_init_bootstrap_zookeepers'
--- juju/providers/common/tests/data/cloud_init_bootstrap_zookeepers	2011-09-23 20:35:02 +0000
+++ juju/providers/common/tests/data/cloud_init_bootstrap_zookeepers	2011-09-29 23:50:28 +0000
@@ -1,8 +1,6 @@
 #cloud-config
 apt-update: true
 apt-upgrade: true
-apt_sources:
-- {source: 'ppa:juju/pkgs'}
 machine-data: {juju-provider-type: dummy, juju-zookeeper-hosts: 'cotswold:2181,longleat:2181,localhost:2181',
   machine-id: passport}
 output: {all: '| tee -a /var/log/cloud-init-output.log'}

=== added file 'juju/providers/common/tests/data/cloud_init_branch_trunk'
--- juju/providers/common/tests/data/cloud_init_branch_trunk	1970-01-01 00:00:00 +0000
+++ juju/providers/common/tests/data/cloud_init_branch_trunk	2011-09-29 23:50:28 +0000
@@ -0,0 +1,17 @@
+#cloud-config
+apt-update: true
+apt-upgrade: true
+apt_sources:
+- {source: 'ppa:juju/pkgs'}
+machine-data: {juju-provider-type: dummy, juju-zookeeper-hosts: 'cotswold:2181,longleat:2181',
+  machine-id: passport}
+output: {all: '| tee -a /var/log/cloud-init-output.log'}
+packages: [bzr, byobu, tmux, python-setuptools, python-twisted, python-argparse, python-txaws,
+  python-zookeeper]
+runcmd: [sudo apt-get install -y python-txzookeeper, sudo mkdir -p /usr/lib/juju,
+  'cd /usr/lib/juju && sudo /usr/bin/bzr co lp:juju juju',
+  cd /usr/lib/juju/juju && sudo python setup.py develop, sudo mkdir -p /var/lib/juju,
+  sudo mkdir -p /var/log/juju, 'JUJU_MACHINE_ID=passport JUJU_ZOOKEEPER=cotswold:2181,longleat:2181
+    python -m juju.agents.machine -n --logfile=/var/log/juju/machine-agent.log
+    --pidfile=/var/run/juju/machine-agent.pid']
+ssh_authorized_keys: [chubb]

=== modified file 'juju/providers/common/tests/data/cloud_init_normal'
--- juju/providers/common/tests/data/cloud_init_normal	2011-09-15 19:29:57 +0000
+++ juju/providers/common/tests/data/cloud_init_normal	2011-09-29 23:50:28 +0000
@@ -1,8 +1,6 @@
 #cloud-config
 apt-update: true
 apt-upgrade: true
-apt_sources:
-- {source: 'ppa:juju/pkgs'}
 machine-data: {juju-provider-type: dummy, juju-zookeeper-hosts: 'cotswold:2181,longleat:2181',
   machine-id: passport}
 output: {all: '| tee -a /var/log/cloud-init-output.log'}

=== added file 'juju/providers/common/tests/data/cloud_init_ppa'
--- juju/providers/common/tests/data/cloud_init_ppa	1970-01-01 00:00:00 +0000
+++ juju/providers/common/tests/data/cloud_init_ppa	2011-09-29 23:50:28 +0000
@@ -0,0 +1,15 @@
+#cloud-config
+apt-update: true
+apt-upgrade: true
+apt_sources:
+- {'source': 'ppa:juju/pkgs'}
+machine-data: {juju-provider-type: dummy, juju-zookeeper-hosts: 'cotswold:2181,longleat:2181',
+  machine-id: passport}
+output: {all: '| tee -a /var/log/cloud-init-output.log'}
+packages: [bzr, byobu, tmux, python-setuptools, python-twisted, python-argparse, python-txaws,
+  python-zookeeper]
+runcmd: [sudo apt-get -y install juju, sudo mkdir -p /var/lib/juju, sudo mkdir
+    -p /var/log/juju, 'JUJU_MACHINE_ID=passport JUJU_ZOOKEEPER=cotswold:2181,longleat:2181
+    python -m juju.agents.machine -n --logfile=/var/log/juju/machine-agent.log
+    --pidfile=/var/run/juju/machine-agent.pid']
+ssh_authorized_keys: [chubb]

=== modified file 'juju/providers/common/tests/test_cloudinit.py'
--- juju/providers/common/tests/test_cloudinit.py	2011-09-22 13:23:00 +0000
+++ juju/providers/common/tests/test_cloudinit.py	2011-09-29 23:50:28 +0000
@@ -1,12 +1,14 @@
 import os
+import stat
 
 import yaml
 
 from juju.errors import CloudInitError
 from juju.lib.testing import TestCase
-from juju.providers.common.cloudinit import CloudInit
+from juju.providers.common.cloudinit import CloudInit, parse_juju_origin
 from juju.providers.dummy import DummyMachine
 
+
 DATA_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), "data")
 
 
@@ -30,6 +32,7 @@
         cloud_init.set_provider_type("dummy")
         cloud_init.set_instance_id_accessor("token")
         cloud_init.set_zookeeper_secret("seekrit")
+        cloud_init.set_juju_source(distro=True)
         if with_zookeepers:
             cloud_init.set_zookeeper_machines([
                 DummyMachine("blah", "blah", "cotswold"),
@@ -74,12 +77,34 @@
             self.assertEquals(str(error), "Please specify one source")
 
     def test_render_normal(self):
-        self.assert_render(self.construct_normal(), "cloud_init_normal")
+        path = os.environ.get("PATH", "")
+        alt_apt_cache_path = self.makeDir()
+        filename = os.path.join(alt_apt_cache_path, "apt-cache")
+        with open(filename, "w") as f:
+            f.write(
+                "#!/bin/bash\n"
+                "cat <<EOF\n"
+                "juju:\n"
+                "  Installed: good-magic-1.0\n"
+                "  Candidate: good-magic-1.0\n"
+                "  Version table:\n"
+                " *** good-magic-1.0\n"
+                "        500 http://us.archive.ubuntu.com/ubuntu/ "
+                "natty/main amd64 Packages\n"
+                "        100 /var/lib/dpkg/status\n"
+                "EOF\n")
+        os.chmod(filename, stat.S_IEXEC | stat.S_IREAD)
+        updated_path = alt_apt_cache_path + ":" + path
+        self.change_environment(PATH=updated_path)
+        import juju
+        self.patch(juju, "__file__",
+                       "/usr/lib/pymodules/python2.7/juju/__init__.pyc")
+        self.assert_render(self.construct_normal(), "cloud_init_distro")
 
     def test_render_ppa_source(self):
         cloud_init = self.construct_normal()
         cloud_init.set_juju_source(ppa=True)
-        self.assert_render(cloud_init, "cloud_init_normal")
+        self.assert_render(cloud_init, "cloud_init_ppa")
 
     def test_render_distro_source(self):
         cloud_init = self.construct_normal()
@@ -91,9 +116,90 @@
         cloud_init.set_juju_source(branch="lp:blah/juju/blah-blah")
         self.assert_render(cloud_init, "cloud_init_branch")
 
+    def test_render_branch_source_if_not_installed(self):
+        import juju
+        self.patch(juju, "__file__", "/not/installed/under/usr")
+        cloud_init = self.construct_normal()
+        self.assert_render(cloud_init, "cloud_init_branch_trunk")
+
     def test_render_bootstrap(self):
         self.assert_render(self.construct_bootstrap(), "cloud_init_bootstrap")
 
     def test_render_bootstrap_with_zookeepers(self):
         self.assert_render(
             self.construct_bootstrap(True), "cloud_init_bootstrap_zookeepers")
+
+
+class ParseJujuOriginTest(TestCase):
+
+    def test_distro_installed(self):
+        data = (
+            "juju:\n"
+            "  Installed: good-magic-1.0\n"
+            "  Candidate: good-magic-1.0\n"
+            "  Version table:\n"
+            " *** good-magic-1.0 0\n"
+            "        500 http://us.archive.ubuntu.com/ubuntu/ "
+            "natty/main amd64 Packages\n"
+            "        100 /var/lib/dpkg/status\n")
+
+        origin, source = parse_juju_origin(data)
+        self.assertEqual(origin, "distro")
+        self.assertEqual(source, None)
+
+    def test_multiple_versions_available(self):
+        data = (
+            "juju:\n"
+            "  Installed: 0.5+bzr366-1juju1~natty1\n"
+            "  Candidate: 0.5+bzr366-1juju1~natty1\n"
+            "  Version table:\n"
+            " *** bad-magic-0.5 0\n"
+            "        500 http://us.archive.ubuntu.com/ubuntu/ "
+            "natty/main amd64 Packages\n"
+            "        100 /var/lib/dpkg/status\n"
+            " *** 0.5+bzr366-1juju1~natty1 0\n"
+            "        500 http://ppa.launchpad.net/juju/pkgs/ubuntu/ "
+            "natty/main amd64 Packages\n"
+            " *** 0.5+bzr356-1juju1~natty1 0\n"
+            "        500 http://ppa.launchpad.net/juju/pkgs/ubuntu/ "
+            "natty/main amd64 Packages\n")
+
+        origin, source = parse_juju_origin(data)
+        self.assertEqual(origin, "ppa")
+        self.assertEqual(source, None)
+
+    def test_distro_not_installed(self):
+        data = (
+            "juju:\n"
+            "  Installed: (none)\n"
+            "  Candidate: good-magic-1.0\n"
+            "  Version table:\n"
+            " *** good-magic-1.0 0\n"
+            "        500 http://us.archive.ubuntu.com/ubuntu/ "
+            "natty/main amd64 Packages\n"
+            "        100 /var/lib/dpkg/status\n")
+
+        origin, source = parse_juju_origin(data)
+        self.assertEqual(origin, "branch")
+        self.assertEqual(source, "lp:juju")
+
+    def test_ppa_installed(self):
+        data = (
+            "juju:\n"
+            "  Installed: 0.5+bzr356-1juju1~natty1\n"
+            "  Candidate: 0.5+bzr356-1juju1~natty1\n"
+            "  Version table:\n"
+            " *** 0.5+bzr356-1juju1~natty1 0\n"
+            "        500 http://ppa.launchpad.net/juju/pkgs/ubuntu/ "
+            "natty/main amd64 Packages\n"
+            "        100 /var/lib/dpkg/status\n")
+
+        origin, source = parse_juju_origin(data)
+        self.assertEqual(origin, "ppa")
+        self.assertEqual(source, None)
+
+    def test_juju_package_is_unknown(self):
+        data = "N: Unable to locate package juju"
+        origin, source = parse_juju_origin(data)
+        self.assertEqual(origin, "branch")
+        self.assertEqual(source, "lp:juju")

=== modified file 'juju/providers/ec2/tests/common.py'
--- juju/providers/ec2/tests/common.py	2011-09-22 13:23:00 +0000
+++ juju/providers/ec2/tests/common.py	2011-09-29 23:50:28 +0000
@@ -22,6 +22,7 @@
 
     def get_config(self):
         return {"type": "ec2",
+                "juju-origin": "distro",
                 "admin-secret": "magic-beans",
                 "access-key": "0f62e973d5f8",
                 "secret-key": "3e5a7c653f59",

=== modified file 'juju/providers/ec2/tests/data/bootstrap_cloud_init'
--- juju/providers/ec2/tests/data/bootstrap_cloud_init	2011-09-23 20:35:02 +0000
+++ juju/providers/ec2/tests/data/bootstrap_cloud_init	2011-09-29 23:50:28 +0000
@@ -1,8 +1,6 @@
 #cloud-config
 apt-update: true
 apt-upgrade: true
-apt_sources:
-- {source: 'ppa:juju/pkgs'}
 machine-data: {juju-provider-type: ec2, juju-zookeeper-hosts: 'localhost:2181',
   machine-id: '0'}
 output: {all: '| tee -a /var/log/cloud-init-output.log'}

=== modified file 'juju/providers/ec2/tests/data/launch_cloud_init'
--- juju/providers/ec2/tests/data/launch_cloud_init	2011-09-15 19:29:57 +0000
+++ juju/providers/ec2/tests/data/launch_cloud_init	2011-09-29 23:50:28 +0000
@@ -1,8 +1,6 @@
 #cloud-config
 apt-update: true
 apt-upgrade: true
-apt_sources:
-- {source: 'ppa:juju/pkgs'}
 machine-data: {juju-provider-type: ec2, juju-zookeeper-hosts: 'es.example.internal:2181',
   machine-id: '1'}
 output: {all: '| tee -a /var/log/cloud-init-output.log'}

=== added file 'juju/providers/ec2/tests/data/launch_cloud_init_branch'
--- juju/providers/ec2/tests/data/launch_cloud_init_branch	1970-01-01 00:00:00 +0000
+++ juju/providers/ec2/tests/data/launch_cloud_init_branch	2011-09-29 23:50:28 +0000
@@ -0,0 +1,20 @@
+#cloud-config
+apt-update: true
+apt-upgrade: true
+apt_sources:
+- {source: 'ppa:juju/pkgs'}
+machine-data: {juju-provider-type: ec2, juju-zookeeper-hosts: 'es.example.internal:2181',
+  machine-id: '1'}
+output: {all: '| tee -a /var/log/cloud-init-output.log'}
+packages: [bzr, byobu, tmux, python-setuptools, python-twisted, python-argparse, python-txaws,
+  python-zookeeper]
+runcmd: [sudo apt-get install -y python-txzookeeper,
+         sudo mkdir -p /usr/lib/juju,
+         'cd /usr/lib/juju && sudo /usr/bin/bzr co lp:~wizard/juju-juicebar juju',
+         cd /usr/lib/juju/juju && sudo python setup.py develop,
+         sudo mkdir -p /var/lib/juju,
+         sudo mkdir -p /var/log/juju,
+         'JUJU_MACHINE_ID=1 JUJU_ZOOKEEPER=es.example.internal:2181
+    python -m juju.agents.machine -n --logfile=/var/log/juju/machine-agent.log
+    --pidfile=/var/run/juju/machine-agent.pid']
+ssh_authorized_keys: [zebra]

=== added file 'juju/providers/ec2/tests/data/launch_cloud_init_ppa'
--- juju/providers/ec2/tests/data/launch_cloud_init_ppa	1970-01-01 00:00:00 +0000
+++ juju/providers/ec2/tests/data/launch_cloud_init_ppa	2011-09-29 23:50:28 +0000
@@ -0,0 +1,15 @@
+#cloud-config
+apt-update: true
+apt-upgrade: true
+apt_sources:
+- {source: 'ppa:juju/pkgs'}
+machine-data: {juju-provider-type: ec2, juju-zookeeper-hosts: 'es.example.internal:2181',
+  machine-id: '1'}
+output: {all: '| tee -a /var/log/cloud-init-output.log'}
+packages: [bzr, byobu, tmux, python-setuptools, python-twisted, python-argparse, python-txaws,
+  python-zookeeper]
+runcmd: [sudo apt-get -y install juju, sudo mkdir -p /var/lib/juju, sudo mkdir
+    -p /var/log/juju, 'JUJU_MACHINE_ID=1 JUJU_ZOOKEEPER=es.example.internal:2181
+    python -m juju.agents.machine -n --logfile=/var/log/juju/machine-agent.log
+    --pidfile=/var/run/juju/machine-agent.pid']
+ssh_authorized_keys: [zebra]

=== modified file 'juju/providers/ec2/tests/test_bootstrap.py'
--- juju/providers/ec2/tests/test_bootstrap.py	2011-09-22 13:23:00 +0000
+++ juju/providers/ec2/tests/test_bootstrap.py	2011-09-29 23:50:28 +0000
@@ -13,6 +13,7 @@
 
 from .common import EC2TestMixin, EC2MachineLaunchMixin
 
+
 DATA_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), "data")
 
 

=== modified file 'juju/providers/ec2/tests/test_launch.py'
--- juju/providers/ec2/tests/test_launch.py	2011-09-16 00:36:31 +0000
+++ juju/providers/ec2/tests/test_launch.py	2011-09-29 23:50:28 +0000
@@ -21,10 +21,11 @@
 class EC2MachineLaunchTest(EC2TestMixin, EC2MachineLaunchMixin, TestCase):
 
     def _mock_launch(self, instance, expect_ami="ami-default",
-                     expect_instance_type="m1.small"):
+                     expect_instance_type="m1.small",
+                     cloud_init="launch_cloud_init"):
 
         def verify_user_data(data):
-            expect_path = os.path.join(DATA_DIR, "launch_cloud_init")
+            expect_path = os.path.join(DATA_DIR, cloud_init)
             with open(expect_path) as f:
                 expect_cloud_init = yaml.load(f.read())
             self.assertEquals(yaml.load(data), expect_cloud_init)
@@ -75,6 +76,63 @@
         return d
 
     @inlineCallbacks
+    def test_provider_launch_using_branch(self):
+        """Can use a juju branch to launch a machine"""
+        self.ec2.describe_security_groups()
+        self.mocker.result(succeed([]))
+        self._mock_create_group()
+        self._mock_create_machine_group("1")
+        self._mock_launch_utils(region="us-east-1")
+        self._mock_get_zookeeper_hosts()
+        self._mock_launch(
+            self.get_instance("i-foobar"),
+            cloud_init="launch_cloud_init_branch")
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        provider.config["juju-origin"] = "lp:~wizard/juju-juicebar"
+        machines = yield provider.start_machine({"machine-id": "1"})
+        self.assert_machine(machines[0], "i-foobar", "")
+
+    @inlineCallbacks
+    def test_provider_launch_using_ppa(self):
+        """Can use the juju ppa to launch a machine"""
+        self.ec2.describe_security_groups()
+        self.mocker.result(succeed([]))
+        self._mock_create_group()
+        self._mock_create_machine_group("1")
+        self._mock_launch_utils(region="us-east-1")
+        self._mock_get_zookeeper_hosts()
+        self._mock_launch(
+            self.get_instance("i-foobar"),
+            cloud_init="launch_cloud_init_ppa")
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        provider.config["juju-origin"] = "ppa"
+        machines = yield provider.start_machine({"machine-id": "1"})
+        self.assert_machine(machines[0], "i-foobar", "")
+
+    @inlineCallbacks
+    def test_provider_launch_using_explicit_distro(self):
+        """Can set juju-origin explicitly to `distro`"""
+        self.ec2.describe_security_groups()
+        self.mocker.result(succeed([]))
+        self._mock_create_group()
+        self._mock_create_machine_group("1")
+        self._mock_launch_utils(region="us-east-1")
+        self._mock_get_zookeeper_hosts()
+        self._mock_launch(
+            self.get_instance("i-foobar"),
+            cloud_init="launch_cloud_init")
+        self.mocker.replay()
+
+        provider = self.get_provider()
+        provider.config["juju-origin"] = "distro"
+        machines = yield provider.start_machine({"machine-id": "1"})
+        self.assert_machine(machines[0], "i-foobar", "")
+
+    @inlineCallbacks
     def test_provider_launch_existing_security_group(self):
         """Verify that the launch works if the env security group exists"""
         instance = Instance("i-foobar", "running", dns_name="x1.example.com")

=== modified file 'juju/providers/orchestra/tests/common.py'
--- juju/providers/orchestra/tests/common.py	2011-09-22 13:23:00 +0000
+++ juju/providers/orchestra/tests/common.py	2011-09-29 23:50:28 +0000
@@ -13,7 +13,9 @@
 
 DATA_DIR = os.path.join(os.path.abspath(os.path.dirname(__file__)), "data")
 
-CONFIG = {"orchestra-server": "somewhe.re",
+CONFIG = {"type": "orchestra",
+          "juju-origin": "distro",
+          "orchestra-server": "somewhe.re",
           "orchestra-user": "user",
           "orchestra-pass": "pass",
           "acquired-mgmt-class": "acquired",

=== modified file 'juju/providers/orchestra/tests/data/bootstrap_user_data'
--- juju/providers/orchestra/tests/data/bootstrap_user_data	2011-09-23 20:35:02 +0000
+++ juju/providers/orchestra/tests/data/bootstrap_user_data	2011-09-29 23:50:28 +0000
@@ -1,8 +1,6 @@
 #cloud-config
 apt-update: true
 apt-upgrade: true
-apt_sources:
-- {source: 'ppa:juju/pkgs'}
 machine-data: {juju-provider-type: orchestra, juju-zookeeper-hosts: 'localhost:2181',
   machine-id: '0'}
 output: {all: '| tee -a /var/log/cloud-init-output.log'}

=== modified file 'juju/providers/orchestra/tests/data/launch_user_data'
--- juju/providers/orchestra/tests/data/launch_user_data	2011-09-16 14:07:03 +0000
+++ juju/providers/orchestra/tests/data/launch_user_data	2011-09-29 23:50:28 +0000
@@ -1,8 +1,6 @@
 #cloud-config
 apt-update: true
 apt-upgrade: true
-apt_sources:
-- {source: 'ppa:juju/pkgs'}
 machine-data: {juju-provider-type: orchestra, juju-zookeeper-hosts: 'jennifer:2181',
   machine-id: '42'}
 output: {all: '| tee -a /var/log/cloud-init-output.log'}