From 46884deba3c271b7aae493729635b3c84343d55f Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 13 Jun 2019 11:50:15 +0200 Subject: [PATCH] redfish: handle missing Bios attribute Currently we log tracebacks on cleaning if a System doesn't have a Bios attribute (seems the case for some Seamicro systems). This patch turns it into a warning and provides correct handling of such cases in other BIOS calls. Also cleans up mocking in the BIOS tests since the current approach can yield surprising results with error inheritance. Change-Id: I34569806c3d34628b688661a3796e8d9b394135c --- ironic/conductor/manager.py | 7 +- ironic/drivers/modules/redfish/bios.py | 24 +++++-- ironic/tests/unit/conductor/test_manager.py | 12 +++- .../unit/drivers/modules/redfish/test_bios.py | 68 ++++++++++++++++--- .../unit/drivers/third_party_driver_mocks.py | 6 +- .../notes/cleaning-bios-d74a4947d2525b80.yaml | 5 ++ 6 files changed, 103 insertions(+), 19 deletions(-) create mode 100644 releasenotes/notes/cleaning-bios-d74a4947d2525b80.yaml diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index dee538b7fb..640ba5a8a7 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 1a0eed9518..6139f9ff89 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 eda1cff0f1..1a6063c127 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 3edcbf1e1e..f60c398425 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) @@ -95,6 +100,31 @@ class RedfishBiosTestCase(db_base.DbTestCase): 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_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): @@ -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) @@ -183,6 +220,19 @@ class RedfishBiosTestCase(db_base.DbTestCase): task.driver.bios._clear_reboot_requested\ .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'}, @@ -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 6e6b188de7..d953ca4759 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 0000000000..adecd9c0ea --- /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.