[ovn] Specify port type if it's a router port when updating

In order to avoid multiple LogicalSwitchPortUpdateUpEvent and
LogicalSwitchPortUpdateDownEvent is mandatory to specify the
router port type when udpating the port.

If the port is not specified when updating the port, the
transactions will trigger a modification on the ovnnb db
that will set the port status to down[0]. Triggering an unnecessary
DownEvent followed by another UpEvent. Those unnecessary event
most likely will trigger a revision conflict.

[0] - https://github.com/ovn-org/ovn/blob/
4f93381d7d38aa21f56fb3ff4ec00490fca12614/northd/northd.c#L15604

Conflicts:
  neutron/common/ovn/constants.py

Closes-Bug: #1955578
Change-Id: I296003a936db16dd3a7d184ec44908fb3f261876
(cherry picked from commit 8c482b8)

Author: averdagu

neutron/common/ovn/constants.py

@@ -354,6 +354,7 @@
 LSP_TYPE_VIRTUAL = 'virtual'
 LSP_TYPE_EXTERNAL = 'external'
 LSP_TYPE_LOCALPORT = 'localport'
+LSP_TYPE_ROUTER = 'router'
 LSP_OPTIONS_VIRTUAL_PARENTS_KEY = 'virtual-parents'
 LSP_OPTIONS_VIRTUAL_IP_KEY = 'virtual-ip'
 LSP_OPTIONS_VIF_PLUG_TYPE_KEY = 'vif-plug-type'

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py

@@ -630,6 +630,13 @@ def update_port(self, context, port, port_object=None):
             txn.add(check_rev_cmd)
             columns_dict = {}
             if utils.is_lsp_router_port(port):
+                # It is needed to specify the port type, if not specified
+                # the AddLSwitchPortCommand will trigger a change
+                # on the northd status column from UP to DOWN, triggering a
+                # LogicalSwitchPortUpdateDownEvent, that will most likely
+                # cause a revision conflict.
+                # https://bugs.launchpad.net/neutron/+bug/1955578
+                columns_dict['type'] = ovn_const.LSP_TYPE_ROUTER
                 port_info.options.update(
                     self._nb_idl.get_router_port_options(port['id']))
             else:

neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

@@ -38,6 +38,7 @@
 from neutron.plugins.ml2.drivers.ovn.mech_driver import mech_driver
 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 ovsdb_monitor
 from neutron.tests import base as tests_base
 from neutron.tests.functional import base
 
@@ -602,6 +603,74 @@ def _test_external_port_create(self, vnic_type):
         self.assertEqual(utils.ovn_name(net_id),
                          str(ovn_port.ha_chassis_group[0].name))
 
+    def _create_router_port(self, vnic_type):
+        net_id = self.n1['network']['id']
+        port_data = {
+            'port': {'network_id': net_id,
+                     'tenant_id': self._tenant_id,
+                     portbindings.VNIC_TYPE: 'normal'}}
+
+        # Create port
+        port_req = self.new_create_request('ports', port_data, self.fmt)
+        port_res = port_req.get_response(self.api)
+        port = self.deserialize(self.fmt, port_res)['port']
+
+        # Update it as lsp port
+        port_upt_data = {
+            'port': {'device_owner': "network:router_gateway"}
+        }
+        port_req = self.new_update_request(
+            'ports', port_upt_data, port['id'], self.fmt)
+        port_res = port_req.get_response(self.api)
+
+    def test_add_external_port_avoid_flapping(self):
+        class LogicalSwitchPortUpdateUpEventTest(event.RowEvent):
+            def __init__(self):
+                self.count = 0
+                table = 'Logical_Switch_Port'
+                events = (self.ROW_UPDATE,)
+                super(LogicalSwitchPortUpdateUpEventTest, self).__init__(
+                    events, table, (('up', '=', True),),
+                    old_conditions=(('up', '=', False),))
+
+            def run(self, event, row, old):
+                self.count += 1
+
+            def get_count(self):
+                return self.count
+
+        class LogicalSwitchPortUpdateDownEventTest(event.RowEvent):
+            def __init__(self):
+                self.count = 0
+                table = 'Logical_Switch_Port'
+                events = (self.ROW_UPDATE,)
+                super(LogicalSwitchPortUpdateDownEventTest, self).__init__(
+                    events, table, (('up', '=', False),),
+                    old_conditions=(('up', '=', True),))
+
+            def run(self, event, row, old):
+                self.count += 1
+
+            def get_count(self):
+                return self.count
+
+        og_up_event = ovsdb_monitor.LogicalSwitchPortUpdateUpEvent(None)
+        og_down_event = ovsdb_monitor.LogicalSwitchPortUpdateDownEvent(None)
+        test_down_event = LogicalSwitchPortUpdateDownEventTest()
+        test_up_event = LogicalSwitchPortUpdateUpEventTest()
+        self.nb_api.idl.notify_handler.unwatch_events(
+            [og_up_event, og_down_event])
+        self.nb_api.idl.notify_handler.watch_events(
+            [test_down_event, test_up_event])
+        # Creating a port the same way as the osp cli cmd
+        # openstack router add port ROUTER PORT
+        # shouldn't trigger an status flapping (up -> down -> up)
+        # it should be created with status false and then change the
+        # status as up, triggering only a LogicalSwitchPortUpdateUpEvent.
+        self._create_router_port(portbindings.VNIC_DIRECT)
+        self.assertEqual(test_down_event.get_count(), 0)
+        self.assertEqual(test_up_event.get_count(), 1)
+
     def test_external_port_create_vnic_direct(self):
         self._test_external_port_create(portbindings.VNIC_DIRECT)