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 81fc6b1165..ea6fa733cd 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,79 @@ 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) @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 +461,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 +493,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 +523,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 +555,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 +572,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 +591,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, 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 + `_.