diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 1b5d56a9bc..dcdb1ca693 100644 --- a/ironic/common/exception.py +++ b/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'") diff --git a/ironic/db/api.py b/ironic/db/api.py index 3401c0216a..a9ab47a701 100644 --- a/ironic/db/api.py +++ b/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. + """ diff --git a/ironic/db/sqlalchemy/alembic/versions/b4130a7fc904_create_nodetraits_table.py b/ironic/db/sqlalchemy/alembic/versions/b4130a7fc904_create_nodetraits_table.py new file mode 100644 index 0000000000..86e3f3c3d4 --- /dev/null +++ b/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) diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 122dd363fa..3e5aaa1170 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/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() diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index dc1293ba56..3a1e7506ae 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/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) diff --git a/ironic/tests/unit/api/utils.py b/ironic/tests/unit/api/utils.py index cf497bc27d..595200e6de 100644 --- a/ironic/tests/unit/api/utils.py +++ b/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 diff --git a/ironic/tests/unit/common/test_release_mappings.py b/ironic/tests/unit/common/test_release_mappings.py index 6ce78cf976..64a174e139 100644 --- a/ironic/tests/unit/common/test_release_mappings.py +++ b/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. diff --git a/ironic/tests/unit/db/sqlalchemy/test_migrations.py b/ironic/tests/unit/db/sqlalchemy/test_migrations.py index 4f47efa68a..a9164ac6bb 100644 --- a/ironic/tests/unit/db/sqlalchemy/test_migrations.py +++ b/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') diff --git a/ironic/tests/unit/db/test_node_traits.py b/ironic/tests/unit/db/test_node_traits.py new file mode 100644 index 0000000000..7f78f60012 --- /dev/null +++ b/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') diff --git a/ironic/tests/unit/db/test_nodes.py b/ironic/tests/unit/db/test_nodes.py index 5f975b2f2a..ff09b62d9f 100644 --- a/ironic/tests/unit/db/test_nodes.py +++ b/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() diff --git a/ironic/tests/unit/db/utils.py b/ironic/tests/unit/db/utils.py index 8621a8ee8f..946a4e3d65 100644 --- a/ironic/tests/unit/db/utils.py +++ b/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 - if 'id' not in kw: - del node['id'] - # Create node with tags will raise an exception. If tags are not - # specified explicitly just delete it. - if 'tags' not in kw: - del node['tags'] + # Let DB generate an ID if one isn't specified explicitly. + # Creating a node with tags or traits will raise an exception. If tags or + # traits are not specified explicitly just delete them. + for field in {'id', 'tags', 'traits'}: + if field not in kw: + del node[field] 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'])