Merge lp:~cjwatson/launchpad/upgrade-keystoneclient-swiftclient into lp:launchpad

Proposed by Colin Watson on 2017-12-19
Status: Needs review
Proposed branch: lp:~cjwatson/launchpad/upgrade-keystoneclient-swiftclient
Merge into: lp:launchpad
Diff against target: 417 lines (+119/-43)
8 files modified
constraints.txt (+7/-5)
lib/lp/services/librarianserver/librariangc.py (+9/-5)
lib/lp/services/librarianserver/storage.py (+3/-3)
lib/lp/services/librarianserver/swift.py (+40/-7)
lib/lp/services/librarianserver/tests/test_gc.py (+2/-1)
lib/lp/services/librarianserver/tests/test_swift.py (+26/-7)
lib/lp/testing/swift/fakeswift.py (+28/-12)
lib/lp/testing/swift/tests/test_fixture.py (+4/-3)
To merge this branch: bzr merge lp:~cjwatson/launchpad/upgrade-keystoneclient-swiftclient
Reviewer Review Type Date Requested Status
William Grant code 2017-12-19 Approve on 2018-01-02
Review via email: mp+335391@code.launchpad.net

Commit Message

Upgrade to python-keystoneclient 0.7.1 and python-swiftclient 2.0.3.

Description of the Change

These were the latest versions in Icehouse, to go with keystone 2014.1.5 and
swift 1.13.1 currently on the server. Upgrading to these allows us to use a
more recent version of pbr that is compatible with other dependencies we'd
like to upgrade.

We need a few tweaks to make everything work properly:

 * quieten overly-noisy logging (needed until python-swiftclient 3.2.0);
 * explicitly restrict HashStream to the desired segment size, since
   otherwise the client will read past the end of the segment and we can end
   up with corrupted hashes;
 * advertise a keystone endpoint in the fake Swift fixture;
 * cope with slightly different exception types due to the client now using
   requests.

To post a comment you must log in.
William Grant (wgrant) wrote :

We'll probably need to tweak staging configs a bit to effectively test this. Have you verified in any depth against a non-prod Swift?

review: Approve (code)
18520. By Colin Watson on 2018-01-02

Tag swiftclient logging comment with XXX.

18521. By Colin Watson on 2018-01-03

Quieten swiftclient logging using a wrapper function rather than a context manager.

Unmerged revisions

18521. By Colin Watson on 2018-01-03

Quieten swiftclient logging using a wrapper function rather than a context manager.

18520. By Colin Watson on 2018-01-02

Tag swiftclient logging comment with XXX.

18519. By Colin Watson on 2017-12-19

Upgrade to python-keystoneclient 0.7.1 and python-swiftclient 2.0.3.

These were the latest versions in Icehouse, to go with keystone 2014.1.5 and
swift 1.13.1 currently on the server. Upgrading to these allows us to use a
more recent version of pbr that is compatible with other dependencies we'd
like to upgrade.

We need a few tweaks to make everything work properly:

 * quieten overly-noisy logging (needed until python-swiftclient 3.2.0);
 * explicitly restrict HashStream to the desired segment size, since
   otherwise the client will read past the end of the segment and we can end
   up with corrupted hashes;
 * advertise a keystone endpoint in the fake Swift fixture;
 * cope with slightly different exception types due to the client now using
   requests.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'constraints.txt'
2--- constraints.txt 2017-12-22 09:59:33 +0000
3+++ constraints.txt 2018-01-03 17:18:41 +0000
4@@ -228,6 +228,7 @@
5 auditor==0.0.3
6 auditorclient==0.0.4
7 auditorfixture==0.0.7
8+Babel==2.5.1
9 backports.lzma==0.0.3
10 BeautifulSoup==3.2.1
11 billiard==3.3.0.20
12@@ -256,7 +257,7 @@
13 httplib2==0.8
14 importlib==1.0.2
15 ipython==0.13.2
16-iso8601==0.1.4
17+iso8601==0.1.12
18 jsautobuild==0.2
19 keyring==0.6.2
20 kombu==3.0.30
21@@ -283,6 +284,7 @@
22 meliae==0.2.0.final.0
23 mock==1.0.1
24 mocker==1.1.1
25+netaddr==0.7.19
26 oauth==1.0
27 oops==0.0.13
28 oops-amqp==0.0.8b1
29@@ -291,9 +293,9 @@
30 oops-twisted==0.0.7
31 oops-wsgi==0.0.8
32 ordereddict==1.1
33-oslo.config==1.1.1
34+oslo.config==1.3.0
35 paramiko==1.7.7.2
36-pbr==0.5.20
37+pbr==0.11.1
38 pgbouncer==0.0.8
39 prettytable==0.7.2
40 psycopg2==2.6.1
41@@ -306,14 +308,14 @@
42 pystache==0.5.3
43 python-dateutil==1.5
44 python-debian==0.1.23
45-python-keystoneclient==0.3.1
46+python-keystoneclient==0.7.1
47 python-memcached==1.58
48 python-mimeparse==0.1.4
49 # XXX: deryck 2012-08-10
50 # See lp:~deryck/python-openid/python-openid-fix1034376 which
51 # reapplied a patch from wgrant to get codehosting going again.
52 python-openid==2.2.5-fix1034376
53-python-swiftclient==1.5.0
54+python-swiftclient==2.0.3
55 PyYAML==3.10
56 rabbitfixture==0.3.6
57 requests==2.7.0
58
59=== modified file 'lib/lp/services/librarianserver/librariangc.py'
60--- lib/lp/services/librarianserver/librariangc.py 2017-04-02 01:17:50 +0000
61+++ lib/lp/services/librarianserver/librariangc.py 2018-01-03 17:18:41 +0000
62@@ -1,4 +1,4 @@
63-# Copyright 2009 Canonical Ltd. This software is licensed under the
64+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
65 # GNU Affero General Public License version 3 (see the file LICENSE).
66
67 """Librarian garbage collection routines"""
68@@ -55,7 +55,8 @@
69 swift_connection = swift.connection_pool.get()
70 container, name = swift.swift_location(content_id)
71 try:
72- swift_connection.head_object(container, name)
73+ swift.quiet_swiftclient(
74+ swift_connection.head_object, container, name)
75 return True
76 except swiftclient.ClientException as x:
77 if x.http_status != 404:
78@@ -79,7 +80,8 @@
79 try:
80 swift_connection = swift.connection_pool.get()
81 container, name = swift.swift_location(content_id)
82- chunks = swift_connection.get_object(
83+ chunks = swift.quiet_swiftclient(
84+ swift_connection.get_object,
85 container, name, resp_chunk_size=STREAM_CHUNK_SIZE)[1]
86 return swift.SwiftStream(swift_connection, chunks)
87 except swiftclient.ClientException as x:
88@@ -562,7 +564,8 @@
89 container, name = swift.swift_location(content_id)
90 with swift.connection() as swift_connection:
91 try:
92- swift_connection.delete_object(container, name)
93+ swift.quiet_swiftclient(
94+ swift_connection.delete_object, container, name)
95 removed.append('Swift')
96 except swiftclient.ClientException as x:
97 if x.http_status != 404:
98@@ -740,7 +743,8 @@
99 container = swift.SWIFT_CONTAINER_PREFIX + str(container_num)
100 try:
101 names = sorted(
102- swift_connection.get_container(
103+ swift.quiet_swiftclient(
104+ swift_connection.get_container,
105 container, full_listing=True)[1],
106 key=lambda x: map(int, x['name'].split('/')))
107 for name in names:
108
109=== modified file 'lib/lp/services/librarianserver/storage.py'
110--- lib/lp/services/librarianserver/storage.py 2016-09-19 13:53:42 +0000
111+++ lib/lp/services/librarianserver/storage.py 2018-01-03 17:18:41 +0000
112@@ -1,4 +1,4 @@
113-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
114+# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
115 # GNU Affero General Public License version 3 (see the file LICENSE).
116
117 __metaclass__ = type
118@@ -10,9 +10,9 @@
119 import tempfile
120
121 from swiftclient import client as swiftclient
122-from twisted.python import log
123 from twisted.internet import defer
124 from twisted.internet.threads import deferToThread
125+from twisted.python import log
126 from twisted.web.static import StaticProducer
127
128 from lp.registry.model.product import Product
129@@ -124,7 +124,7 @@
130 swift_connection = swift.connection_pool.get()
131 try:
132 headers, chunks = yield deferToThread(
133- swift_connection.get_object,
134+ swift.quiet_swiftclient, swift_connection.get_object,
135 container, name, resp_chunk_size=self.CHUNK_SIZE)
136 swift_stream = TxSwiftStream(swift_connection, chunks)
137 defer.returnValue(swift_stream)
138
139=== modified file 'lib/lp/services/librarianserver/swift.py'
140--- lib/lp/services/librarianserver/swift.py 2015-02-16 00:09:14 +0000
141+++ lib/lp/services/librarianserver/swift.py 2018-01-03 17:18:41 +0000
142@@ -1,12 +1,17 @@
143-# Copyright 2013 Canonical Ltd. This software is licensed under the
144+# Copyright 2013-2017 Canonical Ltd. This software is licensed under the
145 # GNU Affero General Public License version 3 (see the file LICENSE).
146
147 """Move files from Librarian disk storage into Swift."""
148
149 __metaclass__ = type
150 __all__ = [
151- 'to_swift', 'filesystem_path', 'swift_location',
152- 'connection', 'connection_pool', 'SWIFT_CONTAINER_PREFIX',
153+ 'SWIFT_CONTAINER_PREFIX',
154+ 'connection',
155+ 'connection_pool',
156+ 'filesystem_path',
157+ 'quiet_swiftclient',
158+ 'swift_location',
159+ 'to_swift',
160 ]
161
162 from contextlib import contextmanager
163@@ -29,6 +34,24 @@
164 ONE_DAY = 24 * 60 * 60
165
166
167+def quiet_swiftclient(func, *args, **kwargs):
168+ # XXX cjwatson 2018-01-02: swiftclient has some very rude logging
169+ # practices: the low-level API calls `logger.exception` when a request
170+ # fails, without considering whether the caller might handle it and
171+ # recover. This was introduced in 1.6.0 and removed in 3.2.0; until
172+ # we're on a new enough version not to need to worry about this, we shut
173+ # up the noisy logging around calls whose failure we can handle.
174+ # Messier still, logging.getLogger('swiftclient') doesn't necessarily
175+ # refer to the Logger instance actually being used by swiftclient, so we
176+ # have to use swiftclient.logger directly.
177+ old_disabled = swiftclient.logger.disabled
178+ try:
179+ swiftclient.logger.disabled = True
180+ return func(*args, **kwargs)
181+ finally:
182+ swiftclient.logger.disabled = old_disabled
183+
184+
185 def to_swift(log, start_lfc_id=None, end_lfc_id=None, remove_func=False):
186 '''Copy a range of Librarian files from disk into Swift.
187
188@@ -118,7 +141,7 @@
189 container, obj_name = swift_location(lfc)
190
191 try:
192- swift_connection.head_container(container)
193+ quiet_swiftclient(swift_connection.head_container, container)
194 log.debug2('{0} container already exists'.format(container))
195 except swiftclient.ClientException as x:
196 if x.http_status != 404:
197@@ -127,7 +150,8 @@
198 swift_connection.put_container(container)
199
200 try:
201- headers = swift_connection.head_object(container, obj_name)
202+ headers = quiet_swiftclient(
203+ swift_connection.head_object, container, obj_name)
204 log.debug(
205 "{0} already exists in Swift({1}, {2})".format(
206 lfc, container, obj_name))
207@@ -188,7 +212,7 @@
208 assert segment <= 9999, 'Insane number of segments'
209 seg_name = '%s/%04d' % (obj_name, segment)
210 seg_size = min(fs_size - fs_file.tell(), MAX_SWIFT_OBJECT_SIZE)
211- md5_stream = HashStream(fs_file)
212+ md5_stream = HashStream(fs_file, length=seg_size)
213 swift_md5_hash = swift_connection.put_object(
214 container, seg_name, md5_stream, seg_size)
215 segment_md5_hash = md5_stream.hash.hexdigest()
216@@ -304,13 +328,20 @@
217
218 class HashStream:
219 """Read a file while calculating a checksum as we go."""
220- def __init__(self, stream, hash_factory=hashlib.md5):
221+ def __init__(self, stream, length=None, hash_factory=hashlib.md5):
222 self._stream = stream
223+ self._length = self._remaining = length
224 self.hash_factory = hash_factory
225 self.hash = hash_factory()
226
227 def read(self, size=-1):
228+ if self._remaining is not None:
229+ if self._remaining <= 0:
230+ return ''
231+ size = min(size, self._remaining)
232 chunk = self._stream.read(size)
233+ if self._remaining is not None:
234+ self._remaining -= len(chunk)
235 self.hash.update(chunk)
236 return chunk
237
238@@ -320,6 +351,8 @@
239 def seek(self, offset):
240 """Seek to offset, and reset the hash."""
241 self.hash = self.hash_factory()
242+ if self._remaining is not None:
243+ self._remaining = self._length - offset
244 return self._stream.seek(offset)
245
246
247
248=== modified file 'lib/lp/services/librarianserver/tests/test_gc.py'
249--- lib/lp/services/librarianserver/tests/test_gc.py 2018-01-02 16:10:26 +0000
250+++ lib/lp/services/librarianserver/tests/test_gc.py 2018-01-03 17:18:41 +0000
251@@ -728,7 +728,8 @@
252 name += suffix
253 with swift.connection() as swift_connection:
254 try:
255- swift_connection.head_object(container, name)
256+ swift.quiet_swiftclient(
257+ swift_connection.head_object, container, name)
258 return True
259 except swiftclient.ClientException as x:
260 if x.http_status == 404:
261
262=== modified file 'lib/lp/services/librarianserver/tests/test_swift.py'
263--- lib/lp/services/librarianserver/tests/test_swift.py 2018-01-02 16:10:26 +0000
264+++ lib/lp/services/librarianserver/tests/test_swift.py 2018-01-03 17:18:41 +0000
265@@ -1,4 +1,4 @@
266-# Copyright 2013 Canonical Ltd. This software is licensed under the
267+# Copyright 2013-2017 Canonical Ltd. This software is licensed under the
268 # GNU Affero General Public License version 3 (see the file LICENSE).
269
270 """Librarian disk to Swift storage tests."""
271@@ -19,14 +19,17 @@
272 from lp.services.features.testing import FeatureFixture
273 from lp.services.librarian.client import LibrarianClient
274 from lp.services.librarian.model import LibraryFileAlias
275+from lp.services.librarianserver import swift
276 from lp.services.librarianserver.storage import LibrarianStorage
277 from lp.services.log.logger import BufferLogger
278 from lp.testing import TestCase
279-from lp.testing.layers import BaseLayer, LaunchpadZopelessLayer, LibrarianLayer
280+from lp.testing.layers import (
281+ BaseLayer,
282+ LaunchpadZopelessLayer,
283+ LibrarianLayer,
284+ )
285 from lp.testing.swift.fixture import SwiftFixture
286
287-from lp.services.librarianserver import swift
288-
289
290 class TestFeedSwift(TestCase):
291 layer = LaunchpadZopelessLayer
292@@ -272,8 +275,8 @@
293 _, obj2 = swift_client.get_object(container, '{0}/0001'.format(name))
294 _, obj3 = swift_client.get_object(container, '{0}/0002'.format(name))
295 self.assertRaises(
296- swiftclient.ClientException, swift_client.get_object,
297- container, '{0}/0003'.format(name))
298+ swiftclient.ClientException, swift.quiet_swiftclient,
299+ swift_client.get_object, container, '{0}/0003'.format(name))
300
301 # Our object round tripped
302 self.assertEqual(obj1 + obj2 + obj3, expected_content)
303@@ -293,7 +296,8 @@
304
305 def test_partial_read(self):
306 empty_sha1 = 'da39a3ee5e6b4b0d3255bfef95601890afd80709'
307- s = swift.HashStream(StringIO('make me another coffee'), hashlib.sha1)
308+ s = swift.HashStream(
309+ StringIO('make me another coffee'), hash_factory=hashlib.sha1)
310 self.assertEqual(s.hash.hexdigest(), empty_sha1)
311 chunk = s.read(4)
312 self.assertEqual(chunk, 'make')
313@@ -304,6 +308,21 @@
314 self.assertEqual(s.hash.hexdigest(),
315 '8c826e573016ce05f3968044f82507b46fd2aa93')
316
317+ def test_limited_length(self):
318+ base_stream = StringIO('make me a coffee')
319+ s = swift.HashStream(base_stream, length=8)
320+ chunk = s.read(4)
321+ self.assertEqual(chunk, 'make')
322+ self.assertEqual(s.hash.hexdigest(),
323+ '099dafc678df7d266c25f95ccf6cde22')
324+ chunk = s.read(8)
325+ self.assertEqual(chunk, ' me ')
326+ self.assertEqual(s.hash.hexdigest(),
327+ '10a0334e435b75f35b1923842bd87f81')
328+ self.assertEqual(s.read(), '')
329+ self.assertEqual(s.tell(), 8)
330+ self.assertEqual(base_stream.tell(), 8)
331+
332 def test_tell(self):
333 s = swift.HashStream(StringIO('hurry up with that coffee'))
334 self.assertEqual(s.tell(), 0)
335
336=== modified file 'lib/lp/testing/swift/fakeswift.py'
337--- lib/lp/testing/swift/fakeswift.py 2017-12-07 12:05:13 +0000
338+++ lib/lp/testing/swift/fakeswift.py 2018-01-03 17:18:41 +0000
339@@ -557,17 +557,33 @@
340 """Compute service catalog for the given request and tenant."""
341 port = request.transport.socket.getsockname()[1]
342 tenant_id = self.tenants[tenant]["id"]
343- base_url = "http://%s:%d/swift/v1" % (self.hostname, port)
344+ base_url = "http://%s:%d" % (self.hostname, port)
345+ keystone_base_url = "%s/keystone/v2.0" % base_url
346+ swift_base_url = "%s/swift/v1" % base_url
347 catalog = [
348- {"endpoints": [
349- {"adminURL": base_url,
350- "id": uuid.uuid4().hex,
351- "internalURL": base_url + "/AUTH_" + tenant_id,
352- "publicURL": base_url + "/AUTH_" + tenant_id,
353- "region": DEFAULT_REGION}
354- ],
355- "endpoints_links": [],
356- "name": "swift",
357- "type": "object-store"
358- }]
359+ {
360+ "endpoints": [{
361+ "adminURL": keystone_base_url,
362+ "id": uuid.uuid4().hex,
363+ "internalURL": keystone_base_url,
364+ "publicURL": keystone_base_url,
365+ "region": DEFAULT_REGION,
366+ }],
367+ "endpoints_links": [],
368+ "name": "keystone",
369+ "type": "identity",
370+ },
371+ {
372+ "endpoints": [{
373+ "adminURL": swift_base_url,
374+ "id": uuid.uuid4().hex,
375+ "internalURL": swift_base_url + "/AUTH_" + tenant_id,
376+ "publicURL": swift_base_url + "/AUTH_" + tenant_id,
377+ "region": DEFAULT_REGION,
378+ }],
379+ "endpoints_links": [],
380+ "name": "swift",
381+ "type": "object-store",
382+ },
383+ ]
384 return catalog
385
386=== modified file 'lib/lp/testing/swift/tests/test_fixture.py'
387--- lib/lp/testing/swift/tests/test_fixture.py 2018-01-02 16:10:26 +0000
388+++ lib/lp/testing/swift/tests/test_fixture.py 2018-01-03 17:18:41 +0000
389@@ -8,9 +8,10 @@
390
391 from datetime import datetime
392 from hashlib import md5
393-import httplib
394
395+from requests.exceptions import ConnectionError
396 from swiftclient import client as swiftclient
397+from swiftclient.exceptions import ClientException
398 from testtools.matchers import (
399 GreaterThan,
400 LessThan,
401@@ -200,14 +201,14 @@
402 # authenticated.
403 self.swift_fixture.shutdown()
404 self.assertRaises(
405- httplib.HTTPException,
406+ ConnectionError,
407 client.get_object, "size", str(size))
408
409 # And even if we bring it back up, existing connections
410 # continue to fail
411 self.swift_fixture.startup()
412 self.assertRaises(
413- httplib.HTTPException,
414+ ClientException,
415 client.get_object, "size", str(size))
416
417 # But fresh connections are fine.