From 03a927cab606071bda96ddd2e4eea817e3a9bf79 Mon Sep 17 00:00:00 2001 From: Ilya Etingof Date: Mon, 20 Apr 2020 16:57:56 +0000 Subject: [PATCH] Revert "Generalize ISO building for virtual media driver" This reverts commit c4d9984c51b15680ab4bfe38764942278ece5e1f. As per upstream/IRC discussion to address the problems documented here [1]. Besides minor hiccups (documented in the story), the proposal is to avoid building a swiss-army-knife function, but to either decompose the code onto smaller functional pieces and call them as appropriate or to introduce some overridable methods/properties for the vendor drivers to define/override. 1. https://storyboard.openstack.org/#!/story/2007576 Change-Id: Iff7fc267c3ed28b1adb270e9d5da16549be532a7 --- ironic/drivers/modules/deploy_utils.py | 31 -- ironic/drivers/modules/ilo/boot.py | 171 +++++---- ironic/drivers/modules/ilo/common.py | 20 +- .../drivers/modules/ilo/firmware_processor.py | 3 +- ironic/drivers/modules/redfish/boot.py | 178 +++++++-- ironic/drivers/modules/virtual_media_base.py | 173 --------- .../unit/drivers/modules/ilo/test_boot.py | 350 ++++++++++++++---- .../unit/drivers/modules/ilo/test_common.py | 39 +- .../modules/ilo/test_firmware_processor.py | 17 +- .../unit/drivers/modules/redfish/test_boot.py | 155 ++++++-- .../unit/drivers/modules/test_deploy_utils.py | 33 -- .../modules/test_virtual_media_base.py | 232 ------------ ...-iso-less-vmedia-ilo-e93002e335cb21e1.yaml | 8 - 13 files changed, 707 insertions(+), 703 deletions(-) delete mode 100644 ironic/drivers/modules/virtual_media_base.py delete mode 100644 ironic/tests/unit/drivers/modules/test_virtual_media_base.py delete mode 100644 releasenotes/notes/add-iso-less-vmedia-ilo-e93002e335cb21e1.yaml diff --git a/ironic/drivers/modules/deploy_utils.py b/ironic/drivers/modules/deploy_utils.py index 060f5322e9..582c334236 100644 --- a/ironic/drivers/modules/deploy_utils.py +++ b/ironic/drivers/modules/deploy_utils.py @@ -16,9 +16,7 @@ import os import re -import shutil import time -from urllib import parse as urlparse from ironic_lib import metrics_utils from ironic_lib import utils as il_utils @@ -824,35 +822,6 @@ def direct_deploy_should_convert_raw_image(node): return CONF.force_raw_images and CONF.agent.stream_raw_images -def copy_image_to_web_server(source_file_path, destination): - """Copies the given image to the http web server. - - This method copies the given image to the http_root location. - It enables read-write access to the image else the deploy fails - as the image file at the web_server url is inaccessible. - - :param source_file_path: The absolute path of the image file - which needs to be copied to the - web server root. - :param destination: The name of the file that - will contain the copied image. - :raises: ImageUploadFailed exception if copying the source - file to the web server fails. - :returns: image url after the source image is uploaded. - - """ - - image_url = urlparse.urljoin(CONF.deploy.http_url, destination) - image_path = os.path.join(CONF.deploy.http_root, destination) - try: - shutil.copyfile(source_file_path, image_path) - except IOError as exc: - raise exception.ImageUploadFailed(image_name=destination, - web_server=CONF.deploy.http_url, - reason=exc) - return image_url - - @image_cache.cleanup(priority=50) class InstanceImageCache(image_cache.ImageCache): diff --git a/ironic/drivers/modules/ilo/boot.py b/ironic/drivers/modules/ilo/boot.py index 8c93ccc97e..b6142bb8be 100644 --- a/ironic/drivers/modules/ilo/boot.py +++ b/ironic/drivers/modules/ilo/boot.py @@ -16,6 +16,8 @@ Boot Interface for iLO drivers and its supporting methods. """ import os +import tempfile +from urllib import parse as urlparse from ironic_lib import metrics_utils from ironic_lib import utils as ironic_utils @@ -38,7 +40,6 @@ from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.ilo import common as ilo_common from ironic.drivers.modules import ipxe from ironic.drivers.modules import pxe -from ironic.drivers.modules import virtual_media_base LOG = logging.getLogger(__name__) @@ -46,45 +47,16 @@ METRICS = metrics_utils.get_metrics_logger(__name__) CONF = cfg.CONF -DEPLOY_ISO_PROPERTIES = { +REQUIRED_PROPERTIES = { 'ilo_deploy_iso': _("UUID (from Glance) of the deployment ISO. " "Required.") } - -DEPLOY_RAMDISK_PROPERTIES = { - 'deploy_kernel': _("URL or Glance UUID of the deployment kernel. " - "Required."), - 'deploy_ramdisk': _("URL or Glance UUID of the ramdisk that is " - "mounted at boot time. Required.") -} - -OPTIONAL_PROPERTIES = { - 'bootloader': _("URL or Glance UUID of the EFI system partition " - "image containing EFI boot loader. This image will be " - "used by ironic when building UEFI-bootable ISO " - "out of kernel and ramdisk. Required for UEFI " - "boot from partition images.") -} - -RESCUE_ISO_PROPERTIES = { +RESCUE_PROPERTIES = { 'ilo_rescue_iso': _("UUID (from Glance) of the rescue ISO. Only " "required if rescue mode is being used and ironic is " "managing booting the rescue ramdisk.") } - -RESCUE_RAMDISK_PROPERTIES = { - 'rescue_kernel': _("URL or Glance UUID of the rescue kernel. " - "Required."), - 'rescue_ramdisk': _("URL or Glance UUID of the ramdisk that is " - "mounted at boot time. Required.") -} - -COMMON_PROPERTIES = DEPLOY_ISO_PROPERTIES - -KERNEL_RAMDISK_LABELS = { - 'deploy': DEPLOY_RAMDISK_PROPERTIES, - 'rescue': RESCUE_RAMDISK_PROPERTIES -} +COMMON_PROPERTIES = REQUIRED_PROPERTIES def parse_driver_info(node, mode='deploy'): @@ -105,37 +77,26 @@ def parse_driver_info(node, mode='deploy'): """ info = node.driver_info d_info = {} - if mode == 'rescue' and info.get('ilo_rescue_iso'): + if mode == 'rescue': d_info['ilo_rescue_iso'] = info.get('ilo_rescue_iso') - elif mode == 'deploy' and info.get('ilo_deploy_iso'): - d_info['ilo_deploy_iso'] = info.get('ilo_deploy_iso') else: - params_to_check = KERNEL_RAMDISK_LABELS[mode] - - d_info = {option: info.get(option) - for option in params_to_check} - - if not any(d_info.values()): - # NOTE(dtantsur): avoid situation when e.g. deploy_kernel comes - # from driver_info but deploy_ramdisk comes from configuration, - # since it's a sign of a potential operator's mistake. - d_info = {k: getattr(CONF.conductor, k) - for k in params_to_check} - - error_msg = (_("Error validating iLO virtual media for %(mode)s. " - "Either 'ilo_%(mode)s_iso' is missing or " - "DEPLOY_RAMDISK_PROPERTIES or RESCUE_RAMDISK_PROPERTIES " - "were missing in node's driver_info.") % {'mode': mode}) + d_info['ilo_deploy_iso'] = info.get('ilo_deploy_iso') + error_msg = (_("Error validating iLO virtual media for %s. Some " + "parameters were missing in node's driver_info") % mode) deploy_utils.check_for_missing_params(d_info, error_msg) - d_info.update( - {option: info.get(option, getattr(CONF.conductor, option, None)) - for option in OPTIONAL_PROPERTIES}) - return d_info +def _get_boot_iso_object_name(node): + """Returns the boot iso object name for a given node. + + :param node: the node for which object name is to be provided. + """ + return "boot-%s" % node.uuid + + def _get_boot_iso(task, root_uuid): """This method returns a boot ISO to boot the node. @@ -210,6 +171,12 @@ def _get_boot_iso(task, root_uuid): LOG.debug("Found boot_iso %s in Glance", boot_iso_uuid) return boot_iso_uuid + if not kernel_href or not ramdisk_href: + LOG.error("Unable to find kernel or ramdisk for " + "image %(image)s to generate boot ISO for %(node)s", + {'image': image_href, 'node': task.node.uuid}) + return + # NOTE(rameshg87): Functionality to share the boot ISOs created for # similar instances (instances with same deployed image) is # not implemented as of now. Creation/Deletion of such a shared boot ISO @@ -219,15 +186,43 @@ def _get_boot_iso(task, root_uuid): # Option 3 - Create boot_iso from kernel/ramdisk, upload to Swift # or web server and provide its name. deploy_iso_uuid = deploy_info['ilo_deploy_iso'] + boot_mode = boot_mode_utils.get_boot_mode(task.node) + boot_iso_object_name = _get_boot_iso_object_name(task.node) + kernel_params = "" + if deploy_utils.get_boot_option(task.node) == "ramdisk": + i_info = task.node.instance_info + kernel_params = "root=/dev/ram0 text " + kernel_params += i_info.get("ramdisk_kernel_arguments", "") + else: + kernel_params = CONF.pxe.pxe_append_params + with tempfile.NamedTemporaryFile(dir=CONF.tempdir) as fileobj: + boot_iso_tmp_file = fileobj.name + images.create_boot_iso(task.context, boot_iso_tmp_file, + kernel_href, ramdisk_href, + deploy_iso_href=deploy_iso_uuid, + root_uuid=root_uuid, + kernel_params=kernel_params, + boot_mode=boot_mode) - use_web_server = CONF.ilo.use_web_server_for_images - container = CONF.ilo.swift_ilo_container + if CONF.ilo.use_web_server_for_images: + boot_iso_url = ( + ilo_common.copy_image_to_web_server(boot_iso_tmp_file, + boot_iso_object_name)) + driver_internal_info = task.node.driver_internal_info + driver_internal_info['boot_iso_created_in_web_server'] = True + task.node.driver_internal_info = driver_internal_info + task.node.save() + LOG.debug("Created boot_iso %(boot_iso)s for node %(node)s", + {'boot_iso': boot_iso_url, 'node': task.node.uuid}) + return boot_iso_url + else: + container = CONF.ilo.swift_ilo_container + swift_api = swift.SwiftAPI() + swift_api.create_object(container, boot_iso_object_name, + boot_iso_tmp_file) - return virtual_media_base.prepare_iso_image( - task, kernel_href, ramdisk_href, - deploy_iso_href=deploy_iso_uuid, - root_uuid=root_uuid, use_web_server=use_web_server, - container=container) + LOG.debug("Created boot_iso %s in Swift", boot_iso_object_name) + return 'swift:%s' % boot_iso_object_name def _clean_up_boot_iso_for_instance(node): @@ -235,21 +230,25 @@ def _clean_up_boot_iso_for_instance(node): :param node: an ironic node object. """ - boot_iso_object_name = virtual_media_base.get_iso_image_name(node) - - if CONF.ilo.use_web_server_for_images: - boot_iso_path = os.path.join( - CONF.deploy.http_root, boot_iso_object_name) - ironic_utils.unlink_without_raise(boot_iso_path) - else: + ilo_boot_iso = node.instance_info.get('ilo_boot_iso') + if not ilo_boot_iso: + return + if ilo_boot_iso.startswith('swift'): swift_api = swift.SwiftAPI() container = CONF.ilo.swift_ilo_container + boot_iso_object_name = _get_boot_iso_object_name(node) try: swift_api.delete_object(container, boot_iso_object_name) except exception.SwiftOperationError as e: LOG.exception("Failed to clean up boot ISO for node " "%(node)s. Error: %(error)s.", {'node': node.uuid, 'error': e}) + elif CONF.ilo.use_web_server_for_images: + result = urlparse.urlparse(ilo_boot_iso) + ilo_boot_iso_name = os.path.basename(result.path) + boot_iso_path = os.path.join( + CONF.deploy.http_root, ilo_boot_iso_name) + ironic_utils.unlink_without_raise(boot_iso_path) def _parse_deploy_info(node): @@ -285,7 +284,19 @@ def _validate_driver_info(task): """ node = task.node ilo_common.parse_driver_info(node) - parse_driver_info(node) + if 'ilo_deploy_iso' not in node.driver_info: + raise exception.MissingParameterValue(_( + "Missing 'ilo_deploy_iso' parameter in node's 'driver_info'.")) + deploy_iso = node.driver_info['ilo_deploy_iso'] + if not service_utils.is_glance_image(deploy_iso): + try: + image_service.HttpImageService().validate_href(deploy_iso) + except exception.ImageRefValidationFailed: + raise exception.InvalidParameterValue(_( + "Virtual media boot accepts only Glance images or " + "HTTP(S) as driver_info['ilo_deploy_iso']. Either '%s' " + "is not a glance UUID or not a valid HTTP(S) URL or " + "the given URL is not reachable.") % deploy_iso) def _validate_instance_image_info(task): @@ -507,19 +518,11 @@ class IloVirtualMediaBoot(base.BootInterface): deploy_nic_mac = deploy_utils.get_single_nic_with_vif_port_id(task) ramdisk_params['BOOTIF'] = deploy_nic_mac - if (node.driver_info.get('ilo_rescue_iso') - and node.provision_state == states.RESCUING): + if node.provision_state == states.RESCUING: iso = node.driver_info['ilo_rescue_iso'] - elif node.driver_info.get('ilo_deploy_iso'): - iso = node.driver_info['ilo_deploy_iso'] else: - mode = deploy_utils.rescue_or_deploy_mode(node) - d_info = parse_driver_info(node, mode) - use_web_server = CONF.ilo.use_web_server_for_images - container = CONF.ilo.swift_ilo_container - iso = virtual_media_base.prepare_deploy_iso( - task, ramdisk_params, mode, d_info, - use_web_server=use_web_server, container=container) + iso = node.driver_info['ilo_deploy_iso'] + ilo_common.setup_vmedia(task, iso, ramdisk_params) @METRICS.timer('IloVirtualMediaBoot.prepare_instance') @@ -633,10 +636,6 @@ class IloVirtualMediaBoot(base.BootInterface): :returns: None :raises: IloOperationError, if some operation on iLO failed. """ - info = task.node.driver_info - - if not info.get('ilo_deploy_iso') and not info.get('ilo_rescue_iso'): - _clean_up_boot_iso_for_instance(task.node) ilo_common.cleanup_vmedia_boot(task) def _configure_vmedia_boot(self, task, root_uuid): diff --git a/ironic/drivers/modules/ilo/common.py b/ironic/drivers/modules/ilo/common.py index eb99c9568e..62c6bbb233 100644 --- a/ironic/drivers/modules/ilo/common.py +++ b/ironic/drivers/modules/ilo/common.py @@ -17,6 +17,7 @@ Common functionalities shared between different iLO modules. """ import os +import shutil import tempfile from urllib import parse as urlparse @@ -133,12 +134,17 @@ def copy_image_to_web_server(source_file_path, destination): :returns: image url after the source image is uploaded. """ - LOG.warning( - 'This method is obsolete and will be removed in next release. ' - 'Please use deploy_utils.copy_image_to_web_server() instead.') - return deploy_utils.copy_image_to_web_server(source_file_path, - destination) + image_url = urlparse.urljoin(CONF.deploy.http_url, destination) + image_path = os.path.join(CONF.deploy.http_root, destination) + try: + shutil.copyfile(source_file_path, image_path) + except IOError as exc: + raise exception.ImageUploadFailed(image_name=destination, + web_server=CONF.deploy.http_url, + reason=exc) + os.chmod(image_path, 0o644) + return image_url def remove_image_from_web_server(object_name): @@ -414,8 +420,8 @@ def _prepare_floppy_image(task, params): images.create_vfat_image(vfat_image_tmpfile, parameters=params) object_name = _get_floppy_image_name(task.node) if CONF.ilo.use_web_server_for_images: - image_url = deploy_utils.copy_image_to_web_server( - vfat_image_tmpfile, object_name) + image_url = copy_image_to_web_server(vfat_image_tmpfile, + object_name) else: image_url = copy_image_to_swift(vfat_image_tmpfile, object_name) diff --git a/ironic/drivers/modules/ilo/firmware_processor.py b/ironic/drivers/modules/ilo/firmware_processor.py index ca704f5b89..53282311e5 100644 --- a/ironic/drivers/modules/ilo/firmware_processor.py +++ b/ironic/drivers/modules/ilo/firmware_processor.py @@ -32,7 +32,6 @@ from ironic.common import exception from ironic.common.i18n import _ from ironic.common import image_service from ironic.common import swift -from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.ilo import common as ilo_common # Supported components for firmware update when invoked through manual clean @@ -371,7 +370,7 @@ def _extract_fw_from_file(node, target_file): "firmware file %(firmware_image)s on web server ...", {'firmware_image': fw_image_location, 'node': node.uuid}) - fw_image_uploaded_url = deploy_utils.copy_image_to_web_server( + 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 diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index 931211ee32..f842a04fdc 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -20,6 +20,7 @@ from urllib import parse as urlparse from ironic_lib import utils as ironic_utils from oslo_log import log +from oslo_serialization import base64 from oslo_utils import importutils from ironic.common import boot_devices @@ -35,7 +36,6 @@ from ironic.drivers import base from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.redfish import utils as redfish_utils -from ironic.drivers.modules import virtual_media_base LOG = log.getLogger(__name__) @@ -392,16 +392,163 @@ class RedfishVirtualMediaBoot(base.BootInterface): return image_url + @staticmethod + def _get_iso_image_name(node): + """Returns the boot iso image name for a given node. + + :param node: the node for which image name is to be provided. + """ + return "boot-%s" % node.uuid + @classmethod def _cleanup_iso_image(cls, task): """Deletes the ISO if it was created for the instance. :param task: an ironic node object. """ - iso_object_name = virtual_media_base.get_iso_image_name(task.node) + iso_object_name = cls._get_iso_image_name(task.node) cls._unpublish_image(iso_object_name) + @classmethod + def _prepare_iso_image(cls, task, kernel_href, ramdisk_href, + bootloader_href=None, configdrive=None, + root_uuid=None, params=None): + """Prepare an ISO to boot the node. + + Build bootable ISO out of `kernel_href` and `ramdisk_href` (and + `bootloader` if it's UEFI boot), then push built image up to Swift and + return a temporary URL. + + :param task: a TaskManager instance containing the node to act on. + :param kernel_href: URL or Glance UUID of the kernel to use + :param ramdisk_href: URL or Glance UUID of the ramdisk to use + :param bootloader_href: URL or Glance UUID of the EFI bootloader + image to use when creating UEFI bootbable ISO + :param configdrive: URL to or a compressed blob of a ISO9660 or + FAT-formatted OpenStack config drive image. This image will be + written onto the built ISO image. Optional. + :param root_uuid: optional uuid of the root partition. + :param params: a dictionary containing 'parameter name'->'value' + mapping to be passed to kernel command line. + :returns: bootable ISO HTTP URL. + :raises: MissingParameterValue, if any of the required parameters are + missing. + :raises: InvalidParameterValue, if any of the parameters have invalid + value. + :raises: ImageCreationFailed, if creating ISO image failed. + """ + if not kernel_href or not ramdisk_href: + raise exception.InvalidParameterValue(_( + "Unable to find kernel or ramdisk for " + "building ISO for %(node)s") % + {'node': task.node.uuid}) + + i_info = task.node.instance_info + + if deploy_utils.get_boot_option(task.node) == "ramdisk": + kernel_params = "root=/dev/ram0 text " + kernel_params += i_info.get("ramdisk_kernel_arguments", "") + + else: + kernel_params = i_info.get( + 'kernel_append_params', CONF.redfish.kernel_append_params) + + if params: + kernel_params = ' '.join( + (kernel_params, ' '.join( + '%s=%s' % kv for kv in params.items()))) + + boot_mode = boot_mode_utils.get_boot_mode_for_deploy(task.node) + + LOG.debug("Trying to create %(boot_mode)s ISO image for node %(node)s " + "with kernel %(kernel_href)s, ramdisk %(ramdisk_href)s, " + "bootloader %(bootloader_href)s and kernel params %(params)s" + "", {'node': task.node.uuid, + 'boot_mode': boot_mode, + 'kernel_href': kernel_href, + 'ramdisk_href': ramdisk_href, + 'bootloader_href': bootloader_href, + 'params': kernel_params}) + + with tempfile.NamedTemporaryFile( + dir=CONF.tempdir, suffix='.iso') as boot_fileobj: + + with tempfile.NamedTemporaryFile( + dir=CONF.tempdir, suffix='.img') as cfgdrv_fileobj: + + configdrive_href = configdrive + + if configdrive: + parsed_url = urlparse.urlparse(configdrive) + if not parsed_url.scheme: + cfgdrv_blob = base64.decode_as_bytes(configdrive) + + with open(cfgdrv_fileobj.name, 'wb') as f: + f.write(cfgdrv_blob) + + configdrive_href = urlparse.urlunparse( + ('file', '', cfgdrv_fileobj.name, '', '', '')) + + LOG.info("Burning configdrive %(url)s to boot ISO image " + "for node %(node)s", {'url': configdrive_href, + 'node': task.node.uuid}) + + boot_iso_tmp_file = boot_fileobj.name + images.create_boot_iso( + task.context, boot_iso_tmp_file, + kernel_href, ramdisk_href, + esp_image_href=bootloader_href, + configdrive_href=configdrive_href, + root_uuid=root_uuid, + kernel_params=kernel_params, + boot_mode=boot_mode) + + iso_object_name = cls._get_iso_image_name(task.node) + + image_url = cls._publish_image( + boot_iso_tmp_file, iso_object_name) + + LOG.debug("Created ISO %(name)s in object store for node %(node)s, " + "exposed as temporary URL " + "%(url)s", {'node': task.node.uuid, + 'name': iso_object_name, + 'url': image_url}) + + return image_url + + @classmethod + def _prepare_deploy_iso(cls, task, params, mode): + """Prepare deploy or rescue ISO image + + Build bootable ISO out of + `[driver_info]/deploy_kernel`/`[driver_info]/deploy_ramdisk` or + `[driver_info]/rescue_kernel`/`[driver_info]/rescue_ramdisk` + and `[driver_info]/bootloader`, then push built image up to Glance + and return temporary Swift URL to the image. + + :param task: a TaskManager instance containing the node to act on. + :param params: a dictionary containing 'parameter name'->'value' + mapping to be passed to kernel command line. + :param mode: either 'deploy' or 'rescue'. + :returns: bootable ISO HTTP URL. + :raises: MissingParameterValue, if any of the required parameters are + missing. + :raises: InvalidParameterValue, if any of the parameters have invalid + value. + :raises: ImageCreationFailed, if creating ISO image failed. + """ + node = task.node + + d_info = cls._parse_driver_info(node) + + kernel_href = d_info.get('%s_kernel' % mode) + ramdisk_href = d_info.get('%s_ramdisk' % mode) + bootloader_href = d_info.get('bootloader') + + return cls._prepare_iso_image( + task, kernel_href, ramdisk_href, bootloader_href, params=params) + @classmethod def _prepare_boot_iso(cls, task, root_uuid=None): """Prepare boot ISO image @@ -451,19 +598,9 @@ class RedfishVirtualMediaBoot(base.BootInterface): bootloader_href = d_info.get('bootloader') - if CONF.redfish.use_swift: - use_web_server = False - container = CONF.redfish.swift_container - timeout = CONF.redfish.swift_object_expiry_timeout - else: - use_web_server = True - container = None - timeout = None - - return virtual_media_base.prepare_iso_image( - task, kernel_href, ramdisk_href, bootloader_href=bootloader_href, - root_uuid=root_uuid, timeout=timeout, - use_web_server=use_web_server, container=container) + return cls._prepare_iso_image( + task, kernel_href, ramdisk_href, bootloader_href, + root_uuid=root_uuid) def get_properties(self): """Return the properties of the interface. @@ -616,16 +753,7 @@ class RedfishVirtualMediaBoot(base.BootInterface): mode = deploy_utils.rescue_or_deploy_mode(node) - if CONF.redfish.use_swift: - use_web_server = False - container = CONF.redfish.swift_container - else: - use_web_server = True - container = None - - iso_ref = virtual_media_base.prepare_deploy_iso( - task, ramdisk_params, mode, d_info, - use_web_server=use_web_server, container=container) + iso_ref = self._prepare_deploy_iso(task, ramdisk_params, mode) self._eject_vmedia(task, sushy.VIRTUAL_MEDIA_CD) self._insert_vmedia(task, iso_ref, sushy.VIRTUAL_MEDIA_CD) diff --git a/ironic/drivers/modules/virtual_media_base.py b/ironic/drivers/modules/virtual_media_base.py deleted file mode 100644 index 9bda27f611..0000000000 --- a/ironic/drivers/modules/virtual_media_base.py +++ /dev/null @@ -1,173 +0,0 @@ -# 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. - -import tempfile - -from oslo_log import log - -from ironic.common import exception -from ironic.common.i18n import _ -from ironic.common import images -from ironic.common import swift -from ironic.conf import CONF -from ironic.drivers.modules import boot_mode_utils -from ironic.drivers.modules import deploy_utils - -LOG = log.getLogger(__name__) - - -def get_iso_image_name(node): - """Returns the boot iso image name for a given node. - - :param node: the node for which image name is to be provided. - """ - return "boot-%s" % node.uuid - - -def prepare_iso_image(task, kernel_href, ramdisk_href, deploy_iso_href=None, - bootloader_href=None, root_uuid=None, - kernel_params=None, timeout=None, - use_web_server=False, container=None): - """Prepare an ISO to boot the node. - - Build bootable ISO out of `kernel_href` and `ramdisk_href` (and - `bootloader` if it's UEFI boot), then push built image up to Swift and - return a temporary URL. - - :param task: a TaskManager instance containing the node to act on. - :param kernel_href: URL or Glance UUID of the kernel to use - :param ramdisk_href: URL or Glance UUID of the ramdisk to use - :param bootloader_href: URL or Glance UUID of the EFI bootloader - image to use when creating UEFI bootbable ISO - :param root_uuid: optional uuid of the root partition. - :param kernel_params: a dictionary containing 'parameter name'->'value' - mapping to be passed to kernel command line. - :param timeout: swift object expiry timeout - :returns: bootable ISO HTTP URL. - :raises: MissingParameterValue, if any of the required parameters are - missing. - :raises: InvalidParameterValue, if any of the parameters have invalid - value. - :raises: ImageCreationFailed, if creating ISO image failed. - """ - if not kernel_href or not ramdisk_href: - raise exception.InvalidParameterValue(_( - "Unable to find kernel or ramdisk for " - "building ISO for %(node)s") % - {'node': task.node.uuid}) - - boot_mode = boot_mode_utils.get_boot_mode_for_deploy(task.node) - - LOG.debug("Trying to create %(boot_mode)s ISO image for node %(node)s " - "with kernel %(kernel_href)s, ramdisk %(ramdisk_href)s, " - "bootloader %(bootloader_href)s and kernel params %(params)s" - "", {'node': task.node.uuid, - 'boot_mode': boot_mode, - 'kernel_href': kernel_href, - 'ramdisk_href': ramdisk_href, - 'bootloader_href': bootloader_href, - 'params': kernel_params}) - - with tempfile.NamedTemporaryFile( - dir=CONF.tempdir, suffix='.iso') as fileobj: - boot_iso_tmp_file = fileobj.name - images.create_boot_iso(task.context, boot_iso_tmp_file, - kernel_href, ramdisk_href, - deploy_iso_href=deploy_iso_href, - esp_image_href=bootloader_href, - root_uuid=root_uuid, - kernel_params=kernel_params, - boot_mode=boot_mode) - - iso_object_name = get_iso_image_name(task.node) - - if use_web_server: - boot_iso_url = ( - deploy_utils.copy_image_to_web_server(boot_iso_tmp_file, - iso_object_name)) - driver_internal_info = task.node.driver_internal_info - driver_internal_info['boot_iso_created_in_web_server'] = True - task.node.driver_internal_info = driver_internal_info - task.node.save() - LOG.debug("Created boot_iso %(boot_iso)s for node %(node)s", - {'boot_iso': boot_iso_url, 'node': task.node.uuid}) - return boot_iso_url - else: - swift_api = swift.SwiftAPI() - - object_headers = None - if task.node.driver == 'redfish': - object_headers = {'X-Delete-After': str(timeout)} - - swift_api.create_object(container, iso_object_name, - boot_iso_tmp_file, - object_headers=object_headers) - - LOG.debug("Created ISO %(name)s in Swift for node %(node)s", - {'node': task.node.uuid, 'name': iso_object_name}) - - if task.node.driver == 'redfish': - boot_iso_url = swift_api.get_temp_url( - container, iso_object_name, timeout) - return boot_iso_url - else: - return 'swift:%s' % iso_object_name - - -def prepare_deploy_iso(task, params, mode, driver_info, - use_web_server=False, container=None): - """Prepare deploy or rescue ISO image - - Build bootable ISO out of - `[driver_info]/deploy_kernel`/`[driver_info]/deploy_ramdisk` or - `[driver_info]/rescue_kernel`/`[driver_info]/rescue_ramdisk` - and `[driver_info]/bootloader`, then push built image up to Glance - and return temporary Swift URL to the image. - - :param task: a TaskManager instance containing the node to act on. - :param params: a dictionary containing 'parameter name'->'value' - mapping to be passed to kernel command line. - :param mode: either 'deploy' or 'rescue'. - :param driver_info: a dictionary containing driver_info values. - :returns: bootable ISO HTTP URL. - :raises: MissingParameterValue, if any of the required parameters are - missing. - :raises: InvalidParameterValue, if any of the parameters have invalid - value. - :raises: ImageCreationFailed, if creating ISO image failed. - """ - - kernel_href = driver_info.get('%s_kernel' % mode) - ramdisk_href = driver_info.get('%s_ramdisk' % mode) - bootloader_href = driver_info.get('bootloader') - timeout = None - - if deploy_utils.get_boot_option(task.node) == "ramdisk": - i_info = task.node.instance_info - kernel_params = "root=/dev/ram0 text " - kernel_params += i_info.get("ramdisk_kernel_arguments", "") - elif task.node.driver == 'redfish': - kernel_params = CONF.redfish.kernel_append_params - timeout = CONF.redfish.swift_object_expiry_timeout - else: - kernel_params = CONF.pxe.pxe_append_params - - if params: - kernel_params = ' '.join( - (kernel_params, ' '.join( - '%s=%s' % kv for kv in params.items()))) - - return prepare_iso_image(task, kernel_href, ramdisk_href, - bootloader_href=bootloader_href, - kernel_params=kernel_params, timeout=timeout, - use_web_server=use_web_server, - container=container) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_boot.py b/ironic/tests/unit/drivers/modules/ilo/test_boot.py index 2c880788e9..1e133a5402 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_boot.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_boot.py @@ -15,6 +15,9 @@ """Test class for boot methods used by iLO modules.""" +import io +import tempfile + from ironic_lib import utils as ironic_utils import mock from oslo_config import cfg @@ -36,7 +39,6 @@ from ironic.drivers.modules.ilo import management as ilo_management from ironic.drivers.modules import ipxe from ironic.drivers.modules import pxe from ironic.drivers.modules.storage import noop as noop_storage -from ironic.drivers.modules import virtual_media_base from ironic.drivers import utils as driver_utils from ironic.tests.unit.drivers.modules.ilo import test_common @@ -48,44 +50,13 @@ class IloBootCommonMethodsTestCase(test_common.BaseIloTest): boot_interface = 'ilo-virtual-media' - def test_parse_driver_info_deploy_iso(self): + def test_parse_driver_info(self): self.node.driver_info['ilo_deploy_iso'] = 'deploy-iso' - expected_driver_info = {'bootloader': None, - 'ilo_deploy_iso': 'deploy-iso'} + expected_driver_info = {'ilo_deploy_iso': 'deploy-iso'} actual_driver_info = ilo_boot.parse_driver_info(self.node) self.assertEqual(expected_driver_info, actual_driver_info) - def test_parse_driver_info_rescue_iso(self): - self.node.driver_info['ilo_rescue_iso'] = 'rescue-iso' - expected_driver_info = {'bootloader': None, - 'ilo_rescue_iso': 'rescue-iso'} - - actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue') - self.assertEqual(expected_driver_info, actual_driver_info) - - def test_parse_driver_info_deploy(self): - self.node.driver_info['deploy_kernel'] = 'kernel' - self.node.driver_info['deploy_ramdisk'] = 'ramdisk' - self.node.driver_info['bootloader'] = 'bootloader' - expected_driver_info = {'deploy_kernel': 'kernel', - 'deploy_ramdisk': 'ramdisk', - 'bootloader': 'bootloader'} - - actual_driver_info = ilo_boot.parse_driver_info(self.node) - self.assertEqual(expected_driver_info, actual_driver_info) - - def test_parse_driver_info_rescue(self): - self.node.driver_info['rescue_kernel'] = 'kernel' - self.node.driver_info['rescue_ramdisk'] = 'ramdisk' - self.node.driver_info['bootloader'] = 'bootloader' - expected_driver_info = {'rescue_kernel': 'kernel', - 'rescue_ramdisk': 'ramdisk', - 'bootloader': 'bootloader'} - - actual_driver_info = ilo_boot.parse_driver_info(self.node, 'rescue') - self.assertEqual(expected_driver_info, actual_driver_info) - def test_parse_driver_info_exc(self): self.assertRaises(exception.MissingParameterValue, ilo_boot.parse_driver_info, self.node) @@ -95,6 +66,11 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): boot_interface = 'ilo-virtual-media' + def test__get_boot_iso_object_name(self): + boot_iso_actual = ilo_boot._get_boot_iso_object_name(self.node) + boot_iso_expected = "boot-%s" % self.node.uuid + self.assertEqual(boot_iso_expected, boot_iso_actual) + @mock.patch.object(image_service.HttpImageService, 'validate_href', spec_set=True, autospec=True) def test__get_boot_iso_http_url(self, service_mock): @@ -152,21 +128,71 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): boot_iso_expected = u'glance://uui\u0111' self.assertEqual(boot_iso_expected, boot_iso_actual) - @mock.patch.object(virtual_media_base, 'prepare_iso_image', spec_set=True, + @mock.patch.object(boot_mode_utils, 'get_boot_mode_for_deploy', + spec_set=True, autospec=True) + @mock.patch.object(ilo_boot.LOG, 'error', spec_set=True, autospec=True) + @mock.patch.object(images, 'get_image_properties', spec_set=True, + autospec=True) + @mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True, + autospec=True) + def test__get_boot_iso_uefi_no_glance_image(self, + deploy_info_mock, + image_props_mock, + log_mock, + boot_mode_mock): + deploy_info_mock.return_value = {'image_source': 'image-uuid', + 'ilo_deploy_iso': 'deploy_iso_uuid'} + image_props_mock.return_value = {'boot_iso': None, + 'kernel_id': None, + 'ramdisk_id': None} + properties = {'capabilities': 'boot_mode:uefi'} + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.properties = properties + boot_iso_result = ilo_boot._get_boot_iso(task, 'root-uuid') + deploy_info_mock.assert_called_once_with(task.node) + image_props_mock.assert_called_once_with( + task.context, 'image-uuid', + ['boot_iso', 'kernel_id', 'ramdisk_id']) + self.assertTrue(log_mock.called) + self.assertFalse(boot_mode_mock.called) + self.assertIsNone(boot_iso_result) + + @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, + autospec=True) + @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) + @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) + @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, + autospec=True) + @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, autospec=True) @mock.patch.object(images, 'get_image_properties', spec_set=True, autospec=True) @mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True, autospec=True) - def test__get_boot_iso_create(self, deploy_info_mock, - image_props_mock, prepare_iso_mock): + def test__get_boot_iso_create(self, deploy_info_mock, image_props_mock, + capability_mock, boot_object_name_mock, + swift_api_mock, + create_boot_iso_mock, tempfile_mock): + CONF.ilo.swift_ilo_container = 'ilo-cont' + CONF.pxe.pxe_append_params = 'kernel-params' + + swift_obj_mock = swift_api_mock.return_value + fileobj_mock = mock.MagicMock(spec=io.BytesIO) + fileobj_mock.name = 'tmpfile' + mock_file_handle = mock.MagicMock(spec=io.BytesIO) + mock_file_handle.__enter__.return_value = fileobj_mock + tempfile_mock.return_value = mock_file_handle + deploy_info_mock.return_value = {'image_source': 'image-uuid', 'ilo_deploy_iso': 'deploy_iso_uuid'} image_props_mock.return_value = {'boot_iso': None, 'kernel_id': 'kernel_uuid', 'ramdisk_id': 'ramdisk_uuid'} - - prepare_iso_mock.return_value = 'swift:boot-iso' + boot_object_name_mock.return_value = 'abcdef' + create_boot_iso_mock.return_value = '/path/to/boot-iso' + capability_mock.return_value = 'uefi' with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: boot_iso_actual = ilo_boot._get_boot_iso(task, 'root-uuid') @@ -174,30 +200,166 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): image_props_mock.assert_called_once_with( task.context, 'image-uuid', ['boot_iso', 'kernel_id', 'ramdisk_id']) - prepare_iso_mock.assert_called_once_with( - task, 'kernel_uuid', 'ramdisk_uuid', + boot_object_name_mock.assert_called_once_with(task.node) + create_boot_iso_mock.assert_called_once_with( + task.context, 'tmpfile', 'kernel_uuid', 'ramdisk_uuid', deploy_iso_href='deploy_iso_uuid', - root_uuid='root-uuid', use_web_server=False, - container='ironic_ilo_container') - boot_iso_expected = 'swift:boot-iso' + root_uuid='root-uuid', + kernel_params='kernel-params', + boot_mode='uefi') + swift_obj_mock.create_object.assert_called_once_with('ilo-cont', + 'abcdef', + 'tmpfile') + boot_iso_expected = 'swift:abcdef' self.assertEqual(boot_iso_expected, boot_iso_actual) - @mock.patch.object(virtual_media_base, 'get_iso_image_name', - spec_set=True, autospec=True) + @mock.patch.object(ilo_common, 'copy_image_to_web_server', spec_set=True, + autospec=True) + @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, + autospec=True) + @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) + @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, + autospec=True) + @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, + autospec=True) + @mock.patch.object(images, 'get_image_properties', spec_set=True, + autospec=True) + @mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True, + autospec=True) + def test__get_boot_iso_recreate_boot_iso_use_webserver( + self, deploy_info_mock, image_props_mock, + capability_mock, boot_object_name_mock, + create_boot_iso_mock, tempfile_mock, + copy_file_mock): + CONF.ilo.swift_ilo_container = 'ilo-cont' + CONF.ilo.use_web_server_for_images = True + CONF.deploy.http_url = "http://10.10.1.30/httpboot" + CONF.deploy.http_root = "/httpboot" + CONF.pxe.pxe_append_params = 'kernel-params' + + fileobj_mock = mock.MagicMock(spec=io.BytesIO) + fileobj_mock.name = 'tmpfile' + mock_file_handle = mock.MagicMock(spec=io.BytesIO) + mock_file_handle.__enter__.return_value = fileobj_mock + tempfile_mock.return_value = mock_file_handle + + ramdisk_href = "http://10.10.1.30/httpboot/ramdisk" + kernel_href = "http://10.10.1.30/httpboot/kernel" + deploy_info_mock.return_value = {'image_source': 'image-uuid', + 'ilo_deploy_iso': 'deploy_iso_uuid'} + image_props_mock.return_value = {'boot_iso': None, + 'kernel_id': kernel_href, + 'ramdisk_id': ramdisk_href} + boot_object_name_mock.return_value = 'new_boot_iso' + create_boot_iso_mock.return_value = '/path/to/boot-iso' + capability_mock.return_value = 'uefi' + copy_file_mock.return_value = "http://10.10.1.30/httpboot/new_boot_iso" + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + driver_internal_info = task.node.driver_internal_info + driver_internal_info['boot_iso_created_in_web_server'] = True + instance_info = task.node.instance_info + old_boot_iso = 'http://10.10.1.30/httpboot/old_boot_iso' + instance_info['ilo_boot_iso'] = old_boot_iso + boot_iso_actual = ilo_boot._get_boot_iso(task, 'root-uuid') + deploy_info_mock.assert_called_once_with(task.node) + image_props_mock.assert_called_once_with( + task.context, 'image-uuid', + ['boot_iso', 'kernel_id', 'ramdisk_id']) + boot_object_name_mock.assert_called_once_with(task.node) + create_boot_iso_mock.assert_called_once_with( + task.context, 'tmpfile', kernel_href, ramdisk_href, + deploy_iso_href='deploy_iso_uuid', + root_uuid='root-uuid', + kernel_params='kernel-params', + boot_mode='uefi') + boot_iso_expected = 'http://10.10.1.30/httpboot/new_boot_iso' + self.assertEqual(boot_iso_expected, boot_iso_actual) + copy_file_mock.assert_called_once_with(fileobj_mock.name, + 'new_boot_iso') + + @mock.patch.object(ilo_common, 'copy_image_to_web_server', spec_set=True, + autospec=True) + @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, + autospec=True) + @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) + @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, + autospec=True) + @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, + autospec=True) + @mock.patch.object(images, 'get_image_properties', spec_set=True, + autospec=True) + @mock.patch.object(ilo_boot, '_parse_deploy_info', spec_set=True, + autospec=True) + def test__get_boot_iso_create_use_webserver_true_ramdisk_webserver( + self, deploy_info_mock, image_props_mock, + capability_mock, boot_object_name_mock, + create_boot_iso_mock, tempfile_mock, + copy_file_mock): + CONF.ilo.swift_ilo_container = 'ilo-cont' + CONF.ilo.use_web_server_for_images = True + CONF.deploy.http_url = "http://10.10.1.30/httpboot" + CONF.deploy.http_root = "/httpboot" + CONF.pxe.pxe_append_params = 'kernel-params' + + fileobj_mock = mock.MagicMock(spec=io.BytesIO) + fileobj_mock.name = 'tmpfile' + mock_file_handle = mock.MagicMock(spec=io.BytesIO) + mock_file_handle.__enter__.return_value = fileobj_mock + tempfile_mock.return_value = mock_file_handle + + ramdisk_href = "http://10.10.1.30/httpboot/ramdisk" + kernel_href = "http://10.10.1.30/httpboot/kernel" + deploy_info_mock.return_value = {'image_source': 'image-uuid', + 'ilo_deploy_iso': 'deploy_iso_uuid'} + image_props_mock.return_value = {'boot_iso': None, + 'kernel_id': kernel_href, + 'ramdisk_id': ramdisk_href} + boot_object_name_mock.return_value = 'abcdef' + create_boot_iso_mock.return_value = '/path/to/boot-iso' + capability_mock.return_value = 'uefi' + copy_file_mock.return_value = "http://10.10.1.30/httpboot/abcdef" + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + boot_iso_actual = ilo_boot._get_boot_iso(task, 'root-uuid') + deploy_info_mock.assert_called_once_with(task.node) + image_props_mock.assert_called_once_with( + task.context, 'image-uuid', + ['boot_iso', 'kernel_id', 'ramdisk_id']) + boot_object_name_mock.assert_called_once_with(task.node) + create_boot_iso_mock.assert_called_once_with( + task.context, 'tmpfile', kernel_href, ramdisk_href, + deploy_iso_href='deploy_iso_uuid', + root_uuid='root-uuid', + kernel_params='kernel-params', + boot_mode='uefi') + boot_iso_expected = 'http://10.10.1.30/httpboot/abcdef' + self.assertEqual(boot_iso_expected, boot_iso_actual) + copy_file_mock.assert_called_once_with(fileobj_mock.name, + 'abcdef') + + @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, + autospec=True) @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) def test__clean_up_boot_iso_for_instance(self, swift_mock, boot_object_name_mock): swift_obj_mock = swift_mock.return_value CONF.ilo.swift_ilo_container = 'ilo-cont' boot_object_name_mock.return_value = 'boot-object' + i_info = self.node.instance_info + i_info['ilo_boot_iso'] = 'swift:bootiso' + self.node.instance_info = i_info + self.node.save() ilo_boot._clean_up_boot_iso_for_instance(self.node) swift_obj_mock.delete_object.assert_called_once_with('ilo-cont', 'boot-object') @mock.patch.object(ilo_boot.LOG, 'exception', spec_set=True, autospec=True) - @mock.patch.object(virtual_media_base, 'get_iso_image_name', - spec_set=True, autospec=True) + @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, + autospec=True) @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) def test__clean_up_boot_iso_for_instance_exc(self, swift_mock, boot_object_name_mock, @@ -207,6 +369,10 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): swift_obj_mock.delete_object.side_effect = exc CONF.ilo.swift_ilo_container = 'ilo-cont' boot_object_name_mock.return_value = 'boot-object' + i_info = self.node.instance_info + i_info['ilo_boot_iso'] = 'swift:bootiso' + self.node.instance_info = i_info + self.node.save() ilo_boot._clean_up_boot_iso_for_instance(self.node) swift_obj_mock.delete_object.assert_called_once_with('ilo-cont', 'boot-object') @@ -214,17 +380,25 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): @mock.patch.object(ironic_utils, 'unlink_without_raise', spec_set=True, autospec=True) - @mock.patch.object(virtual_media_base, 'get_iso_image_name', - spec_set=True, autospec=True) - def test__clean_up_boot_iso_for_instance_on_webserver( - self, boot_object_name_mock, unlink_mock): - boot_object_name_mock.return_value = 'boot-object' + def test__clean_up_boot_iso_for_instance_on_webserver(self, unlink_mock): + CONF.ilo.use_web_server_for_images = True CONF.deploy.http_root = "/webserver" + i_info = self.node.instance_info + i_info['ilo_boot_iso'] = 'http://x.y.z.a/webserver/boot-object' + self.node.instance_info = i_info + self.node.save() boot_iso_path = "/webserver/boot-object" ilo_boot._clean_up_boot_iso_for_instance(self.node) unlink_mock.assert_called_once_with(boot_iso_path) + @mock.patch.object(ilo_boot, '_get_boot_iso_object_name', spec_set=True, + autospec=True) + def test__clean_up_boot_iso_for_instance_no_boot_iso( + self, boot_object_name_mock): + ilo_boot._clean_up_boot_iso_for_instance(self.node) + self.assertFalse(boot_object_name_mock.called) + @mock.patch.object(ilo_boot, 'parse_driver_info', spec_set=True, autospec=True) @mock.patch.object(deploy_utils, 'get_image_instance_info', @@ -238,15 +412,68 @@ class IloBootPrivateMethodsTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, autospec=True) - @mock.patch.object(ilo_boot, 'parse_driver_info', spec_set=True, - autospec=True) - def test__validate_driver_info(self, mock_driver_info, - mock_parse_driver_info): + def test__validate_driver_info_MissingParam(self, mock_parse_driver_info): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: + self.assertRaisesRegex(exception.MissingParameterValue, + "Missing 'ilo_deploy_iso'", + ilo_boot._validate_driver_info, task) + mock_parse_driver_info.assert_called_once_with(task.node) + + @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, + autospec=True) + @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, + autospec=True) + def test__validate_driver_info_valid_uuid(self, mock_parse_driver_info, + mock_is_glance_image): + mock_is_glance_image.return_value = True + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + deploy_iso = '8a81759a-f29b-454b-8ab3-161c6ca1882c' + task.node.driver_info['ilo_deploy_iso'] = deploy_iso ilo_boot._validate_driver_info(task) mock_parse_driver_info.assert_called_once_with(task.node) - mock_driver_info.assert_called_once_with(task.node) + mock_is_glance_image.assert_called_once_with(deploy_iso) + + @mock.patch.object(image_service.HttpImageService, 'validate_href', + spec_set=True, autospec=True) + @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, + autospec=True) + @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, + autospec=True) + def test__validate_driver_info_InvalidParam(self, mock_parse_driver_info, + mock_is_glance_image, + mock_validate_href): + deploy_iso = 'http://abc.org/image/qcow2' + mock_validate_href.side_effect = exception.ImageRefValidationFailed( + image_href='http://abc.org/image/qcow2', reason='fail') + mock_is_glance_image.return_value = False + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.driver_info['ilo_deploy_iso'] = deploy_iso + self.assertRaisesRegex(exception.InvalidParameterValue, + "Virtual media boot accepts", + ilo_boot._validate_driver_info, task) + mock_parse_driver_info.assert_called_once_with(task.node) + mock_validate_href.assert_called_once_with(mock.ANY, deploy_iso) + + @mock.patch.object(image_service.HttpImageService, 'validate_href', + spec_set=True, autospec=True) + @mock.patch.object(service_utils, 'is_glance_image', spec_set=True, + autospec=True) + @mock.patch.object(ilo_common, 'parse_driver_info', spec_set=True, + autospec=True) + def test__validate_driver_info_valid_url(self, mock_parse_driver_info, + mock_is_glance_image, + mock_validate_href): + deploy_iso = 'http://abc.org/image/deploy.iso' + mock_is_glance_image.return_value = False + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.driver_info['ilo_deploy_iso'] = deploy_iso + ilo_boot._validate_driver_info(task) + mock_parse_driver_info.assert_called_once_with(task.node) + mock_validate_href.assert_called_once_with(mock.ANY, deploy_iso) @mock.patch.object(deploy_utils, 'validate_image_properties', spec_set=True, autospec=True) @@ -817,13 +1044,10 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): @mock.patch.object(ilo_common, 'cleanup_vmedia_boot', spec_set=True, autospec=True) - @mock.patch.object(ilo_boot, '_clean_up_boot_iso_for_instance', - spec_set=True, autospec=True) - def test_clean_up_ramdisk(self, cleanup_iso_mock, cleanup_vmedia_mock): + def test_clean_up_ramdisk(self, cleanup_vmedia_mock): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.driver.boot.clean_up_ramdisk(task) - cleanup_iso_mock.assert_called_once_with(task.node) cleanup_vmedia_mock.assert_called_once_with(task) @mock.patch.object(deploy_utils, 'is_iscsi_boot', @@ -1002,7 +1226,7 @@ class IloVirtualMediaBootTestCase(test_common.BaseIloTest): def test_validate_rescue_no_rescue_ramdisk(self): with task_manager.acquire(self.context, self.node.uuid) as task: self.assertRaisesRegex(exception.MissingParameterValue, - 'Either.*ilo_rescue_iso*', + 'Missing.*ilo_rescue_iso', task.driver.boot.validate_rescue, task) diff --git a/ironic/tests/unit/drivers/modules/ilo/test_common.py b/ironic/tests/unit/drivers/modules/ilo/test_common.py index 02ba34715a..466f35a0c3 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_common.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_common.py @@ -19,6 +19,7 @@ import builtins import hashlib import io import os +import shutil import tempfile from ironic_lib import utils as ironic_utils @@ -340,7 +341,7 @@ class IloCommonMethodsTestCase(BaseIloTest): 'ilo_cont', object_name, timeout) self.assertEqual('temp-url', temp_url) - @mock.patch.object(deploy_utils, 'copy_image_to_web_server', + @mock.patch.object(ilo_common, 'copy_image_to_web_server', spec_set=True, autospec=True) @mock.patch.object(images, 'create_vfat_image', spec_set=True, autospec=True) @@ -807,6 +808,42 @@ class IloCommonMethodsTestCase(BaseIloTest): task, False) ilo_mock_object.set_secure_boot_mode.assert_called_once_with(False) + @mock.patch.object(os, 'chmod', spec_set=True, + autospec=True) + @mock.patch.object(shutil, 'copyfile', spec_set=True, + autospec=True) + def test_copy_image_to_web_server(self, copy_mock, + chmod_mock): + CONF.deploy.http_url = "http://x.y.z.a/webserver/" + CONF.deploy.http_root = "/webserver" + expected_url = "http://x.y.z.a/webserver/image-UUID" + source = 'tmp_image_file' + destination = "image-UUID" + image_path = "/webserver/image-UUID" + actual_url = ilo_common.copy_image_to_web_server(source, destination) + self.assertEqual(expected_url, actual_url) + copy_mock.assert_called_once_with(source, image_path) + chmod_mock.assert_called_once_with(image_path, 0o644) + + @mock.patch.object(os, 'chmod', spec_set=True, + autospec=True) + @mock.patch.object(shutil, 'copyfile', spec_set=True, + autospec=True) + def test_copy_image_to_web_server_fails(self, copy_mock, + chmod_mock): + CONF.deploy.http_url = "http://x.y.z.a/webserver/" + CONF.deploy.http_root = "/webserver" + source = 'tmp_image_file' + destination = "image-UUID" + image_path = "/webserver/image-UUID" + exc = exception.ImageUploadFailed('reason') + copy_mock.side_effect = exc + self.assertRaises(exception.ImageUploadFailed, + ilo_common.copy_image_to_web_server, + source, destination) + 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 | diff --git a/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py b/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py index 680cacefd7..89aa96f9f4 100644 --- a/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py +++ b/ironic/tests/unit/drivers/modules/ilo/test_firmware_processor.py @@ -22,7 +22,6 @@ import mock from oslo_utils import importutils from ironic.common import exception -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 as ilo_fw_processor from ironic.tests import base @@ -482,12 +481,10 @@ class FirmwareProcessorTestCase(base.TestCase): utils_mock.process_firmware_image.assert_called_once_with( any_target_file, ilo_object_mock) - @mock.patch.object(deploy_utils, 'copy_image_to_web_server', - autospec=True) @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, copy_mock): + self, utils_mock, ilo_common_mock): # | GIVEN | node_mock = mock.MagicMock(uuid='fake_node_uuid') any_target_file = 'any_target_file' @@ -496,7 +493,7 @@ class FirmwareProcessorTestCase(base.TestCase): # | WHEN | ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file) # | THEN | - copy_mock.assert_not_called() + 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) @@ -516,12 +513,10 @@ class FirmwareProcessorTestCase(base.TestCase): # | THEN | _remove_mock.assert_called_once_with(location_obj) - @mock.patch.object(deploy_utils, 'copy_image_to_web_server', - autospec=True) @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, copy_mock): + self, utils_mock, ilo_common_mock): # | GIVEN | node_mock = mock.MagicMock(uuid='fake_node_uuid') any_target_file = 'any_target_file' @@ -531,17 +526,15 @@ class FirmwareProcessorTestCase(base.TestCase): # | WHEN | ilo_fw_processor._extract_fw_from_file(node_mock, any_target_file) # | THEN | - copy_mock.assert_called_once_with( + ilo_common_mock.copy_image_to_web_server.assert_called_once_with( 'some_location/some_fw_file', 'some_fw_file') - @mock.patch.object(deploy_utils, 'copy_image_to_web_server', - autospec=True) @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, copy_mock): + self, _remove_mock, utils_mock, ilo_common_mock): # | GIVEN | node_mock = mock.MagicMock(uuid='fake_node_uuid') any_target_file = 'any_target_file' diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index ec8ac746cf..c1d86486a7 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -27,7 +27,6 @@ from ironic.drivers.modules import boot_mode_utils from ironic.drivers.modules import deploy_utils from ironic.drivers.modules.redfish import boot as redfish_boot from ironic.drivers.modules.redfish import utils as redfish_utils -from ironic.drivers.modules import virtual_media_base from ironic.tests.unit.db import base as db_base from ironic.tests.unit.db import utils as db_utils from ironic.tests.unit.objects import utils as obj_utils @@ -340,10 +339,113 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_unpublish.assert_called_once_with(object_name) - @mock.patch.object(virtual_media_base, 'prepare_iso_image', autospec=True) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_publish_image', autospec=True) + @mock.patch.object(images, 'create_boot_iso', autospec=True) + def test__prepare_iso_image_uefi( + self, mock_create_boot_iso, mock__publish_image): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + task.node.instance_info.update(deploy_boot_mode='uefi') + + expected_url = 'https://a.b/c.f?e=f' + + mock__publish_image.return_value = expected_url + + url = task.driver.boot._prepare_iso_image( + task, 'http://kernel/img', 'http://ramdisk/img', + 'http://bootloader/img', root_uuid=task.node.uuid) + + object_name = 'boot-%s' % task.node.uuid + + mock__publish_image.assert_called_once_with( + mock.ANY, object_name) + + mock_create_boot_iso.assert_called_once_with( + mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', + boot_mode='uefi', esp_image_href='http://bootloader/img', + configdrive_href=mock.ANY, + kernel_params='nofb nomodeset vga=normal', + root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123') + + self.assertEqual(expected_url, url) + + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_publish_image', autospec=True) + @mock.patch.object(images, 'create_boot_iso', autospec=True) + def test__prepare_iso_image_bios( + self, mock_create_boot_iso, mock__publish_image): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + + expected_url = 'https://a.b/c.f?e=f' + + mock__publish_image.return_value = expected_url + + url = task.driver.boot._prepare_iso_image( + task, 'http://kernel/img', 'http://ramdisk/img', + bootloader_href=None, root_uuid=task.node.uuid) + + object_name = 'boot-%s' % task.node.uuid + + mock__publish_image.assert_called_once_with( + mock.ANY, object_name) + + mock_create_boot_iso.assert_called_once_with( + mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', + boot_mode=None, esp_image_href=None, + configdrive_href=mock.ANY, + kernel_params='nofb nomodeset vga=normal', + root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123') + + self.assertEqual(expected_url, url) + + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_publish_image', autospec=True) + @mock.patch.object(images, 'create_boot_iso', autospec=True) + def test__prepare_iso_image_kernel_params( + self, mock_create_boot_iso, mock__publish_image): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + kernel_params = 'network-config=base64-cloudinit-blob' + + task.node.instance_info.update(kernel_append_params=kernel_params) + + task.driver.boot._prepare_iso_image( + task, 'http://kernel/img', 'http://ramdisk/img', + bootloader_href=None, root_uuid=task.node.uuid) + + mock_create_boot_iso.assert_called_once_with( + mock.ANY, mock.ANY, 'http://kernel/img', 'http://ramdisk/img', + boot_mode=None, esp_image_href=None, + configdrive_href=mock.ANY, + kernel_params=kernel_params, + root_uuid='1be26c0b-03f2-4d2e-ae87-c02d7f33c123') + + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_prepare_iso_image', autospec=True) + def test__prepare_deploy_iso(self, mock__prepare_iso_image): + with task_manager.acquire(self.context, self.node.uuid, + shared=True) as task: + + task.node.driver_info.update( + {'deploy_kernel': 'kernel', + 'deploy_ramdisk': 'ramdisk', + 'bootloader': 'bootloader'} + ) + + task.node.instance_info.update(deploy_boot_mode='uefi') + + task.driver.boot._prepare_deploy_iso(task, {}, 'deploy') + + mock__prepare_iso_image.assert_called_once_with( + mock.ANY, 'kernel', 'ramdisk', 'bootloader', params={}) + + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_prepare_iso_image', autospec=True) @mock.patch.object(images, 'create_boot_iso', autospec=True) def test__prepare_boot_iso(self, mock_create_boot_iso, - mock_prepare_iso_image): + mock__prepare_iso_image): with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.driver_info.update( @@ -360,11 +462,9 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): task.driver.boot._prepare_boot_iso( task, root_uuid=task.node.uuid) - mock_prepare_iso_image.assert_called_once_with( + mock__prepare_iso_image.assert_called_once_with( mock.ANY, 'http://kernel/img', 'http://ramdisk/img', - bootloader_href='bootloader', root_uuid=task.node.uuid, - timeout=900, use_web_server=False, - container='ironic_redfish_container') + 'bootloader', root_uuid=task.node.uuid) @mock.patch.object(redfish_utils, 'parse_driver_info', autospec=True) @mock.patch.object(deploy_utils, 'validate_image_properties', @@ -457,8 +557,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device', autospec=True) - @mock.patch.object(virtual_media_base, 'prepare_deploy_iso', - autospec=True) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_prepare_deploy_iso', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_eject_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -471,14 +571,14 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): def test_prepare_ramdisk_with_params( self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock_prepare_deploy_iso, mock_node_set_boot_device): + mock__prepare_deploy_iso, mock_node_set_boot_device): with task_manager.acquire(self.context, self.node.uuid, shared=False) as task: task.node.provision_state = states.DEPLOYING mock__parse_driver_info.return_value = {} - mock_prepare_deploy_iso.return_value = 'image-url' + mock__prepare_deploy_iso.return_value = 'image-url' task.driver.boot.prepare_ramdisk(task, {}) @@ -497,9 +597,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 'ipa-debug': '1', } - mock_prepare_deploy_iso.assert_called_once_with( - task, expected_params, 'deploy', {}, - use_web_server=False, container='ironic_redfish_container') + mock__prepare_deploy_iso.assert_called_once_with( + task, expected_params, 'deploy') mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) @@ -508,8 +607,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): @mock.patch.object(redfish_boot.manager_utils, 'node_set_boot_device', autospec=True) - @mock.patch.object(virtual_media_base, 'prepare_deploy_iso', - autospec=True) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_prepare_deploy_iso', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_eject_vmedia', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -522,14 +621,14 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): def test_prepare_ramdisk_no_debug( self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock_prepare_deploy_iso, mock_node_set_boot_device): + mock__prepare_deploy_iso, mock_node_set_boot_device): self.config(debug=False) with task_manager.acquire(self.context, self.node.uuid, shared=True) as task: task.node.provision_state = states.DEPLOYING mock__parse_driver_info.return_value = {} - mock_prepare_deploy_iso.return_value = 'image-url' + mock__prepare_deploy_iso.return_value = 'image-url' task.driver.boot.prepare_ramdisk(task, {}) @@ -547,9 +646,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 'ipa-agent-token': mock.ANY, } - mock_prepare_deploy_iso.assert_called_once_with( - task, expected_params, 'deploy', {}, - use_web_server=False, container='ironic_redfish_container') + mock__prepare_deploy_iso.assert_called_once_with( + task, expected_params, 'deploy') mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) @@ -560,8 +658,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_prepare_floppy_image', autospec=True) - @mock.patch.object(virtual_media_base, 'prepare_deploy_iso', - autospec=True) + @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, + '_prepare_deploy_iso', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_has_vmedia_device', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, @@ -576,7 +674,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): def test_prepare_ramdisk_with_floppy( self, mock_boot_mode_utils, mock_node_power_action, mock__parse_driver_info, mock__insert_vmedia, mock__eject_vmedia, - mock__has_vmedia_device, mock_prepare_deploy_iso, + mock__has_vmedia_device, mock__prepare_deploy_iso, mock__prepare_floppy_image, mock_node_set_boot_device): with task_manager.acquire(self.context, self.node.uuid, @@ -589,7 +687,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock__has_vmedia_device.return_value = True mock__prepare_floppy_image.return_value = 'floppy-image-url' - mock_prepare_deploy_iso.return_value = 'cd-image-url' + mock__prepare_deploy_iso.return_value = 'cd-image-url' task.driver.boot.prepare_ramdisk(task, {}) @@ -622,11 +720,8 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): 'ipa-agent-token': mock.ANY, } - expected_d_info = {'config_via_floppy': True} - - mock_prepare_deploy_iso.assert_called_once_with( - task, expected_params, 'deploy', expected_d_info, - use_web_server=False, container='ironic_redfish_container') + mock__prepare_deploy_iso.assert_called_once_with( + task, expected_params, 'deploy') mock_node_set_boot_device.assert_called_once_with( task, boot_devices.CDROM, False) diff --git a/ironic/tests/unit/drivers/modules/test_deploy_utils.py b/ironic/tests/unit/drivers/modules/test_deploy_utils.py index 34ee184590..20e578bfb2 100644 --- a/ironic/tests/unit/drivers/modules/test_deploy_utils.py +++ b/ironic/tests/unit/drivers/modules/test_deploy_utils.py @@ -15,7 +15,6 @@ # under the License. import os -import shutil import tempfile import fixtures @@ -1242,38 +1241,6 @@ class AgentMethodsTestCase(db_base.DbTestCase): self.assertTrue( utils.direct_deploy_should_convert_raw_image(self.node)) - @mock.patch.object(shutil, 'copyfile', spec_set=True, - autospec=True) - def test_copy_image_to_web_server(self, copy_mock): - cfg.CONF.deploy.http_url = "http://x.y.z.a/webserver/" - cfg.CONF.deploy.http_root = "/webserver" - expected_url = "http://x.y.z.a/webserver/image-UUID" - source = 'tmp_image_file' - destination = "image-UUID" - image_path = "/webserver/image-UUID" - actual_url = utils.copy_image_to_web_server(source, destination) - self.assertEqual(expected_url, actual_url) - copy_mock.assert_called_once_with(source, image_path) - - @mock.patch.object(os, 'chmod', spec_set=True, - autospec=True) - @mock.patch.object(shutil, 'copyfile', spec_set=True, - autospec=True) - def test_copy_image_to_web_server_fails(self, copy_mock, - chmod_mock): - cfg.CONF.deploy.http_url = "http://x.y.z.a/webserver/" - cfg.CONF.deploy.http_root = "/webserver" - source = 'tmp_image_file' - destination = "image-UUID" - image_path = "/webserver/image-UUID" - exc = exception.ImageUploadFailed('reason') - copy_mock.side_effect = exc - self.assertRaises(exception.ImageUploadFailed, - utils.copy_image_to_web_server, - source, destination) - copy_mock.assert_called_once_with(source, image_path) - self.assertFalse(chmod_mock.called) - class ValidateImagePropertiesTestCase(db_base.DbTestCase): diff --git a/ironic/tests/unit/drivers/modules/test_virtual_media_base.py b/ironic/tests/unit/drivers/modules/test_virtual_media_base.py deleted file mode 100644 index c1385b0470..0000000000 --- a/ironic/tests/unit/drivers/modules/test_virtual_media_base.py +++ /dev/null @@ -1,232 +0,0 @@ -# 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. - -import tempfile - -import mock -from oslo_config import cfg -import six - -from ironic.common import images -from ironic.common import swift -from ironic.conductor import task_manager -from ironic.drivers.modules import deploy_utils -from ironic.drivers.modules import virtual_media_base -from ironic.drivers import utils as driver_utils -from ironic.tests.unit.db import base as db_base -from ironic.tests.unit.db import utils as db_utils -from ironic.tests.unit.objects import utils as object_utils - -if six.PY3: - import io - file = io.BytesIO - -INFO_DICT = db_utils.get_test_redfish_info() - -CONF = cfg.CONF - - -class VirtualMediaCommonMethodsTestCase(db_base.DbTestCase): - - def setUp(self): - super(VirtualMediaCommonMethodsTestCase, self).setUp() - self.config(enabled_hardware_types=['ilo', 'fake-hardware'], - enabled_boot_interfaces=['ilo-pxe', 'ilo-virtual-media', - 'fake'], - enabled_bios_interfaces=['ilo', 'no-bios'], - enabled_power_interfaces=['ilo', 'fake'], - enabled_management_interfaces=['ilo', 'fake'], - enabled_inspect_interfaces=['ilo', 'fake', 'no-inspect'], - enabled_console_interfaces=['ilo', 'fake', 'no-console'], - enabled_vendor_interfaces=['ilo', 'fake', 'no-vendor']) - self.node = object_utils.create_test_node( - self.context, boot_interface='ilo-virtual-media', - deploy_interface='direct') - - def test_get_iso_image_name(self): - boot_iso_actual = virtual_media_base.get_iso_image_name(self.node) - boot_iso_expected = "boot-%s" % self.node.uuid - self.assertEqual(boot_iso_expected, boot_iso_actual) - - @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, - autospec=True) - @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) - @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) - @mock.patch.object(virtual_media_base, 'get_iso_image_name', - spec_set=True, autospec=True) - @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, - autospec=True) - def test__prepare_iso_image_uefi(self, capability_mock, - iso_image_name_mock, swift_api_mock, - create_boot_iso_mock, tempfile_mock): - CONF.ilo.swift_ilo_container = 'ilo-cont' - CONF.ilo.use_web_server_for_images = False - - swift_obj_mock = swift_api_mock.return_value - fileobj_mock = mock.MagicMock(spec=file) - fileobj_mock.name = 'tmpfile' - mock_file_handle = mock.MagicMock(spec=file) - mock_file_handle.__enter__.return_value = fileobj_mock - tempfile_mock.return_value = mock_file_handle - iso_image_name_mock.return_value = 'abcdef' - create_boot_iso_mock.return_value = '/path/to/boot-iso' - capability_mock.return_value = 'uefi' - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - boot_iso_actual = virtual_media_base.prepare_iso_image( - task, 'kernel_uuid', 'ramdisk_uuid', - deploy_iso_href='deploy_iso_uuid', - bootloader_href='bootloader_uuid', - root_uuid='root-uuid', - kernel_params='kernel-params', - timeout=None, - container=CONF.ilo.swift_ilo_container, - use_web_server=CONF.ilo.use_web_server_for_images) - iso_image_name_mock.assert_called_once_with(task.node) - create_boot_iso_mock.assert_called_once_with( - task.context, 'tmpfile', 'kernel_uuid', 'ramdisk_uuid', - deploy_iso_href='deploy_iso_uuid', - esp_image_href='bootloader_uuid', - root_uuid='root-uuid', - kernel_params='kernel-params', - boot_mode='uefi') - swift_obj_mock.create_object.assert_called_once_with( - 'ilo-cont', 'abcdef', 'tmpfile', None) - boot_iso_expected = 'swift:abcdef' - self.assertEqual(boot_iso_expected, boot_iso_actual) - - @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, - autospec=True) - @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) - @mock.patch.object(swift, 'SwiftAPI', spec_set=True, autospec=True) - @mock.patch.object(virtual_media_base, 'get_iso_image_name', - spec_set=True, autospec=True) - @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, - autospec=True) - def test__prepare_iso_image_bios(self, capability_mock, - iso_image_name_mock, swift_api_mock, - create_boot_iso_mock, tempfile_mock): - CONF.ilo.swift_ilo_container = 'ilo-cont' - CONF.ilo.use_web_server_for_images = False - - swift_obj_mock = swift_api_mock.return_value - fileobj_mock = mock.MagicMock(spec=file) - fileobj_mock.name = 'tmpfile' - mock_file_handle = mock.MagicMock(spec=file) - mock_file_handle.__enter__.return_value = fileobj_mock - tempfile_mock.return_value = mock_file_handle - iso_image_name_mock.return_value = 'abcdef' - create_boot_iso_mock.return_value = '/path/to/boot-iso' - capability_mock.return_value = 'bios' - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - boot_iso_actual = virtual_media_base.prepare_iso_image( - task, 'kernel_uuid', 'ramdisk_uuid', - deploy_iso_href='deploy_iso_uuid', - root_uuid='root-uuid', - kernel_params='kernel-params', - timeout=None, - container=CONF.ilo.swift_ilo_container, - use_web_server=CONF.ilo.use_web_server_for_images) - iso_image_name_mock.assert_called_once_with(task.node) - create_boot_iso_mock.assert_called_once_with( - task.context, 'tmpfile', 'kernel_uuid', 'ramdisk_uuid', - deploy_iso_href='deploy_iso_uuid', - esp_image_href=None, - root_uuid='root-uuid', - kernel_params='kernel-params', - boot_mode='bios') - swift_obj_mock.create_object.assert_called_once_with( - 'ilo-cont', 'abcdef', 'tmpfile', None) - boot_iso_expected = 'swift:abcdef' - self.assertEqual(boot_iso_expected, boot_iso_actual) - - @mock.patch.object(deploy_utils, 'copy_image_to_web_server', spec_set=True, - autospec=True) - @mock.patch.object(tempfile, 'NamedTemporaryFile', spec_set=True, - autospec=True) - @mock.patch.object(images, 'create_boot_iso', spec_set=True, autospec=True) - @mock.patch.object(virtual_media_base, 'get_iso_image_name', - spec_set=True, autospec=True) - @mock.patch.object(driver_utils, 'get_node_capability', spec_set=True, - autospec=True) - def test__prepare_iso_image_use_webserver(self, capability_mock, - iso_image_name_mock, - create_boot_iso_mock, - tempfile_mock, copy_file_mock): - CONF.ilo.use_web_server_for_images = True - CONF.deploy.http_url = "http://10.10.1.30/httpboot" - CONF.deploy.http_root = "/httpboot" - CONF.pxe.pxe_append_params = 'kernel-params' - - fileobj_mock = mock.MagicMock(spec=file) - fileobj_mock.name = 'tmpfile' - mock_file_handle = mock.MagicMock(spec=file) - mock_file_handle.__enter__.return_value = fileobj_mock - tempfile_mock.return_value = mock_file_handle - - ramdisk_href = "http://10.10.1.30/httpboot/ramdisk" - kernel_href = "http://10.10.1.30/httpboot/kernel" - iso_image_name_mock.return_value = 'new_boot_iso' - create_boot_iso_mock.return_value = '/path/to/boot-iso' - capability_mock.return_value = 'uefi' - copy_file_mock.return_value = "http://10.10.1.30/httpboot/new_boot_iso" - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - driver_internal_info = task.node.driver_internal_info - driver_internal_info['boot_iso_created_in_web_server'] = True - boot_iso_actual = virtual_media_base.prepare_iso_image( - task, kernel_href, ramdisk_href, - deploy_iso_href='deploy_iso_uuid', - bootloader_href='bootloader_uuid', - root_uuid='root-uuid', - kernel_params='kernel-params', - use_web_server=CONF.ilo.use_web_server_for_images) - iso_image_name_mock.assert_called_once_with(task.node) - create_boot_iso_mock.assert_called_once_with( - task.context, 'tmpfile', kernel_href, ramdisk_href, - deploy_iso_href='deploy_iso_uuid', - esp_image_href='bootloader_uuid', - root_uuid='root-uuid', - kernel_params='kernel-params', - boot_mode='uefi') - boot_iso_expected = 'http://10.10.1.30/httpboot/new_boot_iso' - self.assertEqual(boot_iso_expected, boot_iso_actual) - copy_file_mock.assert_called_once_with(fileobj_mock.name, - 'new_boot_iso') - - @mock.patch.object(virtual_media_base, 'prepare_iso_image', spec_set=True, - autospec=True) - def test_prepare_deploy_iso(self, prepare_iso_mock): - driver_info = {'deploy_kernel': 'kernel', 'deploy_ramdisk': 'ramdisk', - 'bootloader': 'bootloader'} - CONF.pxe.pxe_append_params = 'kernel-params' - timeout = None - container = 'container' - prepare_iso_mock.return_value = ( - 'swift:boot-b5451849-e088-4a4c-aa5f-4d97b3371dec') - - with task_manager.acquire(self.context, self.node.uuid, - shared=False) as task: - deploy_iso_actual = virtual_media_base.prepare_deploy_iso( - task, {}, 'deploy', driver_info, use_web_server=False, - container=container) - prepare_iso_mock.assert_called_once_with( - task, 'kernel', 'ramdisk', bootloader_href='bootloader', - kernel_params=CONF.pxe.pxe_append_params, timeout=timeout, - use_web_server=False, container='container') - deploy_iso_expected = ( - 'swift:boot-b5451849-e088-4a4c-aa5f-4d97b3371dec') - self.assertEqual(deploy_iso_expected, deploy_iso_actual) diff --git a/releasenotes/notes/add-iso-less-vmedia-ilo-e93002e335cb21e1.yaml b/releasenotes/notes/add-iso-less-vmedia-ilo-e93002e335cb21e1.yaml deleted file mode 100644 index 6636afe413..0000000000 --- a/releasenotes/notes/add-iso-less-vmedia-ilo-e93002e335cb21e1.yaml +++ /dev/null @@ -1,8 +0,0 @@ -features: - - | - Add functionality to perform virtual media boot without - user-built deploy/rescue/boot ISO images for hardware type ilo. - Instead, ironic will build necessary images out of common - kernel/ramdisk pair (though user needs to provide ESP image). - User provided deploy/rescue/boot ISO images are - also supported.