Fix Python 3 issues in the blockbridge driver

* Replace httplib import with six.moves.http_client
* Replace urllib imports with six.moves.urllib, update urllib.
  For example, replace urllib.quote() with urllib.parse.quote().
* test_blockbridge: try get the mock module from unittest.mock, part of
  the Python stdlib since Python 3.3. The third-party mock module has a
  bug on Python 3.4.
* On Python 3, base64.encodestring() expects bytes and returns bytes.
  Encode credentials to UTF-8 and decode base64 from ASCII to get the
  final string as Unicode on Python 3.
* test_cfg_api_auth_scheme_password: parse the connection URL to compare
  it. Comparing directly the URL fails on Python 3 because URL
  parameters are rendered in a random order because of the hash
  randomization.
* Replace pools.values()[0] with list(pools.values())[0]. On Python 3,
  dict.values() returns an iterator.
* tox.ini: add cinder.tests.unit.test_blockbridge to Python 3.4.

Blueprint cinder-python3
Change-Id: I3c6b935680b5427d6ffdc1a00cd5221475209cdd

diff --git a/cinder/tests/unit/test_blockbridge.py b/cinder/tests/unit/test_blockbridge.py
index 09745db03fd283e83e3ad37bb763a1386d24eddf..02727476770b69273513801a05908e8a9ba077b3 100644 (file)
--- a/cinder/tests/unit/test_blockbridge.py
+++ b/cinder/tests/unit/test_blockbridge.py
@@ -16,13 +16,17 @@ Blockbridge EPS iSCSI Volume Driver Tests
 """
 
 import base64
-import httplib
-import urllib
 
-import mock
+try:
+    from unittest import mock
+except ImportError:
+    import mock
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import units
+import six
+from six.moves import http_client
+from six.moves import urllib
 
 from cinder import context
 from cinder import exception
@@ -71,7 +75,7 @@ def common_mocks(f):
     mocks that can't/don't get unset.
     """
     def _common_inner_inner1(inst, *args, **kwargs):
-        @mock.patch("httplib.HTTPSConnection", autospec=True)
+        @mock.patch("six.moves.http_client.HTTPSConnection", autospec=True)
         def _common_inner_inner2(mock_conn):
             inst.mock_httplib = mock_conn
             inst.mock_conn = mock_conn.return_value
@@ -154,7 +158,7 @@ class BlockbridgeISCSIDriverTestCase(test.TestCase):
         self.mock_response.read.return_value = '{}'
         self.mock_response.status = 200
 
-        conn = httplib.HTTPSConnection('whatever', None)
+        conn = http_client.HTTPSConnection('whatever', None)
         conn.request('GET', '/blah', '{}', {})
         rsp = conn.getresponse()
 
@@ -168,7 +172,7 @@ class BlockbridgeISCSIDriverTestCase(test.TestCase):
         self.mock_response.read.return_value = mock_body
         self.mock_response.status = 413
 
-        conn = httplib.HTTPSConnection('whatever', None)
+        conn = http_client.HTTPSConnection('whatever', None)
         conn.request('GET', '/blah', '{}', {})
         rsp = conn.getresponse()
 
@@ -196,28 +200,37 @@ class BlockbridgeISCSIDriverTestCase(test.TestCase):
         with mock.patch.object(self.driver, 'hostname', 'mock-hostname'):
             self.driver.get_volume_stats(True)
 
-        b64_creds = base64.encodestring("%s:%s" % (
-            self.cfg.blockbridge_auth_user,
-            self.cfg.blockbridge_auth_password)).replace("\n", "")
+        creds = "%s:%s" % (self.cfg.blockbridge_auth_user,
+                           self.cfg.blockbridge_auth_password)
+        if six.PY3:
+            creds = creds.encode('utf-8')
+            b64_creds = base64.encodestring(creds).decode('ascii')
+        else:
+            b64_creds = base64.encodestring(creds)
 
         params = dict(
             hostname='mock-hostname',
             version=self.driver.VERSION,
             backend_name='BlockbridgeISCSIDriver',
             pool='OpenStack',
-            query='%2Bopenstack')
+            query='+openstack')
 
-        full_url = ("/api/cinder/status?query=%(query)s&"
-                    "hostname=%(hostname)s&backend_name=%(backend_name)s&"
-                    "version=%(version)s&pool=%(pool)s" % params)
         headers = {
             'Accept': 'application/vnd.blockbridge-3+json',
-            'Authorization': "Basic %s" % b64_creds,
+            'Authorization': "Basic %s" % b64_creds.replace("\n", ""),
             'User-Agent': "cinder-volume/%s" % self.driver.VERSION,
         }
 
         self.mock_conn.request.assert_called_once_with(
-            'GET', full_url, None, headers)
+            'GET', mock.ANY, None, headers)
+        # Parse the URL instead of comparing directly both URLs.
+        # On Python 3, parameters are formatted in a random order because
+        # of the hash randomization.
+        conn_url = self.mock_conn.request.call_args[0][1]
+        conn_params = dict(urllib.parse.parse_qsl(conn_url.split("?", 1)[1]))
+        self.assertTrue(conn_url.startswith("/api/cinder/status?"),
+                        repr(conn_url))
+        self.assertEqual(params, conn_params)
 
     @common_mocks
     def test_create_volume(self):
@@ -508,7 +521,7 @@ class BlockbridgeISCSIDriverTestCase(test.TestCase):
 
         self.assertEqual(expected_props, props)
 
-        ini_name = urllib.quote(self.connector["initiator"], "")
+        ini_name = urllib.parse.quote(self.connector["initiator"], "")
         url = "/volumes/%s/exports/%s" % (self.volume_id, ini_name)
         params = dict(
             chap_user="mock-user-abcdef123456",
@@ -525,7 +538,7 @@ class BlockbridgeISCSIDriverTestCase(test.TestCase):
     def test_terminate_connection(self):
         self.driver.terminate_connection(self.volume, self.connector)
 
-        ini_name = urllib.quote(self.connector["initiator"], "")
+        ini_name = urllib.parse.quote(self.connector["initiator"], "")
         url = "/volumes/%s/exports/%s" % (self.volume_id, ini_name)
         kwargs = dict(
             method='DELETE',

diff --git a/cinder/volume/drivers/blockbridge.py b/cinder/volume/drivers/blockbridge.py
index ff3b887ebb22a69ffa74ab15538cb3781d4a85bc..7661841d1866e8ede20035a7541d6ed83d707c77 100644 (file)
--- a/cinder/volume/drivers/blockbridge.py
+++ b/cinder/volume/drivers/blockbridge.py
@@ -16,14 +16,15 @@ Blockbridge EPS iSCSI Volume Driver
 """
 
 import base64
-import httplib
 import socket
-import urllib
 
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import units
+import six
+from six.moves import http_client
+from six.moves import urllib
 
 from cinder import context
 from cinder import exception
@@ -84,7 +85,12 @@ class BlockbridgeAPIClient(object):
         if self.configuration.blockbridge_auth_scheme == 'password':
             user = self.configuration.safe_get('blockbridge_auth_user')
             pw = self.configuration.safe_get('blockbridge_auth_password')
-            b64_creds = base64.encodestring("%s:%s" % (user, pw))
+            creds = "%s:%s" % (user, pw)
+            if six.PY3:
+                creds = creds.encode('utf-8')
+                b64_creds = base64.encodestring(creds).decode('ascii')
+            else:
+                b64_creds = base64.encodestring(creds)
             authz = "Basic %s" % b64_creds.replace("\n", "")
         elif self.configuration.blockbridge_auth_scheme == 'token':
             token = self.configuration.blockbridge_auth_token or ''
@@ -137,7 +143,7 @@ class BlockbridgeAPIClient(object):
         if method in ['GET', 'DELETE']:
             # For GET method add parameters to the URL
             if params:
-                url += '?' + urllib.urlencode(params)
+                url += '?' + urllib.parse.urlencode(params)
         elif method in ['POST', 'PUT', 'PATCH']:
             body = jsonutils.dumps(params)
             headers['Content-Type'] = 'application/json'
@@ -145,7 +151,7 @@ class BlockbridgeAPIClient(object):
             raise exception.UnknownCmd(cmd=method)
 
         # connect and execute the request
-        connection = httplib.HTTPSConnection(cfg['host'], cfg['port'])
+        connection = http_client.HTTPSConnection(cfg['host'], cfg['port'])
         connection.request(method, url, body, headers)
         response = connection.getresponse()
 
@@ -231,7 +237,7 @@ class BlockbridgeISCSIDriver(driver.ISCSIDriver):
                 reason=_("Blockbridge default pool does not exist"))
 
     def _vol_api_submit(self, vol_id, **kwargs):
-        vol_id = urllib.quote(vol_id, '')
+        vol_id = urllib.parse.quote(vol_id, '')
         rel_url = "/volumes/%s" % vol_id
 
         return self.client.submit(rel_url, **kwargs)
@@ -256,8 +262,8 @@ class BlockbridgeISCSIDriver(driver.ISCSIDriver):
                              params=params, **kwargs)
 
     def _snap_api_submit(self, vol_id, snap_id, **kwargs):
-        vol_id = urllib.quote(vol_id, '')
-        snap_id = urllib.quote(snap_id, '')
+        vol_id = urllib.parse.quote(vol_id, '')
+        snap_id = urllib.parse.quote(snap_id, '')
         rel_url = "/volumes/%s/snapshots/%s" % (vol_id, snap_id)
 
         return self.client.submit(rel_url, **kwargs)
@@ -275,8 +281,8 @@ class BlockbridgeISCSIDriver(driver.ISCSIDriver):
                                      **kwargs)
 
     def _export_api_submit(self, vol_id, ini_name, **kwargs):
-        vol_id = urllib.quote(vol_id, '')
-        ini_name = urllib.quote(ini_name, '')
+        vol_id = urllib.parse.quote(vol_id, '')
+        ini_name = urllib.parse.quote(ini_name, '')
         rel_url = "/volumes/%s/exports/%s" % (vol_id, ini_name)
 
         return self.client.submit(rel_url, **kwargs)
@@ -321,7 +327,7 @@ class BlockbridgeISCSIDriver(driver.ISCSIDriver):
         else:
             # no pool specified or defaulted -- just pick whatever comes out of
             # the dictionary first.
-            return pools.values()[0]
+            return list(pools.values())[0]
 
     def create_volume(self, volume):
         """Create a volume on a Blockbridge EPS backend.

diff --git a/tox.ini b/tox.ini
index f5f46c235910f5622ddd5cf3f42e03be0c79e576..144737a4ad56bfb204ac04f18dfb1f7a0770c745 100644 (file)
--- a/tox.ini
+++ b/tox.ini
@@ -36,6 +36,7 @@ commands =
     cinder.tests.unit.test_backup_swift \
     cinder.tests.unit.test_backup_tsm \
     cinder.tests.unit.test_block_device \
+    cinder.tests.unit.test_blockbridge \
     cinder.tests.unit.test_cloudbyte \
     cinder.tests.unit.test_conf \
     cinder.tests.unit.test_context \