Merge "New configuration parameter to use ipmitool retries"

This commit is contained in:
Zuul 2020-05-28 22:08:51 +00:00 committed by Gerrit Code Review
commit 707a97270f
4 changed files with 93 additions and 3 deletions

View File

@ -37,6 +37,14 @@ opts = [
'sent to a server. There is a risk with some hardware '
'that setting this too low may cause the BMC to crash. '
'Recommended setting is 5 seconds.')),
cfg.BoolOpt('use_ipmitool_retries',
default=True,
help=_('When set to True and the parameters are supported by '
'ipmitool, the number of retries and the retry '
'interval are passed to ipmitool as parameters, and '
'ipmitool will do the retries. When set to False, '
'ironic will retry the ipmitool commands. '
'Recommended setting is True')),
cfg.BoolOpt('kill_on_timeout',
default=True,
mutable=True,

View File

@ -480,10 +480,16 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
if _is_option_supported('timing'):
args.append('-R')
args.append(str(num_tries))
if CONF.ipmi.use_ipmitool_retries:
args.append(str(num_tries))
else:
args.append('1')
args.append('-N')
args.append(str(CONF.ipmi.min_command_interval))
if CONF.ipmi.use_ipmitool_retries:
args.append(str(CONF.ipmi.min_command_interval))
else:
args.append('1')
extra_args = {}
@ -530,9 +536,12 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
IPMITOOL_RETRYABLE_FAILURES
+ CONF.ipmi.additional_retryable_ipmi_errors)
if x in str(e)]
# If Ironic is doing retries then retry all errors
retry_failures = (err_list
or not CONF.ipmi.use_ipmitool_retries)
if ((time.time() > end_time)
or (num_tries == 0)
or not err_list):
or not retry_failures):
LOG.error('IPMI Error while attempting "%(cmd)s" '
'for node %(node)s. Error: %(error)s',
{'node': driver_info['uuid'],

View File

@ -1042,6 +1042,63 @@ class IPMIToolPrivateMethodTestCase(
mock_support.assert_called_once_with('timing')
mock_exec.assert_called_once_with(*args)
@mock.patch.object(ipmi, '_is_option_supported', autospec=True)
@mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub)
@mock.patch.object(utils, 'execute', autospec=True)
def test__exec_ipmitool_with_ironic_retries(
self, mock_exec, mock_support):
args = [
'ipmitool',
'-I', 'lanplus',
'-H', self.info['address'],
'-L', self.info['priv_level'],
'-U', self.info['username'],
'-v',
'-R', '1',
'-N', '1',
'-f', awesome_password_filename,
'A', 'B', 'C',
]
mock_support.return_value = True
mock_exec.return_value = (None, None)
self.config(use_ipmitool_retries=False, group='ipmi')
ipmi._exec_ipmitool(self.info, 'A B C')
mock_support.assert_called_once_with('timing')
mock_exec.assert_called_once_with(*args)
@mock.patch.object(ipmi, '_is_option_supported', autospec=True)
@mock.patch.object(ipmi, '_make_password_file', _make_password_file_stub)
@mock.patch.object(utils, 'execute', autospec=True)
def test__exec_ipmitool_with_ironic_retries_multiple(
self, mock_exec, mock_support):
mock_exec.side_effect = [
processutils.ProcessExecutionError(
stderr="Unknown"
),
processutils.ProcessExecutionError(
stderr="Unknown"
),
processutils.ProcessExecutionError(
stderr="Unknown"
),
]
self.config(min_command_interval=1, group='ipmi')
self.config(command_retry_timeout=3, group='ipmi')
self.config(use_ipmitool_retries=False, group='ipmi')
self.assertRaises(processutils.ProcessExecutionError,
ipmi._exec_ipmitool,
self.info, 'A B C')
mock_support.assert_called_once_with('timing')
self.assertEqual(3, mock_exec.call_count)
def test__exec_ipmitool_wait(self):
mock_popen = mock.MagicMock()
mock_popen.poll.side_effect = [1, 1, 1, 1, 1]

View File

@ -0,0 +1,16 @@
---
features:
- |
Adds a new ``[ipmi]use_ipmitool_retries`` option. When set to
``True`` and timing is supported by ipmitool, the number of
retries and command interval will be passed to ipmitool so
that ipmitool will do the retries. When set to ``False``,
ironic will do the retries. Default is ``True``.
issues:
- |
Some BMCs do not support the ``Channel Cipher Suites`` command
that newer versions of ipmitool use. These versions of
ipmitool will resend this command for each ipmitool retry,
resulting in long response times. Setting
``[ipmi]use_ipmitool_retries`` to ``false`` will avoid this
situation by implementing retries on the ironic level.