yellow team mailing list archive

[Graham Binns <graham@xxxxxxxxxxxxx>]

Graham Binns has proposed merging lp:~gmb/charms/oneiric/buildbot-master/master-smart-timeouts into lp:~yellow/charms/oneiric/buildbot-master/trunk.

Requested reviews:
  Launchpad Yellow Squad (yellow)

For more details, see:
https://code.launchpad.net/~gmb/charms/oneiric/buildbot-master/master-smart-timeouts/+merge/92733


I've solved the problem whereby tests would fail in EC2 environments due to machines not coming up quickly enough by adding a wait_for_machine() function which waits for the machine to come online (it returns early for LXC since this isn't a problem there). wait_for_unit() now calls wait_for_machine() before consuming its own timeout. We can probably tweak the timeout on wait_for_machine() to something much more reasonable.

Benji and I moved the deploy() function into TestCharm in buildbot-master.test on Friday afternoon, so it's better integrated for testing purposes.
-- 
https://code.launchpad.net/~gmb/charms/oneiric/buildbot-master/master-smart-timeouts/+merge/92733
Your team Launchpad Yellow Squad is requested to review the proposed merge of lp:~gmb/charms/oneiric/buildbot-master/master-smart-timeouts into lp:~yellow/charms/oneiric/buildbot-master/trunk.

=== modified file 'hooks/helpers.py'
--- hooks/helpers.py	2012-02-10 21:29:42 +0000
+++ hooks/helpers.py	2012-02-13 10:54:18 +0000
@@ -229,7 +229,49 @@
     return item
 
 
+<<<<<<< TREE
 def wait_for_unit(service_name, timeout=480):
+=======
+def wait_for_machine(num_machines=1, timeout=300):
+    """Wait `timeout` seconds for `num_machines` machines to come up.
+
+    This wait_for... function can be called by other wait_for functions
+    whose timeouts might be too short in situations where only a bare
+    Juju setup has been bootstrapped.
+    """
+    machine_data = yaml.safe_load(run('juju', 'status'))['machines']
+    # You may think this is a hack, and you'd be right. The easiest way
+    # 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 machine_data[0]['dns-name'] == 'localhost':
+        return
+    start_time = time.time()
+    while True:
+        machine_data = yaml.safe_load(run('juju', 'status'))['machines']
+        # Drop the first machine, since it's the Zookeeper and that's
+        # 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.
+        non_zookeeper_machines = [
+            machine_data[key] for key in machine_data.keys()[1:]]
+        if len(non_zookeeper_machines) >= num_machines:
+            all_machines_running = True
+            for machine in non_zookeeper_machines:
+                all_machines_running = (
+                    all_machines_running and
+                    machine['instance-state'] == 'running')
+            if all_machines_running:
+                break
+        if time.time() - start_time >= timeout:
+            raise RuntimeError('timeout waiting for service to start')
+        time.sleep(0.1)
+
+
+def wait_for_unit(service_name, timeout=240):
+    """Wait `timeout` seconds for a given service name to come up."""
+    wait_for_machine(num_machines=1)
+>>>>>>> MERGE-SOURCE
     start_time = time.time()
     while True:
         state = unit_info(service_name, 'state')
@@ -243,6 +285,7 @@
 
 
 def wait_for_relation(service_name, relation_name, timeout=120):
+    """Wait `timeout` seconds for a given relation to come up."""
     start_time = time.time()
     while True:
         relation = unit_info(service_name, 'relations').get(relation_name)
@@ -253,7 +296,8 @@
         time.sleep(0.1)
 
 
-def wait_for_page_contents(url, s, timeout=120):
+def wait_for_page_contents(url, expected_contents, timeout=120):
+    """Wait `timeout` seconds for a given URL to contain a given string."""
     start_time = time.time()
     while True:
         try:
@@ -262,7 +306,7 @@
             pass
         else:
             page = stream.read()
-            if s in page:
+            if expected_contents in page:
                 return page
         if time.time() - start_time >= timeout:
             raise RuntimeError('timeout waiting for contents of ' + url)

=== modified file 'tests/buildbot-master.test'
--- tests/buildbot-master.test	2012-02-10 21:29:42 +0000
+++ tests/buildbot-master.test	2012-02-13 10:54:18 +0000
@@ -4,7 +4,11 @@
 
 from helpers import (
     command,
+<<<<<<< TREE
     make_charm_config_file,
+=======
+    encode_file,
+>>>>>>> MERGE-SOURCE
     unit_info,
     wait_for_page_contents,
     wait_for_unit,
@@ -14,6 +18,7 @@
 
 juju = command('juju')
 
+<<<<<<< TREE
 def deploy(charm_config):
     charm_config_file = make_charm_config_file(charm_config)
     juju('deploy', 'buildbot-master', '--config='+charm_config_file.name)
@@ -23,18 +28,42 @@
     wait_for_page_contents(url, 'Welcome to the Buildbot')
 
 
+=======
+def make_charm_config_file(charm_config):
+    charm_config_file = tempfile.NamedTemporaryFile()
+    charm_config_file.write(yaml.dump(charm_config))
+    charm_config_file.flush()
+    # The NamedTemporaryFile instance is returned instead of just the name
+    # because we want to take advantage of garbage collection-triggered
+    # deletion of the temp file when it goes out of scope in the caller.
+    return charm_config_file
+
+
+>>>>>>> MERGE-SOURCE
 class TestCharm(unittest.TestCase):
 
     def tearDown(self):
         juju('destroy-service', 'buildbot-master')
 
+<<<<<<< TREE
     def test_deploy(self):
         # See if the charm will actual deploy correctly.  This test may be
         # made redundant by standard charm tests run by some future centralized
         # charm repository.
         juju('deploy', 'buildbot-master')
+=======
+    def deploy(self, charm_config=None):
+        additional_args = []
+        if charm_config is not None:
+            charm_config_file = make_charm_config_file(charm_config)
+            additional_args.append('--config=' + charm_config_file.name)
+        juju('deploy', 'buildbot-master', *additional_args)
+>>>>>>> MERGE-SOURCE
         wait_for_unit('buildbot-master')
-        self.assertEqual(unit_info('buildbot-master', 'state'), 'started')
+        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 test_port_opened(self):
         # Deploying a buildbot master should result in it opening a port and
@@ -46,7 +75,7 @@
                 'installdir': '/tmp/buildbot',
                 'config-file': open(bb_config_path).read(),
             }}
-        deploy(charm_config)
+        self.deploy(charm_config)
 
     def test_lpbuildbot(self):
         # Deploying a Launchpad-specific buildbot master does a good job of
@@ -65,7 +94,7 @@
                     ' lucid main universe',
                 'installdir': '/var/lib/buildbot/masters/lpbuildbot',
             }}
-        deploy(charm_config)
+        self.deploy(charm_config)
 
 
 if __name__ == '__main__':