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 <janders@redhat.com>

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:

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):

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.