From dfb083657f138c9cfec2f3af959af6016afbc946 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Fri, 3 Nov 2023 15:38:52 -0400 Subject: [PATCH] Remove migrate_to_port_groups maintenance task TODO says this was introduced in Q and can be removed in U, so let's do it. Removed the DB sync as well. Removed the related OVSDB code to get and delete address sets as these were the only callers according to codesearch. Change-Id: I13dfce7a8f6a5cb9ec91d548242bedce785f8340 --- .../ml2/drivers/ovn/mech_driver/ovsdb/api.py | 18 ------ .../drivers/ovn/mech_driver/ovsdb/commands.py | 20 ------ .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 15 ----- .../ovn/mech_driver/ovsdb/maintenance.py | 30 --------- .../ovn/mech_driver/ovsdb/ovn_db_sync.py | 61 ------------------- .../mech_driver/ovsdb/test_impl_idl_ovn.py | 5 -- .../ovn/mech_driver/ovsdb/test_maintenance.py | 39 ------------ 7 files changed, 188 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py index 2f05b2e68bd..6c4ca4d9b45 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py @@ -262,17 +262,6 @@ class API(api.API, metaclass=abc.ABCMeta): :returns: :class:`Command` with no result """ - @abc.abstractmethod - def delete_address_set(self, name, if_exists=True): - """Delete an address set - - :param name: The name of the address set - :type name: string - :param if_exists: Do not fail if the address set does not exist - :type if_exists: bool - :returns: :class:`Command` with no result - """ - @abc.abstractmethod def get_all_chassis_gateway_bindings(self, chassis_candidate_list=None): @@ -381,13 +370,6 @@ class API(api.API, metaclass=abc.ABCMeta): DHCP_Options matched found. """ - @abc.abstractmethod - def get_address_sets(self): - """Gets all address sets in the OVN_Northbound DB - - :returns: dictionary indexed by name, DB columns as values - """ - @abc.abstractmethod def get_sg_port_groups(self): """Gets port groups in the OVN_Northbound DB that map to SGs. diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index 2faadf0aa1b..598b9b02ab8 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -569,26 +569,6 @@ class DelStaticRouteCommand(command.BaseCommand): break -class DelAddrSetCommand(command.BaseCommand): - def __init__(self, api, name, if_exists): - super(DelAddrSetCommand, self).__init__(api) - self.name = name - self.if_exists = if_exists - - def run_idl(self, txn): - try: - addrset = idlutils.row_by_value(self.api.idl, 'Address_Set', - 'name', self.name) - except idlutils.RowNotFound: - if self.if_exists: - return - msg = _("Address set %s does not exist. " - "Can't delete.") % self.name - raise RuntimeError(msg) - - self.api._tables['Address_Set'].rows[addrset.uuid].delete() - - class UpdateObjectExtIdsCommand(command.BaseCommand): table = None field = 'name' diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py index f6634d51af4..31e0909c728 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py @@ -466,9 +466,6 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): return cmd.DelStaticRouteCommand(self, lrouter, ip_prefix, nexthop, if_exists) - def delete_address_set(self, name, if_exists=True, **columns): - return cmd.DelAddrSetCommand(self, name, if_exists) - def _get_logical_router_port_gateway_chassis(self, lrp): """Get the list of chassis hosting this gateway port. @@ -635,18 +632,6 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): return dhcp_options - def get_address_sets(self): - address_sets = {} - for row in self._tables['Address_Set'].rows.values(): - if not (ovn_const.OVN_SG_EXT_ID_KEY in row.external_ids): - continue - name = getattr(row, 'name') - data = {} - for row_key in getattr(row, "_data", {}): - data[row_key] = getattr(row, row_key) - address_sets[name] = data - return address_sets - def get_router_port_options(self, lsp_name): try: lsp = idlutils.row_by_value(self.idl, 'Logical_Switch_Port', diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 9eb34205661..d51244be77e 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -41,7 +41,6 @@ from neutron.db import ovn_hash_ring_db as hash_ring_db from neutron.db import ovn_revision_numbers_db as revision_numbers_db from neutron.objects import ports as ports_obj from neutron.objects import router as router_obj -from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync from neutron import service from neutron.services.logapi.drivers.ovn import driver as log_driver @@ -299,35 +298,6 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): else: self._ovn_client.update_subnet(context, sn_db_obj, n_db_obj) - # The migration will run just once per neutron-server instance. If the lock - # is held by some other neutron-server instance in the cloud, we'll attempt - # to perform the migration every 10 seconds until completed. - # TODO(jlibosva): Remove the migration to port groups at some point. It's - # been around since Queens release so it is good to drop this soon. - @periodics.periodic(spacing=10, run_immediately=True) - @rerun_on_schema_updates - def migrate_to_port_groups(self): - """Perform the migration from Address Sets to Port Groups. """ - # TODO(dalvarez): Remove this in U cycle when we're sure that all - # versions are running using Port Groups (and OVS >= 2.10). - - # If Port Groups are not supported or we've already migrated, we don't - # need to attempt to migrate again. - if not self._nb_idl.get_address_sets(): - raise periodics.NeverAgain() - - # Only the worker holding a valid lock within OVSDB will perform the - # migration. - if not self.has_lock: - return - - admin_context = n_context.get_admin_context() - nb_sync = ovn_db_sync.OvnNbSynchronizer( - self._ovn_client._plugin, self._nb_idl, self._ovn_client._sb_idl, - None, None) - nb_sync.migrate_to_port_groups(admin_context) - raise periodics.NeverAgain() - def _log_maintenance_inconsistencies(self, create_update_inconsistencies, delete_inconsistencies): if not CONF.debug: diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py index 04c0accee96..320fd7a34b2 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py @@ -1242,67 +1242,6 @@ class OvnNbSynchronizer(OvnDbSynchronizer): txn.add(self.ovn_api.dns_set_records(ls_dns_record.uuid, **dns_records)) - def _delete_address_sets(self, ctx): - with self.ovn_api.transaction(check_error=True) as txn: - for sg in self.core_plugin.get_security_groups(ctx): - for ip_version in ['ip4', 'ip6']: - txn.add(self.ovn_api.delete_address_set( - utils.ovn_addrset_name(sg['id'], ip_version))) - - def _delete_acls_from_lswitches(self, ctx): - with self.ovn_api.transaction(check_error=True) as txn: - for net in self.core_plugin.get_networks(ctx): - # Calling acl_del from ovsdbapp with no ACL will delete - # all the ACLs belonging to that Logical Switch. - txn.add(self.ovn_api.acl_del(utils.ovn_name(net['id']))) - - def _create_sg_port_groups_and_acls(self, ctx, db_ports): - # Create a Port Group per Neutron Security Group - with self.ovn_api.transaction(check_error=True) as txn: - for sg in self.core_plugin.get_security_groups(ctx): - pg_name = utils.ovn_port_group_name(sg['id']) - if self.ovn_api.get_port_group(pg_name): - continue - ext_ids = {ovn_const.OVN_SG_EXT_ID_KEY: sg['id']} - txn.add(self.ovn_api.pg_add( - name=pg_name, acls=[], external_ids=ext_ids)) - acl_utils.add_acls_for_sg_port_group( - self.ovn_api, sg, txn, - self._ovn_client.is_allow_stateless_supported()) - for port in db_ports: - for sg in port['security_groups']: - txn.add(self.ovn_api.pg_add_ports( - utils.ovn_port_group_name(sg), port['id'])) - - def migrate_to_port_groups(self, ctx): - # This routine is responsible for migrating the current Security - # Groups and SG Rules to the new Port Groups implementation. - # 1. Create a Port Group for every existing Neutron Security Group and - # add all its Security Group Rules as ACLs to that Port Group. - # 2. Delete all existing Address Sets in NorthBound database which - # correspond to a Neutron Security Group. - # 3. Delete all the ACLs in every Logical Switch (Neutron network). - - # If we've already migrated, return - if not self.ovn_api.get_address_sets(): - return - - LOG.debug('Port Groups Migration task started') - - # Ignore the floating ip ports with device_owner set to - # constants.DEVICE_OWNER_FLOATINGIP - db_ports = [port for port in - self.core_plugin.get_ports(ctx) if not - utils.is_lsp_ignored(port) and not - utils.is_lsp_trusted(port) and - utils.is_port_security_enabled(port)] - - self._create_sg_port_groups_and_acls(ctx, db_ports) - self._delete_address_sets(ctx) - self._delete_acls_from_lswitches(ctx) - - LOG.debug('Port Groups Migration task completed') - def sync_port_qos_policies(self, ctx): """Sync port QoS policies. diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py index 256f7075ade..1d8d789bc07 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py @@ -865,11 +865,6 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): self.assertEqual(len(dhcp_options['subnets']), 3) self.assertEqual(len(dhcp_options['ports_v4']), 2) - def test_get_address_sets(self): - self._load_nb_db() - address_sets = self.nb_ovn_idl.get_address_sets() - self.assertEqual(len(address_sets), 4) - def test_get_router_floatingip_lbs(self): lrouter_name = 'rtr_name' # Empty diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 7c8f7dc4b6d..cedc59474b5 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -31,7 +31,6 @@ from neutron.db.models import ovn as ovn_models from neutron.db import ovn_revision_numbers_db from neutron.objects import ports as ports_obj from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import maintenance -from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync from neutron.tests.unit import fake_resources as fakes from neutron.tests.unit.plugins.ml2 import test_security_group as test_sg from neutron.tests.unit import testlib_api @@ -109,44 +108,6 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, self.periodic.check_for_inconsistencies() mock_fix_net.assert_called_once_with(mock.ANY, fake_row) - def _test_migrate_to_port_groups_helper(self, a_sets, migration_expected, - never_again): - self.fake_ovn_client._nb_idl.get_address_sets.return_value = a_sets - with mock.patch.object(ovn_db_sync.OvnNbSynchronizer, - 'migrate_to_port_groups') as mtpg: - if never_again: - self.assertRaises(periodics.NeverAgain, - self.periodic.migrate_to_port_groups) - else: - self.periodic.migrate_to_port_groups() - - if migration_expected: - mtpg.assert_called_once_with(mock.ANY) - else: - mtpg.assert_not_called() - - def test_migrate_to_port_groups_not_needed(self): - self._test_migrate_to_port_groups_helper(a_sets=None, - migration_expected=False, - never_again=True) - - def test_migrate_to_port_groups(self): - # Check normal migration path: if the migration has to be done, it will - # take place and won't be attempted in the future. - self._test_migrate_to_port_groups_helper(a_sets=['as1', 'as2'], - migration_expected=True, - never_again=True) - - def test_migrate_to_port_groups_no_lock(self): - with mock.patch.object(maintenance.DBInconsistenciesPeriodics, - 'has_lock', mock.PropertyMock( - return_value=False)): - # Check that if this worker doesn't have the lock, it won't - # perform the migration and it will try again later. - self._test_migrate_to_port_groups_helper(a_sets=['as1', 'as2'], - migration_expected=False, - never_again=False) - def _test_fix_create_update_network(self, ovn_rev, neutron_rev): with db_api.CONTEXT_WRITER.using(self.ctx): self.net['revision_number'] = neutron_rev