diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index f4f1afdda65..2ccc89d50da 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -552,8 +552,13 @@ def sort_ips_by_version(addresses): return ip_map -def is_lsp_router_port(port): - return port.get('device_owner') in const.ROUTER_PORT_OWNERS +def is_lsp_router_port(neutron_port=None, lsp=None): + if neutron_port: + return neutron_port.get('device_owner') in const.ROUTER_PORT_OWNERS + elif lsp: + return (lsp.external_ids.get(constants.OVN_DEVICE_OWNER_EXT_ID_KEY) in + const.ROUTER_PORT_OWNERS) + return False def get_lrouter_ext_gw_static_route(ovn_router): diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py index 56b1b1923ff..31cbcdab7fd 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py @@ -15,6 +15,7 @@ import abc import datetime +from neutron_lib import constants as n_const from neutron_lib import context as neutron_context from neutron_lib.plugins import constants from neutron_lib.plugins import directory @@ -34,6 +35,7 @@ from neutron.common.ovn import hash_ring_manager from neutron.common.ovn import utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf from neutron.db import ovn_hash_ring_db +from neutron.objects import router as router_obj from neutron.plugins.ml2.drivers.ovn.agent import neutron_agent as n_agent from neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.extensions import \ placement @@ -539,6 +541,47 @@ class LogicalSwitchPortUpdateDownEvent(row_event.RowEvent): self.driver.set_port_status_down(row.name) +class LogicalSwitchPortUpdateLogicalRouterPortEvent(row_event.RowEvent): + """Row update event - Logical_Switch_Port, that updates the sibling LRP""" + def __init__(self, driver): + self.driver = driver + table = 'Logical_Switch_Port' + events = (self.ROW_UPDATE,) + super().__init__(events, table, None) + self.event_name = 'LogicalSwitchPortUpdateLogicalRouterPortEvent' + self.l3_plugin = directory.get_plugin(constants.L3) + self.admin_context = neutron_context.get_admin_context() + + def match_fn(self, event, row, old): + device_id = row.external_ids.get(ovn_const.OVN_DEVID_EXT_ID_KEY) + device_owner = row.external_ids.get( + ovn_const.OVN_DEVICE_OWNER_EXT_ID_KEY) + if (not device_id or + device_owner not in n_const.ROUTER_INTERFACE_OWNERS): + # This LSP does not belong to a router. + return False + + lrp_name = utils.ovn_lrouter_port_name(row.name) + if not self.driver.nb_ovn.lookup('Logical_Router_Port', lrp_name, + default=None): + # The LRP has not been created yet. + return False + + # TODO(ralonsoh): store the router "flavor_id" in the LSP.external_ids + # or the LRP.external_ids (better the second). + router = router_obj.Router.get_object(self.admin_context, id=device_id, + fields=('flavor_id', )) + if (utils.is_lsp_router_port(lsp=row) and + router and + utils.is_ovn_provider_router(router)): + return True + return False + + def run(self, event, row, old): + port = self.driver._plugin.get_port(self.admin_context, row.name) + self.l3_plugin._ovn_client.update_router_port(self.admin_context, port) + + class PortBindingUpdateVirtualPortsEvent(row_event.RowEvent): """Row update event - Port_Binding for virtual ports @@ -769,12 +812,16 @@ class OvnNbIdl(OvnIdlDistributedLock): self._lsp_update_up_event = LogicalSwitchPortUpdateUpEvent(driver) self._lsp_update_down_event = LogicalSwitchPortUpdateDownEvent(driver) self._lsp_create_event = LogicalSwitchPortCreateEvent(driver) + self._lsp_lrp_event = ( + LogicalSwitchPortUpdateLogicalRouterPortEvent(driver)) self._fip_create_delete_event = FIPAddDeleteEvent(driver) self.notify_handler.watch_events([self._lsp_create_event, self._lsp_update_up_event, self._lsp_update_down_event, - self._fip_create_delete_event]) + self._fip_create_delete_event, + self._lsp_lrp_event, + ]) @classmethod def from_server(cls, connection_string, helper, driver): diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index dafdaf9b66c..fe40baf7161 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -369,17 +369,6 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase, reason = _("Router port must have at least one IP.") raise n_exc.ServicePortInUse(port_id=original['id'], reason=reason) - if (event == events.AFTER_UPDATE and - utils.is_lsp_router_port(current) and - utils.is_ovn_provider_router( - l3plugin.get_router(context, current['device_id']))): - # We call the update_router port with if_exists, because neutron, - # internally creates the port, and then calls update, which will - # trigger this callback even before we had the chance to create - # the OVN NB DB side - l3plugin._ovn_client.update_router_port(context, - current, if_exists=True) - def get_router_availability_zones(self, router): lr = self._nb_ovn.get_lrouter(router['id']) if not lr: diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index 495c77cec25..9d5f3c0367a 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -14,6 +14,7 @@ import datetime import functools import subprocess +import time from unittest import mock import fixtures as og_fixtures @@ -81,6 +82,24 @@ class DistributedLockTestEvent(event.WaitEvent): self.event.set() +class WaitForLogicalSwitchPortUpdateEvent(event.WaitEvent): + event_name = 'WaitForDataPathBindingCreateEvent' + + def __init__(self): + table = 'Logical_Switch_Port' + events = (self.ROW_UPDATE,) + super().__init__(events, table, None, timeout=15) + + +class WaitForLogicalRouterPortCreateEvent(event.WaitEvent): + event_name = 'WaitForLogicalRouterPortCreateEvent' + + def __init__(self): + table = 'Logical_Router_Port' + events = (self.ROW_CREATE,) + super().__init__(events, table, None, timeout=15) + + class GlobalTestEvent(DistributedLockTestEvent): GLOBAL = True @@ -812,3 +831,47 @@ class TestPortBindingChassisEvent(base.TestOVNFunctionalBase, self.sb_api.lsp_bind(self.port['id'], self.chassis, may_exist=True).execute(check_error=True) self._check_pb_type('') + + +class TestLogicalSwitchPortUpdateLogicalRouterPortEvent( + base.TestOVNFunctionalBase, + test_l3.L3NatTestCaseMixin): + + def setUp(self, **kwargs): + super().setUp(**kwargs) + self.chassis = self.add_fake_chassis('ovs-host1') + self.l3_plugin = directory.get_plugin(plugin_constants.L3) + self.net = self._make_network( + self.fmt, 'ext_net', True, as_admin=True, **kwargs) + self.subnet = self._make_subnet(self.fmt, self.net, '20.0.10.1', + '20.0.10.0/24') + self.ext_api = test_extensions.setup_extensions_middleware( + test_l3.L3TestExtensionManager()) + + def test_create_router_port(self): + router = self._make_router(self.fmt, self._tenant_id) + with mock.patch.object(self.l3_plugin._ovn_client, + 'update_router_port') as mock_update_rp: + lsp_event = WaitForLogicalSwitchPortUpdateEvent() + lrp_event = WaitForLogicalRouterPortCreateEvent() + self.mech_driver.nb_ovn.idl.notify_handler.watch_events( + (lsp_event, lrp_event)) + self._router_interface_action('add', router['router']['id'], + self.subnet['subnet']['id'], None) + self.assertTrue(lsp_event.wait()) + self.assertTrue(lrp_event.wait()) + # Wait for the + # ``LogicalSwitchPortUpdateLogicalRouterPortEvent.run`` call. + time.sleep(1) + mock_update_rp.assert_called() + + def test_create_non_router_port(self): + with mock.patch.object(self.l3_plugin._ovn_client, + 'update_router_port') as mock_update_rp: + row_event = WaitForLogicalSwitchPortUpdateEvent() + self.mech_driver.nb_ovn.idl.notify_handler.watch_event(row_event) + self._create_port(self.fmt, self.net['network']['id']) + # The LogicalSwitchPort event is called but not the router port + # update method because this is not a router port. + self.assertTrue(row_event.wait()) + mock_update_rp.assert_not_called() diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index b1bba71b44f..5dc4c56b41a 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -1719,30 +1719,6 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): delete_nat_calls, any_order=True) self.assertEqual(len(delete_nat_calls), status_upd_mock.call_count) - @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.get_router') - @mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.' - 'ovn_client.OVNClient.update_router_port') - def test_port_update_postcommit(self, update_rp_mock, gr): - context = 'fake_context' - port = {'device_owner': 'foo', 'device_id': 'id'} - gr.return_value = {'flavor_id': None} - self.l3_inst._port_update(resources.PORT, events.AFTER_UPDATE, None, - payload=events.DBEventPayload( - context, - states=(port,))) - update_rp_mock.assert_not_called() - - port = {'device_owner': constants.DEVICE_OWNER_ROUTER_INTF, - 'device_id': 'router_id'} - self.l3_inst._port_update(resources.PORT, events.AFTER_UPDATE, None, - payload=events.DBEventPayload( - context, - states=(port,))) - - update_rp_mock.assert_called_once_with(context, - port, - if_exists=True) - @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.get_router') def test_port_update_before_update_router_port_without_ip(self, gr): context = 'fake_context'