diff --git a/api-ref/source/baremetal-api-v1-nodes.inc b/api-ref/source/baremetal-api-v1-nodes.inc index 49ac3fe230..28e345be16 100644 --- a/api-ref/source/baremetal-api-v1-nodes.inc +++ b/api-ref/source/baremetal-api-v1-nodes.inc @@ -139,7 +139,7 @@ and any defaults added for non-specified fields. Most fields default to "null" or "". The list and example below are representative of the response as of API -microversion 1.46. +microversion 1.48. .. rest_parameters:: parameters.yaml @@ -186,6 +186,8 @@ microversion 1.46. - vendor_interface: vendor_interface - volume: n_volume - conductor_group: conductor_group + - protected: protected + - protected_reason: protected_reason **Example JSON representation of a Node:** @@ -302,6 +304,9 @@ Nova instance, eg. with a request to ``v1/nodes/detail?instance_uuid={NOVA INSTA .. versionadded:: 1.46 Introduced the ``conductor_group`` field. +.. versionadded:: 1.48 + Introduced the ``protected`` and ``protected_reason`` fields. + Normal response codes: 200 Error codes: 400,403,406 @@ -372,6 +377,8 @@ Response - vendor_interface: vendor_interface - volume: n_volume - conductor_group: conductor_group + - protected: protected + - protected_reason: protected_reason **Example detailed list of Nodes:** @@ -400,6 +407,9 @@ only the specified set. .. versionadded:: 1.46 Introduced the ``conductor_group`` field. +.. versionadded:: 1.48 + Introduced the ``protected`` and ``protected_reason`` fields. + Normal response codes: 200 Error codes: 400,403,404,406 @@ -460,6 +470,8 @@ Response - vendor_interface: vendor_interface - volume: n_volume - conductor_group: conductor_group + - protected: protected + - protected_reason: protected_reason **Example JSON representation of a Node:** @@ -546,6 +558,8 @@ Response - vendor_interface: vendor_interface - volume: n_volume - conductor_group: conductor_group + - protected: protected + - protected_reason: protected_reason **Example JSON representation of a Node:** diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 5808cebb6a..bcf8e190fe 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -987,6 +987,18 @@ properties: in: body required: true type: array +protected: + description: | + Whether the node is protected from undeploying, rebuilding and deletion. + in: body + required: false + type: boolean +protected_reason: + description: | + The reason the node is marked as protected. + in: body + required: false + type: string provision_state: description: | The current provisioning state of this Node. diff --git a/api-ref/source/samples/node-create-response.json b/api-ref/source/samples/node-create-response.json index ca165c2238..33d075a10a 100644 --- a/api-ref/source/samples/node-create-response.json +++ b/api-ref/source/samples/node-create-response.json @@ -59,6 +59,8 @@ "power_interface": null, "power_state": null, "properties": {}, + "protected": false, + "protected_reason": null, "provision_state": "enroll", "provision_updated_at": null, "raid_config": {}, diff --git a/api-ref/source/samples/node-show-response.json b/api-ref/source/samples/node-show-response.json index 2ecf7b4c61..8e0078c8de 100644 --- a/api-ref/source/samples/node-show-response.json +++ b/api-ref/source/samples/node-show-response.json @@ -61,6 +61,8 @@ "power_interface": null, "power_state": "power off", "properties": {}, + "protected": false, + "protected_reason": null, "provision_state": "available", "provision_updated_at": "2016-08-18T22:28:49.946416+00:00", "raid_config": {}, diff --git a/api-ref/source/samples/node-update-driver-info-response.json b/api-ref/source/samples/node-update-driver-info-response.json index 53ec73fd23..420772242a 100644 --- a/api-ref/source/samples/node-update-driver-info-response.json +++ b/api-ref/source/samples/node-update-driver-info-response.json @@ -63,6 +63,8 @@ "power_interface": null, "power_state": "power off", "properties": {}, + "protected": false, + "protected_reason": null, "provision_state": "available", "provision_updated_at": "2016-08-18T22:28:49.946416+00:00", "raid_config": {}, diff --git a/api-ref/source/samples/nodes-list-details-response.json b/api-ref/source/samples/nodes-list-details-response.json index f0fc4d8e81..a6ba4480d3 100644 --- a/api-ref/source/samples/nodes-list-details-response.json +++ b/api-ref/source/samples/nodes-list-details-response.json @@ -63,6 +63,8 @@ "power_interface": null, "power_state": "power off", "properties": {}, + "protected": false, + "protected_reason": null, "provision_state": "available", "provision_updated_at": "2016-08-18T22:28:49.946416+00:00", "raid_config": {}, @@ -160,6 +162,8 @@ "power_interface": "ipmitool", "power_state": null, "properties": {}, + "protected": false, + "protected_reason": null, "provision_state": "enroll", "provision_updated_at": null, "raid_config": {}, diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index c313bee163..faefd50ab5 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,13 @@ REST API Version History ======================== +1.48 (Stein, master) +-------------------- + +Added ``protected`` field to the node object to allow protecting deployed nodes +from undeploying, rebuilding or deletion. Also added ``protected_reason`` +to specify the reason of making the node protected. + 1.47 (Stein, master) -------------------- diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 06d6dcef9c..933a33122a 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1066,6 +1066,12 @@ class Node(base.APIBase): automated_clean = types.boolean """Indicates whether the node will perform automated clean or not.""" + protected = types.boolean + """Indicates whether the node is protected from undeploying/rebuilding.""" + + protected_reason = wsme.wsattr(wtypes.text) + """Indicates reason for protecting the node.""" + # NOTE(deva): "conductor_affinity" shouldn't be presented on the # API because it's an internal value. Don't add it here. @@ -1253,7 +1259,8 @@ class Node(base.APIBase): raid_interface=None, vendor_interface=None, storage_interface=None, traits=[], rescue_interface=None, bios_interface=None, conductor_group="", - automated_clean=None) + automated_clean=None, protected=False, + protected_reason=None) # NOTE(matty_dubs): The chassis_uuid getter() is based on the # _chassis_uuid variable: sample._chassis_uuid = 'edcad704-b2da-41d5-96d9-afd580ecfa12' @@ -1913,6 +1920,12 @@ class NodesController(rest.RestController): "be set via the node traits API.") raise exception.Invalid(msg) + if (node.protected is not wtypes.Unset or + node.protected_reason is not wtypes.Unset): + msg = _("Cannot specify protected or protected_reason on node " + "creation. These fields can only be set for active nodes") + raise exception.Invalid(msg) + # NOTE(deva): get_topic_for checks if node.driver is in the hash ring # and raises NoValidHost if it is not. # We need to ensure that node has a UUID before it can diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 7fc9c0ee7b..73c84a2469 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -376,6 +376,8 @@ VERSIONED_FIELDS = { 'deploy_step': versions.MINOR_44_NODE_DEPLOY_STEP, 'conductor_group': versions.MINOR_46_NODE_CONDUCTOR_GROUP, 'automated_clean': versions.MINOR_47_NODE_AUTOMATED_CLEAN, + 'protected': versions.MINOR_48_NODE_PROTECTED, + 'protected_reason': versions.MINOR_48_NODE_PROTECTED, } for field in V31_FIELDS: diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index f7c5c02d25..7b6c61c044 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -85,6 +85,7 @@ BASE_VERSION = 1 # v1.45: reset_interfaces parameter to node's PATCH # v1.46: Add conductor_group to the node object. # v1.47: Add automated_clean to the node object. +# v1.48: Add protected to the node object. MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -134,6 +135,7 @@ MINOR_44_NODE_DEPLOY_STEP = 44 MINOR_45_RESET_INTERFACES = 45 MINOR_46_NODE_CONDUCTOR_GROUP = 46 MINOR_47_NODE_AUTOMATED_CLEAN = 47 +MINOR_48_NODE_PROTECTED = 48 # When adding another version, update: # - MINOR_MAX_VERSION @@ -141,7 +143,7 @@ MINOR_47_NODE_AUTOMATED_CLEAN = 47 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_47_NODE_AUTOMATED_CLEAN +MINOR_MAX_VERSION = MINOR_48_NODE_PROTECTED # String representations of the minor and maximum versions _MIN_VERSION_STRING = '{}.{}'.format(BASE_VERSION, MINOR_1_INITIAL_VERSION) diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 90ae3b1ef4..f38caf04c1 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -786,3 +786,8 @@ class DatabaseVersionTooOld(IronicException): class AgentConnectionFailed(IronicException): _msg_fmt = _("Connection to agent failed: %(reason)s") + + +class NodeProtected(HTTPForbidden): + _msg_fmt = _("Node %(node)s is protected and cannot be undeployed, " + "rebuilt or deleted") diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 2cdbe60b45..87806367b3 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -131,10 +131,10 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.47', + 'api': '1.48', 'rpc': '1.47', 'objects': { - 'Node': ['1.28'], + 'Node': ['1.29', '1.28'], 'Conductor': ['1.3'], 'Chassis': ['1.3'], 'Port': ['1.8'], diff --git a/ironic/conductor/manager.py b/ironic/conductor/manager.py index f63450419d..0c14309336 100644 --- a/ironic/conductor/manager.py +++ b/ironic/conductor/manager.py @@ -135,6 +135,24 @@ class ConductorManager(base_manager.BaseConductorManager): node_obj.create() return node_obj + def _check_update_protected(self, node_obj, delta): + if 'protected' in delta: + if not node_obj.protected: + node_obj.protected_reason = None + elif node_obj.provision_state not in (states.ACTIVE, + states.RESCUE): + raise exception.InvalidState( + "Node %(node)s can only be made protected in provision " + "states 'active' or 'rescue', the current state is " + "'%(state)s'" % + {'node': node_obj.uuid, 'state': node_obj.provision_state}) + + if ('protected_reason' in delta and node_obj.protected_reason and not + node_obj.protected): + raise exception.InvalidParameterValue( + "The protected_reason field can only be set when " + "protected is True") + @METRICS.timer('ConductorManager.update_node') # No need to add these since they are subclasses of InvalidParameterValue: # InterfaceNotFoundInEntrypoint @@ -170,6 +188,8 @@ class ConductorManager(base_manager.BaseConductorManager): node_obj.maintenance_reason = None node_obj.fault = None + self._check_update_protected(node_obj, delta) + # TODO(dtantsur): reconsider allowing changing some (but not all) # interfaces for active nodes in the future. # NOTE(kaifeng): INSPECTING is allowed to keep backwards @@ -736,7 +756,8 @@ class ConductorManager(base_manager.BaseConductorManager): exception.NodeLocked, exception.NodeInMaintenance, exception.InstanceDeployFailure, - exception.InvalidStateRequested) + exception.InvalidStateRequested, + exception.NodeProtected) def do_node_deploy(self, context, node_id, rebuild=False, configdrive=None): """RPC method to initiate deployment to a node. @@ -758,7 +779,7 @@ class ConductorManager(base_manager.BaseConductorManager): async task. :raises: InvalidStateRequested when the requested state is not a valid target from the current state. - + :raises: NodeProtected if the node is protected. """ LOG.debug("RPC do_node_deploy called for node %s.", node_id) @@ -774,6 +795,9 @@ class ConductorManager(base_manager.BaseConductorManager): node=node.uuid) if rebuild: + if node.protected: + raise exception.NodeProtected(node=node.uuid) + event = 'rebuild' # Note(gilliard) Clear these to force the driver to @@ -881,7 +905,8 @@ class ConductorManager(base_manager.BaseConductorManager): @messaging.expected_exceptions(exception.NoFreeConductorWorker, exception.NodeLocked, exception.InstanceDeployFailure, - exception.InvalidStateRequested) + exception.InvalidStateRequested, + exception.NodeProtected) def do_node_tear_down(self, context, node_id): """RPC method to tear down an existing node deployment. @@ -895,12 +920,15 @@ class ConductorManager(base_manager.BaseConductorManager): async task :raises: InvalidStateRequested when the requested state is not a valid target from the current state. - + :raises: NodeProtected if the node is protected. """ LOG.debug("RPC do_node_tear_down called for node %s.", node_id) with task_manager.acquire(context, node_id, shared=False, purpose='node tear down') as task: + if task.node.protected: + raise exception.NodeProtected(node=task.node.uuid) + try: # NOTE(ghe): Valid power driver values are needed to perform # a tear-down. Deploy info is useful to purge the cache but not @@ -2129,7 +2157,8 @@ class ConductorManager(base_manager.BaseConductorManager): @METRICS.timer('ConductorManager.destroy_node') @messaging.expected_exceptions(exception.NodeLocked, exception.NodeAssociated, - exception.InvalidState) + exception.InvalidState, + exception.NodeProtected) def destroy_node(self, context, node_id): """Delete a node. @@ -2140,7 +2169,7 @@ class ConductorManager(base_manager.BaseConductorManager): associated with it. :raises: InvalidState if the node is in the wrong provision state to perform deletion. - + :raises: NodeProtected if the node is protected. """ # NOTE(dtantsur): we allow deleting a node in maintenance mode even if # we would disallow it otherwise. That's done for recovering hopelessly @@ -2152,6 +2181,9 @@ class ConductorManager(base_manager.BaseConductorManager): raise exception.NodeAssociated(node=node.uuid, instance=node.instance_uuid) + if task.node.protected: + raise exception.NodeProtected(node=node.uuid) + # NOTE(lucasagomes): For the *FAIL states we users should # move it to a safe state prior to deletion. This is because we # should try to avoid deleting a node in a dirty/whacky state, diff --git a/ironic/db/sqlalchemy/alembic/versions/93706939026c_add_node_protected_field.py b/ironic/db/sqlalchemy/alembic/versions/93706939026c_add_node_protected_field.py new file mode 100644 index 0000000000..d332fc8294 --- /dev/null +++ b/ironic/db/sqlalchemy/alembic/versions/93706939026c_add_node_protected_field.py @@ -0,0 +1,33 @@ +# 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 Node.protected field + +Revision ID: 93706939026c +Revises: d2b036ae9378 +Create Date: 2018-10-18 14:55:12.489170 + +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = '93706939026c' +down_revision = 'd2b036ae9378' + + +def upgrade(): + op.add_column('nodes', sa.Column('protected', sa.Boolean(), nullable=False, + server_default=sa.false())) + op.add_column('nodes', sa.Column('protected_reason', sa.Text(), + nullable=True)) diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index 70dafcc50c..37fdbcc71d 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -24,7 +24,7 @@ from oslo_db import options as db_options from oslo_db.sqlalchemy import models from oslo_db.sqlalchemy import types as db_types import six.moves.urllib.parse as urlparse -from sqlalchemy import Boolean, Column, DateTime, Index +from sqlalchemy import Boolean, Column, DateTime, false, Index from sqlalchemy import ForeignKey, Integer from sqlalchemy import schema, String, Text from sqlalchemy.ext.declarative import declarative_base @@ -176,6 +176,9 @@ class Node(Base): inspection_started_at = Column(DateTime, nullable=True) extra = Column(db_types.JsonEncodedDict) automated_clean = Column(Boolean, nullable=True) + protected = Column(Boolean, nullable=False, default=False, + server_default=false()) + protected_reason = Column(Text, nullable=True) bios_interface = Column(String(255), nullable=True) boot_interface = Column(String(255), nullable=True) diff --git a/ironic/objects/node.py b/ironic/objects/node.py index 39b3cb68c4..4871158dc8 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -65,7 +65,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): # Version 1.26: Add deploy_step field # Version 1.27: Add conductor_group field # Version 1.28: Add automated_clean field - VERSION = '1.28' + # Version 1.29: Add protected and protected_reason fields + VERSION = '1.29' dbapi = db_api.get_instance() @@ -132,6 +133,9 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): 'extra': object_fields.FlexibleDictField(nullable=True), 'automated_clean': objects.fields.BooleanField(nullable=True), + 'protected': objects.fields.BooleanField(), + 'protected_reason': object_fields.StringField(nullable=True), + 'bios_interface': object_fields.StringField(nullable=True), 'boot_interface': object_fields.StringField(nullable=True), 'console_interface': object_fields.StringField(nullable=True), @@ -575,6 +579,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): this, it should be removed. Version 1.28: automated_clean was added. For versions prior to this, it should be set to None (or removed). + Version 1.29: protected was added. For versions prior to this, it + should be set to False (or removed). :param target_version: the desired version of the object :param remove_unavailable_fields: True to remove fields that are @@ -586,11 +592,16 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): # Convert the different fields depending on version fields = [('rescue_interface', 22), ('traits', 23), - ('bios_interface', 24), ('automated_clean', 28)] + ('bios_interface', 24), ('automated_clean', 28), + ('protected_reason', 29)] for name, minor in fields: self._adjust_field_to_version(name, None, target_version, 1, minor, remove_unavailable_fields) + # NOTE(dtantsur): the default is False for protected + self._adjust_field_to_version('protected', False, target_version, + 1, 29, remove_unavailable_fields) + self._convert_fault_field(target_version, remove_unavailable_fields) self._convert_deploy_step_field(target_version, remove_unavailable_fields) @@ -639,6 +650,8 @@ class NodePayload(notification.NotificationPayloadBase): 'vendor_interface': ('node', 'vendor_interface'), 'power_state': ('node', 'power_state'), 'properties': ('node', 'properties'), + 'protected': ('node', 'protected'), + 'protected_reason': ('node', 'protected_reason'), 'provision_state': ('node', 'provision_state'), 'provision_updated_at': ('node', 'provision_updated_at'), 'resource_class': ('node', 'resource_class'), @@ -660,7 +673,8 @@ class NodePayload(notification.NotificationPayloadBase): # Version 1.8: Add bios interface field exposed via API. # Version 1.9: Add deploy_step field exposed via API. # Version 1.10: Add conductor_group field exposed via API. - VERSION = '1.10' + # Version 1.11: Add protected and protected_reason fields exposed via API. + VERSION = '1.11' fields = { 'clean_step': object_fields.FlexibleDictField(nullable=True), 'conductor_group': object_fields.StringField(nullable=True), @@ -691,6 +705,8 @@ class NodePayload(notification.NotificationPayloadBase): 'name': object_fields.StringField(nullable=True), 'power_state': object_fields.StringField(nullable=True), 'properties': object_fields.FlexibleDictField(nullable=True), + 'protected': object_fields.BooleanField(nullable=True), + 'protected_reason': object_fields.StringField(nullable=True), 'provision_state': object_fields.StringField(nullable=True), 'provision_updated_at': object_fields.DateTimeField(nullable=True), 'resource_class': object_fields.StringField(nullable=True), @@ -737,7 +753,8 @@ class NodeSetPowerStatePayload(NodePayload): # Version 1.8: Parent NodePayload version 1.8 # Version 1.9: Parent NodePayload version 1.9 # Version 1.10: Parent NodePayload version 1.10 - VERSION = '1.10' + # Version 1.11: Parent NodePayload version 1.11 + VERSION = '1.11' fields = { # "to_power" indicates the future target_power_state of the node. A @@ -788,7 +805,8 @@ class NodeCorrectedPowerStatePayload(NodePayload): # Version 1.8: Parent NodePayload version 1.8 # Version 1.9: Parent NodePayload version 1.9 # Version 1.10: Parent NodePayload version 1.10 - VERSION = '1.10' + # Version 1.11: Parent NodePayload version 1.11 + VERSION = '1.11' fields = { 'from_power': object_fields.StringField(nullable=True) @@ -823,7 +841,8 @@ class NodeSetProvisionStatePayload(NodePayload): # Version 1.8: Parent NodePayload version 1.8 # Version 1.9: Parent NodePayload version 1.9 # Version 1.10: Parent NodePayload version 1.10 - VERSION = '1.10' + # Version 1.11: Parent NodePayload version 1.11 + VERSION = '1.11' SCHEMA = dict(NodePayload.SCHEMA, **{'instance_info': ('node', 'instance_info')}) @@ -865,7 +884,8 @@ class NodeCRUDPayload(NodePayload): # Version 1.6: Parent NodePayload version 1.8 # Version 1.7: Parent NodePayload version 1.9 # Version 1.8: Parent NodePayload version 1.10 - VERSION = '1.8' + # Version 1.9: Parent NodePayload version 1.11 + VERSION = '1.9' 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 4e92d11bf8..a9b12d24f8 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -125,6 +125,8 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertNotIn('deploy_step', data['nodes'][0]) self.assertNotIn('conductor_group', data['nodes'][0]) self.assertNotIn('automated_clean', data['nodes'][0]) + self.assertNotIn('protected', data['nodes'][0]) + self.assertNotIn('protected_reason', data['nodes'][0]) def test_get_one(self): node = obj_utils.create_test_node(self.context, @@ -164,6 +166,8 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertIn('deploy_step', data) self.assertIn('conductor_group', data) self.assertIn('automated_clean', data) + self.assertIn('protected', data) + self.assertIn('protected_reason', data) def test_get_one_with_json(self): # Test backward compatibility with guess_content_type_from_ext @@ -286,6 +290,33 @@ class TestListNodes(test_api_base.BaseApiTest): headers={api_base.Version.string: '1.47'}) self.assertEqual(data['automated_clean'], False) + def test_node_protected_hidden_in_lower_version(self): + self._test_node_field_hidden_in_lower_version('protected', + '1.47', '1.48') + + def test_node_protected_reason_hidden_in_lower_version(self): + self._test_node_field_hidden_in_lower_version('protected_reason', + '1.47', '1.48') + + def test_node_protected(self): + for value in (True, False): + node = obj_utils.create_test_node(self.context, protected=value, + provision_state='active', + uuid=uuidutils.generate_uuid()) + data = self.get_json('/nodes/%s' % node.uuid, + headers={api_base.Version.string: '1.48'}) + self.assertIs(data['protected'], value) + self.assertIsNone(data['protected_reason']) + + def test_node_protected_with_reason(self): + node = obj_utils.create_test_node(self.context, protected=True, + provision_state='active', + protected_reason='reason!') + data = self.get_json('/nodes/%s' % node.uuid, + headers={api_base.Version.string: '1.48'}) + self.assertTrue(data['protected']) + self.assertEqual('reason!', data['protected_reason']) + def test_get_one_custom_fields(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) @@ -450,6 +481,14 @@ class TestListNodes(test_api_base.BaseApiTest): headers={api_base.Version.string: '1.47'}) self.assertIn('automated_clean', response) + def test_get_protected_fields(self): + node = obj_utils.create_test_node(self.context, + protected=True) + response = self.get_json('/nodes/%s?fields=%s' % + (node.uuid, 'protected'), + headers={api_base.Version.string: '1.48'}) + self.assertIn('protected', response) + def test_detail(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) @@ -481,6 +520,8 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertIn('traits', data['nodes'][0]) self.assertIn('conductor_group', data['nodes'][0]) self.assertIn('automated_clean', data['nodes'][0]) + self.assertIn('protected', data['nodes'][0]) + self.assertIn('protected_reason', data['nodes'][0]) # never expose the chassis_id self.assertNotIn('chassis_id', data['nodes'][0]) @@ -511,6 +552,8 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertIn('resource_class', data['nodes'][0]) self.assertIn('conductor_group', data['nodes'][0]) self.assertIn('automated_clean', data['nodes'][0]) + self.assertIn('protected', data['nodes'][0]) + self.assertIn('protected_reason', data['nodes'][0]) for field in api_utils.V31_FIELDS: self.assertIn(field, data['nodes'][0]) # never expose the chassis_id @@ -2636,6 +2679,66 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + def test_update_protected(self): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + provision_state='active') + self.mock_update_node.return_value = node + headers = {api_base.Version.string: '1.48'} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/protected', + 'value': True, + 'op': 'replace'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + + def test_update_protected_with_reason(self): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + provision_state='active') + self.mock_update_node.return_value = node + headers = {api_base.Version.string: '1.48'} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/protected', + 'value': True, + 'op': 'replace'}, + {'path': '/protected_reason', + 'value': 'reason!', + 'op': 'replace'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + + def test_update_protected_reason(self): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + provision_state='active', + protected=True) + self.mock_update_node.return_value = node + headers = {api_base.Version.string: '1.48'} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/protected_reason', + 'value': 'reason!', + 'op': 'replace'}], + headers=headers) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.OK, response.status_code) + + def test_update_protected_old_api(self): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid()) + self.mock_update_node.return_value = node + headers = {api_base.Version.string: '1.47'} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/protected', + 'value': True, + 'op': 'replace'}], + headers=headers, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + def _create_node_locally(node): driver_factory.check_and_update_node_interfaces(node) @@ -3235,6 +3338,14 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + def test_create_node_protected_not_allowed(self): + headers = {api_base.Version.string: '1.48'} + ndict = test_api_utils.post_get_test_node(protected=True) + response = self.post_json('/nodes', ndict, headers=headers, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + class TestDelete(test_api_base.BaseApiTest): diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 318b6882dd..4e84f0cb0d 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -491,6 +491,65 @@ class UpdateNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.assertIsNone(res['maintenance_reason']) self.assertIsNone(res['fault']) + def test_update_node_protected_set(self): + for state in ('active', 'rescue'): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + provision_state=state) + + node.protected = True + res = self.service.update_node(self.context, node) + self.assertTrue(res['protected']) + self.assertIsNone(res['protected_reason']) + + def test_update_node_protected_unset(self): + # NOTE(dtantsur): we allow unsetting protected in any state to make + # sure a node cannot get stuck in it. + for state in ('active', 'rescue', 'rescue failed'): + node = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + provision_state=state, + protected=True, + protected_reason='reason') + + # check that ManagerService.update_node actually updates the node + node.protected = False + res = self.service.update_node(self.context, node) + self.assertFalse(res['protected']) + self.assertIsNone(res['protected_reason']) + + def test_update_node_protected_invalid_state(self): + node = obj_utils.create_test_node(self.context, + provision_state='available') + + node.protected = True + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_node, + self.context, + node) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.InvalidState, exc.exc_info[0]) + + res = objects.Node.get_by_uuid(self.context, node['uuid']) + self.assertFalse(res['protected']) + self.assertIsNone(res['protected_reason']) + + def test_update_node_protected_reason_without_protected(self): + node = obj_utils.create_test_node(self.context, + provision_state='active') + + node.protected_reason = 'reason!' + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.update_node, + self.context, + node) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.InvalidParameterValue, exc.exc_info[0]) + + res = objects.Node.get_by_uuid(self.context, node['uuid']) + self.assertFalse(res['protected']) + self.assertIsNone(res['protected_reason']) + def test_update_node_already_locked(self): node = obj_utils.create_test_node(self.context, driver='fake-hardware', extra={'test': 'one'}) @@ -1546,6 +1605,23 @@ class ServiceDoNodeDeployTestCase(mgr_utils.ServiceSetUpMixin, mock_iwdi.assert_called_once_with(self.context, node.instance_info) self.assertNotIn('is_whole_disk_image', node.driver_internal_info) + def test_do_node_deploy_rebuild_protected(self, mock_iwdi): + mock_iwdi.return_value = False + self._start_service() + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + provision_state=states.ACTIVE, + protected=True) + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.do_node_deploy, + self.context, node['uuid'], rebuild=True) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeProtected, exc.exc_info[0]) + # Last_error should be None. + self.assertIsNone(node.last_error) + # Verify reservation has been cleared. + self.assertIsNone(node.reservation) + self.assertFalse(mock_iwdi.called) + def test_do_node_deploy_worker_pool_full(self, mock_iwdi): mock_iwdi.return_value = False prv_state = states.AVAILABLE @@ -2567,6 +2643,18 @@ class DoNodeTearDownTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): # Compare true exception hidden by @messaging.expected_exceptions self.assertEqual(exception.InvalidStateRequested, exc.exc_info[0]) + def test_do_node_tear_down_protected(self): + self._start_service() + # test node.provision_state is incorrect for tear_down + node = obj_utils.create_test_node(self.context, driver='fake-hardware', + provision_state=states.ACTIVE, + protected=True) + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.do_node_tear_down, + self.context, node['uuid']) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeProtected, exc.exc_info[0]) + @mock.patch('ironic.drivers.modules.fake.FakePower.validate') def test_do_node_tear_down_validate_fail(self, mock_validate): # InvalidParameterValue should be re-raised as InstanceDeployFailure @@ -4866,6 +4954,24 @@ class DestroyNodeTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): node.refresh() self.assertIsNone(node.reservation) + def test_destroy_node_protected(self): + self._start_service() + node = obj_utils.create_test_node(self.context, + provision_state=states.ACTIVE, + protected=True, + # Even in maintenance the protected + # nodes are not deleted + maintenance=True) + + exc = self.assertRaises(messaging.rpc.ExpectedException, + self.service.destroy_node, + self.context, node.uuid) + # Compare true exception hidden by @messaging.expected_exceptions + self.assertEqual(exception.NodeProtected, exc.exc_info[0]) + # Verify reservation was released. + node.refresh() + self.assertIsNone(node.reservation) + def test_destroy_node_allowed_in_maintenance(self): self._start_service() node = obj_utils.create_test_node( diff --git a/ironic/tests/unit/db/sqlalchemy/test_migrations.py b/ironic/tests/unit/db/sqlalchemy/test_migrations.py index dd75aa2a4d..465b4af593 100644 --- a/ironic/tests/unit/db/sqlalchemy/test_migrations.py +++ b/ironic/tests/unit/db/sqlalchemy/test_migrations.py @@ -765,6 +765,27 @@ class MigrationCheckersMixin(object): col_names = [column.name for column in nodes.c] self.assertIn('automated_clean', col_names) + def _pre_upgrade_93706939026c(self, engine): + data = { + 'node_uuid': uuidutils.generate_uuid(), + } + + nodes = db_utils.get_table(engine, 'nodes') + nodes.insert().execute({'uuid': data['node_uuid']}) + + return data + + def _check_93706939026c(self, engine, data): + nodes = db_utils.get_table(engine, 'nodes') + col_names = [column.name for column in nodes.c] + self.assertIn('protected', col_names) + self.assertIn('protected_reason', col_names) + + node = nodes.select( + nodes.c.uuid == data['node_uuid']).execute().first() + self.assertFalse(node['protected']) + self.assertIsNone(node['protected_reason']) + def test_upgrade_and_version(self): with patch_with_engine(self.engine): self.migration_api.upgrade('head') diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 75bf016d9c..6e3b7e1c62 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -215,6 +215,8 @@ def get_test_node(**kw): 'resource_class': kw.get('resource_class'), 'traits': kw.get('traits', []), 'automated_clean': kw.get('automated_clean', None), + 'protected': kw.get('protected', False), + 'protected_reason': kw.get('protected_reason', 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 cdd4b3fae1..39c6b867b7 100644 --- a/ironic/tests/unit/objects/test_node.py +++ b/ironic/tests/unit/objects/test_node.py @@ -769,6 +769,66 @@ class TestConvertToVersion(db_base.DbTestCase): self.assertIsNone(node.automated_clean) self.assertEqual({}, node.obj_get_changes()) + def test_protected_supported_missing(self): + # protected_interface not set, should be set to default. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + delattr(node, 'protected') + delattr(node, 'protected_reason') + node.obj_reset_changes() + + node._convert_to_version("1.29") + + self.assertFalse(node.protected) + self.assertIsNone(node.protected_reason) + self.assertEqual({'protected': False, 'protected_reason': None}, + node.obj_get_changes()) + + def test_protected_supported_set(self): + # protected set, no change required. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.protected = True + node.protected_reason = 'foo' + node.obj_reset_changes() + node._convert_to_version("1.29") + self.assertTrue(node.protected) + self.assertEqual('foo', node.protected_reason) + self.assertEqual({}, node.obj_get_changes()) + + def test_protected_unsupported_missing(self): + # protected not set, no change required. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + delattr(node, 'protected') + delattr(node, 'protected_reason') + node.obj_reset_changes() + node._convert_to_version("1.28") + self.assertNotIn('protected', node) + self.assertNotIn('protected_reason', node) + self.assertEqual({}, node.obj_get_changes()) + + def test_protected_unsupported_set_remove(self): + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.protected = True + node.protected_reason = 'foo' + node.obj_reset_changes() + node._convert_to_version("1.28") + self.assertNotIn('protected', node) + self.assertNotIn('protected_reason', node) + self.assertEqual({}, node.obj_get_changes()) + + def test_protected_unsupported_set_no_remove_non_default(self): + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.protected = True + node.protected_reason = 'foo' + node.obj_reset_changes() + node._convert_to_version("1.28", False) + self.assertIsNone(node.automated_clean) + self.assertEqual({'protected': False, 'protected_reason': None}, + node.obj_get_changes()) + class TestNodePayloads(db_base.DbTestCase): diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index c226820128..830719dbfc 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -677,7 +677,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.28-d4aba1f583774326903f7366fbaae752', + 'Node': '1.29-7af860bb4017751104558139c52a1327', 'MyObj': '1.5-9459d30d6954bffc7a9afd347a807ca6', 'Chassis': '1.3-d656e039fd8ae9f34efc232ab3980905', 'Port': '1.8-898a47921f4a1f53fcdddd4eeb179e0b', @@ -685,21 +685,21 @@ expected_object_fingerprints = { 'Conductor': '1.3-d3f53e853b4d58cae5bfbd9a8341af4a', 'EventType': '1.1-aa2ba1afd38553e3880c267404e8d370', 'NotificationPublisher': '1.0-51a09397d6c0687771fb5be9a999605d', - 'NodePayload': '1.10-618d4ebd121671f463836b7c4ec45114', + 'NodePayload': '1.11-f323602c2e9c3edbf2a5567eca087ff5', 'NodeSetPowerStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeSetPowerStatePayload': '1.10-74b186077c80e13060de5e2dac9baed5', + 'NodeSetPowerStatePayload': '1.11-b61e66ef9d100a2cc564d16b12810855', 'NodeCorrectedPowerStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeCorrectedPowerStatePayload': '1.10-f85fc83dceda2d7ed356368cda0f008f', + 'NodeCorrectedPowerStatePayload': '1.11-e6e32a38ca655509802ac3c6d8bc17f6', 'NodeSetProvisionStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeSetProvisionStatePayload': '1.10-80e9581b0663dd47362f0b7ab19e4674', + 'NodeSetProvisionStatePayload': '1.11-d13cb3472eea163de5b0723a08e95d2c', 'VolumeConnector': '1.0-3e0252c0ab6e6b9d158d09238a577d97', 'VolumeTarget': '1.0-0b10d663d8dae675900b2c7548f76f5e', 'ChassisCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', 'ChassisCRUDPayload': '1.0-dce63895d8186279a7dd577cffccb202', 'NodeCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeCRUDPayload': '1.8-8086c2706c8f89db8294ab7511b9337b', + 'NodeCRUDPayload': '1.9-c5e57432274371f7fe32f269519033cf', 'PortCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', 'PortCRUDPayload': '1.2-233d259df442eb15cc584fae1fe81504', 'NodeMaintenanceNotification': '1.0-59acc533c11d306f149846f922739c15', diff --git a/releasenotes/notes/protected-650acb2c8a387e17.yaml b/releasenotes/notes/protected-650acb2c8a387e17.yaml new file mode 100644 index 0000000000..154279329b --- /dev/null +++ b/releasenotes/notes/protected-650acb2c8a387e17.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + It is now possible to protect a provisioned node from being undeployed, + rebuilt or deleted by setting the new ``protected`` field to ``True``. + The new ``protected_reason`` field can be used to document the reason + the node was made protected.