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 <mark@stackhpc.com>

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/<node identifier>/traits lists the traits for a node.
+
+* PUT /v1/nodes/<node identifier>/traits sets all traits for a node.
+
+* PUT /v1/nodes/<node identifier>/traits/<trait> adds a trait to a node.
+
+* DELETE /v1/nodes/<node identifier>/traits removes all traits from a node.
+
+* DELETE /v1/nodes/<node identifier>/traits/<trait> removes a trait from a
+  node.
+
+A node's traits are also included the following node query and list responses:
+
+* GET /v1/nodes/<node identifier>
+
+* 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)
 ---------------------
 

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

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/<node identifier>/traits/<trait> with no body, "
+                    "or all node traits may be replaced via PUT "
+                    "/v1/nodes/<node identifier>/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'}]

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

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)

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'")
 
 

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 = [

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'],

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)

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):

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/<node identifier>/traits`` lists the traits for a node.
+    * ``PUT /v1/nodes/<node identifier>/traits`` sets all traits for a node.
+    * ``PUT /v1/nodes/<node identifier>/traits/<trait>`` adds a trait to a
+      node.
+    * ``DELETE /v1/nodes/<node identifier>/traits`` removes all traits from a
+      node.
+    * ``DELETE /v1/nodes/<node identifier>/traits/<trait>`` removes a trait
+      from a node.
+
+    A node's traits are also included in the following node query and list
+    responses:
+
+    * ``GET /v1/nodes/<node identifier>``
+    * ``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.

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