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

diff --git a/cinder/backup/chunkeddriver.py b/cinder/backup/chunkeddriver.py
index c4858dbe33a40472ebd0e595ba27036cee0513fa..c2711b3eec7ec3374c0f660ece2db38169aa6df1 100644 (file)
--- 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 178823e6d2a1f93a79f4da9f49bba078888c0d71..08b4505ba8d25e41b3493528887e14a407fd477d 100644 (file)
--- 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 ce27fbeb8bd97996eac871903aa58878ee6b1b84..fe0ca965612d8c1f9b95f5f08ee79e7f3dda904e 100644 (file)
--- 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 e385f6b4c97871bcf93b494cf34dd02a665e212c..dfc244afdb599d5e30adc618ff8d633820e5081c 100644 (file)
--- 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 02673fee25ac44a89609a556be66a77e69edb21e..3f2b3e300b814a238e9545cfaeaad12ba8fd0ca0 100644 (file)
--- 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 5501e4d22fb325873aad4d04a874541371500414..f5f46c235910f5622ddd5cf3f42e03be0c79e576 100644 (file)
--- 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 \