Merge lp:~psivaa/uci-engine/swift-failure-exposure into lp:uci-engine

[Para Siva]

To ensure that we do not swallow the swiftclient related exceptions in the test runner worker

To ensure that we do not swallow the swiftclient related exceptions in the test runner worker. Any client error response in swiftclient during file uploads (save_artefact) is raised in data_store as an exception and this MP disables catching that exception in test runner.

The failure analysis, in http://bazaar.launchpad.net/~vila/uci-engine/analysis/view/head:/tasks.rst lists 3 types of possible failures swift related.
1. PUT can fail with 401 (tested with this MP)
2. ail to upload because we can't reset content (this should also raise client exception and therefore will be tested with the same test)
3. Failure due to connecting to keystone (this is being handled by some other tasks)

[Joe Talbott (joetalbott)]

This looks good to me.

review: Approve

[Vincent Ladeuil (vila)]

See inline comment for one fix, otherwise, well done, good to land.

[Vincent Ladeuil (vila)]

And I vote...

review: Needs Fixing

[lp:~psivaa/uci-engine/swift-failure-exposure]

914. By Para Siva on 2015-01-07

Log before raising the exception

[Para Siva (psivaa)]

Thanks for that. Fixed that in r914.

[Vincent Ladeuil (vila)]

Thanks !

review: Approve

[Vincent Ladeuil (vila)]

pep8 ?

[lp:~psivaa/uci-engine/swift-failure-exposure]

915. By Para Siva on 2015-01-07

pyflakes fix

[Para Siva (psivaa)]

pyflakes :) thanks

=== modified file 'test_runner/tstrun/run_worker.py'
--- test_runner/tstrun/run_worker.py	2014-12-15 09:25:00 +0000
+++ test_runner/tstrun/run_worker.py	2015-01-07 16:31:30 +0000
@@ -105,8 +105,12 @@
             artifact_name = '{}.{}'.format(self.artifact_prefix, name)
             self.artifacts.append(
                 dict(name=artifact_name, reference=url, type=kind))
+        # Any swiftclient exception in data_store has to be treated here as
+        # a failure in infra, hence specific client return codes
+        # are not tested here
         except:
             self.logger.exception('Unable to upload {}'.format(description))
+            raise
 
     def save_testbed_console(self, package):
         self.save_artifact('{}-testbed-cloud-init.log'.format(package),
 
=== modified file 'test_runner/tstrun/tests/test_worker.py'
--- test_runner/tstrun/tests/test_worker.py	2014-11-04 12:07:12 +0000
+++ test_runner/tstrun/tests/test_worker.py	2015-01-07 16:31:30 +0000
@@ -16,6 +16,8 @@
 import os
 import unittest
 
+from swiftclient import client
+
 from ucitests import (
     assertions,
     fixtures as uci_fixtures,
@@ -26,6 +28,7 @@
     amqp_utils,
     image_store,
     unit_config,
+    data_store,
 )
 from ci_utils.testing import (
     features,
@@ -227,3 +230,60 @@
                          run_worker.format_britney_result(
                              'trusty', 'armhf', 'adduser', 2,
                              '1.0', [('foo', '1.0')]))
+
+
+class SaveArtefactClient(object):
+    """Fake swift client to emulate swift failures.
+
+    This is injected into a DataStore object to execute failure scenarios
+    """
+
+    def __init__(self, calls):
+        self.calls = calls
+
+    def put_object(self, container, *args, **kwargs):
+        return self.calls(container, *args, **kwargs)
+
+
+class SaveArtefactDataStore(data_store.DataStore):
+    """An instrumented DataStore to emulate errors in container upload."""
+
+    def __init__(self, component, calls):
+        # We don't use the config so we inject a fake but valid one
+        auth_config = {'auth_url': 'http://example.com',
+                       'auth_user': 'user',
+                       'auth_password': 'pass',
+                       'auth_tenant_name': 'tenant',
+                       'auth_region': 'region'}
+        super(SaveArtefactDataStore, self).__init__(
+            component, auth_config)
+        self.client = SaveArtefactClient(calls)
+
+
+class TestTestRequestHandler(unittest.TestCase):
+    """Test the container deletion.
+
+    We don't have a swift server to inject errors into so we're faking that
+    part to exercise the error handling in TestRequestHandler.save_artifact()
+    """
+    def raise_401(self, container, *args, **kwargs):
+        # The return code does not matter so much,
+        # any swiftclient exception in the swift client will have
+        # to be treated as an infrastructure failure
+        raise client.ClientException('401', http_status=401)
+
+    @tests.log_on_failure()
+    def test_save_artefact_401(self, logger):
+        rel_path = 'foo'
+        content = 'foo content'
+        calls = self.raise_401
+        ds = SaveArtefactDataStore('ignored', calls)
+        req_handler = run_worker.TestRequestHandler(logger,
+                                                    'progress_queue_name',
+                                                    ds,
+                                                    'ticket_id',
+                                                    'some desc')
+        with self.assertRaises(data_store.DataStoreException) as cm:
+            req_handler.save_artifact(rel_path, content)
+
+        self.assertIn("Failed to upload file", '{}'.format(cm.exception))