openstack/ironic
Code Issues Proposed changes

Update comments related to ipmi & old BMCs

Browse Source 
The help string for [ipmi]/command_retry_timeout is incorrect. From Sam Betts [1]:
so looking at when this option was introduced ... it was originally introduced
by the ipminative driver which used it as the timeout for waiting until the power
state changed to the requested state. Reading between the lines in the commits
that added this option and documentation, the "setting this too low might break
things" comment originates from bugs in IPMInative. Later on the IPMItool driver
then overloaded this configuration option and changed its meaning by introducing
the min_command_interval config option. It continued to be used for the timeout
for waiting for requested state, but it also now contributed to the the number
of times a IPMItool command would retry retryable errors, which was/is worked out
by dividing retry_timeout by min_command_interval. Now IPMInative no longer in
tree I think we should clean up some of this information.

[1] https://review.openstack.org/#/c/482631/

Change-Id: I8cd8e25a2fb224d477799a2e561573406f9427a9
This commit is contained in:
Ruby Loo 2017-07-13 10:37:27 -04:00
parent 578f01678c
commit 3f345c4a37
3 changed files with 11 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
doc/source/install/configure-ipmi.rst
View File

@ -48,8 +48,8 @@ IPMI configuration
~~~~~~~~~~~~~~~~~~
If there are slow or unresponsive BMCs in the environment, the
``command_retry_timeout`` configuration option in the ``[ipmi]`` section may
need to be lowered. The default is fairly conservative, as setting this timeout
``min_command_interval`` configuration option in the ``[ipmi]`` section may
need to be raised. The default is fairly conservative, as setting this timeout
too low can cause older BMCs to crash and require a hard-reset.
Collecting sensor data

17
etc/ironic/ironic.conf.sample
View File

@ -1903,20 +1903,15 @@
# Maximum time in seconds to retry retryable IPMI operations.
# (An operation is retryable, for example, if the requested
# operation fails because the BMC is busy.) There is a
# tradeoff when setting this value. Setting this too low may
# cause older BMCs to crash and require a hard reset. However,
# setting too high can cause the sync power state periodic
# task to hang when there are slow or unresponsive BMCs.
# (integer value)
# operation fails because the BMC is busy.) Setting this too
# high can cause the sync power state periodic task to hang
# when there are slow or unresponsive BMCs. (integer value)
#command_retry_timeout = 60
# DEPRECATED: Maximum time in seconds to retry IPMI
# operations. There is a tradeoff when setting this value.
# Setting this too low may cause older BMCs to crash and
# require a hard reset. However, setting too high can cause
# the sync power state periodic task to hang when there are
# slow or unresponsive BMCs. (integer value)
# operations. Setting this too high can cause the sync power
# state periodic task to hang when there are slow or
# unresponsive BMCs. (integer value)
# This option is deprecated for removal.
# Its value may be silently ignored in the future.
# Reason: Use option [ipmi]/command_retry_timeout to specify

10
ironic/conf/ipmi.py
View File

@ -25,17 +25,13 @@ opts = [
help=_('Maximum time in seconds to retry retryable IPMI '
'operations. (An operation is retryable, for '
'example, if the requested operation fails '
'because the BMC is busy.) There is a tradeoff when '
'setting this value. Setting this too low may cause '
'older BMCs to crash and require a hard reset. However, '
'setting too high can cause the sync power state '
'because the BMC is busy.) Setting this too high '
'can cause the sync power state '
'periodic task to hang when there are slow or '
'unresponsive BMCs.')),
cfg.IntOpt('retry_timeout',
help=_('Maximum time in seconds to retry IPMI operations. '
'There is a tradeoff when setting this value. Setting '
'this too low may cause older BMCs to crash and require '
'a hard reset. However, setting too high can cause the '
'Setting this too high can cause the '
'sync power state periodic task to hang when there are '
'slow or unresponsive BMCs.'),
deprecated_for_removal=True,