openstack/ironic-python-agent
Code Issues Proposed changes

Merge "Use try_execute from ironic-lib"

Browse Source
This commit is contained in:
Zuul 2021-03-01 13:54:15 +00:00 committed by Gerrit Code Review
commit 4a22c887f8
3 changed files with 47 additions and 62 deletions
ironic_python_agent
hardware.py
tests/unit
test_hardware.py
utils.py

14
ironic_python_agent/hardware.py

@ -1127,7 +1127,7 @@ class GenericHardwareManager(HardwareManager):
freq = cpu_info.get('cpu max mhz', cpu_info.get('cpu mhz')) freq = cpu_info.get('cpu max mhz', cpu_info.get('cpu mhz'))
flags = [] flags = []
out = utils.try_execute('grep', '-Em1', '^flags', '/proc/cpuinfo') out = il_utils.try_execute('grep', '-Em1', '^flags', '/proc/cpuinfo')
if out: if out:
try: try:
# Example output (much longer for a real system): # Example output (much longer for a real system):
@ -1707,9 +1707,9 @@ class GenericHardwareManager(HardwareManager):
configured properly configured properly
""" """
# These modules are rarely loaded automatically # These modules are rarely loaded automatically
utils.try_execute('modprobe', 'ipmi_msghandler') il_utils.try_execute('modprobe', 'ipmi_msghandler')
utils.try_execute('modprobe', 'ipmi_devintf') il_utils.try_execute('modprobe', 'ipmi_devintf')
utils.try_execute('modprobe', 'ipmi_si') il_utils.try_execute('modprobe', 'ipmi_si')
try: try:
# From all the channels 0-15, only 1-11 can be assigned to # From all the channels 0-15, only 1-11 can be assigned to
@ -1750,9 +1750,9 @@ class GenericHardwareManager(HardwareManager):
interract with system tools or critical error occurs. interract with system tools or critical error occurs.
""" """
# These modules are rarely loaded automatically # These modules are rarely loaded automatically
utils.try_execute('modprobe', 'ipmi_msghandler') il_utils.try_execute('modprobe', 'ipmi_msghandler')
utils.try_execute('modprobe', 'ipmi_devintf') il_utils.try_execute('modprobe', 'ipmi_devintf')
utils.try_execute('modprobe', 'ipmi_si') il_utils.try_execute('modprobe', 'ipmi_si')
null_address_re = re.compile(r'^::(/\d{1,3})*$') null_address_re = re.compile(r'^::(/\d{1,3})*$')

75
ironic_python_agent/tests/unit/test_hardware.py

@ -20,6 +20,7 @@ import time
from unittest import mock from unittest import mock
from ironic_lib import disk_utils from ironic_lib import disk_utils
from ironic_lib import utils as il_utils
import netifaces import netifaces
from oslo_concurrency import processutils from oslo_concurrency import processutils
from oslo_config import cfg from oslo_config import cfg
@ -969,12 +970,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
'/sys/class/block/sdfake/device/vendor', 'r') '/sys/class/block/sdfake/device/vendor', 'r')
self.assertEqual('fake-vendor', vendor) self.assertEqual('fake-vendor', vendor)
@mock.patch.object(il_utils, 'execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_cpus(self, mocked_execute): def test_get_cpus(self, mocked_execute, mte):
mocked_execute.side_effect = [ mocked_execute.return_value = (hws.LSCPU_OUTPUT, '')
(hws.LSCPU_OUTPUT, ''), mte.return_value = (hws.CPUINFO_FLAGS_OUTPUT, '')
(hws.CPUINFO_FLAGS_OUTPUT, '')
]
cpus = self.hardware.get_cpus() cpus = self.hardware.get_cpus()
self.assertEqual('Intel(R) Xeon(R) CPU E5-2609 0 @ 2.40GHz', self.assertEqual('Intel(R) Xeon(R) CPU E5-2609 0 @ 2.40GHz',
@ -984,12 +984,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.assertEqual('x86_64', cpus.architecture) self.assertEqual('x86_64', cpus.architecture)
self.assertEqual(['fpu', 'vme', 'de', 'pse'], cpus.flags) self.assertEqual(['fpu', 'vme', 'de', 'pse'], cpus.flags)
@mock.patch.object(il_utils, 'execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_cpus2(self, mocked_execute): def test_get_cpus2(self, mocked_execute, mte):
mocked_execute.side_effect = [ mocked_execute.return_value = (hws.LSCPU_OUTPUT_NO_MAX_MHZ, '')
(hws.LSCPU_OUTPUT_NO_MAX_MHZ, ''), mte.return_value = (hws.CPUINFO_FLAGS_OUTPUT, '')
(hws.CPUINFO_FLAGS_OUTPUT, '')
]
cpus = self.hardware.get_cpus() cpus = self.hardware.get_cpus()
self.assertEqual('Intel(R) Xeon(R) CPU E5-1650 v3 @ 3.50GHz', self.assertEqual('Intel(R) Xeon(R) CPU E5-1650 v3 @ 3.50GHz',
@ -999,12 +998,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.assertEqual('x86_64', cpus.architecture) self.assertEqual('x86_64', cpus.architecture)
self.assertEqual(['fpu', 'vme', 'de', 'pse'], cpus.flags) self.assertEqual(['fpu', 'vme', 'de', 'pse'], cpus.flags)
@mock.patch.object(il_utils, 'execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_cpus_no_flags(self, mocked_execute): def test_get_cpus_no_flags(self, mocked_execute, mte):
mocked_execute.side_effect = [ mocked_execute.return_value = (hws.LSCPU_OUTPUT, '')
(hws.LSCPU_OUTPUT, ''), mte.side_effect = processutils.ProcessExecutionError()
processutils.ProcessExecutionError()
]
cpus = self.hardware.get_cpus() cpus = self.hardware.get_cpus()
self.assertEqual('Intel(R) Xeon(R) CPU E5-2609 0 @ 2.40GHz', self.assertEqual('Intel(R) Xeon(R) CPU E5-2609 0 @ 2.40GHz',
@ -1014,12 +1012,11 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.assertEqual('x86_64', cpus.architecture) self.assertEqual('x86_64', cpus.architecture)
self.assertEqual([], cpus.flags) self.assertEqual([], cpus.flags)
@mock.patch.object(il_utils, 'execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_cpus_illegal_flags(self, mocked_execute): def test_get_cpus_illegal_flags(self, mocked_execute, mte):
mocked_execute.side_effect = [ mocked_execute.return_value = (hws.LSCPU_OUTPUT, '')
(hws.LSCPU_OUTPUT, ''), mte.return_value = ('I am not a flag', '')
('I am not a flag', '')
]
cpus = self.hardware.get_cpus() cpus = self.hardware.get_cpus()
self.assertEqual('Intel(R) Xeon(R) CPU E5-2609 0 @ 2.40GHz', self.assertEqual('Intel(R) Xeon(R) CPU E5-2609 0 @ 2.40GHz',
@ -2190,35 +2187,41 @@ class TestGenericHardwareManager(base.IronicAgentTest):
mock_open.assert_called_once_with( mock_open.assert_called_once_with(
'/sys/block/sdfake/ro', 'r') '/sys/block/sdfake/ro', 'r')
@mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_address(self, mocked_execute): def test_get_bmc_address(self, mocked_execute, mte):
mocked_execute.return_value = '192.1.2.3\n', '' mocked_execute.return_value = '192.1.2.3\n', ''
self.assertEqual('192.1.2.3', self.hardware.get_bmc_address()) self.assertEqual('192.1.2.3', self.hardware.get_bmc_address())
@mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_address_virt(self, mocked_execute): def test_get_bmc_address_virt(self, mocked_execute, mte):
mocked_execute.side_effect = processutils.ProcessExecutionError() mocked_execute.side_effect = processutils.ProcessExecutionError()
self.assertIsNone(self.hardware.get_bmc_address()) self.assertIsNone(self.hardware.get_bmc_address())
@mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_address_zeroed(self, mocked_execute): def test_get_bmc_address_zeroed(self, mocked_execute, mte):
mocked_execute.return_value = '0.0.0.0\n', '' mocked_execute.return_value = '0.0.0.0\n', ''
self.assertEqual('0.0.0.0', self.hardware.get_bmc_address()) self.assertEqual('0.0.0.0', self.hardware.get_bmc_address())
@mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_address_invalid(self, mocked_execute): def test_get_bmc_address_invalid(self, mocked_execute, mte):
# In case of invalid lan channel, stdout is empty and the error # In case of invalid lan channel, stdout is empty and the error
# on stderr is "Invalid channel" # on stderr is "Invalid channel"
mocked_execute.return_value = '\n', 'Invalid channel: 55' mocked_execute.return_value = '\n', 'Invalid channel: 55'
self.assertEqual('0.0.0.0', self.hardware.get_bmc_address()) self.assertEqual('0.0.0.0', self.hardware.get_bmc_address())
@mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_address_random_error(self, mocked_execute): def test_get_bmc_address_random_error(self, mocked_execute, mte):
mocked_execute.return_value = '192.1.2.3\n', 'Random error message' mocked_execute.return_value = '192.1.2.3\n', 'Random error message'
self.assertEqual('192.1.2.3', self.hardware.get_bmc_address()) self.assertEqual('192.1.2.3', self.hardware.get_bmc_address())
@mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_address_iterate_channels(self, mocked_execute): def test_get_bmc_address_iterate_channels(self, mocked_execute, mte):
# For channel 1 we simulate unconfigured IP # For channel 1 we simulate unconfigured IP
# and for any other we return a correct IP address # and for any other we return a correct IP address
def side_effect(*args, **kwargs): def side_effect(*args, **kwargs):
@ -2233,18 +2236,19 @@ class TestGenericHardwareManager(base.IronicAgentTest):
mocked_execute.side_effect = side_effect mocked_execute.side_effect = side_effect
self.assertEqual('192.1.2.3', self.hardware.get_bmc_address()) self.assertEqual('192.1.2.3', self.hardware.get_bmc_address())
@mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_address_not_available(self, mocked_execute): def test_get_bmc_address_not_available(self, mocked_execute, mte):
mocked_execute.return_value = '', '' mocked_execute.return_value = '', ''
self.assertEqual('0.0.0.0', self.hardware.get_bmc_address()) self.assertEqual('0.0.0.0', self.hardware.get_bmc_address())
@mock.patch.object(utils, 'try_execute', autospec=True) @mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_v6address_not_enabled(self, mocked_execute, mte): def test_get_bmc_v6address_not_enabled(self, mocked_execute, mte):
mocked_execute.side_effect = [('ipv4\n', '')] * 11 mocked_execute.side_effect = [('ipv4\n', '')] * 11
self.assertEqual('::/0', self.hardware.get_bmc_v6address()) self.assertEqual('::/0', self.hardware.get_bmc_v6address())
@mock.patch.object(utils, 'try_execute', autospec=True) @mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_v6address_dynamic_address(self, mocked_execute, mte): def test_get_bmc_v6address_dynamic_address(self, mocked_execute, mte):
mocked_execute.side_effect = [ mocked_execute.side_effect = [
@ -2254,7 +2258,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.assertEqual('2001:1234:1234:1234:1234:1234:1234:1234', self.assertEqual('2001:1234:1234:1234:1234:1234:1234:1234',
self.hardware.get_bmc_v6address()) self.hardware.get_bmc_v6address())
@mock.patch.object(utils, 'try_execute', autospec=True) @mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_v6address_static_address_both(self, mocked_execute, mte): def test_get_bmc_v6address_static_address_both(self, mocked_execute, mte):
dynamic_disabled = \ dynamic_disabled = \
@ -2267,12 +2271,13 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.assertEqual('2001:5678:5678:5678:5678:5678:5678:5678', self.assertEqual('2001:5678:5678:5678:5678:5678:5678:5678',
self.hardware.get_bmc_v6address()) self.hardware.get_bmc_v6address())
@mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_v6address_virt(self, mocked_execute): def test_get_bmc_v6address_virt(self, mocked_execute, mte):
mocked_execute.side_effect = processutils.ProcessExecutionError() mocked_execute.side_effect = processutils.ProcessExecutionError()
self.assertIsNone(self.hardware.get_bmc_v6address()) self.assertIsNone(self.hardware.get_bmc_v6address())
@mock.patch.object(utils, 'try_execute', autospec=True) @mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_v6address_invalid_enables(self, mocked_execute, mte): def test_get_bmc_v6address_invalid_enables(self, mocked_execute, mte):
def side_effect(*args, **kwargs): def side_effect(*args, **kwargs):
@ -2282,7 +2287,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
mocked_execute.side_effect = side_effect mocked_execute.side_effect = side_effect
self.assertEqual('::/0', self.hardware.get_bmc_v6address()) self.assertEqual('::/0', self.hardware.get_bmc_v6address())
@mock.patch.object(utils, 'try_execute', autospec=True) @mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_v6address_invalid_get_address(self, mocked_execute, mte): def test_get_bmc_v6address_invalid_get_address(self, mocked_execute, mte):
def side_effect(*args, **kwargs): def side_effect(*args, **kwargs):
@ -2296,7 +2301,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
self.assertEqual('::/0', self.hardware.get_bmc_v6address()) self.assertEqual('::/0', self.hardware.get_bmc_v6address())
@mock.patch.object(hardware, 'LOG', autospec=True) @mock.patch.object(hardware, 'LOG', autospec=True)
@mock.patch.object(utils, 'try_execute', autospec=True) @mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_v6address_ipmitool_invalid_stdout_format( def test_get_bmc_v6address_ipmitool_invalid_stdout_format(
self, mocked_execute, mte, mocked_log): self, mocked_execute, mte, mocked_log):
@ -2313,7 +2318,7 @@ class TestGenericHardwareManager(base.IronicAgentTest):
'command: %(e)s', mock.ANY) 'command: %(e)s', mock.ANY)
mocked_log.warning.assert_has_calls([one_call] * 14) mocked_log.warning.assert_has_calls([one_call] * 14)
@mock.patch.object(utils, 'try_execute', autospec=True) @mock.patch.object(il_utils, 'try_execute', autospec=True)
@mock.patch.object(utils, 'execute', autospec=True) @mock.patch.object(utils, 'execute', autospec=True)
def test_get_bmc_v6address_channel_7(self, mocked_execute, mte): def test_get_bmc_v6address_channel_7(self, mocked_execute, mte):
def side_effect(*args, **kwargs): def side_effect(*args, **kwargs):

20
ironic_python_agent/utils.py

@ -88,26 +88,6 @@ def execute(*cmd, **kwargs):
return ironic_utils.execute(*cmd, **kwargs) return ironic_utils.execute(*cmd, **kwargs)
def try_execute(*cmd, **kwargs):
"""The same as execute but returns None on error.
Executes and logs results from a system command. See docs for
oslo_concurrency.processutils.execute for usage.
Instead of raising an exception on failure, this method simply
returns None in case of failure.
:param cmd: positional arguments to pass to processutils.execute()
:param kwargs: keyword arguments to pass to processutils.execute()
:raises: UnknownArgumentError on receiving unknown arguments
:returns: tuple of (stdout, stderr) or None in some error cases
"""
try:
return execute(*cmd, **kwargs)
except (processutils.ProcessExecutionError, OSError) as e:
LOG.debug('Command failed: %s', e)
def _read_params_from_file(filepath): def _read_params_from_file(filepath):
"""Extract key=value pairs from a file. """Extract key=value pairs from a file.