Merge lp:~bac/charms/precise/juju-gui/config-changed into lp:~juju-gui/charms/precise/juju-gui/trunk

[Brad Crittenden]

Add support for config-changed

The config-changed hook has been added. In order to share code from the
install hook, common routines were moved to utils. Those routines have unit
tests now, except for 'build' which didn't look like much.

Many of the functions in utils were modified for testability where they needed
to write files.

There is an issue lurking. Using 'juju set juju-gui juju-gui-branch=lp:<some
branch> seems to leave nginx in an odd state. It is started but it cannot
find the document root so it may be a race condition. I'll continue to hunt
for the problem while this cut is in review.

[Gary Poster (gary)]

Thank you, Brad. The code is very clean, the tests are fast, and this is nice functionality.

The lurking issue you mentioned *may* be irrelevant when Francesco starts work on bug 188618.

As I understand it, the config-changed hook is called initially, after install and before start. See Kapil's answer to http://askubuntu.com/questions/82683/what-juju-charm-hooks-are-available-and-what-does-each-one-do and the discussion of config-changed that he links to in https://juju.ubuntu.com/docs/drafts/service-config.html#creating-charms . This means that the install hook can sometimes be wildly simple: it should be able to defer all of its work to the config-changed hook. The only trick then, AFAIK, is that config-changed needs to be able to detect if things have been started yet, and not restart if things have started...or, it should embrace making the start hook irrelevant, perhaps, and always start, as you do now. Checking with Kapil or Ben or a charmer about my understanding and related charm best practices would be a reasonable step for all of these observations.

To be honest, my big unhappiness and concern with this branch is that it is blocking Francesco starting cleanly on bug 1088618 tomorrow morning. That is something we must have to stay on schedule. We didn't need the config-changed functionality in order to meet the letter of the law, and as you know, I like to fulfill what we need and *then* back-fill the other bits that we have time for, if we still believe them to be important. I didn't communicate my position on this config-changed fix clearly in that regard, and it's my fault.

There are a number of ways to resolve this, but under the circumstances, I suggest the following. Francesco should start work tomorrow morning with trunk, merged with this branch. He should feel free to make any changes to it that he needs, ignoring my discussion of possible simplification above unless he needs to address it for his goals. He can propose his branch with your branch as a dependency.

You can pair with him to get his branch moving more quickly, if possible. Maybe in your morning you can resolve the answer to the question I raised at the top, and then in the afternoon he can pass the branch to you to take it farther after his EoD.

Francesco, please communicate with Nicola about this situation in case he wants to start bug 1083920: as few changes to the charm as possible during your work will be helpful.

Thanks again, Brad. This is good work. I want it to land, but there's some work to be done on it yet, and I don't want to have it at the expense of our schedule, so let's see if the coordination with Francesco that I propose might work.

Gary

[lp:~bac/charms/precise/juju-gui/config-changed]

21. By Brad Crittenden on 2012-12-18

Merged Francesco's changes. Cleanup, lint, and test changes after merging with trunk.

[Gary Poster (gary)]

Francesco's changes fix the problems you described, and he is not blocked.

Please make a bug/card to at least consider simplifying the install/config-changed/start situation, as I described above (or feel free to ask me to if you do not feel comfortable with writing the bug text).

Thank you!

Gary

review: Approve

[Nicola Larosa (teknico)]

Code looks good to me. Mind you, I only looked at the code, I did not actually run it, though.

review: Approve

=== added file '.bzrignore'
--- .bzrignore	1970-01-01 00:00:00 +0000
+++ .bzrignore	2012-12-18 13:27:18 +0000
@@ -0,0 +1,1 @@
+.emacs*
 
=== added file 'hooks/config-changed'
--- hooks/config-changed	1970-01-01 00:00:00 +0000
+++ hooks/config-changed	2012-12-18 13:27:18 +0000
@@ -0,0 +1,131 @@
+#!/usr/bin/env python2
+# -*- python -*-
+
+# 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
+
+from charmhelpers import (
+    get_config,
+    log,
+    log_entry,
+    log_exit,
+    service_control,
+    STOP,
+)
+from shelltoolbox import (
+    DictDiffer,
+    su,
+)
+
+from utils import (
+    AGENT,
+    build,
+    config_json,
+    fetch,
+    GUI,
+    IMPROV,
+    start_agent,
+    start_gui,
+    start_improv,
+)
+
+
+def handle_config_changes(config, diff):
+    # Handle all configuration file changes.
+    log('Updating configuration.')
+
+    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']
+    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.
+
+    # 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' in added_or_changed:
+                # 'staging' went from False to True, so the agent server is
+                # running and must be stopped.
+                current_api = AGENT
+            else:
+                # Only staging parameters changed, so the existing staging
+                # server must be stopped and later restarted.
+                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)
+    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 'staging' in added_or_changed:
+                # If 'staging' transitions to False we need to stop the backend
+                # and start the agent.
+                current_api = IMPROV
+            else:
+                # The agent is still running but the configuration has been
+                # updated -- bounce it.
+                current_api = AGENT
+            service_control(current_api, STOP)
+            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):
+        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)
+
+
+def main():
+    config = get_config()
+    prev_config = config_json.get()
+    diff = DictDiffer(config, prev_config)
+
+    if not diff.modified:
+        log("No configuration changes, exiting.")
+        sys.exit(0)
+    handle_config_changes(config, diff)
+    config_json.set(config)
+
+
+if __name__ == '__main__':
+    log_entry()
+    try:
+        main()
+    except CalledProcessError as e:
+        log('Exception caught:')
+        log(e.output)
+        raise
+    finally:
+        log_exit()
 
=== modified file 'hooks/install'
--- hooks/install	2012-12-12 17:29:39 +0000
+++ hooks/install	2012-12-18 13:27:18 +0000
@@ -1,12 +1,10 @@
 #!/usr/bin/env python2
 # -*- python -*-
 
-import os
 from subprocess import (
     CalledProcessError,
     check_call,
     )
-import tempfile
 
 # python-shelltoolbox is installed as a dependency of python-charmhelpers.
 check_call(['apt-get', 'install', '-y', 'python-charmhelpers'])
@@ -22,53 +20,33 @@
 )
 from shelltoolbox import (
     apt_get_install,
-    cd,
-    command,
     install_extra_repositories,
-    run,
 )
 
-from utils import cmd_log
-
-
-def fetch(juju_gui_branch, juju_api_branch):
-    """Install required dependencies and retrieve Juju/Juju GUI branches."""
+from utils import (
+    build,
+    cmd_log,
+    config_json,
+    fetch,
+    )
+
+
+DEB_DEPENDENCIES = (
+    'bzr', 'imagemagick', 'make', 'nginx', 'nodejs', 'npm', 'zookeeper')
+
+
+def get_dependencies():
     log('Installing dependencies.')
     cmd_log(install_extra_repositories('ppa:chris-lea/node.js'))
-    cmd_log(
-        apt_get_install('bzr', 'imagemagick', 'make',
-                        'nodejs', 'npm', 'zookeeper', 'nginx'))
-    log('Retrieving source checkouts.')
-    bzr_checkout = command('bzr', 'co', '--lightweight')
-    cmd_log(bzr_checkout(juju_gui_branch, 'juju-gui'))
-    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'):
-        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)
-    log('Setting up nginx.')
-    nginx_default_site = '/etc/nginx/sites-enabled/default'
-    juju_gui_site = '/etc/nginx/sites-available/juju-gui'
-    if os.path.exists(nginx_default_site):
-        os.remove(nginx_default_site)
-    if not os.path.exists(juju_gui_site):
-        cmd_log(run('touch', juju_gui_site))
-        cmd_log(run('chown', 'ubuntu:', juju_gui_site))
-        cmd_log(
-            run('ln', '-s', juju_gui_site,
-                '/etc/nginx/sites-enabled/juju-gui'))
+    cmd_log(apt_get_install(*DEB_DEPENDENCIES))
 
 
 def main():
     config = get_config()
+    get_dependencies()
     fetch(config['juju-gui-branch'], config['juju-api-branch'])
     build(config['command-log-file'])
+    config_json.set(config)
 
 
 if __name__ == '__main__':
 
=== modified file 'hooks/start'
--- hooks/start	2012-12-12 11:42:12 +0000
+++ hooks/start	2012-12-18 13:27:18 +0000
@@ -1,100 +1,20 @@
 #!/usr/bin/env python2
 #-*- python -*-
 
-import json
-import os
-
 from charmhelpers import (
     get_config,
     log,
     log_entry,
     log_exit,
     open_port,
-    service_control,
-    START,
-    unit_get,
-)
-from shelltoolbox import (
-    run,
-    su,
 )
 
 from utils import (
-    get_zookeeper_address,
-    render_to_file,
+    start_agent,
+    start_gui,
+    start_improv,
 )
 
-CURRENT_DIR = os.getcwd()
-JUJU_DIR = os.path.join(CURRENT_DIR, 'juju')
-JUJU_GUI_DIR = os.path.join(CURRENT_DIR, 'juju-gui')
-
-
-def start_gui(juju_api_port, console_enabled, staging):
-    """Set up and start the Juju GUI server."""
-    with su('root'):
-        run('chown', '-R', 'ubuntu:', JUJU_GUI_DIR)
-    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', {'juju_gui_dir': JUJU_GUI_DIR},
-        '/etc/init/juju-gui.conf')
-    log('Generating the Juju GUI configuration file.')
-    context = {
-        'address': unit_get('public-address'),
-        'console_enabled': json.dumps(console_enabled),
-        'port': juju_api_port,
-    }
-    render_to_file(
-        'config.js.template', context,
-        os.path.join(build_dir, 'juju-ui', 'assets', 'config.js'))
-    log('Generating the nginx site configuration file.')
-    context = {
-        'server_root': build_dir
-    }
-    render_to_file(
-        'nginx.conf.template', context,
-        '/etc/nginx/sites-available/juju-gui')
-    log('Starting Juju GUI.')
-    with su('root'):
-        service_control('juju-gui', START)
-
-
-def start_improv(juju_api_port, staging_env):
-    """Start a simulated juju environment using ``improv.py``."""
-    log('Setting up staging start up script.')
-    context = {
-        'juju_dir': JUJU_DIR,
-        'port': juju_api_port,
-        'staging_env': staging_env,
-    }
-    render_to_file(
-        'juju-api-improv.conf.template', context,
-        '/etc/init/juju-api-improv.conf')
-    log('Starting the staging backend.')
-    with su('root'):
-        service_control('juju-api-improv', START)
-
-
-def start_agent(juju_api_port):
-    """Start the Juju agent and connect to the current environment."""
-    # Retrieve the Zookeeper address from the start up script.
-    unit_dir = os.path.realpath(os.path.join(CURRENT_DIR, '..'))
-    agent_file = '/etc/init/juju-{0}.conf'.format(os.path.basename(unit_dir))
-    zookeeper = get_zookeeper_address(agent_file)
-    log('Setting up API agent start up script.')
-    context = {
-        'juju_dir': JUJU_DIR,
-        'port': juju_api_port,
-        'zookeeper': zookeeper,
-    }
-    render_to_file(
-        'juju-api-agent.conf.template', context,
-        '/etc/init/juju-api-agent.conf')
-    log('Starting API agent.')
-    with su('root'):
-        service_control('juju-api-agent', START)
-
 
 def open_ports(juju_api_port):
     """Expose Juju GUI web server and websocket server ports."""
 
=== modified file 'hooks/stop'
--- hooks/stop	2012-12-10 14:37:15 +0000
+++ hooks/stop	2012-12-18 13:27:18 +0000
@@ -2,28 +2,11 @@
 #-*- python -*-
 
 from charmhelpers import (
-    get_config,
-    log,
     log_entry,
     log_exit,
-    service_control,
-    STOP,
 )
-from shelltoolbox import su
-
-
-def stop():
-    """Stop the Juju API agent."""
-    config = get_config()
-    with su('root'):
-        log('Stopping Juju GUI.')
-        service_control('juju-gui', STOP)
-        if config.get('staging'):
-            log('Stopping the staging backend.')
-            service_control('juju-api-improv', STOP)
-        else:
-            log('Stopping API agent.')
-            service_control('juju-api-agent', STOP)
+
+from utils import stop
 
 
 if __name__ == '__main__':
 
=== modified file 'hooks/utils.py'
--- hooks/utils.py	2012-12-07 21:16:55 +0000
+++ hooks/utils.py	2012-12-18 13:27:18 +0000
@@ -1,10 +1,51 @@
 """Juju GUI charm utilities."""
 
+__all__ = [
+    'AGENT',
+    'build',
+    'cmd_log',
+    'get_zookeeper_address',
+    'GUI',
+    'IMPROV',
+    'render_to_file',
+    'start_agent',
+    'start_gui',
+    'start_improv',
+    'stop',
+    ]
+
+import json
 import os
 import logging
-
-from shelltoolbox import search_file
-from charmhelpers import get_config
+import tempfile
+
+from shelltoolbox import (
+    cd,
+    command,
+    run,
+    search_file,
+    Serializer,
+    su,
+    )
+from charmhelpers import (
+    get_config,
+    log,
+    service_control,
+    START,
+    STOP,
+    unit_get,
+)
+
+
+AGENT = 'juju-api-agent'
+IMPROV = 'juju-api-improv'
+GUI = 'juju-gui'
+CURRENT_DIR = os.getcwd()
+JUJU_DIR = os.path.join(CURRENT_DIR, 'juju')
+JUJU_GUI_DIR = os.path.join(CURRENT_DIR, 'juju-gui')
+
+# Store the configuration from on invocation to the next.
+config_json = Serializer('/tmp/config.json')
 
 
 def get_zookeeper_address(agent_file_path):
@@ -36,6 +77,7 @@
 
 results_log = None
 
+
 def _setupLogging():
     global results_log
     if results_log is not None:
@@ -45,7 +87,7 @@
         filename=config['command-log-file'],
         level=logging.INFO,
         format="%(asctime)s: %(name)s@%(levelname)s %(message)s")
-    results_log = logging.getLogger('juju-gui')
+    results_log = logging.getLogger(GUI)
 
 
 def cmd_log(results):
@@ -57,3 +99,124 @@
     # Since 'results' may be multi-line output, start it on a separate line
     # from the logger timestamp, etc.
     results_log.info('\n' + results)
+
+
+def start_improv(juju_api_port, staging_env,
+                 config_path='/etc/init/juju-api-improv.conf'):
+    """Start a simulated juju environment using ``improv.py``."""
+    log('Setting up staging start up script.')
+    context = {
+        'juju_dir': JUJU_DIR,
+        'port': juju_api_port,
+        'staging_env': staging_env,
+    }
+    render_to_file(
+        'juju-api-improv.conf.template', context,
+        config_path)
+    log('Starting the staging backend.')
+    with su('root'):
+        service_control(IMPROV, START)
+
+
+def start_agent(juju_api_port, config_path='/etc/init/juju-api-agent.conf'):
+    """Start the Juju agent and connect to the current environment."""
+    # Retrieve the Zookeeper address from the start up script.
+    unit_dir = os.path.realpath(os.path.join(CURRENT_DIR, '..'))
+    agent_file = '/etc/init/juju-{0}.conf'.format(os.path.basename(unit_dir))
+    zookeeper = get_zookeeper_address(agent_file)
+    log('Setting up API agent start up script.')
+    context = {
+        'juju_dir': JUJU_DIR,
+        'port': juju_api_port,
+        'zookeeper': zookeeper,
+    }
+    render_to_file(
+        'juju-api-agent.conf.template', context,
+        config_path)
+    log('Starting API agent.')
+    with su('root'):
+        service_control(AGENT, START)
+
+
+def start_gui(juju_api_port, console_enabled, staging,
+              config_path='/etc/init/juju-gui.conf',
+              nginx_path='/etc/nginx/sites-available/juju-gui',
+              config_js_path=None):
+    """Set up and start the Juju GUI server."""
+    with su('root'):
+        run('chown', '-R', 'ubuntu:', JUJU_GUI_DIR)
+    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)
+    log('Generating the Juju GUI configuration file.')
+    context = {
+        'address': unit_get('public-address'),
+        'console_enabled': json.dumps(console_enabled),
+        'port': juju_api_port,
+    }
+    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)
+    log('Generating the nginx site configuration file.')
+    context = {
+        'server_root': build_dir
+    }
+    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)
+        service_control(GUI, START)
+
+
+def stop():
+    """Stop the Juju API agent."""
+    config = get_config()
+    with su('root'):
+        log('Stopping Juju GUI.')
+        service_control(GUI, STOP)
+        if config.get('staging'):
+            log('Stopping the staging backend.')
+            service_control(IMPROV, STOP)
+        else:
+            log('Stopping API agent.')
+            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'):
+        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)
+    log('Setting up nginx.')
+    nginx_default_site = '/etc/nginx/sites-enabled/default'
+    juju_gui_site = '/etc/nginx/sites-available/juju-gui'
+    if os.path.exists(nginx_default_site):
+        os.remove(nginx_default_site)
+    if not os.path.exists(juju_gui_site):
+        cmd_log(run('touch', juju_gui_site))
+        cmd_log(run('chown', 'ubuntu:', juju_gui_site))
+        cmd_log(
+            run('ln', '-s', juju_gui_site,
+                '/etc/nginx/sites-enabled/juju-gui'))
 
=== modified file 'revision'
--- revision	2012-12-14 16:48:29 +0000
+++ revision	2012-12-18 13:27:18 +0000
@@ -1,1 +1,1 @@
-15
+16
 
=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2012-12-12 17:29:39 +0000
+++ tests/test_utils.py	2012-12-18 13:27:18 +0000
@@ -1,5 +1,6 @@
 #!/usr/bin/env python2
 
+from contextlib import contextmanager
 import os
 import tempfile
 import unittest
@@ -10,7 +11,13 @@
     cmd_log,
     get_zookeeper_address,
     render_to_file,
+    start_agent,
+    start_gui,
+    start_improv,
+    stop,
 )
+# Import the whole utils package for monkey patching.
+import utils
 
 
 class GetZookeeperAddressTest(unittest.TestCase):
@@ -68,5 +75,106 @@
         self.assertTrue(line.endswith(': juju-gui@INFO \nfoo\n'))
 
 
+class StartStopTest(unittest.TestCase):
+
+    def setUp(self):
+        self.service_names = []
+        self.actions = []
+        self.svc_ctl_call_count = 0
+        self.fake_zk_address = '192.168.5.26'
+        # Monkey patches.
+        self.command = charmhelpers.command
+
+        def service_control_mock(service_name, action):
+            self.svc_ctl_call_count += 1
+            self.service_names.append(service_name)
+            self.actions.append(action)
+
+        def noop(*args):
+            pass
+
+        @contextmanager
+        def su(user):
+            yield None
+
+        def get_zookeeper_address_mock(fp):
+            return self.fake_zk_address
+
+        self.functions = dict(
+            service_control=(utils.service_control, service_control_mock),
+            log=(utils.log, noop),
+            su=(utils.su, su),
+            run=(utils.run, noop),
+            unit_get=(utils.unit_get, noop),
+            get_zookeeper_address=(
+                utils.get_zookeeper_address, get_zookeeper_address_mock)
+            )
+        # Apply the patches.
+        for fn, fcns in self.functions.items():
+            setattr(utils, fn, fcns[1])
+
+        self.destination_file = tempfile.NamedTemporaryFile()
+        self.addCleanup(self.destination_file.close)
+
+    def tearDown(self):
+        # Undo all of the monkey patching.
+        for fn, fcns in self.functions.items():
+            setattr(utils, fn, fcns[0])
+        charmhelpers.command = self.command
+
+    def test_start_improv(self):
+        port = '1234'
+        staging_env = 'large'
+        start_improv(port, staging_env, self.destination_file.name)
+        conf = self.destination_file.read()
+        self.assertTrue('--port %s' % port in conf)
+        self.assertTrue(staging_env + '.json' in conf)
+        self.assertEqual(self.svc_ctl_call_count, 1)
+        self.assertEqual(self.service_names, ['juju-api-improv'])
+        self.assertEqual(self.actions, [charmhelpers.START])
+
+    def test_start_agent(self):
+        port = '1234'
+        start_agent(port, self.destination_file.name)
+        conf = self.destination_file.read()
+        self.assertTrue('--port %s' % port in conf)
+        self.assertTrue('JUJU_ZOOKEEPER=%s' % self.fake_zk_address in conf)
+        self.assertEqual(self.svc_ctl_call_count, 1)
+        self.assertEqual(self.service_names, ['juju-api-agent'])
+        self.assertEqual(self.actions, [charmhelpers.START])
+
+    def test_start_gui(self):
+        port = '1234'
+        nginx_file = tempfile.NamedTemporaryFile()
+        self.addCleanup(nginx_file.close)
+        config_js_file = tempfile.NamedTemporaryFile()
+        self.addCleanup(config_js_file.close)
+        start_gui(port, False, True, self.destination_file.name,
+                  nginx_file.name, config_js_file.name)
+        conf = self.destination_file.read()
+        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])
+
+    def test_stop_staging(self):
+        mock_config = {'staging': True}
+        charmhelpers.command = lambda *args: lambda: dumps(mock_config)
+        stop()
+        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()
+        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])
+
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)