Merge lp:~frankban/charms/precise/juju-gui/bug-1088618-serve-releases into lp:~juju-gui/charms/precise/juju-gui/trunk

[Francesco Banconi]

Updated the charm to serve Juju GUI releases.

I had a pre-implementation call with Gary and
some really useful help from Brad.

Details:

Replaced the juju-gui-branch config option with a new juju-gui-source
one: here you can specify where to deploy the GUI from (stable, trunk, a
branch, a specific version of stable/trunk).

Added a bunch of functions in utils.py: they help parsing the
juju-gui-source option, retrieving the URL of a release, etc.

Added tests for the utility functions above.

Changed the fetch/build function in utils.py, so that now a release
tarball is always used to install the GUI. It can be downloaded from
Launchpad or created using "make distfile" from a juju-gui checkout.
Also reorganized the fetch/build code: separated each of the fetch/build
functions into two different functions (one for the GUI, one for the API).

Updated the install and the config-changed hooks to conform to the
new fetch/setup functions (mentioned above).

Updated the config-changed hook to reflect changes to the options,
install hook and utils.py.

Added one functional test specific to deploying from a branch.
The other tests now default to the latest stable release.

Created and uploaded Juju GUI trunk/stable releases.

Updated the stop hook: now the function in utils.py is called with the
argument it expects.

Overall code lint and clean up.

All those changes result in a big diff (~820 lines), sorry about that.

https://codereview.appspot.com/6977043/

[Francesco Banconi (frankban)]

Reviewers: mp+140745_code.launchpad.net,

Message:
Please take a look.

Description:
Updated the charm to serve Juju GUI releases.

I had a pre-implementation call with Gary and
some really useful help from Brad.

Details:

Replaced the juju-gui-branch config option with a new juju-gui-source
one: here you can specify where to deploy the GUI from (stable, trunk, a
branch, a specific version of stable/trunk).

Added a bunch of functions in utils.py: they help parsing the
juju-gui-source option, retrieving the URL of a release, etc.

Added tests for the utility functions above.

Changed the fetch/build function in utils.py, so that now a release
tarball is always used to install the GUI. It can be downloaded from
Launchpad or created using "make distfile" from a juju-gui checkout.
Also reorganized the fetch/build code: separated each of the fetch/build
functions into two different functions (one for the GUI, one for the
API).

Updated the install and the config-changed hooks to conform to the
new fetch/setup functions (mentioned above).

Updated the config-changed hook to reflect changes to the options,
install hook and utils.py.

Added one functional test specific to deploying from a branch.
The other tests now default to the latest stable release.

Created and uploaded Juju GUI trunk/stable releases.

Updated the stop hook: now the function in utils.py is called with the
argument it expects.

Overall code lint and clean up.

All those changes result in a big diff (~820 lines), sorry about that.

https://code.launchpad.net/~frankban/charms/precise/juju-gui/bug-1088618-serve-releases/+merge/140745

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/6977043/

Affected files:
   A [revision details]
   M config.yaml
   M hooks/config-changed
   M hooks/install
   M hooks/stop
   M hooks/utils.py
   M tests/deploy.test
   M tests/test_utils.py

[Gary Poster (gary)]

This looks great Francesco. :-)

I suggest that you ignore my bzr: idea (see below). We can add that if
we want it later.

I'm running tests for Nicola's branch now. I'll give your branch a try
after that, and then give an official blessing.

https://codereview.appspot.com/6977043/diff/1/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/6977043/diff/1/hooks/utils.py#newcode131
hooks/utils.py:131: if source.startswith('lp:') or
source.startswith('http://'):
I wonder if we should support bzr: too. Might be nice. No biggie.

https://codereview.appspot.com/6977043/diff/1/tests/test_utils.py
File tests/test_utils.py (right):

https://codereview.appspot.com/6977043/diff/1/tests/test_utils.py#newcode85
tests/test_utils.py:85: # Ensure the factory return the expected object
instances.
typo: returns

https://codereview.appspot.com/6977043/

[Gary Poster (gary)]

Good news and bad news. The good news is that the deployment took three
minutes instead of 20 for me. Rock! The bad news is that the tests did
not pass for me. I have to run now, but this is what I see so far.

$ time JUJU_REPOSITORY=~/dev/repo/ ~/bin/jitsu test juju-gui --logdir
/tmp --timeout 40m --no-bootstrap
2012-12-19 19:30:10,417 jitsu.test:INFO Running unit test: deploy.test
test_api_agent (__main__.DeployTest) ... ok
test_branch_source (__main__.DeployTest) ... ERROR
test_customized_api_port (__main__.DeployTest) ... ERROR
test_staging (__main__.DeployTest) ... ERROR
test_api_agent (__main__.DeployToTest) ...

If I see any helpful log messages I'll pass them on later. Anyway,
"Land with changes": please verify that the tests pass for you before
landing.

Thanks

Gary

https://codereview.appspot.com/6977043/

[Gary Poster (gary)]

On 2012/12/20 01:02:21, gary.poster wrote:
> Good news and bad news. The good news is that the deployment took
three minutes
> instead of 20 for me. Rock! The bad news is that the tests did not
pass for
> me. I have to run now, but this is what I see so far.

> $ time JUJU_REPOSITORY=~/dev/repo/ ~/bin/jitsu test juju-gui --logdir
/tmp
> --timeout 40m --no-bootstrap
> 2012-12-19 19:30:10,417 jitsu.test:INFO Running unit test: deploy.test
> test_api_agent (__main__.DeployTest) ... ok
> test_branch_source (__main__.DeployTest) ... ERROR
> test_customized_api_port (__main__.DeployTest) ... ERROR
> test_staging (__main__.DeployTest) ... ERROR
> test_api_agent (__main__.DeployToTest) ...

> If I see any helpful log messages I'll pass them on later. Anyway,
"Land with
> changes": please verify that the tests pass for you before landing.

> Thanks

> Gary

The unit tests all passed just fine, for what it is worth.

Unfortunately, the test logs that jitsu collected for the functional
tests don't appear to be informative.

The tests took 40 minutes and a few seconds, because the
DeployToTest.test_api_agent test hung and I used a maximum 40 minute
test length time with jitsu test.

2012-12-19 19:30:10,417 jitsu.test:INFO Running unit test: deploy.test
test_api_agent (__main__.DeployTest) ... ok
test_branch_source (__main__.DeployTest) ... ERROR
test_customized_api_port (__main__.DeployTest) ... ERROR
test_staging (__main__.DeployTest) ... ERROR
test_api_agent (__main__.DeployToTest) ... 2012-12-19 20:10:10,421
jitsu.test:WARNING Error running unit test deploy.test: 124

I have a vague suspicion that your branch of the charm doesn't like
installing on top of itself, but there could be many other explanations.

I really want to know what you do to debug these functional tests. I
did not get pdbs to work locally (stdout/stdin are not passed through
for this to work, AFAICT), and the logs collected by jitsu test left a
lot to be desired. I wanted to investigate the charm log, but we don't
get that from jitsu test. Maybe we ought to incorporate that into our
own test infrastructure? I have a weekly retrospective card to discuss
this, and see if you or anyone else have any good techniques, or ideas
about what we can do to improve jitsu test for debugging.

Thanks again

Gary

https://codereview.appspot.com/6977043/

[Kapil Thangavelu (hazmat)]

On Wed, Dec 19, 2012 at 9:34 PM, Gary Poster <email address hidden>wrote:

>
> I really want to know what you do to debug these functional tests. I
> did not get pdbs to work locally (stdout/stdin are not passed through
> for this to work, AFAICT), and the logs collected by jitsu test left a
> lot to be desired. I wanted to investigate the charm log, but we don't
> get that from jitsu test. Maybe we ought to incorporate that into our
> own test infrastructure? I have a weekly retrospective card to discuss
> this, and see if you or anyone else have any good techniques, or ideas
> about what we can do to improve jitsu test for debugging.
>

it should definitely include the charm.log if it doesn't thats a bug imo.

fwiw, lp:charmrunner has a tool for collecting *all* of the data available
in an environment (recorder.py)

[Francesco Banconi (frankban)]

On 2012/12/20 03:32:06, gary.poster wrote:

> The unit tests all passed just fine, for what it is worth.

Cool.

19:30:10,417 jitsu.test:INFO Running unit test: deploy.test
> test_api_agent (__main__.DeployTest) ... ok
> test_branch_source (__main__.DeployTest) ... ERROR
> test_customized_api_port (__main__.DeployTest) ... ERROR
> test_staging (__main__.DeployTest) ... ERROR
> test_api_agent (__main__.DeployToTest) ... 2012-12-19 20:10:10,421
> jitsu.test:WARNING Error running unit test deploy.test: 124

> I have a vague suspicion that your branch of the charm doesn't like
installing
> on top of itself, but there could be many other explanations.

It seems so looking at this output. This is really weird because the
tests always
pass when I run them. The last test in that output seems to be truncated
by the
timeout command used by jitsu test.

> I really want to know what you do to debug these functional tests. I
did not
> get pdbs to work locally (stdout/stdin are not passed through for this
to work,
> AFAICT), and the logs collected by jitsu test left a lot to be
desired.

I usually run jitsu test in a shell:
JUJU_REPOSITORY=/home/frankban/devel/juju/store/ jitsu test juju-gui
--timeout 60m --logdir /home/frankban/devel/juju/guicharm/log/
In parallel, in another shell, I run "juju debug-log", so that I can see
the output of all the hooks while they are run.

> I wanted to investigate the charm log, but we don't get that from
jitsu test.

The missing charm log exposes a problem. To simplify, here is what jitsu
test does:

for each *.test file in tests/:
   bootstrap a juju env
   execute the file (using the shell command "timeout --kill-after 2m
TIMEOUT file"
   for each machine in the environment:
     gather logs using rsync: everything in /var/log/juju and
/var/lib/juju/units/*/charm.log
   destroy the environment

In our tests, unittest.TestCase.tearDown destroys the juju-gui service.
This allows us to reuse the same machine in subsequent tests, where we
usually re-deploy
the service, testing different configurations. And this is a good idea
IMHO.
But this also raises a problem: "destroy service" removes the service
directory in
/var/lib/juju/units/, and that directory includes charm.log.

Some ideas:

1) jitsu test seems to suggest you to run tests in separate files. This
would solve
the missing charm.log problem, but IMHO it's ugly:
- we can no longer use the --no-bootstrap option: we still need to
re-deploy the charm
   multiple times, and we need a machine in a clean state;
- even if we can decrease the timeout for each test, the overall time
elapsed will be
   a lot increased: for each test, we'd have to wait for the juju env to
be bootstrapped,
   and for one or two machines to be started on ec2;
- re-deploying the charm in the same machine adds value to our tests,
because we also
   check that our charm correctly cleans things up (e.g. services).
AFAIK, juju reuses
   machines, at least when the provider is ec2, so this represents a real
use case;
- IMHO the functional tests, the way they are written now, are compact
and easy to read:
   splitting them into different files doesn't seem something that should
be imposed
   by the t...

[lp:~frankban/charms/precise/juju-gui/bug-1088618-serve-releases]

38. By Francesco Banconi on 2012-12-20

Fixed typo.

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/6977043/

[Gary Poster (gary)]

Thank you for the detailed debugging information, Francesco. I will experiment with it and it will be good to share it with everyone.

Since the tests pass for you, "Land as is." Hopefully I'll get my local issues sorted.

Gary

On Dec 20, 2012, at 5:25 AM, iFrancesco Banconi <email address hidden> wrote:

> Please take a look.
>
> https://codereview.appspot.com/6977043/
>
> --
> https://code.launchpad.net/~frankban/charms/precise/juju-gui/bug-1088618-serve-releases/+merge/140745
> Your team Juju GUI Hackers is requested to review the proposed merge of lp:~frankban/charms/precise/juju-gui/bug-1088618-serve-releases into lp:~juju-gui/charms/precise/juju-gui/trunk.
>
> --
> Mailing list: https://launchpad.net/~yellow
> Post to : <email address hidden>
> Unsubscribe : https://launchpad.net/~yellow
> More help : https://help.launchpad.net/ListHelp

[Nicola Larosa (teknico)]

Land with changes.

Very nice refactoring job and new code, thanks.

A few small changes below, plus one code placement objection. Also, I
personally prefer that ".tar.gz" extension over the ".tgz" one, but I
guess it's personal taste.

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed
File hooks/config-changed (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50
hooks/config-changed:50: # Restarting of the gui and api services is
handled below.
The two lines above could be replaced by:
# Fetch new sources if needed, and restart GUI and API services

https://codereview.appspot.com/6977043/diff/8001/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/utils.py#newcode310
hooks/utils.py:310: """Set up nginx."""
Well, nginx has nothing to do with the API environment, has it? It looks
like its setup code should be part of setup_gui, and setup_api should be
empty, for now. Shortly we will copy the nginx SSL certificate and
private key to the API environment, that's one thing that should go in
setup_api.

https://codereview.appspot.com/6977043/diff/8001/tests/test_utils.py
File tests/test_utils.py (right):

https://codereview.appspot.com/6977043/diff/8001/tests/test_utils.py#newcode117
tests/test_utils.py:117: """Simulates a Launchpad hosted file returned
by launchpadlib."""
"Simulate..."

https://codereview.appspot.com/6977043/

[Gary Poster (gary)]

I was just reviewing where we were and had two comments. No change to
my previous review/blessing. :-)

Gary

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed
File hooks/config-changed (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50
hooks/config-changed:50: # Restarting of the gui and api services is
handled below.
On 2012/12/20 13:09:14, teknico wrote:
> The two lines above could be replaced by:
> # Fetch new sources if needed, and restart GUI and API services
I think the point of the comment is to reassure readers that these
things will be restarted, just not in this code block. "below" means
"not in this code block."

The comment worked for me. Would this have been clearer for you? "# The
juju_gui_source_changed and juju_api_branch_changed variables control
whether we restart the GUI and the API, respectively, at the end of the
function." Or something. Other ideas?

https://codereview.appspot.com/6977043/diff/8001/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/utils.py#newcode310
hooks/utils.py:310: """Set up nginx."""
On 2012/12/20 13:09:14, teknico wrote:
> Well, nginx has nothing to do with the API environment, has it? It
looks like
> its setup code should be part of setup_gui, and setup_api should be
empty, for
> now. Shortly we will copy the nginx SSL certificate and private key to
the API
> environment, that's one thing that should go in setup_api.

Good point.

https://codereview.appspot.com/6977043/

[lp:~frankban/charms/precise/juju-gui/bug-1088618-serve-releases]

39. By Francesco Banconi on 2012-12-20

Changes per review.

[Francesco Banconi (frankban)]

Hi Nicola,

thanks for the review.

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed
File hooks/config-changed (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50
hooks/config-changed:50: # Restarting of the gui and api services is
handled below.
On 2012/12/20 13:09:14, teknico wrote:
> The two lines above could be replaced by:
> # Fetch new sources if needed, and restart GUI and API services

This pre-existed and I think it's actually correct, the services are not
restarted here.

https://codereview.appspot.com/6977043/diff/8001/hooks/utils.py
File hooks/utils.py (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/utils.py#newcode310
hooks/utils.py:310: """Set up nginx."""
On 2012/12/20 13:09:14, teknico wrote:
> Well, nginx has nothing to do with the API environment, has it? It
looks like
> its setup code should be part of setup_gui, and setup_api should be
empty, for
> now. Shortly we will copy the nginx SSL certificate and private key to
the API
> environment, that's one thing that should go in setup_api.

Good catch, I'll rename this function.

https://codereview.appspot.com/6977043/diff/8001/tests/test_utils.py
File tests/test_utils.py (right):

https://codereview.appspot.com/6977043/diff/8001/tests/test_utils.py#newcode117
tests/test_utils.py:117: """Simulates a Launchpad hosted file returned
by launchpadlib."""
On 2012/12/20 13:09:14, teknico wrote:
> "Simulate..."

Done.

https://codereview.appspot.com/6977043/

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/6977043/

[Nicola Larosa (teknico)]

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed
File hooks/config-changed (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50
hooks/config-changed:50: # Restarting of the gui and api services is
handled below.
gary.poster wrote:
> I think the point of the comment is to reassure readers that these
things
> will be restarted, just not in this code block. "below" means "not in
> this code block."

Oh, I see. I mistook "below" to mean "in the code below, within this
function".

> The comment worked for me. Would this have been clearer for you?
> "# The juju_gui_source_changed and juju_api_branch_changed variables
> control whether we restart the GUI and the API, respectively, at the
end
> of the function." Or something. Other ideas?

Yes, maybe with s/at the end of the function./after this function./, or
similar.

frankban wrote:
> This pre-existed and I think it's actually correct, the services are
not
> restarted here.

Yes, it preexisted, but it was placed *after* a code block, making its
interpretation less ambiguous.

https://codereview.appspot.com/6977043/

[Francesco Banconi (frankban)]

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed
File hooks/config-changed (right):

https://codereview.appspot.com/6977043/diff/8001/hooks/config-changed#newcode50
hooks/config-changed:50: # Restarting of the gui and api services is
handled below.
On 2012/12/20 13:38:35, teknico wrote:

> Yes, it preexisted, but it was placed *after* a code block, making its
> interpretation less ambiguous.

Ok, replacing this comment with the one Gary suggested.

https://codereview.appspot.com/6977043/

[lp:~frankban/charms/precise/juju-gui/bug-1088618-serve-releases]

40. By Francesco Banconi on 2012-12-20

Fixed comment.

41. By Francesco Banconi on 2012-12-20

Merged trunk and resolved conflicts.

42. By Francesco Banconi on 2012-12-20

Turn off TLS.

[Francesco Banconi (frankban)]

Please take a look.

https://codereview.appspot.com/6977043/

[Francesco Banconi (frankban)]

*** Submitted:

Updated the charm to serve Juju GUI releases.

I had a pre-implementation call with Gary and
some really useful help from Brad.

Details:

Replaced the juju-gui-branch config option with a new juju-gui-source
one: here you can specify where to deploy the GUI from (stable, trunk, a
branch, a specific version of stable/trunk).

Added a bunch of functions in utils.py: they help parsing the
juju-gui-source option, retrieving the URL of a release, etc.

Added tests for the utility functions above.

Changed the fetch/build function in utils.py, so that now a release
tarball is always used to install the GUI. It can be downloaded from
Launchpad or created using "make distfile" from a juju-gui checkout.
Also reorganized the fetch/build code: separated each of the fetch/build
functions into two different functions (one for the GUI, one for the
API).

Updated the install and the config-changed hooks to conform to the
new fetch/setup functions (mentioned above).

Updated the config-changed hook to reflect changes to the options,
install hook and utils.py.

Added one functional test specific to deploying from a branch.
The other tests now default to the latest stable release.

Created and uploaded Juju GUI trunk/stable releases.

Updated the stop hook: now the function in utils.py is called with the
argument it expects.

Overall code lint and clean up.

All those changes result in a big diff (~820 lines), sorry about that.

R=gary.poster, teknico
CC=
https://codereview.appspot.com/6977043

https://codereview.appspot.com/6977043/

=== modified file 'README.md'
--- README.md	2012-12-19 11:49:41 +0000
+++ README.md	2012-12-20 18:04:24 +0000
@@ -58,7 +58,9 @@
             machine: 1
             open-ports:
             - 80/tcp
+            <!--- Uncomment when TLS connections are re-enabled.
             - 443/tcp
+            -->
             - 8080/tcp
             public-address: ec2-204-236-250-8.compute-1.amazonaws.com
 

=== modified file 'config.yaml'
--- config.yaml	2012-12-18 17:46:09 +0000
+++ config.yaml	2012-12-20 18:04:24 +0000
@@ -1,9 +1,17 @@
 options:
-  juju-gui-branch:
+  juju-gui-source:
     description: |
-      The Juju GUI Bazaar branch containing the source code to be deployed.
+      Where to install Juju GUI from. Possible values are:
+      - 'stable' (default): the latest stable release will be deployed;
+      - 'trunk': the latest trunk release will be deployed;
+      - a stable version (e.g '0.1.0'): the specified stable version will be
+        deployed;
+      - a trunk version (e.g '0.1.0+build.1'): the specified trunk version
+        will be deployed;
+      - a Bazaar branch (e.g. 'lp:juju-gui'): a release will be created and
+        deployed from the specified Bazaar branch.
     type: string
-    default: lp:juju-gui
+    default: stable
   juju-api-branch:
     description: |
       The Juju API Bazaar branch (implementing the websocket server).

=== modified file 'config/nginx.conf.template'
--- config/nginx.conf.template	2012-12-19 17:24:21 +0000
+++ config/nginx.conf.template	2012-12-20 18:04:24 +0000
@@ -1,16 +1,20 @@
-server {
-    listen 80;
-    server_name _;
-    return 301 https://$host$request_uri;
-}
+# Uncomment to switch back to TLS connections.
+# server {
+#     listen 80;
+#     server_name _;
+#     return 301 https://$host$request_uri;
+# }
 
 server {
-    listen 443 default_server ssl;
+    # Uncomment to switch back to TLS connections.
+    # listen 443 default_server ssl;
+    listen 80;  # Delete this line when TLS connections are re-enabled.
     server_name _;
     root %(server_root)s;
     index index.html;
-    ssl_certificate /etc/ssl/private/juju-gui/server.pem;
-    ssl_certificate_key /etc/ssl/private/juju-gui/server.key;
+    # Uncomment to switch back to TLS connections.
+    # ssl_certificate /etc/ssl/private/juju-gui/server.pem;
+    # ssl_certificate_key /etc/ssl/private/juju-gui/server.key;
 
     # Serve static assets.
     location ^~ /juju-ui/ {

=== modified file 'hooks/config-changed'
--- hooks/config-changed	2012-12-19 17:24:21 +0000
+++ hooks/config-changed	2012-12-20 18:04:24 +0000
@@ -4,8 +4,6 @@
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
-import os.path
-from shutil import rmtree
 from subprocess import CalledProcessError
 import sys
 
@@ -24,11 +22,13 @@
 
 from utils import (
     AGENT,
-    build,
     config_json,
-    fetch,
+    fetch_api,
+    fetch_gui,
     GUI,
     IMPROV,
+    setup_gui,
+    setup_nginx,
     start_agent,
     start_gui,
     start_improv,
@@ -39,33 +39,33 @@
     # Handle all configuration file changes.
     log('Updating configuration.')
 
+    added_or_changed = diff.added_or_changed
+    juju_api_port = config.get('juju-api-port')
     staging = config.get('staging')
-    staging_environment = config.get('staging-environment')
-    juju_api_port = config.get('juju-api-port')
-    added_or_changed = diff.added_or_changed
-
-    # Fetch new branches?
-    gui_branch = None
-    api_branch = None
-    if 'juju-gui-branch' in added_or_changed:
-        if os.path.exists('juju-gui'):
-            rmtree('juju-gui')
-        gui_branch = config['juju-gui-branch']
+
+    # The juju_gui_source_changed and juju_api_branch_changed variables
+    # control whether we restart the GUI and the API, respectively, at the
+    # end of the function.
+    juju_gui_source_changed = False
+    juju_api_branch_changed = False
+
+    # Fetch new sources?
+    if 'juju-gui-source' in added_or_changed:
+        juju_gui_source_changed = True
+        release_tarball = fetch_gui(
+            config['juju-gui-source'], config['command-log-file'])
+        setup_gui(release_tarball)
+        setup_nginx(config['ssl-cert-path'])
     if 'juju-api-branch' in added_or_changed:
-        if os.path.exists('juju'):
-            rmtree('juju')
-        api_branch = config['juju-api-branch']
-    if gui_branch or api_branch:
-        fetch(gui_branch, api_branch)
-        build(config['command-log-file'], config['ssl-cert-path'])
-        # Restarting of the gui and api services is handled below.
+        juju_api_branch_changed = True
+        fetch_api(config['juju-api-branch'])
 
     # Handle changes to the improv server configuration.
     if staging:
         staging_properties = set(
             ['staging', 'staging-environment', 'juju-api-port'])
         staging_changed = added_or_changed & staging_properties
-        if staging_changed or (api_branch is not None):
+        if staging_changed or juju_api_branch_changed:
             if 'staging' in added_or_changed:
                 # 'staging' went from False to True, so the agent server is
                 # running and must be stopped.
@@ -76,14 +76,13 @@
                 current_api = IMPROV
             log('Stopping %s.' % current_api)
             service_control(current_api, STOP)
-
             # Now the improv server can be cleanly started.
-            log('Starting or restarting improv')
-            start_improv(juju_api_port, staging_environment)
+            log('Starting or restarting staging.')
+            start_improv(juju_api_port, config.get('staging-environment'))
     else:
         agent_properties = set(['juju-api-port', 'staging'])
         agent_changed = added_or_changed & agent_properties
-        if agent_changed or (api_branch is not None):
+        if agent_changed or juju_api_branch_changed:
             if 'staging' in added_or_changed:
                 # If 'staging' transitions to False we need to stop the backend
                 # and start the agent.
@@ -93,15 +92,15 @@
                 # updated -- bounce it.
                 current_api = AGENT
             service_control(current_api, STOP)
+            log('Starting or restarting Juju API agent.')
             start_agent(juju_api_port)
 
     # Handle changes to the juju-gui configuration.
     gui_properties = set(
         ['juju-gui-console-enabled', 'juju-api-port', 'staging'])
     gui_changed = added_or_changed & gui_properties
-    if gui_changed or (gui_branch is not None):
+    if gui_changed or juju_gui_source_changed:
         with su('root'):
-            service_control('nginx', STOP)
             service_control(GUI, STOP)
         console_enabled = config.get('juju-gui-console-enabled')
         start_gui(juju_api_port, console_enabled, staging)

=== modified file 'hooks/install'
--- hooks/install	2012-12-18 17:46:09 +0000
+++ hooks/install	2012-12-20 18:04:24 +0000
@@ -4,7 +4,7 @@
 from subprocess import (
     CalledProcessError,
     check_call,
-    )
+)
 
 # python-shelltoolbox is installed as a dependency of python-charmhelpers.
 check_call(['apt-get', 'install', '-y', 'python-charmhelpers'])
@@ -24,16 +24,19 @@
 )
 
 from utils import (
-    build,
     cmd_log,
     config_json,
-    fetch,
-    )
+    fetch_api,
+    fetch_gui,
+    setup_gui,
+    setup_nginx,
+)
 
 
 DEB_DEPENDENCIES = (
     'bzr', 'imagemagick', 'make', 'nginx', 'nodejs', 'npm', 'openssl',
-    'zookeeper')
+    'python-launchpadlib', 'zookeeper',
+)
 
 
 def get_dependencies():
@@ -45,8 +48,11 @@
 def main():
     config = get_config()
     get_dependencies()
-    fetch(config['juju-gui-branch'], config['juju-api-branch'])
-    build(config['command-log-file'], config['ssl-cert-path'])
+    release_tarball = fetch_gui(
+        config['juju-gui-source'], config['command-log-file'])
+    setup_gui(release_tarball)
+    setup_nginx(config['ssl-cert-path'])
+    fetch_api(config['juju-api-branch'])
     config_json.set(config)
 
 

=== modified file 'hooks/start'
--- hooks/start	2012-12-19 11:49:41 +0000
+++ hooks/start	2012-12-20 18:04:24 +0000
@@ -21,7 +21,8 @@
     log('Exposing services.')
     # Open the Juju GUI web server HTTP and HTTPS ports.
     open_port(80)
-    open_port(443)
+    # Uncomment to switch back to TLS connections.
+    # open_port(443)
     # Open the Juju websocket server port.
     open_port(juju_api_port)
 

=== modified file 'hooks/stop'
--- hooks/stop	2012-12-18 13:23:42 +0000
+++ hooks/stop	2012-12-20 18:04:24 +0000
@@ -2,6 +2,7 @@
 #-*- python -*-
 
 from charmhelpers import (
+    get_config,
     log_entry,
     log_exit,
 )
@@ -9,9 +10,14 @@
 from utils import stop
 
 
+def main():
+    config = get_config()
+    stop(config.get('staging'))
+
+
 if __name__ == '__main__':
     log_entry()
     try:
-        stop()
+        main()
     finally:
         log_exit()

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2012-12-19 11:49:41 +0000
+++ hooks/utils.py	2012-12-20 18:04:24 +0000
@@ -2,17 +2,27 @@
 
 __all__ = [
     'AGENT',
-    'build',
+    'bzr_checkout',
     'cmd_log',
+    'CURRENT_DIR',
+    'fetch_api',
+    'fetch_gui',
+    'first_path_in_dir',
+    'get_release_file_url',
     'get_zookeeper_address',
     'GUI',
     'IMPROV',
+    'JUJU_DIR',
+    'JUJU_GUI_DIR',
+    'parse_source',
     'render_to_file',
+    'setup_gui',
+    'setup_nginx',
     'start_agent',
     'start_gui',
     'start_improv',
     'stop',
-    ]
+]
 
 import json
 import os
@@ -20,15 +30,15 @@
 import shutil
 import tempfile
 
+from launchpadlib.launchpad import Launchpad
 from shelltoolbox import (
-    cd,
     command,
     environ,
     run,
     search_file,
     Serializer,
     su,
-    )
+)
 from charmhelpers import (
     get_config,
     log,
@@ -48,6 +58,50 @@
 
 # Store the configuration from on invocation to the next.
 config_json = Serializer('/tmp/config.json')
+# Bazaar checkout command.
+bzr_checkout = command('bzr', 'co', '--lightweight')
+
+
+def first_path_in_dir(directory):
+    """Return the full path of the first file/dir in *directory*."""
+    return os.path.join(directory, os.listdir(directory)[0])
+
+
+def _get_by_attr(collection, attr, value):
+    """Return the first item in collection having attr == value.
+
+    Return None if the item is not found.
+    """
+    for item in collection:
+        if getattr(item, attr) == value:
+            return item
+
+
+def get_release_file_url(project, series_name, release_version):
+    """Return the URL of the release file hosted in Launchpad.
+
+    The returned URL points to a release file for the given project, series
+    name and release version.
+    The argument *project* is a project object as returned by launchpadlib.
+    The arguments *series_name* and *release_version* are strings. If
+    *release_version* is None, the URL of the latest release will be returned.
+    """
+    series = _get_by_attr(project.series, 'name', series_name)
+    if series is None:
+        raise ValueError('%r: series not found' % series_name)
+    releases = list(series.releases)
+    if not releases:
+        raise ValueError('%r: series does not contain releases' % series_name)
+    if release_version is None:
+        release = releases[0]
+    else:
+        release = _get_by_attr(releases, 'version', release_version)
+        if not release:
+            raise ValueError('%r: release not found' % release_version)
+    files = [i for i in release.files if str(i).endswith('.tgz')]
+    if not files:
+        raise ValueError('%r: file not found' % release_version)
+    return files[0].file_link
 
 
 def get_zookeeper_address(agent_file_path):
@@ -62,6 +116,26 @@
     return line.split('=')[1].strip('"')
 
 
+def parse_source(source):
+    """Parse the ``juju-gui-source`` option.
+
+    Return a tuple of two elements representing info on how to deploy Juju GUI.
+    Examples:
+       - ('stable', None): latest stable release;
+       - ('stable', '0.1.0'): stable release v0.1.0;
+       - ('trunk', None): latest trunk release;
+       - ('trunk', '0.1.0+build.1'): trunk release v0.1.0 bzr revision 1;
+       - ('branch', 'lp:juju-gui'): release is made from a branch.
+    """
+    if source in ('stable', 'trunk'):
+        return source, None
+    if source.startswith('lp:') or source.startswith('http://'):
+        return 'branch', source
+    if 'build' in source:
+        return 'trunk', source
+    return 'stable', source
+
+
 def render_to_file(template, context, destination):
     """Render the given *template* into *destination* using *context*.
 
@@ -112,9 +186,7 @@
         'port': juju_api_port,
         'staging_env': staging_env,
     }
-    render_to_file(
-        'juju-api-improv.conf.template', context,
-        config_path)
+    render_to_file('juju-api-improv.conf.template', context, config_path)
     log('Starting the staging backend.')
     with su('root'):
         service_control(IMPROV, START)
@@ -132,9 +204,7 @@
         'port': juju_api_port,
         'zookeeper': zookeeper,
     }
-    render_to_file(
-        'juju-api-agent.conf.template', context,
-        config_path)
+    render_to_file('juju-api-agent.conf.template', context, config_path)
     log('Starting API agent.')
     with su('root'):
         service_control(AGENT, START)
@@ -150,8 +220,7 @@
     build_dir = JUJU_GUI_DIR + '/build-'
     build_dir += 'debug' if staging else 'prod'
     log('Setting up Juju GUI start up script.')
-    render_to_file(
-        'juju-gui.conf.template', {}, config_path)
+    render_to_file('juju-gui.conf.template', {}, config_path)
     log('Generating the Juju GUI configuration file.')
     context = {
         'address': unit_get('public-address'),
@@ -161,29 +230,24 @@
     if config_js_path is None:
         config_js_path = os.path.join(
             build_dir, 'juju-ui', 'assets', 'config.js')
-    render_to_file(
-        'config.js.template', context,
-        config_js_path)
+    render_to_file('config.js.template', context, config_js_path)
     log('Generating the nginx site configuration file.')
     context = {
         'server_root': build_dir
     }
-    render_to_file(
-        'nginx.conf.template', context, nginx_path)
+    render_to_file('nginx.conf.template', context, nginx_path)
     log('Starting Juju GUI.')
     with su('root'):
-        # Stop nginx so it will restart cleanly with the gui.
-        service_control('nginx', STOP)
+        # Start the Juju GUI.
         service_control(GUI, START)
 
 
-def stop():
+def stop(staging):
     """Stop the Juju API agent."""
-    config = get_config()
     with su('root'):
         log('Stopping Juju GUI.')
         service_control(GUI, STOP)
-        if config.get('staging'):
+        if staging:
             log('Stopping the staging backend.')
             service_control(IMPROV, STOP)
         else:
@@ -191,27 +255,60 @@
             service_control(AGENT, STOP)
 
 
-def fetch(juju_gui_branch, juju_api_branch):
-    """Install required dependencies and retrieve Juju/Juju GUI branches."""
-    log('Retrieving source checkouts.')
-    bzr_checkout = command('bzr', 'co', '--lightweight')
-    if juju_gui_branch is not None:
-        cmd_log(run('rm', '-rf', 'juju-gui'))
-        cmd_log(bzr_checkout(juju_gui_branch, 'juju-gui'))
-    if juju_api_branch is not None:
-        cmd_log(run('rm', '-rf', 'juju'))
-        cmd_log(bzr_checkout(juju_api_branch, 'juju'))
-
-
-def build(logpath, ssl_cert_path):
-    """Set up Juju GUI and nginx."""
-    log('Building Juju GUI.')
-    with environ(NO_BZR='1'):
-        with cd('juju-gui'):
-            logdir = os.path.dirname(logpath)
-            fd, name = tempfile.mkstemp(prefix='make-', dir=logdir)
-            log('Output from "make" sent to', name)
-            run('make', stdout=fd, stderr=fd)
+def fetch_gui(juju_gui_source, logpath):
+    """Retrieve the Juju GUI release/branch."""
+    # Retrieve a Juju GUI release.
+    origin, version_or_branch = parse_source(juju_gui_source)
+    if origin == 'branch':
+        # Create a release starting from a branch.
+        juju_gui_source_dir = os.path.join(CURRENT_DIR, 'juju-gui-source')
+        log('Retrieving Juju GUI source checkouts.')
+        cmd_log(run('rm', '-rf', juju_gui_source_dir))
+        cmd_log(bzr_checkout(version_or_branch, juju_gui_source_dir))
+        log('Preparing a Juju GUI release.')
+        logdir = os.path.dirname(logpath)
+        fd, name = tempfile.mkstemp(prefix='make-distfile-', dir=logdir)
+        log('Output from "make distfile" sent to', name)
+        with environ(NO_BZR='1'):
+            run('make', '-C', juju_gui_source_dir, 'distfile',
+                stdout=fd, stderr=fd)
+        release_tarball = first_path_in_dir(
+            os.path.join(juju_gui_source_dir, 'releases'))
+    else:
+        # Retrieve a release from Launchpad.
+        log('Retrieving Juju GUI release.')
+        launchpad = Launchpad.login_anonymously('Juju GUI charm', 'production')
+        project = launchpad.projects['juju-gui']
+        file_url = get_release_file_url(project, origin, version_or_branch)
+        log('Downloading release file from %s.' % file_url)
+        release_tarball = os.path.join(CURRENT_DIR, 'release.tgz')
+        cmd_log(run('curl', '-L', '-o', release_tarball, file_url))
+    return release_tarball
+
+
+def fetch_api(juju_api_branch):
+    """Retrieve the Juju branch."""
+    # Retrieve Juju API source checkout.
+    log('Retrieving Juju API source checkout.')
+    cmd_log(run('rm', '-rf', JUJU_DIR))
+    cmd_log(bzr_checkout(juju_api_branch, JUJU_DIR))
+
+
+def setup_gui(release_tarball):
+    """Set up Juju GUI."""
+    # Uncompress the release tarball.
+    log('Installing Juju GUI.')
+    release_dir = os.path.join(CURRENT_DIR, 'release')
+    cmd_log(run('rm', '-rf', release_dir))
+    os.mkdir(release_dir)
+    uncompress = command('tar', '-x', '-z', '-C', release_dir, '-f')
+    cmd_log(uncompress(release_tarball))
+    # Link the Juju GUI dir to the contents of the release tarball.
+    cmd_log(run('ln', '-sf', first_path_in_dir(release_dir), JUJU_GUI_DIR))
+
+
+def setup_nginx(ssl_cert_path):
+    """Set up nginx."""
     log('Setting up nginx.')
     nginx_default_site = '/etc/nginx/sites-enabled/default'
     juju_gui_site = '/etc/nginx/sites-available/juju-gui'

=== modified file 'tests/deploy.test'
--- tests/deploy.test	2012-12-20 10:52:39 +0000
+++ tests/deploy.test	2012-12-20 18:04:24 +0000
@@ -35,7 +35,7 @@
 
     def setUp(self):
         self.charm = 'juju-gui'
-        self.port = '443'
+        self.port = '80'  # Set to 443 when TLS connections are re-enabled.
 
     def tearDown(self):
         juju('destroy-service', self.charm)
@@ -53,7 +53,8 @@
 
     def check_services(self, hostname, ws_port=8080):
         """Check the services are listening on their tcp ports."""
-        url = 'https://{0}:{1}'.format(hostname, self.port)
+        # Use https below when TLS connections are re-enabled.
+        url = 'http://{0}:{1}'.format(hostname, self.port)
         response = open_url(url)
         self.assertEqual(200, response.getcode())
         ws_url = 'http://{0}:{1}/ws'.format(hostname, ws_port)
@@ -104,7 +105,6 @@
 
     def test_staging(self):
         # Ensure the Juju GUI and improv services are correctly set up.
-        self.services = ('juju-api-improv', 'juju-gui')
         config = {self.charm: {'staging': 'True'}}
         config_file = make_charm_config_file(config)
         hostname = self.deploy(config_path=config_file.name)
@@ -113,6 +113,16 @@
             self.stop_services, hostname, ['juju-api-improv', 'juju-gui'])
         self.check_services(hostname)
 
+    def test_branch_source(self):
+        # Ensure the Juju GUI is correctly deployed from a Bazaar branch.
+        config = {self.charm: {'juju-gui-source': 'lp:juju-gui'}}
+        config_file = make_charm_config_file(config)
+        hostname = self.deploy(config_path=config_file.name)
+        # XXX 2012-11-29 frankban bug=872264: see *stop_services* above.
+        self.addCleanup(
+            self.stop_services, hostname, ['juju-api-agent', 'juju-gui'])
+        self.check_services(hostname)
+
 
 class DeployToTest(DeployTestMixin, unittest.TestCase):
 

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2012-12-18 13:23:42 +0000
+++ tests/test_utils.py	2012-12-20 18:04:24 +0000
@@ -2,14 +2,19 @@
 
 from contextlib import contextmanager
 import os
+import shutil
+from simplejson import dumps
 import tempfile
 import unittest
+
 import charmhelpers
-from simplejson import dumps
-
 from utils import (
+    _get_by_attr,
     cmd_log,
+    first_path_in_dir,
+    get_release_file_url,
     get_zookeeper_address,
+    parse_source,
     render_to_file,
     start_agent,
     start_gui,
@@ -20,6 +25,205 @@
 import utils
 
 
+class AttrDict(dict):
+    """A dict with the ability to access keys as attributes."""
+
+    def __getattr__(self, attr):
+        if attr in self:
+            return self[attr]
+        raise AttributeError
+
+
+class AttrDictTest(unittest.TestCase):
+
+    def test_key_as_attribute(self):
+        # Ensure attributes can be used to retrieve dict values.
+        attr_dict = AttrDict(myattr='myvalue')
+        self.assertEqual('myvalue', attr_dict.myattr)
+
+    def test_attribute_not_found(self):
+        # An AttributeError is raised if the dict does not contain an attribute
+        # corresponding to an existent key.
+        with self.assertRaises(AttributeError):
+            AttrDict().myattr
+
+
+class FirstPathInDirTest(unittest.TestCase):
+
+    def setUp(self):
+        self.directory = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.directory)
+        self.path = os.path.join(self.directory, 'file_or_dir')
+
+    def test_file_path(self):
+        # Ensure the full path of a file is correctly returned.
+        open(self.path, 'w').close()
+        self.assertEqual(self.path, first_path_in_dir(self.directory))
+
+    def test_directory_path(self):
+        # Ensure the full path of a directory is correctly returned.
+        os.mkdir(self.path)
+        self.assertEqual(self.path, first_path_in_dir(self.directory))
+
+    def test_empty_directory(self):
+        # An IndexError is raised if the directory is empty.
+        self.assertRaises(IndexError, first_path_in_dir, self.directory)
+
+
+def make_collection(attr, values):
+    """Create a collection of objects having an attribute named *attr*.
+
+    The value of the *attr* attribute, for each instance, is taken from
+    the *values* sequence.
+    """
+    return [AttrDict({attr: value}) for value in values]
+
+
+class MakeCollectionTest(unittest.TestCase):
+
+    def test_factory(self):
+        # Ensure the factory returns the expected object instances.
+        instances = make_collection('myattr', range(5))
+        self.assertEqual(5, len(instances))
+        for num, instance in enumerate(instances):
+            self.assertEqual(num, instance.myattr)
+
+
+class GetByAttrTest(unittest.TestCase):
+
+    attr = 'myattr'
+    collection = make_collection(attr, range(5))
+
+    def test_item_found(self):
+        # Ensure an object instance is correctly returned if found in
+        # the collection.
+        item = _get_by_attr(self.collection, self.attr, 3)
+        self.assertEqual(3, item.myattr)
+
+    def test_value_not_found(self):
+        # None is returned if the collection does not contain the requested
+        # item.
+        item = _get_by_attr(self.collection, self.attr, '__does_not_exist__')
+        self.assertIsNone(item)
+
+    def test_attr_not_found(self):
+        # An AttributeError is raised if items in collection does not have the
+        # required attribute.
+        with self.assertRaises(AttributeError):
+            _get_by_attr(self.collection, 'another_attr', 0)
+
+
+class FileStub(object):
+    """Simulate a Launchpad hosted file returned by launchpadlib."""
+
+    def __init__(self, file_link):
+        self.file_link = file_link
+
+    def __str__(self):
+        return self.file_link
+
+
+class GetReleaseFileUrlTest(unittest.TestCase):
+
+    project = AttrDict(
+        series=(
+            AttrDict(
+                name='stable',
+                releases=(
+                    AttrDict(
+                        version='0.1.1',
+                        files=(
+                            FileStub('http://example.com/0.1.1.dmg'),
+                            FileStub('http://example.com/0.1.1.tgz'),
+                        ),
+                    ),
+                    AttrDict(
+                        version='0.1.0',
+                        files=(
+                            FileStub('http://example.com/0.1.0.dmg'),
+                            FileStub('http://example.com/0.1.0.tgz'),
+                        ),
+                    ),
+                ),
+            ),
+            AttrDict(
+                name='trunk',
+                releases=(
+                    AttrDict(
+                        version='0.1.1+build.1',
+                        files=(
+                            FileStub('http://example.com/0.1.1+build.1.dmg'),
+                            FileStub('http://example.com/0.1.1+build.1.tgz'),
+                        ),
+                    ),
+                    AttrDict(
+                        version='0.1.0+build.1',
+                        files=(
+                            FileStub('http://example.com/0.1.0+build.1.dmg'),
+                            FileStub('http://example.com/0.1.0+build.1.tgz'),
+                        ),
+                    ),
+                ),
+            ),
+        ),
+    )
+
+    def test_latest_stable_release(self):
+        # Ensure the correct URL is returned for the latest stable release.
+        url = get_release_file_url(self.project, 'stable', None)
+        self.assertEqual('http://example.com/0.1.1.tgz', url)
+
+    def test_latest_trunk_release(self):
+        # Ensure the correct URL is returned for the latest trunk release.
+        url = get_release_file_url(self.project, 'trunk', None)
+        self.assertEqual('http://example.com/0.1.1+build.1.tgz', url)
+
+    def test_specific_stable_release(self):
+        # Ensure the correct URL is returned for a specific version of the
+        # stable release.
+        url = get_release_file_url(self.project, 'stable', '0.1.0')
+        self.assertEqual('http://example.com/0.1.0.tgz', url)
+
+    def test_specific_trunk_release(self):
+        # Ensure the correct URL is returned for a specific version of the
+        # trunk release.
+        url = get_release_file_url(self.project, 'trunk', '0.1.0+build.1')
+        self.assertEqual('http://example.com/0.1.0+build.1.tgz', url)
+
+    def test_series_not_found(self):
+        # A ValueError is raised if the series cannot be found.
+        with self.assertRaises(ValueError) as cm:
+            get_release_file_url(self.project, 'unstable', None)
+        self.assertIn('series not found', str(cm.exception))
+
+    def test_no_releases(self):
+        # A ValueError is raised if the series does not contain releases.
+        project = AttrDict(series=[AttrDict(name='stable', releases=[])])
+        with self.assertRaises(ValueError) as cm:
+            get_release_file_url(project, 'stable', None)
+        self.assertIn('series does not contain releases', str(cm.exception))
+
+    def test_release_not_found(self):
+        # A ValueError is raised if the release cannot be found.
+        with self.assertRaises(ValueError) as cm:
+            get_release_file_url(self.project, 'stable', '2.0')
+        self.assertIn('release not found', str(cm.exception))
+
+    def test_file_not_found(self):
+        # A ValueError is raised if the hosted file cannot be found.
+        project = AttrDict(
+            series=[
+                AttrDict(
+                    name='stable',
+                    releases=[AttrDict(version='0.1.0', files=[])],
+                ),
+            ],
+        )
+        with self.assertRaises(ValueError) as cm:
+            get_release_file_url(project, 'stable', None)
+        self.assertIn('file not found', str(cm.exception))
+
+
 class GetZookeeperAddressTest(unittest.TestCase):
 
     def setUp(self):
@@ -36,6 +240,35 @@
         self.assertEqual(self.zookeeper_address, address)
 
 
+class ParseSourceTest(unittest.TestCase):
+
+    def test_latest_stable_release(self):
+        # Ensure the latest stable release is correctly parsed.
+        expected = ('stable', None)
+        self.assertTupleEqual(expected, parse_source('stable'))
+
+    def test_latest_trunk_release(self):
+        # Ensure the latest trunk release is correctly parsed.
+        expected = ('trunk', None)
+        self.assertTupleEqual(expected, parse_source('trunk'))
+
+    def test_stable_release(self):
+        # Ensure a specific stable release is correctly parsed.
+        expected = ('stable', '0.1.0')
+        self.assertTupleEqual(expected, parse_source('0.1.0'))
+
+    def test_trunk_release(self):
+        # Ensure a specific trunk release is correctly parsed.
+        expected = ('trunk', '0.1.0+build.1')
+        self.assertTupleEqual(expected, parse_source('0.1.0+build.1'))
+
+    def test_bzr_branch(self):
+        # Ensure a Bazaar branch is correctly parsed.
+        sources = ('lp:example', 'http://bazaar.launchpad.net/example')
+        for source in sources:
+            self.assertTupleEqual(('branch', source), parse_source(source))
+
+
 class RenderToFileTest(unittest.TestCase):
 
     def setUp(self):
@@ -155,22 +388,18 @@
         self.assertTrue('/usr/sbin/nginx' in conf)
         nginx_conf = nginx_file.read()
         self.assertTrue('juju-gui/build-debug' in nginx_conf)
-        self.assertEqual(self.svc_ctl_call_count, 2)
-        self.assertEqual(self.service_names, ['nginx', 'juju-gui'])
-        self.assertEqual(self.actions, [charmhelpers.STOP, charmhelpers.START])
+        self.assertEqual(self.svc_ctl_call_count, 1)
+        self.assertEqual(self.service_names, ['juju-gui'])
+        self.assertEqual(self.actions, [charmhelpers.START])
 
     def test_stop_staging(self):
-        mock_config = {'staging': True}
-        charmhelpers.command = lambda *args: lambda: dumps(mock_config)
-        stop()
+        stop(True)
         self.assertEqual(self.svc_ctl_call_count, 2)
         self.assertEqual(self.service_names, ['juju-gui', 'juju-api-improv'])
         self.assertEqual(self.actions, [charmhelpers.STOP, charmhelpers.STOP])
 
     def test_stop_production(self):
-        mock_config = {'staging': False}
-        charmhelpers.command = lambda *args: lambda: dumps(mock_config)
-        stop()
+        stop(False)
         self.assertEqual(self.svc_ctl_call_count, 2)
         self.assertEqual(self.service_names, ['juju-gui', 'juju-api-agent'])
         self.assertEqual(self.actions, [charmhelpers.STOP, charmhelpers.STOP])