yellow team mailing list archive

[Francesco Banconi <francesco.banconi@xxxxxxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/charms/precise/juju-gui/bug-1086788-jitsu-deploy-to into lp:~juju-gui/charms/precise/juju-gui/trunk.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1086788 in juju-gui: "charm should support/test jitsu deploy-to"
  https://bugs.launchpad.net/juju-gui/+bug/1086788

For more details, see:
https://code.launchpad.net/~frankban/charms/precise/juju-gui/bug-1086788-jitsu-deploy-to/+merge/139526

Add support for jitsu deploy-to.

The charm already worked well when deployed in the bootstrap node using
the 'jitsu deploy-to' command. This branch adds a test for this behavior.

Other changes:

Slightly simplified how the staging option is retrieved in the start hook.

Fixed tests by changing the juju-gui upstart configuration file. Before
this change it was not possible for upstart to correctly stop the juju-gui
service, because upstart itself was not able to keep track of the PID.

Fixed the install hook: it was not idempotent (and the tests were broken
for this reason too).

https://codereview.appspot.com/6929057/

-- 
https://code.launchpad.net/~frankban/charms/precise/juju-gui/bug-1086788-jitsu-deploy-to/+merge/139526
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~frankban/charms/precise/juju-gui/bug-1086788-jitsu-deploy-to into lp:~juju-gui/charms/precise/juju-gui/trunk.

=== modified file 'config/juju-gui.conf.template'
--- config/juju-gui.conf.template	2012-12-06 16:03:59 +0000
+++ config/juju-gui.conf.template	2012-12-12 17:45:38 +0000
@@ -1,7 +1,24 @@
 description "Juju GUI"
 author "Canonical"
+# Original author: George Shammas <georgyo@xxxxxxxxx>.
+# See http://wiki.nginx.org/Upstart.
 
-start on runlevel [2345]
+start on (filesystem and net-device-up IFACE=lo)
 stop on runlevel [!2345]
 
-exec service nginx start
+env DAEMON=/usr/sbin/nginx
+env PID=/var/run/nginx.pid
+
+expect fork
+respawn
+respawn limit 10 5
+
+pre-start script
+    # Test configuration, exit if errors are found.
+    $DAEMON -t
+    if [ $? -ne 0 ]
+        then exit $?
+    fi
+end script
+
+exec $DAEMON

=== modified file 'hooks/install'
--- hooks/install	2012-12-10 17:22:24 +0000
+++ hooks/install	2012-12-12 17:45:38 +0000
@@ -1,7 +1,7 @@
 #!/usr/bin/env python2
 # -*- python -*-
 
-import os.path
+import os
 from subprocess import (
     CalledProcessError,
     check_call,
@@ -53,11 +53,16 @@
         log('Output from "make" sent to', name)
         run('make', stdout=fd, stderr=fd)
     log('Setting up nginx.')
-    cmd_log(run('rm', '/etc/nginx/sites-enabled/default'))
-    cmd_log(run('touch', '/etc/nginx/sites-available/juju-gui'))
-    cmd_log(run('chown', 'ubuntu:', '/etc/nginx/sites-available/juju-gui'))
-    cmd_log(run('ln', '-s', '/etc/nginx/sites-available/juju-gui',
-        '/etc/nginx/sites-enabled/juju-gui'))
+    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'))
 
 
 def main():

=== modified file 'hooks/start'
--- hooks/start	2012-12-10 17:22:24 +0000
+++ hooks/start	2012-12-12 17:45:38 +0000
@@ -108,9 +108,9 @@
 def main():
     config = get_config()
     juju_api_port = config['juju-api-port']
-    start_gui(juju_api_port, config['juju-gui-console-enabled'],
-              config.get('staging'))
-    if config.get('staging'):
+    staging = config.get('staging')
+    start_gui(juju_api_port, config['juju-gui-console-enabled'], staging)
+    if staging:
         start_improv(juju_api_port, config['staging-environment'])
     else:
         start_agent(juju_api_port)

=== modified file 'tests/deploy.test'
--- tests/deploy.test	2012-12-10 17:22:24 +0000
+++ tests/deploy.test	2012-12-12 17:45:38 +0000
@@ -31,7 +31,7 @@
     raise err
 
 
-class DeployTest(unittest.TestCase):
+class DeployTestMixin(object):
 
     def setUp(self):
         self.charm = 'juju-gui'
@@ -51,23 +51,6 @@
         for service in services:
             ssh(target, 'sudo', 'service', service, 'stop')
 
-    def deploy(self, config_path=None):
-        """Deploy and expose the Juju GUI charm. Return the service host name.
-
-        Also wait until the service is started.
-        If *config_path* is provided, it will be used when deploying the charm.
-        """
-        args = ['deploy', 'local:{0}'.format(self.charm)]
-        if config_path is not None:
-            args.extend(['--config', config_path])
-        juju(*args)
-        juju('expose', self.charm)
-        jitsu(
-            'watch', '--failfast', self.charm,
-            '--state', 'started', '--open-port', self.port)
-        address = jitsu('get-service-info', self.charm, 'public-address')
-        return address.strip().split(':')[1]
-
     def check_services(self, hostname, ws_port=8080):
         """Check the services are listening on their tcp ports."""
         url = 'http://{0}:{1}'.format(hostname, self.port)
@@ -81,6 +64,26 @@
             open_url(ws_url)
         self.assertEqual(400, context_manager.exception.getcode())
 
+
+class DeployTest(DeployTestMixin, unittest.TestCase):
+
+    def deploy(self, config_path=None):
+        """Deploy and expose the Juju GUI charm. Return the service host name.
+
+        Also wait until the service is started.
+        If *config_path* is provided, it will be used when deploying the charm.
+        """
+        args = ['deploy', 'local:{0}'.format(self.charm)]
+        if config_path is not None:
+            args.extend(['--config', config_path])
+        juju(*args)
+        juju('expose', self.charm)
+        jitsu(
+            'watch', '--failfast', self.charm,
+            '--state', 'started', '--open-port', self.port)
+        address = jitsu('get-service-info', self.charm, 'public-address')
+        return address.strip().split(':')[1]
+
     def test_api_agent(self):
         # Ensure the Juju GUI and API agent services are correctly set up.
         hostname = self.deploy()
@@ -91,13 +94,13 @@
 
     def test_customized_api_port(self):
         # It is possible to customize the port used by the websocket server.
-        config = {self.charm: {'juju-api-port': 80}}
+        config = {self.charm: {'juju-api-port': 8081}}
         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, ws_port=80)
+        self.check_services(hostname, ws_port=8081)
 
     def test_staging(self):
         # Ensure the Juju GUI and improv services are correctly set up.
@@ -111,5 +114,32 @@
         self.check_services(hostname)
 
 
+class DeployToTest(DeployTestMixin, unittest.TestCase):
+
+    def deploy_to(self):
+        """Deploy the Juju GUI charm in the Juju bootstrap node.
+
+        Expose it and return the service host name.
+        Also wait until the service is started.
+        """
+        # The id of the bootstrap node is 0 (the first node started by Juju).
+        jitsu('deploy-to', '0', 'local:{0}'.format(self.charm))
+        juju('expose', self.charm)
+        jitsu(
+            'watch', '--failfast', self.charm,
+            '--state', 'started', '--open-port', self.port)
+        address = jitsu('get-service-info', self.charm, 'public-address')
+        return address.strip().split(':')[1]
+
+    def test_api_agent(self):
+        # Ensure the Juju GUI and API agent services are correctly set up in
+        # the Juju bootstrap node.
+        hostname = self.deploy_to()
+        # 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)
+
+
 if __name__ == '__main__':
     unittest.main(verbosity=2)

=== modified file 'tests/test_utils.py'
--- tests/test_utils.py	2012-12-07 21:16:55 +0000
+++ tests/test_utils.py	2012-12-12 17:45:38 +0000
@@ -47,6 +47,7 @@
         expected = self.template_contents % context
         self.assertEqual(expected, self.destination_file.read())
 
+
 class CmdLogTest(unittest.TestCase):
     def setUp(self):
         # Patch the charmhelpers 'command', which powers get_config.  The