Merge "New configuration parameter to use ipmitool retries" into stable/train
This commit is contained in:
commit
61759fe162
|
@ -35,6 +35,14 @@ opts = [
|
||||||
'sent to a server. There is a risk with some hardware '
|
'sent to a server. There is a risk with some hardware '
|
||||||
'that setting this too low may cause the BMC to crash. '
|
'that setting this too low may cause the BMC to crash. '
|
||||||
'Recommended setting is 5 seconds.')),
|
'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',
|
cfg.BoolOpt('kill_on_timeout',
|
||||||
default=True,
|
default=True,
|
||||||
help=_('Kill `ipmitool` process invoked by ironic to read '
|
help=_('Kill `ipmitool` process invoked by ironic to read '
|
||||||
|
|
|
@ -479,7 +479,10 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
|
||||||
|
|
||||||
if _is_option_supported('timing'):
|
if _is_option_supported('timing'):
|
||||||
args.append('-R')
|
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('-N')
|
||||||
args.append(str(CONF.ipmi.min_command_interval))
|
args.append(str(CONF.ipmi.min_command_interval))
|
||||||
|
@ -529,9 +532,12 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
|
||||||
IPMITOOL_RETRYABLE_FAILURES +
|
IPMITOOL_RETRYABLE_FAILURES +
|
||||||
CONF.ipmi.additional_retryable_ipmi_errors)
|
CONF.ipmi.additional_retryable_ipmi_errors)
|
||||||
if x in six.text_type(e)]
|
if x in six.text_type(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)
|
if ((time.time() > end_time)
|
||||||
or (num_tries == 0)
|
or (num_tries == 0)
|
||||||
or not err_list):
|
or not retry_failures):
|
||||||
LOG.error('IPMI Error while attempting "%(cmd)s" '
|
LOG.error('IPMI Error while attempting "%(cmd)s" '
|
||||||
'for node %(node)s. Error: %(error)s',
|
'for node %(node)s. Error: %(error)s',
|
||||||
{'node': driver_info['uuid'],
|
{'node': driver_info['uuid'],
|
||||||
|
|
|
@ -1026,6 +1026,63 @@ class IPMIToolPrivateMethodTestCase(Base):
|
||||||
mock_support.assert_called_once_with('timing')
|
mock_support.assert_called_once_with('timing')
|
||||||
mock_exec.assert_called_once_with(*args)
|
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', '5',
|
||||||
|
'-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):
|
def test__exec_ipmitool_wait(self):
|
||||||
mock_popen = mock.MagicMock()
|
mock_popen = mock.MagicMock()
|
||||||
mock_popen.poll.side_effect = [1, 1, 1, 1, 1]
|
mock_popen.poll.side_effect = [1, 1, 1, 1, 1]
|
||||||
|
|
|
@ -0,0 +1,6 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
When Ironic is doing IPMI retries the configured ``min_command_interval``
|
||||||
|
should be used instead of a default value of ``1``, which may be too short
|
||||||
|
for some BMCs.
|
|
@ -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.
|
Loading…
Reference in New Issue