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

Proposed by Para Siva on 2015-01-07
Status: Merged
Approved by: Para Siva on 2015-01-07
Approved revision: 915
Merged at revision: 914
Proposed branch: lp:~psivaa/uci-engine/swift-failure-exposure
Merge into: lp:uci-engine
Diff against target: 98 lines (+64/-0)
2 files modified
test_runner/tstrun/run_worker.py (+4/-0)
test_runner/tstrun/tests/test_worker.py (+60/-0)
To merge this branch: bzr merge lp:~psivaa/uci-engine/swift-failure-exposure
Reviewer Review Type Date Requested Status
Vincent Ladeuil (community) Approve on 2015-01-07
Joe Talbott (community) 2015-01-07 Approve on 2015-01-07
Review via email: mp+245745@code.launchpad.net

Commit message

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

Description of the change

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)

To post a comment you must log in.
Joe Talbott (joetalbott) wrote :

This looks good to me.

review: Approve
Vincent Ladeuil (vila) wrote :

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

Vincent Ladeuil (vila) wrote :

And I vote...

review: Needs Fixing
914. By Para Siva on 2015-01-07

Log before raising the exception

Para Siva (psivaa) wrote :

Thanks for that. Fixed that in r914.

Vincent Ladeuil (vila) wrote :

Thanks !

review: Approve
Vincent Ladeuil (vila) wrote :

pep8 ?

915. By Para Siva on 2015-01-07

pyflakes fix

Para Siva (psivaa) wrote :

pyflakes :) thanks

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'test_runner/tstrun/run_worker.py'
2--- test_runner/tstrun/run_worker.py 2014-12-15 09:25:00 +0000
3+++ test_runner/tstrun/run_worker.py 2015-01-07 16:31:30 +0000
4@@ -105,8 +105,12 @@
5 artifact_name = '{}.{}'.format(self.artifact_prefix, name)
6 self.artifacts.append(
7 dict(name=artifact_name, reference=url, type=kind))
8+ # Any swiftclient exception in data_store has to be treated here as
9+ # a failure in infra, hence specific client return codes
10+ # are not tested here
11 except:
12 self.logger.exception('Unable to upload {}'.format(description))
13+ raise
14
15 def save_testbed_console(self, package):
16 self.save_artifact('{}-testbed-cloud-init.log'.format(package),
17
18=== modified file 'test_runner/tstrun/tests/test_worker.py'
19--- test_runner/tstrun/tests/test_worker.py 2014-11-04 12:07:12 +0000
20+++ test_runner/tstrun/tests/test_worker.py 2015-01-07 16:31:30 +0000
21@@ -16,6 +16,8 @@
22 import os
23 import unittest
24
25+from swiftclient import client
26+
27 from ucitests import (
28 assertions,
29 fixtures as uci_fixtures,
30@@ -26,6 +28,7 @@
31 amqp_utils,
32 image_store,
33 unit_config,
34+ data_store,
35 )
36 from ci_utils.testing import (
37 features,
38@@ -227,3 +230,60 @@
39 run_worker.format_britney_result(
40 'trusty', 'armhf', 'adduser', 2,
41 '1.0', [('foo', '1.0')]))
42+
43+
44+class SaveArtefactClient(object):
45+ """Fake swift client to emulate swift failures.
46+
47+ This is injected into a DataStore object to execute failure scenarios
48+ """
49+
50+ def __init__(self, calls):
51+ self.calls = calls
52+
53+ def put_object(self, container, *args, **kwargs):
54+ return self.calls(container, *args, **kwargs)
55+
56+
57+class SaveArtefactDataStore(data_store.DataStore):
58+ """An instrumented DataStore to emulate errors in container upload."""
59+
60+ def __init__(self, component, calls):
61+ # We don't use the config so we inject a fake but valid one
62+ auth_config = {'auth_url': 'http://example.com',
63+ 'auth_user': 'user',
64+ 'auth_password': 'pass',
65+ 'auth_tenant_name': 'tenant',
66+ 'auth_region': 'region'}
67+ super(SaveArtefactDataStore, self).__init__(
68+ component, auth_config)
69+ self.client = SaveArtefactClient(calls)
70+
71+
72+class TestTestRequestHandler(unittest.TestCase):
73+ """Test the container deletion.
74+
75+ We don't have a swift server to inject errors into so we're faking that
76+ part to exercise the error handling in TestRequestHandler.save_artifact()
77+ """
78+ def raise_401(self, container, *args, **kwargs):
79+ # The return code does not matter so much,
80+ # any swiftclient exception in the swift client will have
81+ # to be treated as an infrastructure failure
82+ raise client.ClientException('401', http_status=401)
83+
84+ @tests.log_on_failure()
85+ def test_save_artefact_401(self, logger):
86+ rel_path = 'foo'
87+ content = 'foo content'
88+ calls = self.raise_401
89+ ds = SaveArtefactDataStore('ignored', calls)
90+ req_handler = run_worker.TestRequestHandler(logger,
91+ 'progress_queue_name',
92+ ds,
93+ 'ticket_id',
94+ 'some desc')
95+ with self.assertRaises(data_store.DataStoreException) as cm:
96+ req_handler.save_artifact(rel_path, content)
97+
98+ self.assertIn("Failed to upload file", '{}'.format(cm.exception))

Subscribers

People subscribed via source and target branches