Merge lp:~teknico/charms/precise/juju-gui/integrate-builtin-server into lp:~juju-gui/charms/precise/juju-gui/trunk

[Nicola Larosa]

Integrate the built-in server into the charm.

Add a "builtin-server" option to config.yaml, defaulting to false, to enable
a new Tornado-based built-in web server, in place of haproxy and Apache (still
the default).

Add a "guiserver.conf" Upstart config file, generated via a template.

Add a BuiltinServerMixin to hooks/backend.py .

Factor out common code betwenn HaproxyApacheMixin and BuiltinServerMixin.

Rename options.http to options.insecure in server/guiserver/manage.py .

Integrate testing of TestBackendCommands in TestBackendCommands.

Add a test for write_builtin_server_startup.

Miscellaneuos cleanup.

Please QA by setting "builtin-server" to true in config.yaml and deploying
with "make deploy" (after bootstrapping), see "make help".

https://codereview.appspot.com/12086044/

[Nicola Larosa (teknico)]

Reviewers: mp+177907_code.launchpad.net,

Message:
Please take a look.

Description:
Integrate the built-in server into the charm.

Add a "builtin-server" option to config.yaml, defaulting to false, to
enable
a new Tornado-based built-in web server, in place of haproxy and Apache
(still
the default).

Add a "guiserver.conf" Upstart config file, generated via a template.

Add a BuiltinServerMixin to hooks/backend.py .

Factor out common code betwenn HaproxyApacheMixin and
BuiltinServerMixin.

Rename options.http to options.insecure in server/guiserver/manage.py .

Integrate testing of TestBackendCommands in TestBackendCommands.

Add a test for write_builtin_server_startup.

Miscellaneuos cleanup.

Please QA by setting "builtin-server" to true in config.yaml and
deploying
with "make deploy" (after bootstrapping), see "make help".

https://code.launchpad.net/~teknico/charms/precise/juju-gui/integrate-builtin-server/+merge/177907

(do not edit description out of merge proposal)

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

Affected files:
   M Operation.md
   A [revision details]
   M config.yaml
   A config/guiserver.conf.template
   A deps/tornado-3.1.tar.gz
   M hooks/backend.py
   M hooks/utils.py
   M server/guiserver/manage.py
   M server/guiserver/tests/test_manage.py
   M server/runserver.py
   M tests/test_backends.py
   M tests/test_utils.py

[lp:~teknico/charms/precise/juju-gui/integrate-builtin-server]

89. By Nicola Larosa on 2013-07-31

Merge from trunk.

[Nicola Larosa (teknico)]

Please take a look.

https://codereview.appspot.com/12086044/

[Gary Poster (gary)]

LGTM with changes.

Hi Nicola. This is very nice, thank you. I have some trivial changes
that you can take or leave, including a visit by the Grammar Police and
a few visits by the Idiosyncratic Text Rewriter.

I do have one change I feel strongly about atm, which you might disagree
with. I mutter about it somewhat incoherently below. I am going to try
and be clearer about it up here.

I think that the new upstart script should follow the haproxy upstart
pattern. Namely, it should be created during install, and copied over
in start(). We should also remove the copy, and the haproxy copy when
appropriate, in stop(), I think!

Otherwise, I think we will have problems when we switch from one server
to another. It will also be the right way forward when we think about
upgrading, I think.

I hope you agree, or don't disagree too much. :-)

Thanks!

Gary

https://codereview.appspot.com/12086044/diff/4001/Operation.md
File Operation.md (right):

https://codereview.appspot.com/12086044/diff/4001/Operation.md#newcode40
Operation.md:40: removed, only the built-in server will remain.
removed. Only

or

removed, and only

or

removed; only

https://codereview.appspot.com/12086044/diff/4001/config.yaml
File config.yaml (right):

https://codereview.appspot.com/12086044/diff/4001/config.yaml#newcode171
config.yaml:171: Enable the built-in server, disabling both haproxy and
Apache.
suggestion:

"This is a temporary option. The built-in server will be the only
server in the future."

(unless we want it to serve the sandbox?)

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py
File hooks/backend.py (right):

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode24
hooks/backend.py:24:
Maybe add the following?

"""Mixins are not actually mixed in to the backend class using Python
inheritance machinery. Instead, each mixin is instantiated and
collected in the Backend __init__, as needed. Then the install(),
start(), and stop() methods have a "self" that is the simple
instantiated mixin, and a "backend" argument that is the backend
instance. Python inheritance machinery is somewhat mimicked in that
certain properties and methods are explicitly aggregated on the backend
instance: see the chain_methods function, the merge_properties method,
and their usages."""

<Gary procrastinates from writing doc he is supposed to be doing>

Composition is supposed to be simpler than inheritance, but I'm not sure
this entirely succeeded. My feeling is that it is because its
composition is still so similar in feel to inheritance. I think I might
be happier with this if, rather that Backend's

install = chain_methods('install')

we had a small change like this:

def install(self):
     call_methods(self.mixins, 'install', self)

where call_methods would be something like this.

def call_methods(objs, name, *args):
     for obj in objs:
         f = getattr(objs, name, None)
         if f is not None:
             f(*args)

and so on.

That would be clearer to me. I would be tempted to take it even
farther--make mixins dicts rather than classes, for instance, and values
currently using merge_properties could simply be calculated a...

[lp:~teknico/charms/precise/juju-gui/integrate-builtin-server]

90. By Nicola Larosa on 2013-08-01

Some doc and comment changes per gary's review.

[Francesco Banconi (frankban)]

Thanks for this branch Nicola, good stuff.
I really appreciate your efforts in reorganizing the mixins/backends
stuff. Before starting to QA this, I'd like you to answer/address Gary's
comments and the ones I wrote below.

https://codereview.appspot.com/12086044/diff/4001/config/guiserver.conf.template
File config/guiserver.conf.template (right):

https://codereview.appspot.com/12086044/diff/4001/config/guiserver.conf.template#newcode7
config/guiserver.conf.template:7: exec runserver.py
--guiroot="{{gui_root}}" \
We could use the full path to runserver.py here.

https://codereview.appspot.com/12086044/diff/4001/config/guiserver.conf.template#newcode9
config/guiserver.conf.template:9: {{if api_version}}
What do you think about always providing --apiversion and --sslpath? We
know those values in the hooks, and specifying them here allows for
removing these if blocks and also generates a more explicit config, i.e.
looking at the rendered upstart config we always know where are the ssl
certs and what juju implementation we are currently using.

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py
File hooks/backend.py (right):

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode24
hooks/backend.py:24:
On 2013/07/31 21:18:18, gary.poster wrote:
> Composition is supposed to be simpler than inheritance, but I'm not
sure this
> entirely succeeded. My feeling is that it is because its composition
is still
> so similar in feel to inheritance. I think I might be happier with
this if,
> rather that Backend's

> install = chain_methods('install')

> we had a small change like this:

> def install(self):
> call_methods(self.mixins, 'install', self)

> where call_methods would be something like this.

> def call_methods(objs, name, *args):
> for obj in objs:
> f = getattr(objs, name, None)
> if f is not None:
> f(*args)

> and so on.

+1!

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode35
hooks/backend.py:35: import charmhelpers
Thank you for reordering imports.

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode140
hooks/backend.py:140: api_version = 'python' if utils.legacy_juju() else
'go'
I think the definition of api_version can be moved inside the "if
config.get('builtin-server', False)" block below.
Anyway, is there a reason to prefer config.get('builtin-server', False)
over config['builtin-server']?

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode141
hooks/backend.py:141: if config.get('builtin-server', False):
On 2013/07/31 21:18:18, gary.poster wrote:
> For the change story, do you think we ought to remove the old
haproxy/apache
> startup files? I'm inclined to say yes, but open to alternate
opinions. :-)

> I guess that would happen in "stop"?

I agree. So, IIUC:
- the start method generates the upstart files;
- the stop method (called by the stop and by the config-changed hooks)
removes the upstart file(s) in use for that configuration.

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode143
hooks/backend.py:143: utils.JUJU_GUI_DIR, utils.get_api_address(),
utils.J...

[Nicola Larosa (teknico)]

gary.poster wrote:
> Hi Nicola. This is very nice, thank you. I have some trivial changes
> that you can take or leave, including a visit by the Grammar Police
> and a few visits by the Idiosyncratic Text Rewriter.

Hi fellows, you're always welcome, make yourselves at home. Fancy some
tea? ;-)

> I think that the new upstart script should follow the haproxy upstart
> pattern. Namely, it should be created during install, and copied over
> in start(). We should also remove the copy, and the haproxy copy when
> appropriate, in stop(), I think!

> Otherwise, I think we will have problems when we switch from one
> server to another. It will also be the right way forward when we
> think about upgrading, I think.

Yes, that's a better strategy. See also frankban's clarification.

https://codereview.appspot.com/12086044/diff/4001/Operation.md#newcode40
> Operation.md:40: removed, only the built-in server will remain.
> removed. Only

> or

> removed, and only

> or

> removed; only

Done; third option, I love semicolons.

> https://codereview.appspot.com/12086044/diff/4001/config.yaml
> File config.yaml (right):

https://codereview.appspot.com/12086044/diff/4001/config.yaml#newcode171
> config.yaml:171: Enable the built-in server, disabling both haproxy
> and Apache. suggestion:

> "This is a temporary option. The built-in server will be the only
> server in the future."

Added.

> (unless we want it to serve the sandbox?)

Not sure what you mean here.

> https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py
> File hooks/backend.py (right):

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode24
> hooks/backend.py:24:
> Maybe add the following?

> """Mixins are not actually mixed in to the backend class using Python
> inheritance machinery. Instead, each mixin is instantiated and
> collected in the Backend __init__, as needed. Then the install(),
> start(), and stop() methods have a "self" that is the simple
> instantiated mixin, and a "backend" argument that is the backend
> instance. Python inheritance machinery is somewhat mimicked in that
> certain properties and methods are explicitly aggregated on the
> backend instance: see the chain_methods function, the merge_properties
> method, and their usages."""

Added.

> <Gary procrastinates from writing doc he is supposed to be doing>

That's ok. ;-)

> [rambling snipped]
> Anyway, just rambling. Nothing to do with this branch.

This actually points out something that's been nagging me for a while.
As discussed, the code structure has proved itself hard to read and
understand. I'll try your suggestions in a future branch and see how
much they improve matters.

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode139
> hooks/backend.py:139: # TODO: eventually the option, haproxy and
> Apache will go away
> a bit confusing.

> eventually this option will go away, as well as haproxy and Apache

> or similar?

Done.

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode141
> hooks/backend.py:141: if config.get('builtin-server', False):
> For the change story, do you think we ought to remove the old
> hap...

[Nicola Larosa (teknico)]

frankban wrote:

https://codereview.appspot.com/12086044/diff/4001/config/guiserver.conf.template#newcode7
> config/guiserver.conf.template:7: exec runserver.py
--guiroot="{{gui_root}}" \
> We could use the full path to runserver.py here.

To avoid possible interference with namesakes earlier in PATH. Done.

https://codereview.appspot.com/12086044/diff/4001/config/guiserver.conf.template#newcode9
> config/guiserver.conf.template:9: {{if api_version}}
> What do you think about always providing --apiversion and --sslpath?
> We know those values in the hooks, and specifying them here allows for
> removing these if blocks and also generates a more explicit config,
> i.e. looking at the rendered upstart config we always know where are
> the ssl certs and what juju implementation we are currently using.

Done.

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode24
> hooks/backend.py:24:
> On 2013/07/31 21:18:18, gary.poster wrote:
>> Composition is supposed to be simpler than inheritance, but I'm not
>> sure this entirely succeeded. My feeling is that it is because its
>> composition is still so similar in feel to inheritance. I think I
>> might be happier with this if, rather that Backend's

>> [smip]

>> and so on.

> +1!

Agreed, I'll tackle this in a future branch, as mentioned in the reply
to gary's review.

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode35
> hooks/backend.py:35: import charmhelpers
> Thank you for reordering imports.

One of those little things. :-)

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode140
> hooks/backend.py:140: api_version = 'python' if utils.legacy_juju()
> else 'go'
> I think the definition of api_version can be moved inside the "if
> config.get('builtin-server', False)" block below.

Done.

> Anyway, is there a reason to prefer config.get('builtin-server',
False) over
> config['builtin-server']?

Not really, as discussed I also removed other useless similar cases. The
only ones left are for SSL options without defaults.

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode141
> hooks/backend.py:141: if config.get('builtin-server', False):
> On 2013/07/31 21:18:18, gary.poster wrote:
>> For the change story, do you think we ought to remove the old
>> haproxy/apache startup files? I'm inclined to say yes, but open to
>> alternate opinions. :-)

>> I guess that would happen in "stop"?

> I agree. So, IIUC:
> - the start method generates the upstart files;
> - the stop method (called by the stop and by the config-changed hooks)
> removes the upstart file(s) in use for that configuration.

Agreed, as mentioned in the reply to gary's review.

https://codereview.appspot.com/12086044/diff/4001/hooks/backend.py#newcode143
> hooks/backend.py:143: utils.JUJU_GUI_DIR, utils.get_api_address(),
> utils.JUJU_GUI_DIR is passed here and used in the function as
> gui_root. This does not seem to be correct. I expected "build_dir" to
> be the right value to pass here instead: am I missing something?

True, that was a thinko on my part, thank you.

> Moreover, utils.get_api_address() seems wrong too. Please take a...

[lp:~teknico/charms/precise/juju-gui/integrate-builtin-server]

91. By Nicola Larosa on 2013-08-01

More fixes from reviews.

92. By Nicola Larosa on 2013-08-02

Checkpoint.

93. By Nicola Larosa on 2013-08-02

Redone all install/start/stop logic, fixed tests, added functional test for builtin server.

94. By Nicola Larosa on 2013-08-02

Lint fix.

95. By Nicola Larosa on 2013-08-02

Reenable certificate tests.

96. By Nicola Larosa on 2013-08-02

Readd missing config/haproxy.conf file.

[Nicola Larosa (teknico)]

After applying all the agreed changes the branch is very large (1200+
lines), and the functional tests are not yet passing. No use reproposing
now, see Launchpad for the current status.

The branch mixes two concerns: another refactoring of the
install/start/stop logic, to allow for the agreed upon changes, and the
actual integration of the builtin server.

It'll take a while more, but it seems more manageable to split these two
concerns into two different branches again.

https://codereview.appspot.com/12086044/

[lp:~teknico/charms/precise/juju-gui/integrate-builtin-server]

97. By Nicola Larosa on 2013-08-05

Merge from trunk.

98. By Nicola Larosa on 2013-08-06

Merge from refactor-config-generation-logic.

99. By Nicola Larosa on 2013-08-06

Fix broken config.

100. By Nicola Larosa on 2013-08-06

Add installing the builtin server, fix test, some cleanup.

101. By Nicola Larosa on 2013-08-06

Fix config files and functional test.

102. By Nicola Larosa on 2013-08-06

Increase functional tests timeout.

103. By Nicola Larosa on 2013-08-07

Merge from refactor-config-generation-logic.

104. By Nicola Larosa on 2013-08-07

Add unit tests for the new functions in hooks/utils.py .

105. By Nicola Larosa on 2013-08-07

Clean up exports and path usage in tests.

106. By Nicola Larosa on 2013-08-07

Syntax error.

107. By Nicola Larosa on 2013-08-08

Merge from trunk via refactor-config-generation-logic.

=== modified file 'Operation.md'
--- Operation.md	2013-07-29 10:40:50 +0000
+++ Operation.md	2013-08-08 09:20:50 +0000
@@ -12,8 +12,8 @@
 ## How it works ##
 
 The Juju GUI is a client-side, JavaScript application that runs inside a
-web browser. The browser connects to a custom-made server deployed by
-the charm.
+web browser. The browser connects to a built-in server deployed by the
+charm.
 
 ## Server ##
 
@@ -27,3 +27,14 @@
 Juju connections are bidirectional, using the WebSocket protocol on the
 same port as the HTTPS connection, allowing changes in the Juju
 environment to be propagated and shown immediately by the browser.
+
+## Activation ##
+
+Previously the Juju GUI has been served by a combination of haproxy and
+Apache, specifically deployed and configured by the charm.
+
+The new built-in server replaces them both and can be enabled by
+setting the config option `builtin-server` to `true`.
+
+In the future haproxy, Apache and the mentioned config option will be
+removed; only the built-in server will remain.

=== modified file 'config.yaml'
--- config.yaml	2013-08-05 09:38:39 +0000
+++ config.yaml	2013-08-08 09:20:50 +0000
@@ -166,3 +166,10 @@
       a link to juju.ubuntu.com
     type: boolean
     default: false
+  builtin-server:
+    description: |
+      Enable the built-in server, disabling both haproxy and Apache.
+      This is a temporary option: the built-in server will be
+      the only server in the future.
+    type: boolean
+    default: false

=== added file 'config/guiserver.conf.template'
--- config/guiserver.conf.template	1970-01-01 00:00:00 +0000
+++ config/guiserver.conf.template	2013-08-08 09:20:50 +0000
@@ -0,0 +1,13 @@
+description "GUIServer"
+author "Canonical"
+
+start on (filesystem and net-device-up IFACE=lo)
+stop on runlevel [!2345]
+
+exec /usr/bin/python /usr/local/bin/runserver.py \
+    --guiroot="{{gui_root}}" \
+    --apiurl="{{api_url}}" \
+    --apiversion="{{api_version}}" \
+    --sslpath="{{ssl_cert_path}}" \
+    {{if serve_tests}} --servetests \ {{endif}}
+    {{if insecure}} --insecure {{endif}}

=== added file 'config/haproxy.conf'
--- config/haproxy.conf	1970-01-01 00:00:00 +0000
+++ config/haproxy.conf	2013-08-08 09:20:50 +0000
@@ -0,0 +1,22 @@
+description "HAProxy"
+author "Canonical"
+
+start on (filesystem and net-device-up IFACE=lo)
+stop on runlevel [!2345]
+
+env CONF=/etc/haproxy/haproxy.cfg
+env DAEMON=/usr/sbin/haproxy
+
+expect fork
+respawn
+respawn limit 10 5
+
+pre-start script
+    # Test configuration, exit if errors are found.
+    $DAEMON -c -f $CONF
+    if [ $? -ne 0 ]
+        then exit $?
+    fi
+end script
+
+exec $DAEMON -f $CONF

=== removed file 'config/haproxy.conf'
--- config/haproxy.conf	2013-01-31 12:19:49 +0000
+++ config/haproxy.conf	1970-01-01 00:00:00 +0000
@@ -1,22 +0,0 @@
-description "HAProxy"
-author "Canonical"
-
-start on (filesystem and net-device-up IFACE=lo)
-stop on runlevel [!2345]
-
-env CONF=/etc/haproxy/haproxy.cfg
-env DAEMON=/usr/sbin/haproxy
-
-expect fork
-respawn
-respawn limit 10 5
-
-pre-start script
-    # Test configuration, exit if errors are found.
-    $DAEMON -c -f $CONF
-    if [ $? -ne 0 ]
-        then exit $?
-    fi
-end script
-
-exec $DAEMON -f $CONF

=== added directory 'deps'
=== added file 'deps/tornado-3.1.tar.gz'
Binary files deps/tornado-3.1.tar.gz	1970-01-01 00:00:00 +0000 and deps/tornado-3.1.tar.gz	2013-08-08 09:20:50 +0000 differ
=== modified file 'hooks/backend.py'
--- hooks/backend.py	2013-08-05 09:59:17 +0000
+++ hooks/backend.py	2013-08-08 09:20:50 +0000
@@ -180,6 +180,28 @@
         utils.stop_haproxy_apache()
 
 
+class BuiltinServerMixin(ServerInstallMixinBase):
+    """Manage the builtin server via Upstart."""
+
+    debs = ('openssl', 'python-pip')
+
+    def install(self, backend):
+        utils.install_tornado()
+        utils.install_builtin_server()
+        self._setup_certificates(backend)
+
+    def start(self, backend):
+        config = backend.config
+        build_dir = utils.compute_build_dir(
+            config['staging'], config['serve-tests'])
+        utils.start_builtin_server(
+            build_dir, config['serve-tests'], config['ssl-cert-path'],
+            not config['secure'])
+
+    def stop(self, backend):
+        utils.stop_builtin_server()
+
+
 def chain_methods(name):
     """Helper to compose a set of mixin objects into a callable.
 
@@ -249,7 +271,12 @@
 
         # We always install and start the GUI.
         self.mixins.append(GuiMixin())
-        self.mixins.append(HaproxyApacheMixin())
+        # TODO: eventually this option will go away, as well as haproxy and
+        # Apache.
+        if config['builtin-server']:
+            self.mixins.append(BuiltinServerMixin())
+        else:
+            self.mixins.append(HaproxyApacheMixin())
 
     def different(self, *keys):
         """Return a boolean indicating if the current config

=== modified file 'hooks/utils.py'
--- hooks/utils.py	2013-08-08 08:13:24 +0000
+++ hooks/utils.py	2013-08-08 09:20:50 +0000
@@ -17,19 +17,14 @@
 """Juju GUI charm utilities."""
 
 __all__ = [
-    'AGENT',
-    'APACHE',
     'APACHE_SITE',
     'APACHE_PORTS',
     'API_PORT',
     'CURRENT_DIR',
-    'HAPROXY',
-    'IMPROV',
     'JUJU_DIR',
     'JUJU_GUI_DIR',
     'JUJU_PEM',
     'WEB_PORT',
-    'bzr_checkout',
     'cmd_log',
     'compute_build_dir',
     'fetch_api',
@@ -53,9 +48,11 @@
     'setup_gui',
     'setup_haproxy_config',
     'start_agent',
+    'start_builtin_server',
     'start_haproxy_apache',
     'start_improv',
     'stop_agent',
+    'stop_builtin_server',
     'stop_haproxy_apache',
     'stop_improv',
     'write_gui_config',
@@ -99,20 +96,32 @@
 
 AGENT = 'juju-api-agent'
 APACHE = 'apache2'
+BUILTIN_SERVER = 'guiserver'
+HAPROXY = 'haproxy'
 IMPROV = 'juju-api-improv'
-HAPROXY = 'haproxy'
 
 API_PORT = 8080
 WEB_PORT = 8000
 
 CURRENT_DIR = os.getcwd()
+CONFIG_DIR = os.path.join(CURRENT_DIR, 'config')
 JUJU_DIR = os.path.join(CURRENT_DIR, 'juju')
 JUJU_GUI_DIR = os.path.join(CURRENT_DIR, 'juju-gui')
-APACHE_SITE = '/etc/apache2/sites-available/juju-gui'
-APACHE_PORTS = '/etc/apache2/ports.conf'
-HAPROXY_PATH = '/etc/haproxy/haproxy.cfg'
-SYS_INIT_DIR = '/etc/init/'
-CONFIG_DIR = os.path.join(os.path.dirname(__file__),  '..', 'config')
+SERVER_DIR = os.path.join(CURRENT_DIR, 'server')
+TORNADO_PATH = os.path.join(CURRENT_DIR, 'deps', 'tornado-3.1.tar.gz')
+UNIT_NAME = os.path.basename(os.path.realpath(os.path.join(CURRENT_DIR, '..')))
+
+APACHE_CFG_DIR = os.path.join('', 'etc', 'apache2')
+APACHE_PORTS = os.path.join(APACHE_CFG_DIR, 'ports.conf')
+APACHE_SITE = os.path.join(APACHE_CFG_DIR, 'sites-available', 'juju-gui')
+HAPROXY_CFG_PATH = os.path.join('', 'etc', 'haproxy', 'haproxy.cfg')
+
+SYS_INIT_DIR = os.path.join('', 'etc', 'init')
+AGENT_INIT_PATH = os.path.join(SYS_INIT_DIR, 'juju-api-agent.conf')
+GUISERVER_INIT_PATH = os.path.join(SYS_INIT_DIR, 'guiserver.conf')
+HAPROXY_INIT_PATH = os.path.join(SYS_INIT_DIR, 'haproxy.conf')
+IMPROV_INIT_PATH = os.path.join(SYS_INIT_DIR, 'juju-api-improv.conf')
+
 JUJU_PEM = 'juju.includes-private-key.pem'
 DEB_BUILD_DEPENDENCIES = (
     'bzr', 'g++', 'imagemagick', 'make',  'nodejs', 'npm',
@@ -120,7 +129,7 @@
 
 
 # Store the configuration from on invocation to the next.
-config_json = Serializer('/tmp/config.json')
+config_json = Serializer(os.path.join('', 'tmp', 'config.json'))
 # Bazaar checkout command.
 bzr_checkout = command('bzr', 'co', '--lightweight')
 # Whether or not the charm is deployed using juju-core.
@@ -289,8 +298,7 @@
     The argument *destination* is a file path.
     The argument *context* is a dict-like object.
     """
-    template_path = os.path.join(
-        os.path.dirname(__file__), '..', 'config', template_name)
+    template_path = os.path.join(CONFIG_DIR, template_name)
     template = tempita.Template.from_filename(template_path)
     with open(destination, 'w') as stream:
         stream.write(template.substitute(context))
@@ -328,9 +336,7 @@
         'port': API_PORT,
         'staging_env': staging_env,
     }
-    render_to_file(
-        'juju-api-improv.conf.template', context,
-        '/etc/init/juju-api-improv.conf')
+    render_to_file('juju-api-improv.conf.template', context, IMPROV_INIT_PATH)
     log('Starting the staging backend.')
     with su('root'):
         service_control(IMPROV, START)
@@ -342,15 +348,13 @@
     with su('root'):
         service_control(IMPROV, STOP)
     log('Removing the staging Upstart script.')
-    cmd_log(run('rm', '-f', '/etc/init/juju-api-improv.conf'))
+    cmd_log(run('rm', '-f', IMPROV_INIT_PATH))
 
 
 def start_agent(ssl_cert_path, read_only=False):
     """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 = os.path.join(
-        SYS_INIT_DIR, 'juju-{}.conf'.format(os.path.basename(unit_dir)))
+    agent_file = os.path.join(SYS_INIT_DIR, 'juju-{}.conf'.format(UNIT_NAME))
     zookeeper = get_zookeeper_address(agent_file)
     log('Setting up the API agent Upstart script.')
     context = {
@@ -360,9 +364,7 @@
         'zookeeper': zookeeper,
         'read_only': read_only
     }
-    render_to_file(
-        'juju-api-agent.conf.template', context,
-        '/etc/init/juju-api-agent.conf')
+    render_to_file('juju-api-agent.conf.template', context, AGENT_INIT_PATH)
     log('Starting the API agent.')
     with su('root'):
         service_control(AGENT, START)
@@ -374,7 +376,7 @@
     with su('root'):
         service_control(AGENT, STOP)
     log('Removing the API agent Upstart script.')
-    cmd_log(run('rm', '-f', '/etc/init/juju-api-agent.conf'))
+    cmd_log(run('rm', '-f', AGENT_INIT_PATH))
 
 
 def compute_build_dir(in_staging, serve_tests):
@@ -442,7 +444,7 @@
     is_legacy_juju = legacy_juju()
     if is_legacy_juju:
         # The PyJuju API agent is listening on localhost.
-        api_address = '127.0.0.1:{0}'.format(API_PORT)
+        api_address = '127.0.0.1:{}'.format(API_PORT)
     else:
         # Retrieve the juju-core API server address.
         api_address = get_api_address()
@@ -458,15 +460,14 @@
         'web_port': WEB_PORT,
         'secure': secure
     }
-    render_to_file('haproxy.cfg.template', context, HAPROXY_PATH)
+    render_to_file('haproxy.cfg.template', context, HAPROXY_CFG_PATH)
 
 
 def remove_haproxy_setup():
     """Remove haproxy setup."""
     log('Removing haproxy setup.')
-    cmd_log(run('rm', '-f', HAPROXY_PATH))
-    config_path = os.path.join(SYS_INIT_DIR, 'haproxy.conf')
-    cmd_log(run('rm', '-f', config_path))
+    cmd_log(run('rm', '-f', HAPROXY_CFG_PATH))
+    cmd_log(run('rm', '-f', HAPROXY_INIT_PATH))
 
 
 def setup_apache_config(build_dir, serve_tests=False):
@@ -523,6 +524,60 @@
     remove_apache_setup()
 
 
+def install_tornado():
+    """Install Tornado from a local tarball."""
+    log('Installing Tornado.')
+    with su('root'):
+        cmd_log(run('pip', 'install', TORNADO_PATH))
+
+
+def install_builtin_server():
+    """Install the builtin server code."""
+    log('Installing the builtin server.')
+    setup_cmd = os.path.join(SERVER_DIR, 'setup.py')
+    with su('root'):
+        cmd_log(run('/usr/bin/python', setup_cmd, 'install'))
+
+
+def write_builtin_server_startup(
+        gui_root, ssl_cert_path, serve_tests=False, insecure=False):
+    """Generate the builtin server Upstart file."""
+    log('Generating the builtin server Upstart file.')
+    url_prefix = 'ws' if insecure else 'wss'
+    is_legacy_juju = legacy_juju()
+    api_address = get_api_address()
+    api_url = '{}://127.0.0.1:{}'.format(
+        url_prefix, API_PORT) if is_legacy_juju else '{}://{}'.format(
+        url_prefix, api_address)
+    context = {
+        'gui_root': gui_root,
+        'api_url': api_url,
+        'api_version': 'python' if is_legacy_juju else 'go',
+        'ssl_cert_path': ssl_cert_path,
+        'serve_tests': serve_tests,
+        'insecure': insecure,
+    }
+    render_to_file(
+        'guiserver.conf.template', context, GUISERVER_INIT_PATH)
+
+
+def start_builtin_server(build_dir, serve_tests, ssl_cert_path, insecure):
+    """Start the builtin server."""
+    write_builtin_server_startup(
+        build_dir, ssl_cert_path, serve_tests, insecure)
+    log('Starting the builtin server.')
+    with su('root'):
+        service_control(BUILTIN_SERVER, RESTART)
+
+
+def stop_builtin_server():
+    """Stop the builtin server."""
+    log('Stopping the builtin server.')
+    with su('root'):
+        service_control(BUILTIN_SERVER, STOP)
+    cmd_log(run('rm', '-f', GUISERVER_INIT_PATH))
+
+
 def get_npm_cache_archive_url(Launchpad=Launchpad):
     """Figure out the URL of the most recent NPM cache archive on Launchpad."""
     launchpad = Launchpad.login_anonymously('Juju GUI charm', 'production')

=== modified file 'revision'
--- revision	2013-08-05 15:00:27 +0000
+++ revision	2013-08-08 09:20:50 +0000
@@ -1,1 +1,1 @@
-66
+67

=== modified file 'server/setup.py'
--- server/setup.py	2013-07-17 15:20:47 +0000
+++ server/setup.py	2013-08-08 09:20:50 +0000
@@ -38,7 +38,7 @@
     keywords='juju gui server',
     packages=[
         PROJECT_NAME,
-        '{0}.tests'.format(PROJECT_NAME),
+        '{}.tests'.format(PROJECT_NAME),
     ],
     scripts=['runserver.py'],
     classifiers=[

=== modified file 'tests/20-functional.test'
--- tests/20-functional.test	2013-08-05 15:00:27 +0000
+++ tests/20-functional.test	2013-08-08 09:20:50 +0000
@@ -82,7 +82,7 @@
         Retry loading the page until the page is found or a timeout exception
         is raised.
         """
-        base_url = 'https://{0}:{1}'.format(hostname, self.port)
+        base_url = 'https://{}:{}'.format(hostname, self.port)
         url = urlparse.urljoin(base_url, path)
 
         def page_ready(driver):
@@ -134,7 +134,7 @@
             # started by the charm in the machine. Right now this does not
             # work in pyJuju, so the desired effect is achieved by keeping
             # track of started services and manually stopping them here.
-        target = 'ubuntu@{0}'.format(hostname)
+        target = 'ubuntu@{}'.format(hostname)
         for service in services:
             ssh(target, 'sudo', 'service', service, 'stop')
 
@@ -177,6 +177,17 @@
         # The staging environment contains five deployed services.
         self.assertSetEqual(set(STAGING_SERVICES), self.get_service_names())
 
+    def test_builtin_server(self):
+        # Ensure the Juju GUI and builtin server are correctly set up.
+        unit_info = self.juju_deploy(
+            self.charm, options={'builtin-server': 'true'})
+        hostname = unit_info['public-address']
+        conn = httplib.HTTPSConnection(hostname)
+        conn.request('HEAD', '/')
+        headers = conn.getresponse().getheaders()
+        server_header = dict(headers)['server']
+        self.assertIn('TornadoServer', server_header)
+
     def test_branch_source(self):
         # Ensure the Juju GUI is correctly deployed from a Bazaar branch.
         unit_info = self.juju_deploy(

=== modified file 'tests/test_backends.py'
--- tests/test_backends.py	2013-08-05 09:59:17 +0000
+++ tests/test_backends.py	2013-08-08 09:20:50 +0000
@@ -46,8 +46,8 @@
     """Ensure the correct mixins and property values are collected."""
 
     def test_staging_backend(self):
-        test_backend = backend.Backend(
-            config={'sandbox': False, 'staging': True})
+        test_backend = backend.Backend(config={
+            'sandbox': False, 'staging': True, 'builtin-server': False})
         mixin_names = get_mixin_names(test_backend)
         self.assertEqual(
             ('ImprovMixin', 'GuiMixin', 'HaproxyApacheMixin'),
@@ -57,8 +57,8 @@
             test_backend.debs)
 
     def test_sandbox_backend(self):
-        test_backend = backend.Backend(
-            config={'sandbox': True, 'staging': False})
+        test_backend = backend.Backend(config={
+            'sandbox': True, 'staging': False, 'builtin-server': False})
         mixin_names = get_mixin_names(test_backend)
         self.assertEqual(
             ('SandboxMixin', 'GuiMixin', 'HaproxyApacheMixin'),
@@ -68,8 +68,8 @@
             test_backend.debs)
 
     def test_python_backend(self):
-        test_backend = backend.Backend(
-            config={'sandbox': False, 'staging': False})
+        test_backend = backend.Backend(config={
+            'sandbox': False, 'staging': False, 'builtin-server': False})
         mixin_names = get_mixin_names(test_backend)
         self.assertEqual(
             ('PythonMixin', 'GuiMixin', 'HaproxyApacheMixin'),
@@ -86,8 +86,8 @@
         # Create a fake agent file.
         agent_path = os.path.join(base_dir, 'agent.conf')
         open(agent_path, 'w').close()
-        test_backend = backend.Backend(
-            config={'sandbox': False, 'staging': False})
+        test_backend = backend.Backend(config={
+            'sandbox': False, 'staging': False, 'builtin-server': False})
         # Cleanup.
         utils.CURRENT_DIR = orig_current_dir
         shutil.rmtree(base_dir)
@@ -118,6 +118,8 @@
             'find_missing_packages': utils.find_missing_packages,
             'get_api_address': utils.get_api_address,
             'get_npm_cache_archive_url': utils.get_npm_cache_archive_url,
+            'install_builtin_server': utils.install_builtin_server,
+            'install_tornado': utils.install_tornado,
             'parse_source': utils.parse_source,
             'prime_npm_cache': utils.prime_npm_cache,
             'remove_apache_setup': utils.remove_apache_setup,
@@ -128,6 +130,7 @@
             'setup_haproxy_config': utils.setup_haproxy_config,
             'start_agent': utils.start_agent,
             'start_improv': utils.start_improv,
+            'write_builtin_server_startup': utils.write_builtin_server_startup,
             'write_gui_config': utils.write_gui_config,
         }
         self.charmhelpers_mocks = {
@@ -192,11 +195,12 @@
             self.assertTrue(
                 self.called.get(mocked), '{} was not called'.format(mocked))
 
-    def test_install_improv(self):
+    def test_install_improv_builtin(self):
         test_backend = backend.Backend(config=self.alwaysTrue)
         test_backend.install()
         for mocked in (
             'apt_get_install', 'fetch_api', 'find_missing_packages',
+            'install_builtin_server', 'install_tornado',
         ):
             self.assertTrue(
                 self.called.get(mocked), '{} was not called'.format(mocked))
@@ -211,12 +215,12 @@
             self.assertTrue(
                 self.called.get(mocked), '{} was not called'.format(mocked))
 
-    def test_start_improv(self):
+    def test_start_improv_builtin(self):
         test_backend = backend.Backend(config=self.alwaysTrue)
         test_backend.start()
         for mocked in (
             'compute_build_dir', 'open_port', 'start_improv', 'su',
-            'write_gui_config',
+            'write_builtin_server_startup', 'write_gui_config',
         ):
             self.assertTrue(
                 self.called.get(mocked), '{} was not called'.format(mocked))
@@ -231,16 +235,20 @@
 
     def test_same_config(self):
         test_backend = backend.Backend(
-            config={'sandbox': False, 'staging': False},
-            prev_config={'sandbox': False, 'staging': False},
+            config={
+                'sandbox': False, 'staging': False, 'builtin-server': False},
+            prev_config={
+                'sandbox': False, 'staging': False, 'builtin-server': False},
         )
         self.assertFalse(test_backend.different('sandbox'))
         self.assertFalse(test_backend.different('staging'))
 
     def test_different_config(self):
         test_backend = backend.Backend(
-            config={'sandbox': False, 'staging': False},
-            prev_config={'sandbox': True, 'staging': False},
+            config={
+                'sandbox': False, 'staging': False, 'builtin-server': False},
+            prev_config={
+                'sandbox': True, 'staging': False, 'builtin-server': False},
         )
         self.assertTrue(test_backend.different('sandbox'))
         self.assertFalse(test_backend.different('staging'))

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2013-08-07 09:50:50 +0000
+++ tests/test_utils.py	2013-08-08 09:20:50 +0000
@@ -45,6 +45,8 @@
     log_hook,
     parse_source,
     get_npm_cache_archive_url,
+    install_builtin_server,
+    install_tornado,
     remove_apache_setup,
     remove_haproxy_setup,
     render_to_file,
@@ -52,11 +54,14 @@
     setup_apache_config,
     setup_haproxy_config,
     start_agent,
+    start_builtin_server,
     start_haproxy_apache,
     start_improv,
     stop_agent,
+    stop_builtin_server,
     stop_haproxy_apache,
     stop_improv,
+    write_builtin_server_startup,
     write_gui_config,
 )
 # Import the whole utils package for monkey patching.
@@ -379,7 +384,7 @@
 
     def setUp(self):
         self.zookeeper_address = 'example.com:2000'
-        contents = 'env JUJU_ZOOKEEPER="{0}"\n'.format(self.zookeeper_address)
+        contents = 'env JUJU_ZOOKEEPER="{}"\n'.format(self.zookeeper_address)
         with tempfile.NamedTemporaryFile(delete=False) as agent_file:
             agent_file.write(contents)
             self.agent_file_path = agent_file.name
@@ -674,30 +679,15 @@
                 result, build_dir, 'in_staging: {}, serve_tests: {}'.format(
                     in_staging, serve_tests))
 
-    def test_write_gui_config(self):
-        write_gui_config(
-            False, 'This is login help.', True, True, self.charmworld_url,
-            self.build_dir, use_analytics=True, config_js_path='config')
-        js_conf = self.files['config']
-        self.assertIn('consoleEnabled: false', js_conf)
-        self.assertIn('user: "admin"', js_conf)
-        self.assertIn('password: "admin"', js_conf)
-        self.assertIn('login_help: "This is login help."', js_conf)
-        self.assertIn('readOnly: true', js_conf)
-        self.assertIn("socket_url: 'wss://", js_conf)
-        self.assertIn('socket_protocol: "wss"', js_conf)
-        self.assertIn('charmworldURL: "http://charmworld.example"', js_conf)
-        self.assertIn('useAnalytics: true', js_conf)
-
     def test_setup_haproxy_config(self):
         setup_haproxy_config(self.ssl_cert_path)
         haproxy_conf = self.files['haproxy.cfg']
-        self.assertIn('ca-base {0}'.format(self.ssl_cert_path), haproxy_conf)
-        self.assertIn('crt-base {0}'.format(self.ssl_cert_path), haproxy_conf)
-        self.assertIn('ws1 127.0.0.1:{0}'.format(API_PORT), haproxy_conf)
-        self.assertIn('web1 127.0.0.1:{0}'.format(WEB_PORT), haproxy_conf)
-        self.assertIn('ca-file {0}'.format(JUJU_PEM), haproxy_conf)
-        self.assertIn('crt {0}'.format(JUJU_PEM), haproxy_conf)
+        self.assertIn('ca-base {}'.format(self.ssl_cert_path), haproxy_conf)
+        self.assertIn('crt-base {}'.format(self.ssl_cert_path), haproxy_conf)
+        self.assertIn('ws1 127.0.0.1:{}'.format(API_PORT), haproxy_conf)
+        self.assertIn('web1 127.0.0.1:{}'.format(WEB_PORT), haproxy_conf)
+        self.assertIn('ca-file {}'.format(JUJU_PEM), haproxy_conf)
+        self.assertIn('crt {}'.format(JUJU_PEM), haproxy_conf)
         self.assertIn('redirect scheme https', haproxy_conf)
 
     def test_remove_haproxy_setup(self):
@@ -708,9 +698,9 @@
         setup_apache_config(self.build_dir, serve_tests=True)
         apache_site_conf = self.files['SITE_NOT_THERE']
         self.assertIn('juju-gui/build-', apache_site_conf)
-        self.assertIn('VirtualHost *:{0}'.format(WEB_PORT), apache_site_conf)
+        self.assertIn('VirtualHost *:{}'.format(WEB_PORT), apache_site_conf)
         self.assertIn(
-            'Alias /test {0}/test/'.format(JUJU_GUI_DIR), apache_site_conf)
+            'Alias /test {}/test/'.format(JUJU_GUI_DIR), apache_site_conf)
         apache_ports_conf = self.files['PORTS_NOT_THERE']
         self.assertIn('NameVirtualHost *:8000', apache_ports_conf)
         self.assertIn('Listen 8000', apache_ports_conf)
@@ -720,7 +710,7 @@
         self.assertEqual(self.run_call_count, 3)
 
     def test_start_haproxy_apache(self):
-        start_haproxy_apache('build_dir', False, self.ssl_cert_path, True)
+        start_haproxy_apache(JUJU_GUI_DIR, False, self.ssl_cert_path, True)
         self.assertEqual(self.svc_ctl_call_count, 2)
         self.assertEqual(self.service_names, ['apache2', 'haproxy'])
         self.assertEqual(
@@ -732,6 +722,53 @@
         self.assertEqual(self.service_names, ['haproxy', 'apache2'])
         self.assertEqual(self.actions, [charmhelpers.STOP, charmhelpers.STOP])
 
+    def test_install_tornado(self):
+        install_tornado()
+        self.assertEqual(self.run_call_count, 1)
+
+    def test_install_builtin_server(self):
+        install_builtin_server()
+        self.assertEqual(self.run_call_count, 1)
+
+    def test_write_builtin_server_startup(self):
+        write_builtin_server_startup(
+            JUJU_GUI_DIR, self.ssl_cert_path, serve_tests=True, insecure=True)
+        guiserver_conf = self.files['guiserver.conf']
+        self.assertIn('description "GUIServer"', guiserver_conf)
+        self.assertIn('--apiurl="ws://127.0.0.1:8080"', guiserver_conf)
+        self.assertIn('--apiversion="python"', guiserver_conf)
+        self.assertIn('--servetests', guiserver_conf)
+        self.assertIn('--insecure', guiserver_conf)
+
+    def test_start_builtin_server(self):
+        start_builtin_server(
+            JUJU_GUI_DIR, False, self.ssl_cert_path, insecure=False)
+        self.assertEqual(self.svc_ctl_call_count, 1)
+        self.assertEqual(self.service_names, ['guiserver'])
+        self.assertEqual(self.actions, [charmhelpers.RESTART])
+
+    def test_stop_builtin_server(self):
+        stop_builtin_server()
+        self.assertEqual(self.svc_ctl_call_count, 1)
+        self.assertEqual(self.service_names, ['guiserver'])
+        self.assertEqual(self.actions, [charmhelpers.STOP])
+        self.assertEqual(self.run_call_count, 1)
+
+    def test_write_gui_config(self):
+        write_gui_config(
+            False, 'This is login help.', True, True, self.charmworld_url,
+            self.build_dir, use_analytics=True, config_js_path='config')
+        js_conf = self.files['config']
+        self.assertIn('consoleEnabled: false', js_conf)
+        self.assertIn('user: "admin"', js_conf)
+        self.assertIn('password: "admin"', js_conf)
+        self.assertIn('login_help: "This is login help."', js_conf)
+        self.assertIn('readOnly: true', js_conf)
+        self.assertIn("socket_url: 'wss://", js_conf)
+        self.assertIn('socket_protocol: "wss"', js_conf)
+        self.assertIn('charmworldURL: "http://charmworld.example"', js_conf)
+        self.assertIn('useAnalytics: true', js_conf)
+
     def test_write_gui_config_insecure(self):
         write_gui_config(
             False, 'This is login help.', True, True, self.charmworld_url,