From 4693836a1b58b477298e51cb47622222e3556752 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 8 Aug 2023 15:17:40 +0100 Subject: [PATCH] [OVN] ovn-db-sync check for router port differences Prior to this patch the ovn-db-sync script did not check if the router ports were actually out-of-sync before marking them to be updated. This behavior introduced irrelevant information in the sync report (specially when ran in "log" mode) making the user think that the databases were out-of-sync even when they were not. This patch adds the code checking for differences in the Neutron Router Ports and OVN Logical Router Port entries prior to updating them. Change-Id: Id7bf5a6aa547795ba78724eed59ba9d4fb74f758 Closes-Bug: #2030773 Signed-off-by: Lucas Alvares Gomes --- .../ovn/mech_driver/ovsdb/ovn_db_sync.py | 44 ++++++++++++++++--- neutron/tests/unit/fake_resources.py | 1 + .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 42 ++++++++++++++++++ 3 files changed, 82 insertions(+), 5 deletions(-) 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 29475a020f0..bbbb35298fb 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 @@ -455,6 +455,36 @@ class OvnNbSynchronizer(OvnDbSynchronizer): self.l3_plugin.port_forwarding.db_sync_delete( context, fip_id, txn) + def _is_router_port_changed(self, db_router_port, lrport_nets): + """Check if the router port needs to be updated. + + This method checks for networks and ipv6_ra_configs (if supported) + changes on a given router port. + """ + db_lrport_nets = db_router_port['networks'] + if db_lrport_nets != lrport_nets: + return True + + # Check for ipv6_ra_configs changes + db_lrport_ra = db_router_port['ipv6_ra_configs'] + lrport_ra = {} + ipv6_ra_supported = self.ovn_api.is_col_present( + 'Logical_Router_Port', 'ipv6_ra_configs') + if ipv6_ra_supported: + lrp_name = utils.ovn_lrouter_port_name(db_router_port['id']) + try: + ovn_lrport = self.ovn_api.lrp_get( + lrp_name).execute(check_error=True) + except idlutils.RowNotFound: + # If the port is not found in the OVN database the + # ovn-db-sync script will recreate this port later + # and it will have the latest information. No need + # to update it. + return False + lrport_ra = ovn_lrport.ipv6_ra_configs + + return db_lrport_ra != lrport_ra + def sync_routers_and_rports(self, ctx): """Sync Routers between neutron and NB. @@ -534,6 +564,12 @@ class OvnNbSynchronizer(OvnDbSynchronizer): constants.DEVICE_OWNER_HA_REPLICATED_INT]) for interface in interfaces: db_router_ports[interface['id']] = interface + networks, ipv6_ra_configs = ( + self._ovn_client._get_nets_and_ipv6_ra_confs_for_router_port( + ctx, interface)) + db_router_ports[interface['id']]['networks'] = networks + db_router_ports[interface['id']][ + 'ipv6_ra_configs'] = ipv6_ra_configs lrouters = self.ovn_api.get_all_logical_routers_with_rports() @@ -550,11 +586,9 @@ class OvnNbSynchronizer(OvnDbSynchronizer): if lrouter['name'] in db_routers: for lrport, lrport_nets in lrouter['ports'].items(): if lrport in db_router_ports: - # We dont have to check for the networks and - # ipv6_ra_configs values. Lets add it to the - # update_lrport_list. If they are in sync, then - # update_router_port will be a no-op. - update_lrport_list.append(db_router_ports[lrport]) + if self._is_router_port_changed( + db_router_ports[lrport], lrport_nets): + update_lrport_list.append(db_router_ports[lrport]) del db_router_ports[lrport] else: del_lrouter_ports_list.append( diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index ca745f6b640..e05c32547f8 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -164,6 +164,7 @@ class FakeOvsdbNbOvnIdl(object): self.ha_chassis_group_add_chassis = mock.Mock() self.ha_chassis_group_del_chassis = mock.Mock() self.get_lrouter_gw_ports = mock.Mock() + self.lrp_get = mock.Mock() class FakeOvsdbSbOvnIdl(object): diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index a2a586c99cd..6eb48bcb08e 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py @@ -23,6 +23,7 @@ from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import impl_idl_ovn from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_client from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb import ovn_db_sync from neutron.services.ovn_l3 import plugin as ovn_plugin +from neutron.tests.unit import fake_resources as fakes from neutron.tests.unit.plugins.ml2.drivers.ovn.mech_driver import \ test_mech_driver @@ -1109,6 +1110,47 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): expected_deleted) +class TestIsRouterPortChanged(test_mech_driver.OVNMechanismDriverTestCase): + + def setUp(self): + super(TestIsRouterPortChanged, self).setUp() + self.ovn_nb_synchronizer = ovn_db_sync.OvnNbSynchronizer( + self.plugin, self.mech_driver.nb_ovn, self.mech_driver.sb_ovn, + 'log', self.mech_driver) + + self.db_router_port = { + 'id': 'aa076509-915d-4b1c-8d9d-3db53d9c5faf', + 'networks': ['fdf9:ad62:3a04::1/64'], + 'ipv6_ra_configs': {'address_mode': 'slaac', + 'send_periodic': 'true', + 'mtu': '1442'} + } + self.lrport_nets = ['fdf9:ad62:3a04::1/64'] + self.ovn_lrport = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'ipv6_ra_configs': {'address_mode': 'slaac', + 'send_periodic': 'true', + 'mtu': '1442'}}) + + self.ovn_nb_synchronizer.ovn_api.is_col_present.return_value = True + self.ovn_nb_synchronizer.ovn_api.lrp_get().execute.return_value = ( + self.ovn_lrport) + + def test__is_router_port_changed_not_changed(self): + self.assertFalse(self.ovn_nb_synchronizer._is_router_port_changed( + self.db_router_port, self.lrport_nets)) + + def test__is_router_port_changed_network_changed(self): + self.db_router_port['networks'] = ['172.24.4.26/24', + '2001:db8::206/64'] + self.assertTrue(self.ovn_nb_synchronizer._is_router_port_changed( + self.db_router_port, self.lrport_nets)) + + def test__is_router_port_changed_ipv6_ra_configs_changed(self): + self.db_router_port['ipv6_ra_configs']['mtu'] = '1500' + self.assertTrue(self.ovn_nb_synchronizer._is_router_port_changed( + self.db_router_port, self.lrport_nets)) + + class TestOvnSbSyncML2(test_mech_driver.OVNMechanismDriverTestCase): def test_ovn_sb_sync(self):