openstack/neutron
Code Issues Proposed changes

Merge "Optimize deletion of static routes"

Browse Source
This commit is contained in:
Zuul 2024-05-13 12:11:58 +00:00 committed by Gerrit Code Review
commit 35daa2d868
13 changed files with 129 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py
View File

@ -274,21 +274,6 @@ class API(api.API, metaclass=abc.ABCMeta):
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def delete_static_route(self, lrouter, ip_prefix, nexthop, if_exists=True):
"""Delete static route from logical router.
:param lrouter: The unique name of the lrouter
:type lrouter: string
:param ip_prefix: The prefix of the static route
:type ip_prefix: string
:param nexthop: The nexthop of the static route
:type nexthop: string
:param if_exists: Do not fail if router does not exist
:type if_exists: bool
:returns: :class:`Command` with no result
"""
@abc.abstractmethod
def get_all_chassis_gateway_bindings(self,
chassis_candidate_list=None):

27
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py
View File

@ -675,12 +675,11 @@ class AddStaticRouteCommand(command.BaseCommand):
_addvalue_to_list(lrouter, 'static_routes', row.uuid)
class DelStaticRouteCommand(command.BaseCommand):
def __init__(self, api, lrouter, ip_prefix, nexthop, if_exists):
super(DelStaticRouteCommand, self).__init__(api)
class DelStaticRoutesCommand(command.BaseCommand):
def __init__(self, api, lrouter, routes, if_exists):
super(DelStaticRoutesCommand, self).__init__(api)
self.lrouter = lrouter
self.ip_prefix = ip_prefix
self.nexthop = nexthop
self.routes = routes
self.if_exists = if_exists
def run_idl(self, txn):
@ -693,14 +692,16 @@ class DelStaticRouteCommand(command.BaseCommand):
msg = _("Logical Router %s does not exist") % self.lrouter
raise RuntimeError(msg)
static_routes = getattr(lrouter, 'static_routes', [])
for route in static_routes:
ip_prefix = getattr(route, 'ip_prefix', '')
nexthop = getattr(route, 'nexthop', '')
if self.ip_prefix == ip_prefix and self.nexthop == nexthop:
_delvalue_from_list(lrouter, 'static_routes', route)
route.delete()
break
routes_to_be_deleted = []
for route in getattr(lrouter, 'static_routes', []):
route_tuple = (getattr(route, 'ip_prefix', ''),
getattr(route, 'nexthop', ''))
if route_tuple in self.routes:
routes_to_be_deleted.append(route)
for route in routes_to_be_deleted:
_delvalue_from_list(lrouter, 'static_routes', route)
route.delete()
class UpdateObjectExtIdsCommand(command.BaseCommand):

5
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py
View File

@ -474,9 +474,8 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend):
return cmd.AddStaticRouteCommand(self, lrouter, maintain_bfd,
**columns)
def delete_static_route(self, lrouter, ip_prefix, nexthop, if_exists=True):
return cmd.DelStaticRouteCommand(self, lrouter, ip_prefix, nexthop,
if_exists)
def delete_static_routes(self, lrouter, routes, if_exists=True):
return cmd.DelStaticRoutesCommand(self, lrouter, routes, if_exists)
def _get_logical_router_port_gateway_chassis(self, lrp, priorities=None):
"""Get the list of chassis hosting this gateway port.

18
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py
View File

@ -972,14 +972,16 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase):
for router in self._nb_idl.lr_list().execute(check_error=True):
if not router.external_ids.get(ovn_const.OVN_REV_NUM_EXT_ID_KEY):
continue
for route in self._nb_idl.lr_route_list(router.uuid).execute(
check_error=True):
if (route.nexthop == '' and
route.ip_prefix in (n_const.IPv4_ANY,
n_const.IPv6_ANY)):
cmds.append(
self._nb_idl.delete_static_route(
router.name, route.ip_prefix, ''))
routes_to_delete = [
(r.ip_prefix, '')
for r in self._nb_idl.lr_route_list(router.uuid).execute(
check_error=True)
if r.nexthop == '' and r.ip_prefix in (n_const.IPv4_ANY,
n_const.IPv6_ANY)
]
cmds.append(
self._nb_idl.delete_static_routes(router.name,
routes_to_delete))
if cmds:
with self._nb_idl.transaction(check_error=True) as txn:

21
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
View File

@ -1210,15 +1210,17 @@ class OVNClient(object):
gw_lrouter_name = utils.ovn_name(router_id)
deleted_ports = []
for gw_port in self._get_router_gw_ports(context, router_id):
routes_to_delete = []
for gw_info in self._get_gw_info(context, gw_port):
if gw_info.ip_version == const.IP_VERSION_4:
for network in networks:
txn.add(self._nb_idl.delete_nat_rule_in_lrouter(
gw_lrouter_name, type='snat', logical_ip=network,
external_ip=gw_info.router_ip))
txn.add(self._nb_idl.delete_static_route(
gw_lrouter_name, ip_prefix=gw_info.ip_prefix,
nexthop=gw_info.gateway_ip))
routes_to_delete.append((gw_info.ip_prefix,
gw_info.gateway_ip))
txn.add(self._nb_idl.delete_static_routes(
gw_lrouter_name, routes_to_delete))
txn.add(self._nb_idl.delete_lrouter_port(
utils.ovn_lrouter_port_name(gw_port['id']),
gw_lrouter_name))
@ -1370,11 +1372,14 @@ class OVNClient(object):
self._nb_idl.add_static_route(
lrouter_name, ip_prefix=route['destination'],
nexthop=route['nexthop']))
for route in remove:
commands.append(
self._nb_idl.delete_static_route(
lrouter_name, ip_prefix=route['destination'],
nexthop=route['nexthop']))
routes_to_delete = [
(r['destination'], r['nexthop'])
for r in remove
]
commands.append(
self._nb_idl.delete_static_routes(lrouter_name,
routes_to_delete)
)
self._transaction(commands, txn=txn)
def _get_router_gw_ports(self, context, router_id):

11
neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py
View File

@ -749,11 +749,12 @@ class OvnNbSynchronizer(OvnDbSynchronizer):
if self.mode == SYNC_MODE_REPAIR:
LOG.warning("Delete static routes %s from OVN NB DB",
sroute['del'])
for route in sroute['del']:
txn.add(self.ovn_api.delete_static_route(
utils.ovn_name(sroute['id']),
ip_prefix=route['destination'],
nexthop=route['nexthop']))
routes_to_delete = [
(r['destination'], r['nexthop'])
for r in sroute['del']
]
txn.add(self.ovn_api.delete_static_routes(
utils.ovn_name(sroute['id']), routes_to_delete))
for fip in update_fips_list:
if fip['del']:
LOG.warning("Router %(id)s floating IPs %(fip)s "

40
neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py
View File

@ -626,6 +626,46 @@ class TestNbApi(BaseOvnIdlTest):
self.assertIsNone(
self.nbapi.lookup('HA_Chassis_Group', router_name, default=None))
def _assert_routes_exist(self, lr_name, expected_count):
with self.nbapi.transaction(check_error=True) as txn:
lr = txn.add(self.nbapi.lr_get(lr_name))
actual_count = len(lr.result.static_routes)
self.assertEqual(actual_count, expected_count,
f"Expected {expected_count} routes, "
f"found {actual_count}.")
def test_del_static_routes(self):
lr_name = 'router_with_static_routes_del'
routes = [('0.0.0.0/0', '192.0.2.1'), ('10.0.0.0/24', '192.0.3.1')]
with self.nbapi.transaction(check_error=True) as txn:
txn.add(self.nbapi.lr_add(lr_name))
for ip_prefix, nexthop in routes:
txn.add(self.nbapi.add_static_route(lr_name,
ip_prefix=ip_prefix,
nexthop=nexthop))
self._assert_routes_exist(lr_name, 2)
with self.nbapi.transaction(check_error=True) as txn:
txn.add(self.nbapi.delete_static_routes(lr_name, routes))
self._assert_routes_exist(lr_name, 0)
def test_del_no_static_routes(self):
lr_name = 'router_with_static_routes_del'
routes = []
with self.nbapi.transaction(check_error=True) as txn:
txn.add(self.nbapi.lr_add(lr_name))
self._assert_routes_exist(lr_name, 0)
with self.nbapi.transaction(check_error=True) as txn:
txn.add(self.nbapi.delete_static_routes(lr_name, routes))
self._assert_routes_exist(lr_name, 0)
class TestIgnoreConnectionTimeout(BaseOvnIdlTest):
@classmethod

10
neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py
View File

@ -12,6 +12,7 @@
# License for the specific language governing permissions and limitations
# under the License.
from collections import defaultdict
from collections import namedtuple
import netaddr
@ -797,10 +798,13 @@ class TestOvnNbSync(base.TestOVNFunctionalBase):
txn.add(self.nb_api.add_static_route(lrouter_name,
ip_prefix=ip_prefix,
nexthop=nexthop))
routers = defaultdict(list)
for lrouter_name, ip_prefix, nexthop in self.delete_lrouter_routes:
txn.add(self.nb_api.delete_static_route(lrouter_name,
ip_prefix, nexthop,
routers[lrouter_name].append((ip_prefix, nexthop))
for lrouter_name, routes_to_delete in routers.items():
txn.add(self.nb_api.delete_static_routes(lrouter_name,
routes_to_delete,
True))
for lrouter_name, nat_dict in self.create_lrouter_nats:

2
neutron/tests/unit/fake_resources.py
View File

@ -76,7 +76,7 @@ class FakeOvsdbNbOvnIdl(object):
self.update_acls = mock.Mock()
self.idl = mock.Mock()
self.add_static_route = mock.Mock()
self.delete_static_route = mock.Mock()
self.delete_static_routes = mock.Mock()
self.get_all_chassis_gateway_bindings = mock.Mock()
self.get_chassis_gateways = mock.Mock()
self.get_gateway_chassis_binding = mock.Mock()

23
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py
View File

@ -730,14 +730,14 @@ class TestAddStaticRouteCommand(TestBaseCommand):
'static_routes', fake_static_route.uuid)
class TestDelStaticRouteCommand(TestBaseCommand):
class TestDelStaticRoutesCommand(TestBaseCommand):
def _test_lrouter_no_exist(self, if_exists=True):
with mock.patch.object(idlutils, 'row_by_value',
side_effect=idlutils.RowNotFound):
cmd = commands.DelStaticRouteCommand(
cmd = commands.DelStaticRoutesCommand(
self.ovn_api, 'fake-lrouter',
'30.0.0.0/24', '40.0.0.100',
[('30.0.0.0/24', '40.0.0.100')],
if_exists=if_exists)
if if_exists:
cmd.run_idl(self.transaction)
@ -751,18 +751,21 @@ class TestDelStaticRouteCommand(TestBaseCommand):
self._test_lrouter_no_exist(if_exists=False)
def test_static_route_del(self):
fake_static_route = fakes.FakeOvsdbRow.create_one_ovsdb_row(
fake_static_route1 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'ip_prefix': '50.0.0.0/24', 'nexthop': '40.0.0.101'})
fake_static_route2 = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'ip_prefix': '60.0.0.0/24', 'nexthop': '40.0.0.101'})
fake_lrouter = fakes.FakeOvsdbRow.create_one_ovsdb_row(
attrs={'static_routes': [fake_static_route]})
attrs={'static_routes': [fake_static_route1, fake_static_route2]})
with mock.patch.object(idlutils, 'row_by_value',
return_value=fake_lrouter):
cmd = commands.DelStaticRouteCommand(
cmd = commands.DelStaticRoutesCommand(
self.ovn_api, fake_lrouter.name,
fake_static_route.ip_prefix, fake_static_route.nexthop,
[(fake_static_route1.ip_prefix, fake_static_route1.nexthop),
(fake_static_route2.ip_prefix, fake_static_route2.nexthop)],
if_exists=True)
cmd.run_idl(self.transaction)
fake_lrouter.delvalue.assert_called_once_with(
fake_lrouter.delvalue.assert_called_with(
'static_routes', mock.ANY)
def test_static_route_del_not_found(self):
@ -774,9 +777,9 @@ class TestDelStaticRouteCommand(TestBaseCommand):
attrs={'static_routes': [fake_static_route2]})
with mock.patch.object(idlutils, 'row_by_value',
return_value=fake_lrouter):
cmd = commands.DelStaticRouteCommand(
cmd = commands.DelStaticRoutesCommand(
self.ovn_api, fake_lrouter.name,
fake_static_route1.ip_prefix, fake_static_route1.nexthop,
[(fake_static_route1.ip_prefix, fake_static_route1.nexthop)],
if_exists=True)
cmd.run_idl(self.transaction)
fake_lrouter.delvalue.assert_not_called()

10
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py
View File

@ -1034,13 +1034,13 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight,
self.assertRaises(
periodics.NeverAgain,
self.periodic.check_router_default_route_empty_dst_ip)
nb_idl.delete_static_route.assert_has_calls([
mock.call(router1.name, route1.ip_prefix, route1.nexthop),
mock.call(router1.name, route3.ip_prefix, route3.nexthop),
nb_idl.delete_static_routes.assert_has_calls([
mock.call(router1.name, [(route1.ip_prefix, route1.nexthop),
(route3.ip_prefix, route3.nexthop)]),
])
self.assertEqual(
2,
nb_idl.delete_static_route.call_count)
1,
nb_idl.delete_static_routes.call_count)
@mock.patch.object(ports_obj.PortBinding, 'get_port_binding_by_vnic_type')
def test_add_vnic_type_and_pb_capabilities_to_lsp(self, mock_get_pb):

15
neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py
View File

@ -483,7 +483,7 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase):
ovn_api.delete_lrouter = mock.Mock()
ovn_api.delete_lrouter_port = mock.Mock()
ovn_api.add_static_route = mock.Mock()
ovn_api.delete_static_route = mock.Mock()
ovn_api.delete_static_routes = mock.Mock()
ovn_api.get_all_dhcp_options.return_value = {
'subnets': {'n1-s1': {'cidr': '10.0.0.0/24',
'options':
@ -641,13 +641,14 @@ class TestOvnNbSyncML2(test_mech_driver.OVNMechanismDriverTestCase):
any_order=True)
self.assertEqual(len(add_static_route_list),
ovn_api.add_static_route.call_count)
del_route_calls = [mock.call(mock.ANY, ip_prefix=route['destination'],
nexthop=route['nexthop'])
routes_to_delete = [(route['destination'], route['nexthop'])
for route in del_static_route_list]
ovn_api.delete_static_route.assert_has_calls(del_route_calls,
any_order=True)
self.assertEqual(len(del_static_route_list),
ovn_api.delete_static_route.call_count)
del_route_call = [mock.call(mock.ANY, routes_to_delete)] \
if routes_to_delete else []
ovn_api.delete_static_routes.assert_has_calls(del_route_call)
self.assertEqual(1 if len(del_static_route_list) else 0,
ovn_api.delete_static_routes.call_count)
add_nat_calls = [mock.call(mock.ANY, **nat) for nat in add_snat_list]
ovn_api.add_nat_rule_in_lrouter.assert_has_calls(add_nat_calls,

22
neutron/tests/unit/services/ovn_l3/test_plugin.py
View File

@ -562,7 +562,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
'nexthop': '10.0.0.2'}]}}
self.l3_inst.update_router(self.context, router_id, update_data)
self.assertFalse(self.l3_inst._nb_ovn.add_static_route.called)
self.assertFalse(self.l3_inst._nb_ovn.delete_static_route.called)
self.assertFalse(self.l3_inst._nb_ovn.delete_static_routes.called)
@mock.patch.object(utils, 'get_lrouter_non_gw_routes')
@mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.'
@ -592,9 +592,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
self.l3_inst._nb_ovn.add_static_route.assert_called_once_with(
'neutron-router-id',
ip_prefix='2.2.2.0/24', nexthop='10.0.0.3')
self.l3_inst._nb_ovn.delete_static_route.assert_called_once_with(
'neutron-router-id',
ip_prefix='1.1.1.0/24', nexthop='10.0.0.2')
self.l3_inst._nb_ovn.delete_static_routes.assert_called_once_with(
'neutron-router-id', [('1.1.1.0/24', '10.0.0.2')])
@mock.patch.object(utils, 'get_lrouter_non_gw_routes')
@mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.'
@ -621,9 +620,8 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
events.AFTER_UPDATE,
self, payload)
self.l3_inst._nb_ovn.add_static_route.assert_not_called()
self.l3_inst._nb_ovn.delete_static_route.assert_called_once_with(
'neutron-router-id',
ip_prefix='1.1.1.0/24', nexthop='10.0.0.2')
self.l3_inst._nb_ovn.delete_static_routes.assert_called_once_with(
'neutron-router-id', [('1.1.1.0/24', '10.0.0.2')])
@mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.'
'ovn_client.OVNClient._get_v4_network_of_all_router_ports')
@ -984,7 +982,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
self.l3_inst.update_router(self.context, 'router-id', router)
nb_ovn.lrp_del.assert_not_called()
nb_ovn.delete_static_route.assert_not_called()
nb_ovn.delete_static_routes.assert_not_called()
nb_ovn.delete_nat_rule_in_lrouter.assert_not_called()
nb_ovn.add_lrouter_port.assert_not_called()
nb_ovn.set_lrouter_port_in_lswitch_port.assert_not_called()
@ -1046,7 +1044,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
events.AFTER_UPDATE,
self, payload)
self.l3_inst._nb_ovn.delete_static_route.assert_not_called()
self.l3_inst._nb_ovn.delete_static_routes.assert_not_called()
self.l3_inst._nb_ovn.delete_nat_rule_in_lrouter.assert_not_called()
self.l3_inst._nb_ovn.add_static_route.assert_not_called()
self.l3_inst._nb_ovn.add_nat_rule_in_lrouter.assert_called_once_with(
@ -1084,7 +1082,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase):
self, payload)
nb_ovn = self.l3_inst._nb_ovn
nb_ovn.delete_static_route.assert_not_called()
nb_ovn.delete_static_routes.assert_not_called()
nb_ovn.delete_nat_rule_in_lrouter.assert_called_once_with(
'neutron-router-id', type='snat', logical_ip='10.0.0.0/24',
external_ip='192.168.1.1')
@ -2165,8 +2163,8 @@ class OVNL3ExtrarouteTests(test_l3_gw.ExtGwModeIntTestCase,
test_update_subnet_gateway_for_external_net()
self.l3_inst._nb_ovn.add_static_route.assert_called_once_with(
'neutron-fake_device', ip_prefix='0.0.0.0/0', nexthop='120.0.0.2')
self.l3_inst._nb_ovn.delete_static_route.assert_called_once_with(
'neutron-fake_device', ip_prefix='0.0.0.0/0', nexthop='120.0.0.1')
self.l3_inst._nb_ovn.delete_static_routes.assert_called_once_with(
'neutron-fake_device', [('0.0.0.0/0', '120.0.0.1')])
def test_router_update_gateway_upon_subnet_create_max_ips_ipv6(self):
super(OVNL3ExtrarouteTests, self). \