From 00c052dd0289a563e081e33a93e28c1e8025a999 Mon Sep 17 00:00:00 2001 From: Steve Baker Date: Fri, 23 Jan 2026 09:58:03 +1300 Subject: [PATCH 1/2] Speed the slowest tests Below is an old output of the slowest tests. The redfish power tests are made faster by having a sleep mocking strategy for the BackOffLoopingCall, and the snmp tests are sped up by reducing the config timeout. The slowest tests now take 1 to 4 seconds which are likely all sleep related. ironic.tests.unit.drivers.modules.redfish.test_power.RedfishPowerTestCase.test_reboot_not_reached 43.002 ironic.tests.unit.drivers.modules.redfish.test_power.RedfishPowerTestCase.test_set_power_state 22.958 ironic.tests.unit.drivers.modules.redfish.test_power.RedfishPowerTestCase.test_set_power_state_not_reached 12.134 ironic.tests.unit.drivers.modules.test_snmp.SNMPDeviceDriverTestCase.test_power_reset_on_invalid_state 9.045 ironic.tests.unit.drivers.modules.test_snmp.SNMPDeviceDriverTestCase.test_power_off_invalid_state 9.044 ironic.tests.unit.drivers.modules.test_snmp.SNMPDeviceDriverTestCase.test_power_reset_off_invalid_state 9.043 ironic.tests.unit.drivers.modules.test_snmp.SNMPDeviceDriverTestCase.test_power_on_invalid_state 9.043 ironic.tests.unit.drivers.modules.test_snmp.SNMPDeviceDriverTestCase.test_power_reset_off_timeout 9.042 ironic.tests.unit.drivers.modules.test_snmp.SNMPDeviceDriverTestCase.test_power_on_timeout 9.042 ironic.tests.unit.drivers.modules.test_snmp.SNMPDeviceDriverTestCase.test_power_reset_on_timeout 9.042 Change-Id: I19bdfc6bced9b98f14a26d2b71024e84f410368a Signed-off-by: Steve Baker (cherry picked from commit b13db575017776ce51546d0213c817e0771a250f) --- .../drivers/modules/redfish/test_power.py | 34 ++++++++++++++----- .../tests/unit/drivers/modules/test_snmp.py | 1 + 2 files changed, 27 insertions(+), 8 deletions(-) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_power.py b/ironic/tests/unit/drivers/modules/redfish/test_power.py index 81fc6b1165..0e6d197be2 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_power.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_power.py @@ -13,8 +13,10 @@ # License for the specific language governing permissions and limitations # under the License. +import time from unittest import mock +from oslo_service import loopingcall as lc import sushy from ironic.common import exception @@ -74,12 +76,13 @@ def test_get_power_state(self, mock_get_system): mock_get_system.assert_called_once_with(task.node) mock_get_system.reset_mock() + @mock.patch.object(lc.BackOffLoopingCall, '_sleep', autospec=True) @mock.patch('time.sleep', autospec=True) @mock.patch.object(redfish_mgmt.RedfishManagement, 'restore_boot_device', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_set_power_state(self, mock_get_system, mock_restore_bootdev, - mock_sleep): + mock_sleep, mock_lc_sleep): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: expected_values = [ @@ -126,9 +129,11 @@ def test_set_power_state(self, mock_get_system, mock_restore_bootdev, mock_restore_bootdev.reset_mock() mock_sleep.reset_mock() + @mock.patch.object(lc.BackOffLoopingCall, '_sleep', autospec=True) @mock.patch('time.sleep', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_set_power_state_not_reached(self, mock_get_system, mock_sleep): + def test_set_power_state_not_reached(self, mock_get_system, mock_sleep, + mock_lc_sleep): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: self.config(power_state_change_timeout=2, group='conductor') @@ -361,11 +366,12 @@ def test_set_power_state_conflict_error_refresh_fails( log_msg = mock_log.call_args[0][0] self.assertIn('Failed to refresh system state', log_msg) + @mock.patch.object(lc.BackOffLoopingCall, '_sleep', autospec=True) @mock.patch.object(redfish_mgmt.RedfishManagement, 'restore_boot_device', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_reboot_from_power_off(self, mock_get_system, - mock_restore_bootdev): + mock_restore_bootdev, mock_lc_sleep): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: system_result = [ @@ -388,11 +394,12 @@ def test_reboot_from_power_off(self, mock_get_system, mock_restore_bootdev.assert_called_once_with( task.driver.management, task, system_result[0]) + @mock.patch.object(lc.BackOffLoopingCall, '_sleep', autospec=True) @mock.patch.object(redfish_mgmt.RedfishManagement, 'restore_boot_device', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_reboot_from_power_off_with_disable_power_off( - self, mock_get_system, mock_restore_bootdev): + self, mock_get_system, mock_restore_bootdev, mock_lc_sleep): # NOTE(dtantsur): if a node with disable_power_off is powered off, we # probably cannot do anything about it. This unit test is only here # for consistent coverage. @@ -419,10 +426,12 @@ def test_reboot_from_power_off_with_disable_power_off( mock_restore_bootdev.assert_called_once_with( task.driver.management, task, system_result[0]) + @mock.patch.object(lc.BackOffLoopingCall, '_sleep', autospec=True) @mock.patch.object(redfish_mgmt.RedfishManagement, 'restore_boot_device', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_reboot_from_power_on(self, mock_get_system, mock_restore_bootdev): + def test_reboot_from_power_on(self, mock_get_system, mock_restore_bootdev, + mock_lc_sleep): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: system_result = [ @@ -447,12 +456,14 @@ def test_reboot_from_power_on(self, mock_get_system, mock_restore_bootdev): mock_restore_bootdev.assert_called_once_with( task.driver.management, task, system_result[0]) + @mock.patch.object(lc.BackOffLoopingCall, '_sleep', autospec=True) @mock.patch('time.sleep', autospec=True) @mock.patch.object(redfish_mgmt.RedfishManagement, 'restore_boot_device', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_reboot_from_power_on_with_disable_power_off( - self, mock_get_system, mock_restore_bootdev, mock_sleep): + self, mock_get_system, mock_restore_bootdev, mock_sleep, + mock_lc_sleep): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.disable_power_off = True @@ -477,10 +488,14 @@ def test_reboot_from_power_on_with_disable_power_off( task.driver.management, task, system_result[0]) mock_sleep.assert_called_with(15) + @mock.patch.object(lc.BackOffLoopingCall, '_sleep', autospec=True) + @mock.patch.object(time, 'sleep', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_reboot_not_reached(self, mock_get_system): + def test_reboot_not_reached(self, mock_get_system, mock_sleep, + mock_lc_sleep): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + self.config(power_state_change_timeout=2, group='conductor') fake_system = mock_get_system.return_value fake_system.power_state = sushy.SYSTEM_POWER_STATE_OFF @@ -490,6 +505,7 @@ def test_reboot_not_reached(self, mock_get_system): # Asserts fake_system.reset_system.assert_called_once_with(sushy.RESET_ON) mock_get_system.assert_called_with(task.node) + mock_sleep.assert_called_with(0) @mock.patch.object(sushy, 'Sushy', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) @@ -508,9 +524,11 @@ def test_reboot_fail(self, mock_get_system, mock_sushy): sushy.RESET_FORCE_OFF) mock_get_system.assert_called_once_with(task.node) + @mock.patch.object(lc.BackOffLoopingCall, '_sleep', autospec=True) @mock.patch.object(sushy, 'Sushy', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_reboot_fail_on_power_on(self, mock_get_system, mock_sushy): + def test_reboot_fail_on_power_on(self, mock_get_system, mock_sushy, + mock_lc_sleep): system_result = [ # Initial state mock.Mock(power_state=sushy.SYSTEM_POWER_STATE_ON), diff --git a/ironic/tests/unit/drivers/modules/test_snmp.py b/ironic/tests/unit/drivers/modules/test_snmp.py index 7d89690c9b..831cac285f 100644 --- a/ironic/tests/unit/drivers/modules/test_snmp.py +++ b/ironic/tests/unit/drivers/modules/test_snmp.py @@ -736,6 +736,7 @@ class SNMPDeviceDriverTestCase(db_base.DbTestCase): def setUp(self): super(SNMPDeviceDriverTestCase, self).setUp() self.config(enabled_power_interfaces=['fake', 'snmp']) + self.config(power_timeout=2, group='snmp') snmp._memoized = {} self.node = obj_utils.get_test_node( self.context, From 9e91430a5b982591f8b3684742cc1743d2b03b72 Mon Sep 17 00:00:00 2001 From: Jay Jahns Date: Thu, 6 Aug 2026 17:54:28 -0500 Subject: [PATCH 2/2] Redfish: retry transient 409 conflict on power-on Some BMCs (observed on Dell iDRAC10 / PowerEdge 17G) transiently reject the Redfish "On" reset type with HTTP 409 ActionParameterValueConflict for a short window (up to ~60 seconds) right after a power-off, until the BMC settles. During a deploy, boot_instance issues the power-on only a few seconds after tear_down_agent powers the node off, so the reset lands inside that window and the deploy fails with: Deploy step deploy.boot_instance failed: ... ComputerSystem.Reset returned code 409 ... ActionParameterValueConflict ... the parameter 'ResetType' with the requested value of 'On' does not meet the constraints of the implementation. The value itself is correct -- the same "On" succeeds once the BMC has settled, confirmed by issuing it manually -- and such BMCs may not implement "ForceOn", so switching the reset type is not an option. The existing 400/409 handler in RedfishPower.set_power_state only treats a conflict as success when the node is already in the target state; a transient "not settled yet" conflict is re-raised and fails the deploy. Retry the power-on in _set_power_state when the BMC returns HTTP 409 ActionParameterValueConflict and the node is not yet powered on, until the BMC accepts it or the attempts are exhausted. Two new [redfish] options control the behaviour: power_on_conflict_retry_attempts (default 12) and power_on_conflict_retry_interval (default 5 seconds); set attempts to 0 to disable. Closes-Bug: #2162995 Change-Id: Id8d7535409a560b66a5d771be1751acd1248f61a Signed-off-by: Jay Jahns (cherry picked from commit 238c9e0cce3062c75d0658cabcd8c9ed249636ab) --- ironic/conf/redfish.py | 18 +++++ ironic/drivers/modules/redfish/power.py | 27 +++++++- .../drivers/modules/redfish/test_power.py | 67 +++++++++++++++++++ ...er-on-conflict-retry-76f84c2f6f6e5d4a.yaml | 11 +++ 4 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/redfish-power-on-conflict-retry-76f84c2f6f6e5d4a.yaml diff --git a/ironic/conf/redfish.py b/ironic/conf/redfish.py index 530de73e89..d7d96f7ba3 100644 --- a/ironic/conf/redfish.py +++ b/ironic/conf/redfish.py @@ -228,6 +228,24 @@ 'for POST-related boot device errors. Exponential ' 'backoff is applied, starting from this value up to ' '6x this value.')), + cfg.IntOpt('power_on_conflict_retry_attempts', + min=0, + default=6, + help=_('Maximum number of times to retry a power-on when the ' + 'BMC rejects it with an HTTP 409 ' + '"ActionParameterValueConflict". Some BMCs (e.g. Dell ' + 'iDRAC10) transiently reject the "On" reset type for a ' + 'short window (observed up to ~60 seconds) after a ' + 'power-off, until the BMC settles. This is a ceiling: ' + 'the power-on is retried only until the BMC accepts it, ' + 'so a node that settles sooner returns sooner. Set to 0 ' + 'to disable retrying.')), + cfg.IntOpt('power_on_conflict_retry_interval', + min=1, + default=10, + help=_('Number of seconds to wait between power-on retries ' + 'triggered by an HTTP 409 ' + '"ActionParameterValueConflict" from the BMC.')), ] diff --git a/ironic/drivers/modules/redfish/power.py b/ironic/drivers/modules/redfish/power.py index 8f977cc38f..ed7e4e0d63 100644 --- a/ironic/drivers/modules/redfish/power.py +++ b/ironic/drivers/modules/redfish/power.py @@ -23,6 +23,7 @@ from ironic.common import states from ironic.conductor import task_manager from ironic.conductor import utils as cond_utils +from ironic.conf import CONF from ironic.drivers import base from ironic.drivers.modules.redfish import management as redfish_mgmt from ironic.drivers.modules.redfish import utils as redfish_utils @@ -62,7 +63,31 @@ def _set_power_state(task, system, power_state, timeout=None): :raises: RedfishConnectionError when it fails to connect to Redfish :raises: RedfishError on an error from the Sushy library """ - system.reset_system(SET_POWER_STATE_MAP.get(power_state)) + reset_type = SET_POWER_STATE_MAP.get(power_state) + # NOTE(jayjahns): Some BMCs (e.g. Dell iDRAC10) transiently reject the + # "On" reset type with an HTTP 409 ActionParameterValueConflict for a + # short window after a power-off, until the BMC settles. The value is + # correct (the same "On" is accepted once settled) and such BMCs may not + # implement "ForceOn", so retry the power-on until the BMC accepts it or + # the configured attempts are exhausted. + max_retries = CONF.redfish.power_on_conflict_retry_attempts + for attempt in range(max_retries + 1): + try: + system.reset_system(reset_type) + break + except sushy.exceptions.HTTPError as e: + if (power_state == states.POWER_ON + and e.status_code == 409 + and 'ActionParameterValueConflict' in str(e) + and attempt < max_retries): + LOG.warning('Node %(node)s: BMC rejected power-on with 409 ' + '(ActionParameterValueConflict); retry %(n)d of ' + '%(max)d after the BMC settles.', + {'node': task.node.uuid, 'n': attempt + 1, + 'max': max_retries}) + time.sleep(CONF.redfish.power_on_conflict_retry_interval) + continue + raise target_state = TARGET_STATE_MAP.get(power_state, power_state) if power_state == states.REBOOT: LOG.debug('Waiting 15 seconds to give the node %s a chance to power ' diff --git a/ironic/tests/unit/drivers/modules/redfish/test_power.py b/ironic/tests/unit/drivers/modules/redfish/test_power.py index 0e6d197be2..ea6fa733cd 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_power.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_power.py @@ -366,6 +366,73 @@ def test_set_power_state_conflict_error_refresh_fails( log_msg = mock_log.call_args[0][0] self.assertIn('Failed to refresh system state', log_msg) + @mock.patch.object(redfish_power.time, 'sleep', autospec=True) + @mock.patch.object(redfish_mgmt.RedfishManagement, 'restore_boot_device', + autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_set_power_state_power_on_conflict_retry( + self, mock_get_system, mock_restore_bootdev, mock_sleep): + """POWER_ON retries a transient 409 ActionParameterValueConflict.""" + self.config(power_on_conflict_retry_attempts=3, + power_on_conflict_retry_interval=1, group='redfish') + fake_system = mock_get_system.return_value + + mock_response = mock.Mock(status_code=409) + mock_response.json.return_value = { + 'error': { + 'message': ("The parameter 'ResetType' with the requested " + "value of 'On' does not meet the constraints of " + "the implementation. " + "Base.1.18.ActionParameterValueConflict"), + } + } + conflict_409 = sushy.exceptions.HTTPError( + method='POST', url='test', response=mock_response) + + # Rejected twice while the BMC settles, then accepted. + fake_system.reset_system.side_effect = [conflict_409, conflict_409, + None] + fake_system.power_state = sushy.SYSTEM_POWER_STATE_ON + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.power.set_power_state(task, states.POWER_ON) + + self.assertEqual(3, fake_system.reset_system.call_count) + fake_system.reset_system.assert_called_with(sushy.RESET_ON) + mock_sleep.assert_called() + + @mock.patch.object(redfish_power.time, 'sleep', autospec=True) + @mock.patch.object(redfish_mgmt.RedfishManagement, 'restore_boot_device', + autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + def test_set_power_state_power_on_conflict_retry_exhausted( + self, mock_get_system, mock_restore_bootdev, mock_sleep): + """POWER_ON raises when the 409 conflict never clears.""" + self.config(power_on_conflict_retry_attempts=2, + power_on_conflict_retry_interval=1, group='redfish') + fake_system = mock_get_system.return_value + + mock_response = mock.Mock(status_code=409) + mock_response.json.return_value = { + 'error': {'message': 'Base.1.18.ActionParameterValueConflict'}, + } + conflict_409 = sushy.exceptions.HTTPError( + method='POST', url='test', response=mock_response) + fake_system.reset_system.side_effect = conflict_409 + # The node never reaches the ON state. + fake_system.power_state = sushy.SYSTEM_POWER_STATE_OFF + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises(sushy.exceptions.HTTPError, + task.driver.power.set_power_state, + task, states.POWER_ON) + + # 1 initial attempt + 2 retries. + self.assertEqual(3, fake_system.reset_system.call_count) + mock_sleep.assert_called() + @mock.patch.object(lc.BackOffLoopingCall, '_sleep', autospec=True) @mock.patch.object(redfish_mgmt.RedfishManagement, 'restore_boot_device', autospec=True) diff --git a/releasenotes/notes/redfish-power-on-conflict-retry-76f84c2f6f6e5d4a.yaml b/releasenotes/notes/redfish-power-on-conflict-retry-76f84c2f6f6e5d4a.yaml new file mode 100644 index 0000000000..697cfb8e84 --- /dev/null +++ b/releasenotes/notes/redfish-power-on-conflict-retry-76f84c2f6f6e5d4a.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixed bare-metal deploys failing at the ``boot_instance`` step on BMCs + (such as Dell iDRAC10 / PowerEdge 17G) that transiently reject the Redfish + ``On`` reset type with an HTTP 409 ``ActionParameterValueConflict`` for a + short window after a power-off, until the BMC settles. The ``redfish`` + power interface now retries the power-on until the BMC accepts it, + controlled by the new ``[redfish]power_on_conflict_retry_attempts`` and + ``[redfish]power_on_conflict_retry_interval`` options. See `bug 2162995 + `_.