From 9d3d16b791387acc23e61513bdebdb631b255e37 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Thu, 16 Feb 2023 10:04:06 +0000 Subject: [PATCH] Fix Inventory DB Follow-up to I6b830e5cc30f1fa1f1900e7c45e6f246fa1ec51c Original changa introduced some errors such as mismatched arguments for exceptions Story: 2010275 Task: 46204 Change-Id: I550e048ab22a6cd25502b41d1c579819df369249 --- ironic/common/exception.py | 4 ++++ ironic/db/api.py | 8 -------- ironic/db/sqlalchemy/api.py | 11 ++--------- ironic/objects/node_inventory.py | 17 +---------------- ironic/tests/unit/db/test_node_inventory.py | 15 ++------------- ironic/tests/unit/db/test_nodes.py | 4 ++-- .../tests/unit/objects/test_node_inventory.py | 12 ------------ 7 files changed, 11 insertions(+), 60 deletions(-) diff --git a/ironic/common/exception.py b/ironic/common/exception.py index 731d31486d..a4925faf3c 100644 --- a/ironic/common/exception.py +++ b/ironic/common/exception.py @@ -120,6 +120,10 @@ class VolumeTargetBootIndexAlreadyExists(Conflict): "for the same node already exists.") +class NodeInventoryAlreadyExists(Conflict): + _msg_fmt = _("A node inventory with ID %(id)s already exists.") + + class VifAlreadyAttached(Conflict): _msg_fmt = _("Unable to attach VIF because VIF %(vif)s is already " "attached to Ironic %(object_type)s %(object_uuid)s") diff --git a/ironic/db/api.py b/ironic/db/api.py index 9e16f142a0..42839aa74b 100644 --- a/ironic/db/api.py +++ b/ironic/db/api.py @@ -1450,14 +1450,6 @@ class Connection(object, metaclass=abc.ABCMeta): :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. diff --git a/ironic/db/sqlalchemy/api.py b/ironic/db/sqlalchemy/api.py index 474a49ec51..c33cecff04 100644 --- a/ironic/db/sqlalchemy/api.py +++ b/ironic/db/sqlalchemy/api.py @@ -2585,21 +2585,14 @@ class Connection(api.Connection): 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) + node=node_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) + raise exception.NodeInventoryNotFound(node=node_id) def get_shard_list(self): """Return a list of shards. diff --git a/ironic/objects/node_inventory.py b/ironic/objects/node_inventory.py index db7ddb2702..eccc842dd8 100644 --- a/ironic/objects/node_inventory.py +++ b/ironic/objects/node_inventory.py @@ -45,21 +45,6 @@ class NodeInventory(base.IronicObject, object_base.VersionedObjectDictCompat): 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. @@ -90,7 +75,7 @@ class NodeInventory(base.IronicObject, object_base.VersionedObjectDictCompat): self._from_db_object(self._context, self, db_inventory) def destroy(self, context=None): - """Delete the NodeHistory from the DB. + """Delete the NodeInventory from the DB. :param context: Security context. NOTE: This should only be used internally by the indirection_api. diff --git a/ironic/tests/unit/db/test_node_inventory.py b/ironic/tests/unit/db/test_node_inventory.py index a146903f73..8dc638f2bb 100644 --- a/ironic/tests/unit/db/test_node_inventory.py +++ b/ironic/tests/unit/db/test_node_inventory.py @@ -28,20 +28,9 @@ class DBNodeInventoryTestCase(base.DbTestCase): 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) + self.dbapi.get_node_inventory_by_node_id, + self.node.id) 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 78bba0a35d..f7e858f9ba 100644 --- a/ironic/tests/unit/db/test_nodes.py +++ b/ironic/tests/unit/db/test_nodes.py @@ -766,11 +766,11 @@ class DbNodeTestCase(base.DbTestCase): 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) + 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) + self.dbapi.get_node_inventory_by_node_id, node.id) def test_update_node(self): node = utils.create_test_node() diff --git a/ironic/tests/unit/objects/test_node_inventory.py b/ironic/tests/unit/objects/test_node_inventory.py index 6b57a27806..9eee77a160 100644 --- a/ironic/tests/unit/objects/test_node_inventory.py +++ b/ironic/tests/unit/objects/test_node_inventory.py @@ -24,18 +24,6 @@ class TestNodeInventoryObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn): 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: