diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index 950f2d6166..7e13ec9598 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -24,6 +24,7 @@ from ironic_lib import utils as ironic_utils from oslo_config import cfg from oslo_log import log as logging from oslo_utils import importutils +import six import six.moves.urllib.parse as urlparse from six.moves.urllib.parse import urljoin @@ -135,6 +136,77 @@ def copy_image_to_web_server(source_file_path, destination): return image_url +def remove_image_from_web_server(object_name): + """Removes the given image from the configured http web server. + + This method removes the given image from the http_root location. It deletes + the image if it exists in web server. + + :param object_name: The name of the image file which needs to be removed + from the web server root. + """ + image_path = os.path.join(CONF.deploy.http_root, object_name) + ironic_utils.unlink_without_raise(image_path) + + +def copy_image_to_swift(source_file_path, destination_object_name): + """Uploads the given image to swift. + + This method copies the given image to swift. + + :param source_file_path: The absolute path of the image file which needs + to be copied to swift. + :param destination_object_name: The name of the object that will contain + the copied image. + :raises: SwiftOperationError, if any operation with Swift fails. + :returns: temp url from swift after the source image is uploaded. + + """ + container = CONF.ilo.swift_ilo_container + timeout = CONF.ilo.swift_object_expiry_timeout + + object_headers = {'X-Delete-After': timeout} + swift_api = swift.SwiftAPI() + swift_api.create_object(container, destination_object_name, + source_file_path, object_headers=object_headers) + temp_url = swift_api.get_temp_url(container, destination_object_name, + timeout) + LOG.debug("Uploaded image %(destination_object_name)s to %(container)s.", + {'destination_object_name': destination_object_name, + 'container': container}) + return temp_url + + +def remove_image_from_swift(object_name, associated_with=None): + """Removes the given image from swift. + + This method removes the given image name from swift. It deletes the + image if it exists in CONF.ilo.swift_ilo_container + + :param object_name: The name of the object which needs to be removed + from swift. + :param associated_with: string to depict the component/operation this + object is associated to. + """ + container = CONF.ilo.swift_ilo_container + try: + swift_api = swift.SwiftAPI() + swift_api.delete_object(container, object_name) + except exception.SwiftObjectNotFoundError as e: + LOG.warning( + _LW("Temporary object %(associated_with_msg)s" + "was already deleted from Swift. Error: %(err)s"), + {'associated_with_msg': ("associated with %s " % associated_with + if associated_with else ""), 'err': e}) + except exception.SwiftOperationError as e: + LOG.exception( + _LE("Error while deleting temporary swift object %(object_name)s " + "%(associated_with_msg)s from %(container)s. Error: %(err)s"), + {'object_name': object_name, 'container': container, + 'associated_with_msg': ("associated with %s" % associated_with + if associated_with else ""), 'err': e}) + + def parse_driver_info(node): """Gets the driver specific Node info. @@ -287,8 +359,10 @@ def _prepare_floppy_image(task, params): :param params: a dictionary containing 'parameter name'->'value' mapping to be passed to the deploy ramdisk via the floppy image. :raises: ImageCreationFailed, if it failed while creating the floppy image. + :raises: ImageUploadFailed, if copying the source file to the + web server fails. :raises: SwiftOperationError, if any operation with Swift fails. - :returns: the Swift temp url for the floppy image. + :returns: the http image URL or the Swift temp url for the floppy image. """ with tempfile.NamedTemporaryFile( dir=CONF.tempdir) as vfat_image_tmpfile_obj: @@ -299,22 +373,10 @@ def _prepare_floppy_image(task, params): if CONF.ilo.use_web_server_for_images: image_url = copy_image_to_web_server(vfat_image_tmpfile, object_name) - return image_url else: - container = CONF.ilo.swift_ilo_container - timeout = CONF.ilo.swift_object_expiry_timeout + image_url = copy_image_to_swift(vfat_image_tmpfile, object_name) - object_headers = {'X-Delete-After': timeout} - swift_api = swift.SwiftAPI() - swift_api.create_object(container, object_name, - vfat_image_tmpfile, - object_headers=object_headers) - temp_url = swift_api.get_temp_url(container, object_name, timeout) - - LOG.debug("Uploaded floppy image %(object_name)s to %(container)s " - "for deployment.", - {'object_name': object_name, 'container': container}) - return temp_url + return image_url def destroy_floppy_image_from_web_server(node): @@ -325,8 +387,7 @@ def destroy_floppy_image_from_web_server(node): """ object_name = _get_floppy_image_name(node) - image_path = os.path.join(CONF.deploy.http_root, object_name) - ironic_utils.unlink_without_raise(image_path) + remove_image_from_web_server(object_name) def attach_vmedia(node, device, url): @@ -546,20 +607,8 @@ def cleanup_vmedia_boot(task): LOG.debug("Cleaning up node %s after virtual media boot", task.node.uuid) if not CONF.ilo.use_web_server_for_images: - container = CONF.ilo.swift_ilo_container object_name = _get_floppy_image_name(task.node) - try: - swift_api = swift.SwiftAPI() - swift_api.delete_object(container, object_name) - except exception.SwiftObjectNotFoundError as e: - LOG.warning(_LW("Temporary object associated with virtual floppy " - "was already deleted from Swift. Error: %s"), e) - except exception.SwiftOperationError as e: - LOG.exception(_LE("Error while deleting temporary swift object " - "%(object_name)s from %(container)s associated " - "with virtual floppy. Error: %(error)s"), - {'object_name': object_name, 'container': container, - 'error': e}) + remove_image_from_swift(object_name, 'virtual floppy') else: destroy_floppy_image_from_web_server(task.node) eject_vmedia_devices(task) @@ -648,3 +697,53 @@ def update_secure_boot_mode(task, mode): set_secure_boot_mode(task, mode) LOG.info(_LI('Changed secure boot to %(mode)s for node %(node)s'), {'mode': mode, 'node': task.node.uuid}) + + +def remove_single_or_list_of_files(file_location): + """Removes (deletes) the file or list of files. + + This method only accepts single or list of files to delete. + If single file is passed, this method removes (deletes) the file. + If list of files is passed, this method removes (deletes) each of the + files iteratively. + + :param file_location: a single or a list of file paths + """ + # file_location is a list of files + if isinstance(file_location, list): + for location in file_location: + ironic_utils.unlink_without_raise(location) + # file_location is a single file path + elif isinstance(file_location, six.string_types): + ironic_utils.unlink_without_raise(file_location) + + +def verify_image_checksum(image_location, expected_checksum): + """Verifies checksum (md5) of image file against the expected one. + + This method generates the checksum of the image file on the fly and + verifies it against the expected checksum provided as argument. + + :param image_location: location of image file whose checksum is verified. + :param expected_checksum: checksum to be checked against + :raises: ImageRefValidationFailed, if invalid file path or + verification fails. + """ + try: + with open(image_location, 'rb') as fd: + actual_checksum = utils.hash_file(fd) + except IOError as e: + LOG.error(_LE("Error opening file: %(file)s"), + {'file': image_location}) + raise exception.ImageRefValidationFailed(image_href=image_location, + reason=six.text_type(e)) + + if actual_checksum != expected_checksum: + msg = (_('Error verifying image checksum. Image %(image)s failed to ' + 'verify against checksum %(checksum)s. Actual checksum is: ' + '%(actual_checksum)s') % + {'image': image_location, 'checksum': expected_checksum, + 'actual_checksum': actual_checksum}) + LOG.error(msg) + raise exception.ImageRefValidationFailed(image_href=image_location, + reason=msg) diff --git a/ironic/drivers/modules/ilo/firmware_processor.py b/ironic/drivers/modules/ilo/firmware_processor.py new file mode 100644 index 0000000000..63b80bedd7 --- /dev/null +++ b/ironic/drivers/modules/ilo/firmware_processor.py @@ -0,0 +1,389 @@ +# Copyright 2016 Hewlett Packard Enterprise Development Company, L.P. +# +# 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. +""" +Firmware file processor +""" + +import os +import shutil +import tempfile +import types + +from oslo_config import cfg +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 exception +from ironic.common.i18n import _, _LI +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'] + +# Mandatory fields to be provided as part of firmware image update +# with manual clean step +FIRMWARE_IMAGE_INFO_FIELDS = {'url', 'checksum', 'component'} + +CONF = cfg.CONF + +LOG = logging.getLogger(__name__) + +proliantutils_error = importutils.try_import('proliantutils.exception') +proliantutils_utils = importutils.try_import('proliantutils.utils') + + +def verify_firmware_update_args(func): + """Verifies the firmware update arguments.""" + @six.wraps(func) + def wrapper(self, task, **kwargs): + """Wrapper around ``update_firmware`` call. + + :param task: a TaskManager object. + :raises: InvalidParameterValue if validation fails for input arguments + of firmware update. + """ + firmware_update_mode = kwargs.get('firmware_update_mode') + firmware_images = kwargs.get('firmware_images') + + if firmware_update_mode != 'ilo': + msg = (_("Invalid firmware update mode: %(mode)s. " + "'ilo' is the only supported firmware update mode.") + % {'mode': firmware_update_mode}) + LOG.error(msg) + raise exception.InvalidParameterValue(msg) + + if not firmware_images: + msg = _("Firmware images cannot be an empty list or None. ") + LOG.error(msg) + raise exception.InvalidParameterValue(msg) + + return func(self, task, **kwargs) + + return wrapper + + +def get_and_validate_firmware_image_info(firmware_image_info): + """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 + """ + image_info = firmware_image_info or {} + + LOG.debug("Validating firmware image info: %s ... in progress", image_info) + missing_fields = [] + for field in FIRMWARE_IMAGE_INFO_FIELDS: + if not image_info.get(field): + missing_fields.append(field) + + if missing_fields: + msg = (_("Firmware image info: %(image_info)s is missing the " + "required %(missing)s field/s.") % + {'image_info': image_info, + 'missing': ", ".join(missing_fields)}) + 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 + + +class FirmwareProcessor(object): + """Firmware file processor + + This class helps in downloading the firmware file from url, extracting + the firmware file (if its in compact format) and makes it ready for + firmware update operation. In future, methods can be added as and when + required to extend functionality for different firmware file types. + """ + def __init__(self, url): + # :attribute ``self.parsed_url``: structure returned by urlparse + self._fine_tune_fw_processor(url) + + def _fine_tune_fw_processor(self, url): + """Fine tunes the firmware processor object based on specified url + + :param url: url of firmware file + :raises: InvalidParameterValue, for unsupported firmware url + """ + parsed_url = urlparse.urlparse(url) + self.parsed_url = parsed_url + + url_scheme = parsed_url.scheme + if url_scheme == 'file': + self._download_fw_to = types.MethodType( + _download_file_based_fw_to, self) + elif url_scheme in ('http', 'https'): + self._download_fw_to = types.MethodType( + _download_http_based_fw_to, self) + elif url_scheme == 'swift': + self._download_fw_to = types.MethodType( + _download_swift_based_fw_to, self) + else: + raise exception.InvalidParameterValue( + _('This method does not support URL scheme %(url_scheme)s. ' + 'Invalid URL %(url)s. The supported firmware URL schemes ' + 'are "file", "http", "https" and "swift"') % + {'url': url, 'url_scheme': url_scheme}) + + def process_fw_on(self, node, expected_checksum): + """Processes the firmware file from the url + + This is the template method which downloads the firmware file from + url, verifies checksum and extracts the firmware and makes it ready + for firmware update operation. ``_download_fw_to`` method is set in + the firmware processor object creation factory method, + ``get_fw_processor()``, based on the url type. + :param node: a single Node. + :param expected_checksum: checksum to be checked against. + :returns: wrapper object of raw firmware image location + :raises: IloOperationError, on failure to process firmware file. + :raises: ImageDownloadFailed, on failure to download the original file. + :raises: ImageRefValidationFailed, on failure to verify the checksum. + :raises: SwiftOperationError, if upload to Swift fails. + :raises: ImageUploadFailed, if upload to web server fails. + """ + filename = os.path.basename(self.parsed_url.path) + # create a temp directory where firmware file will be downloaded + temp_dir = tempfile.mkdtemp() + target_file = os.path.join(temp_dir, filename) + + # Note(deray): Operations performed in here: + # + # 1. Download the firmware file to the target file. + # 2. Verify the checksum of the downloaded file. + # 3. Extract the raw firmware file from its compact format + # + try: + LOG.debug("For firmware update, downloading firmware file " + "%(src_file)s to: %(target_file)s ...", + {'src_file': self.parsed_url.geturl(), + 'target_file': target_file}) + self._download_fw_to(target_file) + LOG.debug("For firmware update, verifying checksum of file: " + "%(target_file)s ...", {'target_file': target_file}) + ilo_common.verify_image_checksum(target_file, expected_checksum) + # Extracting raw firmware file from target_file ... + fw_image_location_obj, is_different_file = (_extract_fw_from_file( + node, target_file)) + except exception.IronicException: + with excutils.save_and_reraise_exception(): + # delete the target file along with temp dir and + # re-raise the exception + shutil.rmtree(temp_dir, ignore_errors=True) + + # Note(deray): In case of raw (no need for extraction) firmware files, + # the same firmware file is returned from the extract method. + # Hence, don't blindly delete the firmware file which gets passed on + # to extraction operation after successful extract. Check whether the + # file is same or not and then go ahead deleting it. + if is_different_file: + # delete the entire downloaded content along with temp dir. + shutil.rmtree(temp_dir, ignore_errors=True) + + LOG.info(_LI("Final processed firmware location: %s"), + fw_image_location_obj.fw_image_location) + return fw_image_location_obj + + +def _download_file_based_fw_to(self, target_file): + """File based firmware file downloader (copier) + + It copies the file (url) to temporary location (file location). + Original firmware file location (url) is expected in the format + "file:///tmp/.." + :param target_file: destination file for copying the original firmware + file. + :raises: ImageDownloadFailed, on failure to copy the original file. + """ + src_file = self.parsed_url.path + with open(target_file, 'wb') as fd: + image_service.FileImageService().download(src_file, fd) + + +def _download_http_based_fw_to(self, target_file): + """HTTP based firmware file downloader + + It downloads the file (url) to temporary location (file location). + Original firmware file location (url) is expected in the format + "http://.." + :param target_file: destination file for downloading the original firmware + file. + :raises: ImageDownloadFailed, on failure to download the original file. + """ + src_file = self.parsed_url.geturl() + with open(target_file, 'wb') as fd: + image_service.HttpImageService().download(src_file, fd) + + +def _download_swift_based_fw_to(self, target_file): + """Swift based firmware file downloader + + 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. + """ + # Extract container name and object name + container = self.parsed_url.netloc + objectname = os.path.basename(self.parsed_url.path) + timeout = CONF.ilo.swift_object_expiry_timeout + # Generate temp url using swift API + tempurl = swift.SwiftAPI().get_temp_url(container, objectname, timeout) + # 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) + _download_http_based_fw_to(self, target_file) + + +def _extract_fw_from_file(node, target_file): + """Extracts firmware image file. + + Extracts the firmware image file thru proliantutils and uploads it to the + conductor webserver, if needed. + :param node: an Ironic node object. + :param target_file: firmware file to be extracted from + :returns: tuple of: + a) wrapper object of raw firmware image location + b) a boolean, depending upon whether the raw firmware file was + already in raw format(same file remains, no need to extract) + or compact format (thereby extracted and hence different + file). If uploaded then, then also its a different file. + :raises: ImageUploadFailed, if upload to web server fails. + :raises: SwiftOperationError, if upload to Swift fails. + :raises: IloOperationError, on failure to process firmware file. + """ + ilo_object = ilo_common.get_ilo_object(node) + + try: + # Note(deray): Based upon different iLO firmwares, the firmware file + # which needs to be updated has to be either an http/https or a simple + # file location. If it has to be a http/https location, then conductor + # will take care of uploading the firmware file to web server or + # swift (providing a temp url). + fw_image_location, to_upload, is_extracted = ( + proliantutils_utils.process_firmware_image(target_file, + ilo_object)) + except (proliantutils_error.InvalidInputError, + proliantutils_error.ImageExtractionFailed) as proliantutils_exc: + operation = _("Firmware file extracting as part of manual cleaning") + raise exception.IloOperationError(operation=operation, + error=proliantutils_exc) + + is_different_file = is_extracted + fw_image_filename = os.path.basename(fw_image_location) + fw_image_location_obj = FirmwareImageLocation(fw_image_location, + fw_image_filename) + if to_upload: + is_different_file = True + LOG.debug("For firmware update, hosting firmware file: %s ... ", + fw_image_location) + # fw_image_filename = os.path.basename(fw_image_location) + try: + if CONF.ilo.use_web_server_for_images: + # upload firmware image file to conductor webserver + fw_image_uploaded_url = ilo_common.copy_image_to_web_server( + fw_image_location, fw_image_filename) + + fw_image_location_obj.fw_image_location = fw_image_uploaded_url + fw_image_location_obj.remove = types.MethodType( + _remove_webserver_based_me, fw_image_location_obj) + else: + # upload firmware image file to swift + fw_image_uploaded_url = ilo_common.copy_image_to_swift( + fw_image_location, fw_image_filename) + + fw_image_location_obj.fw_image_location = fw_image_uploaded_url + fw_image_location_obj.remove = types.MethodType( + _remove_swift_based_me, fw_image_location_obj) + finally: + if is_extracted: + # Note(deray): remove the file `fw_image_location` irrespective + # of status of uploading (success or failure) and only if + # extracted (and not passed as in plain binary format). If the + # file is passed in binary format, then the invoking method + # takes care of handling the deletion of the file. + ilo_common.remove_single_or_list_of_files(fw_image_location) + + LOG.debug("For firmware update, hosting firmware file: " + "%(fw_image_location)s ... done. Hosted firmware file: " + "%(fw_image_uploaded_url)s", + {'fw_image_location': fw_image_location, + 'fw_image_uploaded_url': fw_image_uploaded_url}) + else: + fw_image_location_obj.remove = types.MethodType( + _remove_file_based_me, fw_image_location_obj) + + return fw_image_location_obj, is_different_file + + +class FirmwareImageLocation(object): + """Firmware image location class + + This class acts as a wrapper class for the firmware image location. + It primarily helps in removing the firmware files from their respective + locations, made available for firmware update operation. + """ + + def __init__(self, fw_image_location, fw_image_filename): + """Keeps hold of image location and image filename""" + self.fw_image_location = fw_image_location + self.fw_image_filename = fw_image_filename + + def remove(self): + """Exposed method to remove the wrapped firmware file + + This method gets overriden by the remove method for the respective type + of firmware file location it wraps. + """ + pass + + +def _remove_file_based_me(self): + """Removes file based firmware image location""" + ilo_common.remove_single_or_list_of_files(self.fw_image_location) + + +def _remove_swift_based_me(self): + """Removes swift based firmware image location (by its object name)""" + ilo_common.remove_image_from_swift(self.fw_image_filename, + "firmware update") + + +def _remove_webserver_based_me(self): + """Removes webserver based firmware image location (by its file name)""" + ilo_common.remove_image_from_web_server(self.fw_image_filename) diff --git a/ironic/drivers/modules/ilo/management.py b/ironic/drivers/modules/ilo/management.py index 0c74af10b4..4281f7dc31 100644 --- a/ironic/drivers/modules/ilo/management.py +++ b/ironic/drivers/modules/ilo/management.py @@ -17,15 +17,17 @@ iLO Management Interface from oslo_config import cfg from oslo_log import log as logging +from oslo_utils import excutils from oslo_utils import importutils import six from ironic.common import boot_devices from ironic.common import exception -from ironic.common.i18n import _, _LI, _LW +from ironic.common.i18n import _, _LE, _LI, _LW from ironic.conductor import task_manager from ironic.drivers import base from ironic.drivers.modules.ilo import common as ilo_common +from ironic.drivers.modules.ilo import firmware_processor from ironic.drivers.modules import ipmitool LOG = logging.getLogger(__name__) @@ -333,3 +335,100 @@ class IloManagement(base.ManagementInterface): _execute_ilo_clean_step(node, 'activate_license', ilo_license_key) LOG.info(_LI("iLO license activated for node %(node)s."), {'node': node.uuid}) + + @base.clean_step(priority=0, abortable=False, argsinfo={ + 'firmware_update_mode': { + 'description': ( + 'This argument indicates the mode (or mechanism) of ' + 'Out-of-Band (OOB) firmware update procedure. ' + 'Currently, the only supported value is `ilo`.' + ), + 'required': True + }, + 'firmware_images': { + 'description': ( + 'This argument represents the ordered list of dictionaries of ' + 'firmware image url, its checksum (md5) and component type. ' + 'The firmware images will be applied (in the order given) ' + 'one by one on the baremetal server. The different types of ' + 'firmware url schemes supported are: ' + '`file`, `http`, `https` & `swift`. ' + 'And the different firmware components that can be updated ' + 'are: ' + '`ilo`, `cpld`, `power_pic`, `bios` & `chassis`.' + ), + 'required': True + } + }) + @firmware_processor.verify_firmware_update_args + def update_firmware(self, task, **kwargs): + """Updates the firmware. + + :param task: a TaskManager object. + :raises: InvalidParameterValue if update firmware mode is not 'ilo'. + Even applicable for invalid input cases. + :raises: NodeCleaningFailure, on failure to execute step. + """ + node = task.node + fw_location_objs_n_components = [] + firmware_images = kwargs['firmware_images'] + # Note(deray): Processing of firmware images happens here. As part + # of processing checksum validation is also done for the firmware file. + # Processing of firmware file essentially means downloading the file + # on the conductor, validating the checksum of the downloaded content, + # extracting the raw firmware file from its compact format, if it is, + # and hosting the file on a web server or a swift store based on the + # need of the baremetal server iLO firmware update method. + try: + for firmware_image_info in firmware_images: + url, checksum, component = ( + firmware_processor.get_and_validate_firmware_image_info( + firmware_image_info)) + LOG.debug("Processing of firmware file: %(firmware_file)s on " + "node: %(node)s ... in progress", + {'firmware_file': url, 'node': node.uuid}) + + fw_processor = firmware_processor.FirmwareProcessor(url) + fw_location_obj = fw_processor.process_fw_on(node, checksum) + fw_location_objs_n_components.append( + (fw_location_obj, component)) + + LOG.debug("Processing of firmware file: %(firmware_file)s on " + "node: %(node)s ... done", + {'firmware_file': url, 'node': node.uuid}) + except exception.IronicException as ilo_exc: + # delete all the files extracted so far from the extracted list + # and re-raise the exception + for fw_loc_obj_n_comp_tup in fw_location_objs_n_components: + fw_loc_obj_n_comp_tup[0].remove() + LOG.error(_LE("Processing of firmware image: %(firmware_image)s " + "on node: %(node)s ... failed"), + {'firmware_image': firmware_image_info, + 'node': node.uuid}) + raise exception.NodeCleaningFailure(node=node.uuid, reason=ilo_exc) + + # Updating of firmware images happen here. + try: + for fw_location_obj, component in fw_location_objs_n_components: + fw_location = fw_location_obj.fw_image_location + LOG.debug("Firmware update for %(firmware_file)s on " + "node: %(node)s ... in progress", + {'firmware_file': fw_location, 'node': node.uuid}) + + _execute_ilo_clean_step( + node, 'update_firmware', fw_location, component) + + LOG.debug("Firmware update for %(firmware_file)s on " + "node: %(node)s ... done", + {'firmware_file': fw_location, 'node': node.uuid}) + except exception.NodeCleaningFailure: + with excutils.save_and_reraise_exception(): + LOG.error(_LE("Firmware update for %(firmware_file)s on " + "node: %(node)s ... failed"), + {'firmware_file': fw_location, 'node': node.uuid}) + finally: + for fw_loc_obj_n_comp_tup in fw_location_objs_n_components: + fw_loc_obj_n_comp_tup[0].remove() + + LOG.info(_LI("All Firmware operations completed successfully for " + "node: %s."), node.uuid) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index d49c4d0686..d991cac1f1 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -15,6 +15,7 @@ """Test class for common methods used by iLO modules.""" +import hashlib import os import shutil import tempfile @@ -24,6 +25,7 @@ import mock from oslo_config import cfg from oslo_utils import importutils import six +import six.moves.builtins as __builtin__ from ironic.common import boot_devices from ironic.common import exception @@ -718,6 +720,73 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): copy_mock.assert_called_once_with(source, image_path) self.assertFalse(chmod_mock.called) + @mock.patch.object(ilo_common, 'ironic_utils', autospec=True) + def test_remove_image_from_web_server(self, utils_mock): + # | GIVEN | + CONF.deploy.http_url = "http://x.y.z.a/webserver/" + CONF.deploy.http_root = "/webserver" + object_name = 'tmp_image_file' + # | WHEN | + ilo_common.remove_image_from_web_server(object_name) + # | THEN | + (utils_mock.unlink_without_raise. + assert_called_once_with("/webserver/tmp_image_file")) + + @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) + @mock.patch.object(ilo_common, 'LOG') + def test_copy_image_to_swift(self, LOG_mock, swift_api_mock): + # | GIVEN | + self.config(swift_ilo_container='ilo_container', group='ilo') + self.config(swift_object_expiry_timeout=1, group='ilo') + container = CONF.ilo.swift_ilo_container + timeout = CONF.ilo.swift_object_expiry_timeout + + swift_obj_mock = swift_api_mock.return_value + destination_object_name = 'destination_object_name' + source_file_path = 'tmp_image_file' + object_headers = {'X-Delete-After': timeout} + # | WHEN | + ilo_common.copy_image_to_swift(source_file_path, + destination_object_name) + # | THEN | + swift_obj_mock.create_object.assert_called_once_with( + container, destination_object_name, source_file_path, + object_headers=object_headers) + swift_obj_mock.get_temp_url.assert_called_once_with( + container, destination_object_name, timeout) + + @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) + def test_copy_image_to_swift_throws_error_if_swift_operation_fails( + self, swift_api_mock): + # | GIVEN | + self.config(swift_ilo_container='ilo_container', group='ilo') + self.config(swift_object_expiry_timeout=1, group='ilo') + + swift_obj_mock = swift_api_mock.return_value + destination_object_name = 'destination_object_name' + source_file_path = 'tmp_image_file' + swift_obj_mock.create_object.side_effect = ( + exception.SwiftOperationError(operation='create_object', + error='failed')) + # | WHEN | & | THEN | + self.assertRaises(exception.SwiftOperationError, + ilo_common.copy_image_to_swift, + source_file_path, destination_object_name) + + @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) + def test_remove_image_from_swift(self, swift_api_mock): + # | GIVEN | + self.config(swift_ilo_container='ilo_container', group='ilo') + container = CONF.ilo.swift_ilo_container + + swift_obj_mock = swift_api_mock.return_value + object_name = 'object_name' + # | WHEN | + ilo_common.remove_image_from_swift(object_name) + # | THEN | + swift_obj_mock.delete_object.assert_called_once_with( + container, object_name) + @mock.patch.object(ironic_utils, 'unlink_without_raise', spec_set=True, autospec=True) @mock.patch.object(ilo_common, '_get_floppy_image_name', spec_set=True, @@ -774,3 +843,52 @@ class IloCommonMethodsTestCase(db_base.DbTestCase): func_is_secure_boot_req.return_value = False ilo_common.update_secure_boot_mode(task, False) self.assertFalse(func_set_secure_boot_mode.called) + + @mock.patch.object(ironic_utils, 'unlink_without_raise', spec_set=True, + autospec=True) + def test_remove_single_or_list_of_files(self, unlink_mock): + # | GIVEN | + file_list = ['/any_path1/any_file1', + '/any_path2/any_file2', + '/any_path3/any_file3'] + # | WHEN | + ilo_common.remove_single_or_list_of_files(file_list) + # | THEN | + calls = [mock.call('/any_path1/any_file1'), + mock.call('/any_path2/any_file2'), + mock.call('/any_path3/any_file3')] + unlink_mock.assert_has_calls(calls) + + @mock.patch.object(__builtin__, 'open', autospec=True) + def test_verify_image_checksum(self, open_mock): + # | GIVEN | + data = b'Yankee Doodle went to town riding on a pony;' + file_like_object = six.BytesIO(data) + open_mock().__enter__.return_value = file_like_object + actual_hash = hashlib.md5(data).hexdigest() + # | WHEN | + ilo_common.verify_image_checksum(file_like_object, actual_hash) + # | THEN | + # no any exception thrown + + def test_verify_image_checksum_throws_for_nonexistent_file(self): + # | GIVEN | + invalid_file_path = '/some/invalid/file/path' + # | WHEN | & | THEN | + self.assertRaises(exception.ImageRefValidationFailed, + ilo_common.verify_image_checksum, + invalid_file_path, 'hash_xxx') + + @mock.patch.object(__builtin__, 'open', autospec=True) + def test_verify_image_checksum_throws_for_failed_validation(self, + open_mock): + # | GIVEN | + data = b'Yankee Doodle went to town riding on a pony;' + file_like_object = six.BytesIO(data) + open_mock().__enter__.return_value = file_like_object + invalid_hash = 'invalid_hash_value' + # | WHEN | & | THEN | + self.assertRaises(exception.ImageRefValidationFailed, + ilo_common.verify_image_checksum, + file_like_object, + invalid_hash) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py b/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py new file mode 100644 index 0000000000..729cf58aa6 --- /dev/null +++ b/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py @@ -0,0 +1,552 @@ +# Copyright 2016 Hewlett Packard Enterprise Development Company, L.P. +# +# 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. + +"""Test class for Firmware Processor used by iLO management interface.""" + +import mock +from oslo_utils import importutils +import six +from six.moves import builtins as __builtin__ +import six.moves.urllib.parse as urlparse + +if six.PY3: + import io + file = io.BytesIO + +from ironic.common import exception +from ironic.drivers.modules.ilo import common as ilo_common +from ironic.drivers.modules.ilo import firmware_processor as ilo_fw_processor +from ironic.tests import base + +ilo_error = importutils.try_import('proliantutils.exception') + + +class FirmwareProcessorTestCase(base.TestCase): + + def setUp(self): + super(FirmwareProcessorTestCase, self).setUp() + self.any_url = 'http://netloc/path' + self.fw_processor_fake = mock.MagicMock( + parsed_url='set it as required') + + def test_verify_firmware_update_args_throws_for_invalid_update_mode(self): + # | GIVEN | + update_firmware_mock = mock.MagicMock() + firmware_update_args = {'firmware_update_mode': 'invalid_mode', + 'firmware_images': None} + # Note(deray): Need to set __name__ attribute explicitly to keep + # ``six.wraps`` happy. Passing this to the `name` argument at the time + # creation of Mock doesn't help. + update_firmware_mock.__name__ = 'update_firmware_mock' + wrapped_func = (ilo_fw_processor. + verify_firmware_update_args(update_firmware_mock)) + # | WHEN & THEN | + self.assertRaises(exception.InvalidParameterValue, + wrapped_func, + mock.ANY, + mock.ANY, + **firmware_update_args) + + def test_verify_firmware_update_args_throws_for_no_firmware_url(self): + # | GIVEN | + update_firmware_mock = mock.MagicMock() + firmware_update_args = {'firmware_update_mode': 'ilo', + 'firmware_images': []} + update_firmware_mock.__name__ = 'update_firmware_mock' + wrapped_func = (ilo_fw_processor. + verify_firmware_update_args(update_firmware_mock)) + # | WHEN & THEN | + self.assertRaises(exception.InvalidParameterValue, + wrapped_func, + mock.ANY, + mock.ANY, + **firmware_update_args) + + def test_get_and_validate_firmware_image_info(self): + # | GIVEN | + firmware_image_info = { + 'url': 'any_valid_url', + 'checksum': 'b64c8f7799cfbb553d384d34dc43fafe336cc889', + 'component': 'BIOS' + } + # | WHEN | + url, checksum, component = ( + ilo_fw_processor.get_and_validate_firmware_image_info( + firmware_image_info)) + # | THEN | + self.assertEqual('any_valid_url', url) + self.assertEqual('b64c8f7799cfbb553d384d34dc43fafe336cc889', checksum) + self.assertEqual('bios', component) + + def test_get_and_validate_firmware_image_info_fails_for_missing_parameter( + self): + # | GIVEN | + invalid_firmware_image_info = { + 'url': 'any_valid_url', + 'component': 'bios' + } + # | WHEN | & | THEN | + self.assertRaises( + exception.MissingParameterValue, + ilo_fw_processor.get_and_validate_firmware_image_info, + invalid_firmware_image_info) + + def test_get_and_validate_firmware_image_info_fails_for_empty_parameter( + self): + # | GIVEN | + invalid_firmware_image_info = { + 'url': 'any_valid_url', + 'checksum': 'valid_checksum', + 'component': '' + } + # | WHEN | & | THEN | + self.assertRaises( + exception.MissingParameterValue, + ilo_fw_processor.get_and_validate_firmware_image_info, + invalid_firmware_image_info) + + def test_get_and_validate_firmware_image_info_fails_for_invalid_component( + self): + # | GIVEN | + invalid_firmware_image_info = { + 'url': 'any_valid_url', + 'checksum': 'valid_checksum', + 'component': 'INVALID' + } + # | WHEN | & | THEN | + self.assertRaises( + exception.InvalidParameterValue, + ilo_fw_processor.get_and_validate_firmware_image_info, + invalid_firmware_image_info) + + def test_fw_processor_ctor_sets_parsed_url_attrib_of_fw_processor(self): + # | WHEN | + fw_processor = ilo_fw_processor.FirmwareProcessor(self.any_url) + # | THEN | + self.assertEqual(self.any_url, fw_processor.parsed_url.geturl()) + + @mock.patch.object( + ilo_fw_processor, '_download_file_based_fw_to', autospec=True) + def test__download_file_based_fw_to_gets_invoked_for_file_based_firmware( + self, _download_file_based_fw_to_mock): + # | GIVEN | + some_file_url = 'file:///some_location/some_firmware_file' + # | WHEN | + fw_processor = ilo_fw_processor.FirmwareProcessor(some_file_url) + fw_processor._download_fw_to('some_target_file') + # | THEN | + _download_file_based_fw_to_mock.assert_called_once_with( + fw_processor, 'some_target_file') + + @mock.patch.object( + ilo_fw_processor, '_download_http_based_fw_to', autospec=True) + def test__download_http_based_fw_to_gets_invoked_for_http_based_firmware( + self, _download_http_based_fw_to_mock): + # | GIVEN | + for some_http_url in ('http://netloc/path_to_firmware_file', + 'https://netloc/path_to_firmware_file'): + # | WHEN | + fw_processor = ilo_fw_processor.FirmwareProcessor(some_http_url) + fw_processor._download_fw_to('some_target_file') + # | THEN | + _download_http_based_fw_to_mock.assert_called_once_with( + fw_processor, 'some_target_file') + _download_http_based_fw_to_mock.reset_mock() + + @mock.patch.object( + ilo_fw_processor, '_download_swift_based_fw_to', autospec=True) + def test__download_swift_based_fw_to_gets_invoked_for_swift_based_firmware( + self, _download_swift_based_fw_to_mock): + # | GIVEN | + some_swift_url = 'swift://containername/objectname' + # | WHEN | + fw_processor = ilo_fw_processor.FirmwareProcessor(some_swift_url) + fw_processor._download_fw_to('some_target_file') + # | THEN | + _download_swift_based_fw_to_mock.assert_called_once_with( + fw_processor, 'some_target_file') + + def test_fw_processor_ctor_throws_exception_with_invalid_firmware_url( + self): + # | GIVEN | + any_invalid_firmware_url = 'any_invalid_url' + # | WHEN | & | THEN | + self.assertRaises(exception.InvalidParameterValue, + ilo_fw_processor.FirmwareProcessor, + any_invalid_firmware_url) + + @mock.patch.object(ilo_fw_processor, 'tempfile', autospec=True) + @mock.patch.object(ilo_fw_processor, 'os', autospec=True) + @mock.patch.object(ilo_fw_processor, 'shutil', autospec=True) + @mock.patch.object(ilo_common, 'verify_image_checksum', + spec_set=True, autospec=True) + @mock.patch.object( + ilo_fw_processor, '_extract_fw_from_file', autospec=True) + def test_process_fw_on_calls__download_fw_to( + self, _extract_fw_from_file_mock, verify_checksum_mock, + shutil_mock, os_mock, tempfile_mock): + # | GIVEN | + fw_processor = ilo_fw_processor.FirmwareProcessor(self.any_url) + # Now mock the __download_fw_to method of fw_processor instance + _download_fw_to_mock = mock.MagicMock() + fw_processor._download_fw_to = _download_fw_to_mock + + expected_return_location = (ilo_fw_processor.FirmwareImageLocation( + 'some_location/file', 'file')) + _extract_fw_from_file_mock.return_value = (expected_return_location, + True) + node_mock = mock.ANY + checksum_fake = mock.ANY + # | WHEN | + actual_return_location = fw_processor.process_fw_on(node_mock, + checksum_fake) + # | THEN | + _download_fw_to_mock.assert_called_once_with( + os_mock.path.join.return_value) + self.assertEqual(expected_return_location.fw_image_location, + actual_return_location.fw_image_location) + self.assertEqual(expected_return_location.fw_image_filename, + actual_return_location.fw_image_filename) + + @mock.patch.object(ilo_fw_processor, 'tempfile', autospec=True) + @mock.patch.object(ilo_fw_processor, 'os', autospec=True) + @mock.patch.object(ilo_fw_processor, 'shutil', autospec=True) + @mock.patch.object(ilo_common, 'verify_image_checksum', + spec_set=True, autospec=True) + @mock.patch.object( + ilo_fw_processor, '_extract_fw_from_file', autospec=True) + def test_process_fw_on_verifies_checksum_of_downloaded_fw_file( + self, _extract_fw_from_file_mock, verify_checksum_mock, + shutil_mock, os_mock, tempfile_mock): + # | GIVEN | + fw_processor = ilo_fw_processor.FirmwareProcessor(self.any_url) + # Now mock the __download_fw_to method of fw_processor instance + _download_fw_to_mock = mock.MagicMock() + fw_processor._download_fw_to = _download_fw_to_mock + + expected_return_location = (ilo_fw_processor.FirmwareImageLocation( + 'some_location/file', 'file')) + _extract_fw_from_file_mock.return_value = (expected_return_location, + True) + node_mock = mock.ANY + checksum_fake = mock.ANY + # | WHEN | + actual_return_location = fw_processor.process_fw_on(node_mock, + checksum_fake) + # | THEN | + _download_fw_to_mock.assert_called_once_with( + os_mock.path.join.return_value) + verify_checksum_mock.assert_called_once_with( + os_mock.path.join.return_value, checksum_fake) + self.assertEqual(expected_return_location.fw_image_location, + actual_return_location.fw_image_location) + self.assertEqual(expected_return_location.fw_image_filename, + actual_return_location.fw_image_filename) + + @mock.patch.object(ilo_fw_processor, 'tempfile', autospec=True) + @mock.patch.object(ilo_fw_processor, 'os', autospec=True) + @mock.patch.object(ilo_fw_processor, 'shutil', autospec=True) + @mock.patch.object(ilo_common, 'verify_image_checksum', + spec_set=True, autospec=True) + def test_process_fw_on_throws_error_if_checksum_validation_fails( + self, verify_checksum_mock, shutil_mock, os_mock, tempfile_mock): + # | GIVEN | + fw_processor = ilo_fw_processor.FirmwareProcessor(self.any_url) + # Now mock the __download_fw_to method of fw_processor instance + _download_fw_to_mock = mock.MagicMock() + fw_processor._download_fw_to = _download_fw_to_mock + + verify_checksum_mock.side_effect = exception.ImageRefValidationFailed( + image_href='some image', + reason='checksum verification failed') + node_mock = mock.ANY + checksum_fake = mock.ANY + # | WHEN | & | THEN | + self.assertRaises(exception.ImageRefValidationFailed, + fw_processor.process_fw_on, + node_mock, + checksum_fake) + + @mock.patch.object(ilo_fw_processor, 'tempfile', autospec=True) + @mock.patch.object(ilo_fw_processor, 'os', autospec=True) + @mock.patch.object(ilo_fw_processor, 'shutil', autospec=True) + @mock.patch.object(ilo_common, 'verify_image_checksum', + spec_set=True, autospec=True) + @mock.patch.object( + ilo_fw_processor, '_extract_fw_from_file', autospec=True) + def test_process_fw_on_calls__extract_fw_from_file( + self, _extract_fw_from_file_mock, verify_checksum_mock, + shutil_mock, os_mock, tempfile_mock): + # | GIVEN | + fw_processor = ilo_fw_processor.FirmwareProcessor(self.any_url) + # Now mock the __download_fw_to method of fw_processor instance + _download_fw_to_mock = mock.MagicMock() + fw_processor._download_fw_to = _download_fw_to_mock + + expected_return_location = (ilo_fw_processor.FirmwareImageLocation( + 'some_location/file', 'file')) + _extract_fw_from_file_mock.return_value = (expected_return_location, + True) + node_mock = mock.ANY + checksum_fake = mock.ANY + # | WHEN | + actual_return_location = fw_processor.process_fw_on(node_mock, + checksum_fake) + # | THEN | + _extract_fw_from_file_mock.assert_called_once_with( + node_mock, os_mock.path.join.return_value) + self.assertEqual(expected_return_location.fw_image_location, + actual_return_location.fw_image_location) + self.assertEqual(expected_return_location.fw_image_filename, + actual_return_location.fw_image_filename) + + @mock.patch.object(__builtin__, 'open', autospec=True) + @mock.patch.object( + ilo_fw_processor.image_service, 'FileImageService', autospec=True) + def test__download_file_based_fw_to_copies_file_to_target( + self, file_image_service_mock, open_mock): + # | GIVEN | + fd_mock = mock.MagicMock(spec=file) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + any_file_based_firmware_file = 'file:///tmp/any_file_path' + firmware_file_path = '/tmp/any_file_path' + self.fw_processor_fake.parsed_url = urlparse.urlparse( + any_file_based_firmware_file) + # | WHEN | + ilo_fw_processor._download_file_based_fw_to(self.fw_processor_fake, + 'target_file') + # | THEN | + file_image_service_mock.return_value.download.assert_called_once_with( + firmware_file_path, fd_mock) + + @mock.patch.object(__builtin__, 'open', autospec=True) + @mock.patch.object(ilo_fw_processor, 'image_service', autospec=True) + def test__download_http_based_fw_to_downloads_the_fw_file( + self, image_service_mock, open_mock): + # | GIVEN | + fd_mock = mock.MagicMock(spec=file) + open_mock.return_value = fd_mock + fd_mock.__enter__.return_value = fd_mock + any_http_based_firmware_file = 'http://netloc/path_to_firmware_file' + any_target_file = 'any_target_file' + self.fw_processor_fake.parsed_url = urlparse.urlparse( + any_http_based_firmware_file) + # | WHEN | + ilo_fw_processor._download_http_based_fw_to(self.fw_processor_fake, + any_target_file) + # | THEN | + image_service_mock.HttpImageService().download.assert_called_once_with( + any_http_based_firmware_file, fd_mock) + + @mock.patch.object(ilo_fw_processor, 'urlparse', autospec=True) + @mock.patch.object( + ilo_fw_processor, '_download_http_based_fw_to', autospec=True) + @mock.patch.object(ilo_fw_processor, 'swift', autospec=True) + def test__download_swift_based_fw_to_creates_temp_url( + self, swift_mock, _download_http_based_fw_to_mock, urlparse_mock): + # | GIVEN | + any_swift_based_firmware_file = 'swift://containername/objectname' + any_target_file = 'any_target_file' + self.fw_processor_fake.parsed_url = urlparse.urlparse( + any_swift_based_firmware_file) + # | WHEN | + ilo_fw_processor._download_swift_based_fw_to(self.fw_processor_fake, + any_target_file) + # | THEN | + swift_mock.SwiftAPI().get_temp_url.assert_called_once_with( + 'containername', 'objectname', mock.ANY) + + @mock.patch.object(urlparse, 'urlparse', autospec=True) + @mock.patch.object( + ilo_fw_processor, '_download_http_based_fw_to', autospec=True) + @mock.patch.object(ilo_fw_processor, 'swift', autospec=True) + def test__download_swift_based_fw_to_calls__download_http_based_fw_to( + self, swift_mock, _download_http_based_fw_to_mock, urlparse_mock): + """_download_swift_based_fw_to invokes _download_http_based_fw_to + + _download_swift_based_fw_to makes a call to _download_http_based_fw_to + in turn with temp url set as the url attribute of fw_processor instance + """ + # | GIVEN | + any_swift_based_firmware_file = 'swift://containername/objectname' + any_target_file = 'any_target_file' + self.fw_processor_fake.parsed_url = urlparse.urlparse( + any_swift_based_firmware_file) + # | WHEN | + ilo_fw_processor._download_swift_based_fw_to(self.fw_processor_fake, + any_target_file) + # | THEN | + _download_http_based_fw_to_mock.assert_called_once_with( + self.fw_processor_fake, any_target_file) + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) + def test__extract_fw_from_file_calls_process_firmware_image( + self, utils_mock, ilo_common_mock): + # | GIVEN | + node_mock = mock.ANY + any_target_file = 'any_target_file' + ilo_object_mock = ilo_common_mock.get_ilo_object.return_value + utils_mock.process_firmware_image.return_value = ('some_location', + True, True) + # | WHEN | + ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file) + # | THEN | + utils_mock.process_firmware_image.assert_called_once_with( + any_target_file, ilo_object_mock) + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) + def test__extract_fw_from_file_doesnt_upload_firmware( + self, utils_mock, ilo_common_mock): + # | GIVEN | + node_mock = mock.ANY + any_target_file = 'any_target_file' + utils_mock.process_firmware_image.return_value = ( + 'some_location/some_fw_file', False, True) + # | WHEN | + ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file) + # | THEN | + ilo_common_mock.copy_image_to_web_server.assert_not_called() + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) + @mock.patch.object(ilo_fw_processor, '_remove_file_based_me', + autospec=True) + def test__extract_fw_from_file_sets_loc_obj_remove_to_file_if_no_upload( + self, _remove_mock, utils_mock, ilo_common_mock): + # | GIVEN | + node_mock = mock.ANY + any_target_file = 'any_target_file' + utils_mock.process_firmware_image.return_value = ( + 'some_location/some_fw_file', False, True) + # | WHEN | + location_obj, is_different_file = ( + ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file)) + location_obj.remove() + # | THEN | + _remove_mock.assert_called_once_with(location_obj) + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) + def test__extract_fw_from_file_uploads_firmware_to_webserver( + self, utils_mock, ilo_common_mock): + # | GIVEN | + node_mock = mock.ANY + any_target_file = 'any_target_file' + utils_mock.process_firmware_image.return_value = ( + 'some_location/some_fw_file', True, True) + self.config(use_web_server_for_images=True, group='ilo') + # | WHEN | + ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file) + # | THEN | + ilo_common_mock.copy_image_to_web_server.assert_called_once_with( + 'some_location/some_fw_file', 'some_fw_file') + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) + @mock.patch.object(ilo_fw_processor, '_remove_webserver_based_me', + autospec=True) + def test__extract_fw_from_file_sets_loc_obj_remove_to_webserver( + self, _remove_mock, utils_mock, ilo_common_mock): + # | GIVEN | + node_mock = mock.ANY + any_target_file = 'any_target_file' + utils_mock.process_firmware_image.return_value = ( + 'some_location/some_fw_file', True, True) + self.config(use_web_server_for_images=True, group='ilo') + # | WHEN | + location_obj, is_different_file = ( + ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file)) + location_obj.remove() + # | THEN | + _remove_mock.assert_called_once_with(location_obj) + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) + def test__extract_fw_from_file_uploads_firmware_to_swift( + self, utils_mock, ilo_common_mock): + # | GIVEN | + node_mock = mock.ANY + any_target_file = 'any_target_file' + utils_mock.process_firmware_image.return_value = ( + 'some_location/some_fw_file', True, True) + self.config(use_web_server_for_images=False, group='ilo') + # | WHEN | + ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file) + # | THEN | + ilo_common_mock.copy_image_to_swift.assert_called_once_with( + 'some_location/some_fw_file', 'some_fw_file') + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + @mock.patch.object(ilo_fw_processor, 'proliantutils_utils', autospec=True) + @mock.patch.object(ilo_fw_processor, '_remove_swift_based_me', + autospec=True) + def test__extract_fw_from_file_sets_loc_obj_remove_to_swift( + self, _remove_mock, utils_mock, ilo_common_mock): + # | GIVEN | + node_mock = mock.ANY + any_target_file = 'any_target_file' + utils_mock.process_firmware_image.return_value = ( + 'some_location/some_fw_file', True, True) + self.config(use_web_server_for_images=False, group='ilo') + # | WHEN | + location_obj, is_different_file = ( + ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file)) + location_obj.remove() + # | THEN | + _remove_mock.assert_called_once_with(location_obj) + + def test_fw_img_loc_sets_these_attributes(self): + # | GIVEN | + any_loc = 'some_location/some_fw_file' + any_s_filename = 'some_fw_file' + # | WHEN | + location_obj = ilo_fw_processor.FirmwareImageLocation( + any_loc, any_s_filename) + # | THEN | + self.assertEqual(any_loc, location_obj.fw_image_location) + self.assertEqual(any_s_filename, location_obj.fw_image_filename) + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + def test__remove_file_based_me( + self, ilo_common_mock): + # | GIVEN | + fw_img_location_obj_fake = mock.MagicMock() + # | WHEN | + ilo_fw_processor._remove_file_based_me(fw_img_location_obj_fake) + # | THEN | + (ilo_common_mock.remove_single_or_list_of_files. + assert_called_with(fw_img_location_obj_fake.fw_image_location)) + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + def test__remove_swift_based_me(self, ilo_common_mock): + # | GIVEN | + fw_img_location_obj_fake = mock.MagicMock() + # | WHEN | + ilo_fw_processor._remove_swift_based_me(fw_img_location_obj_fake) + # | THEN | + (ilo_common_mock.remove_image_from_swift.assert_called_with( + fw_img_location_obj_fake.fw_image_filename, "firmware update")) + + @mock.patch.object(ilo_fw_processor, 'ilo_common', autospec=True) + def test__remove_webserver_based_me(self, ilo_common_mock): + # | GIVEN | + fw_img_location_obj_fake = mock.MagicMock() + # | WHEN | + ilo_fw_processor._remove_webserver_based_me(fw_img_location_obj_fake) + # | THEN | + (ilo_common_mock.remove_image_from_web_server.assert_called_with( + fw_img_location_obj_fake.fw_image_filename)) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_management.py b/ironic/tests/unit/drivers/modules/ilo/test_management.py index 17cafa53d5..e6683c84b8 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_management.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_management.py @@ -323,3 +323,228 @@ class IloManagementTestCase(db_base.DbTestCase): task, **activate_license_args) self.assertFalse(clean_step_mock.called) + + @mock.patch.object(ilo_management, 'LOG') + @mock.patch.object(ilo_management, '_execute_ilo_clean_step', + spec_set=True, autospec=True) + @mock.patch.object(ilo_management.firmware_processor, 'FirmwareProcessor', + spec_set=True, autospec=True) + @mock.patch.object(ilo_common, 'remove_single_or_list_of_files', + spec_set=True, autospec=True) + def test_update_firmware_calls_clean_step_foreach_url( + self, remove_file_mock, FirmwareProcessor_mock, clean_step_mock, + LOG_mock): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # | GIVEN | + firmware_images = [ + { + 'url': 'file:///any_path', + 'checksum': 'xxxx', + 'component': 'ilo' + }, + { + 'url': 'http://any_url', + 'checksum': 'xxxx', + 'component': 'cpld' + }, + { + 'url': 'https://any_url', + 'checksum': 'xxxx', + 'component': 'power_pic' + }, + { + 'url': 'swift://container/object', + 'checksum': 'xxxx', + 'component': 'bios' + }, + { + 'url': 'file:///any_path', + 'checksum': 'xxxx', + 'component': 'chassis' + } + ] + FirmwareProcessor_mock.return_value.process_fw_on.side_effect = [ + ilo_management.firmware_processor.FirmwareImageLocation( + 'fw_location_for_filepath', 'filepath'), + ilo_management.firmware_processor.FirmwareImageLocation( + 'fw_location_for_httppath', 'httppath'), + ilo_management.firmware_processor.FirmwareImageLocation( + 'fw_location_for_httpspath', 'httpspath'), + ilo_management.firmware_processor.FirmwareImageLocation( + 'fw_location_for_swiftpath', 'swiftpath'), + ilo_management.firmware_processor.FirmwareImageLocation( + 'fw_location_for_another_filepath', 'filepath2') + ] + firmware_update_args = {'firmware_update_mode': 'ilo', + 'firmware_images': firmware_images} + # | WHEN | + task.driver.management.update_firmware(task, + **firmware_update_args) + # | THEN | + calls = [mock.call(task.node, 'update_firmware', + 'fw_location_for_filepath', 'ilo'), + mock.call(task.node, 'update_firmware', + 'fw_location_for_httppath', 'cpld'), + mock.call(task.node, 'update_firmware', + 'fw_location_for_httpspath', 'power_pic'), + mock.call(task.node, 'update_firmware', + 'fw_location_for_swiftpath', 'bios'), + mock.call(task.node, 'update_firmware', + 'fw_location_for_another_filepath', 'chassis'), + ] + clean_step_mock.assert_has_calls(calls) + self.assertTrue(clean_step_mock.call_count == 5) + + def test_update_firmware_throws_if_invalid_update_mode_provided(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # | GIVEN | + firmware_update_args = {'firmware_update_mode': 'invalid_mode', + 'firmware_images': None} + # | WHEN & THEN | + self.assertRaises(exception.InvalidParameterValue, + task.driver.management.update_firmware, + task, + **firmware_update_args) + + def test_update_firmware_throws_error_for_no_firmware_url(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # | GIVEN | + firmware_update_args = {'firmware_update_mode': 'ilo', + 'firmware_images': []} + # | WHEN & THEN | + self.assertRaises(exception.InvalidParameterValue, + task.driver.management.update_firmware, + task, + **firmware_update_args) + + def test_update_firmware_throws_error_for_invalid_component_type(self): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # | GIVEN | + firmware_update_args = {'firmware_update_mode': 'ilo', + 'firmware_images': [ + { + 'url': 'any_valid_url', + 'checksum': 'xxxx', + 'component': 'xyz' + } + ]} + # | WHEN & THEN | + self.assertRaises(exception.NodeCleaningFailure, + task.driver.management.update_firmware, + task, + **firmware_update_args) + + @mock.patch.object(ilo_management, 'LOG') + @mock.patch.object(ilo_management.firmware_processor.FirmwareProcessor, + 'process_fw_on', spec_set=True, autospec=True) + def test_update_firmware_throws_error_for_checksum_validation_error( + self, process_fw_on_mock, LOG_mock): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # | GIVEN | + firmware_update_args = {'firmware_update_mode': 'ilo', + 'firmware_images': [ + { + 'url': 'any_valid_url', + 'checksum': 'invalid_checksum', + 'component': 'bios' + } + ]} + process_fw_on_mock.side_effect = exception.ImageRefValidationFailed + # | WHEN & THEN | + self.assertRaises(exception.NodeCleaningFailure, + task.driver.management.update_firmware, + task, + **firmware_update_args) + + @mock.patch.object(ilo_management, '_execute_ilo_clean_step', + spec_set=True, autospec=True) + @mock.patch.object(ilo_management.firmware_processor, 'FirmwareProcessor', + spec_set=True, autospec=True) + def test_update_firmware_doesnt_update_any_if_processing_on_any_url_fails( + self, FirmwareProcessor_mock, clean_step_mock): + """update_firmware throws error for failure in processing any url + + update_firmware doesn't invoke firmware update of proliantutils + for any url if processing on any firmware url fails. + """ + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # | GIVEN | + firmware_update_args = {'firmware_update_mode': 'ilo', + 'firmware_images': [ + { + 'url': 'any_valid_url', + 'checksum': 'xxxx', + 'component': 'ilo' + }, + { + 'url': 'any_invalid_url', + 'checksum': 'xxxx', + 'component': 'bios' + }] + } + FirmwareProcessor_mock.return_value.process_fw_on.side_effect = [ + ilo_management.firmware_processor.FirmwareImageLocation( + 'extracted_firmware_url_of_any_valid_url', 'filename'), + exception.IronicException + ] + # | WHEN & THEN | + self.assertRaises(exception.NodeCleaningFailure, + task.driver.management.update_firmware, + task, + **firmware_update_args) + self.assertFalse(clean_step_mock.called) + + @mock.patch.object(ilo_management, 'LOG') + @mock.patch.object(ilo_management, '_execute_ilo_clean_step', + spec_set=True, autospec=True) + @mock.patch.object(ilo_management.firmware_processor, 'FirmwareProcessor', + spec_set=True, autospec=True) + @mock.patch.object(ilo_management.firmware_processor.FirmwareImageLocation, + 'remove', spec_set=True, autospec=True) + def test_update_firmware_cleans_all_files_if_exc_thrown( + self, remove_mock, FirmwareProcessor_mock, clean_step_mock, + LOG_mock): + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + # | GIVEN | + firmware_update_args = {'firmware_update_mode': 'ilo', + 'firmware_images': [ + { + 'url': 'any_valid_url', + 'checksum': 'xxxx', + 'component': 'ilo' + }, + { + 'url': 'any_invalid_url', + 'checksum': 'xxxx', + 'component': 'bios' + }] + } + fw_loc_obj_1 = (ilo_management.firmware_processor. + FirmwareImageLocation('extracted_firmware_url_1', + 'filename_1')) + fw_loc_obj_2 = (ilo_management.firmware_processor. + FirmwareImageLocation('extracted_firmware_url_2', + 'filename_2')) + FirmwareProcessor_mock.return_value.process_fw_on.side_effect = [ + fw_loc_obj_1, fw_loc_obj_2 + ] + clean_step_mock.side_effect = exception.NodeCleaningFailure( + node=self.node.uuid, reason='ilo_exc') + # | WHEN & THEN | + self.assertRaises(exception.NodeCleaningFailure, + task.driver.management.update_firmware, + task, + **firmware_update_args) + clean_step_mock.assert_called_once_with( + task.node, 'update_firmware', + 'extracted_firmware_url_1', 'ilo') + self.assertTrue(LOG_mock.error.called) + remove_mock.assert_has_calls([mock.call(fw_loc_obj_1), + mock.call(fw_loc_obj_2)]) diff --git a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py index 97da43980f..1d2b6cea0d 100644 --- a/ironic/tests/unit/drivers/third_party_driver_mock_specs.py +++ b/ironic/tests/unit/drivers/third_party_driver_mock_specs.py @@ -48,6 +48,7 @@ IRONIC_INSPECTOR_CLIENT_SPEC = ( PROLIANTUTILS_SPEC = ( 'exception', 'ilo', + 'utils', ) # pyghmi diff --git a/ironic/tests/unit/drivers/third_party_driver_mocks.py b/ironic/tests/unit/drivers/third_party_driver_mocks.py index 40f20cbcd3..065dd67b4b 100644 --- a/ironic/tests/unit/drivers/third_party_driver_mocks.py +++ b/ironic/tests/unit/drivers/third_party_driver_mocks.py @@ -95,9 +95,15 @@ if not proliantutils: sys.modules['proliantutils.ilo'] = proliantutils.ilo sys.modules['proliantutils.ilo.client'] = proliantutils.ilo.client sys.modules['proliantutils.exception'] = proliantutils.exception + sys.modules['proliantutils.utils'] = proliantutils.utils + proliantutils.utils.process_firmware_image = mock.MagicMock() proliantutils.exception.IloError = type('IloError', (Exception,), {}) command_exception = type('IloCommandNotSupportedError', (Exception,), {}) proliantutils.exception.IloCommandNotSupportedError = command_exception + proliantutils.exception.InvalidInputError = type( + 'InvalidInputError', (Exception,), {}) + proliantutils.exception.ImageExtractionFailed = type( + 'ImageExtractionFailed', (Exception,), {}) if 'ironic.drivers.ilo' in sys.modules: six.moves.reload_module(sys.modules['ironic.drivers.ilo']) diff --git a/releasenotes/notes/ilo-firmware-update-manual-clean-step-e6763dc6dc0d441b.yaml b/releasenotes/notes/ilo-firmware-update-manual-clean-step-e6763dc6dc0d441b.yaml new file mode 100644 index 0000000000..c2b7c32e0c --- /dev/null +++ b/releasenotes/notes/ilo-firmware-update-manual-clean-step-e6763dc6dc0d441b.yaml @@ -0,0 +1,4 @@ +--- +features: + - iLO drivers now provide out-of-band firmware update as a manual cleaning + step, for supported hardware components.