From 4b93ecd5dbf0b1d8ce09f1c44053af3d62929012 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Thu, 9 Dec 2021 15:14:26 +0100 Subject: [PATCH] Enable foreign keys in SQLite For backward compatibility reasons, they're off by default. Fix all unit tests that rely on dangling foreign keys. Change-Id: I54a45fab1aefc03b2252c655e14b375dea3d9f12 --- ironic/db/sqlalchemy/__init__.py | 16 +++++++ .../api/controllers/v1/test_allocation.py | 2 + .../unit/api/controllers/v1/test_port.py | 15 +++++- .../unit/api/controllers/v1/test_portgroup.py | 2 + .../controllers/v1/test_volume_connector.py | 2 + .../api/controllers/v1/test_volume_target.py | 2 + ironic/tests/unit/api/test_acl.py | 26 ++++++----- ironic/tests/unit/conductor/test_manager.py | 46 +++++++++++-------- ironic/tests/unit/db/test_node_history.py | 1 + ironic/tests/unit/db/test_portgroups.py | 1 + ironic/tests/unit/db/test_ports.py | 8 ++++ .../tests/unit/db/test_volume_connectors.py | 1 + ironic/tests/unit/db/test_volume_targets.py | 4 +- .../unit/drivers/modules/network/test_noop.py | 3 +- .../unit/objects/test_volume_connector.py | 4 +- .../tests/unit/objects/test_volume_target.py | 4 +- .../notes/sqlite-fk-8c87a308a02d49bf.yaml | 4 ++ 17 files changed, 104 insertions(+), 37 deletions(-) create mode 100644 releasenotes/notes/sqlite-fk-8c87a308a02d49bf.yaml diff --git a/ironic/db/sqlalchemy/__init__.py b/ironic/db/sqlalchemy/__init__.py index e69de29bb2..0f792361a5 100644 --- a/ironic/db/sqlalchemy/__init__.py +++ b/ironic/db/sqlalchemy/__init__.py @@ -0,0 +1,16 @@ +# 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_db.sqlalchemy import enginefacade + +# NOTE(dtantsur): we want sqlite as close to a real database as possible. +enginefacade.configure(sqlite_fk=True) diff --git a/ironic/tests/unit/api/controllers/v1/test_allocation.py b/ironic/tests/unit/api/controllers/v1/test_allocation.py index 1be54db809..367c06350c 100644 --- a/ironic/tests/unit/api/controllers/v1/test_allocation.py +++ b/ironic/tests/unit/api/controllers/v1/test_allocation.py @@ -420,6 +420,8 @@ class TestListAllocations(test_api_base.BaseApiTest): node_id = self.node.id else: node_id = 100000 + i + obj_utils.create_test_node(self.context, id=node_id, + uuid=uuidutils.generate_uuid()) obj_utils.create_test_allocation( self.context, node_id=node_id, diff --git a/ironic/tests/unit/api/controllers/v1/test_port.py b/ironic/tests/unit/api/controllers/v1/test_port.py index 1f3322accf..12208e0492 100644 --- a/ironic/tests/unit/api/controllers/v1/test_port.py +++ b/ironic/tests/unit/api/controllers/v1/test_port.py @@ -659,6 +659,9 @@ class TestListPorts(test_api_base.BaseApiTest): return True mock_authorize.side_effect = mock_authorize_function + another_node = obj_utils.create_test_node( + self.context, uuid=uuidutils.generate_uuid()) + ports = [] # these ports should be retrieved by the API call for id_ in range(0, 2): @@ -670,7 +673,8 @@ class TestListPorts(test_api_base.BaseApiTest): # these ports should NOT be retrieved by the API call for id_ in range(3, 5): port = obj_utils.create_test_port( - self.context, uuid=uuidutils.generate_uuid(), + self.context, node_id=another_node.id, + uuid=uuidutils.generate_uuid(), address='52:54:00:cf:2d:3%s' % id_) data = self.get_json('/ports', headers={'X-Project-Id': '12345'}) self.assertEqual(len(ports), len(data['ports'])) @@ -896,6 +900,8 @@ class TestListPorts(test_api_base.BaseApiTest): node_id = self.node.id else: node_id = 100000 + i + obj_utils.create_test_node(self.context, id=node_id, + uuid=uuidutils.generate_uuid()) obj_utils.create_test_port(self.context, node_id=node_id, uuid=uuidutils.generate_uuid(), @@ -920,6 +926,8 @@ class TestListPorts(test_api_base.BaseApiTest): node_id = self.node.id else: node_id = 100000 + i + obj_utils.create_test_node(self.context, id=node_id, + uuid=uuidutils.generate_uuid()) obj_utils.create_test_port(self.context, node_id=node_id, uuid=uuidutils.generate_uuid(), @@ -947,6 +955,8 @@ class TestListPorts(test_api_base.BaseApiTest): node_id = self.node.id else: node_id = 100000 + i + obj_utils.create_test_node(self.context, id=node_id, + uuid=uuidutils.generate_uuid()) obj_utils.create_test_port(self.context, node_id=node_id, uuid=uuidutils.generate_uuid(), @@ -1056,7 +1066,8 @@ class TestListPorts(test_api_base.BaseApiTest): return True mock_authorize.side_effect = mock_authorize_function - pg = obj_utils.create_test_portgroup(self.context) + pg = obj_utils.create_test_portgroup(self.context, + node_id=self.node.id) obj_utils.create_test_port(self.context, node_id=self.node.id, portgroup_id=pg.id) data = self.get_json('/ports/detail?portgroup=%s' % pg.uuid, diff --git a/ironic/tests/unit/api/controllers/v1/test_portgroup.py b/ironic/tests/unit/api/controllers/v1/test_portgroup.py index 394e502ef6..07b95c4286 100644 --- a/ironic/tests/unit/api/controllers/v1/test_portgroup.py +++ b/ironic/tests/unit/api/controllers/v1/test_portgroup.py @@ -546,6 +546,8 @@ class TestListPortgroups(test_api_base.BaseApiTest): node_id = self.node.id else: node_id = 100000 + i + obj_utils.create_test_node(self.context, id=node_id, + uuid=uuidutils.generate_uuid()) obj_utils.create_test_portgroup( self.context, node_id=node_id, diff --git a/ironic/tests/unit/api/controllers/v1/test_volume_connector.py b/ironic/tests/unit/api/controllers/v1/test_volume_connector.py index 2da6eb4ea0..70e11a3470 100644 --- a/ironic/tests/unit/api/controllers/v1/test_volume_connector.py +++ b/ironic/tests/unit/api/controllers/v1/test_volume_connector.py @@ -353,6 +353,8 @@ class TestListVolumeConnectors(test_api_base.BaseApiTest): node_id = self.node.id else: node_id = 100000 + i + obj_utils.create_test_node(self.context, id=node_id, + uuid=uuidutils.generate_uuid()) obj_utils.create_test_volume_connector( self.context, node_id=node_id, uuid=uuidutils.generate_uuid(), diff --git a/ironic/tests/unit/api/controllers/v1/test_volume_target.py b/ironic/tests/unit/api/controllers/v1/test_volume_target.py index ffe8aa594c..038f3cb58a 100644 --- a/ironic/tests/unit/api/controllers/v1/test_volume_target.py +++ b/ironic/tests/unit/api/controllers/v1/test_volume_target.py @@ -328,6 +328,8 @@ class TestListVolumeTargets(test_api_base.BaseApiTest): node_id = self.node.id else: node_id = 100000 + i + obj_utils.create_test_node(self.context, id=node_id, + uuid=uuidutils.generate_uuid()) obj_utils.create_test_volume_target( self.context, node_id=node_id, uuid=uuidutils.generate_uuid(), boot_index=i) diff --git a/ironic/tests/unit/api/test_acl.py b/ironic/tests/unit/api/test_acl.py index 99921e9884..5793e95a8a 100644 --- a/ironic/tests/unit/api/test_acl.py +++ b/ironic/tests/unit/api/test_acl.py @@ -26,6 +26,7 @@ from oslo_config import cfg from ironic.api.controllers.v1 import versions as api_versions from ironic.common import exception from ironic.conductor import rpcapi +from ironic.db import api as db_api from ironic.tests.unit.api import base from ironic.tests.unit.db import utils as db_utils @@ -238,7 +239,6 @@ class TestRBACModelBeforeScopesBase(TestACLBase): def _create_test_data(self): allocated_node_id = 31 fake_db_allocation = db_utils.create_test_allocation( - node_id=allocated_node_id, resource_class="CUSTOM_TEST") fake_db_node = db_utils.create_test_node( chassis_id=None, @@ -247,10 +247,12 @@ class TestRBACModelBeforeScopesBase(TestACLBase): fake_db_node_alloced = db_utils.create_test_node( id=allocated_node_id, chassis_id=None, - allocation_id=fake_db_allocation['id'], uuid='22e26c0b-03f2-4d2e-ae87-c02d7f33c000', driver='fake-driverz', owner='z') + dbapi = db_api.get_instance() + dbapi.update_allocation(fake_db_allocation['id'], + dict(node_id=allocated_node_id)) fake_vif_port_id = "ee21d58f-5de2-4956-85ff-33935ea1ca00" fake_db_port = db_utils.create_test_port( node_id=fake_db_node['id'], @@ -263,9 +265,9 @@ class TestRBACModelBeforeScopesBase(TestACLBase): fake_db_deploy_template = db_utils.create_test_deploy_template() fake_db_conductor = db_utils.create_test_conductor() fake_db_volume_target = db_utils.create_test_volume_target( - node_id=fake_db_allocation['id']) + node_id=fake_db_node['id']) fake_db_volume_connector = db_utils.create_test_volume_connector( - node_id=fake_db_allocation['id']) + node_id=fake_db_node['id']) # Trait name aligns with create_test_node_trait. fake_trait = 'trait' fake_setting = 'FAKE_SETTING' @@ -407,9 +409,7 @@ class TestRBACProjectScoped(TestACLBase): node_id=owned_node.id) # Leased nodes - fake_allocation_id = 61 leased_node = db_utils.create_test_node( - allocation_id=fake_allocation_id, uuid=lessee_node_ident, owner=owner_project_id, lessee=lessee_project_id, @@ -425,22 +425,26 @@ class TestRBACProjectScoped(TestACLBase): node_id=leased_node['id']) fake_trait = 'CUSTOM_MEOW' fake_vif_port_id = "0e21d58f-5de2-4956-85ff-33935ea1ca01" + fake_allocation_id = 61 fake_leased_allocation = db_utils.create_test_allocation( id=fake_allocation_id, - node_id=leased_node['id'], owner=lessee_project_id, resource_class="CUSTOM_LEASED") + dbapi = db_api.get_instance() + dbapi.update_allocation(fake_allocation_id, + dict(node_id=leased_node['id'])) + leased_node_history = db_utils.create_test_history( node_id=leased_node.id) # Random objects that shouldn't be project visible - other_port = db_utils.create_test_port( - uuid='abfd8dbb-1732-449a-b760-2224035c6b99', - address='00:00:00:00:00:ff') - other_node = db_utils.create_test_node( uuid='573208e5-cd41-4e26-8f06-ef44022b3793') + other_port = db_utils.create_test_port( + node_id=other_node['id'], + uuid='abfd8dbb-1732-449a-b760-2224035c6b99', + address='00:00:00:00:00:ff') other_pgroup = db_utils.create_test_portgroup( uuid='5810f41c-6585-41fc-b9c9-a94f50d421b5', node_id=other_node['id'], diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index 81984e88bf..dee5b6974c 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -6502,6 +6502,8 @@ class NodeInspectHardware(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): self.assertTrue(mock_inspect.called) +@mock.patch.object(conductor_utils, 'node_history_record', + mock.Mock(spec=conductor_utils.node_history_record)) @mock.patch.object(task_manager, 'acquire', autospec=True) @mock.patch.object(manager.ConductorManager, '_mapped_to_this_conductor', autospec=True) @@ -8207,30 +8209,33 @@ class NodeHistoryRecordCleanupTestCase(mgr_utils.ServiceSetUpMixin, def setUp(self): super(NodeHistoryRecordCleanupTestCase, self).setUp() - self.node1 = obj_utils.get_test_node(self.context, - driver='fake-hardware', - id=10, - uuid=uuidutils.generate_uuid(), - conductor_affinity=1) - self.node2 = obj_utils.get_test_node(self.context, - driver='fake-hardware', - id=11, - uuid=uuidutils.generate_uuid(), - conductor_affinity=1) - self.node3 = obj_utils.get_test_node(self.context, - driver='fake-hardware', - id=12, - uuid=uuidutils.generate_uuid(), - conductor_affinity=1) + CONF.set_override('node_history_max_entries', 2, group='conductor') + CONF.set_override('node_history_cleanup_batch_count', 2, + group='conductor') + self._start_service() + self.node1 = obj_utils.get_test_node( + self.context, + driver='fake-hardware', + id=10, + uuid=uuidutils.generate_uuid(), + conductor_affinity=self.service.conductor.id) + self.node2 = obj_utils.get_test_node( + self.context, + driver='fake-hardware', + id=11, + uuid=uuidutils.generate_uuid(), + conductor_affinity=self.service.conductor.id) + self.node3 = obj_utils.get_test_node( + self.context, + driver='fake-hardware', + id=12, + uuid=uuidutils.generate_uuid(), + conductor_affinity=self.service.conductor.id) self.nodes = [self.node1, self.node2, self.node3] # Create the nodes, as the tasks need to operate across tables. self.node1.create() self.node2.create() self.node3.create() - CONF.set_override('node_history_max_entries', 2, group='conductor') - CONF.set_override('node_history_cleanup_batch_count', 2, - group='conductor') - self._start_service() def test_history_is_pruned_to_config(self): for node in self.nodes: @@ -8305,11 +8310,12 @@ class NodeHistoryRecordCleanupTestCase(mgr_utils.ServiceSetUpMixin, self.assertEqual(1, len(events)) def test_history_pruning_not_other_conductor(self): + another_conductor = obj_utils.create_test_conductor(self.context) node = obj_utils.get_test_node(self.context, driver='fake-hardware', id=33, uuid=uuidutils.generate_uuid(), - conductor_affinity=2) + conductor_affinity=another_conductor.id) # create node so it can be queried node.create() for i in range(0, 3): diff --git a/ironic/tests/unit/db/test_node_history.py b/ironic/tests/unit/db/test_node_history.py index 9e554cd9c0..ac0caca364 100644 --- a/ironic/tests/unit/db/test_node_history.py +++ b/ironic/tests/unit/db/test_node_history.py @@ -59,6 +59,7 @@ class DBNodeHistoryTestCase(base.DbTestCase): for i in range(1, 6): history = db_utils.create_test_history( id=i, uuid=uuidutils.generate_uuid(), + node_id=self.node.id, conductor='test-conductor', user='fake-user', event='Something bad happened but fear not %s' % i, severity='ERROR', event_type='test') diff --git a/ironic/tests/unit/db/test_portgroups.py b/ironic/tests/unit/db/test_portgroups.py index fa04020942..c39bef62ad 100644 --- a/ironic/tests/unit/db/test_portgroups.py +++ b/ironic/tests/unit/db/test_portgroups.py @@ -42,6 +42,7 @@ class DbportgroupTestCase(base.DbTestCase): for i in range(1, count): portgroup = db_utils.create_test_portgroup( uuid=uuidutils.generate_uuid(), + node_id=self.node.id, name='portgroup' + str(i), address='52:54:00:cf:2d:4%s' % i) uuids.append(str(portgroup.uuid)) diff --git a/ironic/tests/unit/db/test_ports.py b/ironic/tests/unit/db/test_ports.py index 18b8a90322..97d1e98a75 100644 --- a/ironic/tests/unit/db/test_ports.py +++ b/ironic/tests/unit/db/test_ports.py @@ -77,6 +77,7 @@ class DbPortTestCase(base.DbTestCase): uuids = [] for i in range(1, 6): port = db_utils.create_test_port(uuid=uuidutils.generate_uuid(), + node_id=self.node.id, address='52:54:00:cf:2d:4%s' % i) uuids.append(str(port.uuid)) # Also add the uuid for the port created in setUp() @@ -89,6 +90,7 @@ class DbPortTestCase(base.DbTestCase): uuids = [] for i in range(1, 6): port = db_utils.create_test_port(uuid=uuidutils.generate_uuid(), + node_id=self.node.id, address='52:54:00:cf:2d:4%s' % i) uuids.append(str(port.uuid)) # Also add the uuid for the port created in setUp() @@ -101,9 +103,12 @@ class DbPortTestCase(base.DbTestCase): self.dbapi.get_port_list, sort_key='foo') def test_get_port_list_filter_by_node_owner(self): + another_node = db_utils.create_test_node( + uuid=uuidutils.generate_uuid()) uuids = [] for i in range(1, 3): port = db_utils.create_test_port(uuid=uuidutils.generate_uuid(), + node_id=another_node.id, address='52:54:00:cf:2d:4%s' % i) for i in range(4, 6): port = db_utils.create_test_port(uuid=uuidutils.generate_uuid(), @@ -117,6 +122,8 @@ class DbPortTestCase(base.DbTestCase): self.assertCountEqual(uuids, res_uuids) def test_get_port_list_filter_by_node_project(self): + another_node = db_utils.create_test_node( + uuid=uuidutils.generate_uuid()) lessee_node = db_utils.create_test_node(uuid=uuidutils.generate_uuid(), lessee=self.node.owner) @@ -128,6 +135,7 @@ class DbPortTestCase(base.DbTestCase): uuids.append(str(port.uuid)) for i in range(4, 6): port = db_utils.create_test_port(uuid=uuidutils.generate_uuid(), + node_id=another_node.id, address='52:54:00:cf:2d:4%s' % i) for i in range(7, 9): port = db_utils.create_test_port(uuid=uuidutils.generate_uuid(), diff --git a/ironic/tests/unit/db/test_volume_connectors.py b/ironic/tests/unit/db/test_volume_connectors.py index b15e032757..005933fa48 100644 --- a/ironic/tests/unit/db/test_volume_connectors.py +++ b/ironic/tests/unit/db/test_volume_connectors.py @@ -68,6 +68,7 @@ class DbVolumeConnectorTestCase(base.DbTestCase): for i in range(1, 6): volume_connector = db_utils.create_test_volume_connector( uuid=uuidutils.generate_uuid(), + node_id=self.node.id, type='iqn', connector_id='iqn.test-%s' % i) uuids.append(str(volume_connector.uuid)) diff --git a/ironic/tests/unit/db/test_volume_targets.py b/ironic/tests/unit/db/test_volume_targets.py index 62db5353fd..200addf83b 100644 --- a/ironic/tests/unit/db/test_volume_targets.py +++ b/ironic/tests/unit/db/test_volume_targets.py @@ -82,6 +82,7 @@ class DbVolumeTargetTestCase(base.DbTestCase): for i in range(1, num): volume_target = db_utils.create_test_volume_target( uuid=uuidutils.generate_uuid(), + node_id=self.node.id, properties={"target_iqn": "iqn.test-%s" % i}, boot_index=i) uuids.append(str(volume_target.uuid)) @@ -150,7 +151,8 @@ class DbVolumeTargetTestCase(base.DbTestCase): def test_update_volume_target_duplicated_nodeid_and_bootindex(self): t = db_utils.create_test_volume_target(uuid=uuidutils.generate_uuid(), - boot_index=1) + boot_index=1, + node_id=self.node.id) self.assertRaises(exception.VolumeTargetBootIndexAlreadyExists, self.dbapi.update_volume_target, t.uuid, diff --git a/ironic/tests/unit/drivers/modules/network/test_noop.py b/ironic/tests/unit/drivers/modules/network/test_noop.py index 6de7812c20..6df55ec682 100644 --- a/ironic/tests/unit/drivers/modules/network/test_noop.py +++ b/ironic/tests/unit/drivers/modules/network/test_noop.py @@ -39,7 +39,8 @@ class NoopInterfaceTestCase(db_base.DbTestCase): self.interface.port_changed(task, self.port) def test_portgroup_changed(self): - portgroup = utils.create_test_portgroup(self.context) + portgroup = utils.create_test_portgroup(self.context, + node_id=self.node.id) with task_manager.acquire(self.context, self.node.id) as task: self.interface.portgroup_changed(task, portgroup) diff --git a/ironic/tests/unit/objects/test_volume_connector.py b/ironic/tests/unit/objects/test_volume_connector.py index 380caf9820..df98f2c368 100644 --- a/ironic/tests/unit/objects/test_volume_connector.py +++ b/ironic/tests/unit/objects/test_volume_connector.py @@ -188,8 +188,10 @@ class TestVolumeConnectorObject(db_base.DbTestCase, self.assertEqual(self.context, c._context) def test_save_after_refresh(self): + node = db_utils.create_test_node() # Ensure that it's possible to do object.save() after object.refresh() - db_volume_connector = db_utils.create_test_volume_connector() + db_volume_connector = db_utils.create_test_volume_connector( + node_id=node['id']) vc = objects.VolumeConnector.get_by_uuid(self.context, db_volume_connector.uuid) diff --git a/ironic/tests/unit/objects/test_volume_target.py b/ironic/tests/unit/objects/test_volume_target.py index cb57e6b396..396ceee5e8 100644 --- a/ironic/tests/unit/objects/test_volume_target.py +++ b/ironic/tests/unit/objects/test_volume_target.py @@ -198,8 +198,10 @@ class TestVolumeTargetObject(db_base.DbTestCase, obj_utils.SchemasTestMixIn): self.assertEqual(self.context, target._context) def test_save_after_refresh(self): + node = db_utils.create_test_node() # Ensure that it's possible to do object.save() after object.refresh() - db_volume_target = db_utils.create_test_volume_target() + db_volume_target = db_utils.create_test_volume_target( + node_id=node.id) vt = objects.VolumeTarget.get_by_uuid(self.context, db_volume_target.uuid) diff --git a/releasenotes/notes/sqlite-fk-8c87a308a02d49bf.yaml b/releasenotes/notes/sqlite-fk-8c87a308a02d49bf.yaml new file mode 100644 index 0000000000..33f11ce9df --- /dev/null +++ b/releasenotes/notes/sqlite-fk-8c87a308a02d49bf.yaml @@ -0,0 +1,4 @@ +--- +upgrade: + - | + Foreign keys are now enabled when SQLite is used as a database.