[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 <lucasagomes@gmail.com>

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(

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):

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):