From 090291221796c5631f367de093056cd2acc7841d Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Wed, 13 Sep 2023 14:21:44 +0200 Subject: [PATCH] Generic API for attaching/detaching virtual media This patch allows to attach or detach a generic image as virtual media device after a node has been provisioned. Closes-Bug: #2033288 Change-Id: I97b68047d769f6fb686c53e89084b5874e02b8c7 --- .../baremetal-api-v1-attach-detach-vmedia.inc | 54 ++++++++ api-ref/source/parameters.yaml | 20 ++- .../samples/node-vmedia-attach-request.json | 4 + ironic/api/controllers/v1/node.py | 107 +++++++++++++-- ironic/api/controllers/v1/utils.py | 5 + ironic/api/controllers/v1/versions.py | 4 +- ironic/common/boot_devices.py | 3 + ironic/common/policy.py | 18 +++ ironic/common/release_mappings.py | 4 +- ironic/conductor/manager.py | 116 +++++++++++++++- ironic/conductor/rpcapi.py | 55 +++++++- ironic/conductor/utils.py | 25 ++++ ironic/drivers/base.py | 26 ++++ ironic/drivers/modules/redfish/vendor.py | 2 + .../unit/api/controllers/v1/test_node.py | 125 ++++++++++++++++++ ironic/tests/unit/conductor/test_manager.py | 56 ++++++++ ...virtual-media-attach-9625f8ac66093b76.yaml | 5 + 17 files changed, 612 insertions(+), 17 deletions(-) create mode 100644 api-ref/source/baremetal-api-v1-attach-detach-vmedia.inc create mode 100644 api-ref/source/samples/node-vmedia-attach-request.json create mode 100644 releasenotes/notes/generic-virtual-media-attach-9625f8ac66093b76.yaml diff --git a/api-ref/source/baremetal-api-v1-attach-detach-vmedia.inc b/api-ref/source/baremetal-api-v1-attach-detach-vmedia.inc new file mode 100644 index 0000000000..b739c79057 --- /dev/null +++ b/api-ref/source/baremetal-api-v1-attach-detach-vmedia.inc @@ -0,0 +1,54 @@ +.. -*- rst -*- + +===================================== +Attach / Detach Virtual Media (nodes) +===================================== + +.. versionadded:: 1.89 + +Attach a generic image as virtual media device to a node or remove +it from a node using the ``v1/nodes/{node_ident}/vmedia`` endpoint. + +Attach a virtual media to a node +================================ + +.. rest_method:: POST /v1/nodes/{node_ident}/vmedia + +Attach virtual media device to a node. + +Normal response code: 204 + +Error codes: 400,401,403,404,409 + +Request +------- + +.. rest_parameters:: parameters.yaml + + - node_ident: node_ident + - device_type: vmedia_device_type + - image_url: vmedia_image_url + - image_download_source: vmedia_image_download_source + +**Example request to attach virtual media to a Node:** + +.. literalinclude:: samples/node-vmedia-attach-request.json + + +Detach virtual media from a node +================================ + +.. rest_method:: DELETE /v1/nodes/{node_ident}/vmedia + +Detach virtual media device from a Node. + +Normal response code: 204 + +Error codes: 400,401,403,404 + +Request +------- + +.. rest_parameters:: parameters.yaml + + - node_ident: node_ident diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 3109b8d0ff..8de5dc1fda 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -2177,7 +2177,25 @@ versions: in: body required: true type: array - +vmedia_device_type: + description: | + The type of the virtual media device used, e.g. CDROM + in: body + required: true + type: string +vmedia_image_download_source: + description: | + How the image is served to the BMC, "http" for a remote location or + "local" to use the local web server. + in: body + required: false + type: string +vmedia_image_url: + description: | + The url of the image to attach to a virtual media device. + in: body + required: true + type: string # variables returned from volume-connector volume_connector_connector_id: description: | diff --git a/api-ref/source/samples/node-vmedia-attach-request.json b/api-ref/source/samples/node-vmedia-attach-request.json new file mode 100644 index 0000000000..9479dde76a --- /dev/null +++ b/api-ref/source/samples/node-vmedia-attach-request.json @@ -0,0 +1,4 @@ +{ + "device_type": "CDROM", + "image_url": "http://image" +} diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index dcd332fc51..ece70cd164 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -17,6 +17,7 @@ import copy import datetime from http import client as http_client import json +import urllib.parse from ironic_lib import metrics_utils import jsonschema @@ -41,6 +42,7 @@ from ironic.api.controllers.v1 import versions from ironic.api.controllers.v1 import volume from ironic.api import method from ironic.common import args +from ironic.common import boot_devices from ironic.common import boot_modes from ironic.common import exception from ironic.common.i18n import _ @@ -316,6 +318,28 @@ VIF_VALIDATOR = args.and_valid( args.dict_valid(id=args.uuid_or_name) ) +VMEDIA_ATTACH_VALIDATOR = args.schema({ + 'type': 'object', + 'properties': { + 'device_type': { + 'type': 'string', + 'enum': boot_devices.VMEDIA_DEVICES, + }, + 'image_url': {'type': 'string'}, + 'image_download_source': { + 'type': 'string', + 'enum': ['http', 'local', 'swift'], + }, + # TODO(dtantsur): these are useful additions in the future, but the + # ISO image code does not support them. + # 'username': {'type': 'string'}, + # 'password': {'type': 'string'}, + # 'insecure': {'type': 'boolean'}, + }, + 'required': ['device_type', 'image_url'], + 'additionalProperties': False, +}) + def get_nodes_controller_reserved_names(): global _NODES_CONTROLLER_RESERVED_WORDS @@ -400,6 +424,18 @@ def validate_network_data(network_data): raise exception.Invalid(msg) +class GetNodeAndTopicMixin: + + def _get_node_and_topic(self, policy_name): + rpc_node = api_utils.check_node_policy_and_retrieve( + policy_name, self.node_ident) + try: + return rpc_node, api.request.rpcapi.get_topic_for(rpc_node) + except exception.NoValidHost as e: + e.code = http_client.BAD_REQUEST + raise + + class BootDeviceController(rest.RestController): _custom_actions = { @@ -1904,20 +1940,11 @@ class NodeMaintenanceController(rest.RestController): self._set_maintenance(rpc_node, False) -class NodeVIFController(rest.RestController): +class NodeVIFController(rest.RestController, GetNodeAndTopicMixin): def __init__(self, node_ident): self.node_ident = node_ident - def _get_node_and_topic(self, policy_name): - rpc_node = api_utils.check_node_policy_and_retrieve( - policy_name, self.node_ident) - try: - return rpc_node, api.request.rpcapi.get_topic_for(rpc_node) - except exception.NoValidHost as e: - e.code = http_client.BAD_REQUEST - raise - @METRICS.timer('NodeVIFController.get_all') @method.expose() def get_all(self): @@ -2119,6 +2146,61 @@ class NodeChildrenController(rest.RestController): '?parent_node={}'.format(rpc_node.uuid))} +class NodeVmediaController(rest.RestController, GetNodeAndTopicMixin): + + def __init__(self, node_ident): + self.node_ident = node_ident + + @METRICS.timer('NodeVmediaController.post') + @method.expose(status_code=http_client.NO_CONTENT) + @method.body('vmedia') + @args.validate(vmedia=VMEDIA_ATTACH_VALIDATOR) + def post(self, vmedia): + """Attach a virtual media to this node + + :param vmedia: a dictionary of information about the attachment. + """ + parsed_url = urllib.parse.urlparse(vmedia['image_url']) + # NOTE(dtantsur): we may eventually support glance images, but for now + # let us reject everything that is not http/https. + if parsed_url.scheme not in ('http', 'https'): + raise exception.Invalid(_("Unsupported or missing URL scheme: %s") + % parsed_url.scheme) + + rpc_node, topic = self._get_node_and_topic( + 'baremetal:node:vmedia:attach') + api.request.rpcapi.attach_virtual_media( + api.request.context, rpc_node.uuid, + device_type=vmedia['device_type'], + image_url=vmedia['image_url'], + image_download_source=vmedia.get('image_download_source', 'local'), + topic=topic) + + @METRICS.timer('NodeVmediaController.delete') + @method.expose(status_code=http_client.NO_CONTENT) + @args.validate(device_types=args.string_list) + def delete(self, device_types=None): + """Detach a virtual media from this node + + :param device_types: A collection of device types. + """ + if device_types: + invalid = [item for item in device_types + if item not in boot_devices.VMEDIA_DEVICES] + if invalid: + raise exception.Invalid( + _("Invalid device type(s) %(invalid)s " + "(valid are %(valid)s)") + % {'invalid': ', '.join(invalid), + 'valid': ', '.join(boot_devices.VMEDIA_DEVICES)}) + + rpc_node, topic = self._get_node_and_topic( + 'baremetal:node:vmedia:detach') + api.request.rpcapi.detach_virtual_media( + api.request.context, rpc_node.uuid, + device_types=device_types, topic=topic) + + class NodesController(rest.RestController): """REST controller for Nodes.""" @@ -2172,6 +2254,7 @@ class NodesController(rest.RestController): 'inventory': NodeInventoryController, 'children': NodeChildrenController, 'firmware': firmware.NodeFirmwareController, + 'vmedia': NodeVmediaController, } @pecan.expose() @@ -2199,7 +2282,9 @@ class NodesController(rest.RestController): or (remainder[0] == 'inventory' and not api_utils.allow_node_inventory()) or (remainder[0] == 'firmware' - and not api_utils.allow_firmware_interface())): + and not api_utils.allow_firmware_interface()) + or (remainder[0] == 'vmedia' + and not api_utils.allow_attach_detach_vmedia())): pecan.abort(http_client.NOT_FOUND) if remainder[0] == 'traits' and not api_utils.allow_traits(): # NOTE(mgoddard): Returning here will ensure we exhibit the diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 16ba0df7c4..b31988ee8b 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -2030,3 +2030,8 @@ def allow_port_name(): Version 1.88 of the API added name field to the port object. """ return api.request.version.minor >= versions.MINOR_88_PORT_NAME + + +def allow_attach_detach_vmedia(): + """Check if we should support virtual media actions.""" + return api.request.version.minor >= versions.MINOR_89_ATTACH_DETACH_VMEDIA diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 8fac078924..3ba160154e 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -126,6 +126,7 @@ BASE_VERSION = 1 # v1.86: Add firmware interface # v1.87: Add service verb # v1.88: Add name field to port. +# v1.89: Add API for attaching/detaching virtual media MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -216,6 +217,7 @@ MINOR_85_UNHOLD_VERB = 85 MINOR_86_FIRMWARE_INTERFACE = 86 MINOR_87_SERVICE = 87 MINOR_88_PORT_NAME = 88 +MINOR_89_ATTACH_DETACH_VMEDIA = 89 # When adding another version, update: # - MINOR_MAX_VERSION @@ -223,7 +225,7 @@ MINOR_88_PORT_NAME = 88 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_88_PORT_NAME +MINOR_MAX_VERSION = MINOR_89_ATTACH_DETACH_VMEDIA # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/boot_devices.py b/ironic/common/boot_devices.py index de5512bda5..1ec4ab0554 100644 --- a/ironic/common/boot_devices.py +++ b/ironic/common/boot_devices.py @@ -49,3 +49,6 @@ ISCSIBOOT = 'iscsiboot' FLOPPY = 'floppy' "Boot from a floppy drive" + +VMEDIA_DEVICES = [DISK, CDROM, FLOPPY] +"""Devices that make sense for virtual media attachment.""" diff --git a/ironic/common/policy.py b/ironic/common/policy.py index 8343afef62..0a33b299e9 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -1018,6 +1018,24 @@ node_policies = [ {'path': '/nodes/{node_ident}/firmware', 'method': 'GET'} ], ), + policy.DocumentedRuleDefault( + name='baremetal:node:vmedia:attach', + check_str=SYSTEM_OR_PROJECT_MEMBER, + scope_types=['system', 'project'], + description='Attach a virtual media device to a node', + operations=[ + {'path': '/nodes/{node_ident}/vmedia', 'method': 'POST'}\ + ], + ), + policy.DocumentedRuleDefault( + name='baremetal:node:vmedia:detach', + check_str=SYSTEM_OR_PROJECT_MEMBER, + scope_types=['system', 'project'], + description='Detach a virtual media device from a node', + operations=[ + {'path': '/nodes/{node_ident}/vmedia', 'method': 'DELETE'} + ], + ), ] deprecated_port_reason = """ diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 4a9bf257dd..6946881416 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -617,8 +617,8 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.88', - 'rpc': '1.58', + 'api': '1.89', + 'rpc': '1.59', 'objects': { 'Allocation': ['1.1'], 'BIOSSetting': ['1.1'], diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 10deaf5d90..5efa3f66af 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -52,6 +52,7 @@ import oslo_messaging as messaging from oslo_utils import excutils from oslo_utils import uuidutils +from ironic.common import boot_devices from ironic.common import driver_factory from ironic.common import exception from ironic.common import faults @@ -75,6 +76,7 @@ from ironic.conf import CONF from ironic.drivers import base as drivers_base from ironic.drivers.modules import deploy_utils from ironic.drivers.modules import image_cache +from ironic.drivers.modules import image_utils from ironic.drivers.modules import inspect_utils from ironic import objects from ironic.objects import base as objects_base @@ -94,7 +96,7 @@ class ConductorManager(base_manager.BaseConductorManager): # NOTE(rloo): This must be in sync with rpcapi.ConductorAPI's. # NOTE(pas-ha): This also must be in sync with # ironic.common.release_mappings.RELEASE_MAPPING['master'] - RPC_API_VERSION = '1.58' + RPC_API_VERSION = '1.59' target = messaging.Target(version=RPC_API_VERSION) @@ -3785,6 +3787,86 @@ class ConductorManager(base_manager.BaseConductorManager): action='service', node=node.uuid, state=node.provision_state) + @METRICS.timer('ConductorManager.attach_virtual_media') + @messaging.expected_exceptions(exception.InvalidParameterValue, + exception.NoFreeConductorWorker, + exception.NodeLocked, + exception.UnsupportedDriverExtension) + def attach_virtual_media(self, context, node_id, device_type, image_url, + image_download_source='local'): + """Attach a virtual media device to the node. + + :param context: request context. + :param node_id: node ID or UUID. + :param image_url: URL of the image to attach, HTTP or HTTPS. + :param image_download_source: Which way to serve the image to the BMC: + "http" to serve it from the provided location, "local" to serve + it from the local web server. + :raises: UnsupportedDriverExtension if the driver does not support + this call. + :raises: InvalidParameterValue if validation of management driver + interface failed. + :raises: NodeLocked if node is locked by another conductor. + :raises: NoFreeConductorWorker when there is no free worker to start + async task. + + """ + LOG.debug("RPC attach_virtual_media called for node %(node)s " + "for device type %(type)s", + {'node': node_id, 'type': device_type}) + with task_manager.acquire(context, node_id, shared=False, + purpose='attaching virtual media') as task: + task.driver.management.validate(task) + # Starting new operation, so clear the previous error. + # We'll be putting an error here soon if we fail task. + task.node.last_error = None + task.node.save() + task.set_spawn_error_hook(utils._spawn_error_handler, + task.node, "attaching virtual media") + task.spawn_after(self._spawn_worker, + do_attach_virtual_media, task, + device_type=device_type, + image_url=image_url, + image_download_source=image_download_source) + + def detach_virtual_media(self, context, node_id, device_types=None): + """Detach some or all virtual media devices from the node. + + :param context: request context. + :param node_id: node ID or UUID. + :param device_types: A collection of device type, ones from + :data:`ironic.common.boot_devices.VMEDIA_DEVICES`. + If not provided, all devices are detached. + :raises: UnsupportedDriverExtension if the driver does not support + this call. + :raises: InvalidParameterValue if validation of management driver + interface failed. + :raises: NodeLocked if node is locked by another conductor. + :raises: NoFreeConductorWorker when there is no free worker to start + async task. + + """ + LOG.debug("RPC detach_virtual_media called for node %(node)s " + "for device types %(type)s", + {'node': node_id, 'type': device_types}) + with task_manager.acquire(context, node_id, shared=False, + purpose='detaching virtual media') as task: + task.driver.management.validate(task) + # Starting new operation, so clear the previous error. + # We'll be putting an error here soon if we fail task. + task.node.last_error = None + task.node.save() + task.set_spawn_error_hook(utils._spawn_error_handler, + task.node, "detaching virtual media") + task.spawn_after(self._spawn_worker, + utils.run_node_action, + task, task.driver.management.detach_virtual_media, + success_msg="Device(s) %(device_types)s detached " + "from node %(node)s", + error_msg="Could not detach device(s) " + "%(device_types)s from node %(node)s: %(exc)s", + device_types=device_types) + # NOTE(TheJulia): This is the end of the class definition for the # conductor manager. Methods for RPC and stuffs should go above this @@ -3971,3 +4053,35 @@ def do_sync_power_state(task, count): task, old_power_state) return count + + +def do_attach_virtual_media(task, device_type, image_url, + image_download_source): + assert device_type in boot_devices.VMEDIA_DEVICES + file_name = "%s.%s" % ( + device_type.lower(), + 'iso' if device_type == boot_devices.CDROM else 'img' + ) + image_url = image_utils.prepare_remote_image( + task, image_url, file_name=file_name, + download_source=image_download_source) + utils.run_node_action( + task, task.driver.management.attach_virtual_media, + success_msg="Device %(device_type)s attached to node %(node)s", + error_msg="Could not attach device %(device_type)s " + "to node %(node)s: %(exc)s", + device_type=device_type, + image_url=image_url) + + +def do_detach_virtual_media(task, device_types): + utils.run_node_action(task, task.driver.management.detach_virtual_media, + success_msg="Device(s) %(device_types)s detached " + "from node %(node)s", + error_msg="Could not detach device(s) " + "%(device_types)s from node %(node)s: %(exc)s", + device_types=device_types) + for device_type in device_types: + suffix = '.iso' if device_type == boot_devices.CDROM else '.img' + image_utils.ImageHandler.unpublish_image_for_node( + task.node, prefix=device_type.lower(), suffix=suffix) diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index f99a6a0633..cb11a7f43f 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -157,12 +157,13 @@ class ConductorAPI(object): | 1.56 - Added continue_inspection | 1.57 - Added do_node_service | 1.58 - Added support for json-rpc port usage + | 1.59 - Added support for attaching/detaching virtual media """ # NOTE(rloo): This must be in sync with manager.ConductorManager's. # NOTE(pas-ha): This also must be in sync with # ironic.common.release_mappings.RELEASE_MAPPING['master'] - RPC_API_VERSION = '1.58' + RPC_API_VERSION = '1.59' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -1437,3 +1438,55 @@ class ConductorAPI(object): node_id=node_id, service_steps=service_steps, disable_ramdisk=disable_ramdisk) + + def attach_virtual_media(self, context, node_id, device_type, image_url, + image_download_source=None, topic=None): + """Attach a virtual media device to the node. + + :param context: request context. + :param node_id: node ID or UUID. + :param image_url: URL of the image to attach, HTTP or HTTPS. + :param image_download_source: Which way to serve the image to the BMC: + "http" to serve it from the provided location, "local" to serve + it from the local web server. + :param topic: RPC topic. Defaults to self.topic. + :raises: UnsupportedDriverExtension if the driver does not support + this call. + :raises: InvalidParameterValue if validation of management driver + interface failed. + :raises: NodeLocked if node is locked by another conductor. + :raises: NoFreeConductorWorker when there is no free worker to start + async task. + + """ + cctxt = self._prepare_call(topic=topic, version='1.59') + return cctxt.call( + context, 'attach_virtual_media', + node_id=node_id, + device_type=device_type, + image_url=image_url) + + def detach_virtual_media(self, context, node_id, device_types=None, + topic=None): + """Detach some or all virtual media devices from the node. + + :param context: request context. + :param node_id: node ID or UUID. + :param device_types: A collection of device type, ones from + :data:`ironic.common.boot_devices.VMEDIA_DEVICES`. + If not provided, all devices are detached. + :param topic: RPC topic. Defaults to self.topic. + :raises: UnsupportedDriverExtension if the driver does not support + this call. + :raises: InvalidParameterValue if validation of management driver + interface failed. + :raises: NodeLocked if node is locked by another conductor. + :raises: NoFreeConductorWorker when there is no free worker to start + async task. + + """ + cctxt = self._prepare_call(topic=topic, version='1.59') + return cctxt.call( + context, 'detach_virtual_media', + node_id=node_id, + device_types=device_types) diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index e79ce3d209..cb30fdf549 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -1840,3 +1840,28 @@ def node_cache_firmware_components(task): except exception.UnsupportedDriverExtension: LOG.warning('Firmware Components are not supported for node %s, ' 'skipping', task.node.uuid) + + +def run_node_action(task, call, error_msg, success_msg=None, **kwargs): + """Run a node action and report any errors via last_error. + + :param task: A TaskManager instance containing the node to act on. + :param call: A callable object to invoke. + :param error_msg: A template for a failure message. Can use %(node)s, + %(exc)s and any variables from kwargs. + :param success_msg: A template for a success message. Can use %(node)s + and any variables from kwargs. + :param kwargs: Arguments to pass to the call. + """ + error = None + try: + call(task, **kwargs) + except Exception as exc: + error = error_msg % dict(kwargs, node=task.node.uuid, exc=exc) + node_history_record(task.node, event=error, error=True) + LOG.error( + error, exc_info=not isinstance(exc, exception.IronicException)) + + task.node.save() + if not error and success_msg: + LOG.info(success_msg, dict(kwargs, node=task.node.uuid)) diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index f41ff74c5c..3dae0b3500 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -1302,6 +1302,32 @@ class ManagementInterface(BaseInterface): raise exception.UnsupportedDriverExtension( driver=task.node.driver, extension='get_mac_addresses') + def attach_virtual_media(self, task, device_type, image_url): + """Attach a virtual media device to the node. + + :param task: A TaskManager instance containing the node to act on. + :param device_type: Device type, one of + :data:`ironic.common.boot_devices.VMEDIA_DEVICES`. + :param image_url: URL of the image to attach, HTTP or HTTPS. + :raises: UnsupportedDriverExtension + + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='attach_virtual_media') + + def detach_virtual_media(self, task, device_types=None): + """Detach some or all virtual media devices from the node. + + :param task: A TaskManager instance containing the node to act on. + :param device_types: A collection of device type, ones from + :data:`ironic.common.boot_devices.VMEDIA_DEVICES`. + If not provided, all devices are detached. + :raises: UnsupportedDriverExtension + + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='detach_virtual_media') + class InspectInterface(BaseInterface): """Interface for inspection-related actions.""" diff --git a/ironic/drivers/modules/redfish/vendor.py b/ironic/drivers/modules/redfish/vendor.py index 97f1ce4d16..78c2591a57 100644 --- a/ironic/drivers/modules/redfish/vendor.py +++ b/ironic/drivers/modules/redfish/vendor.py @@ -96,6 +96,8 @@ class RedfishVendorPassthru(base.VendorInterface): def eject_vmedia(self, task, **kwargs): """Eject a virtual media device. + Deprecated in favour of the generic API. + :param task: A TaskManager object. :param kwargs: The arguments sent with vendor passthru. The optional kwargs are:: diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index c05d7c5dd8..fc97d97ade 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -8625,3 +8625,128 @@ class TestNodeFirmwareComponent(test_api_base.BaseApiTest): expect_errors=True) self.assertEqual(http_client.NOT_FOUND, ret.status_int) + + +@mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for', + lambda *_: 'test-topic') +class TestNodeVmedia(test_api_base.BaseApiTest): + + def setUp(self): + super().setUp() + self.version = "1.89" + self.node = obj_utils.create_test_node(self.context) + + @mock.patch.object(rpcapi.ConductorAPI, 'attach_virtual_media', + autospec=True) + def test_attach(self, mock_attach): + vmedia = {'device_type': boot_devices.CDROM, + 'image_url': 'https://image', + 'image_download_source': 'http'} + ret = self.post_json('/nodes/%s/vmedia' % self.node.uuid, vmedia, + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_int) + mock_attach.assert_called_once_with( + mock.ANY, mock.ANY, self.node.uuid, + device_type=boot_devices.CDROM, image_url='https://image', + image_download_source='http', topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'attach_virtual_media', + autospec=True) + def test_attach_required_only(self, mock_attach): + vmedia = {'device_type': boot_devices.CDROM, + 'image_url': 'http://image'} + ret = self.post_json('/nodes/%s/vmedia' % self.node.uuid, vmedia, + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_int) + mock_attach.assert_called_once_with( + mock.ANY, mock.ANY, self.node.uuid, + device_type=boot_devices.CDROM, image_url='http://image', + image_download_source='local', topic='test-topic') + + def test_attach_missing_device_type(self): + vmedia = {'image_url': 'http://image'} + ret = self.post_json('/nodes/%s/vmedia' % self.node.uuid, vmedia, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_int) + self.assertIn(b"device_type", ret.body) + + def test_attach_invalid_device_type(self): + vmedia = {'device_type': 'cat', + 'image_url': 'http://image'} + ret = self.post_json('/nodes/%s/vmedia' % self.node.uuid, vmedia, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_int) + self.assertIn(b"cat", ret.body) + + def test_attach_missing_image_url(self): + vmedia = {'device_type': boot_devices.CDROM} + ret = self.post_json('/nodes/%s/vmedia' % self.node.uuid, vmedia, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_int) + self.assertIn(b"image_url", ret.body) + + def test_attach_invalid_image_url(self): + vmedia = {'device_type': boot_devices.CDROM, + 'image_url': 'abcd'} + ret = self.post_json('/nodes/%s/vmedia' % self.node.uuid, vmedia, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_int) + self.assertIn(b"URL scheme", ret.body) + + def test_attach_wrong_version(self): + vmedia = {'device_type': boot_devices.CDROM, + 'image_url': 'http://image'} + ret = self.post_json('/nodes/%s/vmedia' % self.node.uuid, vmedia, + headers={api_base.Version.string: "1.87"}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_int) + + @mock.patch.object(rpcapi.ConductorAPI, 'detach_virtual_media', + autospec=True) + def test_detach_everything(self, mock_detach): + ret = self.delete('/nodes/%s/vmedia' % self.node.uuid, + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_int) + mock_detach.assert_called_once_with( + mock.ANY, mock.ANY, self.node.uuid, + device_types=None, topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'detach_virtual_media', + autospec=True) + def test_detach_specific_via_url(self, mock_detach): + ret = self.delete('/nodes/%s/vmedia/%s' + % (self.node.uuid, boot_devices.CDROM), + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_int) + mock_detach.assert_called_once_with( + mock.ANY, mock.ANY, self.node.uuid, + device_types=[boot_devices.CDROM], topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'detach_virtual_media', + autospec=True) + def test_detach_specific_via_argument(self, mock_detach): + ret = self.delete('/nodes/%s/vmedia?device_types=%s' + % (self.node.uuid, boot_devices.CDROM), + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_int) + mock_detach.assert_called_once_with( + mock.ANY, mock.ANY, self.node.uuid, + device_types=[boot_devices.CDROM], topic='test-topic') + + def test_detach_wrong_device_types(self): + ret = self.delete('/nodes/%s/vmedia?device_types=cdrom,cat' + % self.node.uuid, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_int) + self.assertIn(b"cat", ret.body) + + def test_detach_wrong_version(self): + ret = self.delete('/nodes/%s/vmedia' % self.node.uuid, + headers={api_base.Version.string: "1.87"}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_int) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 438b1946e6..c08137a4ab 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -57,8 +57,10 @@ from ironic.conductor import verify from ironic.db import api as dbapi from ironic.drivers import base as drivers_base from ironic.drivers.modules import fake +from ironic.drivers.modules import image_utils from ironic.drivers.modules import inspect_utils from ironic.drivers.modules.network import flat as n_flat +from ironic.drivers.modules import redfish from ironic import objects from ironic.objects import base as obj_base from ironic.objects import fields as obj_fields @@ -69,6 +71,8 @@ from ironic.tests.unit.objects import utils as obj_utils CONF = cfg.CONF +INFO_DICT = db_utils.get_test_redfish_info() + @mgr_utils.mock_record_keepalive class ChangeNodePowerStateTestCase(mgr_utils.ServiceSetUpMixin, @@ -8658,3 +8662,55 @@ class DoNodeServiceTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): call_args=(servicing.do_node_service, mock.ANY, {'foo': 'bar'}, False), err_handler=mock.ANY, target_state='active') + + +@mock.patch.object( + task_manager.TaskManager, 'spawn_after', + lambda self, _spawn, func, *args, **kwargs: func(*args, **kwargs)) +@mgr_utils.mock_record_keepalive +class VirtualMediaTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): + + def setUp(self): + super().setUp() + self.config(enabled_hardware_types=['redfish'], + enabled_power_interfaces=['redfish'], + enabled_boot_interfaces=['redfish-virtual-media'], + enabled_management_interfaces=['redfish'], + enabled_inspect_interfaces=['redfish'], + enabled_bios_interfaces=['redfish']) + self.node = obj_utils.create_test_node( + self.context, driver='redfish', driver_info=INFO_DICT, + provision_state=states.ACTIVE) + + @mock.patch.object(image_utils, 'ISOImageCache', autospec=True) + @mock.patch.object(redfish.management.RedfishManagement, 'validate', + autospec=True) + @mock.patch.object(manager, 'do_attach_virtual_media', + autospec=True) + def test_attach_virtual_media_local(self, mock_attach, mock_validate, + mock_cache): + CONF.set_override('use_swift', 'false', group='redfish') + self.service.attach_virtual_media(self.context, self.node.id, + boot_devices.CDROM, + 'https://url') + mock_validate.assert_called_once_with(mock.ANY, mock.ANY) + mock_attach.assert_called_once_with( + mock.ANY, device_type=boot_devices.CDROM, + image_url='https://url', image_download_source='local') + self.node.refresh() + self.assertIsNone(self.node.last_error) + + @mock.patch.object(redfish.management.RedfishManagement, 'validate', + autospec=True) + @mock.patch.object(manager, 'do_attach_virtual_media', autospec=True) + def test_attach_virtual_media_http(self, mock_attach, mock_validate): + self.service.attach_virtual_media(self.context, self.node.id, + boot_devices.CDROM, + 'https://url', + image_download_source='http') + mock_validate.assert_called_once_with(mock.ANY, mock.ANY) + mock_attach.assert_called_once_with( + mock.ANY, device_type=boot_devices.CDROM, + image_url='https://url', image_download_source='http') + self.node.refresh() + self.assertIsNone(self.node.last_error) diff --git a/releasenotes/notes/generic-virtual-media-attach-9625f8ac66093b76.yaml b/releasenotes/notes/generic-virtual-media-attach-9625f8ac66093b76.yaml new file mode 100644 index 0000000000..3e6ea09a56 --- /dev/null +++ b/releasenotes/notes/generic-virtual-media-attach-9625f8ac66093b76.yaml @@ -0,0 +1,5 @@ +--- +features: + Adds a new capability allowing to attach or detach + generic iso images as virtual media devices after + a node has been provisioned.