Merge "Fix ipmitool timing argument calculation" into stable/victoria

This commit is contained in:
Zuul 2021-04-15 10:29:43 +00:00 committed by Gerrit Code Review
commit 342c43aaf8
3 changed files with 75 additions and 14 deletions

View File

@ -487,6 +487,34 @@ def _get_ipmitool_args(driver_info, pw_file=None):
return args return args
def _ipmitool_timing_args():
if not _is_option_supported('timing'):
return []
if not CONF.ipmi.use_ipmitool_retries:
return [
'-R', '1',
'-N', str(CONF.ipmi.min_command_interval)
]
timeout = CONF.ipmi.command_retry_timeout
interval = CONF.ipmi.min_command_interval
num_tries = 0
cmd_duration = 0
while cmd_duration + interval <= timeout:
cmd_duration += interval
# for each attempt, ipmitool adds one second to the previous
# interval value
interval += 1
num_tries += 1
return [
'-R', str(num_tries),
'-N', str(CONF.ipmi.min_command_interval)
]
def _exec_ipmitool(driver_info, command, check_exit_code=None, def _exec_ipmitool(driver_info, command, check_exit_code=None,
kill_on_timeout=False): kill_on_timeout=False):
"""Execute the ipmitool command. """Execute the ipmitool command.
@ -507,18 +535,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
timeout = CONF.ipmi.command_retry_timeout timeout = CONF.ipmi.command_retry_timeout
# specify retry timing more precisely, if supported args.extend(_ipmitool_timing_args())
num_tries = max((timeout // CONF.ipmi.min_command_interval), 1)
if _is_option_supported('timing'):
args.append('-R')
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))
extra_args = {} extra_args = {}
@ -530,6 +547,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
end_time = (time.time() + timeout) end_time = (time.time() + timeout)
num_tries = max((timeout // CONF.ipmi.min_command_interval), 1)
while True: while True:
num_tries = num_tries - 1 num_tries = num_tries - 1
# NOTE(tenbrae): ensure that no communications are sent to a BMC more # NOTE(tenbrae): ensure that no communications are sent to a BMC more

View File

@ -1075,7 +1075,7 @@ class IPMIToolPrivateMethodTestCase(
'-L', self.info['priv_level'], '-L', self.info['priv_level'],
'-U', self.info['username'], '-U', self.info['username'],
'-v', '-v',
'-R', '12', '-R', '7',
'-N', '5', '-N', '5',
'-f', awesome_password_filename, '-f', awesome_password_filename,
'A', 'B', 'C', 'A', 'B', 'C',
@ -1131,7 +1131,7 @@ class IPMIToolPrivateMethodTestCase(
'-L', self.info['priv_level'], '-L', self.info['priv_level'],
'-U', self.info['username'], '-U', self.info['username'],
'-v', '-v',
'-R', '12', '-R', '7',
'-N', '5', '-N', '5',
'-f', awesome_password_filename, '-f', awesome_password_filename,
'A', 'B', 'C', 'A', 'B', 'C',
@ -1598,6 +1598,40 @@ class IPMIToolPrivateMethodTestCase(
self.info) self.info)
self.assertFalse(mock_status.called) self.assertFalse(mock_status.called)
@mock.patch.object(ipmi, '_is_option_supported', autospec=True)
def test__ipmitool_timing_args(self, mock_support):
# timing arguments not supported
mock_support.return_value = False
self.assertEqual([], ipmi._ipmitool_timing_args())
# handle retries by calling ipmitool repeatedly
mock_support.return_value = True
self.config(use_ipmitool_retries=False, group='ipmi')
self.config(min_command_interval=5, group='ipmi')
self.assertEqual(['-R', '1', '-N', '5'], ipmi._ipmitool_timing_args())
# confirm that different combinations of min_command_interval and
# command_retry_timeout result in the command finishing before
# command_retry_timeout
self.config(use_ipmitool_retries=True, group='ipmi')
# 7 retries, first interval is 5s
# 5 + 6 + 7 + 8 + 9 + 10 + 11 = 56s
self.config(command_retry_timeout=60, group='ipmi')
self.assertEqual(['-R', '7', '-N', '5'], ipmi._ipmitool_timing_args())
# 11 retries, first interval is 5s
# 5 + 6 + 7 + 8 + 9 + 10 + 11 + 12 + 13 + 14 + 15 = 110s
self.config(command_retry_timeout=120, group='ipmi')
self.config(min_command_interval=5, group='ipmi')
self.assertEqual(['-R', '11', '-N', '5'], ipmi._ipmitool_timing_args())
# 7 retries, first interval is 1s
# 1 + 2 + 3 + 4 + 5 + 6 + 7 = 28s
self.config(command_retry_timeout=30, group='ipmi')
self.config(min_command_interval=1, group='ipmi')
self.assertEqual(['-R', '7', '-N', '1'], ipmi._ipmitool_timing_args())
class IPMIToolDriverTestCase(Base): class IPMIToolDriverTestCase(Base):

View File

@ -0,0 +1,9 @@
---
fixes:
- |
Calculating the ipmitool `-N` and `-R` arguments from ironic.conf [ipmi]
`command_retry_timeout` and `min_command_interval` now takes into account the
1 second interval increment that ipmitool adds on each retry event.
Failure-path ipmitool run duration will now be just less than
`command_retry_timeout` instead of much longer.