Fix Python 3 issues in ceph and rbd drivers

* Add cinder.utils.convert_str() function: helper to convert a string to
  a native string: convert to bytes on Python 2, convert to Unicode on
  Python 3. Function needed by ceph and rbd drivers.
* Replace encodeutils.safe_encode() with utils.convert_str()
* test_backup_ceph: get the size of the volume file using os.fstat() to
  fix a test. Before the volume size was a mock, but comparison between
  integers and mocks raise a TypeError on Python 3.
* Fix RBDDriver._update_volume_stats(): replace a/b with a//b to get an integer
  on Python 2 and Python 3 (a/b always returns a float on Python 3)
* Replace map() with a list-comprehension in RBDDriver because a list is
  expected. On Python 3, map() returns an iterator.
* tox.ini: add to following tests to Python 3.4

  - cinder.tests.unit.test_backup_ceph
  - cinder.tests.unit.test_rbd

Blueprint cinder-python3
Change-Id: I7acb0d9eed0d351798133e0c6ef55408fed925dd

diff --git a/cinder/backup/drivers/ceph.py b/cinder/backup/drivers/ceph.py
index 207e09977b486138ad5ad51d92e6815538233ed2..089a5add79570f6df5987921a8c63b664033eec6 100644 (file)
--- a/cinder/backup/drivers/ceph.py
+++ b/cinder/backup/drivers/ceph.py
@@ -51,7 +51,6 @@ import time
 import eventlet
 from oslo_config import cfg
 from oslo_log import log as logging
-from oslo_utils import encodeutils
 from oslo_utils import excutils
 from oslo_utils import units
 from six.moves import range
@@ -104,7 +103,7 @@ class VolumeMetadataBackup(object):
 
     @property
     def name(self):
-        return encodeutils.safe_encode("backup.%s.meta" % self._backup_id)
+        return utils.convert_str("backup.%s.meta" % self._backup_id)
 
     @property
     def exists(self):
@@ -181,9 +180,9 @@ class CephBackupDriver(driver.BackupDriver):
             self.rbd_stripe_count = 0
             self.rbd_stripe_unit = 0
 
-        self._ceph_backup_user = encodeutils.safe_encode(CONF.backup_ceph_user)
-        self._ceph_backup_pool = encodeutils.safe_encode(CONF.backup_ceph_pool)
-        self._ceph_backup_conf = encodeutils.safe_encode(CONF.backup_ceph_conf)
+        self._ceph_backup_user = utils.convert_str(CONF.backup_ceph_user)
+        self._ceph_backup_pool = utils.convert_str(CONF.backup_ceph_pool)
+        self._ceph_backup_conf = utils.convert_str(CONF.backup_ceph_conf)
 
     def _validate_string_args(self, *args):
         """Ensure all args are non-None and non-empty."""
@@ -239,8 +238,7 @@ class CephBackupDriver(driver.BackupDriver):
                                   conffile=self._ceph_backup_conf)
         try:
             client.connect()
-            pool_to_open = encodeutils.safe_encode(pool or
-                                                   self._ceph_backup_pool)
+            pool_to_open = utils.convert_str(pool or self._ceph_backup_pool)
             ioctx = client.open_ioctx(pool_to_open)
             return client, ioctx
         except self.rados.Error:
@@ -263,13 +261,13 @@ class CephBackupDriver(driver.BackupDriver):
         """
         # Ensure no unicode
         if diff_format:
-            return encodeutils.safe_encode("volume-%s.backup.base" % volume_id)
+            return utils.convert_str("volume-%s.backup.base" % volume_id)
         else:
             if backup_id is None:
                 msg = _("Backup id required")
                 raise exception.InvalidParameterValue(msg)
-            return encodeutils.safe_encode("volume-%s.backup.%s" %
-                                           (volume_id, backup_id))
+            return utils.convert_str("volume-%s.backup.%s"
+                                     % (volume_id, backup_id))
 
     def _discard_bytes(self, volume, offset, length):
         """Trim length bytes from offset.
@@ -473,7 +471,7 @@ class CephBackupDriver(driver.BackupDriver):
 
             # Since we have deleted the base image we can delete the source
             # volume backup snapshot.
-            src_name = encodeutils.safe_encode(volume_id)
+            src_name = utils.convert_str(volume_id)
             if src_name in self.rbd.RBD().list(client.ioctx):
                 LOG.debug("Deleting source volume snapshot '%(snapshot)s' "
                           "for backup %(basename)s.",
@@ -538,15 +536,14 @@ class CephBackupDriver(driver.BackupDriver):
         if from_snap is not None:
             cmd1.extend(['--from-snap', from_snap])
         if src_snap:
-            path = encodeutils.safe_encode("%s/%s@%s" %
-                                           (src_pool, src_name, src_snap))
+            path = utils.convert_str("%s/%s@%s"
+                                     % (src_pool, src_name, src_snap))
         else:
-            path = encodeutils.safe_encode("%s/%s" % (src_pool, src_name))
-
+            path = utils.convert_str("%s/%s" % (src_pool, src_name))
         cmd1.extend([path, '-'])
 
         cmd2 = ['rbd', 'import-diff'] + dest_ceph_args
-        rbd_path = encodeutils.safe_encode("%s/%s" % (dest_pool, dest_name))
+        rbd_path = utils.convert_str("%s/%s" % (dest_pool, dest_name))
         cmd2.extend(['-', rbd_path])
 
         ret, stderr = self._piped_execute(cmd1, cmd2)
@@ -757,8 +754,8 @@ class CephBackupDriver(driver.BackupDriver):
         return backup_snaps
 
     def _get_new_snap_name(self, backup_id):
-        return encodeutils.safe_encode("backup.%s.snap.%s" %
-                                       (backup_id, time.time()))
+        return utils.convert_str("backup.%s.snap.%s"
+                                 % (backup_id, time.time()))
 
     def _get_backup_snap_name(self, rbd_image, name, backup_id):
         """Return the name of the snapshot associated with backup_id.
@@ -932,7 +929,7 @@ class CephBackupDriver(driver.BackupDriver):
         with rbd_driver.RADOSClient(self, self._ceph_backup_pool) as client:
             adjust_size = 0
             base_image = self.rbd.Image(client.ioctx,
-                                        encodeutils.safe_encode(backup_base),
+                                        utils.convert_str(backup_base),
                                         read_only=True)
             try:
                 if restore_length != base_image.size():
@@ -942,7 +939,7 @@ class CephBackupDriver(driver.BackupDriver):
 
         if adjust_size:
             with rbd_driver.RADOSClient(self, src_pool) as client:
-                restore_vol_encode = encodeutils.safe_encode(restore_vol)
+                restore_vol_encode = utils.convert_str(restore_vol)
                 dest_image = self.rbd.Image(client.ioctx, restore_vol_encode)
                 try:
                     LOG.debug("Adjusting restore vol size")

diff --git a/cinder/tests/unit/test_backup_ceph.py b/cinder/tests/unit/test_backup_ceph.py
index c497df22de06e8cf1a90240ed91511c325264be8..4ae3c64db1c553d172c1c165095ba6083b94003b 100644 (file)
--- a/cinder/tests/unit/test_backup_ceph.py
+++ b/cinder/tests/unit/test_backup_ceph.py
@@ -307,6 +307,7 @@ class BackupCephTestCase(test.TestCase):
 
         rbd1 = mock.Mock()
         rbd1.read.side_effect = fake_read
+        rbd1.size.return_value = os.fstat(self.volume_file.fileno()).st_size
 
         rbd2 = mock.Mock()
         rbd2.write.side_effect = mock_write_data

diff --git a/cinder/utils.py b/cinder/utils.py
index b204a62e882a1753752d7476d52c60386acb536f..e2b1082ec5448935f2ffa502174ca09d664de9f4 100644 (file)
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -40,6 +40,7 @@ from oslo_concurrency import lockutils
 from oslo_concurrency import processutils
 from oslo_config import cfg
 from oslo_log import log as logging
+from oslo_utils import encodeutils
 from oslo_utils import importutils
 from oslo_utils import timeutils
 import retrying
@@ -816,3 +817,21 @@ def convert_version_to_str(version_int):
 
 def convert_version_to_tuple(version_str):
     return tuple(int(part) for part in version_str.split('.'))
+
+
+def convert_str(text):
+    """Convert to native string.
+
+    Convert bytes and Unicode strings to native strings:
+
+    * convert to bytes on Python 2:
+      encode Unicode using encodeutils.safe_encode()
+    * convert to Unicode on Python 3: decode bytes from UTF-8
+    """
+    if six.PY2:
+        return encodeutils.safe_encode(text)
+    else:
+        if isinstance(text, bytes):
+            return text.decode('utf-8')
+        else:
+            return text

diff --git a/cinder/volume/drivers/rbd.py b/cinder/volume/drivers/rbd.py
index ed665751b09956e907be8047dcf75ecc599edd37..80683f16ab6c32ec6063d3a3e2e59a72a1b59a79 100644 (file)
--- a/cinder/volume/drivers/rbd.py
+++ b/cinder/volume/drivers/rbd.py
@@ -23,7 +23,6 @@ import tempfile
 from eventlet import tpool
 from oslo_config import cfg
 from oslo_log import log as logging
-from oslo_utils import encodeutils
 from oslo_utils import units
 import six
 from six.moves import urllib
@@ -101,9 +100,9 @@ class RBDImageMetadata(object):
     """RBD image metadata to be used with RBDImageIOWrapper."""
     def __init__(self, image, pool, user, conf):
         self.image = image
-        self.pool = encodeutils.safe_encode(pool)
-        self.user = encodeutils.safe_encode(user)
-        self.conf = encodeutils.safe_encode(conf)
+        self.pool = utils.convert_str(pool)
+        self.user = utils.convert_str(user)
+        self.conf = utils.convert_str(conf)
 
 
 class RBDImageIOWrapper(io.RawIOBase):
@@ -218,11 +217,11 @@ class RBDVolumeProxy(object):
                  read_only=False):
         client, ioctx = driver._connect_to_rados(pool)
         if snapshot is not None:
-            snapshot = encodeutils.safe_encode(snapshot)
+            snapshot = utils.convert_str(snapshot)
 
         try:
             self.volume = driver.rbd.Image(ioctx,
-                                           encodeutils.safe_encode(name),
+                                           utils.convert_str(name),
                                            snapshot=snapshot,
                                            read_only=read_only)
         except driver.rbd.Error:
@@ -287,7 +286,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
                      'rbd_ceph_conf', 'rbd_pool']:
             val = getattr(self.configuration, attr)
             if val is not None:
-                setattr(self.configuration, attr, encodeutils.safe_encode(val))
+                setattr(self.configuration, attr, utils.convert_str(val))
 
     def check_for_setup_error(self):
         """Returns an error if prerequisites aren't met."""
@@ -330,7 +329,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
                 clustername=self.configuration.rbd_cluster_name,
                 conffile=self.configuration.rbd_ceph_conf))
         if pool is not None:
-            pool = encodeutils.safe_encode(pool)
+            pool = utils.convert_str(pool)
         else:
             pool = self.configuration.rbd_pool
 
@@ -408,8 +407,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
                                   pool['name'] ==
                                   self.configuration.rbd_pool][0]['stats']
                     stats['free_capacity_gb'] = (
-                        pool_stats['max_avail'] / units.Gi)
-                    used_capacity_gb = pool_stats['bytes_used'] / units.Gi
+                        pool_stats['max_avail'] // units.Gi)
+                    used_capacity_gb = pool_stats['bytes_used'] // units.Gi
                     stats['total_capacity_gb'] = (stats['free_capacity_gb']
                                                   + used_capacity_gb)
         except self.rados.Error:
@@ -458,8 +457,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
         and that clone has rbd_max_clone_depth clones behind it, the source
         volume will be flattened.
         """
-        src_name = encodeutils.safe_encode(src_vref['name'])
-        dest_name = encodeutils.safe_encode(volume['name'])
+        src_name = utils.convert_str(src_vref['name'])
+        dest_name = utils.convert_str(volume['name'])
         flatten_parent = False
 
         # Do full copy if requested
@@ -544,7 +543,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
 
         with RADOSClient(self) as client:
             self.RBDProxy().create(client.ioctx,
-                                   encodeutils.safe_encode(volume['name']),
+                                   utils.convert_str(volume['name']),
                                    size,
                                    order,
                                    old_format=False,
@@ -563,10 +562,10 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
         with RADOSClient(self, src_pool) as src_client:
             with RADOSClient(self) as dest_client:
                 self.RBDProxy().clone(src_client.ioctx,
-                                      encodeutils.safe_encode(src_image),
-                                      encodeutils.safe_encode(src_snap),
+                                      utils.convert_str(src_image),
+                                      utils.convert_str(src_snap),
                                       dest_client.ioctx,
-                                      encodeutils.safe_encode(volume['name']),
+                                      utils.convert_str(volume['name']),
                                       features=src_client.features)
 
     def _resize(self, volume, **kwargs):
@@ -655,7 +654,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
         """Deletes a logical volume."""
         # NOTE(dosaboy): this was broken by commit cbe1d5f. Ensure names are
         #                utf-8 otherwise librbd will barf.
-        volume_name = encodeutils.safe_encode(volume['name'])
+        volume_name = utils.convert_str(volume['name'])
         with RADOSClient(self) as client:
             try:
                 rbd_image = self.rbd.Image(client.ioctx, volume_name)
@@ -725,7 +724,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
     def create_snapshot(self, snapshot):
         """Creates an rbd snapshot."""
         with RBDVolumeProxy(self, snapshot['volume_name']) as volume:
-            snap = encodeutils.safe_encode(snapshot['name'])
+            snap = utils.convert_str(snapshot['name'])
             volume.create_snap(snap)
             volume.protect_snap(snap)
 
@@ -733,8 +732,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
         """Deletes an rbd snapshot."""
         # NOTE(dosaboy): this was broken by commit cbe1d5f. Ensure names are
         #                utf-8 otherwise librbd will barf.
-        volume_name = encodeutils.safe_encode(snapshot['volume_name'])
-        snap_name = encodeutils.safe_encode(snapshot['name'])
+        volume_name = utils.convert_str(snapshot['volume_name'])
+        snap_name = utils.convert_str(snapshot['name'])
         with RBDVolumeProxy(self, volume_name) as volume:
             try:
                 volume.unprotect_snap(snap_name)
@@ -806,7 +805,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
         if not location.startswith(prefix):
             reason = _('Not stored in rbd')
             raise exception.ImageUnacceptable(image_id=location, reason=reason)
-        pieces = map(urllib.parse.unquote, location[len(prefix):].split('/'))
+        pieces = [urllib.parse.unquote(loc)
+                  for loc in location[len(prefix):].split('/')]
         if any(map(lambda p: p == '', pieces)):
             reason = _('Blank components')
             raise exception.ImageUnacceptable(image_id=location, reason=reason)
@@ -985,8 +985,8 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
         with RADOSClient(self) as client:
             rbd_name = existing_ref['source-name']
             self.RBDProxy().rename(client.ioctx,
-                                   encodeutils.safe_encode(rbd_name),
-                                   encodeutils.safe_encode(volume['name']))
+                                   utils.convert_str(rbd_name),
+                                   utils.convert_str(volume['name']))
 
     def manage_existing_get_size(self, volume, existing_ref):
         """Return size of an existing image for manage_existing.
@@ -1004,7 +1004,7 @@ class RBDDriver(driver.RetypeVD, driver.TransferVD, driver.ExtendVD,
             raise exception.ManageExistingInvalidReference(
                 existing_ref=existing_ref, reason=reason)
 
-        rbd_name = encodeutils.safe_encode(existing_ref['source-name'])
+        rbd_name = utils.convert_str(existing_ref['source-name'])
 
         with RADOSClient(self) as client:
             # Raise an exception if we didn't find a suitable rbd image.

diff --git a/tox.ini b/tox.ini
index b6e0f3994ea7976680b3b9881320aa41c5ea6069..5501e4d22fb325873aad4d04a874541371500414 100644 (file)
--- a/tox.ini
+++ b/tox.ini
@@ -31,6 +31,7 @@ commands =
   python -m testtools.run \
     cinder.tests.unit.test_api_urlmap \
     cinder.tests.unit.test_backup \
+    cinder.tests.unit.test_backup_ceph \
     cinder.tests.unit.test_backup_driver_base \
     cinder.tests.unit.test_block_device \
     cinder.tests.unit.test_cloudbyte \
@@ -56,6 +57,7 @@ commands =
     cinder.tests.unit.test_nimble \
     cinder.tests.unit.test_openvstorage \
     cinder.tests.unit.test_qos_specs \
+    cinder.tests.unit.test_rbd \
     cinder.tests.unit.test_remotefs \
     cinder.tests.unit.test_replication \
     cinder.tests.unit.test_san \