From 052d90506fdb75479250705be4ba033f9f025c76 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Fri, 16 Nov 2018 11:10:15 -0800 Subject: [PATCH] Add "owner" information field Adds "owner" field on the node object and exposes it for updates via the API. Additionally, fixed a couple minor items related to the prior where we missed updating version numbers in rebases. Change-Id: Iaaf3db97d21de9b11236cf2d18ffcc3f73f6e50c Story: #2001814 Task: #12550 --- api-ref/source/baremetal-api-v1-nodes.inc | 13 +++ api-ref/source/parameters.yaml | 6 + .../source/samples/node-create-response.json | 1 + .../source/samples/node-show-response.json | 1 + .../node-update-driver-info-response.json | 1 + .../samples/nodes-list-details-response.json | 2 + .../contributor/webapi-version-history.rst | 8 ++ ironic/api/controllers/v1/node.py | 72 ++++++------ ironic/api/controllers/v1/utils.py | 19 +++- ironic/api/controllers/v1/versions.py | 5 +- ironic/common/release_mappings.py | 4 +- .../versions/f190f9d00a11_add_node_owner.py | 32 ++++++ ironic/db/sqlalchemy/api.py | 4 +- ironic/db/sqlalchemy/models.py | 2 +- ironic/objects/node.py | 25 +++-- .../unit/api/controllers/v1/test_node.py | 106 +++++++++++++++++- .../unit/db/sqlalchemy/test_migrations.py | 5 + ironic/tests/unit/db/utils.py | 1 + ironic/tests/unit/objects/test_node.py | 64 ++++++++++- ironic/tests/unit/objects/test_objects.py | 12 +- ...dd-owner-information-52e153faf570747e.yaml | 7 ++ 21 files changed, 331 insertions(+), 59 deletions(-) create mode 100644 ironic/db/sqlalchemy/alembic/versions/f190f9d00a11_add_node_owner.py create mode 100644 releasenotes/notes/add-owner-information-52e153faf570747e.yaml diff --git a/api-ref/source/baremetal-api-v1-nodes.inc b/api-ref/source/baremetal-api-v1-nodes.inc index 28e345be16..8cdb025d08 100644 --- a/api-ref/source/baremetal-api-v1-nodes.inc +++ b/api-ref/source/baremetal-api-v1-nodes.inc @@ -92,6 +92,9 @@ supplied when the Node is created, or the resource may be updated later. .. versionadded:: 1.46 Introduced the ``conductor_group`` field. +.. versionadded: 1.50 + Introduced the ``owner`` field. + Normal response codes: 201 Error codes: 400,403,406 @@ -120,6 +123,7 @@ Request - storage_interface: req_storage_interface - uuid: req_uuid - vendor_interface: req_vendor_interface + - owner: owner **Example Node creation request with a dynamic driver:** @@ -188,6 +192,7 @@ microversion 1.48. - conductor_group: conductor_group - protected: protected - protected_reason: protected_reason + - owner: owner **Example JSON representation of a Node:** @@ -235,6 +240,9 @@ provision state, and maintenance setting for each Node. Introduced the ``conductor_group`` request parameter, to allow filtering the list of returned nodes by conductor group. +.. versionadded: 1.50 + Introduced the ``owner`` field. + Normal response codes: 200 Error codes: 400,403,406 @@ -258,6 +266,7 @@ Request - sort_dir: sort_dir - sort_key: sort_key - detail: detail + - owner: owner Response -------- @@ -307,6 +316,9 @@ Nova instance, eg. with a request to ``v1/nodes/detail?instance_uuid={NOVA INSTA .. versionadded:: 1.48 Introduced the ``protected`` and ``protected_reason`` fields. +.. versionadded: 1.50 + Introduced the ``owner`` field. + Normal response codes: 200 Error codes: 400,403,406 @@ -324,6 +336,7 @@ Request - driver: r_driver - resource_class: r_resource_class - conductor_group: r_conductor_group + - owner: owner - limit: limit - marker: marker - sort_dir: sort_dir diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index c05bc3571b..1ed61419b7 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -856,6 +856,12 @@ nodes: in: body required: true type: array +owner: + description: | + A string or UUID of the tenant who owns the baremetal node. + in: body + required: false + type: string passthru_async: description: | If True the passthru function is invoked asynchronously; if False, diff --git a/api-ref/source/samples/node-create-response.json b/api-ref/source/samples/node-create-response.json index 33d075a10a..e615b226bb 100644 --- a/api-ref/source/samples/node-create-response.json +++ b/api-ref/source/samples/node-create-response.json @@ -36,6 +36,7 @@ "management_interface": null, "name": "test_node_classic", "network_interface": "flat", + "owner": null, "portgroups": [ { "href": "http://127.0.0.1:6385/v1/nodes/6d85703a-565d-469a-96ce-30b6de53079d/portgroups", diff --git a/api-ref/source/samples/node-show-response.json b/api-ref/source/samples/node-show-response.json index 8e0078c8de..d12619e57e 100644 --- a/api-ref/source/samples/node-show-response.json +++ b/api-ref/source/samples/node-show-response.json @@ -38,6 +38,7 @@ "management_interface": null, "name": "test_node_classic", "network_interface": "flat", + "owner": null, "portgroups": [ { "href": "http://127.0.0.1:6385/v1/nodes/6d85703a-565d-469a-96ce-30b6de53079d/portgroups", 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 420772242a..374e6043a9 100644 --- a/api-ref/source/samples/node-update-driver-info-response.json +++ b/api-ref/source/samples/node-update-driver-info-response.json @@ -40,6 +40,7 @@ "management_interface": null, "name": "test_node_classic", "network_interface": "flat", + "owner": null, "portgroups": [ { "href": "http://127.0.0.1:6385/v1/nodes/6d85703a-565d-469a-96ce-30b6de53079d/portgroups", diff --git a/api-ref/source/samples/nodes-list-details-response.json b/api-ref/source/samples/nodes-list-details-response.json index a6ba4480d3..a8fc2faaad 100644 --- a/api-ref/source/samples/nodes-list-details-response.json +++ b/api-ref/source/samples/nodes-list-details-response.json @@ -40,6 +40,7 @@ "management_interface": null, "name": "test_node_classic", "network_interface": "flat", + "owner": "john doe", "portgroups": [ { "href": "http://127.0.0.1:6385/v1/nodes/6d85703a-565d-469a-96ce-30b6de53079d/portgroups", @@ -139,6 +140,7 @@ "management_interface": "ipmitool", "name": "test_node_dynamic", "network_interface": "flat", + "owner": "43e61ec9-8e42-4dcb-bc45-30d66aa93e5b", "portgroups": [ { "href": "http://127.0.0.1:6385/v1/nodes/2b045129-a906-46af-bc1a-092b294b3428/portgroups", diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index 174151117a..7a45603ccf 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,14 @@ REST API Version History ======================== +1.50 (Stein, master) +-------------------- + +Added ``owner`` field to the node object to enable operators to store +information in relation to the owner of a node. The field is up to 255 +characters and MAY be used in a later point in time to allow designation +and deligation of permissions. + 1.49 (Stein, master) -------------------- diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 41c890e41e..d18452cb8b 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -1075,6 +1075,9 @@ class Node(base.APIBase): conductor = wsme.wsattr(wtypes.text, readonly=True) """Represent the conductor currently serving the node""" + owner = wsme.wsattr(wtypes.text) + """Field for storage of physical node owner""" + # NOTE(deva): "conductor_affinity" shouldn't be presented on the # API because it's an internal value. Don't add it here. @@ -1277,7 +1280,7 @@ class Node(base.APIBase): storage_interface=None, traits=[], rescue_interface=None, bios_interface=None, conductor_group="", automated_clean=None, protected=False, - protected_reason=None) + protected_reason=None, owner=None) # NOTE(matty_dubs): The chassis_uuid getter() is based on the # _chassis_uuid variable: sample._chassis_uuid = 'edcad704-b2da-41d5-96d9-afd580ecfa12' @@ -1589,35 +1592,12 @@ class NodesController(rest.RestController): filtered_nodes.append(n) return filtered_nodes - def _create_node_filters(self, chassis_uuid=None, associated=None, - maintenance=None, provision_state=None, - driver=None, resource_class=None, fault=None, - conductor_group=None): - filters = {} - if chassis_uuid: - filters['chassis_uuid'] = chassis_uuid - if associated is not None: - filters['associated'] = associated - if maintenance is not None: - filters['maintenance'] = maintenance - if provision_state: - filters['provision_state'] = provision_state - if driver: - filters['driver'] = driver - if resource_class is not None: - filters['resource_class'] = resource_class - if fault is not None: - filters['fault'] = fault - if conductor_group is not None: - filters['conductor_group'] = conductor_group - return filters - def _get_nodes_collection(self, chassis_uuid, instance_uuid, associated, maintenance, provision_state, marker, limit, sort_key, sort_dir, driver=None, resource_class=None, resource_url=None, fields=None, fault=None, conductor_group=None, - detail=None, conductor=None): + detail=None, conductor=None, owner=None): if self.from_chassis and not chassis_uuid: raise exception.MissingParameterValue( _("Chassis id not specified.")) @@ -1650,10 +1630,22 @@ class NodesController(rest.RestController): # be generated, which we don't want. limit = 0 else: - filters = self._create_node_filters(chassis_uuid, associated, - maintenance, provision_state, - driver, resource_class, fault, - conductor_group) + possible_filters = { + 'maintenance': maintenance, + 'chassis_uuid': chassis_uuid, + 'associated': associated, + 'provision_state': provision_state, + 'driver': driver, + 'resource_class': resource_class, + 'fault': fault, + 'conductor_group': conductor_group, + 'owner': owner, + } + filters = {} + for key, value in possible_filters.items(): + if value is not None: + filters[key] = value + nodes = objects.Node.list(pecan.request.context, limit, marker_obj, sort_key=sort_key, sort_dir=sort_dir, filters=filters) @@ -1764,12 +1756,14 @@ class NodesController(rest.RestController): @expose.expose(NodeCollection, types.uuid, types.uuid, types.boolean, types.boolean, wtypes.text, types.uuid, int, wtypes.text, wtypes.text, wtypes.text, types.listtype, wtypes.text, - wtypes.text, wtypes.text, types.boolean, wtypes.text) + wtypes.text, wtypes.text, types.boolean, wtypes.text, + wtypes.text) def get_all(self, chassis_uuid=None, instance_uuid=None, associated=None, maintenance=None, provision_state=None, marker=None, limit=None, sort_key='id', sort_dir='asc', driver=None, fields=None, resource_class=None, fault=None, - conductor_group=None, detail=None, conductor=None): + conductor_group=None, detail=None, conductor=None, + owner=None): """Retrieve a list of nodes. :param chassis_uuid: Optional UUID of a chassis, to get only nodes for @@ -1799,6 +1793,8 @@ class NodesController(rest.RestController): that conductor_group. :param conductor: Optional string value to get only nodes managed by that conductor. + :param owner: Optional string value that set the owner whose nodes + are to be retrurned. :param fields: Optional, a list with a specified set of fields of the resource to be returned. :param fault: Optional string value to get only nodes with that fault. @@ -1815,6 +1811,7 @@ class NodesController(rest.RestController): api_utils.check_allow_filter_by_fault(fault) api_utils.check_allow_filter_by_conductor_group(conductor_group) api_utils.check_allow_filter_by_conductor(conductor) + api_utils.check_allow_filter_by_owner(owner) fields = api_utils.get_request_return_fields(fields, detail, _DEFAULT_RETURN_FIELDS) @@ -1828,18 +1825,19 @@ class NodesController(rest.RestController): fields=fields, fault=fault, conductor_group=conductor_group, detail=detail, - conductor=conductor) + conductor=conductor, + owner=owner) @METRICS.timer('NodesController.detail') @expose.expose(NodeCollection, types.uuid, types.uuid, types.boolean, types.boolean, wtypes.text, types.uuid, int, wtypes.text, wtypes.text, wtypes.text, wtypes.text, wtypes.text, - wtypes.text, wtypes.text) + wtypes.text, wtypes.text, wtypes.text) def detail(self, chassis_uuid=None, instance_uuid=None, associated=None, maintenance=None, provision_state=None, marker=None, limit=None, sort_key='id', sort_dir='asc', driver=None, resource_class=None, fault=None, conductor_group=None, - conductor=None): + conductor=None, owner=None): """Retrieve a list of nodes with detail. :param chassis_uuid: Optional UUID of a chassis, to get only nodes for @@ -1868,6 +1866,8 @@ class NodesController(rest.RestController): :param fault: Optional string value to get only nodes with that fault. :param conductor_group: Optional string value to get only nodes with that conductor_group. + :param owner: Optional string value that set the owner whose nodes + are to be retrurned. """ cdict = pecan.request.context.to_policy_values() policy.authorize('baremetal:node:get', cdict, cdict) @@ -1877,6 +1877,7 @@ class NodesController(rest.RestController): api_utils.check_allow_specify_resource_class(resource_class) api_utils.check_allow_filter_by_fault(fault) api_utils.check_allow_filter_by_conductor_group(conductor_group) + api_utils.check_allow_filter_by_owner(owner) api_utils.check_allowed_fields([sort_key]) # /detail should only work against collections parent = pecan.request.path.split('/')[:-1][-1] @@ -1895,7 +1896,8 @@ class NodesController(rest.RestController): resource_url=resource_url, fault=fault, conductor_group=conductor_group, - conductor=conductor) + conductor=conductor, + owner=owner) @METRICS.timer('NodesController.validate') @expose.expose(wtypes.text, types.uuid_or_name, types.uuid) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index bed014844f..4fb56f7ea8 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -379,6 +379,7 @@ VERSIONED_FIELDS = { 'protected': versions.MINOR_48_NODE_PROTECTED, 'protected_reason': versions.MINOR_48_NODE_PROTECTED, 'conductor': versions.MINOR_49_CONDUCTORS, + 'owner': versions.MINOR_50_NODE_OWNER, } for field in V31_FIELDS: @@ -544,6 +545,20 @@ def check_allow_filter_by_conductor_group(conductor_group): 'opr': versions.MINOR_46_NODE_CONDUCTOR_GROUP}) +def check_allow_filter_by_owner(owner): + """Check if filtering nodes by owner is allowed. + + Version 1.50 of the API allows filtering nodes by owner. + """ + if (owner is not None and pecan.request.version.minor + < versions.MINOR_50_NODE_OWNER): + raise exception.NotAcceptable(_( + "Request not acceptable. The minimal required API version " + "should be %(base)s.%(opr)s") % + {'base': versions.BASE_VERSION, + 'opr': versions.MINOR_50_NODE_OWNER}) + + def initial_node_provision_state(): """Return node state to use by default when creating new nodes. @@ -930,7 +945,7 @@ def get_request_return_fields(fields, detail, default_fields): def allow_expose_conductors(): """Check if accessing conductor endpoints is allowed. - Version 1.48 of the API exposed conductor endpoints and conductor field + Version 1.49 of the API exposed conductor endpoints and conductor field for the node. """ return pecan.request.version.minor >= versions.MINOR_49_CONDUCTORS @@ -939,7 +954,7 @@ def allow_expose_conductors(): def check_allow_filter_by_conductor(conductor): """Check if filtering nodes by conductor is allowed. - Version 1.48 of the API allows filtering nodes by conductor. + Version 1.49 of the API allows filtering nodes by conductor. """ if conductor is not None and not allow_expose_conductors(): raise exception.NotAcceptable(_( diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 826df55d17..fcc83195aa 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -86,6 +86,8 @@ BASE_VERSION = 1 # 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. +# v1.49: Exposes current conductor on the node object. +# v1.50: Add owner to the node object. MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -137,6 +139,7 @@ MINOR_46_NODE_CONDUCTOR_GROUP = 46 MINOR_47_NODE_AUTOMATED_CLEAN = 47 MINOR_48_NODE_PROTECTED = 48 MINOR_49_CONDUCTORS = 49 +MINOR_50_NODE_OWNER = 50 # When adding another version, update: # - MINOR_MAX_VERSION @@ -144,7 +147,7 @@ MINOR_49_CONDUCTORS = 49 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_49_CONDUCTORS +MINOR_MAX_VERSION = MINOR_50_NODE_OWNER # 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 9579c94f0e..883a4910b8 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -131,10 +131,10 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.49', + 'api': '1.50', 'rpc': '1.47', 'objects': { - 'Node': ['1.29', '1.28'], + 'Node': ['1.30', '1.29', '1.28'], 'Conductor': ['1.3'], 'Chassis': ['1.3'], 'Port': ['1.8'], diff --git a/ironic/db/sqlalchemy/alembic/versions/f190f9d00a11_add_node_owner.py b/ironic/db/sqlalchemy/alembic/versions/f190f9d00a11_add_node_owner.py new file mode 100644 index 0000000000..45ded937ed --- /dev/null +++ b/ironic/db/sqlalchemy/alembic/versions/f190f9d00a11_add_node_owner.py @@ -0,0 +1,32 @@ +# All Rights Reserved. +# +# 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_owner + +Revision ID: f190f9d00a11 +Revises: 93706939026c +Create Date: 2018-11-12 00:33:58.575100 + +""" + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = 'f190f9d00a11' +down_revision = '93706939026c' + + +def upgrade(): + op.add_column('nodes', sa.Column('owner', sa.String(255), + nullable=True)) diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index e702a38ad6..8d32d63858 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -224,7 +224,7 @@ class Connection(api.Connection): 'chassis_uuid', 'associated', 'reserved', 'reserved_by_any_of', 'provisioned_before', 'inspection_started_before', 'fault', - 'conductor_group'} + 'conductor_group', 'owner'} unsupported_filters = set(filters).difference(supported_filters) if unsupported_filters: msg = _("SqlAlchemy API does not support " @@ -232,7 +232,7 @@ class Connection(api.Connection): raise ValueError(msg) for field in ['console_enabled', 'maintenance', 'driver', 'resource_class', 'provision_state', 'uuid', 'id', - 'fault', 'conductor_group']: + 'fault', 'conductor_group', 'owner']: if field in filters: query = query.filter_by(**{field: filters[field]}) if 'chassis_uuid' in filters: diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index 37fdbcc71d..2a17dfa65a 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -179,7 +179,7 @@ class Node(Base): protected = Column(Boolean, nullable=False, default=False, server_default=false()) protected_reason = Column(Text, nullable=True) - + owner = Column(String(255), nullable=True) bios_interface = Column(String(255), nullable=True) boot_interface = Column(String(255), nullable=True) console_interface = Column(String(255), nullable=True) diff --git a/ironic/objects/node.py b/ironic/objects/node.py index 4871158dc8..73b9187455 100644 --- a/ironic/objects/node.py +++ b/ironic/objects/node.py @@ -66,7 +66,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): # Version 1.27: Add conductor_group field # Version 1.28: Add automated_clean field # Version 1.29: Add protected and protected_reason fields - VERSION = '1.29' + # Version 1.30: Add owner field + VERSION = '1.30' dbapi = db_api.get_instance() @@ -149,6 +150,7 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): 'storage_interface': object_fields.StringField(nullable=True), 'vendor_interface': object_fields.StringField(nullable=True), 'traits': object_fields.ObjectField('TraitList', nullable=True), + 'owner': object_fields.StringField(nullable=True), } def as_dict(self, secure=False): @@ -581,6 +583,8 @@ class Node(base.IronicObject, object_base.VersionedObjectDictCompat): 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). + Version 1.30: owner was added. 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 @@ -593,7 +597,7 @@ 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), - ('protected_reason', 29)] + ('protected_reason', 29), ('owner', 30)] for name, minor in fields: self._adjust_field_to_version(name, None, target_version, 1, minor, remove_unavailable_fields) @@ -648,6 +652,7 @@ class NodePayload(notification.NotificationPayloadBase): 'rescue_interface': ('node', 'rescue_interface'), 'storage_interface': ('node', 'storage_interface'), 'vendor_interface': ('node', 'vendor_interface'), + 'owner': ('node', 'owner'), 'power_state': ('node', 'power_state'), 'properties': ('node', 'properties'), 'protected': ('node', 'protected'), @@ -674,7 +679,8 @@ class NodePayload(notification.NotificationPayloadBase): # Version 1.9: Add deploy_step field exposed via API. # Version 1.10: Add conductor_group field exposed via API. # Version 1.11: Add protected and protected_reason fields exposed via API. - VERSION = '1.11' + # Version 1.12: Add node owner field. + VERSION = '1.12' fields = { 'clean_step': object_fields.FlexibleDictField(nullable=True), 'conductor_group': object_fields.StringField(nullable=True), @@ -703,6 +709,7 @@ class NodePayload(notification.NotificationPayloadBase): 'storage_interface': object_fields.StringField(nullable=True), 'vendor_interface': object_fields.StringField(nullable=True), 'name': object_fields.StringField(nullable=True), + 'owner': object_fields.StringField(nullable=True), 'power_state': object_fields.StringField(nullable=True), 'properties': object_fields.FlexibleDictField(nullable=True), 'protected': object_fields.BooleanField(nullable=True), @@ -754,7 +761,8 @@ class NodeSetPowerStatePayload(NodePayload): # Version 1.9: Parent NodePayload version 1.9 # Version 1.10: Parent NodePayload version 1.10 # Version 1.11: Parent NodePayload version 1.11 - VERSION = '1.11' + # Version 1.12: Parent NodePayload version 1.12 + VERSION = '1.12' fields = { # "to_power" indicates the future target_power_state of the node. A @@ -806,7 +814,8 @@ class NodeCorrectedPowerStatePayload(NodePayload): # Version 1.9: Parent NodePayload version 1.9 # Version 1.10: Parent NodePayload version 1.10 # Version 1.11: Parent NodePayload version 1.11 - VERSION = '1.11' + # Version 1.12: Parent NodePayload version 1.12 + VERSION = '1.12' fields = { 'from_power': object_fields.StringField(nullable=True) @@ -842,7 +851,8 @@ class NodeSetProvisionStatePayload(NodePayload): # Version 1.9: Parent NodePayload version 1.9 # Version 1.10: Parent NodePayload version 1.10 # Version 1.11: Parent NodePayload version 1.11 - VERSION = '1.11' + # Version 1.12: Parent NodePayload version 1.12 + VERSION = '1.12' SCHEMA = dict(NodePayload.SCHEMA, **{'instance_info': ('node', 'instance_info')}) @@ -885,7 +895,8 @@ class NodeCRUDPayload(NodePayload): # Version 1.7: Parent NodePayload version 1.9 # Version 1.8: Parent NodePayload version 1.10 # Version 1.9: Parent NodePayload version 1.11 - VERSION = '1.9' + # Version 1.10: Parent NodePayload version 1.12 + VERSION = '1.10' 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 16484ae6db..27f13d7dcf 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -132,6 +132,7 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertNotIn('automated_clean', data['nodes'][0]) self.assertNotIn('protected', data['nodes'][0]) self.assertNotIn('protected_reason', data['nodes'][0]) + self.assertNotIn('owner', data['nodes'][0]) def test_get_one(self): node = obj_utils.create_test_node(self.context, @@ -173,6 +174,7 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertIn('automated_clean', data) self.assertIn('protected', data) self.assertIn('protected_reason', data) + self.assertIn('owner', data) def test_get_one_with_json(self): # Test backward compatibility with guess_content_type_from_ext @@ -326,6 +328,23 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertTrue(data['protected']) self.assertEqual('reason!', data['protected_reason']) + def test_node_owner_hidden_in_lower_version(self): + self._test_node_field_hidden_in_lower_version('owner', + '1.49', '1.50') + + def test_node_owner_null_field(self): + node = obj_utils.create_test_node(self.context, owner=None) + data = self.get_json('/nodes/%s' % node.uuid, + headers={api_base.Version.string: '1.50'}) + self.assertIsNone(data['owner']) + + def test_node_owner_present(self): + node = obj_utils.create_test_node(self.context, + owner="akindofmagic") + data = self.get_json('/nodes/%s' % node.uuid, + headers={api_base.Version.string: '1.50'}) + self.assertEqual(data['owner'], "akindofmagic") + def test_get_one_custom_fields(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) @@ -517,6 +536,13 @@ class TestListNodes(test_api_base.BaseApiTest): headers={api_base.Version.string: '1.49'}) self.assertIn('conductor', response) + def test_get_owner_fields(self): + node = obj_utils.create_test_node(self.context, owner='fred') + fields = 'owner' + response = self.get_json('/nodes/%s?fields=%s' % (node.uuid, fields), + headers={api_base.Version.string: '1.50'}) + self.assertIn('owner', response) + def test_detail(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) @@ -550,6 +576,7 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertIn('automated_clean', data['nodes'][0]) self.assertIn('protected', data['nodes'][0]) self.assertIn('protected_reason', data['nodes'][0]) + self.assertIn('owner', data['nodes'][0]) # never expose the chassis_id self.assertNotIn('chassis_id', data['nodes'][0]) @@ -582,6 +609,7 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertIn('automated_clean', data['nodes'][0]) self.assertIn('protected', data['nodes'][0]) self.assertIn('protected_reason', data['nodes'][0]) + self.assertIn('owner', data['nodes'][0]) for field in api_utils.V31_FIELDS: self.assertIn(field, data['nodes'][0]) # never expose the chassis_id @@ -1575,7 +1603,7 @@ class TestListNodes(test_api_base.BaseApiTest): def test_get_nodes_by_conductor_not_allowed(self): response = self.get_json('/nodes?conductor=rocky.rocks', - headers={api_base.Version.string: "1.47"}, + headers={api_base.Version.string: "1.48"}, expect_errors=True) self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) @@ -1608,6 +1636,36 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertNotIn(node1.uuid, uuids) self.assertIn(node2.uuid, uuids) + def test_get_nodes_by_owner(self): + node1 = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + owner='fred') + node2 = obj_utils.create_test_node(self.context, + uuid=uuidutils.generate_uuid(), + owner='bob') + + for base_url in ('/nodes', '/nodes/detail'): + data = self.get_json(base_url + '?owner=fred', + headers={api_base.Version.string: "1.50"}) + uuids = [n['uuid'] for n in data['nodes']] + self.assertIn(node1.uuid, uuids) + self.assertNotIn(node2.uuid, uuids) + data = self.get_json(base_url + '?owner=bob', + headers={api_base.Version.string: "1.50"}) + uuids = [n['uuid'] for n in data['nodes']] + self.assertIn(node2.uuid, uuids) + self.assertNotIn(node1.uuid, uuids) + + def test_get_nodes_by_owner_not_allowed(self): + for url in ('/nodes?owner=fred', + '/nodes/detail?owner=fred'): + response = self.get_json( + url, headers={api_base.Version.string: "1.48"}, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_code) + self.assertTrue(response.json['error_message']) + def test_get_console_information(self): node = obj_utils.create_test_node(self.context) expected_console_info = {'test': 'test-data'} @@ -2783,6 +2841,19 @@ class TestPatch(test_api_base.BaseApiTest): 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_owner(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.50'} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/owner', + 'value': 'meow', 'op': 'replace'}], headers=headers) self.assertEqual('application/json', response.content_type) @@ -2815,6 +2886,20 @@ class TestPatch(test_api_base.BaseApiTest): self.assertEqual(http_client.BAD_REQUEST, response.status_code) self.assertTrue(response.json['error_message']) + def test_update_owner_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': '/owner', + 'value': 'meow', + '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) @@ -3422,6 +3507,25 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertEqual(http_client.BAD_REQUEST, response.status_int) + def test_create_node_owner(self): + ndict = test_api_utils.post_get_test_node(owner='cowsay') + response = self.post_json('/nodes', ndict, + headers={api_base.Version.string: + str(api_v1.max_version())}) + self.assertEqual(http_client.CREATED, response.status_int) + result = self.get_json('/nodes/%s' % ndict['uuid'], + headers={api_base.Version.string: + str(api_v1.max_version())}) + self.assertEqual('cowsay', result['owner']) + + def test_create_node_owner_old_api_version(self): + headers = {api_base.Version.string: '1.32'} + ndict = test_api_utils.post_get_test_node(owner='bob') + response = self.post_json('/nodes', ndict, headers=headers, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + class TestDelete(test_api_base.BaseApiTest): diff --git a/ironic/tests/unit/db/sqlalchemy/test_migrations.py b/ironic/tests/unit/db/sqlalchemy/test_migrations.py index 465b4af593..10b409f205 100644 --- a/ironic/tests/unit/db/sqlalchemy/test_migrations.py +++ b/ironic/tests/unit/db/sqlalchemy/test_migrations.py @@ -786,6 +786,11 @@ class MigrationCheckersMixin(object): self.assertFalse(node['protected']) self.assertIsNone(node['protected_reason']) + def _check_f190f9d00a11(self, engine, data): + nodes = db_utils.get_table(engine, 'nodes') + col_names = [column.name for column in nodes.c] + self.assertIn('owner', col_names) + 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 9be77ff8e3..a0bc1fb38c 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -218,6 +218,7 @@ def get_test_node(**kw): 'protected': kw.get('protected', False), 'protected_reason': kw.get('protected_reason', None), 'conductor': kw.get('conductor'), + 'owner': kw.get('owner', 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 39c6b867b7..536e86ac00 100644 --- a/ironic/tests/unit/objects/test_node.py +++ b/ironic/tests/unit/objects/test_node.py @@ -775,9 +775,7 @@ class TestConvertToVersion(db_base.DbTestCase): 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}, @@ -829,6 +827,67 @@ class TestConvertToVersion(db_base.DbTestCase): self.assertEqual({'protected': False, 'protected_reason': None}, node.obj_get_changes()) + def test_owner_supported_missing(self): + # owner_interface not set, should be set to default. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + delattr(node, 'owner') + node.obj_reset_changes() + node._convert_to_version("1.30") + self.assertIsNone(node.owner) + self.assertEqual({'owner': None}, + node.obj_get_changes()) + + def test_owner_supported_set(self): + # owner set, no change required. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.owner = "Sure, there is an owner" + node.obj_reset_changes() + node._convert_to_version("1.30") + self.assertEqual("Sure, there is an owner", node.owner) + self.assertEqual({}, node.obj_get_changes()) + + def test_owner_unsupported_missing(self): + # owner not set, no change required. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + delattr(node, 'owner') + node.obj_reset_changes() + node._convert_to_version("1.29") + self.assertNotIn('owner', node) + self.assertEqual({}, node.obj_get_changes()) + + def test_owner_unsupported_set_remove(self): + # owner set, should be removed. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.owner = "magic" + node.obj_reset_changes() + node._convert_to_version("1.29") + self.assertNotIn('owner', node) + self.assertEqual({}, node.obj_get_changes()) + + def test_owner_unsupported_set_no_remove_non_default(self): + # owner set, should be set to default. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.owner = "magic" + node.obj_reset_changes() + node._convert_to_version("1.29", False) + self.assertIsNone(node.owner) + self.assertEqual({'owner': None}, + node.obj_get_changes()) + + def test_owner_unsupported_set_no_remove_default(self): + # owner set, no change required. + node = obj_utils.get_test_node(self.ctxt, **self.fake_node) + + node.owner = None + node.obj_reset_changes() + node._convert_to_version("1.29", False) + self.assertIsNone(node.owner) + self.assertEqual({}, node.obj_get_changes()) + class TestNodePayloads(db_base.DbTestCase): @@ -886,6 +945,7 @@ class TestNodePayloads(db_base.DbTestCase): self.assertEqual(self.node.traits.get_trait_names(), payload.traits) self.assertEqual(self.node.updated_at, payload.updated_at) self.assertEqual(self.node.uuid, payload.uuid) + self.assertEqual(self.node.owner, payload.owner) def test_node_payload(self): payload = objects.NodePayload(self.node) diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 830719dbfc..4f409053c8 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.29-7af860bb4017751104558139c52a1327', + 'Node': '1.30-8313460d6ea5457a527cd3d85e5ee3d8', '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.11-f323602c2e9c3edbf2a5567eca087ff5', + 'NodePayload': '1.12-7d650c2a024357275990681f020512e4', 'NodeSetPowerStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeSetPowerStatePayload': '1.11-b61e66ef9d100a2cc564d16b12810855', + 'NodeSetPowerStatePayload': '1.12-703d110d571cc95b2947bb6bd153fcb8', 'NodeCorrectedPowerStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeCorrectedPowerStatePayload': '1.11-e6e32a38ca655509802ac3c6d8bc17f6', + 'NodeCorrectedPowerStatePayload': '1.12-29cbb6b20a0aeea9e0ab9e17302e9e16', 'NodeSetProvisionStateNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeSetProvisionStatePayload': '1.11-d13cb3472eea163de5b0723a08e95d2c', + 'NodeSetProvisionStatePayload': '1.12-a302ce357ad39a0a4d1ca3c0ee44f0e0', 'VolumeConnector': '1.0-3e0252c0ab6e6b9d158d09238a577d97', 'VolumeTarget': '1.0-0b10d663d8dae675900b2c7548f76f5e', 'ChassisCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', 'ChassisCRUDPayload': '1.0-dce63895d8186279a7dd577cffccb202', 'NodeCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeCRUDPayload': '1.9-c5e57432274371f7fe32f269519033cf', + 'NodeCRUDPayload': '1.10-49590dee863c5ed1193f5deae0a0a2f2', 'PortCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', 'PortCRUDPayload': '1.2-233d259df442eb15cc584fae1fe81504', 'NodeMaintenanceNotification': '1.0-59acc533c11d306f149846f922739c15', diff --git a/releasenotes/notes/add-owner-information-52e153faf570747e.yaml b/releasenotes/notes/add-owner-information-52e153faf570747e.yaml new file mode 100644 index 0000000000..b852f4ddc9 --- /dev/null +++ b/releasenotes/notes/add-owner-information-52e153faf570747e.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Adds API version 1.50 which allows for the storage of an ``owner`` field + on node objects. This is intended for either storage of human parsable + information or the storage of a tenant UUID which could be leveraged + in a future version of the Bare Metal as a Service API.