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.