yellow team mailing list archive

[Francesco Banconi <francesco.banconi@xxxxxxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/charms/precise/juju-gui/bug-1088618-serve-releases into lp:~juju-gui/charms/precise/juju-gui/trunk.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1088618 in juju-gui: "charm should serve only releases (or make a release from a branch)"
  https://bugs.launchpad.net/juju-gui/+bug/1088618

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

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/

-- 
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.

=== modified file 'config.yaml'
--- config.yaml	2012-12-07 18:39:00 +0000
+++ config.yaml	2012-12-19 18:05:33 +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 'hooks/config-changed'
--- hooks/config-changed	2012-12-18 13:23:42 +0000
+++ hooks/config-changed	2012-12-19 18:05:33 +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_api,
+    setup_gui,
     start_agent,
     start_gui,
     start_improv,
@@ -39,33 +39,31 @@
     # 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']
+
+    juju_gui_source_changed = False
+    juju_api_branch_changed = False
+
+    # Fetch new sources?
+    # Restarting of the gui and api services is handled below.
+    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)
     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'])
-        # Restarting of the gui and api services is handled below.
+        juju_api_branch_changed = True
+        fetch_api(config['juju-api-branch'])
+        setup_api()
 
     # 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 +74,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 +90,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 13:23:42 +0000
+++ hooks/install	2012-12-19 18:05:33 +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,15 +24,19 @@
 )
 
 from utils import (
-    build,
     cmd_log,
     config_json,
-    fetch,
-    )
+    fetch_api,
+    fetch_gui,
+    setup_api,
+    setup_gui,
+)
 
 
 DEB_DEPENDENCIES = (
-    'bzr', 'imagemagick', 'make', 'nginx', 'nodejs', 'npm', 'zookeeper')
+    'bzr', 'imagemagick', 'make', 'nginx', 'nodejs', 'npm',
+    'python-launchpadlib', 'zookeeper',
+)
 
 
 def get_dependencies():
@@ -44,8 +48,11 @@
 def main():
     config = get_config()
     get_dependencies()
-    fetch(config['juju-gui-branch'], config['juju-api-branch'])
-    build(config['command-log-file'])
+    release_tarball = fetch_gui(
+        config['juju-gui-source'], config['command-log-file'])
+    setup_gui(release_tarball)
+    fetch_api(config['juju-api-branch'])
+    setup_api()
     config_json.set(config)
 
 

=== modified file 'hooks/stop'
--- hooks/stop	2012-12-18 13:23:42 +0000
+++ hooks/stop	2012-12-19 18:05:33 +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-18 13:23:42 +0000
+++ hooks/utils.py	2012-12-19 18:05:33 +0000
@@ -2,31 +2,42 @@
 
 __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_api',
+    'setup_gui',
     'start_agent',
     'start_gui',
     'start_improv',
     'stop',
-    ]
+]
 
 import json
 import os
 import logging
 import tempfile
 
+from launchpadlib.launchpad import Launchpad
 from shelltoolbox import (
-    cd,
     command,
+    environ,
     run,
     search_file,
     Serializer,
     su,
-    )
+)
 from charmhelpers import (
     get_config,
     log,
@@ -46,6 +57,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):
@@ -60,6 +115,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*.
 
@@ -110,9 +185,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)
@@ -130,9 +203,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)
@@ -148,8 +219,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'),
@@ -159,29 +229,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:
@@ -189,26 +254,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):
-    """Set up Juju GUI and nginx."""
-    log('Building Juju GUI.')
-    with cd('juju-gui'):
+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-', dir=logdir)
-        log('Output from "make" sent to', name)
-        run('make', stdout=fd, stderr=fd)
+        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_api():
+    """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-12 17:29:39 +0000
+++ tests/deploy.test	2012-12-19 18:05:33 +0000
@@ -104,7 +104,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 +112,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-19 18:05:33 +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 return 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):
+    """Simulates 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])