From 583d03e117a95ce24253eec63a3b7ef564e69307 Mon Sep 17 00:00:00 2001 From: Victor Stinner Date: Tue, 23 Jun 2015 14:08:25 +0200 Subject: [PATCH] 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 --- cinder/tests/unit/test_blockbridge.py | 47 +++++++++++++++++---------- cinder/volume/drivers/blockbridge.py | 28 +++++++++------- tox.ini | 1 + 3 files changed, 48 insertions(+), 28 deletions(-) diff --git a/cinder/tests/unit/test_blockbridge.py b/cinder/tests/unit/test_blockbridge.py index 09745db03..027274767 100644 --- 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 ff3b887eb..7661841d1 100644 --- 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 f5f46c235..144737a4a 100644 --- 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 \ -- 2.45.2