From 86d26e7cbe0f8318d69e2b63cf5b027bd0b856b0 Mon Sep 17 00:00:00 2001 From: Kaifeng Wang Date: Thu, 19 Jul 2018 15:12:40 +0800 Subject: [PATCH] Remove deprecated option [ipmi]retry_timeout The option [ipmi]retry_timeout is deprecated at Pike, now it's time to remove it from the tree. Change-Id: I921661db2a6f0c85e717e1a80e5f0c8b6c91d369 Story: #2003028 Task: #23052 --- ironic/conf/ipmi.py | 14 -------- ironic/drivers/modules/ipmitool.py | 10 ++---- .../unit/drivers/modules/test_ipmitool.py | 36 +++++++++---------- ...e-ipmi-retry-timeout-c1b2cf7df6771a43.yaml | 5 +++ 4 files changed, 25 insertions(+), 40 deletions(-) create mode 100644 releasenotes/notes/remove-ipmi-retry-timeout-c1b2cf7df6771a43.yaml diff --git a/ironic/conf/ipmi.py b/ironic/conf/ipmi.py index e957d8c79c..253cebd3d1 100644 --- a/ironic/conf/ipmi.py +++ b/ironic/conf/ipmi.py @@ -29,20 +29,6 @@ opts = [ '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. ' - '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, - deprecated_reason=_('Use option [ipmi]/command_retry_timeout ' - 'to specify the timeout value for ' - 'IPMI command retries, and use option ' - '[conductor]/power_state_change_timeout to ' - 'specify the timeout value for waiting for ' - 'a power operation to complete so that a ' - 'baremetal node reaches the desired ' - 'power state.')), cfg.IntOpt('min_command_interval', default=5, help=_('Minimum time, in seconds, between IPMI operations ' diff --git a/ironic/drivers/modules/ipmitool.py b/ironic/drivers/modules/ipmitool.py index 5c50ef82ea..1dfc13cb12 100644 --- a/ironic/drivers/modules/ipmitool.py +++ b/ironic/drivers/modules/ipmitool.py @@ -406,8 +406,7 @@ def _exec_ipmitool(driver_info, command, check_exit_code=None): args.append(option) args.append(driver_info[name]) - # TODO(sambetts) Remove usage of ipmi.retry_timeout in Queens - timeout = CONF.ipmi.retry_timeout or CONF.ipmi.command_retry_timeout + timeout = CONF.ipmi.command_retry_timeout # specify retry timing more precisely, if supported num_tries = max((timeout // CONF.ipmi.min_command_interval), 1) @@ -483,9 +482,6 @@ def _set_and_wait(task, power_action, driver_info, timeout=None): :returns: one of ironic.common.states """ - # TODO(sambetts) Remove usage of ipmi.retry_timeout in Queens - default_timeout = CONF.ipmi.retry_timeout - if power_action == states.POWER_ON: cmd_name = "on" target_state = states.POWER_ON @@ -495,9 +491,7 @@ def _set_and_wait(task, power_action, driver_info, timeout=None): elif power_action == states.SOFT_POWER_OFF: cmd_name = "soft" target_state = states.POWER_OFF - default_timeout = CONF.conductor.soft_power_off_timeout - - timeout = timeout or default_timeout + timeout = timeout or CONF.conductor.soft_power_off_timeout # NOTE(sambetts): Retries for ipmi power action failure will be handled by # the _exec_ipmitool function, so no need to wrap this call in its own diff --git a/ironic/tests/unit/drivers/modules/test_ipmitool.py b/ironic/tests/unit/drivers/modules/test_ipmitool.py index 009f87c404..dd5dd26f28 100644 --- a/ironic/tests/unit/drivers/modules/test_ipmitool.py +++ b/ironic/tests/unit/drivers/modules/test_ipmitool.py @@ -367,7 +367,7 @@ class IPMIToolPrivateMethodTestCaseMeta(type): # Directly set the configuration values such that # the logic will cause _exec_ipmitool to retry twice. self.config(min_command_interval=1, group='ipmi') - self.config(retry_timeout=2, group='ipmi') + self.config(command_retry_timeout=2, group='ipmi') ipmi._exec_ipmitool(self.info, 'A B C') @@ -388,7 +388,7 @@ class IPMIToolPrivateMethodTestCaseMeta(type): # Directly set the configuration values such that # the logic will cause _exec_ipmitool to timeout. self.config(min_command_interval=1, group='ipmi') - self.config(retry_timeout=1, group='ipmi') + self.config(command_retry_timeout=1, group='ipmi') self.assertRaises(processutils.ProcessExecutionError, ipmi._exec_ipmitool, @@ -419,7 +419,7 @@ class IPMIToolPrivateMethodTestCaseMeta(type): # the logic will cause _exec_ipmitool to retry up # to 3 times. self.config(min_command_interval=1, group='ipmi') - self.config(retry_timeout=3, group='ipmi') + self.config(command_retry_timeout=3, group='ipmi') self.assertRaises(processutils.ProcessExecutionError, ipmi._exec_ipmitool, @@ -1299,7 +1299,7 @@ class IPMIToolPrivateMethodTestCase(Base): @mock.patch.object(ipmi, '_exec_ipmitool', autospec=True) @mock.patch('eventlet.greenthread.sleep', autospec=True) def test__power_on_max_retries(self, sleep_mock, mock_exec): - self.config(retry_timeout=2, group='ipmi') + self.config(command_retry_timeout=2, group='ipmi') def side_effect(driver_info, command): resp_dict = {"power status": ["Chassis Power is off\n", None], @@ -1366,7 +1366,7 @@ class IPMIToolPrivateMethodTestCase(Base): self, sleep_mock, mock_exec, mock_status): # Check that if the call to power state change fails, it doesn't # call power_status(). - self.config(retry_timeout=2, group='ipmi') + self.config(command_retry_timeout=2, group='ipmi') mock_exec.side_effect = exception.IPMIFailure(cmd='power on') @@ -1431,7 +1431,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_power_off', autospec=True) def test_set_power_on_ok(self, mock_off, mock_on): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_on.return_value = states.POWER_ON with task_manager.acquire(self.context, @@ -1444,7 +1444,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_power_off', autospec=True) def test_set_power_on_timeout_ok(self, mock_off, mock_on): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_on.return_value = states.POWER_ON with task_manager.acquire(self.context, @@ -1459,7 +1459,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_off', autospec=True) def test_set_power_on_with_next_boot(self, mock_off, mock_on, mock_next_boot): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_on.return_value = states.POWER_ON with task_manager.acquire(self.context, @@ -1475,7 +1475,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_off', autospec=True) def test_set_power_on_with_next_boot_timeout(self, mock_off, mock_on, mock_next_boot): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_on.return_value = states.POWER_ON with task_manager.acquire(self.context, @@ -1489,7 +1489,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_power_off', autospec=True) def test_set_power_off_ok(self, mock_off, mock_on): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_off.return_value = states.POWER_OFF @@ -1503,7 +1503,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_power_off', autospec=True) def test_set_power_off_timeout_ok(self, mock_off, mock_on): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_off.return_value = states.POWER_OFF @@ -1517,7 +1517,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_soft_power_off', autospec=True) def test_set_soft_power_off_ok(self, mock_off, mock_on): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_off.return_value = states.POWER_OFF @@ -1531,7 +1531,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_soft_power_off', autospec=True) def test_set_soft_power_off_timeout_ok(self, mock_off, mock_on): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_off.return_value = states.POWER_OFF @@ -1546,7 +1546,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_soft_power_off', autospec=True) def test_set_soft_reboot_ok(self, mock_off, mock_on, mock_next_boot): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_off.return_value = states.POWER_OFF mock_on.return_value = states.POWER_ON @@ -1563,7 +1563,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_soft_power_off', autospec=True) def test_set_soft_reboot_timeout_ok(self, mock_off, mock_on, mock_next_boot): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_off.return_value = states.POWER_OFF mock_on.return_value = states.POWER_ON @@ -1580,7 +1580,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_soft_power_off', autospec=True) def test_set_soft_reboot_timeout_fail(self, mock_off, mock_on, mock_next_boot): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_off.side_effect = exception.PowerStateFailure( pstate=states.POWER_ON) @@ -1600,7 +1600,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_power_off', autospec=True) def test_set_power_on_fail(self, mock_off, mock_on): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_on.side_effect = exception.PowerStateFailure( pstate=states.POWER_ON) @@ -1617,7 +1617,7 @@ class IPMIToolDriverTestCase(Base): @mock.patch.object(ipmi, '_power_on', autospec=True) @mock.patch.object(ipmi, '_power_off', autospec=True) def test_set_power_on_timeout_fail(self, mock_off, mock_on): - self.config(retry_timeout=0, group='ipmi') + self.config(command_retry_timeout=0, group='ipmi') mock_on.side_effect = exception.PowerStateFailure(pstate=states.ERROR) with task_manager.acquire(self.context, diff --git a/releasenotes/notes/remove-ipmi-retry-timeout-c1b2cf7df6771a43.yaml b/releasenotes/notes/remove-ipmi-retry-timeout-c1b2cf7df6771a43.yaml new file mode 100644 index 0000000000..89f984a59f --- /dev/null +++ b/releasenotes/notes/remove-ipmi-retry-timeout-c1b2cf7df6771a43.yaml @@ -0,0 +1,5 @@ +--- +other: + - | + The deprecated configuration option ``[ipmi]retry_timeout`` was removed, + use ``[ipmi]command_retry_timeout`` instead.