From c725f541de3a9df995a489fc802b6b1c941b6e01 Mon Sep 17 00:00:00 2001 From: Aparna Date: Wed, 18 Jan 2017 15:48:17 +0000 Subject: [PATCH] Support SUM based firmware update as clean step for iLO drivers This commit adds support to perform Smart Update Manager (SUM) based firmware update as an inband manual clean step `update_firmware_sum` for iLO drivers. Closes-Bug: #1648448 Change-Id: I581d5bc535420d4493394d16cf6cc2bb0ddeb8f7 --- doc/source/admin/drivers/ilo.rst | 88 +++++++++- doc/source/admin/troubleshooting.rst | 2 + driver-requirements.txt | 2 +- .../drivers/modules/ilo/firmware_processor.py | 112 ++++++++---- ironic/drivers/modules/ilo/management.py | 95 +++++++++- ironic/drivers/utils.py | 11 +- .../modules/ilo/test_firmware_processor.py | 65 ++++++- .../drivers/modules/ilo/test_management.py | 165 ++++++++++++++++++ ironic/tests/unit/drivers/test_utils.py | 4 +- ...re-manual-clean-step-e69ade488060cf27.yaml | 6 + 10 files changed, 504 insertions(+), 46 deletions(-) create mode 100644 releasenotes/notes/sum-based-update-firmware-manual-clean-step-e69ade488060cf27.yaml diff --git a/doc/source/admin/drivers/ilo.rst b/doc/source/admin/drivers/ilo.rst index 4e1e62d909..194a313ed4 100644 --- a/doc/source/admin/drivers/ilo.rst +++ b/doc/source/admin/drivers/ilo.rst @@ -235,9 +235,9 @@ Prerequisites which contains a set of modules for managing HPE ProLiant hardware. Install ``proliantutils`` module on the ironic conductor node. Minimum - version required is 2.4.0:: + version required is 2.4.1:: - $ pip install "proliantutils>=2.4.0" + $ pip install "proliantutils>=2.4.1" * ``ipmitool`` command must be present on the service node(s) where ``ironic-conductor`` is running. On most distros, this is provided as part @@ -1095,6 +1095,11 @@ Supported **Manual** Cleaning Operations Some devices firmware cannot be updated via this method, such as: storage controllers, host bus adapters, disk drive firmware, network interfaces and Onboard Administrator (OA). + ``update_firmware_sum``: + Updates all or list of user specified firmware components on the node + using Smart Update Manager (SUM). It is an inband step associated with + the ``management`` interface. See `Smart Update Manager (SUM) based firmware update`_ + for more information on usage. * iLO with firmware version 1.5 is minimally required to support all the operations. @@ -1798,6 +1803,80 @@ All the fields in the firmware image block are mandatory. $ md5sum image.rpm 66cdb090c80b71daa21a67f06ecd3f33 image.rpm +Smart Update Manager (SUM) based firmware update +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The firmware update based on `SUM`_ is an inband clean step supported by iLO +drivers. The firmware update is performed on all or list of user specified +firmware components on the node. Refer to `SUM User Guide`_ to get more +information on SUM based firmware update. + +``update_firmware_sum`` clean step requires the agent ramdisk with +``Proliant Hardware Manager`` from the proliantutils version 2.4.0 or higher. +See `DIB support for Proliant Hardware Manager`_ to create the agent ramdisk +with ``Proliant Hardware Manager``. + +The attributes of ``update_firmware_sum`` clean step are as follows: + +.. csv-table:: + :header: "Attribute", "Description" + :widths: 30, 120 + + "``interface``", "Interface of the clean step, here ``management``" + "``step``", "Name of the clean step, here ``update_firmware_sum``" + "``args``", "Keyword-argument entry (: ) being passed to the clean step" + +The keyword arguments used for the clean step are as follows: + +* ``url``: URL of SPP (Service Pack for Proliant) ISO. It is mandatory. The + URL schemes supported are ``http``, ``https`` and ``swift``. +* ``checksum``: MD5 checksum of SPP ISO to verify the image. It is mandatory. +* ``components``: List of filenames of the fimware components to be flashed. + It is optional. If not provided, the firmware update is performed on all + the firmware components. + +The clean step performs an update on all or a list of firmware components and +returns the SUM log files. The log files include ``hpsum_log.txt`` and +``hpsum_detail_log.txt`` which holds the information about firmware components, +firmware version for each component and their update status. The log object +will be named with the following pattern:: + + [_]_update_firmware_sum_.tar.gz + +Refer to :ref:`retrieve_deploy_ramdisk_logs` for more information on enabling and +viewing the logs returned from the ramdisk. + +An example of ``update_firmware_sum`` clean step: + +.. code-block:: json + + { + "interface": "management", + "step": "update_firmware_sum", + "args": + { + "url": "http://my_address:port/SPP.iso", + "checksum": "abcdefxyz", + "components": ["CP024356.scexe", "CP008097.exe"] + } + } + +The clean step fails if there is any error in the processing of clean step +arguments. The processing error could happen during validation of components' +file extension, image download, image checksum verification or image extraction. +In case of a failure, check Ironic conductor logs carefully to see if there are +any validation or firmware processing related errors which may help in root +cause analysis or gaining an understanding of where things were left off or +where things failed. You can then fix or work around and then try again. + +.. warning:: + This feature is officially supported only with RHEL and SUSE based IPA ramdisk. + Refer to `SUM`_ for supported OS versions for specific SUM version. + +.. note:: + Refer `Guidelines for SPP ISO`_ for steps to get SPP (Service Pack for + ProLiant) ISO. + RAID Support ^^^^^^^^^^^^ @@ -1820,7 +1899,7 @@ configuration of RAID: .. _DIB_raid_support: DIB support for Proliant Hardware Manager -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ To create an agent ramdisk with ``Proliant Hardware Manager``, use the ``proliant-tools`` element in DIB:: @@ -1867,3 +1946,6 @@ See the `proliant-tools`_ for more information on creating agent ramdisk with .. _`iLO 5 management engine`: https://www.hpe.com/us/en/servers/integrated-lights-out-ilo.html#innovations .. _`Redfish`: https://www.dmtf.org/standards/redfish .. _`Gen10 wiki section`: https://wiki.openstack.org/wiki/Ironic/Drivers/iLODrivers/master#Enabling_ProLiant_Gen10_systems_in_Ironic +.. _`Guidelines for SPP ISO`: http://h17007.www1.hpe.com/us/en/enterprise/servers/products/service_pack/spp +.. _`SUM`: http://h17007.www1.hpe.com/us/en/enterprise/servers/products/service_pack/hpsum/index.aspx +.. _`SUM User Guide`: http://h20565.www2.hpe.com/hpsc/doc/public/display?docId=c05210448 diff --git a/doc/source/admin/troubleshooting.rst b/doc/source/admin/troubleshooting.rst index 3efb156acd..ed4232b491 100644 --- a/doc/source/admin/troubleshooting.rst +++ b/doc/source/admin/troubleshooting.rst @@ -190,6 +190,8 @@ API Errors The `debug_tracebacks_in_api` config option may be set to return tracebacks in the API response for all 4xx and 5xx errors. +.. _retrieve_deploy_ramdisk_logs: + Retrieving logs from the deploy ramdisk ======================================= diff --git a/driver-requirements.txt b/driver-requirements.txt index b707d9c0c3..2c3a62b8ed 100644 --- a/driver-requirements.txt +++ b/driver-requirements.txt @@ -4,7 +4,7 @@ # python projects they should package as optional dependencies for Ironic. # These are available on pypi -proliantutils>=2.4.0 +proliantutils>=2.4.1 pysnmp python-ironic-inspector-client>=1.5.0 python-oneviewclient<3.0.0,>=2.5.2 diff --git a/ironic/drivers/modules/ilo/firmware_processor.py b/ironic/drivers/modules/ilo/firmware_processor.py index 0469e04c2b..039bea690c 100644 --- a/ironic/drivers/modules/ilo/firmware_processor.py +++ b/ironic/drivers/modules/ilo/firmware_processor.py @@ -16,6 +16,7 @@ Firmware file processor """ import os +import re import shutil import tempfile import types @@ -33,14 +34,14 @@ from ironic.common import image_service from ironic.common import swift from ironic.drivers.modules.ilo import common as ilo_common -# Supported components for firmware update when invoked -# through manual clean step, ``update_firmware``. -SUPPORTED_FIRMWARE_UPDATE_COMPONENTS = ['ilo', 'cpld', 'power_pic', 'bios', - 'chassis'] +# Supported components for firmware update when invoked through manual clean +# step, ``update_firmware``. +SUPPORTED_ILO_FIRMWARE_UPDATE_COMPONENTS = ['ilo', 'cpld', 'power_pic', 'bios', + 'chassis'] # Mandatory fields to be provided as part of firmware image update # with manual clean step -FIRMWARE_IMAGE_INFO_FIELDS = {'url', 'checksum', 'component'} +FIRMWARE_IMAGE_INFO_FIELDS = {'url', 'checksum'} CONF = cfg.CONF @@ -81,14 +82,52 @@ def verify_firmware_update_args(func): return wrapper -def get_and_validate_firmware_image_info(firmware_image_info): +def _validate_ilo_component(component): + """Validates component with supported values. + + :param component: name of the component to be validated. + :raises: InvalidParameterValue, for unsupported firmware component + """ + if component not in SUPPORTED_ILO_FIRMWARE_UPDATE_COMPONENTS: + msg = (_("Component '%(component)s' for firmware update is not " + "supported in 'ilo' based firmware update. Supported " + "values are: %(supported_components)s") % + {'component': component, 'supported_components': ( + ", ".join(SUPPORTED_ILO_FIRMWARE_UPDATE_COMPONENTS))}) + LOG.error(msg) + raise exception.InvalidParameterValue(msg) + + +def _validate_sum_components(components): + """Validates components' file extension with supported values. + + :param components: A list of components to be updated. + :raises: InvalidParameterValue, for unsupported firmware component + """ + not_supported = [] + for component in components: + if not re.search('\.(scexe|exe|rpm)$', component): + not_supported.append(component) + + if not_supported: + msg = (_("The component files '%s' provided are not supported in " + "'SUM' based firmware update. The valid file extensions are " + "'scexe', 'exe', 'rpm'.") % + ', '.join(x for x in not_supported)) + LOG.error(msg) + raise exception.InvalidParameterValue(msg) + + +def get_and_validate_firmware_image_info(firmware_image_info, + firmware_update_mode): """Validates the firmware image info and returns the retrieved values. :param firmware_image_info: dict object containing the firmware image info :raises: MissingParameterValue, for missing fields (or values) in image info. :raises: InvalidParameterValue, for unsupported firmware component - :returns: tuple of firmware url, checksum, component + :returns: tuple of firmware url, checksum, component when the firmware + update is ilo based. """ image_info = firmware_image_info or {} @@ -98,6 +137,9 @@ def get_and_validate_firmware_image_info(firmware_image_info): if not image_info.get(field): missing_fields.append(field) + if firmware_update_mode == 'ilo' and not image_info.get('component'): + missing_fields.append('component') + if missing_fields: msg = (_("Firmware image info: %(image_info)s is missing the " "required %(missing)s field/s.") % @@ -106,18 +148,15 @@ def get_and_validate_firmware_image_info(firmware_image_info): LOG.error(msg) raise exception.MissingParameterValue(msg) - component = image_info['component'] - component = component.lower() - if component not in SUPPORTED_FIRMWARE_UPDATE_COMPONENTS: - msg = (_("Component for firmware update is not supported. Provided " - "value: %(component)s. Supported values are: " - "%(supported_components)s") % - {'component': component, 'supported_components': ( - ", ".join(SUPPORTED_FIRMWARE_UPDATE_COMPONENTS))}) - LOG.error(msg) - raise exception.InvalidParameterValue(msg) - LOG.debug("Validating firmware image info: %s ... done", image_info) - return image_info['url'], image_info['checksum'], component + if firmware_update_mode == 'sum': + component = image_info.get('components') + if component: + _validate_sum_components(component) + else: + component = image_info['component'].lower() + _validate_ilo_component(component) + LOG.debug("Validating firmware image info: %s ... done", image_info) + return image_info['url'], image_info['checksum'], component class FirmwareProcessor(object): @@ -248,30 +287,39 @@ def _download_http_based_fw_to(self, target_file): image_service.HttpImageService().download(src_file, fd) -def _download_swift_based_fw_to(self, target_file): - """Swift based firmware file downloader +def get_swift_url(parsed_url): + """Gets swift temp url. - It generates a temp url for the swift based firmware url and then downloads - the firmware file via http based downloader to the target file. - Expecting url as swift://containername/objectname - :param target_file: destination file for downloading the original firmware - file. - :raises: SwiftOperationError, on failure to download from swift. - :raises: ImageDownloadFailed, on failure to download the original file. + It generates a temp url for the swift based firmware url to the target + file. Expecting url as swift://containername/objectname. + + :param parsed_url: Parsed url object. + :raises: SwiftOperationError, on failure to get url from swift. """ # Extract container name - container = self.parsed_url.netloc + container = parsed_url.netloc # Extract the object name from the path of the form: # ``/objectname`` OR # ``/pseudo-folder/objectname`` # stripping the leading '/' character. - objectname = self.parsed_url.path.lstrip('/') + objectname = parsed_url.path.lstrip('/') timeout = CONF.ilo.swift_object_expiry_timeout # Generate temp url using swift API - tempurl = swift.SwiftAPI().get_temp_url(container, objectname, timeout) + return swift.SwiftAPI().get_temp_url(container, objectname, timeout) + + +def _download_swift_based_fw_to(self, target_file): + """Swift based firmware file downloader + + It downloads the firmware file via http based downloader to the target + file. Expecting url as swift://containername/objectname + :param target_file: destination file for downloading the original firmware + file. + :raises: ImageDownloadFailed, on failure to download the original file. + """ # set the parsed_url attribute to the newly created tempurl from swift and # delegate the dowloading job to the http_based downloader - self.parsed_url = urlparse.urlparse(tempurl) + self.parsed_url = urlparse.urlparse(get_swift_url(self.parsed_url)) _download_http_based_fw_to(self, target_file) diff --git a/ironic/drivers/modules/ilo/management.py b/ironic/drivers/modules/ilo/management.py index 8c03fc1231..5ffb48dc3d 100644 --- a/ironic/drivers/modules/ilo/management.py +++ b/ironic/drivers/modules/ilo/management.py @@ -20,6 +20,7 @@ from oslo_log import log as logging from oslo_utils import excutils from oslo_utils import importutils import six +import six.moves.urllib.parse as urlparse from ironic.common import boot_devices from ironic.common import exception @@ -27,9 +28,12 @@ from ironic.common.i18n import _ from ironic.conductor import task_manager from ironic.conf import CONF from ironic.drivers import base +from ironic.drivers.modules import agent_base_vendor +from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.ilo import common as ilo_common from ironic.drivers.modules.ilo import firmware_processor from ironic.drivers.modules import ipmitool +from ironic.drivers import utils as driver_utils LOG = logging.getLogger(__name__) @@ -84,6 +88,13 @@ def _execute_ilo_clean_step(node, step, *args, **kwargs): {'node': node.uuid, 'step': step, 'err': ilo_exception}) +def _should_collect_logs(command): + """Returns boolean to check whether logs need to collected or not.""" + return ((CONF.agent.deploy_logs_collect == 'on_failure' and + command['command_status'] == 'FAILED') or + CONF.agent.deploy_logs_collect == 'always') + + class IloManagement(base.ManagementInterface): def get_properties(self): @@ -371,7 +382,7 @@ class IloManagement(base.ManagementInterface): for firmware_image_info in firmware_images: url, checksum, component = ( firmware_processor.get_and_validate_firmware_image_info( - firmware_image_info)) + firmware_image_info, kwargs['firmware_update_mode'])) LOG.debug("Processing of firmware file: %(firmware_file)s on " "node: %(node)s ... in progress", {'firmware_file': url, 'node': node.uuid}) @@ -420,3 +431,85 @@ class IloManagement(base.ManagementInterface): LOG.info("All Firmware update operations completed successfully " "for node: %s.", node.uuid) + + @METRICS.timer('IloManagement.update_firmware_sum') + @base.clean_step(priority=0, abortable=False, argsinfo={ + 'url': { + 'description': ( + "The image location for SPP (Service Pack for Proliant) ISO." + ), + 'required': True + }, + 'checksum': { + 'description': ( + "The md5 checksum of the SPP image file." + ), + 'required': True + }, + 'components': { + 'description': ( + "The list of firmware component filenames. If not specified, " + "SUM updates all the firmware components." + ), + 'required': False} + }) + def update_firmware_sum(self, task, **kwargs): + """Updates the firmware using Smart Update Manager (SUM). + + :param task: a TaskManager object. + :raises: NodeCleaningFailure, on failure to execute step. + """ + node = task.node + # The arguments are validated and sent to the ProliantHardwareManager + # to perform SUM based firmware update clean step. + firmware_processor.get_and_validate_firmware_image_info(kwargs, + 'sum') + + url = kwargs['url'] + if urlparse.urlparse(url).scheme == 'swift': + url = firmware_processor.get_swift_url(urlparse.urlparse(url)) + node.clean_step['args']['url'] = url + + step = node.clean_step + return deploy_utils.agent_execute_clean_step(task, step) + + @staticmethod + @agent_base_vendor.post_clean_step_hook( + interface='management', step='update_firmware_sum') + def _update_firmware_sum_final(task, command): + """Clean step hook after SUM based firmware update operation. + + This method is invoked as a post clean step hook by the Ironic + conductor once firmware update operaion is completed. The clean logs + are collected and stored according to the configured storage backend + when the node is configured to collect the logs. + + :param task: a TaskManager instance. + :param command: A command result structure of the SUM based firmware + update operation returned from agent ramdisk on query of the + status of command(s). + """ + if not _should_collect_logs(command): + return + + node = task.node + try: + driver_utils.store_ramdisk_logs( + node, + command['command_result']['clean_result']['Log Data'], + label='update_firmware_sum') + except exception.SwiftOperationError as e: + LOG.error('Failed to store the logs from the node %(node)s ' + 'for "update_firmware_sum" clean step in Swift. ' + 'Error: %(error)s', + {'node': node.uuid, 'error': e}) + except EnvironmentError as e: + LOG.exception('Failed to store the logs from the node %(node)s ' + 'for "update_firmware_sum" clean step due to a ' + 'file-system related error. Error: %(error)s', + {'node': node.uuid, 'error': e}) + except Exception as e: + LOG.exception('Unknown error when storing logs from the node ' + '%(node)s for "update_firmware_sum" clean step. ' + 'Error: %(error)s', + {'node': node.uuid, 'error': e}) diff --git a/ironic/drivers/utils.py b/ironic/drivers/utils.py index 860a89922a..80d1fafb1b 100644 --- a/ironic/drivers/utils.py +++ b/ironic/drivers/utils.py @@ -263,10 +263,11 @@ def normalize_mac(mac): return mac.replace('-', '').replace(':', '').lower() -def get_ramdisk_logs_file_name(node): +def get_ramdisk_logs_file_name(node, label=None): """Construct the log file name. :param node: A node object. + :param label: A string to label the log file such as a clean step name. :returns: The log file name. """ timestamp = timeutils.utcnow().strftime('%Y-%m-%d-%H-%M-%S') @@ -274,11 +275,14 @@ def get_ramdisk_logs_file_name(node): if node.instance_uuid: file_name_fields.append(node.instance_uuid) + if label: + file_name_fields.append(label) + file_name_fields.append(timestamp) return '_'.join(file_name_fields) + '.tar.gz' -def store_ramdisk_logs(node, logs): +def store_ramdisk_logs(node, logs, label=None): """Store the ramdisk logs. This method stores the ramdisk logs according to the configured @@ -287,12 +291,13 @@ def store_ramdisk_logs(node, logs): :param node: A node object. :param logs: A gzipped and base64 encoded string containing the logs archive. + :param label: A string to label the log file such as a clean step name. :raises: OSError if the directory to save the logs cannot be created. :raises: IOError when the logs can't be saved to the local file system. :raises: SwiftOperationError, if any operation with Swift fails. """ - logs_file_name = get_ramdisk_logs_file_name(node) + logs_file_name = get_ramdisk_logs_file_name(node, label=label) data = base64.decode_as_bytes(logs) if CONF.agent.deploy_logs_storage_backend == 'local': diff --git a/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py b/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py index 498ccd7d9b..4b1e93f597 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py @@ -85,7 +85,7 @@ class FirmwareProcessorTestCase(base.TestCase): # | WHEN | url, checksum, component = ( ilo_fw_processor.get_and_validate_firmware_image_info( - firmware_image_info)) + firmware_image_info, 'ilo')) # | THEN | self.assertEqual(self.any_url, url) self.assertEqual('b64c8f7799cfbb553d384d34dc43fafe336cc889', checksum) @@ -102,7 +102,7 @@ class FirmwareProcessorTestCase(base.TestCase): self.assertRaisesRegex( exception.MissingParameterValue, 'checksum', ilo_fw_processor.get_and_validate_firmware_image_info, - invalid_firmware_image_info) + invalid_firmware_image_info, 'ilo') def test_get_and_validate_firmware_image_info_fails_for_empty_parameter( self): @@ -116,7 +116,7 @@ class FirmwareProcessorTestCase(base.TestCase): self.assertRaisesRegex( exception.MissingParameterValue, 'component', ilo_fw_processor.get_and_validate_firmware_image_info, - invalid_firmware_image_info) + invalid_firmware_image_info, 'ilo') def test_get_and_validate_firmware_image_info_fails_for_invalid_component( self): @@ -130,7 +130,64 @@ class FirmwareProcessorTestCase(base.TestCase): self.assertRaises( exception.InvalidParameterValue, ilo_fw_processor.get_and_validate_firmware_image_info, - invalid_firmware_image_info) + invalid_firmware_image_info, 'ilo') + + def test_get_and_validate_firmware_image_info_sum(self): + # | GIVEN | + result = None + firmware_image_info = { + 'url': self.any_url, + 'checksum': 'b64c8f7799cfbb553d384d34dc43fafe336cc889' + } + # | WHEN | & | THEN | + ret_val = ilo_fw_processor.get_and_validate_firmware_image_info( + firmware_image_info, 'sum') + self.assertEqual(result, ret_val) + + def test_get_and_validate_firmware_image_info_sum_with_component(self): + # | GIVEN | + result = None + firmware_image_info = { + 'url': self.any_url, + 'checksum': 'b64c8f7799cfbb553d384d34dc43fafe336cc889', + 'components': ['CP02345.exe'] + } + # | WHEN | & | THEN | + ret_val = ilo_fw_processor.get_and_validate_firmware_image_info( + firmware_image_info, 'sum') + self.assertEqual(result, ret_val) + + def test_get_and_validate_firmware_image_info_sum_invalid_component( + self): + # | GIVEN | + invalid_firmware_image_info = { + 'url': 'any_url', + 'checksum': 'valid_checksum', + 'components': 'INVALID' + } + # | WHEN | & | THEN | + self.assertRaises( + exception.InvalidParameterValue, + ilo_fw_processor.get_and_validate_firmware_image_info, + invalid_firmware_image_info, 'sum') + + def test__validate_sum_components(self): + result = None + components = ['CP02345.scexe', 'CP02678.exe'] + + ret_val = ilo_fw_processor._validate_sum_components(components) + + self.assertEqual(ret_val, result) + + @mock.patch.object(ilo_fw_processor, 'LOG') + def test__validate_sum_components_fails(self, LOG_mock): + components = ['INVALID'] + + self.assertRaises( + exception.InvalidParameterValue, + ilo_fw_processor._validate_sum_components, components) + + self.assertTrue(LOG_mock.error.called) def test_fw_processor_ctor_sets_parsed_url_attrib_of_fw_processor(self): # | WHEN | diff --git a/ironic/tests/unit/drivers/modules/ilo/test_management.py b/ironic/tests/unit/drivers/modules/ilo/test_management.py index fa2e2a4589..94fbbc16fc 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_management.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_management.py @@ -19,10 +19,13 @@ from oslo_utils import importutils from ironic.common import boot_devices from ironic.common import exception +from ironic.common import states from ironic.conductor import task_manager +from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.ilo import common as ilo_common from ironic.drivers.modules.ilo import management as ilo_management from ironic.drivers.modules import ipmitool +from ironic.drivers import utils as driver_utils from ironic.tests.unit.conductor import mgr_utils from ironic.tests.unit.db import base as db_base from ironic.tests.unit.db import utils as db_utils @@ -544,3 +547,165 @@ class IloManagementTestCase(db_base.DbTestCase): self.assertTrue(LOG_mock.error.called) remove_mock.assert_has_calls([mock.call(fw_loc_obj_1), mock.call(fw_loc_obj_2)]) + + @mock.patch.object(deploy_utils, 'agent_execute_clean_step', + autospec=True) + def test_update_firmware_sum_mode_with_component(self, execute_mock): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + execute_mock.return_value = states.CLEANWAIT + # | GIVEN | + firmware_update_args = { + 'url': 'http://any_url', + 'checksum': 'xxxx', + 'component': ['CP02345.scexe', 'CP02567.exe']} + clean_step = {'step': 'update_firmware', + 'interface': 'management', + 'args': firmware_update_args} + task.node.clean_step = clean_step + # | WHEN | + return_value = task.driver.management.update_firmware_sum( + task, **firmware_update_args) + # | THEN | + self.assertEqual(states.CLEANWAIT, return_value) + execute_mock.assert_called_once_with(task, clean_step) + + @mock.patch.object(ilo_management.firmware_processor, + 'get_swift_url', autospec=True) + @mock.patch.object(deploy_utils, 'agent_execute_clean_step', + autospec=True) + def test_update_firmware_sum_mode_swift_url(self, execute_mock, + swift_url_mock): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + swift_url_mock.return_value = "http://path-to-file" + execute_mock.return_value = states.CLEANWAIT + # | GIVEN | + firmware_update_args = { + 'url': 'swift://container/object', + 'checksum': 'xxxx', + 'components': ['CP02345.scexe', 'CP02567.exe']} + clean_step = {'step': 'update_firmware', + 'interface': 'management', + 'args': firmware_update_args} + task.node.clean_step = clean_step + # | WHEN | + return_value = task.driver.management.update_firmware_sum( + task, **firmware_update_args) + # | THEN | + self.assertEqual(states.CLEANWAIT, return_value) + self.assertEqual(task.node.clean_step['args']['url'], + "http://path-to-file") + + @mock.patch.object(deploy_utils, 'agent_execute_clean_step', + autospec=True) + def test_update_firmware_sum_mode_without_component(self, execute_mock): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + execute_mock.return_value = states.CLEANWAIT + # | GIVEN | + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx'} + clean_step = {'step': 'update_firmware', + 'interface': 'management', + 'args': firmware_update_args} + task.node.clean_step = clean_step + # | WHEN | + return_value = task.driver.management.update_firmware_sum( + task, **firmware_update_args) + # | THEN | + self.assertEqual(states.CLEANWAIT, return_value) + execute_mock.assert_called_once_with(task, clean_step) + + def test_update_firmware_sum_mode_invalid_component(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # | GIVEN | + firmware_update_args = { + 'url': 'any_valid_url', + 'checksum': 'xxxx', + 'components': ['CP02345']} + # | WHEN & THEN | + self.assertRaises(exception.InvalidParameterValue, + task.driver.management.update_firmware_sum, + task, + **firmware_update_args) + + @mock.patch.object(driver_utils, 'store_ramdisk_logs') + def test__update_firmware_sum_final_with_logs(self, store_mock): + self.config(deploy_logs_collect='always', group='agent') + command = {'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} + } + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.management._update_firmware_sum_final( + task, command) + store_mock.assert_called_once_with(task.node, 'aaaabbbbcccdddd', + label='update_firmware_sum') + + @mock.patch.object(driver_utils, 'store_ramdisk_logs') + def test__update_firmware_sum_final_without_logs(self, store_mock): + self.config(deploy_logs_collect='on_failure', group='agent') + command = {'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} + } + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.management._update_firmware_sum_final( + task, command) + self.assertFalse(store_mock.called) + + @mock.patch.object(ilo_management, 'LOG', spec_set=True, autospec=True) + @mock.patch.object(driver_utils, 'store_ramdisk_logs') + def test__update_firmware_sum_final_swift_error(self, store_mock, + log_mock): + self.config(deploy_logs_collect='always', group='agent') + command = {'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} + } + store_mock.side_effect = exception.SwiftOperationError('Error') + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.management._update_firmware_sum_final( + task, command) + self.assertTrue(log_mock.error.called) + + @mock.patch.object(ilo_management, 'LOG', spec_set=True, autospec=True) + @mock.patch.object(driver_utils, 'store_ramdisk_logs') + def test__update_firmware_sum_final_environment_error(self, store_mock, + log_mock): + self.config(deploy_logs_collect='always', group='agent') + command = {'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} + } + store_mock.side_effect = EnvironmentError('Error') + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.management._update_firmware_sum_final( + task, command) + self.assertTrue(log_mock.exception.called) + + @mock.patch.object(ilo_management, 'LOG', spec_set=True, autospec=True) + @mock.patch.object(driver_utils, 'store_ramdisk_logs') + def test__update_firmware_sum_final_unknown_exception(self, store_mock, + log_mock): + self.config(deploy_logs_collect='always', group='agent') + command = {'command_status': 'SUCCEEDED', + 'command_result': { + 'clean_result': {'Log Data': 'aaaabbbbcccdddd'}} + } + store_mock.side_effect = Exception('Error') + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.driver.management._update_firmware_sum_final( + task, command) + self.assertTrue(log_mock.exception.called) diff --git a/ironic/tests/unit/drivers/test_utils.py b/ironic/tests/unit/drivers/test_utils.py index d2d5be86e6..cb07c547ba 100644 --- a/ironic/tests/unit/drivers/test_utils.py +++ b/ironic/tests/unit/drivers/test_utils.py @@ -358,7 +358,7 @@ class UtilsRamdiskLogsTestCase(tests_base.TestCase): mock_swift.return_value.create_object.assert_called_once_with( container_name, file_name, mock.ANY, object_headers={'X-Delete-After': '86400'}) - mock_logs_name.assert_called_once_with(self.node) + mock_logs_name.assert_called_once_with(self.node, label=None) @mock.patch.object(os, 'makedirs', autospec=True) @mock.patch.object(driver_utils, @@ -379,4 +379,4 @@ class UtilsRamdiskLogsTestCase(tests_base.TestCase): mock_open.assert_called_once_with(expected_path, 'wb') mock_makedirs.assert_called_once_with(log_path) - mock_logs_name.assert_called_once_with(self.node) + mock_logs_name.assert_called_once_with(self.node, label=None) diff --git a/releasenotes/notes/sum-based-update-firmware-manual-clean-step-e69ade488060cf27.yaml b/releasenotes/notes/sum-based-update-firmware-manual-clean-step-e69ade488060cf27.yaml new file mode 100644 index 0000000000..4c15d93759 --- /dev/null +++ b/releasenotes/notes/sum-based-update-firmware-manual-clean-step-e69ade488060cf27.yaml @@ -0,0 +1,6 @@ +--- +features: + - iLO drivers now support firmware update based on `Smart Update Manager + `_ + (SUM) as an in-band manual cleaning step ``update_firmware_sum`` for + all the hardware components.