From c9cf2347ea61023591909ebd9ebf6ef0327f9cc2 Mon Sep 17 00:00:00 2001 From: Himanshu Roy Date: Mon, 10 Jun 2024 16:38:38 +0530 Subject: [PATCH] add virtual media GET api Closes-Bug: 2072307 Change-Id: I6020a7904639f5b6628bcabb5a861ecc397a8b05 Signed-off-by: Himanshu Roy --- .../source/baremetal-api-v1-get-vmedia.inc | 21 +++++ ironic/api/controllers/v1/node.py | 17 ++++ ironic/api/controllers/v1/utils.py | 5 ++ ironic/api/controllers/v1/versions.py | 4 +- ironic/common/policy.py | 9 ++ ironic/common/release_mappings.py | 4 +- ironic/conductor/manager.py | 27 +++++- ironic/conductor/rpcapi.py | 21 ++++- ironic/drivers/base.py | 10 +++ ironic/drivers/modules/redfish/boot.py | 82 +++++++++++++++++++ ironic/drivers/modules/redfish/management.py | 12 +++ .../unit/api/controllers/v1/test_node.py | 26 +++++- .../modules/redfish/test_management.py | 6 ++ ...ic-virtual-media-get-f09003e5031b9c3d.yaml | 5 ++ 14 files changed, 243 insertions(+), 6 deletions(-) create mode 100644 api-ref/source/baremetal-api-v1-get-vmedia.inc create mode 100644 releasenotes/notes/generic-virtual-media-get-f09003e5031b9c3d.yaml diff --git a/api-ref/source/baremetal-api-v1-get-vmedia.inc b/api-ref/source/baremetal-api-v1-get-vmedia.inc new file mode 100644 index 0000000000..188de1ccab --- /dev/null +++ b/api-ref/source/baremetal-api-v1-get-vmedia.inc @@ -0,0 +1,21 @@ +.. -*- rst -*- + +===================================== +Get Virtual Media (nodes) +===================================== + +.. versionadded:: 1.93 + +Get a list of virtual media devices attached to a node using +the ``v1/nodes/{node_ident}/vmedia`` endpoint. + +Get virtual media devices attached to a node +================================ + +.. rest_method:: GET /v1/nodes/{node_ident}/vmedia + +Get virtual media devices attached to a node. + +Normal response code: 200 + +Error codes: 400,401,403,404,409 diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 446a298035..605d409c81 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -2220,6 +2220,23 @@ class NodeVmediaController(rest.RestController, GetNodeAndTopicMixin): def __init__(self, node_ident): self.node_ident = node_ident + @METRICS.timer('NodeVmediaController.get') + @method.expose(status_code=http_client.OK) + def get(self): + """Get virtual media details for this node + + """ + # NOTE(hroyrh) checking for api version here + # rather than separating the get function into + # a different controller + if not api_utils.allow_get_vmedia(): + pecan.abort(http_client.NOT_FOUND) + rpc_node, topic = self._get_node_and_topic( + 'baremetal:node:vmedia:get') + return api.request.rpcapi.get_virtual_media( + api.request.context, rpc_node.uuid, + topic=topic) + @METRICS.timer('NodeVmediaController.post') @method.expose(status_code=http_client.NO_CONTENT) @method.body('vmedia') diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index b5b8b94ac1..f8846f38de 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -2209,3 +2209,8 @@ def allow_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 + + +def allow_get_vmedia(): + """Check if we should support get virtual media action.""" + return api.request.version.minor >= versions.MINOR_93_GET_VMEDIA diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index af51000e1c..df41028b46 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -130,6 +130,7 @@ BASE_VERSION = 1 # v1.90: Accept ovn vtep switch metadata schema to port.local_link_connection # v1.91: Remove special treatment of .json for API objects # v1.92: Add runbooks API +# v1.93: Add GET API for virtual media MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -224,6 +225,7 @@ MINOR_89_ATTACH_DETACH_VMEDIA = 89 MINOR_90_OVN_VTEP = 90 MINOR_91_DOT_JSON = 91 MINOR_92_RUNBOOKS = 92 +MINOR_93_GET_VMEDIA = 93 # When adding another version, update: # - MINOR_MAX_VERSION @@ -231,7 +233,7 @@ MINOR_92_RUNBOOKS = 92 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_92_RUNBOOKS +MINOR_MAX_VERSION = MINOR_93_GET_VMEDIA # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/policy.py b/ironic/common/policy.py index 3d9a6795ef..f45b23ab8b 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -1088,6 +1088,15 @@ node_policies = [ {'path': '/nodes/{node_ident}/vmedia', 'method': 'DELETE'} ], ), + policy.DocumentedRuleDefault( + name='baremetal:node:vmedia:get', + check_str=SYSTEM_OR_PROJECT_READER, + scope_types=['system', 'project'], + description='Get virtual media device details from a node', + operations=[ + {'path': '/nodes/{node_ident}/vmedia', 'method': 'GET'} + ], + ), ] deprecated_port_reason = """ diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 9abbc9a546..0fcde58dea 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -709,8 +709,8 @@ RELEASE_MAPPING = { # make it below. To release, we will preserve a version matching # the release as a separate block of text, like above. 'master': { - 'api': '1.92', - 'rpc': '1.60', + 'api': '1.93', + 'rpc': '1.61', 'objects': { 'Allocation': ['1.1'], 'BIOSSetting': ['1.1'], diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index b3e67975e3..58654fc2d1 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -97,7 +97,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.60' + RPC_API_VERSION = '1.61' target = messaging.Target(version=RPC_API_VERSION) @@ -3851,6 +3851,31 @@ class ConductorManager(base_manager.BaseConductorManager): action='service', node=node.uuid, state=node.provision_state) + @METRICS.timer('ConductorManager.get_virtual_media') + @messaging.expected_exceptions(exception.InvalidParameterValue, + exception.NodeLocked, + exception.UnsupportedDriverExtension) + def get_virtual_media(self, context, node_id): + """Get all virtual media devices from the node. + + :param context: request context. + :param node_id: node ID or UUID. + :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 get_virtual_media called for node %(node)s ", + {'node': node_id}) + with task_manager.acquire(context, node_id, + purpose='get virtual media devices') as task: + task.driver.management.validate(task) + return task.driver.management.get_virtual_media(task) + @METRICS.timer('ConductorManager.attach_virtual_media') @messaging.expected_exceptions(exception.InvalidParameterValue, exception.NoFreeConductorWorker, diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index a4dc4a6c7b..4943dab92f 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -159,12 +159,13 @@ class ConductorAPI(object): | 1.58 - Added support for json-rpc port usage | 1.59 - Added support for attaching/detaching virtual media | 1.60 - Added continue_node_service + | 1.61 - Added get virtual media support """ # 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.60' + RPC_API_VERSION = '1.61' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -1506,3 +1507,21 @@ class ConductorAPI(object): context, 'detach_virtual_media', node_id=node_id, device_types=device_types) + + def get_virtual_media(self, context, node_id, topic=None): + """Get all virtual media devices from the node. + + :param context: request context. + :param node_id: node ID or UUID. + :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. + + """ + cctxt = self._prepare_call(topic=topic, version='1.61') + return cctxt.call( + context, 'get_virtual_media', + node_id=node_id) diff --git a/ironic/drivers/base.py b/ironic/drivers/base.py index 7b878b286d..bebbbe8253 100644 --- a/ironic/drivers/base.py +++ b/ironic/drivers/base.py @@ -1302,6 +1302,16 @@ class ManagementInterface(BaseInterface): raise exception.UnsupportedDriverExtension( driver=task.node.driver, extension='get_mac_addresses') + def get_virtual_media(self, task): + """Get all virtual media devices from the node. + + :param task: A TaskManager instance containing the node to act on. + :raises: UnsupportedDriverExtension + + """ + raise exception.UnsupportedDriverExtension( + driver=task.node.driver, extension='get_virtual_media') + def attach_virtual_media(self, task, device_type, image_url): """Attach a virtual media device to the node. diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index ba009f6380..de1142b4c7 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -197,6 +197,57 @@ def _has_vmedia_via_manager(manager): return False +def _get_vmedia(task, managers): + """GET virtual media details + + :param task: A task from Task Manager. + :param managers: A list of System managers. + :raises: InvalidParameterValue, if no suitable virtual CD or DVD is + found on the node. + """ + + err_msgs = [] + vmedia_list = [] + system = redfish_utils.get_system(task.node) + if _has_vmedia_via_systems(system): + vmedia_get = _get_vmedia_resources(task, system, err_msgs) + if vmedia_get: + for vmedia in vmedia_get: + media_type_list = [] + for media_type in vmedia.media_types: + media_type_list.append(media_type.value) + + media = { + "media_types": media_type_list, + "inserted": vmedia.inserted, + "image": vmedia.image + } + vmedia_list.append(media) + return vmedia_list + else: + for manager in managers: + vmedia_get = _get_vmedia_resources(task, manager, err_msgs) + if vmedia_get: + for vmedia in vmedia_get: + media_type_list = [] + for media_type in vmedia.media_types: + media_type_list.append(media_type.value) + + media = { + "media_types": media_type_list, + "inserted": vmedia.inserted, + "image": vmedia.image + } + vmedia_list.append(media) + return vmedia_list + if err_msgs: + exc_msg = ("All virtual media GET attempts failed. " + "Most recent error: ", err_msgs[-1]) + else: + exc_msg = 'No suitable virtual media device found' + raise exception.InvalidParameterValue(exc_msg) + + def _insert_vmedia(task, managers, boot_url, boot_device): """Insert bootable ISO image into virtual CD or DVD @@ -340,6 +391,23 @@ def _eject_vmedia(task, managers, boot_device=None): return found +@tenacity.retry(retry=tenacity.retry_if_exception(_test_retry), + stop=tenacity.stop_after_attempt(3), + wait=tenacity.wait_fixed(3), + reraise=True) +def _get_vmedia_resources(task, resource, err_msgs): + """Get virtual media for a given redfish resource (System/Manager) + + :param task: A task from TaskManager. + :param resource: A redfish resource either a System or Manager. + :param err_msgs: A list that will contain all errors found + """ + LOG.info("Get virtual media details for node=%(node)s", + {'node': task.node.uuid}) + + return resource.virtual_media.get_members() + + def _eject_vmedia_from_resource(task, resource, boot_device=None): """Eject virtual media from a given redfish resource (System/Manager) @@ -383,6 +451,20 @@ def _eject_vmedia_from_resource(task, resource, boot_device=None): return found +def get_vmedia(task): + """Get the attached virtual CDs and DVDs for a node + + :param task: A task from TaskManager. + :raises: InvalidParameterValue, if no suitable virtual CD or DVD is + found on the node. + """ + LOG.info('Called redfish.boot.get_vmedia, for ' + 'node=%(node)s', + {'node': task.node.uuid}) + system = redfish_utils.get_system(task.node) + return _get_vmedia(task, system.managers) + + def insert_vmedia(task, image_url, device_type): """Insert virtual CDs and DVDs diff --git a/ironic/drivers/modules/redfish/management.py b/ironic/drivers/modules/redfish/management.py index 682154719f..65931d2789 100644 --- a/ironic/drivers/modules/redfish/management.py +++ b/ironic/drivers/modules/redfish/management.py @@ -1340,6 +1340,18 @@ class RedfishManagement(base.ManagementInterface): LOG.error(msg) raise exception.RedfishError(error=msg) + @task_manager.require_exclusive_lock + def get_virtual_media(self, task): + """Get all virtual media devices from the node. + + :param task: A task from TaskManager. + + """ + LOG.info('Called redfish.management.get_virtual_media,' + 'for the node=%(node)s', + {'node': task.node.uuid}) + return redfish_boot.get_vmedia(task) + @task_manager.require_exclusive_lock def attach_virtual_media(self, task, device_type, image_url): """Attach a virtual media device to the node. diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 553843d114..065892174f 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -8908,9 +8908,33 @@ class TestNodeVmedia(test_api_base.BaseApiTest): def setUp(self): super().setUp() - self.version = "1.89" + self.version = "1.93" self.node = obj_utils.create_test_node(self.context) + @mock.patch.object(rpcapi.ConductorAPI, 'get_virtual_media', + autospec=True) + def test_get(self, mock_get): + mock_vmedia_list = [ + {'media_types': ['CD', 'DVD'], + 'inserted': 'false', + 'image': ''}, + {'media_types': ['Floppy', 'USBStick'], + 'inserted': 'false', + 'image': ''} + ] + mock_get.return_value = mock_vmedia_list + ret = self.get_json('/nodes/%s/vmedia' % self.node.uuid, + headers={api_base.Version.string: self.version}) + self.assertEqual(mock_vmedia_list, ret) + mock_get.assert_called_once_with( + mock.ANY, mock.ANY, self.node.uuid, topic='test-topic') + + def test_get_wrong_version(self): + ret = self.get_json('/nodes/%s/vmedia' % self.node.uuid, + headers={api_base.Version.string: "1.92"}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_int) + @mock.patch.object(rpcapi.ConductorAPI, 'attach_virtual_media', autospec=True) def test_attach(self, mock_attach): diff --git a/ironic/tests/unit/drivers/modules/redfish/test_management.py b/ironic/tests/unit/drivers/modules/redfish/test_management.py index 043f117641..12b400f0fb 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_management.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_management.py @@ -1866,6 +1866,12 @@ class RedfishManagementTestCase(db_base.DbTestCase): shared=True) as task: self.assertIsNone(task.driver.management.get_mac_addresses(task)) + @mock.patch.object(redfish_boot, 'get_vmedia', autospec=True) + def test_get_virtual_media(self, mock_get_vmedia): + with task_manager.acquire(self.context, self.node.uuid) as task: + task.driver.management.get_virtual_media(task) + mock_get_vmedia.assert_called_once_with(task) + @mock.patch.object(redfish_boot, 'insert_vmedia', autospec=True) def test_attach_virtual_media(self, mock_insert_vmedia): with task_manager.acquire(self.context, self.node.uuid) as task: diff --git a/releasenotes/notes/generic-virtual-media-get-f09003e5031b9c3d.yaml b/releasenotes/notes/generic-virtual-media-get-f09003e5031b9c3d.yaml new file mode 100644 index 0000000000..3464a6780e --- /dev/null +++ b/releasenotes/notes/generic-virtual-media-get-f09003e5031b9c3d.yaml @@ -0,0 +1,5 @@ +--- +features: + Adds a new capability allowing to fetch the list + of virtual media devices attached to a node by + making a GET request. \ No newline at end of file