Port test_volume to Python 3

* don't use hasttr() to check if a lazy SQLAlchemy is loaded or not: use
  the obj_attr_is_set() method instead. On Python 3, hasattr() pass
  through SQLAlchemy exceptions which is unexpected.
* Replace a/b with a//b to use integer division, not float division.
* Replace filter() with list-comprehension using an if.
* Replace file() with open() and open /dev/null in binary mode (not text
  mode, so Unicode on Python 3).
* test_volume require "import cinder.volume.targets.tgt" on Python 3
* Use a key function to sort a list of dictionaries. Dictionaries are no
  more comparable on Python 3.
* tox.ini: add the following tests to Python 3.4.

  - cinder.tests.unit.keymgr.test_mock_key_mgr
  - cinder.tests.unit.test_volume

Note: test_list_availability_zones_enabled_service() of test_volume
was broken, the test checked that the result of .sort() is None...

Blueprint cinder-python3
Change-Id: If1a26acc0138db9bda7fde1cb1f40093d9b3c494

diff --git a/cinder/tests/unit/keymgr/mock_key_mgr.py b/cinder/tests/unit/keymgr/mock_key_mgr.py
index 8bae75fe554215ca63050da7eddfbc0d083fa4f5..39f2caa6449956adb8291f87b96bb17098dadd7f 100644 (file)
--- a/cinder/tests/unit/keymgr/mock_key_mgr.py
+++ b/cinder/tests/unit/keymgr/mock_key_mgr.py
@@ -27,6 +27,7 @@ this class.
 """
 
 import array
+import binascii
 import uuid
 
 from cinder import exception
@@ -54,14 +55,14 @@ class MockKeyManager(key_mgr.KeyManager):
     def _generate_hex_key(self, **kwargs):
         key_length = kwargs.get('key_length', 256)
         # hex digit => 4 bits
-        hex_encoded = utils.generate_password(length=key_length / 4,
+        hex_encoded = utils.generate_password(length=key_length // 4,
                                               symbolgroups='0123456789ABCDEF')
         return hex_encoded
 
     def _generate_key(self, **kwargs):
         _hex = self._generate_hex_key(**kwargs)
-        return key.SymmetricKey('AES',
-                                array.array('B', _hex.decode('hex')).tolist())
+        key_bytes = array.array('B', binascii.unhexlify(_hex)).tolist()
+        return key.SymmetricKey('AES', key_bytes)
 
     def create_key(self, ctxt, **kwargs):
         """Creates a key.

diff --git a/cinder/tests/unit/keymgr/test_mock_key_mgr.py b/cinder/tests/unit/keymgr/test_mock_key_mgr.py
index 7f2cd9e6686bb13bab6d5895772805fe648c858d..dfcd8ba153bfb625f470a632891ae5cfbdc4b471 100644 (file)
--- a/cinder/tests/unit/keymgr/test_mock_key_mgr.py
+++ b/cinder/tests/unit/keymgr/test_mock_key_mgr.py
@@ -46,14 +46,14 @@ class MockKeyManagerTestCase(test_key_mgr.KeyManagerTestCase):
         for length in [64, 128, 256]:
             key_id = self.key_mgr.create_key(self.ctxt, key_length=length)
             key = self.key_mgr.get_key(self.ctxt, key_id)
-            self.assertEqual(length / 8, len(key.get_encoded()))
+            self.assertEqual(length // 8, len(key.get_encoded()))
 
     def test_create_null_context(self):
         self.assertRaises(exception.NotAuthorized,
                           self.key_mgr.create_key, None)
 
     def test_store_key(self):
-        secret_key = array.array('B', ('0' * 64).decode('hex')).tolist()
+        secret_key = array.array('B', b'\x00' * 32).tolist()
         _key = keymgr_key.SymmetricKey('AES', secret_key)
         key_id = self.key_mgr.store_key(self.ctxt, _key)
 

diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py
index 227b1b530423657fcafa747a78a8f8d3a8c59142..d6f3c71d23e86114ca79e0d7086a554f83bfae5e 100644 (file)
--- a/cinder/tests/unit/test_volume.py
+++ b/cinder/tests/unit/test_volume.py
@@ -64,6 +64,7 @@ from cinder.volume import driver
 from cinder.volume.drivers import lvm
 from cinder.volume import manager as vol_manager
 from cinder.volume import rpcapi as volume_rpcapi
+import cinder.volume.targets.tgt
 from cinder.volume import utils as volutils
 from cinder.volume import volume_types
 
@@ -239,15 +240,18 @@ class AvailabilityZoneTestCase(BaseVolumeTestCase):
         self.stubs.Set(db, 'service_get_all_by_topic',
                        stub_service_get_all_by_topic)
 
+        def sort_func(obj):
+            return obj['name']
+
         volume_api = cinder.volume.api.API()
         azs = volume_api.list_availability_zones()
-        azs = list(azs).sort()
+        azs = sorted(azs, key=sort_func)
 
-        expected = [
+        expected = sorted([
             {'name': 'pung', 'available': False},
             {'name': 'pong', 'available': True},
             {'name': 'ping', 'available': True},
-        ].sort()
+        ], key=sort_func)
 
         self.assertEqual(expected, azs)
 
@@ -4165,6 +4169,9 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.volume.create_volume(self.context, volume['id'])
         volume['status'] = 'in-use'
 
+        def sort_func(obj):
+            return obj['name']
+
         volume_api = cinder.volume.api.API()
 
         # Update fails when status != available
@@ -6026,7 +6033,7 @@ class GenericVolumeDriverTestCase(DriverTestCase):
 
         mock_volume_get.return_value = vol
         mock_get_connector_properties.return_value = properties
-        f = mock_file_open.return_value = file('/dev/null')
+        f = mock_file_open.return_value = open('/dev/null', 'rb')
 
         backup_service.backup(backup_obj, f, None)
         self.volume.driver._attach_volume.return_value = attach_info, vol
@@ -6067,7 +6074,7 @@ class GenericVolumeDriverTestCase(DriverTestCase):
         mock_volume_get.return_value = vol
         self.volume.driver._create_temp_cloned_volume.return_value = temp_vol
         mock_get_connector_properties.return_value = properties
-        f = mock_file_open.return_value = file('/dev/null')
+        f = mock_file_open.return_value = open('/dev/null', 'rb')
 
         backup_service.backup(backup_obj, f, None)
         self.volume.driver._attach_volume.return_value = attach_info, vol
@@ -6125,7 +6132,7 @@ class GenericVolumeDriverTestCase(DriverTestCase):
         mock_volume_get.return_value = vol
         mock_connect_volume.return_value = {'type': 'local',
                                             'path': '/dev/null'}
-        f = mock_file_open.return_value = file('/dev/null')
+        f = mock_file_open.return_value = open('/dev/null', 'rb')
 
         self.volume.driver._connect_device
         backup_service.backup(backup_obj, f, None)
@@ -6924,7 +6931,7 @@ class LVMVolumeDriverTestCase(DriverTestCase):
 
         mock_volume_get.return_value = vol
         mock_get_connector_properties.return_value = properties
-        f = mock_file_open.return_value = file('/dev/null')
+        f = mock_file_open.return_value = open('/dev/null', 'rb')
 
         backup_service.backup(backup_obj, f, None)
         self.volume.driver._attach_volume.return_value = attach_info
@@ -7003,7 +7010,7 @@ class LVMVolumeDriverTestCase(DriverTestCase):
         self.volume.driver._delete_temp_snapshot = mock.MagicMock()
 
         mock_get_connector_properties.return_value = properties
-        f = mock_file_open.return_value = file('/dev/null')
+        f = mock_file_open.return_value = open('/dev/null', 'rb')
 
         backup_service.backup(backup_obj, f, None)
         self.volume.driver._attach_volume.return_value = attach_info

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 5db3a7b5303c14d475d94da3edd2dd056ff84f2a..f7c3d809a4290c696c1fe84a9c1dacd221f50774 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -798,7 +798,7 @@ class API(base.Base):
         except Exception:
             with excutils.save_and_reraise_exception():
                 try:
-                    if hasattr(snapshot, 'id'):
+                    if snapshot.obj_attr_is_set('id'):
                         snapshot.destroy()
                 finally:
                     QUOTAS.rollback(context, reservations)

diff --git a/cinder/volume/flows/api/create_volume.py b/cinder/volume/flows/api/create_volume.py
index 118c8b484d0484c510d799cbf5fe298f205045b1..6156a225b0f995e9c1085720f89db581430a1fe5 100644 (file)
--- a/cinder/volume/flows/api/create_volume.py
+++ b/cinder/volume/flows/api/create_volume.py
@@ -207,7 +207,7 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask):
 
         # Check image size is not larger than volume size.
         image_size = utils.as_int(image_meta['size'], quiet=False)
-        image_size_in_gb = (image_size + GB - 1) / GB
+        image_size_in_gb = (image_size + GB - 1) // GB
         if image_size_in_gb > size:
             msg = _('Size of specified image %(image_size)sGB'
                     ' is larger than volume size %(volume_size)sGB.')

diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py
index 7a7d0cc189711f5e6efc1910b2225872a9986d2c..cfeefd291026dd9d32cf6b03469aae5517ce8a25 100644 (file)
--- a/cinder/volume/manager.py
+++ b/cinder/volume/manager.py
@@ -2496,8 +2496,8 @@ class VolumeManager(manager.SchedulerDependentManager):
 
         sorted_snapshots = []
         for vol in volumes:
-            found_snaps = filter(
-                lambda snap: snap['id'] == vol['snapshot_id'], snapshots)
+            found_snaps = [snap for snap in snapshots
+                           if snap['id'] == vol['snapshot_id']]
             if not found_snaps:
                 LOG.error(_LE("Source snapshot cannot be found for target "
                               "volume %(volume_id)s."),
@@ -2519,9 +2519,8 @@ class VolumeManager(manager.SchedulerDependentManager):
 
         sorted_source_vols = []
         for vol in volumes:
-            found_source_vols = filter(
-                lambda source_vol: source_vol['id'] == vol['source_volid'],
-                source_vols)
+            found_source_vols = [source_vol for source_vol in source_vols
+                                 if source_vol['id'] == vol['source_volid']]
             if not found_source_vols:
                 LOG.error(_LE("Source volumes cannot be found for target "
                               "volume %(volume_id)s."),

diff --git a/tox.ini b/tox.ini
index 1eb5fc02ad3f44c4c112b0f1ef4d68d5fd22bea7..27a9220468fb0b4655181573baae82213140eed3 100644 (file)
--- a/tox.ini
+++ b/tox.ini
@@ -30,6 +30,7 @@ downloadcache = ~/cache/pip
 commands =
   python -m testtools.run \
     cinder.tests.unit.image.test_glance \
+    cinder.tests.unit.keymgr.test_mock_key_mgr \
     cinder.tests.unit.targets.test_base_iscsi_driver \
     cinder.tests.unit.targets.test_cxt_driver \
     cinder.tests.unit.targets.test_iser_driver \
@@ -97,6 +98,7 @@ commands =
     cinder.tests.unit.test_v6000_common \
     cinder.tests.unit.test_vmware_vmdk \
     cinder.tests.unit.test_vmware_volumeops \
+    cinder.tests.unit.test_volume \
     cinder.tests.unit.test_volume_configuration \
     cinder.tests.unit.test_volume_glance_metadata \
     cinder.tests.unit.test_volume_rpcapi \