openstack/neutron
Code Issues Proposed changes

Do not update static routes in snat-ns for dvr router with ha

Browse Source 
If a router is distributed with ha enabled, then the keepalived service
is responsible for setting static routes. This patch adds a check if
the router ha is disabled before adding routes. Otherwise, there are
duplicate routes and the issue when this route needs to be removed.
In addition this patch fixes multipath route in the snat-ns if no HA is
enabled.

Closes-Bug: #1999678
Signed-off-by: Anton Kurbatov <Anton.Kurbatov@acronis.com>
Change-Id: I8f1004b3fe2cad79cb61aa942b257c1508d18b68
This commit is contained in:
Anton Kurbatov
2022-12-14 16:50:06 +00:00
parent b5fdad3cfb
commit f3c743d090
4 changed files with 71 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
neutron/agent/l3/dvr_edge_ha_router.py
View File

@@ -154,3 +154,7 @@ class DvrEdgeHaRouter(dvr_edge_router.DvrEdgeRouter,
namespace=self.snat_namespace.name,
prefix=constants.SNAT_INT_DEV_PREFIX,
mtu=port.get('mtu'))
def _should_update_snat_routing_table(self):
# NOTE: keepalived is responsible for routes updating
return False

19
neutron/agent/l3/dvr_edge_router.py
View File

@@ -244,18 +244,29 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
self._add_snat_rules(ex_gw_port, self.snat_iptables_manager,
interface_name)
def update_routing_table(self, operation, route):
def _should_update_snat_routing_table(self):
if self.get_ex_gw_port() and self._is_this_snat_host():
ns_name = self.snat_namespace.name
# NOTE: For now let us apply the static routes both in SNAT
# namespace and Router Namespace, to reduce the complexity.
if self.snat_namespace.exists():
self._update_routing_table(operation, route, ns_name)
return True
else:
LOG.error("The SNAT namespace %s does not exist for "
"the router.", ns_name)
"the router.", self.snat_namespace.name)
return False
def update_routing_table(self, operation, route):
if self._should_update_snat_routing_table():
ns_name = self.snat_namespace.name
self._update_routing_table(operation, route, ns_name)
super(DvrEdgeRouter, self).update_routing_table(operation, route)
def update_routing_table_ecmp(self, route_list):
if self._should_update_snat_routing_table():
ns_name = self.snat_namespace.name
self._update_routing_table_ecmp(route_list, ns_name)
super(DvrEdgeRouter, self).update_routing_table_ecmp(route_list)
def delete(self):
super(DvrEdgeRouter, self).delete()
if self.snat_namespace.exists():

7
neutron/agent/l3/router_info.py
View File

@@ -187,15 +187,18 @@ class RouterInfo(BaseRouterInfo):
def update_routing_table(self, operation, route):
self._update_routing_table(operation, route, self.ns_name)
def update_routing_table_ecmp(self, route_list):
def _update_routing_table_ecmp(self, route_list, namespace):
multipath = [dict(via=route['nexthop'])
for route in route_list]
try:
ip_lib.add_ip_route(self.ns_name, route_list[0]['destination'],
ip_lib.add_ip_route(namespace, route_list[0]['destination'],
via=multipath)
except (RuntimeError, OSError, pyroute2_exc.NetlinkError):
pass
def update_routing_table_ecmp(self, route_list):
self._update_routing_table_ecmp(route_list, self.ns_name)
def check_and_remove_ecmp_route(self, old_routes, remove_route):
route_list = []
for route in old_routes:

52
neutron/tests/unit/agent/l3/test_agent.py
View File

@@ -1848,8 +1848,8 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
def test_process_router_snat_enabled_random_fully_false(self):
self._test_process_router_snat_enabled(False)
def _test_update_routing_table(self, is_snat_host=True):
router = l3_test_common.prepare_router_data()
def _test_update_routing_table(self, is_snat_host=True, enable_ha=False):
router = l3_test_common.prepare_router_data(enable_ha=enable_ha)
uuid = router['id']
s_netns = 'snat-' + uuid
q_netns = 'qrouter-' + uuid
@@ -1858,16 +1858,18 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
calls = [mock.call('replace', fake_route1, q_netns)]
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
self._set_ri_kwargs(agent, uuid, router)
ri = dvr_router.DvrEdgeRouter(HOSTNAME, **self.ri_kwargs)
router_cls = (dvr_edge_ha_router.DvrEdgeHaRouter if enable_ha else
dvr_router.DvrEdgeRouter)
ri = router_cls(HOSTNAME, **self.ri_kwargs)
ri._update_routing_table = mock.Mock()
with mock.patch.object(ri, '_is_this_snat_host') as snat_host:
snat_host.return_value = is_snat_host
ri.update_routing_table('replace', fake_route1)
if is_snat_host:
ri._update_routing_table('replace', fake_route1, s_netns)
if is_snat_host and not enable_ha:
calls += [mock.call('replace', fake_route1, s_netns)]
ri._update_routing_table.assert_has_calls(calls, any_order=True)
self.assertEqual(len(calls), ri._update_routing_table.call_count)
def test_process_update_snat_routing_table(self):
self._test_update_routing_table()
@@ -1875,6 +1877,46 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
def test_process_not_update_snat_routing_table(self):
self._test_update_routing_table(is_snat_host=False)
def test_process_not_update_ha_routing_table(self):
self._test_update_routing_table(enable_ha=True)
def _test_update_routing_table_ecmp(self, is_snat_host=True,
enable_ha=False):
router = l3_test_common.prepare_router_data()
uuid = router['id']
s_netns = 'snat-' + uuid
q_netns = 'qrouter-' + uuid
fake_route_list = [{'destination': '135.207.0.0/16',
'nexthop': '19.4.4.200'},
{'destination': '135.207.0.0/16',
'nexthop': '19.4.4.201'}]
calls = [mock.call(fake_route_list, q_netns)]
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
self._set_ri_kwargs(agent, uuid, router)
router_cls = (dvr_edge_ha_router.DvrEdgeHaRouter if enable_ha else
dvr_router.DvrEdgeRouter)
ri = router_cls(HOSTNAME, **self.ri_kwargs)
ri._update_routing_table_ecmp = mock.Mock()
with mock.patch.object(ri, '_is_this_snat_host') as snat_host:
snat_host.return_value = is_snat_host
ri.update_routing_table_ecmp(fake_route_list)
if is_snat_host and not enable_ha:
calls += [mock.call(fake_route_list, s_netns)]
ri._update_routing_table_ecmp.assert_has_calls(calls,
any_order=True)
self.assertEqual(
len(calls), ri._update_routing_table_ecmp.call_count)
def test_process_update_snat_routing_table_ecmp(self):
self._test_update_routing_table_ecmp()
def test_process_not_update_snat_routing_table_ecmp(self):
self._test_update_routing_table_ecmp(is_snat_host=False)
def test_process_not_update_ha_routing_table_ecmp(self):
self._test_update_routing_table_ecmp(enable_ha=True)
def test_process_router_interface_added(self):
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
router = l3_test_common.prepare_router_data()