Merge lp:~vila/ubuntu-ci-services-itself/1292461-testbed-console-acquisition into lp:ubuntu-ci-services-itself

[Vincent Ladeuil]

Better error handling when setting up the test bed.

This refactors the message handling for the test runner to better isolate the
failures during the testbed setup.

More details in the bug comment.

While I was there I added some more progress reports so we get a better idea
on how the time is spent during the request handling.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:390
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/427/
Executed test runs:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/uci-engine-ci/427/rebuild

review: Approve (continuous-integration)

[Andy Doan (doanac)]

The handle_request method is starting to get a little complex, but lets deal with that post-phase0

review: Approve

[Vincent Ladeuil (vila)]

> The handle_request method is starting to get a little complex, but lets deal
> with that post-phase0

Yeah and requires a bit too much care, full agreement.

On the other hand, the plan AIUI is to make it handle even more...

=== modified file 'test_runner/tstrun/run_worker.py'
--- test_runner/tstrun/run_worker.py	2014-03-14 10:37:28 +0000
+++ test_runner/tstrun/run_worker.py	2014-03-14 16:30:25 +0000
@@ -28,31 +28,31 @@
         super(TestRunnerWorker, self).__init__('test_runner')
         self.data_store = None
 
-    def save_subunit(self, log, package, stream, retval):
+    def save_subunit(self, log, package, stream, results):
         # make this exception-safe since we can already report pass/fail
         # with or without this file
         try:
-            log.info('saving subunit results for {}'.format(package))
+            log.info('Saving subunit results for {}'.format(package))
             name = 'test-runner.{}-subunit-stream'.format(package)
             url = self.data_store.put_file(name, stream, 'text/plain')
-            retval.setdefault('artifacts', []).append({
+            results.setdefault('artifacts', []).append({
                 'name': name,
                 'reference': url,
                 'type': 'RESULTS',
             })
         except:
             log.exception(
-                'unable to upload subunit result for {}'.format(package))
+                'Unable to upload subunit result for {}'.format(package))
 
-    def save_testbed_console(self, log, test_bed, retval):
+    def save_testbed_console(self, log, test_bed, results):
         # make this exception-safe since we can already report pass/fail
         # with or without this file
         try:
-            log.info('saving testbed console}')
+            log.info('Saving testbed console')
             name = 'test-runner.testbed-cloud-init.log'
             console = test_bed.get_cloud_init_console()
             url = self.data_store.put_file(name, console, 'text/plain')
-            retval.setdefault('artifacts', []).append({
+            results.setdefault('artifacts', []).append({
                 'name': name,
                 'reference': url,
                 'type': 'LOGS',
@@ -61,29 +61,48 @@
             log.exception('unable to upload testbed console')
 
     def handle_request(self, log, params):
+        ticket_id = params['ticket_id']
         progress_queue = params['progress_trigger']
         image_id = params['image_id']
         package_list = params['package_list']
 
+        results = {}
+
         def status_cb(msg):
             log.info(msg)
             amqp_utils.progress_update(progress_queue, {'message': msg})
 
-        self.data_store = self._create_data_store(params['ticket_id'])
-        # The tests will succeed unless they fail ;)
-        notify = amqp_utils.progress_completed
-        results = {}
-        test_bed = None
-        flavors = tstrun.get_auth_config()['tr_flavors']
+        self.data_store = self._create_data_store(ticket_id)
         try:
+            status_cb('Setting up the testbed for ticket {}'.format(ticket_id))
+            flavors = tstrun.get_auth_config()['tr_flavors']
             test_bed = testbed.TestBed('testbed-{}'.format(progress_queue),
                                        flavors, image_id, status_cb)
+        except:
+            log.exception(
+                'The testbed creation for {} failed'.format(ticket_id))
+            return amqp_utils.progress_failed, results
+
+        try:
             test_bed.setup()
+        except:
+            log.exception(
+                'The testbed setup for {} failed'.format(ticket_id))
+            if test_bed.instance is not None:
+                self.save_testbed_console(log, test_bed, results)
+                test_bed.teardown()
+            return amqp_utils.progress_failed, results
+
+        status_cb('The test bed is ready')
+        # The tests will succeed unless they fail ;)
+        notify = amqp_utils.progress_completed
+        try:
             for package in package_list:
                 if params.get('cancelled'):
-                    log.error('request has been cancelled, exiting')
+                    log.error('The request  for {} has been cancelled,'
+                              ' exiting'.format(ticket_id))
                     return amqp_utils.progress_failed, results
-                status_cb('testing %s' % package)
+                status_cb('Testing {}'.format(package))
                 # uci-vms shell adt-run ...  --- adt-virt-null
                 return_code, subunit_stream = test_bed.run_test(package)
                 # 0 is success, 8 is skipped and considered a success
@@ -91,11 +110,14 @@
                     # At least one test failed
                     notify = amqp_utils.progress_failed
                 self.save_subunit(log, package, subunit_stream, results)
+            status_cb('Test completed for ticket {}'.format(ticket_id))
             return notify, results
+        except:
+            log.exception(
+                'Exception while handling ticket {}'.format(ticket_id))
+            raise
         finally:
-            if test_bed:
-                self.save_testbed_console(log, test_bed, results)
-                test_bed.teardown()
+            test_bed.teardown()
 
 
 if __name__ == '__main__':
 
=== modified file 'test_runner/tstrun/testbed.py'
--- test_runner/tstrun/testbed.py	2014-03-14 10:37:28 +0000
+++ test_runner/tstrun/testbed.py	2014-03-14 16:30:25 +0000
@@ -189,8 +189,9 @@
         self.instance = self.nova.servers.create(
             name=self.name, flavor=flavor, image=image,
             userdata=self.user_data, keypair=self.nova_ssh_key_name)
-        # FIXME: This times out for hpcloud because it requires a floating ip
-        # for the instance see http://pad.lv/1288622 -- vila 2014-03-06
+        # FIXME: This times out for hpcloud (when used for testing from dev
+        # host) because it requires a floating ip for the instance see
+        # http://pad.lv/1288622 -- vila 2014-03-06
         self.wait_for_ip()
         self.wait_for_cloud_init()