[4.20] OCPBUGS-114410: idrac10 retry conflict power-on

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.')),
 ]
 
 

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 '

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
@@ -76,12 +78,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 = [
@@ -128,9 +131,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')
@@ -363,11 +368,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 = [
@@ -390,11 +463,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.
@@ -421,10 +495,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 = [
@@ -449,12 +525,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
@@ -479,10 +557,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
 
@@ -492,6 +574,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)
@@ -510,9 +593,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),

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,

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
+    <https://bugs.launchpad.net/ironic/+bug/2162995>`_.