From 46ff51487accf8881e468b975f4479c85daa8163 Mon Sep 17 00:00:00 2001 From: Cenne Date: Fri, 18 Jun 2021 12:20:40 +0200 Subject: [PATCH] Add `boot_mode` and `secure_boot` to node object and expose in api * add fields to Node object * expose them at endpoint `/v1/nodes/{node_ident}/states` * update states on powersync / entering managed state. * tests * update api endpoint info in api-ref Story: 2008567 Task: 41709 Change-Id: Iddd1421a6fa37d69da56658a2fefa5bc8cfd15e4 --- .../baremetal-api-v1-node-management.inc | 7 +- api-ref/source/parameters.yaml | 10 + .../samples/node-get-state-response.json | 2 + .../contributor/webapi-version-history.rst | 10 + ironic/api/controllers/v1/node.py | 6 + ironic/api/controllers/v1/utils.py | 2 + ironic/api/controllers/v1/versions.py | 4 +- ironic/common/release_mappings.py | 4 +- ironic/conductor/manager.py | 4 + ironic/conductor/utils.py | 44 +++++ ...46a214450_add_boot_mode_and_secure_boot.py | 32 +++ ironic/db/sqlalchemy/models.py | 3 + ironic/drivers/modules/boot_mode_utils.py | 3 + ironic/objects/node.py | 29 ++- .../unit/api/controllers/v1/test_node.py | 80 ++++++++ ironic/tests/unit/conductor/test_manager.py | 4 + ironic/tests/unit/conductor/test_utils.py | 186 ++++++++++++++++++ .../unit/db/sqlalchemy/test_migrations.py | 12 ++ ironic/tests/unit/db/utils.py | 2 + ironic/tests/unit/objects/test_node.py | 77 ++++++++ ironic/tests/unit/objects/test_objects.py | 12 +- .../node-boot-mode-0662effa2a2644dc.yaml | 14 ++ 22 files changed, 530 insertions(+), 17 deletions(-) create mode 100644 ironic/db/sqlalchemy/alembic/versions/c1846a214450_add_boot_mode_and_secure_boot.py create mode 100644 releasenotes/notes/node-boot-mode-0662effa2a2644dc.yaml diff --git a/api-ref/source/baremetal-api-v1-node-management.inc b/api-ref/source/baremetal-api-v1-node-management.inc index 96f3e833a7..0f2f3e1756 100644 --- a/api-ref/source/baremetal-api-v1-node-management.inc +++ b/api-ref/source/baremetal-api-v1-node-management.inc @@ -264,7 +264,10 @@ Node State Summary .. rest_method:: GET /v1/nodes/{node_ident}/states -Get a summary of the Node's current power, provision, raid, and console status. +Get a summary of the Node's current power, provision, boot mode, raid, and console status. + +.. versionadded:: 1.75 + Introduced ``boot_mode`` and ``secure_boot`` fields. Normal response code: 200 @@ -289,6 +292,8 @@ Response - console_enabled: console_enabled - raid_config: raid_config - target_raid_config: target_raid_config + - boot_mode: boot_mode + - secure_boot: secure_boot **Example node state:** diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index ef9b1b9756..65c9d029d6 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -601,6 +601,11 @@ boot_interface: in: body required: true type: string +boot_mode: + description: | + The current boot mode state (uefi/bios) + in: body + type: string candidate_nodes: description: | A list of UUIDs of the nodes that are candidates for this allocation. @@ -1848,6 +1853,11 @@ retired_reason: in: body required: false type: string +secure_boot: + description: | + Indicates whether node is currently booted with secure_boot turned on. + in: body + type: boolean standalone_ports_supported: description: | Indicates whether ports that are members of this portgroup can be diff --git a/api-ref/source/samples/node-get-state-response.json b/api-ref/source/samples/node-get-state-response.json index 009552bebf..8b23c08caa 100644 --- a/api-ref/source/samples/node-get-state-response.json +++ b/api-ref/source/samples/node-get-state-response.json @@ -1,10 +1,12 @@ { + "boot_mode": "uefi", "console_enabled": false, "last_error": null, "power_state": "power off", "provision_state": "available", "provision_updated_at": "2016-08-18T22:28:49.946416+00:00", "raid_config": {}, + "secure_boot": true, "target_power_state": null, "target_provision_state": null, "target_raid_config": {} diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index ed17af161b..325c87e135 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,14 +2,24 @@ REST API Version History ======================== +1.75 (Xena, ?) +---------------------- +Add `boot_mode` and `secure_boot` to node object and expose their state at: + +* ``/v1/nodes/{node_ident}/states`` + 1.74 (Xena, 18.0) ---------------------- Add support for BIOS registry fields which include details about the BIOS setting. Included in the ``/v1/nodes/{node_ident}/bios/{setting}`` response. + Add a new selector to include the fields in the BIOS settings list: + * ``/v1/nodes/{node_ident}/bios/?detail=`` + Also add a fields selector to the the BIOS settings list: + * ``/v1/nodes/{node_ident}/bios/?fields=`` 1.73 (Xena, 18.0) diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index f5fbb22759..8a53c4fbbf 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -141,6 +141,7 @@ def node_schema(): 'automated_clean': {'type': ['string', 'boolean', 'null']}, 'bios_interface': {'type': ['string', 'null']}, 'boot_interface': {'type': ['string', 'null']}, + 'boot_mode': {'type': ['string', 'null']}, 'chassis_uuid': {'type': ['string', 'null']}, 'conductor_group': {'type': ['string', 'null']}, 'console_enabled': {'type': ['string', 'boolean', 'null']}, @@ -172,6 +173,7 @@ def node_schema(): 'resource_class': {'type': ['string', 'null'], 'maxLength': 80}, 'retired': {'type': ['string', 'boolean', 'null']}, 'retired_reason': {'type': ['string', 'null']}, + 'secure_boot': {'type': ['string', 'boolean', 'null']}, 'storage_interface': {'type': ['string', 'null']}, 'uuid': {'type': ['string', 'null']}, 'vendor_interface': {'type': ['string', 'null']}, @@ -694,6 +696,8 @@ def node_states_convert(rpc_node): 'target_provision_state', 'provision_updated_at'] if api_utils.allow_raid_config(): attr_list.extend(['raid_config', 'target_raid_config']) + if api.request.version.minor >= versions.MINOR_75_NODE_BOOT_MODE: + attr_list.extend(['boot_mode', 'secure_boot']) states = {} for attr in attr_list: states[attr] = getattr(rpc_node, attr) @@ -1221,6 +1225,7 @@ def _get_fields_for_node_query(fields=None): valid_fields = ['automated_clean', 'bios_interface', 'boot_interface', + 'boot_mode', 'clean_step', 'conductor_group', 'console_enabled', @@ -1261,6 +1266,7 @@ def _get_fields_for_node_query(fields=None): 'resource_class', 'retired', 'retired_reason', + 'secure_boot', 'storage_interface', 'target_power_state', 'target_provision_state', diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 4a753c4d11..62a57da6c2 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -803,6 +803,8 @@ VERSIONED_FIELDS = { 'retired_reason': versions.MINOR_61_NODE_RETIRED, 'lessee': versions.MINOR_65_NODE_LESSEE, 'network_data': versions.MINOR_66_NODE_NETWORK_DATA, + 'boot_mode': versions.MINOR_75_NODE_BOOT_MODE, + 'secure_boot': versions.MINOR_75_NODE_BOOT_MODE, } for field in V31_FIELDS: diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 677a27475c..1af83c431b 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -112,6 +112,7 @@ BASE_VERSION = 1 # v1.72: Add agent_status and agent_status_message to /v1/heartbeat # 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. MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -188,6 +189,7 @@ MINOR_71_RBAC_SCOPES = 71 MINOR_72_HEARTBEAT_STATUS = 72 MINOR_73_DEPLOY_UNDEPLOY_VERBS = 73 MINOR_74_BIOS_REGISTRY = 74 +MINOR_75_NODE_BOOT_MODE = 75 # When adding another version, update: # - MINOR_MAX_VERSION @@ -195,7 +197,7 @@ MINOR_74_BIOS_REGISTRY = 74 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_74_BIOS_REGISTRY +MINOR_MAX_VERSION = MINOR_75_NODE_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/release_mappings.py b/ironic/common/release_mappings.py index 91fff47143..b0ede5a47b 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -339,12 +339,12 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.74', + 'api': '1.75', 'rpc': '1.54', 'objects': { 'Allocation': ['1.1'], 'BIOSSetting': ['1.1'], - 'Node': ['1.35'], + 'Node': ['1.36', '1.35'], 'Conductor': ['1.3'], 'Chassis': ['1.3'], 'Deployment': ['1.0'], diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index fbe4dfa386..01f340fcbb 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -1173,6 +1173,8 @@ class ConductorManager(base_manager.BaseConductorManager): utils.node_cache_bios_settings(task, node) # Cache the vendor if possible utils.node_cache_vendor(task) + # Cache also boot_mode and secure_boot states + utils.node_cache_boot_mode(task) if power_state != node.power_state: old_power_state = node.power_state @@ -3586,6 +3588,8 @@ def do_sync_power_state(task, count): # Make sure we have the vendor cached (if for some reason it failed during # the transition to manageable or a really old API version was used). utils.node_cache_vendor(task) + # Also make sure to cache the current boot_mode and secure_boot states + utils.node_cache_boot_mode(task) if node.power_state and node.power_state == power_state: # No action is needed diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index cbadd091c5..d0ffb869f8 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -1428,3 +1428,47 @@ def node_cache_vendor(task): task.node.save() LOG.info("Detected vendor %(vendor)s for node %(node)s", {'vendor': vendor, 'node': task.node.uuid}) + + +def node_cache_boot_mode(task): + """Cache boot_mode and secure_boot state if supported by driver. + + Cache current boot_mode and secure_boot in ironic's node representation + + :param task: a TaskManager instance containing the node to check. + """ + # Try to retrieve boot mode and secure_boot state + try: + boot_mode = task.driver.management.get_boot_mode(task) + except exception.UnsupportedDriverExtension: + boot_mode = None + except Exception as exc: + LOG.warning('Unexpected exception when trying to detect 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)) + return + try: + secure_boot = task.driver.management.get_secure_boot_state(task) + except exception.UnsupportedDriverExtension: + secure_boot = None + except Exception as exc: + LOG.warning('Unexpected exception when trying to detect secure_boot ' + 'state 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)) + return + + if (boot_mode != task.node.boot_mode + or secure_boot != task.node.secure_boot): + # Update node if current values different from node's last known info. + # Get exclusive lock in case we don't have one already. + task.upgrade_lock(purpose='caching boot_mode or secure_boot state') + task.node.boot_mode = boot_mode + task.node.secure_boot = secure_boot + task.node.save() + LOG.info("Updated boot_mode %(boot_mode)s, secure_boot %(secure_boot)s" + "for node %(node)s", + {'boot_mode': boot_mode, 'secure_boot': secure_boot}) diff --git a/ironic/db/sqlalchemy/alembic/versions/c1846a214450_add_boot_mode_and_secure_boot.py b/ironic/db/sqlalchemy/alembic/versions/c1846a214450_add_boot_mode_and_secure_boot.py new file mode 100644 index 0000000000..58e7f31be7 --- /dev/null +++ b/ironic/db/sqlalchemy/alembic/versions/c1846a214450_add_boot_mode_and_secure_boot.py @@ -0,0 +1,32 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +"""add boot_mode and secure_boot + +Revision ID: c1846a214450 +Revises: 2bbd96b6ccb9 +Create Date: 2021-06-21 15:57:37.330442 + +""" +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'c1846a214450' +down_revision = '2bbd96b6ccb9' + + +def upgrade(): + op.add_column('nodes', sa.Column('boot_mode', + sa.String(length=16), nullable=True)) + op.add_column('nodes', sa.Column('secure_boot', + sa.Boolean(), nullable=True)) diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index 96c18af260..6a1c73d62d 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -209,6 +209,9 @@ class Node(Base): power_interface = Column(String(255), nullable=True) vendor_interface = Column(String(255), nullable=True) + boot_mode = Column(String(16), nullable=True) + secure_boot = Column(Boolean, nullable=True) + class Port(Base): """Represents a network port of a bare metal node.""" diff --git a/ironic/drivers/modules/boot_mode_utils.py b/ironic/drivers/modules/boot_mode_utils.py index 9ddf648495..b88e6d3db3 100644 --- a/ironic/drivers/modules/boot_mode_utils.py +++ b/ironic/drivers/modules/boot_mode_utils.py @@ -68,6 +68,7 @@ def _set_boot_mode_on_bm(task, ironic_boot_mode, fail_if_unsupported=False): LOG.info("Baremetal node boot mode is set to boot " "mode %(boot_mode)s", {'uuid': task.node.uuid, 'boot_mode': ironic_boot_mode}) + manager_utils.node_cache_boot_mode(task) def sync_boot_mode(task): @@ -331,6 +332,7 @@ def configure_secure_boot_if_needed(task): exc_info=not isinstance(exc, exception.IronicException)) else: LOG.info('Secure boot has been enabled for node %s', task.node.uuid) + manager_utils.node_cache_boot_mode(task) @task_manager.require_exclusive_lock @@ -356,3 +358,4 @@ def deconfigure_secure_boot_if_needed(task): exc_info=not isinstance(exc, exception.IronicException)) else: LOG.info('Secure boot has been disabled for node %s', task.node.uuid) + manager_utils.node_cache_boot_mode(task) diff --git a/ironic/objects/node.py b/ironic/objects/node.py index bcd34abdef..d8a919f2cc 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -76,7 +76,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): # Version 1.33: Add retired and retired_reason fields # Version 1.34: Add lessee field # Version 1.35: Add network_data field - VERSION = '1.35' + # Version 1.36: Add boot_mode and secure_boot fields + VERSION = '1.36' dbapi = db_api.get_instance() @@ -166,6 +167,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): 'retired': objects.fields.BooleanField(nullable=True), 'retired_reason': object_fields.StringField(nullable=True), 'network_data': object_fields.FlexibleDictField(nullable=True), + 'boot_mode': object_fields.StringField(nullable=True), + 'secure_boot': object_fields.BooleanField(nullable=True), } def as_dict(self, secure=False, mask_configdrive=True): @@ -644,6 +647,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): be set to None or removed. Version 1.35: network_data was added. For versions prior to this, it should be set to empty dict (or removed). + Version 1.36: boot_mode, secure_boot were was added. Defaults are None. + For versions prior to this, it should be set to None or removed. :param target_version: the desired version of the object :param remove_unavailable_fields: True to remove fields that are @@ -658,7 +663,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): ('bios_interface', 24), ('fault', 25), ('automated_clean', 28), ('protected_reason', 29), ('owner', 30), ('allocation_id', 31), ('description', 32), - ('retired_reason', 33), ('lessee', 34)] + ('retired_reason', 33), ('lessee', 34), ('boot_mode', 36), + ('secure_boot', 36)] for name, minor in fields: self._adjust_field_to_version(name, None, target_version, @@ -700,6 +706,8 @@ class NodePayload(notification.NotificationPayloadBase): 'description': ('node', 'description'), 'driver': ('node', 'driver'), 'extra': ('node', 'extra'), + 'boot_mode': ('node', 'boot_mode'), + 'secure_boot': ('node', 'secure_boot'), 'inspection_finished_at': ('node', 'inspection_finished_at'), 'inspection_started_at': ('node', 'inspection_started_at'), 'instance_uuid': ('node', 'instance_uuid'), @@ -754,7 +762,8 @@ class NodePayload(notification.NotificationPayloadBase): # Version 1.13: Add description field. # Version 1.14: Add retired and retired_reason fields exposed via API. # Version 1.15: Add node lessee field. - VERSION = '1.15' + # Version 1.16: Add boot_mode and secure_boot fields. + VERSION = '1.16' fields = { 'clean_step': object_fields.FlexibleDictField(nullable=True), 'conductor_group': object_fields.StringField(nullable=True), @@ -764,6 +773,8 @@ class NodePayload(notification.NotificationPayloadBase): 'description': object_fields.StringField(nullable=True), 'driver': object_fields.StringField(nullable=True), 'extra': object_fields.FlexibleDictField(nullable=True), + 'boot_mode': object_fields.StringField(nullable=True), + 'secure_boot': object_fields.BooleanField(nullable=True), 'inspection_finished_at': object_fields.DateTimeField(nullable=True), 'inspection_started_at': object_fields.DateTimeField(nullable=True), 'instance_uuid': object_fields.UUIDField(nullable=True), @@ -843,7 +854,8 @@ class NodeSetPowerStatePayload(NodePayload): # Version 1.13: Parent NodePayload version 1.13 # Version 1.14: Parent NodePayload version 1.14 # Version 1.15: Parent NodePayload version 1.15 - VERSION = '1.15' + # Version 1.16: Parent NodePayload version 1.16 + VERSION = '1.16' fields = { # "to_power" indicates the future target_power_state of the node. A @@ -899,7 +911,8 @@ class NodeCorrectedPowerStatePayload(NodePayload): # Version 1.13: Parent NodePayload version 1.13 # Version 1.14: Parent NodePayload version 1.14 # Version 1.15: Parent NodePayload version 1.15 - VERSION = '1.15' + # Version 1.16: Parent NodePayload version 1.16 + VERSION = '1.16' fields = { 'from_power': object_fields.StringField(nullable=True) @@ -940,7 +953,8 @@ class NodeSetProvisionStatePayload(NodePayload): # Version 1.14: Parent NodePayload version 1.14 # Version 1.15: Parent NodePayload version 1.15 # Version 1.16: add driver_internal_info - VERSION = '1.16' + # Version 1.17: Parent NodePayload version 1.16 + VERSION = '1.17' SCHEMA = dict(NodePayload.SCHEMA, **{'instance_info': ('node', 'instance_info'), @@ -989,7 +1003,8 @@ class NodeCRUDPayload(NodePayload): # Version 1.11: Parent NodePayload version 1.13 # Version 1.12: Parent NodePayload version 1.14 # Version 1.13: Parent NodePayload version 1.15 - VERSION = '1.13' + # Version 1.14: Parent NodePayload version 1.16 + VERSION = '1.14' SCHEMA = dict(NodePayload.SCHEMA, **{'instance_info': ('node', 'instance_info'), diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 775d4b97a5..0ae598026c 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -471,6 +471,42 @@ class TestListNodes(test_api_base.BaseApiTest): headers={api_base.Version.string: '1.66'}) self.assertEqual(data['network_data'], NETWORK_DATA) + def test_node_boot_mode_hidden_in_lower_version(self): + self._test_node_field_hidden_in_lower_version('boot_mode', + '1.74', '1.75') + + def test_node_secure_boot_hidden_in_lower_version(self): + self._test_node_field_hidden_in_lower_version('secure_boot', + '1.74', '1.75') + + def test_node_boot_mode_null_field(self): + node = obj_utils.create_test_node(self.context) + data = self.get_json('/nodes/%s' % node.uuid, + headers={api_base.Version.string: '1.75'}) + self.assertIsNone(data['boot_mode']) + self.assertIsNone(data['secure_boot']) + + def test_node_boot_mode(self): + for value in ('bios', 'uefi'): + node = obj_utils.create_test_node(self.context, + boot_mode=value, + uuid=uuidutils.generate_uuid()) + data = self.get_json('/nodes/%s' % node.uuid, + headers={api_base.Version.string: '1.75'}) + self.assertEqual(data['boot_mode'], value) + self.assertIsNone(data['secure_boot']) + + def test_node_secure_boot(self): + for value in (True, False): + node = obj_utils.create_test_node(self.context, + boot_mode='uefi', + secure_boot=value, + uuid=uuidutils.generate_uuid()) + data = self.get_json('/nodes/%s' % node.uuid, + headers={api_base.Version.string: '1.75'}) + self.assertEqual(data['boot_mode'], 'uefi') + self.assertEqual(data['secure_boot'], value) + def test_get_one_custom_fields(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) @@ -1699,6 +1735,50 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertEqual(fake_error, data['last_error']) self.assertFalse(data['console_enabled']) + def test_node_states_boot_mode(self): + for value in ('bios', 'uefi'): + node = obj_utils.create_test_node(self.context, + boot_mode=value, + uuid=uuidutils.generate_uuid()) + data = self.get_json('/nodes/%s/states' % node.uuid, + headers={api_base.Version.string: '1.75'}) + self.assertEqual(data['boot_mode'], value) + self.assertIsNone(data['secure_boot']) + + def test_node_states_secure_boot(self): + for value in (True, False): + node = obj_utils.create_test_node(self.context, + boot_mode='uefi', + secure_boot=value, + uuid=uuidutils.generate_uuid()) + data = self.get_json('/nodes/%s/states' % node.uuid, + headers={api_base.Version.string: '1.75'}) + self.assertEqual(data['boot_mode'], 'uefi') + self.assertEqual(data['secure_boot'], value) + + def _test_node_states_subfield_hidden_in_lower_version(self, field, + old_version, + new_version): + node = obj_utils.create_test_node(self.context) + data = self.get_json( + '/nodes/%s/states' % node.uuid, + headers={api_base.Version.string: old_version}) + self.assertNotIn(field, data) + data = self.get_json( + '/nodes/%s/states' % node.uuid, + headers={api_base.Version.string: new_version}) + self.assertIn(field, data) + + def test_node_states_boot_mode_hidden_in_lower_version(self): + self._test_node_states_subfield_hidden_in_lower_version('boot_mode', + '1.74', + '1.75') + + def test_node_states_secure_boot_hidden_in_lower_version(self): + self._test_node_states_subfield_hidden_in_lower_version('secure_boot', + '1.74', + '1.75') + def test_node_by_instance_uuid(self): node = obj_utils.create_test_node( self.context, diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 3bd12a3fb4..31e8e94bc2 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -4653,6 +4653,10 @@ class ManagerDoSyncPowerStateTestCase(db_base.DbTestCase): self.driver = mock.Mock(spec_set=drivers_base.BareDriver) self.driver.management.detect_vendor.side_effect = \ exception.UnsupportedDriverExtension + self.driver.management.get_boot_mode.side_effect = \ + exception.UnsupportedDriverExtension + self.driver.management.get_secure_boot_state.side_effect = \ + exception.UnsupportedDriverExtension self.power = self.driver.power self.node = obj_utils.create_test_node( self.context, driver='fake-hardware', maintenance=False, diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index c94aff01c1..15bfa768d2 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -2310,6 +2310,192 @@ class CacheVendorTestCase(db_base.DbTestCase): self.assertTrue(mock_log.called) +@mock.patch.object(fake.FakeManagement, 'get_secure_boot_state', + autospec=True) +@mock.patch.object(fake.FakeManagement, 'get_boot_mode', + autospec=True) +class CacheBootModeTestCase(db_base.DbTestCase): + + def setUp(self): + super(CacheBootModeTestCase, self).setUp() + self.node = obj_utils.create_test_node(self.context, + driver='fake-hardware', + properties={}) + + def test_noneness(self, mock_get_boot, mock_get_secure): + mock_get_boot.return_value = None + mock_get_secure.return_value = None + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_boot_mode(task) + mock_get_boot.assert_called_once_with( + task.driver.management, task) + mock_get_secure.assert_called_once_with( + task.driver.management, task) + # If nothing to save, lock needn't be upgraded + self.assertTrue(task.shared) + + self.node.refresh() + self.assertIsNone(self.node.boot_mode) + self.assertIsNone(self.node.secure_boot) + + def test_unsupported(self, mock_get_boot, mock_get_secure): + mock_get_boot.side_effect = exception.UnsupportedDriverExtension + mock_get_secure.side_effect = exception.UnsupportedDriverExtension + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_boot_mode(task) + mock_get_boot.assert_called_once_with( + task.driver.management, task) + mock_get_secure.assert_called_once_with( + task.driver.management, task) + # If nothing to save, lock needn't be upgraded + self.assertTrue(task.shared) + + self.node.refresh() + self.assertIsNone(self.node.boot_mode) + self.assertIsNone(self.node.secure_boot) + + def test_retreive_and_set(self, mock_get_boot, mock_get_secure): + mock_get_boot.return_value = "fake-efi" + mock_get_secure.return_value = True + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_boot_mode(task) + mock_get_boot.assert_called_once_with( + task.driver.management, task) + mock_get_secure.assert_called_once_with( + task.driver.management, task) + # Verify it upgraded lock + self.assertFalse(task.shared) + + self.node.refresh() + self.assertEqual("fake-efi", self.node.boot_mode) + self.assertTrue(self.node.secure_boot) + + def test_already_present(self, mock_get_boot, mock_get_secure): + self.node.boot_mode = "fake-efi" + self.node.secure_boot = True + self.node.save() + + mock_get_boot.return_value = "fake-efi" + mock_get_secure.return_value = True + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_boot_mode(task) + mock_get_boot.assert_called_once_with( + task.driver.management, task) + mock_get_secure.assert_called_once_with( + task.driver.management, task) + # If no changes, lock needn't be upgraded + self.assertTrue(task.shared) + + self.node.refresh() + self.assertEqual("fake-efi", self.node.boot_mode) + self.assertTrue(self.node.secure_boot) + + def test_change_secure_off(self, mock_get_boot, mock_get_secure): + self.node.boot_mode = "fake-efi" + self.node.secure_boot = True + self.node.save() + + mock_get_boot.return_value = "fake-efi" + mock_get_secure.return_value = False + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_boot_mode(task) + mock_get_boot.assert_called_once_with( + task.driver.management, task) + mock_get_secure.assert_called_once_with( + task.driver.management, task) + # Verify it upgraded lock + self.assertFalse(task.shared) + + self.node.refresh() + self.assertEqual("fake-efi", self.node.boot_mode) + self.assertFalse(self.node.secure_boot) + + def test_change_secure_off_to_none(self, mock_get_boot, mock_get_secure): + # Check that False and None are treated as distinct + # Say during a transition from uefi to bios + self.node.boot_mode = "fake-hybrid" + self.node.secure_boot = False + self.node.save() + + mock_get_boot.return_value = "fake-hybrid" + mock_get_secure.return_value = None + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_boot_mode(task) + mock_get_boot.assert_called_once_with( + task.driver.management, task) + mock_get_secure.assert_called_once_with( + task.driver.management, task) + # Verify it upgraded lock + self.assertFalse(task.shared) + + self.node.refresh() + self.assertEqual("fake-hybrid", self.node.boot_mode) + self.assertIsNone(self.node.secure_boot) + + @mock.patch.object(conductor_utils.LOG, 'warning', autospec=True) + def test_failed_boot_mode(self, mock_log, mock_get_boot, mock_get_secure): + self.node.boot_mode = "fake-efi" + self.node.secure_boot = True + self.node.save() + + mock_get_boot.side_effect = RuntimeError + mock_get_secure.return_value = None + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_boot_mode(task) + mock_get_boot.assert_called_once_with( + task.driver.management, task) + # Test that function aborts and doesn't do anything else. + # NOTE(cenne): Do we want to update states to None instead? + self.assertFalse(mock_get_secure.called) + self.assertTrue(task.shared) + + self.assertTrue(mock_log.called) + # Verify no changes + self.node.refresh() + self.assertEqual("fake-efi", self.node.boot_mode) + self.assertTrue(self.node.secure_boot) + + @mock.patch.object(conductor_utils.LOG, 'warning', autospec=True) + def test_failed_secure(self, mock_log, mock_get_boot, mock_get_secure): + self.node.boot_mode = "fake-efi" + self.node.secure_boot = True + self.node.save() + + mock_get_boot.return_value = "fake-efi" + mock_get_secure.side_effect = RuntimeError + + with task_manager.acquire(self.context, self.node.id, + shared=True) as task: + conductor_utils.node_cache_boot_mode(task) + mock_get_boot.assert_called_once_with( + task.driver.management, task) + mock_get_secure.assert_called_once_with( + task.driver.management, task) + # Test that function aborts and doesn't do anything else. + # NOTE(cenne): Do we want to update states to None instead? + self.assertTrue(task.shared) + + self.assertTrue(mock_log.called) + # Verify no changes + self.node.refresh() + self.assertEqual("fake-efi", self.node.boot_mode) + self.assertTrue(self.node.secure_boot) + + class GetConfigDriveImageTestCase(db_base.DbTestCase): def setUp(self): diff --git a/ironic/tests/unit/db/sqlalchemy/test_migrations.py b/ironic/tests/unit/db/sqlalchemy/test_migrations.py index 3b81e3b92a..0381107e9c 100644 --- a/ironic/tests/unit/db/sqlalchemy/test_migrations.py +++ b/ironic/tests/unit/db/sqlalchemy/test_migrations.py @@ -1010,6 +1010,18 @@ class MigrationCheckersMixin(object): self.assertIsInstance( nodes.c.network_data.type, sqlalchemy.types.String) + def _check_c1846a214450(self, engine, data): + nodes = db_utils.get_table(engine, 'nodes') + col_names = [column.name for column in nodes.c] + self.assertIn('boot_mode', col_names) + self.assertIn('secure_boot', col_names) + self.assertIsInstance(nodes.c.boot_mode.type, + sqlalchemy.types.String) + # in some backends bool type is integer + self.assertIsInstance(nodes.c.secure_boot.type, + (sqlalchemy.types.Boolean, + sqlalchemy.types.Integer)) + def _pre_upgrade_cd2c80feb331(self, engine): data = { 'node_uuid': uuidutils.generate_uuid(), diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index cb8e553386..c0b060eef8 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -233,6 +233,8 @@ def get_test_node(**kw): 'retired_reason': kw.get('retired_reason', None), 'lessee': kw.get('lessee', None), 'network_data': kw.get('network_data'), + 'boot_mode': kw.get('boot_mode', None), + 'secure_boot': kw.get('secure_boot', None), } for iface in drivers_base.ALL_INTERFACES: diff --git a/ironic/tests/unit/objects/test_node.py b/ironic/tests/unit/objects/test_node.py index 313495161c..06996c5914 100644 --- a/ironic/tests/unit/objects/test_node.py +++ b/ironic/tests/unit/objects/test_node.py @@ -1292,6 +1292,81 @@ class TestConvertToVersion(db_base.DbTestCase): self.assertIsNone(node.lessee) self.assertEqual({}, node.obj_get_changes()) + def test_boot_mode_supported_missing(self): + # boot_mode and secure_boot not set, should be set to default. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + delattr(node, 'boot_mode') + delattr(node, 'secure_boot') + node.obj_reset_changes() + node._convert_to_version("1.36") + self.assertIsNone(node.boot_mode) + self.assertIsNone(node.secure_boot) + self.assertEqual({'boot_mode': None, + 'secure_boot': None}, + node.obj_get_changes()) + + def test_boot_mode_supported_set(self): + # boot_mode and secure_boot set, no change required. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.boot_mode = "uefi" + node.secure_boot = True + node.obj_reset_changes() + node._convert_to_version("1.36") + self.assertEqual("uefi", node.boot_mode) + self.assertTrue(node.secure_boot) + self.assertEqual({}, node.obj_get_changes()) + + def test_boot_mode_unsupported_missing(self): + # boot_mode and secure_boot not set, no change required. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + delattr(node, 'boot_mode') + delattr(node, 'secure_boot') + node.obj_reset_changes() + node._convert_to_version("1.35") + self.assertNotIn('boot_mode', node) + self.assertNotIn('secure_boot', node) + self.assertEqual({}, node.obj_get_changes()) + + def test_boot_mode_unsupported_set_remove(self): + # boot_mode and secure_boot set, should be removed. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.boot_mode = "uefi" + node.secure_boot = True + node.obj_reset_changes() + node._convert_to_version("1.35") + self.assertNotIn('boot_mode', node) + self.assertNotIn('secure_boot', node) + self.assertEqual({}, node.obj_get_changes()) + + def test_boot_mode_unsupported_set_no_remove_non_default(self): + # boot_mode and secure_boot set, should be set to default. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.boot_mode = "uefi" + node.secure_boot = True + node.obj_reset_changes() + node._convert_to_version("1.35", False) + self.assertIsNone(node.boot_mode) + self.assertIsNone(node.secure_boot) + self.assertEqual({'boot_mode': None, + 'secure_boot': None}, + node.obj_get_changes()) + + def test_boot_mode_unsupported_set_no_remove_default(self): + # boot_mode and secure_boot set, no change required. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.boot_mode = None + node.secure_boot = None + node.obj_reset_changes() + node._convert_to_version("1.35", False) + self.assertIsNone(node.boot_mode) + self.assertIsNone(node.secure_boot) + self.assertEqual({}, node.obj_get_changes()) + class TestNodePayloads(db_base.DbTestCase): @@ -1308,6 +1383,8 @@ class TestNodePayloads(db_base.DbTestCase): self.assertEqual(self.node.created_at, payload.created_at) self.assertEqual(self.node.driver, payload.driver) self.assertEqual(self.node.extra, payload.extra) + self.assertEqual(self.node.boot_mode, payload.boot_mode) + self.assertEqual(self.node.secure_boot, payload.secure_boot) self.assertEqual(self.node.inspection_finished_at, payload.inspection_finished_at) self.assertEqual(self.node.inspection_started_at, diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 4e38e123da..7eefb3c59e 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -676,7 +676,7 @@ class TestObject(_LocalTest, _TestObject): # version bump. It is an MD5 hash of the object fields and remotable methods. # The fingerprint values should only be changed if there is a version bump. expected_object_fingerprints = { - 'Node': '1.35-aee8ecf5c4d0ed590eb484762aee7fca', + 'Node': '1.36-8a080e31ba89ca5f09e859bd259b54dc', 'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6', 'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905', 'Port': '1.10-67381b065c597c8d3a13c5dbc6243c33', @@ -684,21 +684,21 @@ expected_object_fingerprints = { 'Conductor': '1.3-d3f53e853b4d58cae5bfbd9a8341af4a', 'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370', 'NotificationPublisher': '1.0-51a09397d6c0687771fb5be9a999605d', - 'NodePayload': '1.15-86ee30dbf374be4cf17c5b501d9e2e7b', + 'NodePayload': '1.16-9298b3aba63ab2b9c3359afd90fb9230', 'NodeSetPowerStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeSetPowerStatePayload': '1.15-3c64b07a2b96c2661e7743b47ed43705', + 'NodeSetPowerStatePayload': '1.16-d3695780185716e75683ebbba4f8a2e6', 'NodeCorrectedPowerStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeCorrectedPowerStatePayload': '1.15-59a224a9191cdc9f1acc2e0dcd2d3adb', + 'NodeCorrectedPowerStatePayload': '1.16-fdf636b04ba0827ee0c5ec20730b790d', 'NodeSetProvisionStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeSetProvisionStatePayload': '1.16-c5a8eea43c514baf721fc61ce5d9d5a4', + 'NodeSetProvisionStatePayload': '1.17-4efa07190b276f52fda09d846b4690a8', 'VolumeConnector': '1.0-3e0252c0ab6e6b9d158d09238a577d97', 'VolumeTarget': '1.0-0b10d663d8dae675900b2c7548f76f5e', 'ChassisCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', 'ChassisCRUDPayload': '1.0-dce63895d8186279a7dd577cffccb202', 'NodeCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeCRUDPayload': '1.13-8f673253ff8d7389897a6a80d224ac33', + 'NodeCRUDPayload': '1.14-abe3a744767e5ada9f8370cf0caa1862', 'PortCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', 'PortCRUDPayload': '1.4-9411a1701077ae9dc0aea27d6bf586fc', 'NodeMaintenanceNotification': '1.0-59acc533c11d306f149846f922739c15', diff --git a/releasenotes/notes/node-boot-mode-0662effa2a2644dc.yaml b/releasenotes/notes/node-boot-mode-0662effa2a2644dc.yaml new file mode 100644 index 0000000000..caf092144e --- /dev/null +++ b/releasenotes/notes/node-boot-mode-0662effa2a2644dc.yaml @@ -0,0 +1,14 @@ +--- + +features: + + - | + Adds ``boot_mode`` and ``secure_boot`` fields to node. These indicate the + boot mode (bios/uefi) and secure boot state (True/False) detected in the + most recent power sync or during transition to the ``manageable`` state. + If underlying driver does not support detecting these, they shall be + populated with null values. + + These fields are also be available under a node's states endpoint: + + * ``/v1/nodes/{node_ident}/states``