From 4eb0dbf7b5cd5af8b97c4b02c54a6f58f8da704a Mon Sep 17 00:00:00 2001 From: Iury Gregory Melo Ferreira Date: Tue, 6 Jun 2023 23:51:54 -0300 Subject: [PATCH] RedfishFirmware Interface Change-Id: I75b2433fade0c36522024c16608d61cd663b38d5 --- .../baremetal-api-v1-nodes-firmware.inc | 2 +- ironic/api/controllers/v1/utils.py | 2 +- ironic/conductor/cleaning.py | 2 + ironic/conductor/steps.py | 21 +- ironic/conductor/utils.py | 12 + ironic/drivers/modules/deploy_utils.py | 6 +- ironic/drivers/modules/fake.py | 2 + ironic/drivers/modules/redfish/firmware.py | 452 ++++++++++++++++++ .../drivers/modules/redfish/firmware_utils.py | 44 ++ ironic/drivers/modules/redfish/utils.py | 36 ++ ironic/drivers/redfish.py | 5 + ironic/objects/firmware.py | 14 +- ironic/tests/unit/conductor/test_steps.py | 9 + .../drivers/modules/ilo/test_management.py | 8 +- .../unit/drivers/modules/ilo/test_raid.py | 4 +- .../unit/drivers/modules/redfish/test_bios.py | 5 +- .../drivers/modules/redfish/test_firmware.py | 40 ++ .../modules/redfish/test_management.py | 9 +- .../unit/drivers/modules/redfish/test_raid.py | 6 +- .../unit/drivers/modules/test_agent_base.py | 16 +- ironic/tests/unit/drivers/test_redfish.py | 3 +- .../firmware-interface-8ad6f91aa1f746a0.yaml | 31 ++ setup.cfg | 1 + 23 files changed, 690 insertions(+), 40 deletions(-) create mode 100644 ironic/drivers/modules/redfish/firmware.py create mode 100644 ironic/tests/unit/drivers/modules/redfish/test_firmware.py create mode 100644 releasenotes/notes/firmware-interface-8ad6f91aa1f746a0.yaml diff --git a/api-ref/source/baremetal-api-v1-nodes-firmware.inc b/api-ref/source/baremetal-api-v1-nodes-firmware.inc index ed17e0b7b2..15ea99270d 100644 --- a/api-ref/source/baremetal-api-v1-nodes-firmware.inc +++ b/api-ref/source/baremetal-api-v1-nodes-firmware.inc @@ -4,7 +4,7 @@ Node Firmware (nodes) ===================== -.. versionadded:: 1.84 +.. versionadded:: 1.86 Given a Node identifier (``uuid`` or ``name``), the API exposes the list of all Firmware Components associated with that Node. diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 10c631df81..3a7806ecad 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -2018,6 +2018,6 @@ def allow_continue_inspection_endpoint(): def allow_firmware_interface(): """Check if we should support firmware interface and endpoints. - Version 1.84 of the API added support for firmware interface. + Version 1.86 of the API added support for firmware interface. """ return api.request.version.minor >= versions.MINOR_86_FIRMWARE_INTERFACE diff --git a/ironic/conductor/cleaning.py b/ironic/conductor/cleaning.py index 1e79f971b3..cd2d999397 100644 --- a/ironic/conductor/cleaning.py +++ b/ironic/conductor/cleaning.py @@ -85,6 +85,8 @@ def do_node_clean(task, clean_steps=None, disable_ramdisk=False): # Retrieve BIOS config settings for this node utils.node_cache_bios_settings(task, node) + # Retrieve Firmware Components for this node if possible + utils.node_cache_firmware_components(task) # Allow the deploy driver to set up the ramdisk again (necessary for # IPA cleaning) try: diff --git a/ironic/conductor/steps.py b/ironic/conductor/steps.py index 3dfbb2e854..dcdfa469cb 100644 --- a/ironic/conductor/steps.py +++ b/ironic/conductor/steps.py @@ -31,9 +31,10 @@ CLEANING_INTERFACE_PRIORITY = { # by which interface is implementing the clean step. The clean step of the # interface with the highest value here, will be executed first in that # case. - 'vendor': 6, - 'power': 5, - 'management': 4, + 'vendor': 7, + 'power': 6, + 'management': 5, + 'firmware': 4, 'deploy': 3, 'bios': 2, 'raid': 1, @@ -46,9 +47,10 @@ DEPLOYING_INTERFACE_PRIORITY = { # TODO(rloo): If we think it makes sense to have the interface priorities # the same for cleaning & deploying, replace the two with one e.g. # 'INTERFACE_PRIORITIES'. - 'vendor': 6, - 'power': 5, - 'management': 4, + 'vendor': 7, + 'power': 6, + 'management': 5, + 'firmware': 4, 'deploy': 3, 'bios': 2, 'raid': 1, @@ -61,11 +63,12 @@ VERIFYING_INTERFACE_PRIORITY = { # by which interface is implementing the verify step. The verifying step of # the interface with the highest value here, will be executed first in # that case. - 'power': 12, - 'management': 11, - 'boot': 8, + 'power': 13, + 'management': 12, + 'firmware': 11, 'inspect': 10, 'deploy': 9, + 'boot': 8, 'bios': 7, 'raid': 6, 'vendor': 5, diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 1255c8691f..e79ce3d209 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -1828,3 +1828,15 @@ def servicing_error_handler(task, logmsg, errmsg=None, traceback=False, if set_fail_state and node.provision_state != states.SERVICEFAIL: task.process_event('fail') + + +def node_cache_firmware_components(task): + """Do caching of firmware components if supported by driver""" + + try: + LOG.debug('Getting Firmware Components for node %s', task.node.uuid) + task.driver.firmware.validate(task) + task.driver.firmware.cache_firmware_components(task) + except exception.UnsupportedDriverExtension: + LOG.warning('Firmware Components are not supported for node %s, ' + 'skipping', task.node.uuid) diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 010e384d1c..18a506869b 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -1536,10 +1536,12 @@ def prepare_agent_boot(task): task.driver.boot.prepare_ramdisk(task, deploy_opts) -def reboot_to_finish_step(task): +def reboot_to_finish_step(task, timeout=None): """Reboot the node into IPA to finish a deploy/clean step. :param task: a TaskManager instance. + :param timeout: timeout (in seconds) positive integer (> 0) for any + power state. ``None`` indicates to use default timeout. :returns: states.CLEANWAIT if cleaning operation in progress or states.DEPLOYWAIT if deploy operation in progress. """ @@ -1552,7 +1554,7 @@ def reboot_to_finish_step(task): manager_utils.node_power_action(task, states.POWER_OFF) prepare_agent_boot(task) - manager_utils.node_power_action(task, states.REBOOT) + manager_utils.node_power_action(task, states.REBOOT, timeout) return get_async_step_return_state(task.node) diff --git a/ironic/drivers/modules/fake.py b/ironic/drivers/modules/fake.py index 625ffbb172..0889098c78 100644 --- a/ironic/drivers/modules/fake.py +++ b/ironic/drivers/modules/fake.py @@ -477,6 +477,8 @@ class FakeFirmware(base.FirmwareInterface): 'needs to contain a dictionary with name/value pairs'), 'required': True}}) def update(self, task, settings): + LOG.debug('Calling update clean step with settings %s.', + settings) sleep(CONF.fake.firmware_delay) def cache_firmware_components(self, task): diff --git a/ironic/drivers/modules/redfish/firmware.py b/ironic/drivers/modules/redfish/firmware.py new file mode 100644 index 0000000000..207c61aadf --- /dev/null +++ b/ironic/drivers/modules/redfish/firmware.py @@ -0,0 +1,452 @@ +# +# 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 urllib.parse import urlparse + +from ironic_lib import metrics_utils +from oslo_log import log +from oslo_utils import importutils +from oslo_utils import timeutils + +from ironic.common import exception +from ironic.common.i18n import _ +from ironic.common import states +from ironic.conductor import periodics +from ironic.conductor import utils as manager_utils +from ironic.conf import CONF +from ironic.drivers import base +from ironic.drivers.modules import deploy_utils +from ironic.drivers.modules.redfish import firmware_utils +from ironic.drivers.modules.redfish import utils as redfish_utils +from ironic import objects + +LOG = log.getLogger(__name__) + +METRICS = metrics_utils.get_metrics_logger(__name__) + +sushy = importutils.try_import('sushy') + + +class RedfishFirmware(base.FirmwareInterface): + + _FW_SETTINGS_ARGSINFO = { + 'settings': { + 'description': ( + 'A list of dicts with firmware components to be updated' + ), + 'required': True + } + } + + def __init__(self): + super(RedfishFirmware, self).__init__() + if sushy is None: + raise exception.DriverLoadError( + driver='redfish', + reason=_("Unable to import the sushy library")) + + def get_properties(self): + """Return the properties of the interface. + + :returns: dictionary of : entries. + """ + return redfish_utils.COMMON_PROPERTIES.copy() + + def validate(self, task): + """Validates the driver information needed by the redfish driver. + + :param task: a TaskManager instance containing the node to act on. + :raises: InvalidParameterValue on malformed parameter(s) + :raises: MissingParameterValue on missing parameter(s) + """ + redfish_utils.parse_driver_info(task.node) + + def cache_firmware_components(self, task): + """Store or update Firmware Components on the given node. + + This method stores Firmware Components to the firmware_information + table during 'cleaning' operation. It will also update the timestamp + of each Firmware Component. + + :param task: a TaskManager instance. + :raises: UnsupportedDriverExtension, if the node's driver doesn't + support getting Firmware Components from bare metal. + """ + + node_id = task.node.id + settings = [] + # NOTE(iurygregory): currently we will only retrieve BIOS and BMC + # firmware information trough the redfish system and manager. + + system = redfish_utils.get_system(task.node) + + bios_fw = {'component': 'bios', + 'current_version': system.bios_version} + settings.append(bios_fw) + + # NOTE(iurygregory): normally we only relay on the System to + # perform actions, but to retrieve the BMC Firmware we need to + # access the Manager. + try: + manager = redfish_utils.get_manager(task.node, system) + bmc_fw = {'component': 'bmc', + 'current_version': manager.firmware_version} + settings.append(bmc_fw) + except exception.RedfishError: + LOG.warning('No manager available to retrieve Firmware ' + 'from the bmc of node %s', task.node.uuid) + + if not settings: + error_msg = (_('Cannot retrieve firmware for node %s.') + % task.node.uuid) + LOG.error(error_msg) + raise exception.UnsupportedDriverExtension(error_msg) + + create_list, update_list, nochange_list = ( + objects.FirmwareComponentList.sync_firmware_components( + task.context, node_id, settings)) + + if create_list: + for new_fw in create_list: + new_fw_cmp = objects.FirmwareComponent( + task.context, + node_id=node_id, + component=new_fw['component'], + current_version=new_fw['current_version'] + ) + new_fw_cmp.create() + if update_list: + for up_fw in update_list: + up_fw_cmp = objects.FirmwareComponent.get( + task.context, + node_id=node_id, + name=up_fw['component'] + ) + up_fw_cmp.last_version_flashed = up_fw.get('current_version') + up_fw_cmp.current_version = up_fw.get('current_version') + up_fw_cmp.save() + + @METRICS.timer('RedfishFirmware.update') + @base.deploy_step(priority=0, argsinfo=_FW_SETTINGS_ARGSINFO) + @base.clean_step(priority=0, abortable=False, + argsinfo=_FW_SETTINGS_ARGSINFO, + requires_ramdisk=True) + @base.cache_firmware_components + def update(self, task, settings): + """Update the Firmware on the node using the settings for components. + + :param task: a TaskManager instance. + :param settings: a list of dictionaries, each dictionary contains the + component name and the url that will be used to update the + firmware. + :raises: UnsupportedDriverExtension, if the node's driver doesn't + support update via the interface. + :raises: InvalidParameterValue, if validation of the settings fails. + :raises: MissingParamterValue, if some required parameters are + missing. + :returns: states.CLEANWAIT if Firmware update with the settings is in + progress asynchronously of None if it is complete. + """ + node = task.node + + update_service = redfish_utils.get_update_service(node) + + LOG.debug('Updating Firmware on node %(node_uuid)s with settings ' + '%(settings)s', + {'node_uuid': node.uuid, 'settings': settings}) + + self._execute_firmware_update(node, update_service, settings) + + fw_upd = settings[0] + wait_interval = fw_upd.get('wait') + + deploy_utils.set_async_step_flags( + node, + reboot=True, + skip_current_step=True, + polling=True + ) + + return deploy_utils.reboot_to_finish_step(task, timeout=wait_interval) + + def _execute_firmware_update(self, node, update_service, settings): + """Executes the next firmware update to the node + + Executes the first firmware update in the settings list to the node. + + :param node: the node that will have a firmware update executed. + :param update_service: the sushy firmware update service. + :param settings: remaining settings for firmware update that needs + to be executed. + """ + fw_upd = settings[0] + component_url, cleanup = self._stage_firmware_file(node, fw_upd) + + LOG.debug('Applying new firmware %(url)s for %(component)s on node ' + '%(node_uuid)s', + {'url': fw_upd['url'], 'component': fw_upd['component'], + 'node_uuid': node.uuid}) + + task_monitor = update_service.simple_update(component_url) + + fw_upd['task_monitor'] = task_monitor.task_monitor_uri + node.set_driver_internal_info('redfish_fw_updates', settings) + + if cleanup: + fw_clean = node.driver_internal_info.get('firmware_cleanup') + if not fw_clean: + fw_clean = [cleanup] + elif cleanup not in fw_clean: + fw_clean.append(cleanup) + node.set_driver_internal_info('firmware_cleanup', fw_clean) + + def _continue_updates(self, task, update_service, settings): + """Continues processing the firmware updates + + Continues to process the firmware updates on the node. + + Note that the caller must have an exclusive lock on the node. + + :param task: a TaskManager instance containing the node to act on. + :param update_service: the sushy firmware update service + :param settings: the remaining firmware updates to apply + """ + node = task.node + fw_upd = settings[0] + wait_interval = fw_upd.get('wait') + if wait_interval: + time_now = str(timeutils.utcnow().isoformat()) + fw_upd['wait_start_time'] = time_now + + LOG.debug('Waiting at %(time)s for %(seconds)s seconds after ' + '%(component)s firmware update %(url)s ' + 'on node %(node)s', + {'time': time_now, + 'seconds': wait_interval, + 'component': fw_upd['component'], + 'url': fw_upd['url'], + 'node': node.uuid}) + + node.set_driver_internal_info('redfish_fw_updates', settings) + node.save() + return + + if len(settings) == 1: + self._clear_updates(node) + + LOG.info('Firmware updates completed for node %(node)s', + {'node': node.uuid}) + + manager_utils.notify_conductor_resume_clean(task) + else: + settings.pop(0) + self._execute_firmware_update(node, + update_service, + settings) + node.save() + manager_utils.node_power_action(task, states.REBOOT) + + def _clear_updates(self, node): + """Clears firmware updates artifacts + + Clears firmware updates from driver_internal_info and any files + that were staged. + + Note that the caller must have an exclusive lock on the node. + + :param node: the node to clear the firmware updates from + """ + firmware_utils.cleanup(node) + node.del_driver_internal_info('redfish_fw_updates') + node.del_driver_internal_info('firmware_cleanup') + node.save() + + @METRICS.timer('RedfishFirmware._query_update_failed') + @periodics.node_periodic( + purpose='checking if async update of firmware component failed', + spacing=CONF.redfish.firmware_update_fail_interval, + filters={'reserved': False, 'provision_state': states.CLEANFAIL, + 'maintenance': True}, + predicate_extra_fields=['driver_internal_info'], + predicate=lambda n: n.driver_internal_info.get('redfish_fw_updates'), + ) + def _query_update_failed(self, task, manager, context): + + """Periodic job to check for failed firmware updates.""" + # A firmware update failed. Discard any remaining firmware + # updates so when the user takes the node out of + # maintenance mode, pending firmware updates do not + # automatically continue. + LOG.error('Update firmware failed for node %(node)s. ' + 'Discarding remaining firmware updates.', + {'node': task.node.uuid}) + + task.upgrade_lock() + self._clear_updates(task.node) + + @METRICS.timer('RedfishFirmware._query_update_status') + @periodics.node_periodic( + purpose='checking async update of firmware component', + spacing=CONF.redfish.firmware_update_fail_interval, + filters={'reserved': False, 'provision_state': states.CLEANWAIT}, + predicate_extra_fields=['driver_internal_info'], + predicate=lambda n: n.driver_internal_info.get('redfish_fw_updates'), + ) + def _query_update_status(self, task, manager, context): + """Periodic job to check firmware update tasks.""" + self._check_node_redfish_firmware_update(task) + + @METRICS.timer('RedfishFirmware._check_node_redfish_firmware_update') + def _check_node_redfish_firmware_update(self, task): + """Check the progress of running firmware update on a node.""" + + node = task.node + + settings = node.driver_internal_info['redfish_fw_updates'] + current_update = settings[0] + + try: + update_service = redfish_utils.get_update_service(node) + except exception.RedfishConnectionError as e: + # If the BMC firmware is being updated, the BMC will be + # unavailable for some amount of time. + LOG.warning('Unable to communicate with firmware update service ' + 'on node %(node)s. Will try again on the next poll. ' + 'Error: %(error)s', + {'node': node.uuid, + 'error': e}) + return + + wait_start_time = current_update.get('wait_start_time') + if wait_start_time: + wait_start = timeutils.parse_isotime(wait_start_time) + + elapsed_time = timeutils.utcnow(True) - wait_start + if elapsed_time.seconds >= current_update['wait']: + LOG.debug('Finished waiting after firmware update ' + '%(firmware_image)s on node %(node)s. ' + 'Elapsed time: %(seconds)s seconds', + {'firmware_image': current_update['url'], + 'node': node.uuid, + 'seconds': elapsed_time.seconds}) + current_update.pop('wait', None) + current_update.pop('wait_start_time', None) + + self._continue_updates(task, update_service, settings) + else: + LOG.debug('Continuing to wait after firmware update ' + '%(firmware_image)s on node %(node)s. ' + 'Elapsed time: %(seconds)s seconds', + {'firmware_image': current_update['url'], + 'node': node.uuid, + 'seconds': elapsed_time.seconds}) + + return + + try: + task_monitor = redfish_utils.get_task_monitor( + node, current_update['task_monitor']) + except exception.RedfishError: + # The BMC deleted the Task before we could query it + LOG.warning('Firmware update completed for node %(node)s, ' + 'firmware %(firmware_image)s, but success of the ' + 'update is unknown. Assuming update was successful.', + {'node': node.uuid, + 'firmware_image': current_update['url']}) + self._continue_updates(task, update_service, settings) + return + + if not task_monitor.is_processing: + # The last response does not necessarily contain a Task, + # so get it + sushy_task = task_monitor.get_task() + + # Only parse the messages if the BMC did not return parsed + # messages + messages = [] + if sushy_task.messages and not sushy_task.messages[0].message: + sushy_task.parse_messages() + + if sushy_task.messages is not None: + messages = [m.message for m in sushy_task.messages] + + task.upgrade_lock() + if (sushy_task.task_state == sushy.TASK_STATE_COMPLETED + and sushy_task.task_status in + [sushy.HEALTH_OK, sushy.HEALTH_WARNING]): + LOG.info('Firmware update succeeded for node %(node)s, ' + 'firmware %(firmware_image)s: %(messages)s', + {'node': node.uuid, + 'firmware_image': current_update['url'], + 'messages': ", ".join(messages)}) + + self._continue_updates(task, update_service, settings) + else: + error_msg = (_('Firmware update failed for node %(node)s, ' + 'firmware %(firmware_image)s. ' + 'Error: %(errors)s') % + {'node': node.uuid, + 'firmware_image': current_update['url'], + 'errors': ", ".join(messages)}) + + self._clear_updates(node) + if task.node.clean_step: + manager_utils.cleaning_error_handler(task, error_msg) + else: + manager_utils.deploying_error_handler(task, error_msg) + + else: + LOG.debug('Firmware update in progress for node %(node)s, ' + 'firmware %(firmware_image)s.', + {'node': node.uuid, + 'firmware_image': current_update['url']}) + + def _stage_firmware_file(self, node, component_update): + + try: + url = component_update['url'] + name = component_update['component'] + parsed_url = urlparse(url) + scheme = parsed_url.scheme.lower() + source = (CONF.redfish.firmware_source).lower() + + # Keep it simple, in further processing TLS does not matter + if scheme == 'https': + scheme = 'http' + + # If source and scheme is HTTP, then no staging, + # returning original location + if scheme == 'http' and source == scheme: + LOG.debug('For node %(node)s serving firmware for ' + '%(component)s from original location %(url)s', + {'node': node.uuid, 'component': name, 'url': url}) + return url, None + + # If source and scheme is Swift, then not moving, but + # returning Swift temp URL + if scheme == 'swift' and source == scheme: + temp_url = firmware_utils.get_swift_temp_url(parsed_url) + LOG.debug('For node %(node)s serving original firmware at ' + 'for %(component)s at %(url)s via Swift temporary ' + 'url %(temp_url)s', + {'node': node.uuid, 'component': name, 'url': url, + 'temp_url': temp_url}) + return temp_url, None + + # For remaining, download the image to temporary location + temp_file = firmware_utils.download_to_temp(node, url) + + return firmware_utils.stage(node, source, temp_file) + + except exception.IronicException: + firmware_utils.cleanup(node) diff --git a/ironic/drivers/modules/redfish/firmware_utils.py b/ironic/drivers/modules/redfish/firmware_utils.py index feeec2df23..843597d8f7 100644 --- a/ironic/drivers/modules/redfish/firmware_utils.py +++ b/ironic/drivers/modules/redfish/firmware_utils.py @@ -63,6 +63,36 @@ _UPDATE_FIRMWARE_SCHEMA = { "additionalProperties": False } } + +_FIRMWARE_INTERFACE_UPDATE_SCHEMA = { + "$schema": "http://json-schema.org/schema#", + "title": "update_firmware clean step schema", + "type": "array", + # list of firmware update images + "items": { + "type": "object", + "required": ["component", "url"], + "properties": { + "component": { + "description": "name of the firmware component to be updated", + "type": "string", + "minLenght": 1 + }, + "url": { + "description": "URL for firmware file", + "type": "string", + "minLength": 1 + }, + "wait": { + "description": "optional wait time for firmware update", + "type": "integer", + "minimum": 1 + } + }, + "additionalProperties": False + } +} + _FIRMWARE_SUBDIR = 'firmware' @@ -80,6 +110,20 @@ def validate_update_firmware_args(firmware_images): % {'firmware_images': firmware_images, 'err': err}) +def validate_firmware_interface_update_args(settings): + """Validate ``update`` step input argument + + :param settings: args to validate. + :raises: InvalidParameterValue When argument is not valid + """ + try: + jsonschema.validate(settings, _FIRMWARE_INTERFACE_UPDATE_SCHEMA) + except jsonschema.ValidationError as err: + raise exception.InvalidParameterValue( + _('Invalid firmware update %(settings)s. Errors: %(err)s') + % {'settings': settings, 'err': err}) + + def get_swift_temp_url(parsed_url): """Gets Swift temporary URL diff --git a/ironic/drivers/modules/redfish/utils.py b/ironic/drivers/modules/redfish/utils.py index e4af83071e..4182c84d11 100644 --- a/ironic/drivers/modules/redfish/utils.py +++ b/ironic/drivers/modules/redfish/utils.py @@ -475,3 +475,39 @@ def wait_until_get_system_ready(node): driver_info = parse_driver_info(node) system_id = driver_info['system_id'] return _get_system(driver_info, system_id) + + +def get_manager(node, system, manager_id=None): + """Get a node's manager. + + :param system: a Sushy system object + :param manager_id: the id of the manager + :return: a sushy Manager + :raises: RedfishError when the System doesn't have Managers associated + """ + + try: + sushy_manager = None + available_managers = system.managers + if available_managers: + if manager_id is None: + sushy_manager = available_managers[0] + else: + for manager in available_managers: + if manager.identity == manager_id: + sushy_manager = manager + if sushy_manager is None: + raise Exception("Couldn't find any Sushy Manager") + return sushy_manager + except sushy.exceptions.MissingAttributeError as e: + LOG.error('Redfish Managers for node %(node)s are not associated ' + 'with system %(system)s. Error %(error)s', + {'system': system.identity, + 'node': node.uuid, 'error': e}) + raise exception.RedfishError(error=e) + except Exception as exc: + LOG.error('Redfish Manager was not found for ' + 'node %(node)s under system %(system)s. Error %(error)s', + {'system': system.identity, + 'node': node.uuid, 'error': exc}) + raise exception.RedfishError(error=exc) diff --git a/ironic/drivers/redfish.py b/ironic/drivers/redfish.py index 3852cd35fe..094119e7d2 100644 --- a/ironic/drivers/redfish.py +++ b/ironic/drivers/redfish.py @@ -21,6 +21,7 @@ from ironic.drivers.modules import noop_mgmt from ironic.drivers.modules import pxe from ironic.drivers.modules.redfish import bios as redfish_bios from ironic.drivers.modules.redfish import boot as redfish_boot +from ironic.drivers.modules.redfish import firmware as redfish_firmware from ironic.drivers.modules.redfish import inspect as redfish_inspect from ironic.drivers.modules.redfish import management as redfish_mgmt from ironic.drivers.modules.redfish import power as redfish_power @@ -69,3 +70,7 @@ class RedfishHardware(generic.GenericHardware): def supported_raid_interfaces(self): """List of supported raid interfaces.""" return [redfish_raid.RedfishRAID, noop.NoRAID, agent.AgentRAID] + + @property + def supported_firmware_interfaces(self): + return [redfish_firmware.RedfishFirmware, noop.NoFirmware] diff --git a/ironic/objects/firmware.py b/ironic/objects/firmware.py index 2a0f5f23ec..d30bc16911 100644 --- a/ironic/objects/firmware.py +++ b/ironic/objects/firmware.py @@ -145,11 +145,15 @@ class FirmwareComponentList(base.IronicObjectListBase, base.IronicObject): for cmp in components: if cmp['component'] in current_components_dict: values = current_components_dict[cmp['component']] - - cv_changed = cmp['current_version'] \ - != values.get('current_version') - lvf_changed = cmp['last_version_flashed'] \ - != values.get('last_version_flashed') + if values.get('last_version_flashed') is None: + lvf_changed = False + cv_changed = cmp['current_version'] \ + != values.get('current_version') + else: + lvf_changed = cmp['current_version'] \ + != values.get('last_version_flashed') + cv_changed = cmp['current_version'] \ + != values.get('current_version') if cv_changed or lvf_changed: update_list.append(cmp) diff --git a/ironic/tests/unit/conductor/test_steps.py b/ironic/tests/unit/conductor/test_steps.py index 09d267af0e..64317c1818 100644 --- a/ironic/tests/unit/conductor/test_steps.py +++ b/ironic/tests/unit/conductor/test_steps.py @@ -585,6 +585,11 @@ class NodeCleaningStepsTestCase(db_base.DbTestCase): 'abortable': False, 'argsinfo': None, 'interface': 'vendor', 'priority': 1, 'requires_ramdisk': True, 'step': 'log_passthrough'} + self.firmware_step = { + 'abortable': False, 'argsinfo': {}, 'interface': 'firmware', + 'priority': 0, 'requires_ramdisk': True, + 'step': 'update' + } # Automated cleaning should be executed in this order self.clean_steps = [self.deploy_erase, self.power_update, @@ -595,6 +600,8 @@ class NodeCleaningStepsTestCase(db_base.DbTestCase): 'argsinfo': {'arg1': {'description': 'desc1', 'required': True}, 'arg2': {'description': 'desc2'}}} + @mock.patch('ironic.drivers.modules.fake.FakeFirmware.get_clean_steps', + lambda self, taks: []) @mock.patch('ironic.drivers.modules.fake.FakeBIOS.get_clean_steps', lambda self, task: []) @mock.patch('ironic.drivers.modules.fake.FakeDeploy.get_clean_steps', @@ -619,6 +626,8 @@ class NodeCleaningStepsTestCase(db_base.DbTestCase): self.assertEqual(self.clean_steps, steps) + @mock.patch('ironic.drivers.modules.fake.FakeFirmware.get_clean_steps', + lambda self, task: []) @mock.patch('ironic.drivers.modules.fake.FakeVendorB.get_clean_steps', lambda self, task: []) @mock.patch('ironic.drivers.modules.fake.FakeBIOS.get_clean_steps', diff --git a/ironic/tests/unit/drivers/modules/ilo/test_management.py b/ironic/tests/unit/drivers/modules/ilo/test_management.py index f087c4d586..93799547ae 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_management.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_management.py @@ -1681,7 +1681,7 @@ class Ilo5ManagementTestCase(db_base.DbTestCase): ilo_mock_object.do_disk_erase.assert_called_once_with( 'HDD', 'overwrite') self.assertEqual(states.CLEANWAIT, result) - mock_power.assert_called_once_with(task, states.REBOOT) + mock_power.assert_called_once_with(task, states.REBOOT, None) @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) @@ -1712,7 +1712,7 @@ class Ilo5ManagementTestCase(db_base.DbTestCase): ilo_mock_object.do_disk_erase.assert_called_once_with( 'SSD', 'block') self.assertEqual(states.CLEANWAIT, result) - mock_power.assert_called_once_with(task, states.REBOOT) + mock_power.assert_called_once_with(task, states.REBOOT, None) @mock.patch.object(deploy_utils, 'build_agent_options', autospec=True) @@ -1746,7 +1746,7 @@ class Ilo5ManagementTestCase(db_base.DbTestCase): ilo_mock_object.do_disk_erase.assert_called_once_with( 'SSD', 'block') self.assertEqual(states.CLEANWAIT, result) - mock_power.assert_called_once_with(task, states.REBOOT) + mock_power.assert_called_once_with(task, states.REBOOT, None) @mock.patch.object(ilo_management.LOG, 'info', autospec=True) @mock.patch.object(ilo_management.Ilo5Management, @@ -1802,7 +1802,7 @@ class Ilo5ManagementTestCase(db_base.DbTestCase): ilo_mock_object.do_disk_erase.assert_called_once_with( 'HDD', 'zero') self.assertEqual(states.CLEANWAIT, result) - mock_power.assert_called_once_with(task, states.REBOOT) + mock_power.assert_called_once_with(task, states.REBOOT, None) @mock.patch.object(ilo_management.LOG, 'info', autospec=True) @mock.patch.object(ilo_common, 'get_ilo_object', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_raid.py b/ironic/tests/unit/drivers/modules/ilo/test_raid.py index fcb0314b10..c102a5d80f 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_raid.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_raid.py @@ -84,7 +84,7 @@ class Ilo5RAIDTestCase(db_base.DbTestCase): self.assertFalse( task.node.driver_internal_info.get( 'skip_current_deploy_step')) - mock_reboot.assert_called_once_with(task, states.REBOOT) + mock_reboot.assert_called_once_with(task, states.REBOOT, None) def test__prepare_for_read_raid_create_raid_cleaning(self): self.node.clean_step = {'step': 'create_configuration', @@ -122,7 +122,7 @@ class Ilo5RAIDTestCase(db_base.DbTestCase): self.assertEqual( task.node.driver_internal_info.get( 'skip_current_deploy_step'), False) - mock_reboot.assert_called_once_with(task, states.REBOOT) + mock_reboot.assert_called_once_with(task, states.REBOOT, None) def test__prepare_for_read_raid_delete_raid_cleaning(self): self.node.clean_step = {'step': 'create_configuration', diff --git a/ironic/tests/unit/drivers/modules/redfish/test_bios.py b/ironic/tests/unit/drivers/modules/redfish/test_bios.py index 1d307300f4..bccaad0ebc 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_bios.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_bios.py @@ -203,10 +203,11 @@ class RedfishBiosTestCase(db_base.DbTestCase): if fast_track: mock_power_action.assert_has_calls([ mock.call(task, states.POWER_OFF), - mock.call(task, states.REBOOT), + mock.call(task, states.REBOOT, None), ]) else: - mock_power_action.assert_called_once_with(task, states.REBOOT) + mock_power_action.assert_called_once_with( + task, states.REBOOT, None) if step == 'factory_reset': bios.reset_bios.assert_called_once() if step == 'apply_configuration': diff --git a/ironic/tests/unit/drivers/modules/redfish/test_firmware.py b/ironic/tests/unit/drivers/modules/redfish/test_firmware.py new file mode 100644 index 0000000000..c3e984c8d2 --- /dev/null +++ b/ironic/tests/unit/drivers/modules/redfish/test_firmware.py @@ -0,0 +1,40 @@ +# +# 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.tests.unit.db import base as db_base +from ironic.tests.unit.db import utils as db_utils +from ironic.tests.unit.objects import utils as obj_utils + +sushy = importutils.try_import('sushy') + +INFO_DICT = db_utils.get_test_redfish_info() + + +@mock.patch('oslo_utils.eventletutils.EventletEvent.wait', + lambda *args, **kwargs: None) +class RedfishFirmwareTestCase(db_base.DbTestCase): + + def setUp(self): + super(RedfishFirmwareTestCase, self).setUp() + self.config(enabled_bios_interfaces=['redfish'], + enabled_hardware_types=['redfish'], + enabled_power_interfaces=['redfish'], + enabled_boot_interfaces=['redfish-virtual-media'], + enabled_management_interfaces=['redfish'], + enabled_firmware_interfaces=['redfish']) + self.node = obj_utils.create_test_node( + self.context, driver='redfish', driver_info=INFO_DICT) diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index 1d752d909a..6599c18415 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -865,7 +865,8 @@ class RedfishManagementTestCase(db_base.DbTestCase): task.node, reboot=True, skip_current_step=True, polling=True) mock_get_async_step_return_state.assert_called_once_with( task.node) - mock_node_power_action.assert_called_once_with(task, states.REBOOT) + mock_node_power_action.assert_called_once_with( + task, states.REBOOT, None) @mock.patch.object(redfish_mgmt.RedfishManagement, '_stage_firmware_file', autospec=True) @@ -919,7 +920,8 @@ class RedfishManagementTestCase(db_base.DbTestCase): task.node, reboot=True, skip_current_step=True, polling=True) mock_get_async_step_return_state.assert_called_once_with( task.node) - mock_node_power_action.assert_called_once_with(task, states.REBOOT) + mock_node_power_action.assert_called_once_with( + task, states.REBOOT, None) @mock.patch.object(redfish_mgmt.RedfishManagement, '_stage_firmware_file', autospec=True) @@ -979,7 +981,8 @@ class RedfishManagementTestCase(db_base.DbTestCase): task.node, reboot=True, skip_current_step=True, polling=True) mock_get_async_step_return_state.assert_called_once_with( task.node) - mock_node_power_action.assert_called_once_with(task, states.REBOOT) + mock_node_power_action.assert_called_once_with( + task, states.REBOOT, None) def test_update_firmware_invalid_args(self): with task_manager.acquire(self.context, self.node.uuid, diff --git a/ironic/tests/unit/drivers/modules/redfish/test_raid.py b/ironic/tests/unit/drivers/modules/redfish/test_raid.py index 843be735c9..e651d8b858 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_raid.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_raid.py @@ -406,7 +406,8 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.node, reboot=True, skip_current_step=True, polling=True) mock_get_async_step_return_state.assert_called_once_with( task.node) - mock_node_power_action.assert_called_once_with(task, states.REBOOT) + mock_node_power_action.assert_called_once_with( + task, states.REBOOT, None) mock_build_agent_options.assert_called_once_with(task.node) self.assertEqual(mock_prepare_ramdisk.call_count, 1) # Async operation, raid_config shouldn't be updated yet @@ -1123,7 +1124,8 @@ class RedfishRAIDTestCase(db_base.DbTestCase): task.node, reboot=True, skip_current_step=True, polling=True) mock_get_async_step_return_state.assert_called_once_with( task.node) - mock_node_power_action.assert_called_once_with(task, states.REBOOT) + mock_node_power_action.assert_called_once_with( + task, states.REBOOT, None) mock_build_agent_options.assert_called_once_with(task.node) self.assertEqual(mock_prepare_ramdisk.call_count, 1) self.assertEqual( diff --git a/ironic/tests/unit/drivers/modules/test_agent_base.py b/ironic/tests/unit/drivers/modules/test_agent_base.py index c589d520be..6d285cb0cf 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_base.py +++ b/ironic/tests/unit/drivers/modules/test_agent_base.py @@ -1593,7 +1593,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): agent_base._post_step_reboot(task, 'clean') self.assertTrue(mock_build_opt.called) self.assertTrue(mock_prepare.called) - mock_reboot.assert_called_once_with(task, states.REBOOT) + mock_reboot.assert_called_once_with(task, states.REBOOT, None) self.assertTrue(task.node.driver_internal_info['cleaning_reboot']) self.assertNotIn('agent_secret_token', task.node.driver_internal_info) @@ -1612,7 +1612,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): agent_base._post_step_reboot(task, 'deploy') self.assertTrue(mock_build_opt.called) self.assertTrue(mock_prepare.called) - mock_reboot.assert_called_once_with(task, states.REBOOT) + mock_reboot.assert_called_once_with(task, states.REBOOT, None) self.assertTrue( task.node.driver_internal_info['deployment_reboot']) self.assertNotIn('agent_secret_token', @@ -1633,7 +1633,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): agent_base._post_step_reboot(task, 'clean') self.assertTrue(mock_build_opt.called) self.assertTrue(mock_prepare.called) - mock_reboot.assert_called_once_with(task, states.REBOOT) + mock_reboot.assert_called_once_with(task, states.REBOOT, None) self.assertIn('agent_secret_token', task.node.driver_internal_info) @@ -1649,7 +1649,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: agent_base._post_step_reboot(task, 'clean') - mock_reboot.assert_called_once_with(task, states.REBOOT) + mock_reboot.assert_called_once_with(task, states.REBOOT, None) mock_handler.assert_called_once_with(task, mock.ANY, traceback=True) self.assertNotIn('cleaning_reboot', @@ -1667,7 +1667,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: agent_base._post_step_reboot(task, 'deploy') - mock_reboot.assert_called_once_with(task, states.REBOOT) + mock_reboot.assert_called_once_with(task, states.REBOOT, None) mock_handler.assert_called_once_with(task, mock.ANY, traceback=True) self.assertNotIn('deployment_reboot', @@ -1686,7 +1686,7 @@ class AgentDeployMixinTest(AgentDeployMixinBaseTest): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: agent_base._post_step_reboot(task, 'service') - mock_reboot.assert_called_once_with(task, states.REBOOT) + mock_reboot.assert_called_once_with(task, states.REBOOT, None) mock_handler.assert_called_once_with(task, mock.ANY, traceback=True) self.assertNotIn('servicing_reboot', @@ -1829,7 +1829,7 @@ class ContinueCleaningTest(AgentDeployMixinBaseTest): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: self.deploy.continue_cleaning(task) - reboot_mock.assert_called_once_with(task, states.REBOOT) + reboot_mock.assert_called_once_with(task, states.REBOOT, None) @mock.patch.object(cleaning, 'continue_node_clean', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', @@ -2147,7 +2147,7 @@ class ContinueServiceTest(AgentDeployMixinBaseTest): with task_manager.acquire(self.context, self.node['uuid'], shared=False) as task: self.deploy.continue_servicing(task) - reboot_mock.assert_called_once_with(task, states.REBOOT) + reboot_mock.assert_called_once_with(task, states.REBOOT, None) @mock.patch.object(servicing, 'continue_node_service', autospec=True) @mock.patch.object(agent_client.AgentClient, 'get_commands_status', diff --git a/ironic/tests/unit/drivers/test_redfish.py b/ironic/tests/unit/drivers/test_redfish.py index b692c6167c..be34fbf0a0 100644 --- a/ironic/tests/unit/drivers/test_redfish.py +++ b/ironic/tests/unit/drivers/test_redfish.py @@ -33,7 +33,8 @@ class RedfishHardwareTestCase(db_base.DbTestCase): enabled_boot_interfaces=['redfish-virtual-media'], enabled_management_interfaces=['redfish'], enabled_inspect_interfaces=['redfish'], - enabled_bios_interfaces=['redfish']) + enabled_bios_interfaces=['redfish'], + enabled_firmware_interfaces=['redfish']) def test_default_interfaces(self): node = obj_utils.create_test_node(self.context, driver='redfish') diff --git a/releasenotes/notes/firmware-interface-8ad6f91aa1f746a0.yaml b/releasenotes/notes/firmware-interface-8ad6f91aa1f746a0.yaml new file mode 100644 index 0000000000..06964b1093 --- /dev/null +++ b/releasenotes/notes/firmware-interface-8ad6f91aa1f746a0.yaml @@ -0,0 +1,31 @@ +--- +features: + - | + Adds Firmware Interface support to ironic, we would like to receive + feedback since this is a new feature we introduced and we as a developer + community have limited hardware access, reach out to us in case of any + unexpected behavior. + + - Adds version 1.86 of the Bare Metal API, which includes: + + * List all firmware components of a node via the + ``GET /v1/nodes/{node_ident}/firmware`` API. + + * The ``firmware_interface`` field of the node resource. A firmware + interface can be set when creating or updating a node. + + * The ``default_firmware_interface`` and ``enabled_firmware_interface`` + fields of the driver resource. + + - Adds new configuration options for the firmware interface feature: + + * Firmware interfaces are enabled via + ``[DEFAULT]/enabled_firmware_interfaces``. A default firmware + interface to use when creating or updating nodes can be specified with + ``[DEFAULT]/default_firmware_interface``. + + - Available interfaces: ``redfish``, ``no-firmware`` and ``fake``. + + - Support to update firmware of BIOS and BMC via ``update`` step, can be + done via clean or deploy steps, the node should be using the + ``redfish`` driver and set the ``firmware_interface``. diff --git a/setup.cfg b/setup.cfg index e9b2ec837b..9512a1288c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -97,6 +97,7 @@ ironic.hardware.interfaces.deploy = ironic.hardware.interfaces.firmware = fake = ironic.drivers.modules.fake:FakeFirmware no-firmware = ironic.drivers.modules.noop:NoFirmware + redfish = ironic.drivers.modules.redfish.firmware:RedfishFirmware ironic.hardware.interfaces.inspect = agent = ironic.drivers.modules.inspector:AgentInspect