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

@@ -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

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

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)
 

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:

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

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

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/conf0', base64.b64encode(b'contents0')),
-            ('/etc/conf1', base64.b64encode('contents1')),
+            ('/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

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,

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()

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)
 
 

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
+oslotest==1.0