From 39cd751a9063ae04e1649ccbca5496c3754d706e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aija=20Jaunt=C4=93va?= Date: Wed, 19 May 2021 03:13:22 -0400 Subject: [PATCH] Refactor iDRAC OEM extension manager calls - Re-usable helper created to avoid duplication. - Although there is only one manager for system in known iDRAC systems still iterate through collection for future changes. - Restructured exception raising and error logging for better feedback. - Removed some unit tests to avoid duplication that is covered by method specific unit tests Change-Id: I03fdb48e47c9557c207a20ee876eccf3f3459d9f --- ironic/drivers/modules/drac/boot.py | 67 +------ ironic/drivers/modules/drac/inspect.py | 46 +---- ironic/drivers/modules/drac/management.py | 185 +++--------------- ironic/drivers/modules/drac/raid.py | 43 +--- ironic/drivers/modules/drac/utils.py | 121 ++++++++++++ .../unit/drivers/modules/drac/test_boot.py | 98 ++-------- .../unit/drivers/modules/drac/test_inspect.py | 27 --- .../drivers/modules/drac/test_management.py | 124 ++---------- .../unit/drivers/modules/drac/test_raid.py | 45 +---- .../unit/drivers/modules/drac/test_utils.py | 103 ++++++++++ 10 files changed, 295 insertions(+), 564 deletions(-) create mode 100644 ironic/drivers/modules/drac/utils.py create mode 100644 ironic/tests/unit/drivers/modules/drac/test_utils.py 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/inspect.py b/ironic/drivers/modules/drac/inspect.py index 3c7985e514..aa2ad3dae1 100644 --- a/ironic/drivers/modules/drac/inspect.py +++ b/ironic/drivers/modules/drac/inspect.py @@ -27,6 +27,7 @@ from ironic.common import states from ironic.common import utils from ironic.drivers import base from ironic.drivers.modules.drac import common as drac_common +from ironic.drivers.modules.drac import utils as drac_utils from ironic.drivers.modules import inspect_utils from ironic.drivers.modules.redfish import inspect as redfish_inspect from ironic.drivers.modules.redfish import utils as redfish_utils @@ -118,48 +119,11 @@ class DracRedfishInspect(redfish_inspect.RedfishInspect): # how to get it, and Dell does not have OEM redfish calls # to selectively retrieve it at this time. # Get instance of Sushy OEM manager object + pxe_port_macs_list = drac_utils.execute_oem_manager_method( + task, 'get PXE port MAC addresses', + lambda m: m.get_pxe_port_macs_bios(ethernet_interfaces_mac)) + pxe_port_macs = [mac for mac in pxe_port_macs_list] - for manager in system.managers: - try: - # Get instance of Sushy OEM manager object - oem_manager = manager.get_oem_extension('Dell') - except sushy.exceptions.OEMExtensionNotFoundError as e: - error_msg = (_("Search for Sushy OEM extension package " - "'sushy-oem-idrac' failed for node " - "%(node)s. Ensure it's installed. " - " Error: %(error)s") % - {'node': task.node.uuid, 'error': e}) - LOG.error(error_msg) - raise exception.RedfishError(error=error_msg) - - try: - pxe_port_macs_list = oem_manager.get_pxe_port_macs_bios( - ethernet_interfaces_mac) - pxe_port_macs = [mac for mac in pxe_port_macs_list] - return pxe_port_macs - except sushy.exceptions.OEMExtensionNotFoundError as e: - error_msg = (_("Search for Sushy OEM extension package " - "'sushy-oem-idrac' failed for node " - " %(node)s. Ensure it is installed. " - "Error: %(error)s") % - {'node': task.node.uuid, 'error': e}) - LOG.debug(error_msg) - continue - LOG.info("Get pxe port MAC addresses for %(node)s via OEM", - {'node': task.node.uuid}) - break - else: - error_msg = (_('iDRAC Redfish Get pxe port MAC addresse ' - '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) return pxe_port_macs diff --git a/ironic/drivers/modules/drac/management.py b/ironic/drivers/modules/drac/management.py index 88b7c4bbe8..c7ab389d98 100644 --- a/ironic/drivers/modules/drac/management.py +++ b/ironic/drivers/modules/drac/management.py @@ -40,6 +40,7 @@ from ironic.drivers import base from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.drac import common as drac_common from ironic.drivers.modules.drac import job as drac_job +from ironic.drivers.modules.drac import utils as drac_utils from ironic.drivers.modules.redfish import management as redfish_management from ironic.drivers.modules.redfish import utils as redfish_utils @@ -362,35 +363,9 @@ class DracRedfishManagement(redfish_management.RedfishManagement): error=(_("No managers found for %(node)s") % {'node': task.node.uuid})) - for manager in system.managers: - - 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: - configuration = manager_oem.export_system_configuration() - LOG.info("Exported %(node)s configuration via OEM", - {'node': task.node.uuid}) - except sushy.exceptions.SushyError as e: - LOG.debug("Sushy OEM extension Python package " - "'sushy-oem-idrac' failed to export system " - " configuration 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 - break + configuration = drac_utils.execute_oem_manager_method( + task, 'export system configuration', + lambda m: m.export_system_configuration()) if configuration and configuration.status_code == 200: configuration = {"oem": {"interface": "idrac-redfish", @@ -442,58 +417,25 @@ class DracRedfishManagement(redfish_management.RedfishManagement): 'configuration_name': import_configuration_location, 'interface': interface})) - system = redfish_utils.get_system(task.node) + task_monitor = drac_utils.execute_oem_manager_method( + task, 'import system configuration', + lambda m: m.import_system_configuration( + json.dumps(configuration["oem"]["data"])),) - if not system.managers: - raise exception.DracOperationError( - error=(_("No managers found for %(node)s") % - {'node': task.node.uuid})) + info = task.node.driver_internal_info + info['import_task_monitor_url'] = task_monitor.task_monitor_uri + task.node.driver_internal_info = info - for manager in system.managers: - 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) + deploy_utils.set_async_step_flags( + task.node, + reboot=True, + skip_current_step=True, + polling=True) + deploy_opts = deploy_utils.build_agent_options(task.node) + task.driver.boot.prepare_ramdisk(task, deploy_opts) + manager_utils.node_power_action(task, states.REBOOT) - try: - task_monitor = manager_oem.import_system_configuration( - json.dumps(configuration["oem"]["data"])) - - info = task.node.driver_internal_info - info['import_task_monitor_url'] = task_monitor.task_monitor_uri - task.node.driver_internal_info = info - - deploy_utils.set_async_step_flags( - task.node, - reboot=True, - skip_current_step=True, - polling=True) - deploy_opts = deploy_utils.build_agent_options(task.node) - task.driver.boot.prepare_ramdisk(task, deploy_opts) - manager_utils.node_power_action(task, states.REBOOT) - - return deploy_utils.get_async_step_return_state(task.node) - except sushy.exceptions.SushyError as e: - LOG.debug("Sushy OEM extension Python package " - "'sushy-oem-idrac' failed to import system " - " configuration 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 - - raise exception.DracOperationError( - error=(_("Failed to import configuration for node %(node)s") % - {'node': task.node.uuid})) + return deploy_utils.get_async_step_return_state(task.node) @base.clean_step(priority=0, argsinfo=IMPORT_EXPORT_CONFIGURATION_ARGSINFO) @@ -646,46 +588,9 @@ class DracRedfishManagement(redfish_management.RedfishManagement): on. :raises: RedfishError on an error. """ - system = redfish_utils.get_system(task.node) - for manager in system.managers: - try: - oem_manager = 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: - oem_manager.job_service.delete_jobs(job_ids=['JID_CLEARALL']) - except sushy.exceptions.SushyError as e: - error_msg = ('Failed to clear iDRAC job queue 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}) - LOG.debug(error_msg) - continue - LOG.info('Cleared iDRAC job queue for node %(node)s', - {'node': task.node.uuid}) - break - else: - error_msg = (_('iDRAC Redfish clear job queue 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, 'clear job queue', + lambda m: m.job_service.delete_jobs(job_ids=['JID_CLEARALL'])) @METRICS.timer('DracRedfishManagement.reset_idrac') @base.clean_step(priority=0) @@ -696,45 +601,11 @@ class DracRedfishManagement(redfish_management.RedfishManagement): on. :raises: RedfishError on an error. """ - system = redfish_utils.get_system(task.node) - for manager in system.managers: - try: - oem_manager = 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: - oem_manager.reset_idrac() - except sushy.exceptions.SushyError as e: - error_msg = ('Failed to reset iDRAC 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}) - LOG.debug(error_msg) - continue - redfish_utils.wait_until_get_system_ready(task.node) - LOG.info('Reset iDRAC for node %(node)s', {'node': task.node.uuid}) - break - else: - error_msg = (_('iDRAC Redfish reset iDRAC 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, 'reset iDRAC', lambda m: m.reset_idrac()) + redfish_utils.wait_until_get_system_ready(task.node) + LOG.info('Reset iDRAC for node %(node)s done', + {'node': task.node.uuid}) @METRICS.timer('DracRedfishManagement.known_good_state') @base.clean_step(priority=0) diff --git a/ironic/drivers/modules/drac/raid.py b/ironic/drivers/modules/drac/raid.py index 2d3e639a84..b42b5b21fc 100644 --- a/ironic/drivers/modules/drac/raid.py +++ b/ironic/drivers/modules/drac/raid.py @@ -35,8 +35,8 @@ from ironic.drivers import base from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.drac import common as drac_common from ironic.drivers.modules.drac import job as drac_job +from ironic.drivers.modules.drac import utils as drac_utils from ironic.drivers.modules.redfish import raid as redfish_raid -from ironic.drivers.modules.redfish import utils as redfish_utils drac_exceptions = importutils.try_import('dracclient.exceptions') drac_constants = importutils.try_import('dracclient.constants') @@ -1209,44 +1209,9 @@ def _is_realtime_ready(task): :raises RedfishError: If can't find OEM extension or it fails to execute """ - system = redfish_utils.get_system(task.node) - for manager in system.managers: - 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: - return manager_oem.lifecycle_service.is_realtime_ready() - except sushy.exceptions.SushyError as e: - LOG.debug("Failed to get real time ready status 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 - break - - else: - error_msg = (_("iDRAC Redfish get real time ready status 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) + return drac_utils.execute_oem_manager_method( + task, 'get real-time ready status', + lambda m: m.lifecycle_service.is_realtime_ready()) class DracRedfishRAID(redfish_raid.RedfishRAID): 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_inspect.py b/ironic/tests/unit/drivers/modules/drac/test_inspect.py index 70e94ce49e..97dcf30f9d 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/drac/test_inspect.py @@ -635,33 +635,6 @@ class DracRedfishInspectionTestCase(test_utils.BaseDracTest): pxe_port_macs = task.driver.inspect._get_pxe_port_macs(task) self.assertEqual(expected_pxe_mac, pxe_port_macs) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test__get_pxe_port_macs_missing_oem(self, mock_get_system): - mock_system = self.init_system_mock(mock_get_system.return_value) - mock_manager = mock.MagicMock() - mock_system.boot.mode = 'bios' - mock_system.managers = [mock_manager] - set_mgr = ( - mock_manager.get_oem_extension.return_value.get_pxe_port_macs_bios) - set_mgr.side_effect = sushy.exceptions.OEMExtensionNotFoundError - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - self.assertRaises(exception.RedfishError, - task.driver.inspect._get_pxe_port_macs, - task) - - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test__get_pxe_port_macs_no_manager(self, mock_get_system): - mock_system = self.init_system_mock(mock_get_system.return_value) - mock_system.boot.mode = 'bios' - mock_system.managers = [] - with task_manager.acquire(self.context, self.node.uuid, - shared=True) as task: - self.assertRaises(exception.RedfishError, - task.driver.inspect._get_pxe_port_macs, - task) - @mock.patch.object(redfish_inspect.RedfishInspect, 'inspect_hardware', autospec=True) @mock.patch.object(inspect_utils, 'create_ports_if_not_exist', diff --git a/ironic/tests/unit/drivers/modules/drac/test_management.py b/ironic/tests/unit/drivers/modules/drac/test_management.py index d367fca8b7..b93c566545 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_management.py +++ b/ironic/tests/unit/drivers/modules/drac/test_management.py @@ -34,6 +34,7 @@ from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.drac import common as drac_common from ironic.drivers.modules.drac import job as drac_job from ironic.drivers.modules.drac import management as drac_mgmt +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 @@ -848,50 +849,6 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest): self.assertRaises(exception.MissingParameterValue, self.management.export_configuration, task, None) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_export_configuration_no_managers(self, mock_get_system): - task = mock.Mock(node=self.node, context=self.context) - mock_get_system.return_value.managers = [] - - self.assertRaises(exception.DracOperationError, - self.management.export_configuration, task, 'edge') - - @mock.patch.object(drac_mgmt, 'LOG', autospec=True) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_export_configuration_oem_not_found(self, mock_get_system, - mock_log): - task = mock.Mock(node=self.node, context=self.context) - fake_manager1 = mock.Mock() - fake_manager1.get_oem_extension.side_effect = ( - sushy.exceptions.OEMExtensionNotFoundError) - mock_get_system.return_value.managers = [fake_manager1] - - self.assertRaises(exception.RedfishError, - self.management.export_configuration, task, 'edge') - self.assertEqual(mock_log.error.call_count, 1) - - @mock.patch.object(drac_mgmt, 'LOG', autospec=True) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_export_configuration_all_managers_fail(self, mock_get_system, - mock_log): - task = mock.Mock(node=self.node, context=self.context) - fake_manager_oem1 = mock.Mock() - fake_manager_oem1.export_system_configuration.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.export_system_configuration.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] - - self.assertRaises(exception.DracOperationError, - self.management.export_configuration, - task, 'edge') - self.assertEqual(mock_log.debug.call_count, 2) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_export_configuration_export_failed(self, mock_get_system): task = mock.Mock(node=self.node, context=self.context) @@ -905,7 +862,7 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest): self.assertRaises(exception.DracOperationError, self.management.export_configuration, task, 'edge') - @mock.patch.object(drac_mgmt, 'LOG', autospec=True) + @mock.patch.object(drac_utils, 'LOG', autospec=True) @mock.patch.object(molds, 'save_configuration', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_export_configuration_success(self, mock_get_system, @@ -950,66 +907,6 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest): self.assertRaises(exception.DracOperationError, self.management.import_configuration, task, 'edge') - @mock.patch.object(molds, 'get_configuration', autospec=True) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_import_configuration_no_managers(self, mock_get_system, - mock_get_configuration): - task = mock.Mock(node=self.node, context=self.context) - fake_system = mock.Mock(managers=[]) - mock_get_configuration.return_value = json.loads( - '{"oem": {"interface": "idrac-redfish", ' - '"data": {"prop1": "value1", "prop2": 2}}}') - mock_get_system.return_value = fake_system - - self.assertRaises(exception.DracOperationError, - self.management.import_configuration, task, 'edge') - - @mock.patch.object(drac_mgmt, 'LOG', autospec=True) - @mock.patch.object(molds, 'get_configuration', autospec=True) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_import_configuration_oem_not_found(self, mock_get_system, - mock_get_configuration, - mock_log): - task = mock.Mock(node=self.node, context=self.context) - fake_manager1 = mock.Mock() - fake_manager1.get_oem_extension.side_effect = ( - sushy.exceptions.OEMExtensionNotFoundError) - fake_system = mock.Mock(managers=[fake_manager1]) - mock_get_system.return_value = fake_system - mock_get_configuration.return_value = json.loads( - '{"oem": {"interface": "idrac-redfish", ' - '"data": {"prop1": "value1", "prop2": 2}}}') - - self.assertRaises(exception.RedfishError, - self.management.import_configuration, task, 'edge') - self.assertEqual(mock_log.error.call_count, 1) - - @mock.patch.object(drac_mgmt, 'LOG', autospec=True) - @mock.patch.object(molds, 'get_configuration', autospec=True) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test_import_configuration_all_managers_fail(self, mock_get_system, - mock_get_configuration, - mock_log): - task = mock.Mock(node=self.node, context=self.context) - fake_manager_oem1 = mock.Mock() - fake_manager_oem1.import_system_configuration.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.import_system_configuration.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] - mock_get_configuration.return_value = json.loads( - '{"oem": {"interface": "idrac-redfish", ' - '"data": {"prop1": "value1", "prop2": 2}}}') - - self.assertRaises(exception.DracOperationError, - self.management.import_configuration, task, 'edge') - self.assertEqual(mock_log.debug.call_count, 2) - @mock.patch.object(molds, 'get_configuration', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_import_configuration_incorrect_interface(self, mock_get_system, @@ -1031,7 +928,7 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest): @mock.patch.object(deploy_utils, 'set_async_step_flags', autospec=True) @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) @mock.patch.object(manager_utils, 'node_power_action', autospec=True) - @mock.patch.object(drac_mgmt, 'LOG', autospec=True) + @mock.patch.object(drac_utils, 'LOG', autospec=True) @mock.patch.object(molds, 'get_configuration', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test_import_configuration_success( @@ -1420,7 +1317,7 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest): mock_deploy_handler.assert_called_once_with( task, 'error', 'log message') - @mock.patch.object(drac_mgmt, 'redfish_utils', autospec=True) + @mock.patch.object(drac_utils, 'redfish_utils', autospec=True) def test_clear_job_queue(self, mock_redfish_utils): mock_system = mock_redfish_utils.get_system.return_value mock_manager = mock.MagicMock() @@ -1433,8 +1330,10 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest): mock_manager_oem.job_service.delete_jobs.assert_called_once_with( job_ids=['JID_CLEARALL']) - @mock.patch.object(drac_mgmt, 'redfish_utils', autospec=True) - def test_reset_idrac(self, mock_redfish_utils): + @mock.patch.object(redfish_utils, 'wait_until_get_system_ready', + autospec=True) + @mock.patch.object(drac_utils, 'redfish_utils', autospec=True) + def test_reset_idrac(self, mock_redfish_utils, mock_wait_system_ready): mock_system = mock_redfish_utils.get_system.return_value mock_manager = mock.MagicMock() mock_system.managers = [mock_manager] @@ -1445,8 +1344,11 @@ class DracRedfishManagementTestCase(test_utils.BaseDracTest): task.driver.management.reset_idrac(task) mock_manager_oem.reset_idrac.assert_called_once_with() - @mock.patch.object(drac_mgmt, 'redfish_utils', autospec=True) - def test_known_good_state(self, mock_redfish_utils): + @mock.patch.object(redfish_utils, 'wait_until_get_system_ready', + autospec=True) + @mock.patch.object(drac_utils, 'redfish_utils', autospec=True) + def test_known_good_state(self, mock_redfish_utils, + mock_wait_system_ready): mock_system = mock_redfish_utils.get_system.return_value mock_manager = mock.MagicMock() mock_system.managers = [mock_manager] diff --git a/ironic/tests/unit/drivers/modules/drac/test_raid.py b/ironic/tests/unit/drivers/modules/drac/test_raid.py index 3dd48f1b0e..84ac06af66 100644 --- a/ironic/tests/unit/drivers/modules/drac/test_raid.py +++ b/ironic/tests/unit/drivers/modules/drac/test_raid.py @@ -30,6 +30,7 @@ from ironic.drivers import base from ironic.drivers.modules.drac import common as drac_common from ironic.drivers.modules.drac import job as drac_job from ironic.drivers.modules.drac import raid as drac_raid +from ironic.drivers.modules.drac import utils as drac_utils from ironic.drivers.modules.redfish import raid as redfish_raid from ironic.drivers.modules.redfish import utils as redfish_utils from ironic.tests.unit.drivers.modules.drac import utils as test_utils @@ -2333,49 +2334,7 @@ class DracRedfishRAIDTestCase(test_utils.BaseDracTest): self.assertTrue(is_ready) self.assertEqual(2, mock_ready.call_count) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test__is_realtime_ready_no_managers(self, mock_get_system): - task = mock.Mock(node=self.node, context=self.context) - fake_system = mock.Mock(managers=[]) - mock_get_system.return_value = fake_system - self.assertRaises(exception.RedfishError, - drac_raid._is_realtime_ready, task) - - @mock.patch.object(drac_raid, 'LOG', autospec=True) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test__is_realtime_ready_oem_not_found(self, mock_get_system, mock_log): - task = mock.Mock(node=self.node, context=self.context) - fake_manager1 = mock.Mock() - fake_manager1.get_oem_extension.side_effect = ( - sushy.exceptions.OEMExtensionNotFoundError) - fake_system = mock.Mock(managers=[fake_manager1]) - mock_get_system.return_value = fake_system - self.assertRaises(exception.RedfishError, - drac_raid._is_realtime_ready, task) - self.assertEqual(mock_log.error.call_count, 1) - - @mock.patch.object(drac_raid, 'LOG', autospec=True) - @mock.patch.object(redfish_utils, 'get_system', autospec=True) - def test__is_realtime_ready_all_managers_fail(self, mock_get_system, - mock_log): - task = mock.Mock(node=self.node, context=self.context) - fake_manager_oem1 = mock.Mock() - fake_manager_oem1.lifecycle_service.is_realtime_ready.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.lifecycle_service.is_realtime_ready.side_effect = ( - sushy.exceptions.SushyError) - fake_manager2 = mock.Mock() - fake_manager2.get_oem_extension.return_value = fake_manager_oem2 - fake_system = mock.Mock(managers=[fake_manager1, fake_manager2]) - mock_get_system.return_value = fake_system - self.assertRaises(exception.RedfishError, - drac_raid._is_realtime_ready, task) - self.assertEqual(mock_log.debug.call_count, 2) - - @mock.patch.object(drac_raid, 'LOG', autospec=True) + @mock.patch.object(drac_utils, 'LOG', autospec=True) @mock.patch.object(redfish_utils, 'get_system', autospec=True) def test__is_realtime_ready(self, mock_get_system, mock_log): task = mock.Mock(node=self.node, context=self.context) 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())