From c121bef9f0ffc1cf6719b7fdc1792f7e677291b3 Mon Sep 17 00:00:00 2001 From: Jay Faulkner Date: Fri, 4 Apr 2014 13:30:42 -0700 Subject: [PATCH] Compatibility fixes for Python 3.3 1) Added a py33 environment to tox 2) Updated tests to mock the correctly named builtins.open based on python version 3) Other minor compatibility fixes Tests for Python 3.3 will not pass due to this bug: https://github.com/eventlet/eventlet/issues/83 among possibly others in eventlet. Change-Id: Ie4b512a926fa690ee77a71a89851c871ea1f6be0 --- ironic_python_agent/agent.py | 7 ++++--- ironic_python_agent/base.py | 3 ++- ironic_python_agent/configdrive.py | 8 ++++---- ironic_python_agent/hardware.py | 12 ++++++++++- ironic_python_agent/standby.py | 2 +- ironic_python_agent/tests/agent.py | 10 +++++++-- ironic_python_agent/tests/configdrive.py | 26 +++++++++++++++--------- ironic_python_agent/tests/hardware.py | 8 +++++++- ironic_python_agent/tests/standby.py | 20 +++++++++++------- ironic_python_agent/utils.py | 2 +- requirements.txt | 2 +- 11 files changed, 68 insertions(+), 32 deletions(-) diff --git a/ironic_python_agent/agent.py b/ironic_python_agent/agent.py index 80a744125..8bef4661c 100644 --- a/ironic_python_agent/agent.py +++ b/ironic_python_agent/agent.py @@ -19,6 +19,7 @@ import threading import time import pkg_resources +import six from stevedore import extension from wsgiref import simple_server @@ -148,7 +149,7 @@ class IronicPythonAgent(object): return self.node['uuid'] def list_command_results(self): - return self.command_results.values() + return list(self.command_results.values()) def get_command_result(self, result_id): try: @@ -171,7 +172,7 @@ class IronicPythonAgent(object): extension_part, command_part = self._split_command(command_name) if len(self.command_results) > 0: - last_command = self.command_results.values()[-1] + last_command = list(self.command_results.values())[-1] if not last_command.is_done(): raise errors.CommandExecutionError('agent is busy') @@ -192,7 +193,7 @@ class IronicPythonAgent(object): result = base.SyncCommandResult(command_name, kwargs, False, - unicode(e)) + six.text_type(e)) self.command_results[result.id] = result return result diff --git a/ironic_python_agent/base.py b/ironic_python_agent/base.py index 8a16e1140..5cd81e582 100644 --- a/ironic_python_agent/base.py +++ b/ironic_python_agent/base.py @@ -17,6 +17,7 @@ limitations under the License. import threading import uuid +import six from ironic_python_agent import encoding from ironic_python_agent import errors @@ -31,7 +32,7 @@ class AgentCommandStatus(object): class BaseCommandResult(encoding.Serializable): def __init__(self, command_name, command_params): - self.id = unicode(uuid.uuid4()) + self.id = six.text_type(uuid.uuid4()) self.command_name = command_name self.command_params = command_params self.command_status = AgentCommandStatus.RUNNING diff --git a/ironic_python_agent/configdrive.py b/ironic_python_agent/configdrive.py index f2039d34a..c27c78a75 100644 --- a/ironic_python_agent/configdrive.py +++ b/ironic_python_agent/configdrive.py @@ -38,13 +38,13 @@ class ConfigDriveWriter(object): os.makedirs(os.path.join(location, prefix, 'content')) metadata = {} - for k, v in self.metadata.iteritems(): + for k, v in self.metadata.items(): metadata[k] = v if self.files: metadata['files'] = [] filenumber = 0 - for filepath, contents in self.files.iteritems(): + for filepath, contents in self.files.items(): content_path = '/content/{0:04}'.format(filenumber) file_info = { 'content_path': content_path, @@ -72,10 +72,10 @@ def write_configdrive(location, metadata, files, prefix='openstack', """ writer = ConfigDriveWriter() - for k, v in metadata.iteritems(): + for k, v in metadata.items(): writer.add_metadata(k, v) - for path, b64_contents in files.iteritems(): + for path, b64_contents in files.items(): contents = base64.b64decode(b64_contents) writer.add_file(path, contents) diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index ff481e77b..d5fca7a01 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -15,9 +15,11 @@ limitations under the License. """ import abc +import functools import os import subprocess +import six import stevedore from ironic_python_agent import encoding @@ -169,7 +171,15 @@ def get_manager(): # There will always be at least one extension available (the # GenericHardwareManager). - preferred_extension = sorted(extension_manager, _compare_extensions)[0] + if six.PY2: + preferred_extension = sorted( + extension_manager, + _compare_extensions)[0] + else: + preferred_extension = sorted( + extension_manager, + key=functools.cmp_to_key(_compare_extensions))[0] + preferred_manager = preferred_extension.obj if preferred_manager.evaluate_hardware_support() <= 0: diff --git a/ironic_python_agent/standby.py b/ironic_python_agent/standby.py index 8bdbc8bf4..0e11f83ee 100644 --- a/ironic_python_agent/standby.py +++ b/ironic_python_agent/standby.py @@ -118,7 +118,7 @@ def _download_image(image_info): def _verify_image(image_info, image_location): hashes = image_info['hashes'] - for k, v in hashes.iteritems(): + for k, v in hashes.items(): algo = getattr(hashlib, k, None) if algo is None: continue diff --git a/ironic_python_agent/tests/agent.py b/ironic_python_agent/tests/agent.py index 3a25cd707..bed74eaa1 100644 --- a/ironic_python_agent/tests/agent.py +++ b/ironic_python_agent/tests/agent.py @@ -20,6 +20,7 @@ import time import mock from oslotest import base as test_base import pkg_resources +import six from stevedore import extension from wsgiref import simple_server @@ -32,6 +33,11 @@ from ironic_python_agent import hardware EXPECTED_ERROR = RuntimeError('command execution failed') +if six.PY2: + OPEN_FUNCTION_NAME = '__builtin__.open' +else: + OPEN_FUNCTION_NAME = 'builtins.open' + def foo_execute(*args, **kwargs): if kwargs['fail']: @@ -248,13 +254,13 @@ class TestBaseAgent(test_base.BaseTestCase): class TestAgentCmd(test_base.BaseTestCase): @mock.patch('ironic_python_agent.openstack.common.log.getLogger') - @mock.patch('__builtin__.open') + @mock.patch(OPEN_FUNCTION_NAME) def test__get_kernel_params_fail(self, logger_mock, open_mock): open_mock.side_effect = Exception params = agent_cmd._get_kernel_params() self.assertEqual(params, {}) - @mock.patch('__builtin__.open') + @mock.patch(OPEN_FUNCTION_NAME) def test__get_kernel_params(self, open_mock): kernel_line = 'api-url=http://localhost:9999 baz foo=bar\n' open_mock.return_value.__enter__ = lambda s: s diff --git a/ironic_python_agent/tests/configdrive.py b/ironic_python_agent/tests/configdrive.py index 209b078ee..8801dcd33 100644 --- a/ironic_python_agent/tests/configdrive.py +++ b/ironic_python_agent/tests/configdrive.py @@ -21,10 +21,16 @@ import json import mock from oslotest import base as test_base +import six from ironic_python_agent import configdrive from ironic_python_agent import utils +if six.PY2: + OPEN_FUNCTION_NAME = '__builtin__.open' +else: + OPEN_FUNCTION_NAME = 'builtins.open' + class ConfigDriveWriterTestCase(test_base.BaseTestCase): def setUp(self): @@ -43,12 +49,12 @@ class ConfigDriveWriterTestCase(test_base.BaseTestCase): self.assertEqual(files, {'/etc/filename': 'contents'}) @mock.patch('os.makedirs', autospec=True) - @mock.patch('__builtin__.open', autospec=True) + @mock.patch(OPEN_FUNCTION_NAME, autospec=True) def test_write_no_files(self, open_mock, makedirs_mock): metadata = {'admin_pass': 'password', 'hostname': 'test'} json_metadata = json.dumps(metadata) metadata_path = '/lol/ironic/latest/meta_data.json' - for k, v in metadata.iteritems(): + for k, v in metadata.items(): self.writer.add_metadata(k, v) open_mock.return_value.__enter__ = lambda s: s @@ -65,16 +71,16 @@ class ConfigDriveWriterTestCase(test_base.BaseTestCase): self.assertEqual(makedirs_calls, makedirs_mock.call_args_list) @mock.patch('os.makedirs', autospec=True) - @mock.patch('__builtin__.open', autospec=True) + @mock.patch(OPEN_FUNCTION_NAME, autospec=True) def test_write_with_files(self, open_mock, makedirs_mock): metadata = {'admin_pass': 'password', 'hostname': 'test'} - for k, v in metadata.iteritems(): + for k, v in metadata.items(): self.writer.add_metadata(k, v) files = utils.get_ordereddict([ ('/etc/conf0', 'contents0'), ('/etc/conf1', 'contents1'), ]) - for path, contents in files.iteritems(): + for path, contents in files.items(): self.writer.add_file(path, contents) open_mock.return_value.__enter__ = lambda s: s @@ -117,12 +123,12 @@ class ConfigDriveWriterTestCase(test_base.BaseTestCase): self.assertEqual(makedirs_calls, makedirs_mock.call_args_list) @mock.patch('os.makedirs', autospec=True) - @mock.patch('__builtin__.open', autospec=True) + @mock.patch(OPEN_FUNCTION_NAME, autospec=True) def test_write_configdrive(self, open_mock, makedirs_mock): metadata = {'admin_pass': 'password', 'hostname': 'test'} files = utils.get_ordereddict([ - ('/etc/conf0', base64.b64encode('contents0')), - ('/etc/conf1', base64.b64encode('contents1')), + ('/etc/conf0', base64.b64encode(b'contents0')), + ('/etc/conf1', base64.b64encode(b'contents1')), ]) metadata['files'] = [ {'content_path': '/content/0000', 'path': '/etc/conf0'}, @@ -148,10 +154,10 @@ class ConfigDriveWriterTestCase(test_base.BaseTestCase): open_calls = [ mock.call('/lol/openstack/content/0000', 'wb'), - mock.call().write('contents0'), + mock.call().write(b'contents0'), mock.call().__exit__(None, None, None), mock.call('/lol/openstack/content/0001', 'wb'), - mock.call().write('contents1'), + mock.call().write(b'contents1'), mock.call().__exit__(None, None, None), mock.call('/lol/openstack/latest/meta_data.json', 'wb'), # already checked diff --git a/ironic_python_agent/tests/hardware.py b/ironic_python_agent/tests/hardware.py index ad02d9b07..ce7b4a128 100644 --- a/ironic_python_agent/tests/hardware.py +++ b/ironic_python_agent/tests/hardware.py @@ -16,9 +16,15 @@ limitations under the License. import mock from oslotest import base as test_base +import six from ironic_python_agent import hardware +if six.PY2: + OPEN_FUNCTION_NAME = '__builtin__.open' +else: + OPEN_FUNCTION_NAME = 'builtins.open' + class TestGenericHardwareManager(test_base.BaseTestCase): def setUp(self): @@ -27,7 +33,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase): @mock.patch('os.listdir') @mock.patch('os.path.exists') - @mock.patch('__builtin__.open') + @mock.patch(OPEN_FUNCTION_NAME) def test_list_network_interfaces(self, mocked_open, mocked_exists, diff --git a/ironic_python_agent/tests/standby.py b/ironic_python_agent/tests/standby.py index 0a2248af2..1ac8910d0 100644 --- a/ironic_python_agent/tests/standby.py +++ b/ironic_python_agent/tests/standby.py @@ -16,10 +16,16 @@ limitations under the License. import mock from oslotest import base as test_base +import six from ironic_python_agent import errors from ironic_python_agent import standby +if six.PY2: + OPEN_FUNCTION_NAME = '__builtin__.open' +else: + OPEN_FUNCTION_NAME = 'builtins.open' + class TestStandbyExtension(test_base.BaseTestCase): def setUp(self): @@ -101,7 +107,7 @@ class TestStandbyExtension(test_base.BaseTestCase): location = standby._image_location(image_info) self.assertEqual(location, '/tmp/fake_id') - @mock.patch('__builtin__.open', autospec=True) + @mock.patch(OPEN_FUNCTION_NAME, autospec=True) @mock.patch('subprocess.call', autospec=True) def test_write_image(self, call_mock, open_mock): image_info = self._build_fake_image_info() @@ -124,7 +130,7 @@ class TestStandbyExtension(test_base.BaseTestCase): call_mock.assert_called_once_with(command) - @mock.patch('__builtin__.open', autospec=True) + @mock.patch(OPEN_FUNCTION_NAME, autospec=True) @mock.patch('subprocess.call', autospec=True) def test_copy_configdrive_to_disk(self, call_mock, open_mock): device = '/dev/sda' @@ -147,7 +153,7 @@ class TestStandbyExtension(test_base.BaseTestCase): call_mock.assert_called_once_with(command) @mock.patch('hashlib.md5', autospec=True) - @mock.patch('__builtin__.open', autospec=True) + @mock.patch(OPEN_FUNCTION_NAME, autospec=True) @mock.patch('requests.get', autospec=True) def test_download_image(self, requests_mock, open_mock, md5_mock): image_info = self._build_fake_image_info() @@ -159,7 +165,7 @@ class TestStandbyExtension(test_base.BaseTestCase): read_mock = open_mock.return_value.read read_mock.return_value = 'content' hexdigest_mock = md5_mock.return_value.hexdigest - hexdigest_mock.return_value = image_info['hashes'].values()[0] + hexdigest_mock.return_value = list(image_info['hashes'].values())[0] standby._download_image(image_info) requests_mock.assert_called_once_with(image_info['urls'][0], @@ -179,7 +185,7 @@ class TestStandbyExtension(test_base.BaseTestCase): image_info) @mock.patch('ironic_python_agent.standby._verify_image', autospec=True) - @mock.patch('__builtin__.open', autospec=True) + @mock.patch(OPEN_FUNCTION_NAME, autospec=True) @mock.patch('requests.get', autospec=True) def test_download_image_verify_fails(self, requests_mock, open_mock, verify_mock): @@ -191,7 +197,7 @@ class TestStandbyExtension(test_base.BaseTestCase): standby._download_image, image_info) - @mock.patch('__builtin__.open', autospec=True) + @mock.patch(OPEN_FUNCTION_NAME, autospec=True) @mock.patch('hashlib.sha1', autospec=True) @mock.patch('hashlib.md5', autospec=True) def test_verify_image_success(self, md5_mock, sha1_mock, open_mock): @@ -208,7 +214,7 @@ class TestStandbyExtension(test_base.BaseTestCase): # make sure we only check one hash, even though both are valid self.assertEqual(md5_mock.call_count + sha1_mock.call_count, 1) - @mock.patch('__builtin__.open', autospec=True) + @mock.patch(OPEN_FUNCTION_NAME, autospec=True) @mock.patch('hashlib.md5', autospec=True) def test_verify_image_failure(self, md5_mock, open_mock): image_info = self._build_fake_image_info() diff --git a/ironic_python_agent/utils.py b/ironic_python_agent/utils.py index 7bacdf698..ebaf7e5ce 100644 --- a/ironic_python_agent/utils.py +++ b/ironic_python_agent/utils.py @@ -15,7 +15,6 @@ limitations under the License. """ import collections -import ordereddict from ironic_python_agent.openstack.common import gettextutils as gtu from ironic_python_agent.openstack.common import log as logging @@ -29,6 +28,7 @@ def get_ordereddict(*args, **kwargs): try: return collections.OrderedDict(*args, **kwargs) except AttributeError: + import ordereddict return ordereddict.OrderedDict(*args, **kwargs) diff --git a/requirements.txt b/requirements.txt index 6c69480cc..53281e62a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,4 +8,4 @@ eventlet>=0.13.0 oslo.config>=1.2.0 Babel>=1.3 iso8601>=0.1.9 -oslotest==1.0 \ No newline at end of file +oslotest==1.0