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

[Colin Watson]

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.

[William Grant (wgrant)]

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)

[Colin Watson (cjwatson)]

OK, I eventually managed to get Icehouse Keystone and Swift deployed locally, and tested this against them. librarian-feed-swift, librariangc, downloads from the librarian after renaming aside the local files, and segmented uploads (with the threshold tweaked down to 5MiB from 5GiB) all work fine. I'm going to go ahead and land this, but let me know if you think of anything else I ought to test.

=== modified file 'constraints.txt'
--- constraints.txt	2017-12-22 09:59:33 +0000
+++ constraints.txt	2018-02-13 16:28:38 +0000
@@ -228,6 +228,7 @@
 auditor==0.0.3
 auditorclient==0.0.4
 auditorfixture==0.0.7
+Babel==2.5.1
 backports.lzma==0.0.3
 BeautifulSoup==3.2.1
 billiard==3.3.0.20
@@ -256,7 +257,7 @@
 httplib2==0.8
 importlib==1.0.2
 ipython==0.13.2
-iso8601==0.1.4
+iso8601==0.1.12
 jsautobuild==0.2
 keyring==0.6.2
 kombu==3.0.30
@@ -283,6 +284,7 @@
 meliae==0.2.0.final.0
 mock==1.0.1
 mocker==1.1.1
+netaddr==0.7.19
 oauth==1.0
 oops==0.0.13
 oops-amqp==0.0.8b1
@@ -291,9 +293,9 @@
 oops-twisted==0.0.7
 oops-wsgi==0.0.8
 ordereddict==1.1
-oslo.config==1.1.1
+oslo.config==1.3.0
 paramiko==1.7.7.2
-pbr==0.5.20
+pbr==0.11.1
 pgbouncer==0.0.8
 prettytable==0.7.2
 psycopg2==2.6.1
@@ -306,14 +308,14 @@
 pystache==0.5.3
 python-dateutil==1.5
 python-debian==0.1.23
-python-keystoneclient==0.3.1
+python-keystoneclient==0.7.1
 python-memcached==1.58
 python-mimeparse==0.1.4
 # XXX: deryck 2012-08-10
 # See lp:~deryck/python-openid/python-openid-fix1034376 which
 # reapplied a patch from wgrant to get codehosting going again.
 python-openid==2.2.5-fix1034376
-python-swiftclient==1.5.0
+python-swiftclient==2.0.3
 PyYAML==3.10
 rabbitfixture==0.3.6
 requests==2.7.0

=== modified file 'lib/lp/services/librarianserver/librariangc.py'
--- lib/lp/services/librarianserver/librariangc.py	2017-04-02 01:17:50 +0000
+++ lib/lp/services/librarianserver/librariangc.py	2018-02-13 16:28:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Librarian garbage collection routines"""
@@ -55,7 +55,8 @@
         swift_connection = swift.connection_pool.get()
         container, name = swift.swift_location(content_id)
         try:
-            swift_connection.head_object(container, name)
+            swift.quiet_swiftclient(
+                swift_connection.head_object, container, name)
             return True
         except swiftclient.ClientException as x:
             if x.http_status != 404:
@@ -79,7 +80,8 @@
         try:
             swift_connection = swift.connection_pool.get()
             container, name = swift.swift_location(content_id)
-            chunks = swift_connection.get_object(
+            chunks = swift.quiet_swiftclient(
+                swift_connection.get_object,
                 container, name, resp_chunk_size=STREAM_CHUNK_SIZE)[1]
             return swift.SwiftStream(swift_connection, chunks)
         except swiftclient.ClientException as x:
@@ -562,7 +564,8 @@
                 container, name = swift.swift_location(content_id)
                 with swift.connection() as swift_connection:
                     try:
-                        swift_connection.delete_object(container, name)
+                        swift.quiet_swiftclient(
+                            swift_connection.delete_object, container, name)
                         removed.append('Swift')
                     except swiftclient.ClientException as x:
                         if x.http_status != 404:
@@ -740,7 +743,8 @@
             container = swift.SWIFT_CONTAINER_PREFIX + str(container_num)
             try:
                 names = sorted(
-                    swift_connection.get_container(
+                    swift.quiet_swiftclient(
+                        swift_connection.get_container,
                         container, full_listing=True)[1],
                     key=lambda x: map(int, x['name'].split('/')))
                 for name in names:

=== modified file 'lib/lp/services/librarianserver/storage.py'
--- lib/lp/services/librarianserver/storage.py	2016-09-19 13:53:42 +0000
+++ lib/lp/services/librarianserver/storage.py	2018-02-13 16:28:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -10,9 +10,9 @@
 import tempfile
 
 from swiftclient import client as swiftclient
-from twisted.python import log
 from twisted.internet import defer
 from twisted.internet.threads import deferToThread
+from twisted.python import log
 from twisted.web.static import StaticProducer
 
 from lp.registry.model.product import Product
@@ -124,7 +124,7 @@
             swift_connection = swift.connection_pool.get()
             try:
                 headers, chunks = yield deferToThread(
-                    swift_connection.get_object,
+                    swift.quiet_swiftclient, swift_connection.get_object,
                     container, name, resp_chunk_size=self.CHUNK_SIZE)
                 swift_stream = TxSwiftStream(swift_connection, chunks)
                 defer.returnValue(swift_stream)

=== modified file 'lib/lp/services/librarianserver/swift.py'
--- lib/lp/services/librarianserver/swift.py	2015-02-16 00:09:14 +0000
+++ lib/lp/services/librarianserver/swift.py	2018-02-13 16:28:38 +0000
@@ -1,12 +1,17 @@
-# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2013-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Move files from Librarian disk storage into Swift."""
 
 __metaclass__ = type
 __all__ = [
-    'to_swift', 'filesystem_path', 'swift_location',
-    'connection', 'connection_pool', 'SWIFT_CONTAINER_PREFIX',
+    'SWIFT_CONTAINER_PREFIX',
+    'connection',
+    'connection_pool',
+    'filesystem_path',
+    'quiet_swiftclient',
+    'swift_location',
+    'to_swift',
     ]
 
 from contextlib import contextmanager
@@ -29,6 +34,24 @@
 ONE_DAY = 24 * 60 * 60
 
 
+def quiet_swiftclient(func, *args, **kwargs):
+    # XXX cjwatson 2018-01-02: swiftclient has some very rude logging
+    # practices: the low-level API calls `logger.exception` when a request
+    # fails, without considering whether the caller might handle it and
+    # recover.  This was introduced in 1.6.0 and removed in 3.2.0; until
+    # we're on a new enough version not to need to worry about this, we shut
+    # up the noisy logging around calls whose failure we can handle.
+    # Messier still, logging.getLogger('swiftclient') doesn't necessarily
+    # refer to the Logger instance actually being used by swiftclient, so we
+    # have to use swiftclient.logger directly.
+    old_disabled = swiftclient.logger.disabled
+    try:
+        swiftclient.logger.disabled = True
+        return func(*args, **kwargs)
+    finally:
+        swiftclient.logger.disabled = old_disabled
+
+
 def to_swift(log, start_lfc_id=None, end_lfc_id=None, remove_func=False):
     '''Copy a range of Librarian files from disk into Swift.
 
@@ -118,7 +141,7 @@
             container, obj_name = swift_location(lfc)
 
             try:
-                swift_connection.head_container(container)
+                quiet_swiftclient(swift_connection.head_container, container)
                 log.debug2('{0} container already exists'.format(container))
             except swiftclient.ClientException as x:
                 if x.http_status != 404:
@@ -127,7 +150,8 @@
                 swift_connection.put_container(container)
 
             try:
-                headers = swift_connection.head_object(container, obj_name)
+                headers = quiet_swiftclient(
+                    swift_connection.head_object, container, obj_name)
                 log.debug(
                     "{0} already exists in Swift({1}, {2})".format(
                         lfc, container, obj_name))
@@ -188,7 +212,7 @@
             assert segment <= 9999, 'Insane number of segments'
             seg_name = '%s/%04d' % (obj_name, segment)
             seg_size = min(fs_size - fs_file.tell(), MAX_SWIFT_OBJECT_SIZE)
-            md5_stream = HashStream(fs_file)
+            md5_stream = HashStream(fs_file, length=seg_size)
             swift_md5_hash = swift_connection.put_object(
                 container, seg_name, md5_stream, seg_size)
             segment_md5_hash = md5_stream.hash.hexdigest()
@@ -304,13 +328,20 @@
 
 class HashStream:
     """Read a file while calculating a checksum as we go."""
-    def __init__(self, stream, hash_factory=hashlib.md5):
+    def __init__(self, stream, length=None, hash_factory=hashlib.md5):
         self._stream = stream
+        self._length = self._remaining = length
         self.hash_factory = hash_factory
         self.hash = hash_factory()
 
     def read(self, size=-1):
+        if self._remaining is not None:
+            if self._remaining <= 0:
+                return ''
+            size = min(size, self._remaining)
         chunk = self._stream.read(size)
+        if self._remaining is not None:
+            self._remaining -= len(chunk)
         self.hash.update(chunk)
         return chunk
 
@@ -320,6 +351,8 @@
     def seek(self, offset):
         """Seek to offset, and reset the hash."""
         self.hash = self.hash_factory()
+        if self._remaining is not None:
+            self._remaining = self._length - offset
         return self._stream.seek(offset)
 
 

=== modified file 'lib/lp/services/librarianserver/tests/test_gc.py'
--- lib/lp/services/librarianserver/tests/test_gc.py	2018-01-02 16:10:26 +0000
+++ lib/lp/services/librarianserver/tests/test_gc.py	2018-02-13 16:28:38 +0000
@@ -728,7 +728,8 @@
             name += suffix
         with swift.connection() as swift_connection:
             try:
-                swift_connection.head_object(container, name)
+                swift.quiet_swiftclient(
+                    swift_connection.head_object, container, name)
                 return True
             except swiftclient.ClientException as x:
                 if x.http_status == 404:

=== modified file 'lib/lp/services/librarianserver/tests/test_swift.py'
--- lib/lp/services/librarianserver/tests/test_swift.py	2018-01-02 16:10:26 +0000
+++ lib/lp/services/librarianserver/tests/test_swift.py	2018-02-13 16:28:38 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2013-2017 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Librarian disk to Swift storage tests."""
@@ -19,14 +19,17 @@
 from lp.services.features.testing import FeatureFixture
 from lp.services.librarian.client import LibrarianClient
 from lp.services.librarian.model import LibraryFileAlias
+from lp.services.librarianserver import swift
 from lp.services.librarianserver.storage import LibrarianStorage
 from lp.services.log.logger import BufferLogger
 from lp.testing import TestCase
-from lp.testing.layers import BaseLayer, LaunchpadZopelessLayer, LibrarianLayer
+from lp.testing.layers import (
+    BaseLayer,
+    LaunchpadZopelessLayer,
+    LibrarianLayer,
+    )
 from lp.testing.swift.fixture import SwiftFixture
 
-from lp.services.librarianserver import swift
-
 
 class TestFeedSwift(TestCase):
     layer = LaunchpadZopelessLayer
@@ -272,8 +275,8 @@
         _, obj2 = swift_client.get_object(container, '{0}/0001'.format(name))
         _, obj3 = swift_client.get_object(container, '{0}/0002'.format(name))
         self.assertRaises(
-            swiftclient.ClientException, swift_client.get_object,
-            container, '{0}/0003'.format(name))
+            swiftclient.ClientException, swift.quiet_swiftclient,
+            swift_client.get_object, container, '{0}/0003'.format(name))
 
         # Our object round tripped
         self.assertEqual(obj1 + obj2 + obj3, expected_content)
@@ -293,7 +296,8 @@
 
     def test_partial_read(self):
         empty_sha1 = 'da39a3ee5e6b4b0d3255bfef95601890afd80709'
-        s = swift.HashStream(StringIO('make me another coffee'), hashlib.sha1)
+        s = swift.HashStream(
+            StringIO('make me another coffee'), hash_factory=hashlib.sha1)
         self.assertEqual(s.hash.hexdigest(), empty_sha1)
         chunk = s.read(4)
         self.assertEqual(chunk, 'make')
@@ -304,6 +308,21 @@
         self.assertEqual(s.hash.hexdigest(),
                          '8c826e573016ce05f3968044f82507b46fd2aa93')
 
+    def test_limited_length(self):
+        base_stream = StringIO('make me a coffee')
+        s = swift.HashStream(base_stream, length=8)
+        chunk = s.read(4)
+        self.assertEqual(chunk, 'make')
+        self.assertEqual(s.hash.hexdigest(),
+                         '099dafc678df7d266c25f95ccf6cde22')
+        chunk = s.read(8)
+        self.assertEqual(chunk, ' me ')
+        self.assertEqual(s.hash.hexdigest(),
+                         '10a0334e435b75f35b1923842bd87f81')
+        self.assertEqual(s.read(), '')
+        self.assertEqual(s.tell(), 8)
+        self.assertEqual(base_stream.tell(), 8)
+
     def test_tell(self):
         s = swift.HashStream(StringIO('hurry up with that coffee'))
         self.assertEqual(s.tell(), 0)

=== modified file 'lib/lp/testing/swift/fakeswift.py'
--- lib/lp/testing/swift/fakeswift.py	2017-12-07 12:05:13 +0000
+++ lib/lp/testing/swift/fakeswift.py	2018-02-13 16:28:38 +0000
@@ -557,17 +557,33 @@
         """Compute service catalog for the given request and tenant."""
         port = request.transport.socket.getsockname()[1]
         tenant_id = self.tenants[tenant]["id"]
-        base_url = "http://%s:%d/swift/v1" % (self.hostname, port)
+        base_url = "http://%s:%d" % (self.hostname, port)
+        keystone_base_url = "%s/keystone/v2.0" % base_url
+        swift_base_url = "%s/swift/v1" % base_url
         catalog = [
-            {"endpoints": [
-                {"adminURL": base_url,
-                 "id": uuid.uuid4().hex,
-                 "internalURL": base_url + "/AUTH_" + tenant_id,
-                 "publicURL": base_url + "/AUTH_" + tenant_id,
-                 "region": DEFAULT_REGION}
-                ],
-             "endpoints_links": [],
-             "name": "swift",
-             "type": "object-store"
-             }]
+            {
+                "endpoints": [{
+                    "adminURL": keystone_base_url,
+                    "id": uuid.uuid4().hex,
+                    "internalURL": keystone_base_url,
+                    "publicURL": keystone_base_url,
+                    "region": DEFAULT_REGION,
+                    }],
+                "endpoints_links": [],
+                "name": "keystone",
+                "type": "identity",
+                },
+            {
+                "endpoints": [{
+                    "adminURL": swift_base_url,
+                    "id": uuid.uuid4().hex,
+                    "internalURL": swift_base_url + "/AUTH_" + tenant_id,
+                    "publicURL": swift_base_url + "/AUTH_" + tenant_id,
+                    "region": DEFAULT_REGION,
+                    }],
+                "endpoints_links": [],
+                "name": "swift",
+                "type": "object-store",
+                },
+            ]
         return catalog

=== modified file 'lib/lp/testing/swift/tests/test_fixture.py'
--- lib/lp/testing/swift/tests/test_fixture.py	2018-01-02 16:10:26 +0000
+++ lib/lp/testing/swift/tests/test_fixture.py	2018-02-13 16:28:38 +0000
@@ -8,9 +8,10 @@
 
 from datetime import datetime
 from hashlib import md5
-import httplib
 
+from requests.exceptions import ConnectionError
 from swiftclient import client as swiftclient
+from swiftclient.exceptions import ClientException
 from testtools.matchers import (
     GreaterThan,
     LessThan,
@@ -200,14 +201,14 @@
         # authenticated.
         self.swift_fixture.shutdown()
         self.assertRaises(
-            httplib.HTTPException,
+            ConnectionError,
             client.get_object, "size", str(size))
 
         # And even if we bring it back up, existing connections
         # continue to fail
         self.swift_fixture.startup()
         self.assertRaises(
-            httplib.HTTPException,
+            ClientException,
             client.get_object, "size", str(size))
 
         # But fresh connections are fine.

=== modified file 'lib/lp_sitecustomize.py'
--- lib/lp_sitecustomize.py	2017-12-19 12:32:57 +0000
+++ lib/lp_sitecustomize.py	2018-02-13 16:28:38 +0000
@@ -91,9 +91,13 @@
     Logger, which is unfortunate as we disable them in the handler. Not
     only does swiftclient then emit lots of noise, but it also turns
     keystoneclient debugging on.
+
+    keystoneclient logs credentials at DEBUG.
     """
     swiftclient_logger = logging.getLogger('swiftclient')
     swiftclient_logger.setLevel(logging.INFO)
+    keystoneclient_logger = logging.getLogger('keystoneclient')
+    keystoneclient_logger.setLevel(logging.INFO)
 
 
 def silence_zcml_logger():