From bba3041ccf6397587e51c676781f6a204396bdc5 Mon Sep 17 00:00:00 2001 From: Jacob Anders Date: Fri, 12 Sep 2025 16:00:31 +1000 Subject: [PATCH] Make cache_firmware_components more resilient during upgrades There have been reports of firmware upgrades failing on Gen11 iLO machines with GET NetworkAdepters returning 400s responses. This change attempts to resolve this by catching the exception relevant to the fault Change-Id: I62095c2b61d14688d2dcbcdcfd29e9391af2c0ba Signed-off-by: Jacob Anders --- ironic/drivers/modules/redfish/firmware.py | 17 +++- .../drivers/modules/redfish/test_firmware.py | 85 +++++++++++++++++++ ...apters-fix-ilo-gen11-2f43f4fbdb995231.yaml | 7 ++ 3 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/networkadapters-fix-ilo-gen11-2f43f4fbdb995231.yaml diff --git a/ironic/drivers/modules/redfish/firmware.py b/ironic/drivers/modules/redfish/firmware.py index 9facb99451a..aee1f35cc6f 100644 --- a/ironic/drivers/modules/redfish/firmware.py +++ b/ironic/drivers/modules/redfish/firmware.py @@ -110,12 +110,25 @@ class RedfishFirmware(base.FirmwareInterface): LOG.warning('No manager available to retrieve Firmware ' 'from the bmc of node %s', task.node.uuid) - nic_components = self.retrieve_nic_components(task, system) + nic_components = None + try: + nic_components = self.retrieve_nic_components(task, system) + except (exception.RedfishError, + sushy.exceptions.BadRequestError) as e: + # NOTE(janders) if an exception is raised, log a warning + # with exception details. This is important for HP hardware + # which at the time of writing this are known to return 400 + # responses to GET NetworkAdapters while OS isn't fully booted + LOG.warning('Unable to access NetworkAdapters on node ' + '%(node_uuid)s, Error: %(error)s', + {'node_uuid': task.node.uuid, 'error': e}) + # NOTE(janders) if no exception is raised but no NICs are returned, + # state that clearly but in a lower severity message if nic_components == []: LOG.debug('Could not retrieve Firmware Package Version from ' 'NetworkAdapters on node %(node_uuid)s', {'node_uuid': task.node.uuid}) - else: + elif nic_components: settings.extend(nic_components) if not settings: diff --git a/ironic/tests/unit/drivers/modules/redfish/test_firmware.py b/ironic/tests/unit/drivers/modules/redfish/test_firmware.py index 4e468381ebe..5a9d176d831 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_firmware.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_firmware.py @@ -267,6 +267,91 @@ class RedfishFirmwareTestCase(db_base.DbTestCase): {'node_uuid': self.node.uuid} ) + @mock.patch.object(redfish_fw, 'LOG', autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(redfish_utils, 'get_manager', autospec=True) + @mock.patch.object(objects, 'FirmwareComponentList', autospec=True) + def test_retrieve_nic_components_redfish_connection_error( + self, sync_fw_cmp_mock, manager_mock, system_mock, log_mock): + """Test that RedfishConnectionError during NIC retrieval is handled.""" + system_mock.return_value.identity = "System1" + system_mock.return_value.bios_version = '1.0.0' + manager_mock.return_value.identity = "Manager1" + manager_mock.return_value.firmware_version = '1.0.0' + + sync_fw_cmp_mock.sync_firmware_components.return_value = ( + [{'component': 'bios', 'current_version': '1.0.0'}, + {'component': 'bmc', 'current_version': '1.0.0'}], + [], []) + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + with mock.patch.object(task.driver.firmware, + 'retrieve_nic_components', + autospec=True) as mock_retrieve: + connection_error = exception.RedfishError( + 'Connection failed') + mock_retrieve.side_effect = connection_error + + task.driver.firmware.cache_firmware_components(task) + + # Verify warning log for exception is called + log_mock.warning.assert_any_call( + 'Unable to access NetworkAdapters on node %(node_uuid)s, ' + 'Error: %(error)s', + {'node_uuid': self.node.uuid, 'error': connection_error} + ) + + # Verify debug log for empty NIC list is NOT called + # (since we caught an exception, not an empty list) + debug_calls = [call for call in log_mock.debug.call_args_list + if 'Could not retrieve Firmware Package Version from ' + 'NetworkAdapters' in str(call)] + self.assertEqual(len(debug_calls), 0) + + @mock.patch.object(redfish_fw, 'LOG', autospec=True) + @mock.patch.object(redfish_utils, 'get_system', autospec=True) + @mock.patch.object(redfish_utils, 'get_manager', autospec=True) + @mock.patch.object(objects, 'FirmwareComponentList', autospec=True) + def test_retrieve_nic_components_sushy_bad_request_error( + self, sync_fw_cmp_mock, manager_mock, system_mock, log_mock): + """Test that sushy BadRequestError during NIC retrieval is handled.""" + system_mock.return_value.identity = "System1" + system_mock.return_value.bios_version = '1.0.0' + manager_mock.return_value.identity = "Manager1" + manager_mock.return_value.firmware_version = '1.0.0' + + sync_fw_cmp_mock.sync_firmware_components.return_value = ( + [{'component': 'bios', 'current_version': '1.0.0'}, + {'component': 'bmc', 'current_version': '1.0.0'}], + [], []) + + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + with mock.patch.object(task.driver.firmware, + 'retrieve_nic_components', + autospec=True) as mock_retrieve: + bad_request_error = sushy.exceptions.BadRequestError( + method='GET', url='/redfish/v1/Chassis/1/NetworkAdapters', + response=mock.Mock(status_code=400)) + mock_retrieve.side_effect = bad_request_error + + task.driver.firmware.cache_firmware_components(task) + + # Verify warning log for exception is called + log_mock.warning.assert_any_call( + 'Unable to access NetworkAdapters on node %(node_uuid)s, ' + 'Error: %(error)s', + {'node_uuid': self.node.uuid, 'error': bad_request_error} + ) + + # Verify debug log for empty NIC list is NOT called + # (since we caught an exception, not an empty list) + debug_calls = [call for call in log_mock.debug.call_args_list + if 'Could not retrieve Firmware Package Version from ' + 'NetworkAdapters' in str(call)] + self.assertEqual(len(debug_calls), 0) + @mock.patch.object(redfish_utils, 'LOG', autospec=True) @mock.patch.object(redfish_utils, '_get_connection', autospec=True) def test_missing_updateservice(self, conn_mock, log_mock): diff --git a/releasenotes/notes/networkadapters-fix-ilo-gen11-2f43f4fbdb995231.yaml b/releasenotes/notes/networkadapters-fix-ilo-gen11-2f43f4fbdb995231.yaml new file mode 100644 index 00000000000..b0de9beb656 --- /dev/null +++ b/releasenotes/notes/networkadapters-fix-ilo-gen11-2f43f4fbdb995231.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixes an issue where firmware upgrades would fail on HP iLO Generation 11 + machines when the NetworkAdapters endpoint returns HTTP 400 (Bad Request) + responses during the firmware component caching process. Exception + handling allows the code to continue execution past that issue.