Browse Source

Merge "New configuration parameter to use ipmitool retries" into stable/ussuri

changes/14/735614/1
Zuul 1 month ago
committed by Gerrit Code Review
parent
commit
79dbcb1746
4 changed files with 93 additions and 3 deletions
  1. +8
    -0
      ironic/conf/ipmi.py
  2. +12
    -3
      ironic/drivers/modules/ipmitool.py
  3. +57
    -0
      ironic/tests/unit/drivers/modules/test_ipmitool.py
  4. +16
    -0
      releasenotes/notes/ipmitool-use_ipmitool_retries-b55b2b8ed5cab603.yaml

+ 8
- 0
ironic/conf/ipmi.py View File

@@ -35,6 +35,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,
help=_('Kill `ipmitool` process invoked by ironic to read '


+ 12
- 3
ironic/drivers/modules/ipmitool.py 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'],


+ 57
- 0
ironic/tests/unit/drivers/modules/test_ipmitool.py View File

@@ -1041,6 +1041,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]


+ 16
- 0
releasenotes/notes/ipmitool-use_ipmitool_retries-b55b2b8ed5cab603.yaml 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.

Loading…
Cancel
Save