Merge "redfish: handle missing Bios attribute"

6 files changed, 103 insertions, 19 deletions

diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py
index 3a2d6b7da..8c28307ce 100644
--- a/ironic/conductor/manager.py
+++ b/ironic/conductor/manager.py
@@ -1307,10 +1307,13 @@ class ConductorManager(base_manager.BaseConductorManager):
 
         # Do caching of bios settings if supported by driver,
         # this will be called for both manual and automated cleaning.
-        # TODO(zshi) remove this check when classic drivers are removed
         try:
             task.driver.bios.cache_bios_settings(task)
-        except Exception as e:
+        except exception.UnsupportedDriverExtension:
+            LOG.warning('BIOS settings are not supported for node %s, '
+                        'skipping', task.node.uuid)
+        # TODO(zshi) remove this check when classic drivers are removed
+        except Exception:
             msg = (_('Caching of bios settings failed on node %(node)s. '
                      'Continuing with node cleaning.')
                    % {'node': node.uuid})
diff --git a/ironic/drivers/modules/redfish/bios.py b/ironic/drivers/modules/redfish/bios.py
index 1a0eed951..6139f9ff8 100644
--- a/ironic/drivers/modules/redfish/bios.py
+++ b/ironic/drivers/modules/redfish/bios.py
@@ -50,11 +50,20 @@ class RedfishBIOS(base.BIOSInterface):
         :param task: a TaskManager instance containing the node to act on.
         :raises: RedfishConnectionError when it fails to connect to Redfish
         :raises: RedfishError on an error from the Sushy library
+        :raises: UnsupportedDriverExtension if the system does not support BIOS
+            settings
         """
 
         node_id = task.node.id
         system = redfish_utils.get_system(task.node)
-        attributes = system.bios.attributes
+        try:
+            attributes = system.bios.attributes
+        except sushy.exceptions.MissingAttributeError:
+            error_msg = _('Cannot fetch BIOS attributes for node %s, '
+                          'BIOS settings are not supported.') % task.node.uuid
+            LOG.error(error_msg)
+            raise exception.UnsupportedDriverExtension(error_msg)
+
         settings = []
         # Convert Redfish BIOS attributes to Ironic BIOS settings
         if attributes:
@@ -88,11 +97,10 @@ class RedfishBIOS(base.BIOSInterface):
         :raises: RedfishError on an error from the Sushy library
         """
         system = redfish_utils.get_system(task.node)
-        bios = system.bios
         LOG.debug('Factory reset BIOS settings for node %(node_uuid)s',
                   {'node_uuid': task.node.uuid})
         try:
-            bios.reset_bios()
+            system.bios.reset_bios()
         except sushy.exceptions.SushyError as e:
             error_msg = (_('Redfish BIOS factory reset failed for node '
                            '%(node)s. Error: %(error)s') %
@@ -122,7 +130,15 @@ class RedfishBIOS(base.BIOSInterface):
         """
 
         system = redfish_utils.get_system(task.node)
-        bios = system.bios
+        try:
+            bios = system.bios
+        except sushy.exceptions.MissingAttributeError:
+            error_msg = (_('Redfish BIOS factory reset failed for node '
+                           '%s, because BIOS settings are not supported.') %
+                         task.node.uuid)
+            LOG.error(error_msg)
+            raise exception.RedfishError(error=error_msg)
+
         # Convert Ironic BIOS settings to Redfish BIOS attributes
         attributes = {s['name']: s['value'] for s in settings}
 
diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py
index eda1cff0f..1a6063c12 100644
--- a/ironic/tests/unit/conductor/test_manager.py
+++ b/ironic/tests/unit/conductor/test_manager.py
@@ -3513,8 +3513,11 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def _test__do_node_clean_cache_bios(self, mock_bios, mock_validate,
                                         mock_prep, mock_next_step, mock_steps,
                                         mock_log, clean_steps=None,
+                                        enable_unsupported=False,
                                         enable_exception=False):
-        if enable_exception:
+        if enable_unsupported:
+            mock_bios.side_effect = exception.UnsupportedDriverExtension('')
+        elif enable_exception:
             mock_bios.side_effect = exception.IronicException('test')
         mock_prep.return_value = states.NOSTATE
         tgt_prov_state = states.MANAGEABLE if clean_steps else states.AVAILABLE
@@ -3553,6 +3556,13 @@ class DoNodeCleanTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase):
     def test__do_node_clean_automated_cache_bios_exception(self):
         self._test__do_node_clean_cache_bios(enable_exception=True)
 
+    def test__do_node_clean_manual_cache_bios_unsupported(self):
+        self._test__do_node_clean_cache_bios(clean_steps=[self.deploy_raid],
+                                             enable_unsupported=True)
+
+    def test__do_node_clean_automated_cache_bios_unsupported(self):
+        self._test__do_node_clean_cache_bios(enable_unsupported=True)
+
     @mock.patch('ironic.drivers.modules.fake.FakePower.validate',
                 autospec=True)
     def test__do_node_clean_automated_disabled(self, mock_validate):
diff --git a/ironic/tests/unit/drivers/modules/redfish/test_bios.py b/ironic/tests/unit/drivers/modules/redfish/test_bios.py
index 3edcbf1e1..f60c39842 100644
--- a/ironic/tests/unit/drivers/modules/redfish/test_bios.py
+++ b/ironic/tests/unit/drivers/modules/redfish/test_bios.py
@@ -31,8 +31,13 @@ sushy = importutils.try_import('sushy')
 INFO_DICT = db_utils.get_test_redfish_info()
 
 
-class MockedSushyError(Exception):
-    pass
+class NoBiosSystem(object):
+    identity = '/redfish/v1/Systems/1234'
+
+    @property
+    def bios(self):
+        raise sushy.exceptions.MissingAttributeError(attribute='Bios',
+                                                     resource=self)
 
 
 @mock.patch('eventlet.greenthread.sleep', lambda _t: None)
@@ -97,6 +102,31 @@ class RedfishBiosTestCase(db_base.DbTestCase):
 
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     @mock.patch.object(objects, 'BIOSSettingList', autospec=True)
+    def test_cache_bios_settings_no_bios(self, mock_setting_list,
+                                         mock_get_system):
+        create_list = []
+        update_list = []
+        delete_list = []
+        nochange_list = [{'name': 'EmbeddedSata', 'value': 'Raid'},
+                         {'name': 'NicBoot1', 'value': 'NetworkBoot'}]
+        mock_setting_list.sync_node_setting.return_value = (
+            create_list, update_list, delete_list, nochange_list
+        )
+        mock_get_system.return_value = NoBiosSystem()
+
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=True) as task:
+            self.assertRaisesRegex(exception.UnsupportedDriverExtension,
+                                   'BIOS settings are not supported',
+                                   task.driver.bios.cache_bios_settings, task)
+            mock_get_system.assert_called_once_with(task.node)
+            mock_setting_list.sync_node_setting.assert_not_called()
+            mock_setting_list.create.assert_not_called()
+            mock_setting_list.save.assert_not_called()
+            mock_setting_list.delete.assert_not_called()
+
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    @mock.patch.object(objects, 'BIOSSettingList', autospec=True)
     def test_cache_bios_settings(self, mock_setting_list, mock_get_system):
         create_list = [{'name': 'DebugMode', 'value': 'enabled'}]
         update_list = [{'name': 'BootMode', 'value': 'Uefi'},
@@ -139,14 +169,21 @@ class RedfishBiosTestCase(db_base.DbTestCase):
             bios = mock_get_system(task.node).bios
             bios.reset_bios.assert_called_once()
 
-    @mock.patch('ironic.drivers.modules.redfish.bios.sushy')
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
-    def test_factory_reset_fail(self, mock_get_system, mock_sushy):
-        mock_sushy.exceptions.SushyError = MockedSushyError
+    def test_factory_reset_fail(self, mock_get_system):
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             bios = mock_get_system(task.node).bios
-            bios.reset_bios.side_effect = MockedSushyError
+            bios.reset_bios.side_effect = sushy.exceptions.SushyError
+            self.assertRaisesRegex(
+                exception.RedfishError, 'BIOS factory reset failed',
+                task.driver.bios.factory_reset, task)
+
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    def test_factory_reset_not_supported(self, mock_get_system):
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            mock_get_system.return_value = NoBiosSystem()
             self.assertRaisesRegex(
                 exception.RedfishError, 'BIOS factory reset failed',
                 task.driver.bios.factory_reset, task)
@@ -184,6 +221,19 @@ class RedfishBiosTestCase(db_base.DbTestCase):
                 .assert_called_once_with(task)
 
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
+    def test_apply_configuration_not_supported(self, mock_get_system):
+        settings = [{'name': 'ProcTurboMode', 'value': 'Disabled'},
+                    {'name': 'NicBoot1', 'value': 'NetworkBoot'}]
+        with task_manager.acquire(self.context, self.node.uuid,
+                                  shared=False) as task:
+            mock_get_system.return_value = NoBiosSystem()
+            self.assertRaisesRegex(exception.RedfishError,
+                                   'BIOS settings are not supported',
+                                   task.driver.bios.apply_configuration,
+                                   task, settings)
+            mock_get_system.assert_called_once_with(task.node)
+
+    @mock.patch.object(redfish_utils, 'get_system', autospec=True)
     def test_check_bios_attrs(self, mock_get_system):
         settings = [{'name': 'ProcTurboMode', 'value': 'Disabled'},
                     {'name': 'NicBoot1', 'value': 'NetworkBoot'}]
@@ -200,16 +250,14 @@ class RedfishBiosTestCase(db_base.DbTestCase):
             task.driver.bios._check_bios_attrs \
                 .assert_called_once_with(task, attributes, requested_attrs)
 
-    @mock.patch('ironic.drivers.modules.redfish.bios.sushy')
     @mock.patch.object(redfish_utils, 'get_system', autospec=True)
-    def test_apply_configuration_fail(self, mock_get_system, mock_sushy):
+    def test_apply_configuration_fail(self, mock_get_system):
         settings = [{'name': 'ProcTurboMode', 'value': 'Disabled'},
                     {'name': 'NicBoot1', 'value': 'NetworkBoot'}]
-        mock_sushy.exceptions.SushyError = MockedSushyError
         with task_manager.acquire(self.context, self.node.uuid,
                                   shared=False) as task:
             bios = mock_get_system(task.node).bios
-            bios.set_attributes.side_effect = MockedSushyError
+            bios.set_attributes.side_effect = sushy.exceptions.SushyError
             self.assertRaisesRegex(
                 exception.RedfishError, 'BIOS apply configuration failed',
                 task.driver.bios.apply_configuration, task, settings)
diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py
index 6e6b188de..d953ca475 100644
--- a/ironic/tests/unit/drivers/third_party_driver_mocks.py
+++ b/ironic/tests/unit/drivers/third_party_driver_mocks.py
@@ -237,9 +237,11 @@ if not sushy:
     sushy.exceptions.SushyError = (
         type('SushyError', (MockKwargsException,), {}))
     sushy.exceptions.ConnectionError = (
-        type('ConnectionError', (MockKwargsException,), {}))
+        type('ConnectionError', (sushy.exceptions.SushyError,), {}))
     sushy.exceptions.ResourceNotFoundError = (
-        type('ResourceNotFoundError', (MockKwargsException,), {}))
+        type('ResourceNotFoundError', (sushy.exceptions.SushyError,), {}))
+    sushy.exceptions.MissingAttributeError = (
+        type('MissingAttributeError', (sushy.exceptions.SushyError,), {}))
     sushy.auth = mock.MagicMock(spec_set=mock_specs.SUSHY_AUTH_SPEC)
     sys.modules['sushy.auth'] = sushy.auth
 
diff --git a/releasenotes/notes/cleaning-bios-d74a4947d2525b80.yaml b/releasenotes/notes/cleaning-bios-d74a4947d2525b80.yaml
new file mode 100644
index 000000000..adecd9c0e
--- /dev/null
+++ b/releasenotes/notes/cleaning-bios-d74a4947d2525b80.yaml
@@ -0,0 +1,5 @@
+---
+fixes:
+  - |
+    Fixes traceback on cleaning of nodes with the ``redfish`` hardware type
+    if their BMC does not support BIOS settings.