Node traits: Add DB API & model

Adds a NodeTrait model to the DB and DBAPI methods for the following:

* Set/unset node traits
* Get node traits
* Add/remove a single trait
* Check for existence of a node trait

All node traits are deleted when their owner node is deleted. Traits
cannot be specified on node creation currently.

Change-Id: I8b375a33dcb94d2cc73eaa93a5cc5aed86c27178
Partial-Bug: #1722194

ironic/common/exception.py

@@ -758,3 +758,7 @@ class VifInvalidForAttach(Conflict):
 class AgentAPIError(IronicException):
     _msg_fmt = _('Agent API for node %(node)s returned HTTP status code '
                  '%(status)s with error: %(error)s')
+
+
+class NodeTraitNotFound(IronicException):
+    _msg_fmt = _("Node %(node_id)s doesn't have a trait '%(trait)s'")

ironic/db/api.py

@@ -143,7 +143,7 @@ class Connection(object):
                          'properties': { ... },
                          'extra': { ... },
                         }
-        :raises: InvalidParameterValue if create a node with tags.
+        :raises: InvalidParameterValue if 'values' contains 'tags' or 'traits'.
         :returns: A node.
         """
 
@@ -186,7 +186,7 @@ class Connection(object):
         """Destroy a node and its associated resources.
 
         Destroy a node, including any associated ports, port groups,
-        tags, volume connectors, and volume targets.
+        tags, traits, volume connectors, and volume targets.
 
         :param node_id: The ID or UUID of a node.
         """
@@ -922,3 +922,69 @@ class Connection(object):
                   of migrated objects.
         """
         # TODO(rloo) Delete this in Rocky cycle.
+
+    @abc.abstractmethod
+    def set_node_traits(self, node_id, traits):
+        """Replace all of the node traits with specified list of traits.
+
+        This ignores duplicate traits in the specified list.
+
+        :param node_id: The id of a node.
+        :param traits: List of traits.
+        :returns: A list of NodeTrait objects.
+        :raises: InvalidParameterValue if setting the traits would exceed the
+            per-node traits limit.
+        :raises: NodeNotFound if the node is not found.
+        """
+
+    @abc.abstractmethod
+    def unset_node_traits(self, node_id):
+        """Remove all traits of the node.
+
+        :param node_id: The id of a node.
+        :raises: NodeNotFound if the node is not found.
+        """
+
+    @abc.abstractmethod
+    def get_node_traits_by_node_id(self, node_id):
+        """Get node traits based on its id.
+
+        :param node_id: The id of a node.
+        :returns: A list of NodeTrait objects.
+        :raises: NodeNotFound if the node is not found.
+        """
+
+    @abc.abstractmethod
+    def add_node_trait(self, node_id, trait):
+        """Add trait to the node.
+
+        If the node_id and trait pair already exists, this should still
+        succeed.
+
+        :param node_id: The id of a node.
+        :param trait: A trait string.
+        :returns: the NodeTrait object.
+        :raises: InvalidParameterValue if adding the trait would exceed the
+            per-node traits limit.
+        :raises: NodeNotFound if the node is not found.
+        """
+
+    @abc.abstractmethod
+    def delete_node_trait(self, node_id, trait):
+        """Delete specified trait from the node.
+
+        :param node_id: The id of a node.
+        :param trait: A trait string.
+        :raises: NodeNotFound if the node is not found.
+        :raises: NodeTraitNotFound if the trait is not found.
+        """
+
+    @abc.abstractmethod
+    def node_trait_exists(self, node_id, trait):
+        """Check if the specified trait exists on the node.
+
+        :param node_id: The id of a node.
+        :param trait: A trait string.
+        :returns: True if the trait exists otherwise False.
+        :raises: NodeNotFound if the node is not found.
+        """

ironic/db/sqlalchemy/alembic/versions/b4130a7fc904_create_nodetraits_table.py

@@ -0,0 +1,43 @@
+#    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.
+
+"""Create node_traits table
+
+Revision ID: b4130a7fc904
+Revises: 405cfe08f18d
+Create Date: 2017-12-20 10:20:07.911788
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = 'b4130a7fc904'
+down_revision = '405cfe08f18d'
+
+from alembic import op
+import sqlalchemy as sa
+
+
+def upgrade():
+    op.create_table(
+        'node_traits',
+        sa.Column('version', sa.String(length=15), nullable=True),
+        sa.Column('created_at', sa.DateTime(), nullable=True),
+        sa.Column('updated_at', sa.DateTime(), nullable=True),
+        sa.Column('node_id', sa.Integer(), nullable=False,
+                  autoincrement=False),
+        sa.Column('trait', sa.String(length=255), nullable=False),
+        sa.ForeignKeyConstraint(['node_id'], ['nodes.id'], ),
+        sa.PrimaryKeyConstraint('node_id', 'trait'),
+        mysql_ENGINE='InnoDB',
+        mysql_DEFAULT_CHARSET='UTF8'
+    )
+    op.create_index('node_traits_idx', 'node_traits', ['trait'], unique=False)

ironic/db/sqlalchemy/api.py

@@ -48,6 +48,10 @@ LOG = log.getLogger(__name__)
 
 _CONTEXT = threading.local()
 
+# NOTE(mgoddard): We limit the number of traits per node to 50 as this is the
+# maximum number of traits per resource provider allowed in placement.
+MAX_TRAITS_PER_NODE = 50
+
 
 def get_backend():
     """The backend is this module itself."""
@@ -324,6 +328,11 @@ class Connection(api.Connection):
             msg = _("Cannot create node with tags.")
             raise exception.InvalidParameterValue(err=msg)
 
+        # TODO(mgoddard): Support creating node with traits
+        if 'traits' in values:
+            msg = _("Cannot create node with traits.")
+            raise exception.InvalidParameterValue(err=msg)
+
         node = models.Node()
         node.update(values)
         with _session_for_write() as session:
@@ -338,8 +347,9 @@ class Connection(api.Connection):
                         instance_uuid=values['instance_uuid'],
                         node=values['uuid'])
                 raise exception.NodeAlreadyExists(uuid=values['uuid'])
-            # Set tags to [] for new created node
+            # Set tags & traits to [] for new created node
             node['tags'] = []
+            node['traits'] = []
             return node
 
     def get_node_by_id(self, node_id):
@@ -409,6 +419,11 @@ class Connection(api.Connection):
             tag_query = model_query(models.NodeTag).filter_by(node_id=node_id)
             tag_query.delete()
 
+            # Delete all traits attached to the node
+            trait_query = model_query(
+                models.NodeTrait).filter_by(node_id=node_id)
+            trait_query.delete()
+
             volume_connector_query = model_query(
                 models.VolumeConnector).filter_by(node_id=node_id)
             volume_connector_query.delete()
@@ -1265,3 +1280,87 @@ class Connection(api.Connection):
                 break
 
         return total_to_migrate, total_migrated
+
+    @staticmethod
+    def _verify_max_traits_per_node(node_id, num_traits):
+        """Verify that an operation would not exceed the per-node trait limit.
+
+        :param node_id: The ID of a node.
+        :param num_traits: The number of traits the node would have after the
+            operation.
+        :raises: InvalidParameterValue if the operation would exceed the
+            per-node trait limit.
+        """
+        if num_traits > MAX_TRAITS_PER_NODE:
+            msg = _("Could not modify traits for node %(node_id)s as it would "
+                    "exceed the maximum number of traits per node "
+                    "(%(num_traits)d vs. %(max_traits)d)")
+            raise exception.InvalidParameterValue(
+                msg, node_id=node_id, num_traits=num_traits,
+                max_traits=MAX_TRAITS_PER_NODE)
+
+    @oslo_db_api.retry_on_deadlock
+    def set_node_traits(self, node_id, traits):
+        # Remove duplicate traits
+        traits = set(traits)
+
+        self._verify_max_traits_per_node(node_id, len(traits))
+
+        with _session_for_write() as session:
+            # NOTE(mgoddard): Node existence is checked in unset_node_traits.
+            self.unset_node_traits(node_id)
+            node_traits = []
+            for trait in traits:
+                node_trait = models.NodeTrait(trait=trait, node_id=node_id)
+                session.add(node_trait)
+                node_traits.append(node_trait)
+
+        return node_traits
+
+    @oslo_db_api.retry_on_deadlock
+    def unset_node_traits(self, node_id):
+        self._check_node_exists(node_id)
+        with _session_for_write():
+            model_query(models.NodeTrait).filter_by(node_id=node_id).delete()
+
+    def get_node_traits_by_node_id(self, node_id):
+        self._check_node_exists(node_id)
+        result = (model_query(models.NodeTrait)
+                  .filter_by(node_id=node_id)
+                  .all())
+        return result
+
+    @oslo_db_api.retry_on_deadlock
+    def add_node_trait(self, node_id, trait):
+        node_trait = models.NodeTrait(trait=trait, node_id=node_id)
+
+        self._check_node_exists(node_id)
+        try:
+            with _session_for_write() as session:
+                session.add(node_trait)
+                session.flush()
+
+                num_traits = (model_query(models.NodeTrait)
+                              .filter_by(node_id=node_id).count())
+                self._verify_max_traits_per_node(node_id, num_traits)
+        except db_exc.DBDuplicateEntry:
+            # NOTE(mgoddard): Ignore traits duplicates
+            pass
+
+        return node_trait
+
+    @oslo_db_api.retry_on_deadlock
+    def delete_node_trait(self, node_id, trait):
+        self._check_node_exists(node_id)
+        with _session_for_write():
+            result = model_query(models.NodeTrait).filter_by(
+                node_id=node_id, trait=trait).delete()
+
+            if not result:
+                raise exception.NodeTraitNotFound(node_id=node_id, trait=trait)
+
+    def node_trait_exists(self, node_id, trait):
+        self._check_node_exists(node_id)
+        q = model_query(
+            models.NodeTrait).filter_by(node_id=node_id, trait=trait)
+        return model_query(q.exists()).scalar()

ironic/db/sqlalchemy/models.py

@@ -278,3 +278,15 @@ class VolumeTarget(Base):
     boot_index = Column(Integer)
     volume_id = Column(String(36))
     extra = Column(db_types.JsonEncodedDict)
+
+
+class NodeTrait(Base):
+    """Represents a trait of a bare metal node."""
+
+    __tablename__ = 'node_traits'
+    __table_args__ = (
+        Index('node_traits_idx', 'trait'),
+        table_args())
+    node_id = Column(Integer, ForeignKey('nodes.id'),
+                     primary_key=True, nullable=False)
+    trait = Column(String(255), primary_key=True, nullable=False)

ironic/tests/unit/api/utils.py

@@ -99,6 +99,7 @@ def node_post_data(**kw):
     node.pop('conductor_affinity')
     node.pop('chassis_id')
     node.pop('tags')
+    node.pop('traits')
 
     # NOTE(jroll): pop out fields that were introduced in later API versions,
     # unless explicitly requested. Otherwise, these will cause tests using

ironic/tests/unit/common/test_release_mappings.py

@@ -91,7 +91,8 @@ class ReleaseMappingsTestCase(base.TestCase):
     def test_contains_all_db_objects(self):
         self.assertIn('master', release_mappings.RELEASE_MAPPING)
         model_names = set((s.__name__ for s in models.Base.__subclasses__()))
-        exceptions = set(['NodeTag', 'ConductorHardwareInterfaces'])
+        exceptions = set(['NodeTag', 'ConductorHardwareInterfaces',
+                          'NodeTrait'])
         # NOTE(xek): As a rule, all models which can be changed between
         # releases or are sent through RPC should have their counterpart
         # versioned objects.

ironic/tests/unit/db/sqlalchemy/test_migrations.py

@@ -640,6 +640,31 @@ class MigrationCheckersMixin(object):
         self.assertIsInstance(nodes.c.rescue_interface.type,
                               sqlalchemy.types.String)
 
+    def _pre_upgrade_b4130a7fc904(self, engine):
+        # Create a node to which traits can be added.
+        data = {'uuid': uuidutils.generate_uuid()}
+        nodes = db_utils.get_table(engine, 'nodes')
+        nodes.insert().execute(data)
+        node = nodes.select(nodes.c.uuid == data['uuid']).execute().first()
+        data['id'] = node['id']
+        return data
+
+    def _check_b4130a7fc904(self, engine, data):
+        node_traits = db_utils.get_table(engine, 'node_traits')
+        col_names = [column.name for column in node_traits.c]
+        self.assertIn('node_id', col_names)
+        self.assertIsInstance(node_traits.c.node_id.type,
+                              sqlalchemy.types.Integer)
+        self.assertIn('trait', col_names)
+        self.assertIsInstance(node_traits.c.trait.type,
+                              sqlalchemy.types.String)
+
+        trait = {'node_id': data['id'], 'trait': 'trait1'}
+        node_traits.insert().execute(trait)
+        trait = node_traits.select(
+            node_traits.c.node_id == data['id']).execute().first()
+        self.assertEqual('trait1', trait['trait'])
+
     def test_upgrade_and_version(self):
         with patch_with_engine(self.engine):
             self.migration_api.upgrade('head')

ironic/tests/unit/db/test_node_traits.py

@@ -0,0 +1,160 @@
+#    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.
+
+"""Tests for manipulating NodeTraits via the DB API"""
+
+from ironic.common import exception
+from ironic.tests.unit.db import base
+from ironic.tests.unit.db import utils as db_utils
+
+
+class DbNodeTraitTestCase(base.DbTestCase):
+
+    def setUp(self):
+        super(DbNodeTraitTestCase, self).setUp()
+        self.node = db_utils.create_test_node()
+
+    def test_set_node_traits(self):
+        result = self.dbapi.set_node_traits(self.node.id, ['trait1', 'trait2'])
+        self.assertEqual(self.node.id, result[0].node_id)
+        self.assertItemsEqual(['trait1', 'trait2'],
+                              [trait.trait for trait in result])
+
+        result = self.dbapi.set_node_traits(self.node.id, [])
+        self.assertEqual([], result)
+
+    def test_set_node_traits_duplicate(self):
+        result = self.dbapi.set_node_traits(self.node.id,
+                                            ['trait1', 'trait2', 'trait2'])
+        self.assertEqual(self.node.id, result[0].node_id)
+        self.assertItemsEqual(['trait1', 'trait2'],
+                              [trait.trait for trait in result])
+
+    def test_set_node_traits_at_limit(self):
+        traits = ['trait%d' % n for n in range(50)]
+        result = self.dbapi.set_node_traits(self.node.id, traits)
+        self.assertEqual(self.node.id, result[0].node_id)
+        self.assertItemsEqual(traits, [trait.trait for trait in result])
+
+    def test_set_node_traits_over_limit(self):
+        traits = ['trait%d' % n for n in range(51)]
+        self.assertRaises(exception.InvalidParameterValue,
+                          self.dbapi.set_node_traits, self.node.id, traits)
+        # Ensure the traits were not set.
+        result = self.dbapi.get_node_traits_by_node_id(self.node.id)
+        self.assertEqual([], result)
+
+    def test_set_node_traits_node_not_exist(self):
+        self.assertRaises(exception.NodeNotFound,
+                          self.dbapi.set_node_traits, '1234',
+                          ['trait1', 'trait2'])
+
+    def test_get_node_traits_by_node_id(self):
+        self.dbapi.set_node_traits(self.node.id, ['trait1', 'trait2'])
+        result = self.dbapi.get_node_traits_by_node_id(self.node.id)
+        self.assertEqual(self.node.id, result[0].node_id)
+        self.assertItemsEqual(['trait1', 'trait2'],
+                              [trait.trait for trait in result])
+
+    def test_get_node_traits_empty(self):
+        result = self.dbapi.get_node_traits_by_node_id(self.node.id)
+        self.assertEqual([], result)
+
+    def test_get_node_traits_node_not_exist(self):
+        self.assertRaises(exception.NodeNotFound,
+                          self.dbapi.get_node_traits_by_node_id, '123')
+
+    def test_unset_node_traits(self):
+        self.dbapi.set_node_traits(self.node.id, ['trait1', 'trait2'])
+        self.dbapi.unset_node_traits(self.node.id)
+        result = self.dbapi.get_node_traits_by_node_id(self.node.id)
+        self.assertEqual([], result)
+
+    def test_unset_empty_node_traits(self):
+        self.dbapi.unset_node_traits(self.node.id)
+        result = self.dbapi.get_node_traits_by_node_id(self.node.id)
+        self.assertEqual([], result)
+
+    def test_unset_node_traits_node_not_exist(self):
+        self.assertRaises(exception.NodeNotFound,
+                          self.dbapi.unset_node_traits, '123')
+
+    def test_add_node_trait(self):
+        result = self.dbapi.add_node_trait(self.node.id, 'trait1')
+        self.assertEqual(self.node.id, result.node_id)
+        self.assertEqual('trait1', result.trait)
+
+    def test_add_node_trait_duplicate(self):
+        self.dbapi.add_node_trait(self.node.id, 'trait1')
+        result = self.dbapi.add_node_trait(self.node.id, 'trait1')
+        self.assertEqual(self.node.id, result.node_id)
+        self.assertEqual('trait1', result.trait)
+        result = self.dbapi.get_node_traits_by_node_id(self.node.id)
+        self.assertEqual(['trait1'], [trait.trait for trait in result])
+
+    def test_add_node_trait_at_limit(self):
+        traits = ['trait%d' % n for n in range(49)]
+        self.dbapi.set_node_traits(self.node.id, traits)
+
+        result = self.dbapi.add_node_trait(self.node.id, 'trait49')
+        self.assertEqual(self.node.id, result.node_id)
+        self.assertEqual('trait49', result.trait)
+
+    def test_add_node_trait_duplicate_at_limit(self):
+        traits = ['trait%d' % n for n in range(50)]
+        self.dbapi.set_node_traits(self.node.id, traits)
+
+        result = self.dbapi.add_node_trait(self.node.id, 'trait49')
+        self.assertEqual(self.node.id, result.node_id)
+        self.assertEqual('trait49', result.trait)
+
+    def test_add_node_trait_over_limit(self):
+        traits = ['trait%d' % n for n in range(50)]
+        self.dbapi.set_node_traits(self.node.id, traits)
+
+        self.assertRaises(exception.InvalidParameterValue,
+                          self.dbapi.add_node_trait, self.node.id, 'trait50')
+        # Ensure the trait was not added.
+        result = self.dbapi.get_node_traits_by_node_id(self.node.id)
+        self.assertNotIn('trait50', [trait.trait for trait in result])
+
+    def test_add_node_trait_node_not_exist(self):
+        self.assertRaises(exception.NodeNotFound,
+                          self.dbapi.add_node_trait, '123', 'trait1')
+
+    def test_delete_node_trait(self):
+        self.dbapi.set_node_traits(self.node.id, ['trait1', 'trait2'])
+        self.dbapi.delete_node_trait(self.node.id, 'trait1')
+        result = self.dbapi.get_node_traits_by_node_id(self.node.id)
+        self.assertEqual(1, len(result))
+        self.assertEqual('trait2', result[0].trait)
+
+    def test_delete_node_trait_not_found(self):
+        self.assertRaises(exception.NodeTraitNotFound,
+                          self.dbapi.delete_node_trait, self.node.id, 'trait1')
+
+    def test_delete_node_trait_node_not_found(self):
+        self.assertRaises(exception.NodeNotFound,
+                          self.dbapi.delete_node_trait, '123', 'trait1')
+
+    def test_node_trait_exists(self):
+        self.dbapi.set_node_traits(self.node.id, ['trait1', 'trait2'])
+        result = self.dbapi.node_trait_exists(self.node.id, 'trait1')
+        self.assertTrue(result)
+
+    def test_node_trait_not_exists(self):
+        result = self.dbapi.node_trait_exists(self.node.id, 'trait1')
+        self.assertFalse(result)
+
+    def test_node_trait_node_not_exist(self):
+        self.assertRaises(exception.NodeNotFound,
+                          self.dbapi.node_trait_exists, '123', 'trait1')

ironic/tests/unit/db/test_nodes.py

@@ -38,6 +38,11 @@ class DbNodeTestCase(base.DbTestCase):
                           utils.create_test_node,
                           tags=['tag1', 'tag2'])
 
+    def test_create_node_with_traits(self):
+        self.assertRaises(exception.InvalidParameterValue,
+                          utils.create_test_node,
+                          traits=['trait1', 'trait2'])
+
     def test_create_node_already_exists(self):
         utils.create_test_node()
         self.assertRaises(exception.NodeAlreadyExists,
@@ -399,6 +404,26 @@ class DbNodeTestCase(base.DbTestCase):
         self.assertRaises(exception.VolumeTargetNotFound,
                           self.dbapi.get_volume_target_by_id, target.id)
 
+    def test_traits_get_destroyed_after_destroying_a_node(self):
+        node = utils.create_test_node()
+
+        trait = utils.create_test_node_trait(node_id=node.id)
+
+        self.assertTrue(self.dbapi.node_trait_exists(node.id, trait.trait))
+        self.dbapi.destroy_node(node.id)
+        self.assertRaises(exception.NodeNotFound,
+                          self.dbapi.node_trait_exists, node.id, trait.trait)
+
+    def test_traits_get_destroyed_after_destroying_a_node_by_uuid(self):
+        node = utils.create_test_node()
+
+        trait = utils.create_test_node_trait(node_id=node.id)
+
+        self.assertTrue(self.dbapi.node_trait_exists(node.id, trait.trait))
+        self.dbapi.destroy_node(node.uuid)
+        self.assertRaises(exception.NodeNotFound,
+                          self.dbapi.node_trait_exists, node.id, trait.trait)
+
     def test_update_node(self):
         node = utils.create_test_node()
 

ironic/tests/unit/db/utils.py

@@ -194,6 +194,7 @@ def get_test_node(**kw):
         'target_raid_config': kw.get('target_raid_config'),
         'tags': kw.get('tags', []),
         'resource_class': kw.get('resource_class'),
+        'traits': kw.get('traits', []),
     }
 
     for iface in drivers_base.ALL_INTERFACES:
@@ -213,13 +214,12 @@ def create_test_node(**kw):
 
     """
     node = get_test_node(**kw)
-    # Let DB generate ID if it isn't specified explicitly
+    # Let DB generate an ID if one isn't specified explicitly.
-    if 'id' not in kw:
+    # Creating a node with tags or traits will raise an exception. If tags or
-        del node['id']
+    # traits are not specified explicitly just delete them.
-    # Create node with tags will raise an exception. If tags are not
+    for field in {'id', 'tags', 'traits'}:
-    # specified explicitly just delete it.
+        if field not in kw:
-    if 'tags' not in kw:
+            del node[field]
-        del node['tags']
     dbapi = db_api.get_instance()
     return dbapi.create_node(node)
 
@@ -489,3 +489,28 @@ def create_test_node_tag(**kw):
     tag = get_test_node_tag(**kw)
     dbapi = db_api.get_instance()
     return dbapi.add_node_tag(tag['node_id'], tag['tag'])
+
+
+def get_test_node_trait(**kw):
+    return {
+        # TODO(mgoddard): Replace None below with the NodeTrait RPC object
+        # VERSION when the RPC object is added.
+        'version': kw.get('version', None),
+        "trait": kw.get("trait", "trait1"),
+        "node_id": kw.get("node_id", "123"),
+        'created_at': kw.get('created_at'),
+        'updated_at': kw.get('updated_at'),
+    }
+
+
+def create_test_node_trait(**kw):
+    """Create test node trait entry in DB and return NodeTrait DB object.
+
+    Function to be used to create test NodeTrait objects in the database.
+
+    :param kw: kwargs with overriding values for trait's attributes.
+    :returns: Test NodeTrait DB object.
+    """
+    trait = get_test_node_trait(**kw)
+    dbapi = db_api.get_instance()
+    return dbapi.add_node_trait(trait['node_id'], trait['trait'])