openstack/ironic-python-agent
Code Issues Proposed changes

Enforce all flake8 rules except E129

Browse Source 
Bring ironic-python-agent in line with the other ironic projects.

Stop ignoring all E12* errors except E129
Stop ignoring E711

Change-Id: Icb9bc198473d1b5e807c20869eb2af7f4d7ac360
This commit is contained in:
John L. Villalovos 2015-10-02 10:01:00 -07:00
parent 0144e79df5
commit dcbba2b121
17 changed files with 104 additions and 97 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
imagebuild/coreos/coreos-oem-inject.py
View File

@ -144,7 +144,8 @@ def main():
os.makedirs(output_dir)
output_kernel = os.path.join(output_dir, os.path.basename(kernel))
output_cpio = os.path.join(output_dir,
output_cpio = os.path.join(
output_dir,
os.path.basename(orig_cpio).replace('.cpio.gz', '-oem.cpio.gz'))
inject_oem(orig_cpio, oem_dir, output_cpio)
shutil.copy(kernel, output_kernel)

4
ironic_python_agent/agent.py
View File

@ -223,7 +223,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
be found.
"""
iface_list = [iface.serialize()['name'] for iface in
hardware.dispatch_to_managers('list_network_interfaces')]
hardware.dispatch_to_managers('list_network_interfaces')]
iface_list = [name for name in iface_list if 'lo' not in name]
if len(iface_list) == 0:
@ -287,7 +287,7 @@ class IronicPythonAgent(base.ExecuteCommandMixin):
content = self.api_client.lookup_node(
hardware_info=hardware.dispatch_to_managers(
'list_hardware_info'),
'list_hardware_info'),
timeout=self.lookup_timeout,
starting_interval=self.lookup_interval,
node_uuid=uuid)

26
ironic_python_agent/cmd/agent.py
View File

@ -28,14 +28,14 @@ APARAMS = utils.get_agent_params()
cli_opts = [
cfg.StrOpt('api_url',
default=APARAMS.get('ipa-api-url', 'http://127.0.0.1:6385'),
deprecated_name='api-url',
help='URL of the Ironic API'),
default=APARAMS.get('ipa-api-url', 'http://127.0.0.1:6385'),
deprecated_name='api-url',
help='URL of the Ironic API'),
cfg.StrOpt('listen_host',
default=APARAMS.get('ipa-listen-host', '0.0.0.0'),
deprecated_name='listen-host',
help='The IP address to listen on.'),
default=APARAMS.get('ipa-listen-host', '0.0.0.0'),
deprecated_name='listen-host',
help='The IP address to listen on.'),
cfg.IntOpt('listen_port',
default=int(APARAMS.get('ipa-listen-port', 9999)),
@ -43,10 +43,10 @@ cli_opts = [
help='The port to listen on'),
cfg.StrOpt('advertise_host',
default=APARAMS.get('ipa-advertise-host', None),
deprecated_name='advertise_host',
help='The host to tell Ironic to reply and send '
'commands to.'),
default=APARAMS.get('ipa-advertise-host', None),
deprecated_name='advertise_host',
help='The host to tell Ironic to reply and send '
'commands to.'),
cfg.IntOpt('advertise_port',
default=int(APARAMS.get('ipa-advertise-port', 9999)),
@ -88,9 +88,9 @@ cli_opts = [
'exceeded.'),
cfg.StrOpt('driver_name',
default=APARAMS.get('ipa-driver-name', 'agent_ipmitool'),
deprecated_name='driver-name',
help='The Ironic driver in use for this node'),
default=APARAMS.get('ipa-driver-name', 'agent_ipmitool'),
deprecated_name='driver-name',
help='The Ironic driver in use for this node'),
cfg.FloatOpt('lldp_timeout',
default=APARAMS.get('lldp-timeout', 30.0),

2
ironic_python_agent/extensions/standby.py
View File

@ -150,7 +150,7 @@ def _download_image(image_info):
f.write(chunk)
except Exception as e:
msg = 'Unable to write image to {0}. Error: {1}'.format(
image_location, str(e))
image_location, str(e))
raise errors.ImageDownloadError(image_info['id'], msg)
totaltime = time.time() - starttime

37
ironic_python_agent/hardware.py
View File

@ -358,7 +358,7 @@ class GenericHardwareManager(HardwareManager):
def _is_device(self, interface_name):
device_path = '{0}/class/net/{1}/device'.format(self.sys_path,
interface_name)
interface_name)
return os.path.exists(device_path)
def list_network_interfaces(self):
@ -431,9 +431,10 @@ class GenericHardwareManager(HardwareManager):
if hint_value != current_value:
LOG.debug("Root device hint %(hint)s=%(value)s does not "
"match the device %(device)s value of "
"%(current)s", {'hint': hint,
'value': hint_value, 'device': device,
'current': current_value})
"%(current)s", {
'hint': hint,
'value': hint_value, 'device': device,
'current': current_value})
return False
return True
@ -465,7 +466,8 @@ class GenericHardwareManager(HardwareManager):
return dev.name
else:
raise errors.DeviceNotFound("No suitable device was found for "
raise errors.DeviceNotFound(
"No suitable device was found for "
"deployment using these hints %s" % root_device_hints)
def erase_block_device(self, node, block_device):
@ -483,7 +485,7 @@ class GenericHardwareManager(HardwareManager):
return
msg = ('Unable to erase block device {0}: device is unsupported.'
).format(block_device.name)
).format(block_device.name)
LOG.error(msg)
raise errors.IncompatibleHardwareMethodError(msg)
@ -501,7 +503,7 @@ class GenericHardwareManager(HardwareManager):
'--iterations', str(npasses), block_device.name)
except (processutils.ProcessExecutionError, OSError) as e:
msg = ("Erasing block device %(dev)s failed with error %(err)s ",
{'dev': block_device.name, 'err': e})
{'dev': block_device.name, 'err': e})
LOG.error(msg)
return False
@ -517,7 +519,7 @@ class GenericHardwareManager(HardwareManager):
if os.path.exists(vm_device_label):
link = os.readlink(vm_device_label)
device = os.path.normpath(os.path.join(os.path.dirname(
vm_device_label), link))
vm_device_label), link))
if block_device.name == device:
return True
return False
@ -552,12 +554,14 @@ class GenericHardwareManager(HardwareManager):
return False
if 'enabled' in security_lines:
raise errors.BlockDeviceEraseError(('Block device {0} already has '
'a security password set').format(block_device.name))
raise errors.BlockDeviceEraseError(
('Block device {0} already has a security password set'
).format(block_device.name))
if 'not frozen' not in security_lines:
raise errors.BlockDeviceEraseError(('Block device {0} is frozen '
'and cannot be erased').format(block_device.name))
raise errors.BlockDeviceEraseError(
('Block device {0} is frozen and cannot be erased'
).format(block_device.name))
utils.execute('hdparm', '--user-master', 'u', '--security-set-pass',
'NULL', block_device.name)
@ -573,8 +577,9 @@ class GenericHardwareManager(HardwareManager):
# Verify that security is now 'not enabled'
security_lines = self._get_ata_security_lines(block_device)
if 'not enabled' not in security_lines:
raise errors.BlockDeviceEraseError(('An unknown error occurred '
'erasing block device {0}').format(block_device.name))
raise errors.BlockDeviceEraseError(
('An unknown error occurred erasing block device {0}'
).format(block_device.name))
return True
@ -625,7 +630,7 @@ def _get_managers():
extensions = sorted(extension_manager, _compare_extensions)
else:
extensions = sorted(extension_manager,
key=functools.cmp_to_key(_compare_extensions))
key=functools.cmp_to_key(_compare_extensions))
preferred_managers = []
@ -712,7 +717,7 @@ def dispatch_to_managers(method, *args, **kwargs):
return getattr(manager, method)(*args, **kwargs)
except(errors.IncompatibleHardwareMethodError):
LOG.debug('HardwareManager {0} does not support {1}'
.format(manager, method))
.format(manager, method))
except Exception as e:
LOG.exception('Unexpected error dispatching %(method)s to '
'manager %(manager)s: %(e)s',

10
ironic_python_agent/tests/functional/base.py
View File

@ -33,9 +33,9 @@ class FunctionalBase(test_base.BaseTestCase):
# Build a basic standalone agent using the config option defaults.
# 127.0.0.1:6835 is the fake Ironic client.
self.agent = agent.IronicPythonAgent(
'http://127.0.0.1:6835', 'localhost', ('0.0.0.0',
int(test_port)), 3, 10, None, 300, 1,
'agent_ipmitool', True)
'http://127.0.0.1:6835', 'localhost',
('0.0.0.0', int(test_port)), 3, 10, None, 300, 1, 'agent_ipmitool',
True)
self.process = multiprocessing.Process(
target=self.agent.run)
self.process.start()
@ -46,8 +46,8 @@ class FunctionalBase(test_base.BaseTestCase):
max_tries = os.environ.get('IPA_WAIT_TIME', '2')
while tries < int(max_tries):
try:
return requests.get('http://localhost:%s/v1/commands' %
test_port)
return requests.get(
'http://localhost:%s/v1/commands' % test_port)
except requests.ConnectionError:
time.sleep(.1)
tries += 1

5
ironic_python_agent/tests/functional/test_commands.py
View File

@ -20,7 +20,8 @@ from ironic_python_agent.tests.functional import base
class TestCommands(base.FunctionalBase):
def test_empty_commands(self):
commands = requests.get('http://localhost:%s/v1/commands' %
os.environ.get('TEST_PORT', '9999'))
commands = requests.get(
'http://localhost:%s/v1/commands' % os.environ.get('TEST_PORT',
'9999'))
self.assertEqual(200, commands.status_code)
self.assertEqual({'commands': []}, commands.json())

4
ironic_python_agent/tests/unit/extensions/test_base.py
View File

@ -134,7 +134,7 @@ class TestExtensionDecorators(test_base.BaseTestCase):
result.command_status)
self.assertEqual(None, result.command_error)
self.assertEqual({'result': 'fake_async_command: v1'},
result.command_result)
result.command_result)
self.agent.force_heartbeat.assert_called_once_with()
def test_async_command_success_without_agent(self):
@ -148,7 +148,7 @@ class TestExtensionDecorators(test_base.BaseTestCase):
result.command_status)
self.assertEqual(None, result.command_error)
self.assertEqual({'result': 'fake_async_command: v1'},
result.command_result)
result.command_result)
def test_async_command_validation_failure(self):
self.assertRaises(errors.InvalidCommandParamsError,

17
ironic_python_agent/tests/unit/extensions/test_image.py
View File

@ -85,7 +85,7 @@ class TestImageExtension(test_base.BaseTestCase):
self.fake_dir + '/proc'),
mock.call(('chroot %s /bin/bash -c '
'"/usr/sbin/grub-install %s"' %
(self.fake_dir, self.fake_dev)), shell=True,
(self.fake_dir, self.fake_dev)), shell=True,
env_variables={'PATH': '/sbin:/bin'}),
mock.call(('chroot %s /bin/bash -c '
'"/usr/sbin/grub-mkconfig -o '
@ -130,7 +130,7 @@ class TestImageExtension(test_base.BaseTestCase):
self.fake_dir + '/boot/efi'),
mock.call(('chroot %s /bin/bash -c '
'"/usr/sbin/grub-install %s"' %
(self.fake_dir, self.fake_dev)), shell=True,
(self.fake_dir, self.fake_dev)), shell=True,
env_variables={'PATH': '/sbin:/bin'}),
mock.call(('chroot %s /bin/bash -c '
'"/usr/sbin/grub-mkconfig -o '
@ -159,8 +159,8 @@ class TestImageExtension(test_base.BaseTestCase):
@mock.patch.object(os, 'makedirs')
@mock.patch.object(image, '_get_partition')
def test__install_grub2_uefi_umount_fails(
self, mock_get_part_uuid, mkdir_mock, environ_mock,
mock_execute, mock_dispatch):
self, mock_get_part_uuid, mkdir_mock, environ_mock, mock_execute,
mock_dispatch):
mock_get_part_uuid.side_effect = [self.fake_root_part,
self.fake_efi_system_part]
@ -186,7 +186,7 @@ class TestImageExtension(test_base.BaseTestCase):
self.fake_dir + '/boot/efi'),
mock.call(('chroot %s /bin/bash -c '
'"/usr/sbin/grub-install %s"' %
(self.fake_dir, self.fake_dev)), shell=True,
(self.fake_dir, self.fake_dev)), shell=True,
env_variables={'PATH': '/sbin:/bin'}),
mock.call(('chroot %s /bin/bash -c '
'"/usr/sbin/grub-mkconfig -o '
@ -217,8 +217,7 @@ class TestImageExtension(test_base.BaseTestCase):
KNAME="test2" UUID="%s" TYPE="part"''' % self.fake_root_uuid)
mock_execute.side_effect = (None, None, [lsblk_output])
root_part = image._get_partition(self.fake_dev,
self.fake_root_uuid)
root_part = image._get_partition(self.fake_dev, self.fake_root_uuid)
self.assertEqual('/dev/test2', root_part)
expected = [mock.call('partx', '-u', self.fake_dev, attempts=3,
delay_on_retry=True),
@ -228,7 +227,7 @@ class TestImageExtension(test_base.BaseTestCase):
self.assertFalse(mock_dispatch.called)
def test__get_partition_no_device_found(self, mock_execute,
mock_dispatch):
mock_dispatch):
lsblk_output = ('''KNAME="test" UUID="" TYPE="disk"
KNAME="test1" UUID="256a39e3-ca3c-4fb8-9cc2-b32eec441f47" TYPE="part"
KNAME="test2" UUID="" TYPE="part"''')
@ -245,7 +244,7 @@ class TestImageExtension(test_base.BaseTestCase):
self.assertFalse(mock_dispatch.called)
def test__get_partition_command_fail(self, mock_execute,
mock_dispatch):
mock_dispatch):
mock_execute.side_effect = (None, None,
processutils.ProcessExecutionError('boom'))
self.assertRaises(errors.CommandExecutionError,

2
ironic_python_agent/tests/unit/extensions/test_iscsi.py
View File

@ -63,7 +63,7 @@ class TestISCSIExtension(test_base.BaseTestCase):
mock_dispatch.assert_called_once_with('get_os_install_device')
mock_wait_iscsi.assert_called_once_with()
self.assertEqual({'iscsi_target_iqn': self.fake_iqn},
result.command_result)
result.command_result)
@mock.patch.object(os.path, 'exists', lambda x: False)
def test_start_iscsi_target_fail_wait_daemon(self, mock_execute,

2
ironic_python_agent/tests/unit/test_errors.py
View File

@ -131,7 +131,7 @@ class TestErrors(test_base.BaseTestCase):
(errors.IncompatibleHardwareMethodError(), DEFAULT_DETAILS),
(errors.IncompatibleHardwareMethodError(DETAILS),
SAME_DETAILS),
]
]
for (obj, check_details) in cases:
self._test_class(obj, check_details)

57
ironic_python_agent/tests/unit/test_hardware.py
View File

@ -142,16 +142,16 @@ BLK_DEVICE_TEMPLATE_SMALL = (
)
SHRED_OUTPUT = (
'shred: /dev/sda: pass 1/2 (random)...\n'
'shred: /dev/sda: pass 1/2 (random)...4.9GiB/29GiB 17%\n'
'shred: /dev/sda: pass 1/2 (random)...15GiB/29GiB 51%\n'
'shred: /dev/sda: pass 1/2 (random)...20GiB/29GiB 69%\n'
'shred: /dev/sda: pass 1/2 (random)...29GiB/29GiB 100%\n'
'shred: /dev/sda: pass 2/2 (000000)...\n'
'shred: /dev/sda: pass 2/2 (000000)...4.9GiB/29GiB 17%\n'
'shred: /dev/sda: pass 2/2 (000000)...15GiB/29GiB 51%\n'
'shred: /dev/sda: pass 2/2 (000000)...20GiB/29GiB 69%\n'
'shred: /dev/sda: pass 2/2 (000000)...29GiB/29GiB 100%\n'
'shred: /dev/sda: pass 1/2 (random)...\n'
'shred: /dev/sda: pass 1/2 (random)...4.9GiB/29GiB 17%\n'
'shred: /dev/sda: pass 1/2 (random)...15GiB/29GiB 51%\n'
'shred: /dev/sda: pass 1/2 (random)...20GiB/29GiB 69%\n'
'shred: /dev/sda: pass 1/2 (random)...29GiB/29GiB 100%\n'
'shred: /dev/sda: pass 2/2 (000000)...\n'
'shred: /dev/sda: pass 2/2 (000000)...4.9GiB/29GiB 17%\n'
'shred: /dev/sda: pass 2/2 (000000)...15GiB/29GiB 51%\n'
'shred: /dev/sda: pass 2/2 (000000)...20GiB/29GiB 69%\n'
'shred: /dev/sda: pass 2/2 (000000)...29GiB/29GiB 100%\n'
)
@ -229,11 +229,14 @@ class TestHardwareManagerLoading(test_base.BaseTestCase):
fake_ep = mock.Mock()
fake_ep.module_name = 'fake'
fake_ep.attrs = ['fake attrs']
ext1 = extension.Extension('fake_generic0', fake_ep, None,
ext1 = extension.Extension(
'fake_generic0', fake_ep, None,
FakeHardwareManager(hardware.HardwareSupport.GENERIC))
ext2 = extension.Extension('fake_mainline0', fake_ep, None,
ext2 = extension.Extension(
'fake_mainline0', fake_ep, None,
FakeHardwareManager(hardware.HardwareSupport.MAINLINE))
ext3 = extension.Extension('fake_generic1', fake_ep, None,
ext3 = extension.Extension(
'fake_generic1', fake_ep, None,
FakeHardwareManager(hardware.HardwareSupport.GENERIC))
self.correct_hw_manager = ext2.obj
self.fake_ext_mgr = extension.ExtensionManager.make_test_instance([
@ -596,10 +599,10 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
@mock.patch.object(utils, 'execute')
def test_erase_block_device_notsupported_shred(self, mocked_execute):
hdparm_output = HDPARM_INFO_TEMPLATE % {
'supported': 'not\tsupported',
'enabled': 'not\tenabled',
'frozen': 'not\tfrozen',
'enhanced_erase': 'not\tsupported: enhanced erase',
'supported': 'not\tsupported',
'enabled': 'not\tenabled',
'frozen': 'not\tfrozen',
'enhanced_erase': 'not\tsupported: enhanced erase',
}
mocked_execute.side_effect = [
@ -654,7 +657,7 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
@mock.patch.object(os, 'readlink', autospec=True)
@mock.patch.object(os.path, 'exists', autospec=True)
def test__is_virtual_media_device_path_doesnt_exist(self, mocked_exists,
mocked_link):
mocked_link):
mocked_exists.return_value = False
block_device = hardware.BlockDevice('/dev/sda', 'big', 1073741824,
True)
@ -670,8 +673,9 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
True)
res = self.hardware._shred_block_device(self.node, block_device)
self.assertFalse(res)
mocked_execute.assert_called_once_with('shred', '--force', '--zero',
'--verbose', '--iterations', '1', '/dev/sda')
mocked_execute.assert_called_once_with(
'shred', '--force', '--zero', '--verbose', '--iterations', '1',
'/dev/sda')
@mock.patch.object(utils, 'execute')
def test_erase_block_device_shred_fail_processerror(self, mocked_execute):
@ -680,8 +684,9 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
True)
res = self.hardware._shred_block_device(self.node, block_device)
self.assertFalse(res)
mocked_execute.assert_called_once_with('shred', '--force', '--zero',
'--verbose', '--iterations', '1', '/dev/sda')
mocked_execute.assert_called_once_with(
'shred', '--force', '--zero', '--verbose', '--iterations', '1',
'/dev/sda')
@mock.patch.object(utils, 'execute')
def test_erase_block_device_ata_security_enabled(self, mocked_execute):
@ -778,10 +783,10 @@ class TestGenericHardwareManager(test_base.BaseTestCase):
expected_option,
'NULL', '/dev/sda')
test_security_erase_option(self,
'\tsupported: enhanced erase', '--security-erase-enhanced')
test_security_erase_option(self,
'\tnot\tsupported: enhanced erase', '--security-erase')
test_security_erase_option(
self, '\tsupported: enhanced erase', '--security-erase-enhanced')
test_security_erase_option(
self, '\tnot\tsupported: enhanced erase', '--security-erase')
@mock.patch.object(utils, 'execute')
def test_get_bmc_address(self, mocked_execute):

4
ironic_python_agent/tests/unit/test_ironic_api_client.py
View File

@ -135,7 +135,7 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
node_uuid=None)
url = '{api_url}v1/drivers/{driver}/vendor_passthru/lookup'.format(
api_url=API_URL, driver=DRIVER)
api_url=API_URL, driver=DRIVER)
request_args = self.api_client.session.request.call_args[0]
self.assertEqual(request_args[0], 'POST')
self.assertEqual(request_args[1], url)
@ -266,7 +266,7 @@ class TestBaseIronicPythonAgent(test_base.BaseTestCase):
node_uuid='uuid')
url = '{api_url}v1/drivers/{driver}/vendor_passthru/lookup'.format(
api_url=API_URL, driver=DRIVER)
api_url=API_URL, driver=DRIVER)
request_args = self.api_client.session.request.call_args[0]
self.assertEqual(request_args[0], 'POST')
self.assertEqual(request_args[1], url)

13
ironic_python_agent/tests/unit/test_multi_hardware.py
View File

@ -111,13 +111,12 @@ class TestMultipleHardwareManagerLoading(test_base.BaseTestCase):
fake_ep = mock.Mock()
fake_ep.module_name = 'fake'
fake_ep.attrs = ['fake attrs']
self.generic_hwm = extension.Extension('fake_generic', fake_ep, None,
FakeGenericHardwareManager())
self.mainline_hwm = extension.Extension('fake_mainline', fake_ep, None,
FakeMainlineHardwareManager())
self.fake_ext_mgr = extension.ExtensionManager.make_test_instance([
self.generic_hwm, self.mainline_hwm
])
self.generic_hwm = extension.Extension(
'fake_generic', fake_ep, None, FakeGenericHardwareManager())
self.mainline_hwm = extension.Extension(
'fake_mainline', fake_ep, None, FakeMainlineHardwareManager())
self.fake_ext_mgr = extension.ExtensionManager.make_test_instance(
[self.generic_hwm, self.mainline_hwm])
self.extension_mgr_patcher = mock.patch('stevedore.ExtensionManager')
self.mocked_extension_mgr = self.extension_mgr_patcher.start()

7
ironic_python_agent/tests/unit/test_utils.py
View File

@ -80,12 +80,11 @@ exit 1
fp = open(tmpfilename2, 'r')
runs = fp.read()
fp.close()
self.assertNotEqual(runs.strip(), 'failure', 'stdin did not '
'always get passed '
'correctly')
self.assertNotEqual(runs.strip(), 'failure',
'stdin did not always get passed correctly')
runs = int(runs.strip())
self.assertEqual(10, runs,
'Ran %d times instead of 10.' % (runs,))
'Ran %d times instead of 10.' % (runs,))
finally:
os.unlink(tmpfilename)
os.unlink(tmpfilename2)

4
ironic_python_agent/utils.py
View File

@ -220,8 +220,8 @@ def parse_root_device_hints():
error_msg = ('No device can be found because the following hints: '
'"%(not_supported)s" are not supported by this version '
'of IPA. Supported hints are: "%(supported)s"',
{'not_supported': ', '.join(not_supported),
'supported': ', '.join(SUPPORTED_ROOT_DEVICE_HINTS)})
{'not_supported': ', '.join(not_supported),
'supported': ', '.join(SUPPORTED_ROOT_DEVICE_HINTS)})
raise errors.DeviceNotFound(error_msg)
# Normalise the values

4
tox.ini
View File

@ -56,9 +56,7 @@ commands =
python setup.py build_sphinx
[flake8]
# E711: ignored because it is normal to use "column == None" in sqlalchemy
ignore = E12,E711
ignore = E129
exclude = .venv,.git,.tox,dist,doc,*openstack/common*,*lib/python*,*egg,build,*ironic/nova*,tools
[hacking]