From b119247bea89aec5604e7ac61675aed240195931 Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Mon, 4 Mar 2019 13:16:04 +0100 Subject: [PATCH] Enable ipv6_forwarding in HA router's namespace When HA router is created in "stanby" mode, ipv6 forwarding is disabled by default in its namespace. But when router is transitioned to be "master" on node, ipv6 forwarding should be enabled. This was fine for routers with configured gateway but we somehow missed the case when router don't have gateway configured. Because of that missing ipv6 forwarding setting in such case, IPv6 W-E traffic between 2 subnets was not working fine in L3 HA case. This patch fixes it by adding configuring ipv6_forwarding on "all" interface in router's namespace always, even if it don't have gateway configured. Change-Id: I8b1b2b426f7a26a4b2407a83f9bf29dd6e9ba7b0 CLoses-Bug: #1818224 --- neutron/agent/l3/ha.py | 34 +++++++----- .../tests/functional/agent/l3/framework.py | 4 +- .../functional/agent/l3/test_ha_router.py | 25 ++++++++- neutron/tests/unit/agent/l3/test_agent.py | 52 ++++++++++++------- 4 files changed, 80 insertions(+), 35 deletions(-) diff --git a/neutron/agent/l3/ha.py b/neutron/agent/l3/ha.py index 9ef3bd878bf..899be9b7f00 100644 --- a/neutron/agent/l3/ha.py +++ b/neutron/agent/l3/ha.py @@ -125,7 +125,7 @@ class AgentMixin(object): # configuration to keepalived-state-change in order to remove the # dependency that currently exists on l3-agent running for the IPv6 # failover. - self._configure_ipv6_params_on_ext_gw_port_if_necessary(ri, state) + self._configure_ipv6_params(ri, state) if self.conf.enable_metadata_proxy: self._update_metadata_proxy(ri, router_id, state) self._update_radvd_daemon(ri, state) @@ -133,25 +133,31 @@ class AgentMixin(object): self.state_change_notifier.queue_event((router_id, state)) self.l3_ext_manager.ha_state_change(self.context, state_change_data) - def _configure_ipv6_params_on_ext_gw_port_if_necessary(self, ri, state): + def _configure_ipv6_params(self, ri, state): + if not self.use_ipv6: + return + + ipv6_forwarding_enable = state == 'master' + if ri.router.get('distributed', False): + namespace = ri.ha_namespace + else: + namespace = ri.ns_name + + if ipv6_forwarding_enable: + ri.driver.configure_ipv6_forwarding( + namespace, 'all', ipv6_forwarding_enable) + # If ipv6 is enabled on the platform, ipv6_gateway config flag is # not set and external_network associated to the router does not # include any IPv6 subnet, enable the gateway interface to accept # Router Advts from upstream router for default route on master # instances as well as ipv6 forwarding. Otherwise, disable them. ex_gw_port_id = ri.ex_gw_port and ri.ex_gw_port['id'] - if not ex_gw_port_id: - return - - interface_name = ri.get_external_device_name(ex_gw_port_id) - if ri.router.get('distributed', False): - namespace = ri.ha_namespace - else: - namespace = ri.ns_name - - enable = state == 'master' - ri._configure_ipv6_params_on_gw(ri.ex_gw_port, namespace, - interface_name, enable) + if ex_gw_port_id: + interface_name = ri.get_external_device_name(ex_gw_port_id) + ri._configure_ipv6_params_on_gw( + ri.ex_gw_port, namespace, interface_name, + ipv6_forwarding_enable) def _update_metadata_proxy(self, ri, router_id, state): # NOTE(slaweq): Since the metadata proxy is spawned in the qrouter diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index effe7a6f872..f9eb87e3730 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -108,7 +108,8 @@ class L3AgentTestFramework(base.BaseSudoTestCase): extra_routes=True, enable_fip=True, enable_snat=True, num_internal_ports=1, - dual_stack=False, v6_ext_gw_with_sub=True, + dual_stack=False, enable_gw=True, + v6_ext_gw_with_sub=True, enable_pf_floating_ip=False, qos_policy_id=None): if ip_version == constants.IP_VERSION_6 and not dual_stack: @@ -124,6 +125,7 @@ class L3AgentTestFramework(base.BaseSudoTestCase): enable_ha=enable_ha, extra_routes=extra_routes, dual_stack=dual_stack, + enable_gw=enable_gw, v6_ext_gw_with_sub=( v6_ext_gw_with_sub), enable_pf_floating_ip=( diff --git a/neutron/tests/functional/agent/l3/test_ha_router.py b/neutron/tests/functional/agent/l3/test_ha_router.py index ba2a90dc8f2..df06e5a1a74 100644 --- a/neutron/tests/functional/agent/l3/test_ha_router.py +++ b/neutron/tests/functional/agent/l3/test_ha_router.py @@ -118,8 +118,11 @@ class L3HATestCase(framework.L3AgentTestFramework): router_info['gw_port'] = ex_port router.process() self._assert_ipv6_accept_ra(router, expected_ra) + # As router is going first to master and than to backup mode, + # ipv6_forwarding should be enabled on "all" interface always after + # that transition self._assert_ipv6_forwarding(router, expected_forwarding, - expected_forwarding) + True) @testtools.skipUnless(ipv6_utils.is_enabled_and_bind_by_default(), "IPv6 is not enabled") @@ -363,6 +366,24 @@ class L3HATestCase(framework.L3AgentTestFramework): self._wait_until_ipv6_forwarding_has_state( router.ns_name, external_device_name, 1) + @testtools.skipUnless(ipv6_utils.is_enabled_and_bind_by_default(), + "IPv6 is not enabled") + def test_ha_router_without_gw_ipv6_forwarding_state(self): + router_info = self.generate_router_info( + enable_ha=True, enable_gw=False) + router_info[constants.HA_INTERFACE_KEY]['status'] = ( + constants.PORT_STATUS_DOWN) + router = self.manage_router(self.agent, router_info) + + common_utils.wait_until_true(lambda: router.ha_state == 'backup') + self._wait_until_ipv6_forwarding_has_state(router.ns_name, 'all', 0) + + router.router[constants.HA_INTERFACE_KEY]['status'] = ( + constants.PORT_STATUS_ACTIVE) + self.agent._process_updated_router(router.router) + common_utils.wait_until_true(lambda: router.ha_state == 'master') + self._wait_until_ipv6_forwarding_has_state(router.ns_name, 'all', 1) + class L3HATestFailover(framework.L3AgentTestFramework): @@ -405,7 +426,7 @@ class L3HATestFailover(framework.L3AgentTestFramework): self._assert_ipv6_forwarding(new_master, True, True) self._assert_ipv6_accept_ra(new_slave, False) # after transition from master -> slave, 'all' IPv6 forwarding should - # still be enabled + # be enabled self._assert_ipv6_forwarding(new_slave, False, True) def test_ha_router_lost_gw_connection(self): diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index effe33a77fe..a698e85dd86 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -270,29 +270,45 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): spawn_metadata_proxy.assert_called() destroy_metadata_proxy.assert_not_called() - def _test__configure_ipv6_params_on_ext_gw_port_if_necessary_helper( - self, state, enable_expected): + def _test__configure_ipv6_params_helper(self, state, gw_port_id): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) router_info = l3router.RouterInfo(agent, _uuid(), {}, **self.ri_kwargs) - router_info.ex_gw_port = {'id': _uuid()} - with mock.patch.object(router_info, '_configure_ipv6_params_on_gw' - ) as mock_configure_ipv6: - agent._configure_ipv6_params_on_ext_gw_port_if_necessary( - router_info, state) - interface_name = router_info.get_external_device_name( - router_info.ex_gw_port['id']) + if gw_port_id: + router_info.ex_gw_port = {'id': gw_port_id} + expected_forwarding_state = state == 'master' + with mock.patch.object( + router_info.driver, "configure_ipv6_forwarding" + ) as configure_ipv6_forwarding, mock.patch.object( + router_info, "_configure_ipv6_params_on_gw" + ) as configure_ipv6_on_gw: + agent._configure_ipv6_params(router_info, state) - mock_configure_ipv6.assert_called_once_with( - router_info.ex_gw_port, router_info.ns_name, interface_name, - enable_expected) + if state == 'master': + configure_ipv6_forwarding.assert_called_once_with( + router_info.ns_name, 'all', expected_forwarding_state) + else: + configure_ipv6_forwarding.assert_not_called() - def test__configure_ipv6_params_on_ext_gw_port_if_necessary_master(self): - self._test__configure_ipv6_params_on_ext_gw_port_if_necessary_helper( - 'master', True) + if gw_port_id: + interface_name = router_info.get_external_device_name( + router_info.ex_gw_port['id']) + configure_ipv6_on_gw.assert_called_once_with( + router_info.ex_gw_port, router_info.ns_name, + interface_name, expected_forwarding_state) + else: + configure_ipv6_on_gw.assert_not_called() - def test__configure_ipv6_params_on_ext_gw_port_if_necessary_backup(self): - self._test__configure_ipv6_params_on_ext_gw_port_if_necessary_helper( - 'backup', False) + def test__configure_ipv6_params_master(self): + self._test__configure_ipv6_params_helper('master', gw_port_id=_uuid()) + + def test__configure_ipv6_params_backup(self): + self._test__configure_ipv6_params_helper('backup', gw_port_id=_uuid()) + + def test__configure_ipv6_params_master_no_gw_port(self): + self._test__configure_ipv6_params_helper('master', gw_port_id=None) + + def test__configure_ipv6_params_backup_no_gw_port(self): + self._test__configure_ipv6_params_helper('backup', gw_port_id=None) def test_check_ha_state_for_router_master_standby(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)