From 864331a94bddfd1860e0db9c6763fbf4f3ab6507 Mon Sep 17 00:00:00 2001 From: John Garbutt Date: Tue, 9 Jan 2018 17:19:58 +0000 Subject: [PATCH] API: Node Traits API Adds basic CRUD on traits, including set and delete of individual traits. Bumps the API microversion to 1.37. NOTE: This patch does not implement the advanced filtering outlined in the OpenStack API working group's tags guidelines[1]. That will be implemented in a separate microversion as a follow up patch. [1] http://specs.openstack.org/openstack/api-wg/guidelines/tags.html Change-Id: I313fa01fbf20bf0ff19f102ea63b02e72ac2b856 Partial-Bug: #1722194 Co-Authored-By: Mark Goddard --- .../contributor/webapi-version-history.rst | 27 ++ etc/ironic/policy.json.sample | 14 + ironic/api/controllers/v1/node.py | 140 +++++++- ironic/api/controllers/v1/utils.py | 31 ++ ironic/api/controllers/v1/versions.py | 4 +- ironic/common/exception.py | 2 +- ironic/common/policy.py | 19 + ironic/common/release_mappings.py | 2 +- .../unit/api/controllers/v1/test_node.py | 332 +++++++++++++++++- .../unit/api/controllers/v1/test_utils.py | 30 ++ .../notes/node-traits-2d950b62eea24491.yaml | 28 ++ requirements.txt | 1 + 12 files changed, 622 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/node-traits-2d950b62eea24491.yaml diff --git a/doc/source/contributor/webapi-version-history.rst b/doc/source/contributor/webapi-version-history.rst index fba45cd71e..bfd7eac5f6 100644 --- a/doc/source/contributor/webapi-version-history.rst +++ b/doc/source/contributor/webapi-version-history.rst @@ -2,6 +2,33 @@ REST API Version History ======================== +1.37 (Queens, 10.1.0) +--------------------- + +Adds support for node traits, with the following new endpoints. + +* GET /v1/nodes//traits lists the traits for a node. + +* PUT /v1/nodes//traits sets all traits for a node. + +* PUT /v1/nodes//traits/ adds a trait to a node. + +* DELETE /v1/nodes//traits removes all traits from a node. + +* DELETE /v1/nodes//traits/ removes a trait from a + node. + +A node's traits are also included the following node query and list responses: + +* GET /v1/nodes/ + +* GET /v1/nodes/detail + +* GET /v1/nodes?fields=traits + +Traits cannot be specified on node creation, nor can they be updated via a +PATCH request on the node. + 1.36 (Queens, 10.0.0) --------------------- diff --git a/etc/ironic/policy.json.sample b/etc/ironic/policy.json.sample index a5310f944b..2c90fa5119 100644 --- a/etc/ironic/policy.json.sample +++ b/etc/ironic/policy.json.sample @@ -98,6 +98,20 @@ # DELETE /nodes/{node_ident}/vifs/{node_vif_ident} #"baremetal:node:vif:detach": "rule:is_admin" +# List node traits +# GET /nodes/{node_ident}/traits +#"baremetal:node:traits:list": "rule:is_admin or rule:is_observer" + +# Add a trait to, or replace all traits of, a node +# PUT /nodes/{node_ident}/traits +# PUT /nodes/{node_ident}/traits/{trait} +#"baremetal:node:traits:set": "rule:is_admin" + +# Remove one or all traits from a node +# DELETE /nodes/{node_ident}/traits +# DELETE /nodes/{node_ident}/traits/{trait} +#"baremetal:node:traits:delete": "rule:is_admin" + # Retrieve Port records # GET /ports # GET /ports/detail diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 312161a13f..429e92a7f2 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -156,6 +156,9 @@ def hide_fields_in_newer_versions(obj): if not api_utils.allow_storage_interface(): obj.storage_interface = wsme.Unset + if not api_utils.allow_traits(): + obj.traits = wsme.Unset + def update_state_in_older_versions(obj): """Change provision state names for API backwards compatibility. @@ -686,6 +689,107 @@ def _check_clean_steps(clean_steps): exc) +class Traits(base.APIBase): + """API representation of the traits for a node.""" + + traits = wtypes.ArrayType(str) + """node traits""" + + @classmethod + def sample(cls): + traits = ["CUSTOM_TRAIT1", "CUSTOM_TRAIT2"] + return cls(traits=traits) + + +def _get_trait_names(traits): + if not traits: + return [] + return [t.trait for t in traits] + + +class NodeTraitsController(rest.RestController): + + def __init__(self, node_ident): + super(NodeTraitsController, self).__init__() + self.node_ident = node_ident + + @METRICS.timer('NodeTraitsController.get_all') + @expose.expose(Traits) + def get_all(self): + """List node traits.""" + cdict = pecan.request.context.to_policy_values() + policy.authorize('baremetal:node:traits:list', cdict, cdict) + node = api_utils.get_rpc_node(self.node_ident) + traits = objects.TraitList.get_by_node_id(pecan.request.context, + node.id) + return Traits(traits=_get_trait_names(traits)) + + @METRICS.timer('NodeTraitsController.put') + @expose.expose(None, wtypes.text, wtypes.ArrayType(str), + status_code=http_client.NO_CONTENT) + def put(self, trait=None, traits=None): + """Add a trait to a node. + + :param trait: String value; trait to add to a node, or None. Mutually + exclusive with 'traits'. If not None, adds this trait to the node. + :param traits: List of Strings; traits to set for a node, or None. + Mutually exclusive with 'trait'. If not None, replaces the node's + traits with this list. + """ + cdict = pecan.request.context.to_policy_values() + policy.authorize('baremetal:node:traits:set', cdict, cdict) + node = api_utils.get_rpc_node(self.node_ident) + + if (trait and traits is not None) or not (trait or traits is not None): + msg = _("A single node trait may be added via PUT " + "/v1/nodes//traits/ with no body, " + "or all node traits may be replaced via PUT " + "/v1/nodes//traits with the list of " + "traits specified in the request body.") + raise exception.Invalid(msg) + + if trait: + if pecan.request.body and pecan.request.json_body: + # Ensure PUT nodes/uuid1/traits/trait1 with a non-empty body + # fails. + msg = _("No body should be provided when adding a trait") + raise exception.Invalid(msg) + traits = [trait] + replace = False + else: + replace = True + + for trait in traits: + api_utils.validate_trait(trait) + + topic = pecan.request.rpcapi.get_topic_for(node) + pecan.request.rpcapi.add_node_traits( + pecan.request.context, node.id, traits, replace=replace, + topic=topic) + + @METRICS.timer('NodeTraitsController.delete') + @expose.expose(None, wtypes.text, + status_code=http_client.NO_CONTENT) + def delete(self, trait=None): + """Remove one or all traits from a node. + + :param trait: String value; trait to remove from a node, or None. If + None, all traits are removed. + """ + cdict = pecan.request.context.to_policy_values() + policy.authorize('baremetal:node:traits:delete', cdict, cdict) + node = api_utils.get_rpc_node(self.node_ident) + + if trait: + traits = [trait] + else: + traits = None + + topic = pecan.request.rpcapi.get_topic_for(node) + pecan.request.rpcapi.remove_node_traits( + pecan.request.context, node.id, traits, topic=topic) + + class Node(base.APIBase): """API representation of a bare metal node. @@ -849,6 +953,9 @@ class Node(base.APIBase): vendor_interface = wsme.wsattr(wtypes.text) """The vendor interface to be used for this node""" + traits = wtypes.ArrayType(str) + """The traits associated with this node""" + # NOTE(deva): "conductor_affinity" shouldn't be presented on the # API because it's an internal value. Don't add it here. @@ -862,7 +969,12 @@ class Node(base.APIBase): # Add fields we expose. if hasattr(self, k): self.fields.append(k) - setattr(self, k, kwargs.get(k, wtypes.Unset)) + # TODO(jroll) is there a less hacky way to do this? + if k == 'traits' and 'traits' in kwargs: + value = _get_trait_names(kwargs['traits']) + else: + value = kwargs.get(k, wtypes.Unset) + setattr(self, k, value) # NOTE(lucasagomes): chassis_id is an attribute created on-the-fly # by _set_chassis_uuid(), it needs to be present in the fields so @@ -998,7 +1110,7 @@ class Node(base.APIBase): deploy_interface=None, inspect_interface=None, management_interface=None, power_interface=None, raid_interface=None, vendor_interface=None, - storage_interface=None) + storage_interface=None, traits=[]) # NOTE(matty_dubs): The chassis_uuid getter() is based on the # _chassis_uuid variable: sample._chassis_uuid = 'edcad704-b2da-41d5-96d9-afd580ecfa12' @@ -1258,13 +1370,15 @@ class NodesController(rest.RestController): invalid_sort_key_list = ['properties', 'driver_info', 'extra', 'instance_info', 'driver_internal_info', - 'clean_step', 'raid_config', 'target_raid_config'] + 'clean_step', 'raid_config', 'target_raid_config', + 'traits'] _subcontroller_map = { 'ports': port.PortsController, 'portgroups': portgroup.PortgroupsController, 'vifs': NodeVIFController, 'volume': volume.VolumeController, + 'traits': NodeTraitsController, } @pecan.expose() @@ -1280,6 +1394,11 @@ class NodesController(rest.RestController): (remainder[0] == 'vifs' and not api_utils.allow_vifs_subcontroller())): pecan.abort(http_client.NOT_FOUND) + if remainder[0] == 'traits' and not api_utils.allow_traits(): + # NOTE(mgoddard): Returning here will ensure we exhibit the + # behaviour of previous releases for microversions without this + # endpoint. + return subcontroller = self._subcontroller_map.get(remainder[0]) if subcontroller: return subcontroller(node_ident=ident), remainder[1:] @@ -1394,7 +1513,9 @@ class NodesController(rest.RestController): """Update rpc_node based on changed fields in a node. """ - for field in objects.Node.fields: + # NOTE(mgoddard): Traits cannot be updated via a node PATCH. + fields = set(objects.Node.fields) - {'traits'} + for field in fields: try: patch_val = getattr(node, field) except AttributeError: @@ -1622,6 +1743,11 @@ class NodesController(rest.RestController): node.storage_interface is not wtypes.Unset): raise exception.NotAcceptable() + if node.traits is not wtypes.Unset: + msg = _("Cannot specify node traits on node creation. Traits must " + "be set via the node traits API.") + 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 @@ -1693,6 +1819,12 @@ class NodesController(rest.RestController): if s_interface and not api_utils.allow_storage_interface(): raise exception.NotAcceptable() + traits = api_utils.get_patch_values(patch, '/traits') + if traits: + msg = _("Cannot update node traits via node patch. Node traits " + "should be updated via the node traits API.") + raise exception.Invalid(msg) + rpc_node = api_utils.get_rpc_node(node_ident) remove_inst_uuid_patch = [{'op': 'remove', 'path': '/instance_uuid'}] diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index 270ce48e72..4c50bb8bb9 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -14,8 +14,10 @@ # under the License. import inspect +import re import jsonpatch +import os_traits from oslo_config import cfg from oslo_utils import uuidutils import pecan @@ -65,6 +67,9 @@ V31_FIELDS = [ 'vendor_interface', ] +STANDARD_TRAITS = os_traits.get_traits() +CUSTOM_TRAIT_REGEX = re.compile("^%s[A-Z0-9_]+$" % os_traits.CUSTOM_NAMESPACE) + def validate_limit(limit): if limit is None: @@ -84,6 +89,22 @@ def validate_sort_dir(sort_dir): return sort_dir +def validate_trait(trait): + error = wsme.exc.ClientSideError( + _('Invalid trait. A valid trait must be no longer than 255 ' + 'characters. Standard traits are defined in the os_traits library. ' + 'A custom trait must start with the prefix CUSTOM_ and use ' + 'the following characters: A-Z, 0-9 and _')) + if len(trait) > 255 or len(trait) < 1: + raise error + + if trait in STANDARD_TRAITS: + return + + if CUSTOM_TRAIT_REGEX.match(trait) is None: + raise error + + def apply_jsonpatch(doc, patch): for p in patch: if p['op'] == 'add' and p['path'].count('/') == 1: @@ -302,6 +323,8 @@ def check_allowed_fields(fields): raise exception.NotAcceptable() if 'storage_interface' in fields and not allow_storage_interface(): raise exception.NotAcceptable() + if 'traits' in fields and not allow_traits(): + raise exception.NotAcceptable() def check_allowed_portgroup_fields(fields): @@ -630,3 +653,11 @@ def get_controller_reserved_names(cls): reserved_names += cls._custom_actions.keys() return reserved_names + + +def allow_traits(): + """Check if traits are allowed for the node. + + Version 1.37 of the API allows traits for the node. + """ + return pecan.request.version.minor >= versions.MINOR_37_NODE_TRAITS diff --git a/ironic/api/controllers/v1/versions.py b/ironic/api/controllers/v1/versions.py index 7f2d00bd56..f81e5e8289 100644 --- a/ironic/api/controllers/v1/versions.py +++ b/ironic/api/controllers/v1/versions.py @@ -73,6 +73,7 @@ BASE_VERSION = 1 # v1.34: Add physical network field to port. # v1.35: Add ability to provide configdrive when rebuilding node. # v1.36: Add Ironic Python Agent version support. +# v1.37: Add node traits. MINOR_0_JUNO = 0 MINOR_1_INITIAL_VERSION = 1 @@ -111,6 +112,7 @@ MINOR_33_STORAGE_INTERFACE = 33 MINOR_34_PORT_PHYSICAL_NETWORK = 34 MINOR_35_REBUILD_CONFIG_DRIVE = 35 MINOR_36_AGENT_VERSION_HEARTBEAT = 36 +MINOR_37_NODE_TRAITS = 37 # When adding another version, update: # - MINOR_MAX_VERSION @@ -118,7 +120,7 @@ MINOR_36_AGENT_VERSION_HEARTBEAT = 36 # explanation of what changed in the new version # - common/release_mappings.py, RELEASE_MAPPING['master']['api'] -MINOR_MAX_VERSION = MINOR_36_AGENT_VERSION_HEARTBEAT +MINOR_MAX_VERSION = MINOR_37_NODE_TRAITS # 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 4fd79ba65f..8cbc15b8ca 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -760,7 +760,7 @@ class AgentAPIError(IronicException): '%(status)s with error: %(error)s') -class NodeTraitNotFound(IronicException): +class NodeTraitNotFound(NotFound): _msg_fmt = _("Node %(node_id)s doesn't have a trait '%(trait)s'") diff --git a/ironic/common/policy.py b/ironic/common/policy.py index fa74b8895f..7589b7d520 100644 --- a/ironic/common/policy.py +++ b/ironic/common/policy.py @@ -180,6 +180,25 @@ node_policies = [ 'Detach a VIF from a node', [{'path': '/nodes/{node_ident}/vifs/{node_vif_ident}', 'method': 'DELETE'}]), + + policy.DocumentedRuleDefault( + 'baremetal:node:traits:list', + 'rule:is_admin or rule:is_observer', + 'List node traits', + [{'path': '/nodes/{node_ident}/traits', 'method': 'GET'}]), + policy.DocumentedRuleDefault( + 'baremetal:node:traits:set', + 'rule:is_admin', + 'Add a trait to, or replace all traits of, a node', + [{'path': '/nodes/{node_ident}/traits', 'method': 'PUT'}, + {'path': '/nodes/{node_ident}/traits/{trait}', 'method': 'PUT'}]), + policy.DocumentedRuleDefault( + 'baremetal:node:traits:delete', + 'rule:is_admin', + 'Remove one or all traits from a node', + [{'path': '/nodes/{node_ident}/traits', 'method': 'DELETE'}, + {'path': '/nodes/{node_ident}/traits/{trait}', + 'method': 'DELETE'}]), ] port_policies = [ diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index 276a04c81d..6b0e944a72 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -121,7 +121,7 @@ RELEASE_MAPPING = { } }, 'master': { - 'api': '1.36', + 'api': '1.37', 'rpc': '1.44', 'objects': { 'Node': ['1.23'], diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index eab547299e..5b0490a26d 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -118,6 +118,7 @@ class TestListNodes(test_api_base.BaseApiTest): for field in api_utils.V31_FIELDS: self.assertNotIn(field, data['nodes'][0]) self.assertNotIn('storage_interface', data['nodes'][0]) + self.assertNotIn('traits', data['nodes'][0]) # never expose the chassis_id self.assertNotIn('chassis_id', data['nodes'][0]) @@ -152,6 +153,7 @@ class TestListNodes(test_api_base.BaseApiTest): for field in api_utils.V31_FIELDS: self.assertIn(field, data) self.assertIn('storage_interface', data) + self.assertIn('traits', data) # never expose the chassis_id self.assertNotIn('chassis_id', data) @@ -179,6 +181,13 @@ class TestListNodes(test_api_base.BaseApiTest): headers={api_base.Version.string: '1.32'}) self.assertNotIn('storage_interface', data) + def test_node_traits_hidden_in_lower_version(self): + node = obj_utils.create_test_node(self.context) + data = self.get_json( + '/nodes/%s' % node.uuid, + headers={api_base.Version.string: '1.36'}) + self.assertNotIn('traits', data) + def test_get_one_custom_fields(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) @@ -297,6 +306,25 @@ class TestListNodes(test_api_base.BaseApiTest): headers={api_base.Version.string: str(api_v1.max_version())}) self.assertIn('storage_interface', response) + def test_get_traits_fields_invalid_api_version(self): + node = obj_utils.create_test_node(self.context, + chassis_id=self.chassis.id) + fields = 'traits' + response = self.get_json( + '/nodes/%s?fields=%s' % (node.uuid, fields), + headers={api_base.Version.string: '1.36'}, + expect_errors=True) + self.assertEqual(http_client.NOT_ACCEPTABLE, response.status_int) + + def test_get_traits_fields(self): + node = obj_utils.create_test_node(self.context, + chassis_id=self.chassis.id) + fields = 'traits' + response = self.get_json( + '/nodes/%s?fields=%s' % (node.uuid, fields), + headers={api_base.Version.string: str(api_v1.max_version())}) + self.assertIn('traits', response) + def test_detail(self): node = obj_utils.create_test_node(self.context, chassis_id=self.chassis.id) @@ -325,6 +353,7 @@ class TestListNodes(test_api_base.BaseApiTest): for field in api_utils.V31_FIELDS: self.assertIn(field, data['nodes'][0]) self.assertIn('storage_interface', data['nodes'][0]) + self.assertIn('traits', data['nodes'][0]) # never expose the chassis_id self.assertNotIn('chassis_id', data['nodes'][0]) @@ -455,6 +484,18 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertEqual(node.storage_interface, new_data['nodes'][0]["storage_interface"]) + def test_hide_fields_in_newer_versions_traits(self): + node = obj_utils.create_test_node(self.context) + objects.TraitList.create(self.context, node.id, ['CUSTOM_1']) + node.refresh() + + data = self.get_json( + '/nodes/detail', headers={api_base.Version.string: '1.36'}) + self.assertNotIn('traits', data['nodes'][0]) + new_data = self.get_json( + '/nodes/detail', headers={api_base.Version.string: '1.37'}) + self.assertEqual(['CUSTOM_1'], new_data['nodes'][0]["traits"]) + def test_many(self): nodes = [] for id in range(5): @@ -563,9 +604,11 @@ class TestListNodes(test_api_base.BaseApiTest): def test_sort_key_invalid(self): invalid_keys_list = ['foo', 'properties', 'driver_info', 'extra', 'instance_info', 'driver_internal_info', - 'clean_step'] + 'clean_step', 'traits'] + headers = {api_base.Version.string: str(api_v1.max_version())} for invalid_key in invalid_keys_list: response = self.get_json('/nodes?sort_key=%s' % invalid_key, + headers=headers, expect_errors=True) self.assertEqual(http_client.BAD_REQUEST, response.status_int) self.assertEqual('application/json', response.content_type) @@ -1160,6 +1203,25 @@ class TestListNodes(test_api_base.BaseApiTest): def test_get_nodes_by_resource_class_invalid_api_version_detail(self): self._test_get_nodes_by_resource_class_invalid_api_version(detail=True) + def _test_get_nodes_by_traits_not_allowed(self, detail=False): + if detail: + base_url = '/nodes/detail?traits=%s' + else: + base_url = '/nodes?traits=%s' + + response = self.get_json( + base_url % 'CUSTOM_TRAIT_1', + headers={api_base.Version.string: str(api_v1.max_version())}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + self.assertTrue(response.json['error_message']) + + def test_get_nodes_by_traits_not_allowed(self): + self._test_get_nodes_by_traits_not_allowed(detail=False) + + def test_get_nodes_by_traits_not_allowed_detail(self): + self._test_get_nodes_by_traits_not_allowed(detail=True) + def test_get_console_information(self): node = obj_utils.create_test_node(self.context) expected_console_info = {'test': 'test-data'} @@ -2100,6 +2162,20 @@ 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_traits(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: str(api_v1.max_version())} + response = self.patch_json('/nodes/%s' % node.uuid, + [{'path': '/traits', + 'value': ['CUSTOM_1'], + 'op': 'add'}], + headers=headers, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + def _create_node_locally(node): driver_factory.check_and_update_node_interfaces(node) @@ -2205,6 +2281,14 @@ class TestPost(test_api_base.BaseApiTest): result = self._test_create_node(headers=headers, **node) self.assertEqual(expected, result[field]) + def test_create_node_specify_traits(self): + headers = {api_base.Version.string: str(api_v1.max_version())} + ndict = test_api_utils.post_get_test_node() + ndict['traits'] = ['CUSTOM_4'] + response = self.post_json('/nodes', ndict, headers=headers, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, response.status_int) + def test_create_node_specify_interfaces_bad_version(self): headers = {api_base.Version.string: '1.30'} for field in api_utils.V31_FIELDS: @@ -4081,3 +4165,249 @@ class TestAttachDetachVif(test_api_base.BaseApiTest): self.assertEqual(http_client.CONFLICT, ret.status_code) self.assertTrue(ret.json['error_message']) + + +class TestTraits(test_api_base.BaseApiTest): + + def setUp(self): + super(TestTraits, self).setUp() + self.version = "1.37" + self.node = obj_utils.create_test_node( + self.context, + provision_state=states.AVAILABLE, name='node-39') + self.traits = ['CUSTOM_1', 'CUSTOM_2'] + self._add_traits(self.node, self.traits) + p = mock.patch.object(rpcapi.ConductorAPI, 'get_topic_for') + self.mock_gtf = p.start() + self.mock_gtf.return_value = 'test-topic' + self.addCleanup(p.stop) + + def _add_traits(self, node, traits): + if traits: + node.traits = objects.TraitList.create( + self.context, node.id, traits) + + def test_get_all_traits(self): + ret = self.get_json('/nodes/%s/traits' % self.node.uuid, + headers={api_base.Version.string: self.version}) + self.assertEqual({'traits': ['CUSTOM_1', 'CUSTOM_2']}, ret) + + def test_get_all_traits_fails_with_node_not_found(self): + ret = self.get_json('/nodes/badname/traits', + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_code) + + def test_get_all_traits_fails_with_bad_version(self): + ret = self.get_json('/nodes/%s/traits' % self.node.uuid, + headers={api_base.Version.string: "1.36"}, + expect_errors=True) + self.assertEqual(http_client.NOT_ACCEPTABLE, ret.status_code) + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_set_all_traits(self, mock_add): + request_body = {'traits': ['CUSTOM_3']} + ret = self.put_json('/nodes/%s/traits' % self.node.name, + request_body, + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_code) + mock_add.assert_called_once_with(mock.ANY, self.node.id, + ['CUSTOM_3'], replace=True, + topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_set_all_traits_empty(self, mock_add): + request_body = {'traits': []} + ret = self.put_json('/nodes/%s/traits' % self.node.name, + request_body, + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_code) + mock_add.assert_called_once_with(mock.ANY, self.node.id, + [], replace=True, + topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_set_all_traits_rejects_bad_trait(self, mock_add): + request_body = {'traits': ['CUSTOM_3', 'BAD_TRAIT']} + ret = self.put_json('/nodes/%s/traits' % self.node.name, + request_body, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(mock_add.called) + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_set_all_traits_rejects_too_long_trait(self, mock_add): + # Maximum length is 255. + long_trait = 'CUSTOM_' + 'T' * 249 + request_body = {'traits': ['CUSTOM_3', long_trait]} + ret = self.put_json('/nodes/%s/traits' % self.node.name, + request_body, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(mock_add.called) + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_set_all_traits_rejects_no_body(self, mock_add): + ret = self.put_json('/nodes/%s/traits' % self.node.name, {}, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(mock_add.called) + + def test_set_all_traits_fails_with_bad_version(self): + request_body = {'traits': []} + ret = self.put_json('/nodes/%s/traits' % self.node.uuid, request_body, + headers={api_base.Version.string: "1.36"}, + expect_errors=True) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, ret.status_code) + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_add_single_trait(self, mock_add): + ret = self.put_json('/nodes/%s/traits/CUSTOM_3' % self.node.name, {}, + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_code) + mock_add.assert_called_once_with(mock.ANY, self.node.id, + ['CUSTOM_3'], replace=False, + topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_no_add_single_trait_via_body(self, mock_add): + request_body = {'trait': 'CUSTOM_3'} + ret = self.put_json('/nodes/%s/traits' % self.node.name, + request_body, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(mock_add.called) + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_no_add_single_trait_via_body_2(self, mock_add): + request_body = {'traits': ['CUSTOM_3']} + ret = self.put_json('/nodes/%s/traits/CUSTOM_3' % self.node.name, + request_body, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(mock_add.called) + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_add_single_trait_rejects_bad_trait(self, mock_add): + ret = self.put_json('/nodes/%s/traits/bad_trait' % self.node.name, {}, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(mock_add.called) + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_add_single_trait_rejects_too_long_trait(self, mock_add): + # Maximum length is 255. + long_trait = 'CUSTOM_' + 'T' * 249 + ret = self.put_json('/nodes/%s/traits/%s' % ( + self.node.name, long_trait), {}, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + self.assertFalse(mock_add.called) + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_add_single_trait_fails_max_trait_limit(self, mock_add): + mock_add.side_effect = exception.InvalidParameterValue( + err='too many traits') + ret = self.put_json('/nodes/%s/traits/CUSTOM_3' % self.node.name, {}, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.BAD_REQUEST, ret.status_code) + mock_add.assert_called_once_with(mock.ANY, self.node.id, + ['CUSTOM_3'], replace=False, + topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_add_single_trait_fails_if_node_locked(self, mock_add): + mock_add.side_effect = exception.NodeLocked( + node=self.node.uuid, host='host1') + ret = self.put_json('/nodes/%s/traits/CUSTOM_3' % self.node.name, {}, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + mock_add.assert_called_once_with(mock.ANY, self.node.id, + ['CUSTOM_3'], replace=False, + topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'add_node_traits') + def test_add_single_trait_fails_if_node_not_found(self, mock_add): + mock_add.side_effect = exception.NodeNotFound(node=self.node.uuid) + ret = self.put_json('/nodes/%s/traits/CUSTOM_3' % self.node.name, {}, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_code) + mock_add.assert_called_once_with(mock.ANY, self.node.id, + ['CUSTOM_3'], replace=False, + topic='test-topic') + + def test_add_single_traits_fails_with_bad_version(self): + ret = self.put_json('/nodes/%s/traits/CUSTOM_TRAIT1' % self.node.uuid, + {}, headers={api_base.Version.string: "1.36"}, + expect_errors=True) + self.assertEqual(http_client.METHOD_NOT_ALLOWED, ret.status_code) + + @mock.patch.object(rpcapi.ConductorAPI, 'remove_node_traits') + def test_delete_all_traits(self, mock_remove): + ret = self.delete('/nodes/%s/traits' % self.node.name, + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_code) + mock_remove.assert_called_once_with(mock.ANY, self.node.id, + None, topic='test-topic') + + def test_delete_all_traits_fails_with_bad_version(self): + ret = self.delete('/nodes/%s/traits' % self.node.uuid, + headers={api_base.Version.string: "1.36"}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_code) + + @mock.patch.object(rpcapi.ConductorAPI, 'remove_node_traits') + def test_delete_trait(self, mock_remove): + ret = self.delete('/nodes/%s/traits/CUSTOM_1' % self.node.name, + headers={api_base.Version.string: self.version}) + self.assertEqual(http_client.NO_CONTENT, ret.status_code) + mock_remove.assert_called_once_with(mock.ANY, self.node.id, + ['CUSTOM_1'], topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'remove_node_traits') + def test_delete_trait_fails_if_node_locked(self, mock_remove): + mock_remove.side_effect = exception.NodeLocked( + node=self.node.uuid, host='host1') + ret = self.delete('/nodes/%s/traits/CUSTOM_1' % self.node.name, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.CONFLICT, ret.status_code) + mock_remove.assert_called_once_with(mock.ANY, self.node.id, + ['CUSTOM_1'], topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'remove_node_traits') + def test_delete_trait_fails_if_node_not_found(self, mock_remove): + mock_remove.side_effect = exception.NodeNotFound(node=self.node.uuid) + ret = self.delete('/nodes/%s/traits/CUSTOM_1' % self.node.name, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_code) + mock_remove.assert_called_once_with(mock.ANY, self.node.id, + ['CUSTOM_1'], topic='test-topic') + + @mock.patch.object(rpcapi.ConductorAPI, 'remove_node_traits') + def test_delete_trait_fails_if_trait_not_found(self, mock_remove): + mock_remove.side_effect = exception.NodeTraitNotFound( + node_id=self.node.uuid, trait='CUSTOM_12') + ret = self.delete('/nodes/%s/traits/CUSTOM_12' % self.node.name, + headers={api_base.Version.string: self.version}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_code) + mock_remove.assert_called_once_with(mock.ANY, self.node.id, + ['CUSTOM_12'], topic='test-topic') + + def test_delete_trait_fails_with_bad_version(self): + ret = self.delete('/nodes/%s/traits/CUSTOM_TRAIT1' % self.node.uuid, + headers={api_base.Version.string: "1.36"}, + expect_errors=True) + self.assertEqual(http_client.NOT_FOUND, ret.status_code) diff --git a/ironic/tests/unit/api/controllers/v1/test_utils.py b/ironic/tests/unit/api/controllers/v1/test_utils.py index 54f038da4f..96d8d6e583 100644 --- a/ironic/tests/unit/api/controllers/v1/test_utils.py +++ b/ironic/tests/unit/api/controllers/v1/test_utils.py @@ -15,6 +15,7 @@ # under the License. import mock +import os_traits from oslo_config import cfg from oslo_utils import uuidutils import pecan @@ -58,6 +59,28 @@ class TestApiUtils(base.TestCase): utils.validate_sort_dir, 'fake-sort') + def test_validate_trait(self): + utils.validate_trait(os_traits.HW_CPU_X86_AVX2) + utils.validate_trait("CUSTOM_1") + utils.validate_trait("CUSTOM_TRAIT_GOLD") + self.assertRaises(wsme.exc.ClientSideError, + utils.validate_trait, "A" * 256) + self.assertRaises(wsme.exc.ClientSideError, + utils.validate_trait, "CuSTOM_1") + self.assertRaises(wsme.exc.ClientSideError, + utils.validate_trait, "") + self.assertRaises(wsme.exc.ClientSideError, + utils.validate_trait, "CUSTOM_bob") + self.assertRaises(wsme.exc.ClientSideError, + utils.validate_trait, "CUSTOM_1-BOB") + self.assertRaises(wsme.exc.ClientSideError, + utils.validate_trait, "aCUSTOM_1a") + large = "CUSTOM_" + ("1" * 248) + self.assertEqual(255, len(large)) + utils.validate_trait(large) + self.assertRaises(wsme.exc.ClientSideError, + utils.validate_trait, large + "1") + def test_get_patch_values_no_path(self): patch = [{'path': '/name', 'op': 'update', 'value': 'node-0'}] path = '/invalid' @@ -427,6 +450,13 @@ class TestApiUtils(base.TestCase): mock_request.version.minor = 32 self.assertFalse(utils.allow_storage_interface()) + @mock.patch.object(pecan, 'request', spec_set=['version']) + def test_allow_traits(self, mock_request): + mock_request.version.minor = 37 + self.assertTrue(utils.allow_traits()) + mock_request.version.minor = 36 + self.assertFalse(utils.allow_traits()) + @mock.patch.object(pecan, 'request', spec_set=['version']) @mock.patch.object(objects.Port, 'supports_physical_network') def test_allow_port_physical_network_no_pin(self, mock_spn, mock_request): diff --git a/releasenotes/notes/node-traits-2d950b62eea24491.yaml b/releasenotes/notes/node-traits-2d950b62eea24491.yaml new file mode 100644 index 0000000000..8e576fd691 --- /dev/null +++ b/releasenotes/notes/node-traits-2d950b62eea24491.yaml @@ -0,0 +1,28 @@ +--- +features: + - | + Adds a ``traits`` field to the node resource, which will be used by the + Compute service to define which nodes may match a Compute flavor using + qualitative attributes. + + The following new endpoints have been added to the Bare Metal REST API in + version 1.37: + + * ``GET /v1/nodes//traits`` lists the traits for a node. + * ``PUT /v1/nodes//traits`` sets all traits for a node. + * ``PUT /v1/nodes//traits/`` adds a trait to a + node. + * ``DELETE /v1/nodes//traits`` removes all traits from a + node. + * ``DELETE /v1/nodes//traits/`` removes a trait + from a node. + + A node's traits are also included in the following node query and list + responses: + + * ``GET /v1/nodes/`` + * ``GET /v1/nodes/detail`` + * ``GET /v1/nodes?fields=traits`` + + Traits cannot be specified on node creation, nor can they be updated via a + ``PATCH`` request on the node. diff --git a/requirements.txt b/requirements.txt index de4ee8d7a5..fad6eb8c7f 100644 --- a/requirements.txt +++ b/requirements.txt @@ -30,6 +30,7 @@ oslo.serialization!=2.19.1,>=2.18.0 # Apache-2.0 oslo.service!=1.28.1,>=1.24.0 # Apache-2.0 oslo.utils>=3.33.0 # Apache-2.0 osprofiler>=1.4.0 # Apache-2.0 +os-traits>=0.4.0 # Apache-2.0 pecan!=1.0.2,!=1.0.3,!=1.0.4,!=1.2,>=1.0.0 # BSD requests>=2.14.2 # Apache-2.0 rfc3986>=0.3.1 # Apache-2.0