From 423c92814bcd7ae5e66d05a861be14e4d84f6e25 Mon Sep 17 00:00:00 2001 From: Arnau Verdaguer Date: Thu, 16 Dec 2021 21:53:27 +0100 Subject: [PATCH] [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 8c482b83f2cf6f5495f4df2e5698595db704798d) --- neutron/common/ovn/constants.py | 1 + .../ovn/mech_driver/ovsdb/ovn_client.py | 7 ++ .../ovn/mech_driver/test_mech_driver.py | 69 +++++++++++++++++++ 3 files changed, 77 insertions(+) diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 9fb0fbe602a..5cc295e9086 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -291,6 +291,7 @@ LSP_TYPE_LOCALNET = 'localnet' 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_MCAST_FLOOD_REPORTS = 'mcast_flood_reports' diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index 03cb6d5172a..af64c132f3d 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -563,6 +563,13 @@ class OVNClient(object): 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: diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 5892419e9a0..21a99c90f74 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -36,6 +36,7 @@ from neutron.db import ovn_revision_numbers_db as db_rev 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 @@ -505,6 +506,74 @@ class TestExternalPorts(base.TestOVNFunctionalBase): 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)