Merge "Support SUM based firmware update as clean step for iLO drivers"

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 (<name>: <value>) 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::
+
+    <node-uuid>[_<instance-uuid>]_update_firmware_sum_<timestamp yyyy-mm-dd-hh-mm-ss>.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

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
 =======================================
 

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

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
+# Supported components for firmware update when invoked through manual clean
-# through manual clean step, ``update_firmware``.
+# step, ``update_firmware``.
-SUPPORTED_FIRMWARE_UPDATE_COMPONENTS = ['ilo', 'cpld', 'power_pic', 'bios',
+SUPPORTED_ILO_FIRMWARE_UPDATE_COMPONENTS = ['ilo', 'cpld', 'power_pic', 'bios',
-                                        'chassis']
+                                            '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']
+    if firmware_update_mode == 'sum':
-    component = component.lower()
+        component = image_info.get('components')
-    if component not in SUPPORTED_FIRMWARE_UPDATE_COMPONENTS:
+        if component:
-        msg = (_("Component for firmware update is not supported. Provided "
+            _validate_sum_components(component)
-                 "value: %(component)s. Supported values are: "
+    else:
-                 "%(supported_components)s") %
+        component = image_info['component'].lower()
-               {'component': component, 'supported_components': (
+        _validate_ilo_component(component)
-                ", ".join(SUPPORTED_FIRMWARE_UPDATE_COMPONENTS))})
+        LOG.debug("Validating firmware image info: %s ... done", image_info)
-        LOG.error(msg)
+        return image_info['url'], image_info['checksum'], component
-        raise exception.InvalidParameterValue(msg)
-    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):
+def get_swift_url(parsed_url):
-    """Swift based firmware file downloader
+    """Gets swift temp url.
 
-    It generates a temp url for the swift based firmware url and then downloads
+    It generates a temp url for the swift based firmware url to the target
-    the firmware file via http based downloader to the target file.
+    file. Expecting url as swift://containername/objectname.
-    Expecting url as swift://containername/objectname
+
-    :param target_file: destination file for downloading the original firmware
+    :param parsed_url: Parsed url object.
-                        file.
+    :raises: SwiftOperationError, on failure to get url from swift.
-    :raises: SwiftOperationError, on failure to download from swift.
-    :raises: ImageDownloadFailed, on failure to download the original file.
     """
     # 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)
 
 

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})

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':

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 |

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)

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)

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
+    <http://h20564.www2.hpe.com/hpsc/doc/public/display?docId=emr_na-c04637207&sp4ts.oid=5182020>`_
+    (SUM) as an in-band manual cleaning step ``update_firmware_sum`` for
+    all the hardware components.