Static routes not added to qrouter namespace for DVR
Today static routes are added to the SNAT namespace
for DVR routers. But they are not added to the qrouter
namespace.
Also while configuring the static routes to SNAT
namespace, the router is not checked for the existence
of the gateway.
When routes are added to a router without a gateway the
routes are only configured in the router namespace, but
when a gateway is set later, those routes have to be
populated in the snat_namespace as well.
This patch addresses the above mentioned issues.
Closes-Bug: #1499785
Closes-Bug: #1499787
Conflicts:
neutron/agent/l3/dvr_edge_router.py
neutron/tests/functional/agent/test_l3_agent.py
neutron/tests/functional/agent/l3/framework.py
neutron/tests/functional/agent/l3/test_dvr_router.py
Change-Id: I37e0d0d723fcc727faa09028045b776957c75a82
(cherry picked from commit 158f9eabe2
)
This commit is contained in:
parent
c356f1d413
commit
4bb364044b
|
@ -19,6 +19,7 @@ from neutron.agent.l3 import dvr_snat_ns
|
|||
from neutron.agent.l3 import router_info as router
|
||||
from neutron.agent.linux import ip_lib
|
||||
from neutron.agent.linux import iptables_manager
|
||||
from neutron.i18n import _LE
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
@ -35,6 +36,11 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
|
|||
ex_gw_port, interface_name)
|
||||
if self._is_this_snat_host():
|
||||
self._create_dvr_gateway(ex_gw_port, interface_name)
|
||||
# NOTE: When a router is created without a gateway the routes get
|
||||
# added to the router namespace, but if we wanted to populate
|
||||
# the same routes to the snat namespace after the gateway port
|
||||
# is added, we need to call routes_updated here.
|
||||
self.routes_updated([], self.router['routes'])
|
||||
|
||||
def external_gateway_updated(self, ex_gw_port, interface_name):
|
||||
if not self._is_this_snat_host():
|
||||
|
@ -175,7 +181,17 @@ 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, namespace=None):
|
||||
ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name(self.router['id'])
|
||||
super(DvrEdgeRouter, self).update_routing_table(operation, route,
|
||||
namespace=ns_name)
|
||||
def update_routing_table(self, operation, route):
|
||||
if self.get_ex_gw_port() and self._is_this_snat_host():
|
||||
ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name(
|
||||
self.router['id'])
|
||||
# NOTE: For now let us apply the static routes both in SNAT
|
||||
# namespace and Router Namespace, to reduce the complexity.
|
||||
ip_wrapper = ip_lib.IPWrapper(namespace=ns_name)
|
||||
if ip_wrapper.netns.exists(ns_name):
|
||||
super(DvrEdgeRouter, self)._update_routing_table(
|
||||
operation, route, namespace=ns_name)
|
||||
else:
|
||||
LOG.error(_LE("The SNAT namespace %s does not exist for "
|
||||
"the router."), ns_name)
|
||||
super(DvrEdgeRouter, self).update_routing_table(operation, route)
|
||||
|
|
|
@ -175,15 +175,12 @@ class HaRouter(router.RouterInfo):
|
|||
def get_router_cidrs(self, device):
|
||||
return set(self._get_cidrs_from_keepalived(device.name))
|
||||
|
||||
def routes_updated(self):
|
||||
new_routes = self.router['routes']
|
||||
|
||||
def routes_updated(self, old_routes, new_routes):
|
||||
instance = self._get_keepalived_instance()
|
||||
instance.virtual_routes.extra_routes = [
|
||||
keepalived.KeepalivedVirtualRoute(
|
||||
route['destination'], route['nexthop'])
|
||||
for route in new_routes]
|
||||
self.routes = new_routes
|
||||
|
||||
def _add_default_gw_virtual_route(self, ex_gw_port, interface_name):
|
||||
gateway_ips = self._get_external_gw_ips(ex_gw_port)
|
||||
|
|
|
@ -116,15 +116,10 @@ class RouterInfo(object):
|
|||
ip_wrapper = ip_lib.IPWrapper(namespace=namespace)
|
||||
ip_wrapper.netns.execute(cmd, check_exit_code=False)
|
||||
|
||||
def update_routing_table(self, operation, route, namespace=None):
|
||||
if namespace is None:
|
||||
namespace = self.ns_name
|
||||
self._update_routing_table(operation, route, namespace)
|
||||
def update_routing_table(self, operation, route):
|
||||
self._update_routing_table(operation, route, self.ns_name)
|
||||
|
||||
def routes_updated(self):
|
||||
new_routes = self.router['routes']
|
||||
|
||||
old_routes = self.routes
|
||||
def routes_updated(self, old_routes, new_routes):
|
||||
adds, removes = common_utils.diff_list_of_dict(old_routes,
|
||||
new_routes)
|
||||
for route in adds:
|
||||
|
@ -138,7 +133,6 @@ class RouterInfo(object):
|
|||
for route in removes:
|
||||
LOG.debug("Removed route entry is '%s'", route)
|
||||
self.update_routing_table('delete', route)
|
||||
self.routes = new_routes
|
||||
|
||||
def get_ex_gw_port(self):
|
||||
return self.router.get('gw_port')
|
||||
|
@ -692,7 +686,8 @@ class RouterInfo(object):
|
|||
agent.pd.sync_router(self.router['id'])
|
||||
self.process_external(agent)
|
||||
# Process static routes for router
|
||||
self.routes_updated()
|
||||
self.routes_updated(self.routes, self.router['routes'])
|
||||
self.routes = self.router['routes']
|
||||
|
||||
# Update ex_gw_port and enable_snat on the router info cache
|
||||
self.ex_gw_port = self.get_ex_gw_port()
|
||||
|
|
|
@ -278,8 +278,10 @@ class L3AgentTestFramework(base.BaseSudoTestCase):
|
|||
self.assertTrue(self.device_exists_with_ips_and_mac(
|
||||
device, router.get_internal_device_name, router.ns_name))
|
||||
|
||||
def _assert_extra_routes(self, router):
|
||||
routes = ip_lib.get_routing_table(4, namespace=router.ns_name)
|
||||
def _assert_extra_routes(self, router, namespace=None):
|
||||
if namespace is None:
|
||||
namespace = router.ns_name
|
||||
routes = ip_lib.get_routing_table(4, namespace=namespace)
|
||||
routes = [{'nexthop': route['nexthop'],
|
||||
'destination': route['destination']} for route in routes]
|
||||
|
||||
|
@ -1091,7 +1093,7 @@ class TestDvrRouter(L3AgentTestFramework):
|
|||
|
||||
# We get the router info particular to a dvr router
|
||||
router_info = self.generate_dvr_router_info(
|
||||
enable_ha, enable_snat)
|
||||
enable_ha, enable_snat, extra_routes=True)
|
||||
|
||||
# We need to mock the get_agent_gateway_port return value
|
||||
# because the whole L3PluginApi is mocked and we need the port
|
||||
|
@ -1120,19 +1122,27 @@ class TestDvrRouter(L3AgentTestFramework):
|
|||
self._assert_snat_chains(router)
|
||||
self._assert_floating_ip_chains(router)
|
||||
self._assert_metadata_chains(router)
|
||||
self._assert_extra_routes(router)
|
||||
self._assert_rfp_fpr_mtu(router, custom_mtu)
|
||||
if enable_snat:
|
||||
snat_ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name(
|
||||
router.router_id)
|
||||
self._assert_extra_routes(router, namespace=snat_ns_name)
|
||||
|
||||
self._delete_router(self.agent, router.router_id)
|
||||
self._assert_interfaces_deleted_from_ovs()
|
||||
self._assert_router_does_not_exist(router)
|
||||
|
||||
def generate_dvr_router_info(
|
||||
self, enable_ha=False, enable_snat=False, **kwargs):
|
||||
def generate_dvr_router_info(self,
|
||||
enable_ha=False,
|
||||
enable_snat=False,
|
||||
extra_routes=False,
|
||||
**kwargs):
|
||||
router = l3_test_common.prepare_router_data(
|
||||
enable_snat=enable_snat,
|
||||
enable_floating_ip=True,
|
||||
enable_ha=enable_ha,
|
||||
extra_routes=extra_routes,
|
||||
num_internal_ports=2,
|
||||
**kwargs)
|
||||
internal_ports = router.get(l3_constants.INTERFACE_KEY, [])
|
||||
router['distributed'] = True
|
||||
|
@ -1505,3 +1515,21 @@ class TestDvrRouter(L3AgentTestFramework):
|
|||
self.agent.context,
|
||||
floating_agent_gw_port[0]['network_id'])
|
||||
self.assertFalse(self._namespace_exists(fip_ns))
|
||||
|
||||
def test_dvr_router_static_routes(self):
|
||||
"""Test to validate the extra routes on dvr routers."""
|
||||
self.agent.conf.agent_mode = 'dvr_snat'
|
||||
router_info = self.generate_dvr_router_info(enable_snat=True)
|
||||
router1 = self.manage_router(self.agent, router_info)
|
||||
self.assertTrue(self._namespace_exists(router1.ns_name))
|
||||
self._assert_snat_namespace_exists(router1)
|
||||
snat_ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name(
|
||||
router1.router_id)
|
||||
# Now try to add routes that are suitable for both the
|
||||
# router namespace and the snat namespace.
|
||||
router1.router['routes'] = [{'destination': '8.8.4.0/24',
|
||||
'nexthop': '35.4.0.20'}]
|
||||
self.agent._process_updated_router(router1.router)
|
||||
router_updated = self.agent.router_info[router_info['id']]
|
||||
self._assert_extra_routes(router_updated, namespace=snat_ns_name)
|
||||
self._assert_extra_routes(router_updated)
|
||||
|
|
|
@ -1096,14 +1096,14 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
|
|||
router)
|
||||
self.assertEqual(self.send_adv_notif.call_count, 1)
|
||||
|
||||
def test_update_routing_table(self):
|
||||
# Just verify the correct namespace was used in the call
|
||||
def _test_update_routing_table(self, is_snat_host=True):
|
||||
router = l3_test_common.prepare_router_data()
|
||||
uuid = router['id']
|
||||
netns = 'snat-' + uuid
|
||||
s_netns = 'snat-' + uuid
|
||||
q_netns = 'qrouter-' + uuid
|
||||
fake_route1 = {'destination': '135.207.0.0/16',
|
||||
'nexthop': '1.2.3.4'}
|
||||
|
||||
'nexthop': '19.4.4.200'}
|
||||
calls = [mock.call('replace', fake_route1, q_netns)]
|
||||
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
|
||||
ri = dvr_router.DvrEdgeRouter(
|
||||
agent,
|
||||
|
@ -1113,10 +1113,19 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
|
|||
**self.ri_kwargs)
|
||||
ri._update_routing_table = mock.Mock()
|
||||
|
||||
ri.update_routing_table('replace', fake_route1)
|
||||
ri._update_routing_table.assert_called_once_with('replace',
|
||||
fake_route1,
|
||||
netns)
|
||||
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)
|
||||
calls += [mock.call('replace', fake_route1, s_netns)]
|
||||
ri._update_routing_table.assert_has_calls(calls, any_order=True)
|
||||
|
||||
def test_process_update_snat_routing_table(self):
|
||||
self._test_update_routing_table()
|
||||
|
||||
def test_process_not_update_snat_routing_table(self):
|
||||
self._test_update_routing_table(is_snat_host=False)
|
||||
|
||||
def test_process_router_interface_added(self):
|
||||
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
|
||||
|
|
|
@ -98,7 +98,7 @@ class TestRouterInfo(base.BaseTestCase):
|
|||
'nexthop': "10.100.10.30"}]
|
||||
ri.routes = fake_old_routes
|
||||
ri.router['routes'] = fake_new_routes
|
||||
ri.routes_updated()
|
||||
ri.routes_updated(fake_old_routes, fake_new_routes)
|
||||
|
||||
expected = [['ip', 'route', 'replace', 'to', '110.100.30.0/24',
|
||||
'via', '10.100.10.30'],
|
||||
|
@ -106,18 +106,18 @@ class TestRouterInfo(base.BaseTestCase):
|
|||
'via', '10.100.10.30']]
|
||||
|
||||
self._check_agent_method_called(expected)
|
||||
|
||||
ri.routes = fake_new_routes
|
||||
fake_new_routes = [{'destination': "110.100.30.0/24",
|
||||
'nexthop': "10.100.10.30"}]
|
||||
ri.router['routes'] = fake_new_routes
|
||||
ri.routes_updated()
|
||||
ri.routes_updated(ri.routes, fake_new_routes)
|
||||
expected = [['ip', 'route', 'delete', 'to', '110.100.31.0/24',
|
||||
'via', '10.100.10.30']]
|
||||
|
||||
self._check_agent_method_called(expected)
|
||||
fake_new_routes = []
|
||||
ri.router['routes'] = fake_new_routes
|
||||
ri.routes_updated()
|
||||
ri.routes_updated(ri.routes, fake_new_routes)
|
||||
|
||||
expected = [['ip', 'route', 'delete', 'to', '110.100.30.0/24',
|
||||
'via', '10.100.10.30']]
|
||||
|
|
Loading…
Reference in New Issue