From 59b0dc459907a0c13562c70037ee32d25742ca73 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Tue, 25 Oct 2022 10:37:06 +0100 Subject: [PATCH] Implements node inventory: database Prepare the ironic database to accommodate node inventory received from the inspector once the API is implemented. Story: 2010275 Task: 46204 Change-Id: I6b830e5cc30f1fa1f1900e7c45e6f246fa1ec51c --- ironic/common/exception.py | 4 + ironic/common/release_mappings.py | 1 + ironic/db/api.py | 30 +++++ .../0ac0f39bc5aa_add_node_inventory_table.py | 46 ++++++++ ironic/db/sqlalchemy/api.py | 42 +++++++ ironic/db/sqlalchemy/models.py | 12 ++ ironic/objects/__init__.py | 1 + ironic/objects/node_inventory.py | 104 ++++++++++++++++++ .../unit/db/sqlalchemy/test_migrations.py | 31 ++++++ ironic/tests/unit/db/test_node_inventory.py | 47 ++++++++ ironic/tests/unit/db/test_nodes.py | 9 ++ ironic/tests/unit/db/utils.py | 27 +++++ .../tests/unit/objects/test_node_inventory.py | 61 ++++++++++ ironic/tests/unit/objects/test_objects.py | 1 + 14 files changed, 416 insertions(+) create mode 100644 ironic/db/sqlalchemy/alembic/versions/0ac0f39bc5aa_add_node_inventory_table.py create mode 100644 ironic/objects/node_inventory.py create mode 100644 ironic/tests/unit/db/test_node_inventory.py create mode 100644 ironic/tests/unit/objects/test_node_inventory.py diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 38316d2e7b..6714b7d4af 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -828,6 +828,10 @@ class NodeHistoryNotFound(NotFound): _msg_fmt = _("Node history record %(history)s could not be found.") +class NodeInventoryNotFound(NotFound): + _msg_fmt = _("Node inventory record %(inventory)s could not be found.") + + class IncorrectConfiguration(IronicException): _msg_fmt = _("Supplied configuration is incorrect and must be fixed. " "Error: %(error)s") diff --git a/ironic/common/release_mappings.py b/ironic/common/release_mappings.py index d32027b597..555ff164d1 100644 --- a/ironic/common/release_mappings.py +++ b/ironic/common/release_mappings.py @@ -518,6 +518,7 @@ RELEASE_MAPPING = { 'BIOSSetting': ['1.1'], 'Node': ['1.36'], 'NodeHistory': ['1.0'], + 'NodeInventory': ['1.0'], 'Conductor': ['1.3'], 'Chassis': ['1.3'], 'Deployment': ['1.0'], diff --git a/ironic/db/api.py b/ironic/db/api.py index 45e3fe2caa..dc723a5fc8 100644 --- a/ironic/db/api.py +++ b/ironic/db/api.py @@ -1425,3 +1425,33 @@ class Connection(object, metaclass=abc.ABCMeta): count operation. This can be a single provision state value or a list of values. """ + + @abc.abstractmethod + def create_node_inventory(self, values): + """Create a new inventory record. + + :param values: Dict of values. + """ + + @abc.abstractmethod + def destroy_node_inventory_by_node_id(self, inventory_node_id): + """Destroy a inventory record. + + :param inventory_uuid: The uuid of a inventory record + """ + + @abc.abstractmethod + def get_node_inventory_by_id(self, inventory_id): + """Return a node inventory representation. + + :param inventory_id: The id of a inventory record. + :returns: An inventory of a node. + """ + + @abc.abstractmethod + def get_node_inventory_by_node_id(self, node_id): + """Get the node inventory for a given node. + + :param node_id: The integer node ID. + :returns: An inventory of a node. + """ diff --git a/ironic/db/sqlalchemy/alembic/versions/0ac0f39bc5aa_add_node_inventory_table.py b/ironic/db/sqlalchemy/alembic/versions/0ac0f39bc5aa_add_node_inventory_table.py new file mode 100644 index 0000000000..c6a12f6ddd --- /dev/null +++ b/ironic/db/sqlalchemy/alembic/versions/0ac0f39bc5aa_add_node_inventory_table.py @@ -0,0 +1,46 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + + +"""add node inventory table + +Revision ID: 0ac0f39bc5aa +Revises: 9ef41f07cb58 +Create Date: 2022-10-25 17:15:38.181544 + +""" + +from alembic import op +from oslo_db.sqlalchemy import types as db_types +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision = '0ac0f39bc5aa' +down_revision = '9ef41f07cb58' + + +def upgrade(): + op.create_table('node_inventory', + 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('id', sa.Integer(), nullable=False), + sa.Column('inventory_data', db_types.JsonEncodedDict( + mysql_as_long=True).impl, nullable=True), + sa.Column('plugin_data', db_types.JsonEncodedDict( + mysql_as_long=True).impl, nullable=True), + sa.Column('node_id', sa.Integer(), nullable=True), + sa.PrimaryKeyConstraint('id'), + sa.ForeignKeyConstraint(['node_id'], ['nodes.id'], ), + sa.Index('inventory_node_id_idx', 'node_id'), + mysql_engine='InnoDB', + mysql_charset='UTF8MB3') diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 7902e9634e..a0cefea35b 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -859,6 +859,11 @@ class Connection(api.Connection): models.NodeHistory).filter_by(node_id=node_id) history_query.delete() + # delete all inventory for this node + inventory_query = session.query( + models.NodeInventory).filter_by(node_id=node_id) + inventory_query.delete() + query.delete() def update_node(self, node_id, values): @@ -2548,3 +2553,40 @@ class Connection(api.Connection): ) ) ) + + @oslo_db_api.retry_on_deadlock + def create_node_inventory(self, values): + inventory = models.NodeInventory() + inventory.update(values) + with _session_for_write() as session: + try: + session.add(inventory) + session.flush() + except db_exc.DBDuplicateEntry: + raise exception.NodeInventoryAlreadyExists( + id=values['id']) + return inventory + + @oslo_db_api.retry_on_deadlock + def destroy_node_inventory_by_node_id(self, node_id): + with _session_for_write() as session: + query = session.query(models.NodeInventory).filter_by( + node_id=node_id) + count = query.delete() + if count == 0: + raise exception.NodeInventoryNotFound( + node_id=node_id) + + def get_node_inventory_by_id(self, inventory_id): + query = model_query(models.NodeInventory).filter_by(id=inventory_id) + try: + return query.one() + except NoResultFound: + raise exception.NodeInventoryNotFound(inventory=inventory_id) + + def get_node_inventory_by_node_id(self, node_id): + query = model_query(models.NodeInventory).filter_by(node_id=node_id) + try: + return query.one() + except NoResultFound: + raise exception.NodeInventoryNotFound(node_id=node_id) diff --git a/ironic/db/sqlalchemy/models.py b/ironic/db/sqlalchemy/models.py index fad29f095f..34de1b364a 100644 --- a/ironic/db/sqlalchemy/models.py +++ b/ironic/db/sqlalchemy/models.py @@ -465,6 +465,18 @@ class NodeHistory(Base): node_id = Column(Integer, ForeignKey('nodes.id'), nullable=True) +class NodeInventory(Base): + """Represents an inventory of a baremetal node.""" + __tablename__ = 'node_inventory' + __table_args__ = ( + Index('inventory_node_id_idx', 'node_id'), + table_args()) + id = Column(Integer, primary_key=True) + inventory_data = Column(db_types.JsonEncodedDict(mysql_as_long=True)) + plugin_data = Column(db_types.JsonEncodedDict(mysql_as_long=True)) + node_id = Column(Integer, ForeignKey('nodes.id'), nullable=True) + + def get_class(model_name): """Returns the model class with the specified name. diff --git a/ironic/objects/__init__.py b/ironic/objects/__init__.py index e8de08d5ab..ae0307af2c 100644 --- a/ironic/objects/__init__.py +++ b/ironic/objects/__init__.py @@ -32,6 +32,7 @@ def register_all(): __import__('ironic.objects.deployment') __import__('ironic.objects.node') __import__('ironic.objects.node_history') + __import__('ironic.objects.node_inventory') __import__('ironic.objects.port') __import__('ironic.objects.portgroup') __import__('ironic.objects.trait') diff --git a/ironic/objects/node_inventory.py b/ironic/objects/node_inventory.py new file mode 100644 index 0000000000..db7ddb2702 --- /dev/null +++ b/ironic/objects/node_inventory.py @@ -0,0 +1,104 @@ +# 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. + +from oslo_versionedobjects import base as object_base + +from ironic.db import api as dbapi +from ironic.objects import base +from ironic.objects import fields as object_fields + + +@base.IronicObjectRegistry.register +class NodeInventory(base.IronicObject, object_base.VersionedObjectDictCompat): + # Version 1.0: Initial version + VERSION = '1.0' + + dbapi = dbapi.get_instance() + + fields = { + 'id': object_fields.IntegerField(), + 'node_id': object_fields.IntegerField(nullable=True), + 'inventory_data': object_fields.FlexibleDictField(nullable=True), + 'plugin_data': object_fields.FlexibleDictField(nullable=True), + } + + @classmethod + def _from_node_object(cls, context, node): + """Convert a node into a virtual `NodeInventory` object.""" + result = cls(context) + result._update_from_node_object(node) + return result + + def _update_from_node_object(self, node): + """Update the NodeInventory object from the node.""" + for src, dest in self.node_mapping.items(): + setattr(self, dest, getattr(node, src, None)) + for src, dest in self.instance_info_mapping.items(): + setattr(self, dest, node.instance_info.get(src)) + + @classmethod + def get_by_id(cls, context, inventory_id): + """Get a NodeInventory object by its integer ID. + + :param cls: the :class:`NodeInventory` + :param context: Security context + :param history_id: The ID of a inventory. + :returns: A :class:`NodeInventory` object. + :raises: NodeInventoryNotFound + + """ + db_inventory = cls.dbapi.get_node_inventory_by_id(inventory_id) + inventory = cls._from_db_object(context, cls(), db_inventory) + return inventory + + @classmethod + def get_by_node_id(cls, context, node_id): + """Get a NodeInventory object by its node ID. + + :param cls: the :class:`NodeInventory` + :param context: Security context + :param uuid: The UUID of a NodeInventory. + :returns: A :class:`NodeInventory` object. + :raises: NodeInventoryNotFound + + """ + db_inventory = cls.dbapi.get_node_inventory_by_node_id(node_id) + inventory = cls._from_db_object(context, cls(), db_inventory) + return inventory + + def create(self, context=None): + """Create a NodeInventory record in the DB. + + :param context: Security context. NOTE: This should only + be used internally by the indirection_api. + Unfortunately, RPC requires context as the first + argument, even though we don't use it. + A context should be set when instantiating the + object, e.g.: NodeHistory(context) + """ + values = self.do_version_changes_for_db() + db_inventory = self.dbapi.create_node_inventory(values) + self._from_db_object(self._context, self, db_inventory) + + def destroy(self, context=None): + """Delete the NodeHistory from the DB. + + :param context: Security context. NOTE: This should only + be used internally by the indirection_api. + Unfortunately, RPC requires context as the first + argument, even though we don't use it. + A context should be set when instantiating the + object, e.g.: NodeInventory(context) + :raises: NodeInventoryNotFound + """ + self.dbapi.destroy_node_inventory_by_node_id(self.node_id) + self.obj_reset_changes() diff --git a/ironic/tests/unit/db/sqlalchemy/test_migrations.py b/ironic/tests/unit/db/sqlalchemy/test_migrations.py index 90669dbd96..f9081a6dff 100644 --- a/ironic/tests/unit/db/sqlalchemy/test_migrations.py +++ b/ironic/tests/unit/db/sqlalchemy/test_migrations.py @@ -1240,6 +1240,23 @@ class MigrationCheckersMixin(object): self.assertIsInstance(node_history.c.user.type, sqlalchemy.types.String) + def _check_0ac0f39bc5aa(self, engine, data): + node_inventory = db_utils.get_table(engine, 'node_inventory') + col_names = [column.name for column in node_inventory.c] + + expected_names = ['version', 'created_at', 'updated_at', 'id', + 'node_id', 'inventory_data', 'plugin_data'] + self.assertEqual(sorted(expected_names), sorted(col_names)) + + self.assertIsInstance(node_inventory.c.created_at.type, + sqlalchemy.types.DateTime) + self.assertIsInstance(node_inventory.c.updated_at.type, + sqlalchemy.types.DateTime) + self.assertIsInstance(node_inventory.c.id.type, + sqlalchemy.types.Integer) + self.assertIsInstance(node_inventory.c.node_id.type, + sqlalchemy.types.Integer) + def test_upgrade_and_version(self): with patch_with_engine(self.engine): self.migration_api.upgrade('head') @@ -1326,6 +1343,13 @@ class TestMigrationsMySQL(MigrationCheckersMixin, "'%s'" % data['uuid'])).one() self.assertEqual(test_text, i_info[0]) + def _check_0ac0f39bc5aa(self, engine, data): + node_inventory = db_utils.get_table(engine, 'node_inventory') + self.assertIsInstance(node_inventory.c.inventory_data.type, + sqlalchemy.dialects.mysql.LONGTEXT) + self.assertIsInstance(node_inventory.c.plugin_data.type, + sqlalchemy.dialects.mysql.LONGTEXT) + class TestMigrationsPostgreSQL(MigrationCheckersMixin, WalkVersionsMixin, @@ -1333,6 +1357,13 @@ class TestMigrationsPostgreSQL(MigrationCheckersMixin, test_base.BaseTestCase): FIXTURE = test_fixtures.PostgresqlOpportunisticFixture + def _check_0ac0f39bc5aa(self, engine, data): + node_inventory = db_utils.get_table(engine, 'node_inventory') + self.assertIsInstance(node_inventory.c.inventory_data.type, + sqlalchemy.types.Text) + self.assertIsInstance(node_inventory.c.plugin_data.type, + sqlalchemy.types.Text) + class ModelsMigrationSyncMixin(object): diff --git a/ironic/tests/unit/db/test_node_inventory.py b/ironic/tests/unit/db/test_node_inventory.py new file mode 100644 index 0000000000..a146903f73 --- /dev/null +++ b/ironic/tests/unit/db/test_node_inventory.py @@ -0,0 +1,47 @@ +# 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. + +from ironic.common import exception +from ironic.tests.unit.db import base +from ironic.tests.unit.db import utils as db_utils + + +class DBNodeInventoryTestCase(base.DbTestCase): + + def setUp(self): + super(DBNodeInventoryTestCase, self).setUp() + self.node = db_utils.create_test_node() + self.inventory = db_utils.create_test_inventory( + id=0, node_id=self.node.id, + inventory_data={"inventory": "test_inventory"}, + plugin_data={"plugin_data": "test_plugin_data"}) + + def test_destroy_node_inventory_by_node_id(self): + self.dbapi.destroy_node_inventory_by_node_id(self.inventory.node_id) + self.assertRaises(exception.NodeInventoryNotFound, + self.dbapi.get_node_inventory_by_id, + self.inventory.id) + + def test_get_inventory_by_id(self): + res = self.dbapi.get_node_inventory_by_id(self.inventory.id) + self.assertEqual(self.inventory.inventory_data, res.inventory_data) + + def test_get_inventory_by_id_not_found(self): + self.assertRaises(exception.NodeInventoryNotFound, + self.dbapi.get_node_inventory_by_id, -1) + + def test_get_inventory_by_node_id(self): + res = self.dbapi.get_node_inventory_by_node_id(self.inventory.node_id) + self.assertEqual(self.inventory.id, res.id) + + def test_get_history_by_node_id_empty(self): + self.assertEqual([], self.dbapi.get_node_history_by_node_id(10)) diff --git a/ironic/tests/unit/db/test_nodes.py b/ironic/tests/unit/db/test_nodes.py index bb030a80c3..78bba0a35d 100644 --- a/ironic/tests/unit/db/test_nodes.py +++ b/ironic/tests/unit/db/test_nodes.py @@ -763,6 +763,15 @@ class DbNodeTestCase(base.DbTestCase): self.assertRaises(exception.NodeHistoryNotFound, self.dbapi.get_node_history_by_id, history.id) + def test_inventory_get_destroyed_after_destroying_a_node_by_uuid(self): + node = utils.create_test_node() + + inventory = utils.create_test_inventory(node_id=node.id) + + self.dbapi.destroy_node(node.uuid) + self.assertRaises(exception.NodeInventoryNotFound, + self.dbapi.get_node_inventory_by_id, inventory.id) + 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 e2eae83cc8..c59ee1cc42 100644 --- a/ironic/tests/unit/db/utils.py +++ b/ironic/tests/unit/db/utils.py @@ -28,6 +28,7 @@ from ironic.objects import conductor from ironic.objects import deploy_template from ironic.objects import node from ironic.objects import node_history +from ironic.objects import node_inventory from ironic.objects import port from ironic.objects import portgroup from ironic.objects import trait @@ -721,3 +722,29 @@ def create_test_history(**kw): del history['id'] dbapi = db_api.get_instance() return dbapi.create_node_history(history) + + +def get_test_inventory(**kw): + return { + 'id': kw.get('id', 345), + 'version': kw.get('version', node_inventory.NodeInventory.VERSION), + 'node_id': kw.get('node_id', 123), + 'inventory_data': kw.get('inventory', {"inventory": "test"}), + 'plugin_data': kw.get('plugin_data', {"pdata": {"plugin": "data"}}), + 'created_at': kw.get('created_at'), + 'updated_at': kw.get('updated_at'), + } + + +def create_test_inventory(**kw): + """Create test inventory entry in DB and return NodeInventory DB object. + + :param kw: kwargs with overriding values for port's attributes. + :returns: Test NodeInventory DB object. + """ + inventory = get_test_inventory(**kw) + # Let DB generate ID if it isn't specified explicitly + if 'id' not in kw: + del inventory['id'] + dbapi = db_api.get_instance() + return dbapi.create_node_inventory(inventory) diff --git a/ironic/tests/unit/objects/test_node_inventory.py b/ironic/tests/unit/objects/test_node_inventory.py new file mode 100644 index 0000000000..6b57a27806 --- /dev/null +++ b/ironic/tests/unit/objects/test_node_inventory.py @@ -0,0 +1,61 @@ +# 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. + +from unittest import mock + +from ironic import objects +from ironic.tests.unit.db import base as db_base +from ironic.tests.unit.db import utils as db_utils +from ironic.tests.unit.objects import utils as obj_utils + + +class TestNodeInventoryObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn): + + def setUp(self): + super(TestNodeInventoryObject, self).setUp() + self.fake_inventory = db_utils.get_test_inventory() + + def test_get_by_id(self): + with mock.patch.object(self.dbapi, 'get_node_inventory_by_id', + autospec=True) as mock_get: + id_ = self.fake_inventory['id'] + mock_get.return_value = self.fake_inventory + + inventory = objects.NodeInventory.get_by_id(self.context, id_) + + mock_get.assert_called_once_with(id_) + self.assertIsInstance(inventory, objects.NodeInventory) + self.assertEqual(self.context, inventory._context) + + def test_create(self): + with mock.patch.object(self.dbapi, 'create_node_inventory', + autospec=True) as mock_db_create: + mock_db_create.return_value = self.fake_inventory + new_inventory = objects.NodeInventory( + self.context, **self.fake_inventory) + new_inventory.create() + + mock_db_create.assert_called_once_with(self.fake_inventory) + + def test_destroy(self): + node_id = self.fake_inventory['node_id'] + with mock.patch.object(self.dbapi, 'get_node_inventory_by_node_id', + autospec=True) as mock_get: + mock_get.return_value = self.fake_inventory + with mock.patch.object(self.dbapi, + 'destroy_node_inventory_by_node_id', + autospec=True) as mock_db_destroy: + inventory = objects.NodeInventory.get_by_node_id(self.context, + node_id) + inventory.destroy() + + mock_db_destroy.assert_called_once_with(node_id) diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index 7c58cf4aa4..2e45ee0436 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -721,6 +721,7 @@ expected_object_fingerprints = { 'DeployTemplateCRUDPayload': '1.0-200857e7e715f58a5b6d6b700ab73a3b', 'Deployment': '1.0-ff10ae028c5968f1596131d85d7f5f9d', 'NodeHistory': '1.0-9b576c6481071e7f7eac97317fa29418', + 'NodeInventory': '1.0-97692fec24e20ab02022b9db54e8f539', }