yellow team mailing list archive

[Francesco Banconi <frankban@xxxxxxxxx>]

Francesco Banconi has proposed merging lp:~frankban/charms/oneiric/buildbot-master/upgrade-charm into lp:~yellow/charms/oneiric/buildbot-master/trunk.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-master/upgrade-charm/+merge/95535

== Changes ==

- Added upgrade-charm symlink
- Updated helpers (with some refactoring to functions running "juju status")
- Updated install hook: now it runs config-changed during charm upgrade
- Some changes in functional tests

The file helper.py is in sync with the one present in 
lp:~frankban/charms/oneiric/buildbot-slave/upgrade-charm

Currently the upgrade-charm test does not work due to a bug of juju:
see https://bugs.launchpad.net/juju/+bug/941873
-- 
https://code.launchpad.net/~frankban/charms/oneiric/buildbot-master/upgrade-charm/+merge/95535
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~frankban/charms/oneiric/buildbot-master/upgrade-charm into lp:~yellow/charms/oneiric/buildbot-master/trunk.

=== modified file 'hooks/buildbot-relation-broken'
--- hooks/buildbot-relation-broken	2012-02-13 15:59:35 +0000
+++ hooks/buildbot-relation-broken	2012-03-02 11:26:19 +0000
@@ -3,12 +3,12 @@
 # Copyright 2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+from shelltoolbox import su
 from helpers import (
     log,
     log_entry,
     log_exit,
     run,
-    su,
     )
 from local import (
     buildbot_reconfig,

=== modified file 'hooks/config-changed'
--- hooks/config-changed	2012-02-29 22:43:30 +0000
+++ hooks/config-changed	2012-03-02 11:26:19 +0000
@@ -78,7 +78,7 @@
             install_extra_repositories(extra_repo)
         except subprocess.CalledProcessError as e:
             log('Error adding repository: ' + extra_repo)
-            log(e)
+            log(str(e))
             raise
         restart_required = True
     if extra_pkgs and 'extra-packages' in added_or_changed:

=== modified file 'hooks/helpers.py'
--- hooks/helpers.py	2012-02-29 22:43:30 +0000
+++ hooks/helpers.py	2012-03-02 11:26:19 +0000
@@ -6,15 +6,21 @@
 __metaclass__ = type
 __all__ = [
     'get_config',
+    'juju_status',
     'log',
     'log_entry',
     'log_exit',
+    'make_charm_config_file',
     'relation_get',
     'relation_set',
     'unit_info',
+    'wait_for_machine',
+    'wait_for_page_contents',
+    'wait_for_relation',
+    'wait_for_unit',
     ]
 
-from collections import namedtuple
+from contextlib import contextmanager
 import json
 import operator
 from shelltoolbox import (
@@ -22,14 +28,13 @@
     run,
     script_name,
     )
+import os
 import tempfile
 import time
 import urllib2
 import yaml
 
 
-Env = namedtuple('Env', 'uid gid home')
-
 log = command('juju-log')
 
 
@@ -67,10 +72,18 @@
     return charm_config_file
 
 
+def juju_status(key):
+    return yaml.safe_load(run('juju', 'status'))[key]
+
+
+def get_charm_revision(service_name):
+    service = juju_status('services')[service_name]
+    return int(service['charm'].split('-')[-1])
+
+
 def unit_info(service_name, item_name, data=None):
-    if data is None:
-        data = yaml.safe_load(run('juju', 'status'))
-    service = data['services'].get(service_name)
+    services = juju_status('services') if data is None else data['services']
+    service = services.get(service_name)
     if service is None:
         # XXX 2012-02-08 gmb:
         #     This allows us to cope with the race condition that we
@@ -83,8 +96,27 @@
     return item
 
 
-def get_machine_data():
-    return yaml.safe_load(run('juju', 'status'))['machines']
+@contextmanager
+def maintain_charm_revision(path=None):
+    if path is None:
+        path = os.path.join(os.path.dirname(__file__), '..', 'revision')
+    revision = open(path).read()
+    try:
+        yield revision
+    finally:
+        with open(path, 'w') as f:
+            f.write(revision)
+
+
+def upgrade_charm(service_name, timeout=120):
+    next_revision = get_charm_revision(service_name) + 1
+    start_time = time.time()
+    run('juju', 'upgrade-charm', service_name)
+    while get_charm_revision(service_name) != next_revision:
+        if time.time() - start_time >= timeout:
+            raise RuntimeError('timeout waiting for charm to be upgraded')
+        time.sleep(0.1)
+    return next_revision
 
 
 def wait_for_machine(num_machines=1, timeout=300):
@@ -98,7 +130,7 @@
     # to tell what environment we're working in (LXC vs EC2) is to check
     # the dns-name of the first machine. If it's localhost we're in LXC
     # and we can just return here.
-    if get_machine_data()[0]['dns-name'] == 'localhost':
+    if juju_status('machines')[0]['dns-name'] == 'localhost':
         return
     start_time = time.time()
     while True:
@@ -106,7 +138,7 @@
         # not a machine that we need to wait for. This will only work
         # for EC2 environments, which is why we return early above if
         # we're in LXC.
-        machine_data = get_machine_data()
+        machine_data = juju_status('machines')
         non_zookeeper_machines = [
             machine_data[key] for key in machine_data.keys()[1:]]
         if len(non_zookeeper_machines) >= num_machines:

=== modified file 'hooks/install'
--- hooks/install	2012-03-01 14:06:36 +0000
+++ hooks/install	2012-03-02 11:26:19 +0000
@@ -110,10 +110,12 @@
     old_config = config_json.get()
     buildbot_dir = old_config.get('installdir', config['installdir'])
     cleanup(buildbot_dir)
+    if os.path.basename(__file__) == 'upgrade-charm':
+        log('Running config-changed from upgrade-charm hook.')
+        run('hooks/config-changed')
 
 
 if __name__ == '__main__':
-
     log_entry()
     try:
         main()

=== added symlink 'hooks/upgrade-charm'
=== target is u'install'
=== modified file 'revision'
--- revision	2012-02-29 09:57:12 +0000
+++ revision	2012-03-02 11:26:19 +0000
@@ -1,2 +1,1 @@
 1
-

=== modified file 'tests/buildbot-master.test'
--- tests/buildbot-master.test	2012-02-13 10:59:34 +0000
+++ tests/buildbot-master.test	2012-03-02 11:26:19 +0000
@@ -5,11 +5,14 @@
 from helpers import (
     command,
     make_charm_config_file,
+    maintain_charm_revision,
     unit_info,
+    upgrade_charm,
     wait_for_page_contents,
     wait_for_unit,
     )
 import os.path
+import time
 import unittest
 
 juju = command('juju')
@@ -27,27 +30,35 @@
             additional_args.append('--config=' + charm_config_file.name)
         juju('deploy', 'buildbot-master', *additional_args)
         wait_for_unit('buildbot-master')
+
+    def expose_and_check_page(self):
         juju('expose', 'buildbot-master')
         addr = unit_info('buildbot-master', 'public-address')
         url = 'http://{}:8010'.format(addr)
         wait_for_page_contents(url, 'Welcome to the Buildbot')
 
+    def get_config_file_contents(self):
+        bb_config_path = os.path.join(os.path.dirname(__file__), 'test.cfg')
+        return open(bb_config_path).read()
+
+    def get_config(self):
+        return {
+            'buildbot-master': {
+                'extra-packages': 'git',
+                'installdir': '/tmp/buildbot',
+                'config-file': self.get_config_file_contents(),
+            }}
+
     def test_port_opened(self):
         # Deploying a buildbot master should result in it opening a port and
         # serving its status via HTTP.
-        bb_config_path = os.path.join(os.path.dirname(__file__), 'test.cfg')
-        charm_config = {
-            'buildbot-master': {
-                'extra-packages': 'git',
-                'installdir': '/tmp/buildbot',
-                'config-file': open(bb_config_path).read(),
-            }}
-        self.deploy(charm_config)
+        self.deploy(self.get_config())
+        self.expose_and_check_page()
 
     def test_lpbuildbot(self):
         # Deploying a Launchpad-specific buildbot master does a good job of
         # exercising the configuration parameters.  For example, the
-        # configuration in this test adds a repositroy (lucid main universe),
+        # configuration in this test adds a repository (lucid main universe),
         # installs a non-default buildbot package, and fetches the buildbot
         # configuration from bzr.
         charm_config = {
@@ -62,6 +73,17 @@
                 'installdir': '/var/lib/buildbot/masters/lpbuildbot',
             }}
         self.deploy(charm_config)
+        self.expose_and_check_page()
+
+    def test_upgrade_charm(self):
+        # Ensure the charm can be upgraded without errors.
+        self.deploy(self.get_config())
+        with maintain_charm_revision():
+            upgrade_charm('buildbot-master')
+        # Wait for the charm to upgrade using sleep, since there is no
+        # other confirmation at the moment but the state to remain 'started'.
+        time.sleep(10)
+        self.expose_and_check_page()
 
 
 if __name__ == '__main__':