From bc95c92f7c122b1217459a1d7a125fae47749e6e Mon Sep 17 00:00:00 2001 From: Cenne Date: Thu, 8 Jul 2021 18:37:45 +0200 Subject: [PATCH] Add api endpoints for changing boot_mode and secure_boot state Done: - Node API endpoints expose - RPC methods - Conductor Manager methods - Conductor utils new methods - RBAC new policies - Node API tests - Manager Tests (+ some testing for utils methods) - RBAC tests - Docs (api-ref) - REST API version history - Releasenotes Story: 2008567 Task: 41709 Change-Id: I2d72389edf546b99c536c6b130ca85ababf80591 --- .../baremetal-api-v1-node-management.inc | 69 +++ api-ref/source/parameters.yaml | 14 + .../samples/node-set-boot-mode-bios.json | 3 + .../samples/node-set-boot-mode-uefi.json | 3 + .../samples/node-set-secure-boot-off.json | 3 + .../samples/node-set-secure-boot-on.json | 3 + .../contributor/webapi-version-history.rst | 10 +- ironic/api/controllers/v1/node.py | 107 +++++ ironic/api/controllers/v1/versions.py | 4 +- ironic/common/policy.py | 20 + ironic/common/release_mappings.py | 4 +- ironic/conductor/manager.py | 60 ++- ironic/conductor/rpcapi.py | 43 +- ironic/conductor/utils.py | 108 ++++- .../unit/api/controllers/v1/test_node.py | 150 ++++++ ironic/tests/unit/api/test_rbac_legacy.yaml | 50 ++ .../unit/api/test_rbac_project_scoped.yaml | 104 +++++ .../unit/api/test_rbac_system_scoped.yaml | 48 ++ ironic/tests/unit/conductor/test_manager.py | 426 ++++++++++++++++++ ironic/tests/unit/conductor/test_rpcapi.py | 15 + .../node-boot-mode-0662effa2a2644dc.yaml | 2 +- ...boot-mode-change-api-c5e392e3cd6ea54b.yaml | 13 + 22 files changed, 1248 insertions(+), 11 deletions(-) create mode 100644 api-ref/source/samples/node-set-boot-mode-bios.json create mode 100644 api-ref/source/samples/node-set-boot-mode-uefi.json create mode 100644 api-ref/source/samples/node-set-secure-boot-off.json create mode 100644 api-ref/source/samples/node-set-secure-boot-on.json create mode 100644 releasenotes/notes/node-boot-mode-change-api-c5e392e3cd6ea54b.yaml diff --git a/api-ref/source/baremetal-api-v1-node-management.inc b/api-ref/source/baremetal-api-v1-node-management.inc index 0f2f3e1756..60986be712 100644 --- a/api-ref/source/baremetal-api-v1-node-management.inc +++ b/api-ref/source/baremetal-api-v1-node-management.inc @@ -299,6 +299,75 @@ Response .. literalinclude:: samples/node-get-state-response.json +Change Node Boot Mode +===================== + +.. rest_method:: PUT /v1/nodes/{node_ident}/states/boot_mode + +Request a change to the Node's boot mode. + +.. versionadded:: 1.76 + A change in node's boot mode can be requested. + +Normal response code: 202 (Accepted) + +Error codes: + - 409 (Conflict, NodeLocked, ClientError) + - 400 (Invalid, InvalidStateRequested, InvalidParameterValue) + - 404 (NotFound) + - 503 (NoFreeConductorWorkers) + +Request +------- + +.. rest_parameters:: parameters.yaml + + - node_ident: node_ident + - target: req_target_boot_mode + +**Example request for UEFI boot:** + +.. literalinclude:: samples/node-set-boot-mode-uefi.json + +**Example request for Legacy BIOS boot:** + +.. literalinclude:: samples/node-set-boot-mode-bios.json + + +Change Node Secure Boot +======================= + +.. rest_method:: PUT /v1/nodes/{node_ident}/states/secure_boot + +Request a change to the Node's secure boot state. + +.. versionadded:: 1.76 + A change in node's secure boot state can be requested. + +Normal response code: 202 (Accepted) + +Error codes: + - 409 (Conflict, NodeLocked, ClientError) + - 400 (Invalid, InvalidStateRequested, InvalidParameterValue) + - 404 (NotFound) + - 503 (NoFreeConductorWorkers) + +Request +------- + +.. rest_parameters:: parameters.yaml + + - node_ident: node_ident + - target: req_target_secure_boot + +**Example request to turn off secure boot:** + +.. literalinclude:: samples/node-set-secure-boot-off.json + +**Example request to turn on secure boot:** + +.. literalinclude:: samples/node-set-secure-boot-on.json + Change Node Power State ======================= diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 04b58aa91c..266986c1b9 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -1754,6 +1754,13 @@ req_storage_interface: in: body required: false type: string +req_target_boot_mode: + description: | + If a boot mode change has been requested, this field represents the + requested (ie, "target") state, either "uefi" or "bios". + in: body + required: true + type: string req_target_power_state: description: | If a power state transition has been requested, this field represents the @@ -1770,6 +1777,13 @@ req_target_raid_config: in: body required: true type: JSON +req_target_secure_boot: + description: | + If a secure boot change has been requested, this field represents the + requested (ie, "target") state, either ``true`` or ``false``. + in: body + required: true + type: boolean req_uuid: description: | The UUID for the resource. diff --git a/api-ref/source/samples/node-set-boot-mode-bios.json b/api-ref/source/samples/node-set-boot-mode-bios.json new file mode 100644 index 0000000000..cb81af1867 --- /dev/null +++ b/api-ref/source/samples/node-set-boot-mode-bios.json @@ -0,0 +1,3 @@ +{ + "target": "bios" +} diff --git a/api-ref/source/samples/node-set-boot-mode-uefi.json b/api-ref/source/samples/node-set-boot-mode-uefi.json new file mode 100644 index 0000000000..461a5818b7 --- /dev/null +++ b/api-ref/source/samples/node-set-boot-mode-uefi.json @@ -0,0 +1,3 @@ +{ + "target": "uefi" +} diff --git a/api-ref/source/samples/node-set-secure-boot-off.json b/api-ref/source/samples/node-set-secure-boot-off.json new file mode 100644 index 0000000000..e8ad3700ee --- /dev/null +++ b/api-ref/source/samples/node-set-secure-boot-off.json @@ -0,0 +1,3 @@ +{ + "target": false +} diff --git a/api-ref/source/samples/node-set-secure-boot-on.json b/api-ref/source/samples/node-set-secure-boot-on.json new file mode 100644 index 0000000000..4c1a00634b --- /dev/null +++ b/api-ref/source/samples/node-set-secure-boot-on.json @@ -0,0 +1,3 @@ +{ + "target": true +} diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index 37cda35952..6d6b715c78 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,9 +2,17 @@ REST API Version History ======================== +1.76 (Xena, ?) +---------------------- +Add endpoints for changing boot mode and secure boot state of node +asynchronously: + +* ``PUT /v1/nodes/{node_ident}/states/boot_mode`` +* ``PUT /v1/nodes/{node_ident}/states/secure_boot`` + 1.75 (Xena, 18.1) ---------------------- -Add `boot_mode` and `secure_boot` to node object and expose their state at: +Add ``boot_mode`` and ``secure_boot`` to node object and expose their state at: * ``/v1/nodes/{node_ident}/states`` diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 8a53c4fbbf..e40743617f 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -40,6 +40,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_modes from ironic.common import exception from ironic.common.i18n import _ from ironic.common import policy @@ -120,6 +121,9 @@ ALLOWED_TARGET_POWER_STATES = (ir_states.POWER_ON, ir_states.SOFT_REBOOT, ir_states.SOFT_POWER_OFF) +ALLOWED_TARGET_BOOT_MODES = (boot_modes.LEGACY_BIOS, + boot_modes.UEFI) + _NODE_DESCRIPTION_MAX_LENGTH = 4096 _NETWORK_DATA_SCHEMA = None @@ -710,6 +714,8 @@ def node_states_convert(rpc_node): class NodeStatesController(rest.RestController): _custom_actions = { + 'boot_mode': ['PUT'], + 'secure_boot': ['PUT'], 'power': ['PUT'], 'provision': ['PUT'], 'raid': ['PUT'], @@ -822,6 +828,107 @@ class NodeStatesController(rest.RestController): url_args = '/'.join([node_ident, 'states']) api.response.location = link.build_url('nodes', url_args) + @METRICS.timer('NodeStatesController.boot_mode') + @method.expose(status_code=http_client.ACCEPTED) + @args.validate(node_ident=args.uuid_or_name, target=args.string) + def boot_mode(self, node_ident, target): + """Asynchronous set the boot mode of the node. + + :param node_ident: the UUID or logical name of a node. + :param target: The desired boot_mode for the node. (uefi/bios) + :raises: NotFound (HTTP 404) if requested version of the API + is less than 1.76. + :raises: InvalidParameterValue (HTTP 400) if the requested target + state is not valid. + :raises: Conflict (HTTP 409) if a node is in adopting state or + another transient state. + + """ + rpc_node = api_utils.check_node_policy_and_retrieve( + 'baremetal:node:set_boot_mode', node_ident) + topic = api.request.rpcapi.get_topic_for(rpc_node) + + if (api.request.version.minor + < versions.MINOR_76_NODE_CHANGE_BOOT_MODE): + raise exception.NotFound( + (_("This endpoint is supported starting with the API version " + "1.%(min_version)s") % + {'min_version': versions.MINOR_76_NODE_CHANGE_BOOT_MODE})) + + if target not in ALLOWED_TARGET_BOOT_MODES: + msg = (_("Invalid boot mode %(mode)s requested for node. " + "Allowed boot modes are: " + "%(modes)s") % + {'mode': target, + 'modes': ', '.join(ALLOWED_TARGET_BOOT_MODES)}) + raise exception.InvalidParameterValue(msg) + + # NOTE(cenne): This currenly includes the ADOPTING state + if rpc_node.provision_state in ir_states.UNSTABLE_STATES: + msg = _("Node is in %(state)s state. Since node is transitioning, " + "the boot mode will not be set as this may interfere " + "with ongoing changes and result in erroneous modification" + ". Try again later.") + raise exception.Conflict(msg, + action=target, node=node_ident, + state=rpc_node.provision_state + ) + api.request.rpcapi.change_node_boot_mode(api.request.context, + rpc_node.uuid, target, + topic=topic) + # Set the HTTP Location Header + url_args = '/'.join([node_ident, 'states']) + api.response.location = link.build_url('nodes', url_args) + + @METRICS.timer('NodeStatesController.secure_boot') + @method.expose(status_code=http_client.ACCEPTED) + @args.validate(node_ident=args.uuid_or_name, target=args.boolean) + def secure_boot(self, node_ident, target): + """Asynchronous set the secure_boot state of the node. + + :param node_ident: the UUID or logical name of a node. + :param target: The desired secure_boot for the node. (True/False) + :raises: NotFound (HTTP 404) if requested version of the API + is less than 1.76. + :raises: InvalidParameterValue (HTTP 400) if the requested target + state is not valid. + :raises: Conflict (HTTP 409) if a node is in adopting state. + + """ + rpc_node = api_utils.check_node_policy_and_retrieve( + 'baremetal:node:set_secure_boot', node_ident) + topic = api.request.rpcapi.get_topic_for(rpc_node) + + if (api.request.version.minor + < versions.MINOR_76_NODE_CHANGE_BOOT_MODE): + raise exception.NotFound( + (_("This endpoint is supported starting with the API version " + "1.%(min_version)s") % + {'min_version': versions.MINOR_76_NODE_CHANGE_BOOT_MODE})) + # NOTE(cenne): This is to exclude target=None or other invalid values + if target not in (True, False): + msg = (_("Invalid secure_boot %(state)s requested for node. " + "Allowed secure_boot states are: True, False) ") % + {'state': target}) + raise exception.InvalidParameterValue(msg) + + # NOTE(cenne): This currenly includes the ADOPTING state + if rpc_node.provision_state in ir_states.UNSTABLE_STATES: + msg = _("Node is in %(state)s state. Since node is transitioning, " + "the boot mode will not be set as this may interfere " + "with ongoing changes and result in erroneous modification" + ". Try again later.") + raise exception.Conflict(msg, + action=target, node=node_ident, + state=rpc_node.provision_state + ) + api.request.rpcapi.change_node_secure_boot(api.request.context, + rpc_node.uuid, target, + topic=topic) + # Set the HTTP Location Header + url_args = '/'.join([node_ident, 'states']) + api.response.location = link.build_url('nodes', url_args) + def _do_provision_action(self, rpc_node, target, configdrive=None, clean_steps=None, deploy_steps=None, rescue_password=None, disable_ramdisk=None): diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 1af83c431b..9cd890e8f8 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -113,6 +113,7 @@ BASE_VERSION = 1 # v1.73: Add support for deploy and undeploy verbs # v1.74: Add bios registry to /v1/nodes/{node}/bios/{setting} # v1.75: Add boot_mode, secure_boot fields to node object. +# v1.76: Add support for changing boot_mode and secure_boot state MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -190,6 +191,7 @@ MINOR_72_HEARTBEAT_STATUS = 72 MINOR_73_DEPLOY_UNDEPLOY_VERBS = 73 MINOR_74_BIOS_REGISTRY = 74 MINOR_75_NODE_BOOT_MODE = 75 +MINOR_76_NODE_CHANGE_BOOT_MODE = 76 # When adding another version, update: # - MINOR_MAX_VERSION @@ -197,7 +199,7 @@ MINOR_75_NODE_BOOT_MODE = 75 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_75_NODE_BOOT_MODE +MINOR_MAX_VERSION = MINOR_76_NODE_CHANGE_BOOT_MODE # 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 dfff7d49cf..29040c32bd 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -770,6 +770,26 @@ node_policies = [ ], deprecated_rule=deprecated_node_set_power_state ), + policy.DocumentedRuleDefault( + name='baremetal:node:set_boot_mode', + check_str=SYSTEM_OR_PROJECT_MEMBER, + scope_types=['system', 'project'], + description='Change Node boot mode', + operations=[ + {'path': '/nodes/{node_ident}/states/boot_mode', 'method': 'PUT'} + ], + deprecated_rule=deprecated_node_set_power_state + ), + policy.DocumentedRuleDefault( + name='baremetal:node:set_secure_boot', + check_str=SYSTEM_OR_PROJECT_MEMBER, + scope_types=['system', 'project'], + description='Change Node secure boot state', + operations=[ + {'path': '/nodes/{node_ident}/states/secure_boot', 'method': 'PUT'} + ], + deprecated_rule=deprecated_node_set_power_state + ), policy.DocumentedRuleDefault( name='baremetal:node:set_provision_state', check_str=SYSTEM_OR_OWNER_MEMBER_AND_LESSEE_ADMIN, diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index e7c2c6912e..ee7634aaba 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -371,8 +371,8 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.75', - 'rpc': '1.54', + 'api': '1.76', + 'rpc': '1.55', 'objects': { 'Allocation': ['1.1'], 'BIOSSetting': ['1.1'], diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index 01f340fcbb..9f3fb185dc 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -91,7 +91,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.54' + RPC_API_VERSION = '1.55' target = messaging.Target(version=RPC_API_VERSION) @@ -346,6 +346,64 @@ class ConductorManager(base_manager.BaseConductorManager): task.spawn_after(self._spawn_worker, utils.node_power_action, task, new_state, timeout=power_timeout) + @METRICS.timer('ConductorManager.change_node_boot_mode') + @messaging.expected_exceptions(exception.InvalidParameterValue, + exception.NoFreeConductorWorker, + exception.NodeLocked) + def change_node_boot_mode(self, context, node_id, new_state): + """RPC method to encapsulate changes to a node's boot_mode. + + :param context: an admin context. + :param node_id: the id or uuid of a node. + :param new_state: the desired boot mode for the node. + :raises: NoFreeConductorWorker when there is no free worker to start + async task. + :raises: InvalidParameterValue + """ + LOG.debug("RPC change_node_boot_mode called for node %(node)s. " + "The desired new state is %(state)s.", + {'node': node_id, 'state': new_state}) + with task_manager.acquire(context, node_id, shared=False, + purpose='changing node boot mode') 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, "changing node boot mode") + task.spawn_after(self._spawn_worker, + utils.node_change_boot_mode, task, new_state) + + @METRICS.timer('ConductorManager.change_node_secure_boot') + @messaging.expected_exceptions(exception.InvalidParameterValue, + exception.NoFreeConductorWorker, + exception.NodeLocked) + def change_node_secure_boot(self, context, node_id, new_state): + """RPC method to encapsulate changes to a node's secure_boot state. + + :param context: an admin context. + :param node_id: the id or uuid of a node. + :param new_state: the desired secure_boot state for the node. + :raises: NoFreeConductorWorker when there is no free worker to start + async task. + :raises: InvalidParameterValue + """ + LOG.debug("RPC change_node_secure_boot called for node %(node)s. " + "The desired new state is %(state)s.", + {'node': node_id, 'state': new_state}) + with task_manager.acquire(context, node_id, shared=False, + purpose='changing node secure') 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, "changing node secure boot") + task.spawn_after(self._spawn_worker, + utils.node_change_secure_boot, task, new_state) + @METRICS.timer('ConductorManager.vendor_passthru') @messaging.expected_exceptions(exception.NoFreeConductorWorker, exception.NodeLocked, diff --git a/ironic/conductor/rpcapi.py b/ironic/conductor/rpcapi.py index d05f75228a..ec1cf05891 100644 --- a/ironic/conductor/rpcapi.py +++ b/ironic/conductor/rpcapi.py @@ -108,12 +108,13 @@ class ConductorAPI(object): | 1.53 - Added disable_ramdisk to do_node_clean. | 1.54 - Added optional agent_status and agent_status_message to heartbeat + | 1.55 - Added change_node_boot_mode """ # 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.54' + RPC_API_VERSION = '1.55' def __init__(self, topic=None): super(ConductorAPI, self).__init__() @@ -281,6 +282,46 @@ class ConductorAPI(object): return cctxt.call(context, 'change_node_power_state', node_id=node_id, new_state=new_state, timeout=timeout) + def change_node_boot_mode(self, context, node_id, new_state, + topic=None): + """Change a node's boot mode. + + Synchronously, acquire lock and start the conductor background task + to change boot mode of a node. + + :param context: request context. + :param node_id: node id or uuid. + :param new_state: one of ironic.common.boot_modes values + ('bios' / 'uefi') + :param topic: RPC topic. Defaults to self.topic. + :raises: NoFreeConductorWorker when there is no free worker to start + async task. + + """ + cctxt = self.client.prepare(topic=topic or self.topic, version='1.55') + return cctxt.call(context, 'change_node_boot_mode', node_id=node_id, + new_state=new_state) + + def change_node_secure_boot(self, context, node_id, new_state, + topic=None): + """Change a node's secure_boot state. + + Synchronously, acquire lock and start the conductor background task + to change secure_boot state of a node. + + :param context: request context. + :param node_id: node id or uuid. + :param new_state: Target secure boot state + (True => 'on' / False => 'off') + :param topic: RPC topic. Defaults to self.topic. + :raises: NoFreeConductorWorker when there is no free worker to start + async task. + + """ + cctxt = self.client.prepare(topic=topic or self.topic, version='1.55') + return cctxt.call(context, 'change_node_secure_boot', node_id=node_id, + new_state=new_state) + def vendor_passthru(self, context, node_id, driver_method, http_method, info, topic=None): """Receive requests for vendor-specific actions. diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index 491e4a8837..bbc489dfc9 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -133,14 +133,22 @@ def node_set_boot_mode(task, mode): return task.driver.management.validate(task) + try: + supported_boot_modes = ( + task.driver.management.get_supported_boot_modes(task) + ) + except exception.UnsupportedDriverExtension: + LOG.debug( + "Cannot determine supported boot modes of driver " + "%(driver)s. Will make an attempt to set boot mode %(mode)s", + {'driver': task.node.driver, 'mode': mode}) + supported_boot_modes = () - boot_modes = task.driver.management.get_supported_boot_modes(task) - - if mode not in boot_modes: + if supported_boot_modes and mode not in supported_boot_modes: msg = _("Unsupported boot mode %(mode)s specified for " "node %(node_id)s. Supported boot modes are: " "%(modes)s") % {'mode': mode, - 'modes': ', '.join(boot_modes), + 'modes': ', '.join(supported_boot_modes), 'node_id': task.node.uuid} raise exception.InvalidParameterValue(msg) @@ -1473,3 +1481,95 @@ def node_cache_boot_mode(task): "for node %(node)s", {'boot_mode': boot_mode, 'secure_boot': secure_boot, 'node': task.node.uuid}) + + +def node_change_boot_mode(task, target_boot_mode): + """Change boot mode to requested state for node + + :param task: a TaskManager instance containing the node to act on. + :param target_boot_mode: Any boot mode in :mod:`ironic.common.boot_modes`. + """ + try: + current_boot_mode = task.driver.management.get_boot_mode(task) + except Exception as exc: + current_boot_mode = None + LOG.warning('Unexpected exception when trying to detect boot_mode ' + 'while changing boot mode for node ' + '%(node)s. %(class)s: %(exc)s', + {'node': task.node.uuid, + 'class': type(exc).__name__, 'exc': exc}, + exc_info=not isinstance(exc, exception.IronicException)) + + if (current_boot_mode is not None + and target_boot_mode == current_boot_mode): + LOG.info("Target boot mode '%(target)s', and current boot mode " + "'%(current)s' are identical. No change being made " + "for node %(node)s", + {'target': target_boot_mode, 'current': current_boot_mode, + 'node': task.node.uuid}) + return + try: + task.driver.management.set_boot_mode(task, mode=target_boot_mode) + except Exception as exc: + LOG.error('Unexpected exception when trying to change boot_mode ' + 'to %(target)s for node %(node)s. %(class)s: %(exc)s', + {'node': task.node.uuid, 'target': target_boot_mode, + 'class': type(exc).__name__, 'exc': exc}, + exc_info=not isinstance(exc, exception.IronicException)) + task.node.last_error = ( + "Failed to change boot mode to '%(target)s'. " + "Error: %(err)s" % {'target': target_boot_mode, 'err': exc}) + task.node.save() + else: + LOG.info("Changed boot_mode to %(mode)s for node %(node)s", + {'mode': target_boot_mode, 'node': task.node.uuid}) + task.node.boot_mode = target_boot_mode + task.node.save() + + +def node_change_secure_boot(task, secure_boot_target): + """Change secure_boot state to requested state for node + + :param task: a TaskManager instance containing the node to act on. + :param secure_boot_target: Target secure_boot state + OneOf(True => on, False => off) + :type secure_boot_target: boolean + """ + try: + secure_boot_current = task.driver.management.get_secure_boot_state( + task) + except Exception as exc: + secure_boot_current = None + LOG.warning('Unexpected exception when trying to detect secure_boot ' + 'state while changing secure_boot for node ' + '%(node)s. %(class)s: %(exc)s', + {'node': task.node.uuid, + 'class': type(exc).__name__, 'exc': exc}, + exc_info=not isinstance(exc, exception.IronicException)) + + if (secure_boot_current is not None + and secure_boot_target == secure_boot_current): + LOG.info("Target secure_boot state '%(target)s', and current " + "secure_boot state '%(current)s' are identical. " + "No change being made for node %(node)s", + {'target': secure_boot_target, + 'current': secure_boot_current, + 'node': task.node.uuid}) + return + try: + task.driver.management.set_secure_boot_state(task, secure_boot_target) + except Exception as exc: + LOG.error('Unexpected exception when trying to change secure_boot ' + 'to %(target)s for node %(node)s. %(class)s: %(exc)s', + {'node': task.node.uuid, 'target': secure_boot_target, + 'class': type(exc).__name__, 'exc': exc}, + exc_info=not isinstance(exc, exception.IronicException)) + task.node.last_error = ( + "Failed to change secure_boot state to '%(target)s'. " + "Error: %(err)s" % {'target': secure_boot_target, 'err': exc}) + task.node.save() + else: + LOG.info("Changed secure_boot state to %(state)s for node %(node)s", + {'state': secure_boot_target, 'node': task.node.uuid}) + task.node.secure_boot = secure_boot_target + task.node.save() diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 0ae598026c..9b37a542f5 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -35,6 +35,7 @@ from ironic.api.controllers.v1 import notification_utils from ironic.api.controllers.v1 import utils as api_utils from ironic.api.controllers.v1 import versions from ironic.common import boot_devices +from ironic.common import boot_modes from ironic.common import components from ironic.common import driver_factory from ironic.common import exception @@ -5136,6 +5137,14 @@ class TestPut(test_api_base.BaseApiTest): autospec=True) self.mock_cnps = p.start() self.addCleanup(p.stop) + p = mock.patch.object(rpcapi.ConductorAPI, 'change_node_boot_mode', + autospec=True) + self.mock_cnbm = p.start() + self.addCleanup(p.stop) + p = mock.patch.object(rpcapi.ConductorAPI, 'change_node_secure_boot', + autospec=True) + self.mock_cnsb = p.start() + self.addCleanup(p.stop) p = mock.patch.object(rpcapi.ConductorAPI, 'do_node_deploy', autospec=True) self.mock_dnd = p.start() @@ -5301,6 +5310,147 @@ class TestPut(test_api_base.BaseApiTest): {'target': 'not-supported'}, expect_errors=True) self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + def _test_boot_mode_success(self, target_state, api_version): + + body = {'target': target_state} + + if api_version is None: + response = self.put_json( + '/nodes/%s/states/boot_mode' % self.node.uuid, body) + else: + response = self.put_json( + '/nodes/%s/states/boot_mode' % self.node.uuid, body, + headers={api_base.Version.string: api_version}) + + self.assertEqual(http_client.ACCEPTED, response.status_code) + self.assertEqual(b'', response.body) + self.mock_cnbm.assert_called_once_with(mock.ANY, + mock.ANY, + self.node.uuid, + target_state, + topic='test-topic') + # Check location header + self.assertIsNotNone(response.location) + expected_location = '/v1/nodes/%s/states' % self.node.uuid + self.assertEqual(urlparse.urlparse(response.location).path, + expected_location) + + def _test_boot_mode_failure(self, target_state, http_status_code, + api_version): + + body = {'target': target_state} + + if api_version is None: + response = self.put_json( + '/nodes/%s/states/boot_mode' % self.node.uuid, body, + expect_errors=True) + else: + response = self.put_json( + '/nodes/%s/states/boot_mode' % self.node.uuid, body, + headers={api_base.Version.string: api_version}, + expect_errors=True) + + self.assertEqual(http_status_code, response.status_code) + self.assertEqual('application/json', response.content_type) + self.assertTrue(response.json['error_message']) + + def test_boot_mode_uefi_valid_soft_ver(self): + self._test_boot_mode_success(boot_modes.UEFI, "1.76") + + def test_boot_mode_uefi_older_soft_ver(self): + self._test_boot_mode_failure( + boot_modes.UEFI, http_client.NOT_FOUND, "1.75") + + def test_boot_mode_bios_valid_soft_ver(self): + self._test_boot_mode_success(boot_modes.LEGACY_BIOS, "1.76") + + def test_boot_mode_bios_older_soft_ver(self): + self._test_boot_mode_failure( + boot_modes.LEGACY_BIOS, http_client.NOT_FOUND, "1.75") + + def test_boot_mode_invalid_request(self): + self._test_boot_mode_failure( + 'unsupported-efi', http_client.BAD_REQUEST, "1.76") + + def _test_secure_boot_success(self, target_state, api_version): + + body = {'target': target_state} + + if api_version is None: + response = self.put_json( + '/nodes/%s/states/secure_boot' % self.node.uuid, body) + else: + response = self.put_json( + '/nodes/%s/states/secure_boot' % self.node.uuid, body, + headers={api_base.Version.string: api_version}) + + self.assertEqual(http_client.ACCEPTED, response.status_code) + self.assertEqual(b'', response.body) + self.mock_cnsb.assert_called_once_with(mock.ANY, + mock.ANY, + self.node.uuid, + target_state, + topic='test-topic') + # Check location header + self.assertIsNotNone(response.location) + expected_location = '/v1/nodes/%s/states' % self.node.uuid + self.assertEqual(urlparse.urlparse(response.location).path, + expected_location) + + def _test_secure_boot_failure(self, target_state, http_status_code, + api_version): + + body = {'target': target_state} + + if api_version is None: + response = self.put_json( + '/nodes/%s/states/secure_boot' % self.node.uuid, body, + expect_errors=True) + else: + response = self.put_json( + '/nodes/%s/states/secure_boot' % self.node.uuid, body, + headers={api_base.Version.string: api_version}, + expect_errors=True) + + self.assertEqual(http_status_code, response.status_code) + self.assertEqual('application/json', response.content_type) + self.assertTrue(response.json['error_message']) + + def test_secure_boot_on_valid_soft_ver(self): + self._test_secure_boot_success(True, "1.76") + + def test_secure_boot_on_older_soft_ver(self): + self._test_secure_boot_failure( + True, http_client.NOT_FOUND, "1.75") + + def test_secure_boot_off_valid_soft_ver(self): + self._test_secure_boot_success(False, "1.76") + + def test_secure_boot_off_older_soft_ver(self): + self._test_secure_boot_failure( + False, http_client.NOT_FOUND, "1.75") + + def test_secure_boot_off_valid_undocumented_request_zero(self): + self._test_secure_boot_success(0, "1.76") + + def test_secure_boot_on_valid_undocumented_request_one(self): + self._test_secure_boot_success(1, "1.76") + + def test_secure_boot_on_invalid_request_two(self): + self._test_secure_boot_failure(2, http_client.BAD_REQUEST, "1.76") + + def test_secure_boot_invalid_request_nullstr(self): + self._test_secure_boot_failure( + '', http_client.BAD_REQUEST, "1.76") + + def test_secure_boot_invalid_request_boo(self): + self._test_secure_boot_failure( + 'boo!', http_client.BAD_REQUEST, "1.76") + + def test_secure_boot_invalid_request_None(self): + self._test_secure_boot_failure( + None, http_client.BAD_REQUEST, "1.76") + def test_power_change_when_being_cleaned(self): for state in (states.CLEANING, states.CLEANWAIT): self.node.provision_state = state diff --git a/ironic/tests/unit/api/test_rbac_legacy.yaml b/ironic/tests/unit/api/test_rbac_legacy.yaml index 2f981e68fc..a665d15fc2 100644 --- a/ironic/tests/unit/api/test_rbac_legacy.yaml +++ b/ironic/tests/unit/api/test_rbac_legacy.yaml @@ -412,6 +412,56 @@ nodes_states_power_put_observer: assert_status: 403 deprecated: true +nodes_states_boot_mode_put_admin: + path: '/v1/nodes/{node_ident}/states/boot_mode' + method: put + headers: *admin_headers + body: &boot_mode_body + target: "uefi" + assert_status: 503 + deprecated: true + +nodes_states_boot_mode_put_member: + path: '/v1/nodes/{node_ident}/states/boot_mode' + method: put + headers: *member_headers + body: *boot_mode_body + assert_status: 404 + deprecated: true + +nodes_states_boot_mode_put_observer: + path: '/v1/nodes/{node_ident}/states/boot_mode' + method: put + headers: *observer_headers + body: *boot_mode_body + assert_status: 403 + deprecated: true + +nodes_states_secure_boot_put_admin: + path: '/v1/nodes/{node_ident}/states/secure_boot' + method: put + headers: *admin_headers + body: &secure_boot_body + target: "true" + assert_status: 503 + deprecated: true + +nodes_states_secure_boot_put_member: + path: '/v1/nodes/{node_ident}/states/secure_boot' + method: put + headers: *member_headers + body: *secure_boot_body + assert_status: 404 + deprecated: true + +nodes_states_secure_boot_put_observer: + path: '/v1/nodes/{node_ident}/states/secure_boot' + method: put + headers: *observer_headers + body: *secure_boot_body + assert_status: 403 + deprecated: true + nodes_states_provision_put_admin: path: '/v1/nodes/{node_ident}/states/provision' method: put diff --git a/ironic/tests/unit/api/test_rbac_project_scoped.yaml b/ironic/tests/unit/api/test_rbac_project_scoped.yaml index 6ec74d2ddc..81e7f646f4 100644 --- a/ironic/tests/unit/api/test_rbac_project_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_project_scoped.yaml @@ -863,6 +863,110 @@ third_party_admin_cannot_put_power_state_change: body: *power_body assert_status: 404 +# Boot mode state + +owner_admin_can_put_boot_mode_state_change: + path: '/v1/nodes/{owner_node_ident}/states/boot_mode' + method: put + headers: *owner_admin_headers + body: &boot_mode_body + target: "uefi" + assert_status: 503 + +lessee_admin_can_put_boot_mode_state_change: + path: '/v1/nodes/{lessee_node_ident}/states/boot_mode' + method: put + headers: *lessee_admin_headers + body: *boot_mode_body + assert_status: 503 + +owner_member_can_put_boot_mode_state_change: + path: '/v1/nodes/{owner_node_ident}/states/boot_mode' + method: put + headers: *owner_member_headers + body: *boot_mode_body + assert_status: 503 + +lessee_member_can_put_boot_mode_state_change: + path: '/v1/nodes/{lessee_node_ident}/states/boot_mode' + method: put + headers: *lessee_member_headers + body: *boot_mode_body + assert_status: 503 + +owner_reader_cannot_put_boot_mode_state_change: + path: '/v1/nodes/{owner_node_ident}/states/boot_mode' + method: put + headers: *owner_reader_headers + body: *boot_mode_body + assert_status: 403 + +lessee_reader_cannot_put_boot_mode_state_change: + path: '/v1/nodes/{lessee_node_ident}/states/boot_mode' + method: put + headers: *lessee_reader_headers + body: *boot_mode_body + assert_status: 403 + +third_party_admin_cannot_put_boot_mode_state_change: + path: '/v1/nodes/{node_ident}/states/boot_mode' + method: put + headers: *third_party_admin_headers + body: *boot_mode_body + assert_status: 404 + +# Secure Boot state + +owner_admin_can_put_secure_boot_state_change: + path: '/v1/nodes/{owner_node_ident}/states/secure_boot' + method: put + headers: *owner_admin_headers + body: &secure_boot_body + target: "true" + assert_status: 503 + +lessee_admin_can_put_secure_boot_state_change: + path: '/v1/nodes/{lessee_node_ident}/states/secure_boot' + method: put + headers: *lessee_admin_headers + body: *secure_boot_body + assert_status: 503 + +owner_member_can_put_secure_boot_state_change: + path: '/v1/nodes/{owner_node_ident}/states/secure_boot' + method: put + headers: *owner_member_headers + body: *secure_boot_body + assert_status: 503 + +lessee_member_can_put_secure_boot_state_change: + path: '/v1/nodes/{lessee_node_ident}/states/secure_boot' + method: put + headers: *lessee_member_headers + body: *secure_boot_body + assert_status: 503 + +owner_reader_cannot_put_secure_boot_state_change: + path: '/v1/nodes/{owner_node_ident}/states/secure_boot' + method: put + headers: *owner_reader_headers + body: *secure_boot_body + assert_status: 403 + +lessee_reader_cannot_put_secure_boot_state_change: + path: '/v1/nodes/{lessee_node_ident}/states/secure_boot' + method: put + headers: *lessee_reader_headers + body: *secure_boot_body + assert_status: 403 + +third_party_admin_cannot_put_secure_boot_state_change: + path: '/v1/nodes/{node_ident}/states/secure_boot' + method: put + headers: *third_party_admin_headers + body: *secure_boot_body + assert_status: 404 + # Provision states owner_admin_can_change_provision_state: diff --git a/ironic/tests/unit/api/test_rbac_system_scoped.yaml b/ironic/tests/unit/api/test_rbac_system_scoped.yaml index c0126b04dc..032001fb21 100644 --- a/ironic/tests/unit/api/test_rbac_system_scoped.yaml +++ b/ironic/tests/unit/api/test_rbac_system_scoped.yaml @@ -378,6 +378,54 @@ nodes_states_power_put_reader: body: *power_body assert_status: 403 +# Boot mode state + +nodes_states_boot_mode_put_admin: + path: '/v1/nodes/{node_ident}/states/boot_mode' + method: put + headers: *admin_headers + body: &boot_mode_body + target: "uefi" + assert_status: 503 + +nodes_states_boot_mode_put_member: + path: '/v1/nodes/{node_ident}/states/boot_mode' + method: put + headers: *scoped_member_headers + body: *boot_mode_body + assert_status: 503 + +nodes_states_boot_mode_put_reader: + path: '/v1/nodes/{node_ident}/states/boot_mode' + method: put + headers: *reader_headers + body: *boot_mode_body + assert_status: 403 + +# Secure Boot state + +nodes_states_secure_boot_put_admin: + path: '/v1/nodes/{node_ident}/states/secure_boot' + method: put + headers: *admin_headers + body: &secure_boot_body + target: "true" + assert_status: 503 + +nodes_states_secure_boot_put_member: + path: '/v1/nodes/{node_ident}/states/secure_boot' + method: put + headers: *scoped_member_headers + body: *secure_boot_body + assert_status: 503 + +nodes_states_secure_boot_put_reader: + path: '/v1/nodes/{node_ident}/states/secure_boot' + method: put + headers: *reader_headers + body: *secure_boot_body + assert_status: 403 + nodes_states_provision_put_admin: path: '/v1/nodes/{node_ident}/states/provision' method: put diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 897e19d3e0..1603b19be3 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -34,6 +34,7 @@ from oslo_versionedobjects import fields import tenacity from ironic.common import boot_devices +from ironic.common import boot_modes from ironic.common import components from ironic.common import driver_factory from ironic.common import exception @@ -447,6 +448,431 @@ class ChangeNodePowerStateTestCase(mgr_utils.ServiceSetUpMixin, self.assertIsNone(node.last_error) +@mgr_utils.mock_record_keepalive +class ChangeNodeBootModeTestCase(mgr_utils.ServiceSetUpMixin, + db_base.DbTestCase): + @mock.patch.object(fake.FakeManagement, 'set_boot_mode', autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_boot_mode', autospec=True) + def test_change_node_boot_mode_valid(self, get_boot_mock, set_boot_mock): + # Test change_node_boot_mode including integration with + # conductor.utils.node_change_boot_mode + get_boot_mock.side_effect = [boot_modes.LEGACY_BIOS, # before setting + boot_modes.UEFI] # after setting + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + boot_mode=boot_modes.LEGACY_BIOS) + self._start_service() + + self.service.change_node_boot_mode(self.context, + node.uuid, + boot_modes.UEFI) + self._stop_service() + + set_boot_mock.assert_called_once_with(mock.ANY, mock.ANY, + mode=boot_modes.UEFI) + self.assertEqual(get_boot_mock.call_count, 1) + # Call once before setting to see if it's required + + node.refresh() + self.assertEqual(boot_modes.UEFI, node.boot_mode) + self.assertIsNone(node.last_error) + # Verify the reservation has been cleared by + # background task's link callback. + self.assertIsNone(node.reservation) + + @mock.patch.object(fake.FakeManagement, 'set_boot_mode', autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_boot_mode', autospec=True) + def test_change_node_boot_mode_existing(self, get_boot_mock, + set_boot_mock): + # Test change_node_boot_mode including integration with + # conductor.utils.node_change_boot_mode when target==current + get_boot_mock.return_value = boot_modes.LEGACY_BIOS + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + boot_mode=boot_modes.LEGACY_BIOS) + self._start_service() + + self.service.change_node_boot_mode(self.context, + node.uuid, + boot_modes.LEGACY_BIOS) + self._stop_service() + + set_boot_mock.assert_not_called() + self.assertEqual(get_boot_mock.call_count, 1) + # Called once before setting to see if it's even required + + node.refresh() + self.assertEqual(boot_modes.LEGACY_BIOS, node.boot_mode) + self.assertIsNone(node.last_error) + # Verify the reservation has been cleared by + # background task's link callback. + self.assertIsNone(node.reservation) + + @mock.patch.object(conductor_utils, 'node_change_boot_mode', + autospec=True) + def test_change_node_boot_mode_node_already_locked(self, ncbm_mock): + # Test change_node_boot_mode with mocked + # conductor.utils.node_change_boot_mode. + fake_reservation = 'fake-reserv' + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + boot_mode=boot_modes.LEGACY_BIOS, + reservation=fake_reservation) + self._start_service() + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.change_node_boot_mode, + self.context, + node.uuid, + boot_modes.LEGACY_BIOS) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeLocked, exc.exc_info[0]) + + # In this test worker should not be spawned, but waiting to make sure + # the below perform_mock assertion is valid. + self._stop_service() + self.assertFalse(ncbm_mock.called, 'node_change_boot_mode has been ' + 'unexpectedly called.') + # Verify existing reservation wasn't broken. + node.refresh() + self.assertEqual(fake_reservation, node.reservation) + + def test_change_node_boot_mode_worker_pool_full(self): + # Test change_node_boot_mode including integration with + # conductor.utils.change_node_boot_mode. + initial_state = boot_modes.LEGACY_BIOS + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + boot_mode=initial_state) + self._start_service() + + with mock.patch.object(self.service, + '_spawn_worker', autospec=True) as spawn_mock: + spawn_mock.side_effect = exception.NoFreeConductorWorker() + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.change_node_boot_mode, + self.context, + node.uuid, + boot_modes.UEFI) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NoFreeConductorWorker, exc.exc_info[0]) + + spawn_mock.assert_called_once_with(mock.ANY, mock.ANY, + mock.ANY) + node.refresh() + self.assertEqual(initial_state, node.boot_mode) + self.assertIsNotNone(node.last_error) + # Verify the picked reservation has been cleared due to full pool. + self.assertIsNone(node.reservation) + + @mock.patch.object(fake.FakeManagement, 'set_boot_mode', autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_boot_mode', autospec=True) + def test_change_node_boot_mode_exception_in_background_task( + self, get_boot_mock, set_boot_mock): + # Test change_node_boot_mode including integration with + # conductor.utils.node_change_boot_mode. + initial_state = boot_modes.LEGACY_BIOS + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + boot_mode=initial_state) + self._start_service() + + get_boot_mock.return_value = boot_modes.LEGACY_BIOS + new_state = boot_modes.UEFI + set_boot_mock.side_effect = exception.UnsupportedDriverExtension( + driver=fake, extension='set_boot_mode') + + self.service.change_node_boot_mode(self.context, + node.uuid, + new_state) + self._stop_service() + + # Call once before setting to see if it was required + self.assertEqual(get_boot_mock.call_count, 1) + set_boot_mock.assert_called_once_with(mock.ANY, mock.ANY, + new_state) + node.refresh() + self.assertEqual(initial_state, node.boot_mode) + self.assertIsNotNone(node.last_error) + # Verify the reservation has been cleared by background task's + # link callback despite exception in background task. + self.assertIsNone(node.reservation) + + @mock.patch.object(fake.FakeManagement, 'validate', autospec=True) + def test_change_node_boot_mode_validate_fail(self, validate_mock): + # Test change_node_power_state where task.driver.management.validate + # fails + initial_state = boot_modes.UEFI + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + boot_mode=initial_state) + self._start_service() + + validate_mock.side_effect = exception.InvalidParameterValue( + 'wrong management driver info') + new_state = boot_modes.LEGACY_BIOS + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.change_node_boot_mode, + self.context, + node.uuid, + new_state) + + self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) + + node.refresh() + validate_mock.assert_called_once_with(mock.ANY, mock.ANY) + self.assertEqual(initial_state, node.boot_mode) + self.assertIsNone(node.last_error) + + @mock.patch.object(fake.FakeManagement, 'set_boot_mode', autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_boot_mode', autospec=True) + def test_change_node_boot_mode_exception_getting_current(self, + get_boot_mock, + set_boot_mock): + # Test change_node_boot_mode smooth opertion when get_boot_mode mode + # raises an exception + initial_state = boot_modes.LEGACY_BIOS + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + boot_mode=initial_state) + self._start_service() + + get_boot_mock.side_effect = exception.UnsupportedDriverExtension( + driver=fake, extension='get_boot_mode') + new_state = boot_modes.UEFI + + self.service.change_node_boot_mode(self.context, + node.uuid, + new_state) + self._stop_service() + + # Call once before setting to see if it is required + self.assertEqual(get_boot_mock.call_count, 1) + set_boot_mock.assert_called_once_with(mock.ANY, mock.ANY, + new_state) + node.refresh() + self.assertEqual(new_state, node.boot_mode) + self.assertIsNone(node.last_error) + # Verify the reservation has been cleared by + # background task's link callback. + self.assertIsNone(node.reservation) + + +@mgr_utils.mock_record_keepalive +class ChangeNodeSecureBootTestCase(mgr_utils.ServiceSetUpMixin, + db_base.DbTestCase): + @mock.patch.object(fake.FakeManagement, 'set_secure_boot_state', + autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_secure_boot_state', + autospec=True) + def test_change_node_secure_boot_valid(self, get_boot_mock, set_boot_mock): + # Test change_node_secure_boot including integration with + # conductor.utils.node_change_secure_boot + get_boot_mock.side_effect = [False, # before setting + True] # after setting + initial_state = False + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + secure_boot=initial_state) + self._start_service() + target_state = True + self.service.change_node_secure_boot(self.context, + node.uuid, + target_state) + self._stop_service() + + set_boot_mock.assert_called_once_with(mock.ANY, mock.ANY, + target_state) + self.assertEqual(get_boot_mock.call_count, 1) + # Call once before setting to see if it's required + + node.refresh() + self.assertEqual(target_state, node.secure_boot) + self.assertIsNone(node.last_error) + # Verify the reservation has been cleared by + # background task's link callback. + self.assertIsNone(node.reservation) + + @mock.patch.object(fake.FakeManagement, 'set_secure_boot_state', + autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_secure_boot_state', + autospec=True) + def test_change_node_secure_boot_existing(self, get_boot_mock, + set_boot_mock): + # Test change_node_secure_boot including integration with + # conductor.utils.node_change_secure_boot when target==current + get_boot_mock.return_value = False + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + secure_boot=False) + self._start_service() + + self.service.change_node_secure_boot(self.context, + node.uuid, + False) + self._stop_service() + + set_boot_mock.assert_not_called() + self.assertEqual(get_boot_mock.call_count, 1) + # Called once before setting to see if it's even required + + node.refresh() + self.assertEqual(False, node.secure_boot) + self.assertIsNone(node.last_error) + # Verify the reservation has been cleared by + # background task's link callback. + self.assertIsNone(node.reservation) + + @mock.patch.object(conductor_utils, 'node_change_secure_boot', + autospec=True) + def test_change_node_secure_boot_node_already_locked(self, ncbm_mock): + # Test change_node_secure_boot with mocked + # conductor.utils.node_change_secure_boot. + fake_reservation = 'fake-reserv' + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + secure_boot=False, + reservation=fake_reservation) + self._start_service() + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.change_node_secure_boot, + self.context, + node.uuid, + False) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeLocked, exc.exc_info[0]) + + # In this test worker should not be spawned, but waiting to make sure + # the below perform_mock assertion is valid. + self._stop_service() + self.assertFalse(ncbm_mock.called, 'node_change_secure_boot has been ' + 'unexpectedly called.') + # Verify existing reservation wasn't broken. + node.refresh() + self.assertEqual(fake_reservation, node.reservation) + + def test_change_node_secure_boot_worker_pool_full(self): + # Test change_node_secure_boot including integration with + # conductor.utils.change_node_secure_boot. + initial_state = False + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + secure_boot=initial_state) + self._start_service() + + with mock.patch.object(self.service, + '_spawn_worker', autospec=True) as spawn_mock: + spawn_mock.side_effect = exception.NoFreeConductorWorker() + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.change_node_secure_boot, + self.context, + node.uuid, + True) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NoFreeConductorWorker, exc.exc_info[0]) + + spawn_mock.assert_called_once_with(mock.ANY, mock.ANY, + mock.ANY) + node.refresh() + self.assertEqual(initial_state, node.secure_boot) + self.assertIsNotNone(node.last_error) + # Verify the picked reservation has been cleared due to full pool. + self.assertIsNone(node.reservation) + + @mock.patch.object(fake.FakeManagement, 'set_secure_boot_state', + autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_secure_boot_state', + autospec=True) + def test_change_node_secure_boot_exception_in_background_task( + self, get_boot_mock, set_boot_mock): + # Test change_node_secure_boot including integration with + # conductor.utils.node_change_secure_boot. + initial_state = False + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + secure_boot=initial_state) + self._start_service() + + get_boot_mock.return_value = False + new_state = True + set_boot_mock.side_effect = exception.UnsupportedDriverExtension( + driver=fake, extension='set_secure_boot_state') + + self.service.change_node_secure_boot(self.context, + node.uuid, + new_state) + self._stop_service() + + # Call once before setting to see if it was required + self.assertEqual(get_boot_mock.call_count, 1) + set_boot_mock.assert_called_once_with(mock.ANY, mock.ANY, + new_state) + node.refresh() + self.assertEqual(initial_state, node.secure_boot) + self.assertIsNotNone(node.last_error) + # Verify the reservation has been cleared by background task's + # link callback despite exception in background task. + self.assertIsNone(node.reservation) + + @mock.patch.object(fake.FakeManagement, 'validate', autospec=True) + def test_change_node_secure_boot_validate_fail(self, validate_mock): + # Test change_node_power_state where task.driver.management.validate + # fails + initial_state = True + node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + secure_boot=initial_state) + self._start_service() + + validate_mock.side_effect = exception.InvalidParameterValue( + 'wrong management driver info') + new_state = False + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.change_node_secure_boot, + self.context, + node.uuid, + new_state) + + self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) + + node.refresh() + validate_mock.assert_called_once_with(mock.ANY, mock.ANY) + self.assertEqual(initial_state, node.secure_boot) + self.assertIsNone(node.last_error) + + @mock.patch.object(fake.FakeManagement, 'set_secure_boot_state', + autospec=True) + @mock.patch.object(fake.FakeManagement, 'get_secure_boot_state', + autospec=True) + def test_change_node_secure_boot_exception_getting_current(self, + get_boot_mock, + set_boot_mock): + # Test change_node_secure_boot smooth opertion when + # get_secure_boot_state raises an exception + initial_state = False + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + secure_boot=initial_state) + self._start_service() + + get_boot_mock.side_effect = exception.UnsupportedDriverExtension( + driver=fake, extension='get_secure_boot_state') + new_state = True + + self.service.change_node_secure_boot(self.context, + node.uuid, + new_state) + self._stop_service() + + # Call once before setting to see if it is required + self.assertEqual(get_boot_mock.call_count, 1) + set_boot_mock.assert_called_once_with(mock.ANY, mock.ANY, + new_state) + node.refresh() + self.assertEqual(new_state, node.secure_boot) + self.assertIsNone(node.last_error) + # Verify the reservation has been cleared by + # background task's link callback. + self.assertIsNone(node.reservation) + + @mgr_utils.mock_record_keepalive class CreateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): def test_create_node(self): diff --git a/ironic/tests/unit/conductor/test_rpcapi.py b/ironic/tests/unit/conductor/test_rpcapi.py index 0c7763e837..314199498e 100644 --- a/ironic/tests/unit/conductor/test_rpcapi.py +++ b/ironic/tests/unit/conductor/test_rpcapi.py @@ -26,6 +26,7 @@ import oslo_messaging as messaging from oslo_messaging import _utils as messaging_utils from ironic.common import boot_devices +from ironic.common import boot_modes from ironic.common import components from ironic.common import exception from ironic.common import indicator_states @@ -301,6 +302,20 @@ class RPCAPITestCase(db_base.DbTestCase): node_id=self.fake_node['uuid'], new_state=states.POWER_ON) + def test_change_node_boot_mode(self): + self._test_rpcapi('change_node_boot_mode', + 'call', + version='1.55', + node_id=self.fake_node['uuid'], + new_state=boot_modes.LEGACY_BIOS) + + def test_change_node_secure_boot(self): + self._test_rpcapi('change_node_secure_boot', + 'call', + version='1.55', + node_id=self.fake_node['uuid'], + new_state=True) + def test_vendor_passthru(self): self._test_rpcapi('vendor_passthru', 'call', diff --git a/releasenotes/notes/node-boot-mode-0662effa2a2644dc.yaml b/releasenotes/notes/node-boot-mode-0662effa2a2644dc.yaml index 2832fd0443..4f01fa643b 100644 --- a/releasenotes/notes/node-boot-mode-0662effa2a2644dc.yaml +++ b/releasenotes/notes/node-boot-mode-0662effa2a2644dc.yaml @@ -7,4 +7,4 @@ features: If underlying driver does not support detecting these, they shall be populated with null values. These fields are also available under a node's states endpoint: - * ``/v1/nodes/{node_ident}/states`` + ``/v1/nodes/{node_ident}/states`` diff --git a/releasenotes/notes/node-boot-mode-change-api-c5e392e3cd6ea54b.yaml b/releasenotes/notes/node-boot-mode-change-api-c5e392e3cd6ea54b.yaml new file mode 100644 index 0000000000..824e408082 --- /dev/null +++ b/releasenotes/notes/node-boot-mode-change-api-c5e392e3cd6ea54b.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + Adds endpoints to change boot mode and secure boot state of node. + + * ``PUT /v1/nodes/{node_ident}/states/boot_mode`` + * ``PUT /v1/nodes/{node_ident}/states/secure_boot`` + + The API will respond with 202 (Accepted) on validating the request + and accepting to process it. Changes occur asynchronously in a + background task. The user can then poll the states endpoint + ``/v1/nodes/{node_ident}/states`` for observing current status of the + requested change.