diff --git a/ironic/drivers/modules/drac/boot.py b/ironic/drivers/modules/drac/boot.py index f04ce959c9..94efc65b97 100644 --- a/ironic/drivers/modules/drac/boot.py +++ b/ironic/drivers/modules/drac/boot.py @@ -1,6 +1,6 @@ # Copyright 2019 Red Hat, Inc. # All Rights Reserved. -# Copyright (c) 2019 Dell Inc. or its subsidiaries. +# Copyright (c) 2019-2021 Dell Inc. or its subsidiaries. # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -18,8 +18,7 @@ from oslo_log import log from oslo_utils import importutils from ironic.common import boot_devices -from ironic.common import exception -from ironic.common.i18n import _ +from ironic.drivers.modules.drac import utils as drac_utils from ironic.drivers.modules.redfish import boot as redfish_boot from ironic.drivers.modules.redfish import utils as redfish_utils @@ -105,60 +104,8 @@ class DracRedfishVirtualMediaBoot(redfish_boot.RedfishVirtualMediaBoot): system = redfish_utils.get_system(task.node) - for manager in system.managers: - - # This call makes Sushy go fishing in the ocean of Sushy - # OEM extensions installed on the system. If it finds one - # for 'Dell' which implements the 'Manager' resource - # extension, it uses it to create an object which - # instantiates itself from the OEM JSON. The object is - # returned here. - # - # If the extension could not be found for one manager, it - # will not be found for any others until it is installed, so - # abruptly exit the for loop. The vendor and resource name, - # 'Dell' and 'Manager', respectively, used to search for the - # extension are invariant in the loop. - try: - manager_oem = manager.get_oem_extension('Dell') - except sushy.exceptions.OEMExtensionNotFoundError as e: - error_msg = (_("Search for Sushy OEM extension Python package " - "'sushy-oem-idrac' failed for node %(node)s. " - "Ensure it is installed. Error: %(error)s") % - {'node': task.node.uuid, 'error': e}) - LOG.error(error_msg) - raise exception.RedfishError(error=error_msg) - - try: - manager_oem.set_virtual_boot_device( - device, persistent=persistent, manager=manager, - system=system) - except sushy.exceptions.SushyError as e: - LOG.debug("Sushy OEM extension Python package " - "'sushy-oem-idrac' failed to set virtual boot " - "device with system %(system)s manager %(manager)s " - "for node %(node)s. Will try next manager, if " - "available. Error: %(error)s", - {'system': system.uuid if system.uuid else - system.identity, - 'manager': manager.uuid if manager.uuid else - manager.identity, - 'node': task.node.uuid, - 'error': e}) - continue - - LOG.info("Set node %(node)s boot device to %(device)s via OEM", - {'node': task.node.uuid, 'device': device}) - break - - else: - error_msg = (_('iDRAC Redfish set boot device failed for node ' - '%(node)s, because system %(system)s has no ' - 'manager%(no_manager)s.') % - {'node': task.node.uuid, - 'system': system.uuid if system.uuid else - system.identity, - 'no_manager': '' if not system.managers else - ' which could'}) - LOG.error(error_msg) - raise exception.RedfishError(error=error_msg) + drac_utils.execute_oem_manager_method( + task, 'set virtual boot device', + lambda m, manager: m.set_virtual_boot_device( + device, persistent=persistent, system=system, + manager=manager), pass_manager=True) diff --git a/ironic/drivers/modules/drac/utils.py b/ironic/drivers/modules/drac/utils.py new file mode 100644 index 0000000000..21073d56e4 --- /dev/null +++ b/ironic/drivers/modules/drac/utils.py @@ -0,0 +1,121 @@ +# Copyright (c) 2021 Dell Inc. or its subsidiaries. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_log import log +from oslo_utils import importutils + +from ironic.common import exception +from ironic.drivers.modules.redfish import utils as redfish_utils + +LOG = log.getLogger(__name__) + +sushy = importutils.try_import('sushy') + + +def execute_oem_manager_method( + task, process_name, lambda_oem_func, pass_manager=False): + """Loads OEM manager and executes passed method on it. + + Known iDRAC Redfish systems has only one manager, but as Redfish + schema allows a list this method iterates through all values in case + this changes in future. If there are several managers, this will + try starting from the first in the list until the first success. + + :param task: a TaskManager instance. + :param process_name: user friendly name of method to be executed. + Used in exception and log messages. + :param lambda_oem_func: method to execute as lambda function with + input parameter OEM extension manager. + Example: lambda m: m.reset_idrac() + For older versions also support second input parameter Redfish + manager itself when pass_manager set to True. + :param pass_manager: whether to pass manager itself to executed + OEM extension method. This is for backward compability, new + functions must not pass manager, but acquire it internally. Will + be removed in future. + :returns: Returned value of lambda_oem_func + :raises: RedfishError if can't execute OEM function either because + there are no managers to the system, failed to load OEM + extension or execution of the OEM method failed itself. + """ + + system = redfish_utils.get_system(task.node) + + if not system.managers: + raise exception.RedfishError( + "System %(system)s has no managers" % + {'system': system.uuid if system.uuid else system.identity}) + + oem_error_msgs = [] + for manager in system.managers: + # This call makes Sushy go fishing in the ocean of Sushy + # OEM extensions installed on the system. If it finds one + # for 'Dell' which implements the 'Manager' resource + # extension, it uses it to create an object which + # instantiates itself from the OEM JSON. The object is + # returned here. + # + # If the extension could not be found for one manager, it + # will not be found for any others until it is installed, so + # abruptly exit the for loop. The vendor and resource name, + # 'Dell' and 'Manager', respectively, used to search for the + # extension are invariant in the loop. + try: + manager_oem = manager.get_oem_extension('Dell') + except sushy.exceptions.OEMExtensionNotFoundError as e: + error_msg = (_("Search for Sushy OEM extension Python package " + "'sushy-oem-idrac' failed for node %(node)s. " + "Ensure it is installed. Error: %(error)s") % + {'node': task.node.uuid, 'error': e}) + LOG.error(error_msg) + raise exception.RedfishError(error=error_msg) + + try: + if pass_manager: + result = lambda_oem_func(manager_oem, manager) + else: + result = lambda_oem_func(manager_oem) + LOG.info("Completed: %(process_name)s with system %(system)s " + "manager %(manager)s for node %(node)s", + {'process_name': process_name, + 'system': system.uuid if system.uuid else + system.identity, + 'manager': manager.uuid if manager.uuid else + manager.identity, + 'node': task.node.uuid}) + return result + except sushy.exceptions.SushyError as e: + error_msg = (_("Manager %(manager)s: %(error)s" % + {'manager': manager.uuid if manager.uuid else + manager.identity, 'error': e})) + LOG.debug("Failed: %(process_name)s with system %(system)s " + "for node %(node)s. Will try next manager, if " + "available. Error: %(error)s", + {'process_name': process_name, + 'system': system.uuid if system.uuid else + system.identity, + 'node': task.node.uuid, + 'error': error_msg}) + oem_error_msgs.append(error_msg) + else: + error_msg = (_('In system %(system)s for node %(node)s all managers ' + 'failed: %(process_name)s. Errors: %(oem_error_msgs)s' % + {'system': system.uuid if system.uuid else + system.identity, + 'node': task.node.uuid, + 'process_name': process_name, + 'oem_error_msgs': oem_error_msgs if oem_error_msgs else + 'unknown'})) + LOG.error(error_msg) + raise exception.RedfishError(error=error_msg) diff --git a/ironic/tests/unit/drivers/modules/drac/test_boot.py b/ironic/tests/unit/drivers/modules/drac/test_boot.py index bed8be2ef2..92ff32d81c 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_boot.py +++ b/ironic/tests/unit/drivers/modules/drac/test_boot.py @@ -1,6 +1,6 @@ # Copyright 2019 Red Hat, Inc. # All Rights Reserved. -# Copyright (c) 2019 Dell Inc. or its subsidiaries. +# Copyright (c) 2019-2021 Dell Inc. or its subsidiaries. # # Licensed under the Apache License, Version 2.0 (the "License"); you may # not use this file except in compliance with the License. You may obtain @@ -23,10 +23,9 @@ from unittest import mock from oslo_utils import importutils from ironic.common import boot_devices -from ironic.common import exception from ironic.conductor import task_manager from ironic.drivers.modules import deploy_utils -from ironic.drivers.modules.drac import boot as drac_boot +from ironic.drivers.modules.redfish import utils as redfish_utils from ironic.tests.unit.db import utils as db_utils from ironic.tests.unit.drivers.modules.drac import utils as test_utils from ironic.tests.unit.objects import utils as obj_utils @@ -36,7 +35,7 @@ sushy = importutils.try_import('sushy') INFO_DICT = dict(db_utils.get_test_redfish_info(), **test_utils.INFO_DICT) -@mock.patch.object(drac_boot, 'redfish_utils', autospec=True) +@mock.patch.object(redfish_utils, 'get_system', autospec=True) class DracBootTestCase(test_utils.BaseDracTest): def setUp(self): @@ -46,7 +45,7 @@ class DracBootTestCase(test_utils.BaseDracTest): @mock.patch.object(deploy_utils, 'validate_image_properties', autospec=True) - def test_validate_correct_vendor(self, mock_redfish_utils, + def test_validate_correct_vendor(self, mock_get_system, mock_validate_image_properties): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: @@ -66,14 +65,10 @@ class DracBootTestCase(test_utils.BaseDracTest): task.driver.boot.validate(task) - def test__set_boot_device_persistent(self, mock_redfish_utils): - - mock_system = mock_redfish_utils.get_system.return_value - + def test__set_boot_device_persistent(self, mock_get_system): mock_manager = mock.MagicMock() - + mock_system = mock_get_system.return_value mock_system.managers = [mock_manager] - mock_manager_oem = mock_manager.get_oem_extension.return_value with task_manager.acquire(self.context, self.node.uuid, @@ -85,14 +80,10 @@ class DracBootTestCase(test_utils.BaseDracTest): 'cd', persistent=True, manager=mock_manager, system=mock_system) - def test__set_boot_device_cd(self, mock_redfish_utils): - - mock_system = mock_redfish_utils.get_system.return_value - + def test__set_boot_device_cd(self, mock_get_system): + mock_system = mock_get_system.return_value mock_manager = mock.MagicMock() - mock_system.managers = [mock_manager] - mock_manager_oem = mock_manager.get_oem_extension.return_value with task_manager.acquire(self.context, self.node.uuid, @@ -103,14 +94,10 @@ class DracBootTestCase(test_utils.BaseDracTest): 'cd', persistent=False, manager=mock_manager, system=mock_system) - def test__set_boot_device_floppy(self, mock_redfish_utils): - - mock_system = mock_redfish_utils.get_system.return_value - + def test__set_boot_device_floppy(self, mock_get_system): + mock_system = mock_get_system.return_value mock_manager = mock.MagicMock() - mock_system.managers = [mock_manager] - mock_manager_oem = mock_manager.get_oem_extension.return_value with task_manager.acquire(self.context, self.node.uuid, @@ -121,72 +108,11 @@ class DracBootTestCase(test_utils.BaseDracTest): 'floppy', persistent=False, manager=mock_manager, system=mock_system) - def test__set_boot_device_disk(self, mock_redfish_utils): - - mock_system = mock_redfish_utils.get_system.return_value + def test__set_boot_device_disk(self, mock_get_system): + mock_system = mock_get_system.return_value with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.boot._set_boot_device(task, boot_devices.DISK) self.assertFalse(mock_system.called) - - def test__set_boot_device_missing_oem(self, mock_redfish_utils): - - mock_system = mock_redfish_utils.get_system.return_value - - mock_manager = mock.MagicMock() - - mock_system.managers = [mock_manager] - - mock_manager.get_oem_extension.side_effect = ( - sushy.exceptions.OEMExtensionNotFoundError) - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - self.assertRaises(exception.RedfishError, - task.driver.boot._set_boot_device, - task, boot_devices.CDROM) - - def test__set_boot_device_failover(self, mock_redfish_utils): - - mock_system = mock_redfish_utils.get_system.return_value - - mock_manager_fail = mock.MagicMock() - mock_manager_ok = mock.MagicMock() - - mock_system.managers = [mock_manager_fail, mock_manager_ok] - - mock_svbd_fail = (mock_manager_fail.get_oem_extension - .return_value.set_virtual_boot_device) - - mock_svbd_ok = (mock_manager_ok.get_oem_extension - .return_value.set_virtual_boot_device) - - mock_svbd_fail.side_effect = sushy.exceptions.SushyError - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - task.driver.boot._set_boot_device(task, boot_devices.CDROM) - - self.assertFalse(mock_system.called) - - mock_svbd_fail.assert_called_once_with( - 'cd', manager=mock_manager_fail, persistent=False, - system=mock_system) - - mock_svbd_ok.assert_called_once_with( - 'cd', manager=mock_manager_ok, persistent=False, - system=mock_system) - - def test__set_boot_device_no_manager(self, mock_redfish_utils): - - mock_system = mock_redfish_utils.get_system.return_value - - mock_system.managers = [] - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - self.assertRaises(exception.RedfishError, - task.driver.boot._set_boot_device, - task, boot_devices.CDROM) diff --git a/ironic/tests/unit/drivers/modules/drac/test_utils.py b/ironic/tests/unit/drivers/modules/drac/test_utils.py new file mode 100644 index 0000000000..d664575501 --- /dev/null +++ b/ironic/tests/unit/drivers/modules/drac/test_utils.py @@ -0,0 +1,103 @@ +# Copyright (c) 2021 Dell Inc. or its subsidiaries. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from unittest import mock + +from oslo_utils import importutils + +from ironic.common import exception +from ironic.conductor import task_manager +from ironic.drivers.modules.drac import utils as drac_utils +from ironic.drivers.modules.redfish import utils as redfish_utils +from ironic.tests.unit.drivers.modules.drac import utils as test_utils +from ironic.tests.unit.objects import utils as obj_utils + +sushy = importutils.try_import('sushy') + +INFO_DICT = test_utils.INFO_DICT + + +@mock.patch.object(redfish_utils, 'get_system', autospec=True) +class DracUtilsOemManagerTestCase(test_utils.BaseDracTest): + + def setUp(self): + super(DracUtilsOemManagerTestCase, self).setUp() + self.node = obj_utils.create_test_node(self.context, + driver='idrac', + driver_info=INFO_DICT) + self.config(enabled_hardware_types=['idrac'], + enabled_management_interfaces=['idrac-redfish']) + + def test_execute_oem_manager_method(self, mock_get_system): + fake_manager_oem = mock.Mock() + fake_manager_oem.test_method.return_value = 42 + fake_manager = mock.Mock() + fake_manager.get_oem_extension.return_value = fake_manager_oem + mock_get_system.return_value.managers = [fake_manager] + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + result = drac_utils.execute_oem_manager_method( + task, 'test method', lambda m: m.test_method()) + + self.assertEqual(42, result) + + def test_execute_oem_manager_method_no_managers(self, mock_get_system): + mock_get_system.return_value.managers = [] + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises( + exception.RedfishError, + drac_utils.execute_oem_manager_method, + task, + 'test method', + lambda m: m.test_method()) + + def test_execute_oem_manager_method_oem_not_found(self, mock_get_system): + fake_manager = mock.Mock() + fake_manager.get_oem_extension.side_effect = ( + sushy.exceptions.OEMExtensionNotFoundError) + mock_get_system.return_value.managers = [fake_manager] + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises( + exception.RedfishError, + drac_utils.execute_oem_manager_method, + task, + 'test method', + lambda m: m.test_method()) + + def test_execute_oem_manager_method_managers_fail(self, mock_get_system): + fake_manager_oem1 = mock.Mock() + fake_manager_oem1.test_method.side_effect = ( + sushy.exceptions.SushyError) + fake_manager1 = mock.Mock() + fake_manager1.get_oem_extension.return_value = fake_manager_oem1 + fake_manager_oem2 = mock.Mock() + fake_manager_oem2.test_method.side_effect = ( + sushy.exceptions.SushyError) + fake_manager2 = mock.Mock() + fake_manager2.get_oem_extension.return_value = fake_manager_oem2 + mock_get_system.return_value.managers = [fake_manager1, fake_manager2] + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + self.assertRaises( + exception.RedfishError, + drac_utils.execute_oem_manager_method, + task, + 'test method', + lambda m: m.test_method())