Merge pull request #553 from lmiccini/OSPRH-28415

Retry Redfish power-on after 400 Bad Request when server is OFF

Author: openshift-merge-bot[bot]

templates/instanceha/bin/instanceha.py

@@ -1870,9 +1870,27 @@ def _redfish_reset(url, user, passwd, timeout, action, config_mgr):
                 # Check if server is already powered off
                 power_state = _get_power_state('redfish', url=url, user=user, passwd=passwd, timeout=timeout, config_mgr=config_mgr)
                 if power_state == 'OFF':
-                    logging.debug("Redfish reset successful: %s on %s (already off)", action, safe_url)
-                    logging.info("Redfish reset successful: %s (already off)", action)
-                    return True
+                    if action in ['On', 'ForceOn']:
+                        # Server is OFF but not yet ready to accept power-on
+                        # (e.g. PRIMEQUEST 4400E needs time after ForceOff before
+                        # accepting On). Retry with backoff.
+                        if attempt < MAX_FENCING_RETRIES - 1:
+                            logging.warning(
+                                "Redfish reset: %s rejected with %d while server is OFF "
+                                "(attempt %d/%d), server may not be ready yet, retrying...",
+                                action, response.status_code, attempt + 1, MAX_FENCING_RETRIES)
+                            time.sleep(FENCING_RETRY_DELAY_SECONDS * (attempt + 1))
+                            continue
+                        else:
+                            logging.error(
+                                "Redfish reset failed: %s rejected with %d while server is OFF "
+                                "after %d attempts for %s",
+                                action, response.status_code, MAX_FENCING_RETRIES, safe_url)
+                            return False
+                    else:
+                        logging.debug("Redfish reset successful: %s on %s (already off)", action, safe_url)
+                        logging.info("Redfish reset successful: %s (already off)", action)
+                        return True
                 else:
                     logging.error("Redfish reset failed: %s conflict but not OFF (status: %s) for %s", response.status_code, power_state, safe_url)
                     return False

test/instanceha/test_fencing_agents.py

@@ -201,6 +201,91 @@ def test_redfish_reset_400_bad_request_not_off(self, mock_get, mock_post):
         mock_post.assert_called_once()
         mock_get.assert_called_once()
 
+    @patch('instanceha.requests.post')
+    @patch('instanceha.requests.get')
+    @patch('instanceha.time.sleep')
+    def test_redfish_reset_400_on_action_retries_when_off(self, mock_sleep, mock_get, mock_post):
+        """Test Redfish reset retries On action with 400 when server is OFF.
+
+        Some hardware (e.g. PRIMEQUEST 4400E) returns 400 Bad Request when
+        an On operation is sent too soon after ForceOff, because the system
+        is not yet ready to accept the power-on command even though
+        PowerState is already Off. The retry logic should handle this.
+        """
+        mock_config_manager = Mock()
+        mock_config_manager.get_requests_ssl_config.return_value = False
+
+        # First two POST attempts return 400, third succeeds with 200
+        mock_post.side_effect = [
+            Mock(status_code=400),
+            Mock(status_code=400),
+            Mock(status_code=200),
+        ]
+
+        # GET returns OFF for both 400 responses (server is off but not ready)
+        mock_get_response = Mock()
+        mock_get_response.status_code = 200
+        mock_get_response.json.return_value = {'PowerState': 'Off'}
+        mock_get.return_value = mock_get_response
+
+        result = instanceha._redfish_reset(
+            'http://test-server/redfish/v1/Systems/1', 'user', 'pass', 30,
+            'On', mock_config_manager)
+
+        self.assertTrue(result)
+        self.assertEqual(mock_post.call_count, 3)
+        self.assertEqual(mock_get.call_count, 2)  # Called on each 400
+        self.assertEqual(mock_sleep.call_count, 2)  # Sleep between retries
+
+    @patch('instanceha.requests.post')
+    @patch('instanceha.requests.get')
+    @patch('instanceha.time.sleep')
+    def test_redfish_reset_400_on_action_retry_exhausted(self, mock_sleep, mock_get, mock_post):
+        """Test Redfish reset fails after max retries on 400 for On action."""
+        mock_config_manager = Mock()
+        mock_config_manager.get_requests_ssl_config.return_value = False
+
+        # All POST attempts return 400
+        mock_post.return_value = Mock(status_code=400)
+
+        # GET always returns OFF (server off but never ready)
+        mock_get_response = Mock()
+        mock_get_response.status_code = 200
+        mock_get_response.json.return_value = {'PowerState': 'Off'}
+        mock_get.return_value = mock_get_response
+
+        result = instanceha._redfish_reset(
+            'http://test-server/redfish/v1/Systems/1', 'user', 'pass', 30,
+            'On', mock_config_manager)
+
+        self.assertFalse(result)
+        self.assertEqual(mock_post.call_count, 3)  # MAX_FENCING_RETRIES
+        self.assertEqual(mock_get.call_count, 3)
+        self.assertEqual(mock_sleep.call_count, 2)  # No sleep after last attempt
+
+    @patch('instanceha.requests.post')
+    @patch('instanceha.requests.get')
+    def test_redfish_reset_400_forceoff_already_off(self, mock_get, mock_post):
+        """Test Redfish reset with 400 for ForceOff when already off succeeds immediately."""
+        mock_config_manager = Mock()
+        mock_config_manager.get_requests_ssl_config.return_value = False
+
+        mock_post.return_value = Mock(status_code=400)
+
+        mock_get_response = Mock()
+        mock_get_response.status_code = 200
+        mock_get_response.json.return_value = {'PowerState': 'Off'}
+        mock_get.return_value = mock_get_response
+
+        result = instanceha._redfish_reset(
+            'http://test-server/redfish/v1/Systems/1', 'user', 'pass', 30,
+            'ForceOff', mock_config_manager)
+
+        # ForceOff with 400 and power state OFF should succeed immediately (no retry)
+        self.assertTrue(result)
+        mock_post.assert_called_once()
+        mock_get.assert_called_once()
+
     @patch('instanceha.requests.post')
     def test_redfish_reset_success(self, mock_post):
         """Test successful Redfish reset."""