Fix ipmitool timing argument calculation

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.

Change-Id: Ia3d8d85497651290c62341ac121e2aa438b4ac50
This commit is contained in:
Steve Baker 2020-10-06 13:31:47 +13:00
parent a44118c42c
commit 1de3db3b16
3 changed files with 75 additions and 14 deletions

View File

@ -443,6 +443,34 @@ def _get_ipmitool_args(driver_info, pw_file=None):
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,
kill_on_timeout=False):
"""Execute the ipmitool command.
@ -463,18 +491,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
timeout = CONF.ipmi.command_retry_timeout
# specify retry timing more precisely, if supported
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))
args.extend(_ipmitool_timing_args())
extra_args = {}
@ -486,6 +503,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None,
end_time = (time.time() + timeout)
num_tries = max((timeout // CONF.ipmi.min_command_interval), 1)
while True:
num_tries = num_tries - 1
# NOTE(tenbrae): ensure that no communications are sent to a BMC more

View File

@ -1050,7 +1050,7 @@ class IPMIToolPrivateMethodTestCase(
'-L', self.info['priv_level'],
'-U', self.info['username'],
'-v',
'-R', '12',
'-R', '7',
'-N', '5',
'-f', awesome_password_filename,
'A', 'B', 'C',
@ -1106,7 +1106,7 @@ class IPMIToolPrivateMethodTestCase(
'-L', self.info['priv_level'],
'-U', self.info['username'],
'-v',
'-R', '12',
'-R', '7',
'-N', '5',
'-f', awesome_password_filename,
'A', 'B', 'C',
@ -1572,6 +1572,40 @@ class IPMIToolPrivateMethodTestCase(
self.info)
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):

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.