From 72b7a18583767b9f8449345312ad2ecddd93fc00 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 23 Jun 2015 13:37:42 +0200 Subject: [PATCH] Fix Python 3 issues in the swift backup driver * On Python 3, encode/decode JSON to/from UTF-8 * Use byte strings for volume content * Replace dict.keys()[0] and dict.value()[0] with list(dict.items())[0]. Get the key and the value at once to ensure that they are consistent. On Python 3, items() returns an iterator: create a list to use the [0] operator. * SwiftBackupDriver: use a bytearray instead of a string because bytes += bytes is inefficient on Python 3, whereas str += str is optimized on Python 2. Replace also StringIO with BytesIO. * BytesIO has no more len attribute on Python 3: get the length of the content instead. * Replace buffer(bytearray(128)) with b'\0' * 128 to create a string of 128 zeroed bytes * Replace "rw" file mode with "w+" ("rw" raises an exception on Python 3). * tox.ini: add the following tests to Python 3.4 - cinder.tests.unit.test_backup_swift - cinder.tests.unit.test_backup_tsm Blueprint cinder-python3 Change-Id: I62d7ef8041847f31b5d06a92fa2edb65c6780497 --- cinder/backup/chunkeddriver.py | 19 ++++++++++++++----- cinder/backup/drivers/swift.py | 6 +++--- cinder/tests/unit/backup/fake_swift_client.py | 3 +++ cinder/tests/unit/test_backup_swift.py | 6 +++--- cinder/tests/unit/test_backup_tsm.py | 6 +++--- tox.ini | 2 ++ 6 files changed, 28 insertions(+), 14 deletions(-) diff --git a/cinder/backup/chunkeddriver.py b/cinder/backup/chunkeddriver.py index c4858dbe3..c2711b3ee 100644 --- a/cinder/backup/chunkeddriver.py +++ b/cinder/backup/chunkeddriver.py @@ -198,6 +198,8 @@ class ChunkedBackupDriver(driver.BackupDriver): if extra_metadata: metadata['extra_metadata'] = extra_metadata metadata_json = json.dumps(metadata, sort_keys=True, indent=2) + if six.PY3: + metadata_json = metadata_json.encode('utf-8') with self.get_object_writer(container, filename) as writer: writer.write(metadata_json) LOG.debug('_write_metadata finished. Metadata: %s.', metadata_json) @@ -217,6 +219,8 @@ class ChunkedBackupDriver(driver.BackupDriver): sha256file['chunk_size'] = self.sha_block_size_bytes sha256file['sha256s'] = sha256_list sha256file_json = json.dumps(sha256file, sort_keys=True, indent=2) + if six.PY3: + sha256file_json = sha256file_json.encode('utf-8') with self.get_object_writer(container, filename) as writer: writer.write(sha256file_json) LOG.debug('_write_sha256file finished.') @@ -229,6 +233,8 @@ class ChunkedBackupDriver(driver.BackupDriver): {'container': container, 'filename': filename}) with self.get_object_reader(container, filename) as reader: metadata_json = reader.read() + if six.PY3: + metadata_json = metadata_json.decode('utf-8') metadata = json.loads(metadata_json) LOG.debug('_read_metadata finished. Metadata: %s.', metadata_json) return metadata @@ -241,6 +247,8 @@ class ChunkedBackupDriver(driver.BackupDriver): {'container': container, 'filename': filename}) with self.get_object_reader(container, filename) as reader: sha256file_json = reader.read() + if six.PY3: + sha256file_json = sha256file_json.decode('utf-8') sha256file = json.loads(sha256file_json) LOG.debug('_read_sha256file finished (%s).', sha256file) return sha256file @@ -452,7 +460,7 @@ class ChunkedBackupDriver(driver.BackupDriver): while True: data_offset = volume_file.tell() data = volume_file.read(self.chunk_size_bytes) - if data == '': + if data == b'': break # Calculate new shas with the datablock. @@ -543,8 +551,9 @@ class ChunkedBackupDriver(driver.BackupDriver): extra_metadata = metadata.get('extra_metadata') container = backup['container'] metadata_objects = metadata['objects'] - metadata_object_names = sum((obj.keys() for obj in metadata_objects), - []) + metadata_object_names = [] + for obj in metadata_objects: + metadata_object_names.extend(obj.keys()) LOG.debug('metadata_object_names = %s.', metadata_object_names) prune_list = [self._metadata_filename(backup), self._sha256_filename(backup)] @@ -557,7 +566,7 @@ class ChunkedBackupDriver(driver.BackupDriver): raise exception.InvalidBackup(reason=err) for metadata_object in metadata_objects: - object_name = metadata_object.keys()[0] + object_name, obj = list(metadata_object.items())[0] LOG.debug('restoring object. backup: %(backup_id)s, ' 'container: %(container)s, object name: ' '%(object_name)s, volume: %(volume_id)s.', @@ -574,7 +583,7 @@ class ChunkedBackupDriver(driver.BackupDriver): body = reader.read() compression_algorithm = metadata_object[object_name]['compression'] decompressor = self._get_compressor(compression_algorithm) - volume_file.seek(metadata_object.values()[0]['offset']) + volume_file.seek(obj['offset']) if decompressor is not None: LOG.debug('decompressing data using %s algorithm', compression_algorithm) diff --git a/cinder/backup/drivers/swift.py b/cinder/backup/drivers/swift.py index 178823e6d..08b4505ba 100644 --- a/cinder/backup/drivers/swift.py +++ b/cinder/backup/drivers/swift.py @@ -180,7 +180,7 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): self.container = container self.object_name = object_name self.conn = conn - self.data = '' + self.data = bytearray() def __enter__(self): return self @@ -192,11 +192,11 @@ class SwiftBackupDriver(chunkeddriver.ChunkedBackupDriver): self.data += data def close(self): - reader = six.StringIO(self.data) + reader = six.BytesIO(self.data) try: etag = self.conn.put_object(self.container, self.object_name, reader, - content_length=reader.len) + content_length=len(self.data)) except socket.error as err: raise exception.SwiftConnectionFailed(reason=err) LOG.debug('swift MD5 for %(object_name)s: %(etag)s', diff --git a/cinder/tests/unit/backup/fake_swift_client.py b/cinder/tests/unit/backup/fake_swift_client.py index ce27fbeb8..fe0ca9656 100644 --- a/cinder/tests/unit/backup/fake_swift_client.py +++ b/cinder/tests/unit/backup/fake_swift_client.py @@ -19,6 +19,7 @@ import socket import zlib from oslo_log import log as logging +import six from six.moves import http_client from swiftclient import client as swift @@ -94,6 +95,8 @@ class FakeSwiftConnection(object): 'offset': 20} }] metadata_json = json.dumps(metadata, sort_keys=True, indent=2) + if six.PY3: + metadata_json = metadata_json.encode('utf-8') fake_object_body = metadata_json return (fake_object_header, fake_object_body) diff --git a/cinder/tests/unit/test_backup_swift.py b/cinder/tests/unit/test_backup_swift.py index e385f6b4c..dfc244afd 100644 --- a/cinder/tests/unit/test_backup_swift.py +++ b/cinder/tests/unit/test_backup_swift.py @@ -569,7 +569,7 @@ class BackupSwiftTestCase(test.TestCase): def test_prepare_output_data_effective_compression(self): service = swift_dr.SwiftBackupDriver(self.ctxt) # Set up buffer of 128 zeroed bytes - fake_data = buffer(bytearray(128)) + fake_data = b'\0' * 128 result = service._prepare_output_data(fake_data) @@ -580,7 +580,7 @@ class BackupSwiftTestCase(test.TestCase): self.flags(backup_compression_algorithm='none') service = swift_dr.SwiftBackupDriver(self.ctxt) # Set up buffer of 128 zeroed bytes - fake_data = buffer(bytearray(128)) + fake_data = b'\0' * 128 result = service._prepare_output_data(fake_data) @@ -590,7 +590,7 @@ class BackupSwiftTestCase(test.TestCase): def test_prepare_output_data_ineffective_compression(self): service = swift_dr.SwiftBackupDriver(self.ctxt) # Set up buffer of 128 zeroed bytes - fake_data = buffer(bytearray(128)) + fake_data = b'\0' * 128 # Pre-compress so that compression in the driver will be ineffective. already_compressed_data = service.compressor.compress(fake_data) diff --git a/cinder/tests/unit/test_backup_tsm.py b/cinder/tests/unit/test_backup_tsm.py index 02673fee2..3f2b3e300 100644 --- a/cinder/tests/unit/test_backup_tsm.py +++ b/cinder/tests/unit/test_backup_tsm.py @@ -279,7 +279,7 @@ class BackupTSMTestCase(test.TestCase): self._create_backup_db_entry(backup_id2, mode) self._create_backup_db_entry(backup_id3, mode) - with open(VOLUME_PATH, 'rw') as volume_file: + with open(VOLUME_PATH, 'w+') as volume_file: # Create two backups of the volume backup1 = objects.Backup.get_by_id(self.ctxt, backup_id1) self.driver.backup(backup1, volume_file) @@ -311,7 +311,7 @@ class BackupTSMTestCase(test.TestCase): self._create_backup_db_entry(backup_id1, mode) self._create_backup_db_entry(backup_id2, mode) - with open(VOLUME_PATH, 'rw') as volume_file: + with open(VOLUME_PATH, 'w+') as volume_file: # Create two backups of the volume backup1 = objects.Backup.get_by_id(self.ctxt, 123) self.driver.backup(backup1, volume_file) @@ -342,7 +342,7 @@ class BackupTSMTestCase(test.TestCase): backup_id1 = 123 self._create_backup_db_entry(backup_id1, mode) - with open(VOLUME_PATH, 'rw') as volume_file: + with open(VOLUME_PATH, 'w+') as volume_file: # Create two backups of the volume backup1 = objects.Backup.get_by_id(self.ctxt, 123) self.assertRaises(exception.InvalidBackup, diff --git a/tox.ini b/tox.ini index 5501e4d22..f5f46c235 100644 --- a/tox.ini +++ b/tox.ini @@ -33,6 +33,8 @@ commands = cinder.tests.unit.test_backup \ cinder.tests.unit.test_backup_ceph \ cinder.tests.unit.test_backup_driver_base \ + cinder.tests.unit.test_backup_swift \ + cinder.tests.unit.test_backup_tsm \ cinder.tests.unit.test_block_device \ cinder.tests.unit.test_cloudbyte \ cinder.tests.unit.test_conf \ -- 2.45.2