diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index 62cbe6c7cdb..03cdfda096e 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.py @@ -398,7 +398,10 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter): def process_floating_ip_nat_rules(self): if self._is_this_snat_host(): - self.process_floating_ip_nat_rules_for_centralized_floatingip() + if not self.snat_iptables_manager: + LOG.debug("DVR router: no snat rules to be handled") + else: + self.process_floating_ip_nat_rules_for_centralized_floatingip() # Cover mixed dvr_snat and compute node, aka a dvr_snat node has both # centralized and distributed floating IPs. diff --git a/neutron/tests/functional/agent/l3/framework.py b/neutron/tests/functional/agent/l3/framework.py index 5de8313bf4b..3d9ba2bf3e9 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -524,19 +524,22 @@ class L3AgentTestFramework(base.BaseSudoTestCase): msg = 'PID file %s is not present.' % pid_file self.fail(msg) - def _assert_snat_chains(self, router): - self.assertFalse(router.iptables_manager.is_chain_empty( - 'nat', 'snat')) - self.assertFalse(router.iptables_manager.is_chain_empty( - 'nat', 'POSTROUTING')) + def _assert_snat_chains(self, router, enable_gw=True): + check = self.assertFalse if enable_gw else self.assertTrue + check(router.iptables_manager.is_chain_empty('nat', 'snat')) + check(router.iptables_manager.is_chain_empty('nat', 'POSTROUTING')) - def _assert_floating_ip_chains(self, router, snat_bound_fip=False): + def _assert_floating_ip_chains(self, router, snat_bound_fip=False, + enable_gw=True): if snat_bound_fip: - self.assertFalse(router.snat_iptables_manager.is_chain_empty( - 'nat', 'float-snat')) + if enable_gw: + self.assertFalse(router.snat_iptables_manager.is_chain_empty( + 'nat', 'float-snat')) + else: + self.assertIsNone(router.snat_iptables_manager) - self.assertFalse(router.iptables_manager.is_chain_empty( - 'nat', 'float-snat')) + check = self.assertFalse if enable_gw else self.assertTrue + check(router.iptables_manager.is_chain_empty('nat', 'float-snat')) def _assert_iptables_rules_converged(self, router): # if your code is failing on this line, it means you are not generating @@ -563,7 +566,7 @@ 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, namespace=None): + def _assert_extra_routes(self, router, namespace=None, enable_gw=True): if namespace is None: namespace = router.ns_name routes = ip_lib.list_ip_routes(namespace, constants.IP_VERSION_4) @@ -571,17 +574,24 @@ class L3AgentTestFramework(base.BaseSudoTestCase): 'destination': route['cidr']} for route in routes] for extra_route in router.router['routes']: - self.assertIn(extra_route, routes) + check = self.assertIn if enable_gw else self.assertNotIn + check(extra_route, routes) def _assert_onlink_subnet_routes( - self, router, ip_versions, namespace=None): + self, router, ip_versions, namespace=None, enable_gw=True): ns_name = namespace or router.ns_name routes = [] for ip_version in ip_versions: _routes = ip_lib.list_ip_routes(ns_name, ip_version) routes.extend(_routes) routes = set(route['cidr'] for route in routes) - extra_subnets = router.get_ex_gw_port()['extra_subnets'] + ex_gw_port = router.get_ex_gw_port() + if not ex_gw_port: + if not enable_gw: + return + self.fail('GW port is enabled but not present in the router') + + extra_subnets = ex_gw_port['extra_subnets'] for extra_subnet in (route['cidr'] for route in extra_subnets): self.assertIn(extra_subnet, routes) diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py index 7c121e8d9df..f1c3a88f88a 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -215,6 +215,15 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): self._dvr_router_lifecycle(enable_ha=True, enable_snat=True, snat_bound_fip=True) + def test_dvr_lifecycle_no_ha_with_snat_with_fips_with_cent_fips_no_gw( + self): + self._dvr_router_lifecycle(enable_ha=False, enable_snat=True, + snat_bound_fip=True, enable_gw=False) + + def test_dvr_lifecycle_ha_with_snat_with_fips_with_cent_fips_no_gw(self): + self._dvr_router_lifecycle(enable_ha=True, enable_snat=True, + snat_bound_fip=True, enable_gw=False) + def _check_routes(self, expected_routes, actual_routes): actual_routes = [{key: route[key] for key in expected_routes[0].keys()} for route in actual_routes] @@ -541,7 +550,8 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): custom_mtu=2000, ip_version=lib_constants.IP_VERSION_4, dual_stack=False, - snat_bound_fip=False): + snat_bound_fip=False, + enable_gw=True): '''Test dvr router lifecycle :param enable_ha: sets the ha value for the router. @@ -557,12 +567,13 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): # We get the router info particular to a dvr router router_info = self.generate_dvr_router_info( enable_ha, enable_snat, extra_routes=True, - snat_bound_fip=snat_bound_fip) + snat_bound_fip=snat_bound_fip, enable_gw=enable_gw) for key in ('_interfaces', '_snat_router_interfaces', '_floatingip_agent_interfaces'): - for port in router_info[key]: + for port in router_info.get(key, []): port['mtu'] = custom_mtu - router_info['gw_port']['mtu'] = custom_mtu + if router_info['gw_port']: + router_info['gw_port']['mtu'] = custom_mtu if enable_ha: router_info['_ha_interface']['mtu'] = custom_mtu @@ -580,7 +591,10 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): # manage the router (create it, create namespaces, # attach interfaces, etc...) router = self.manage_router(self.agent, router_info) - if enable_ha: + if enable_ha and not enable_gw: + port = router.get_ex_gw_port() + self.assertEqual({}, port) + elif enable_ha and enable_gw: port = router.get_ex_gw_port() interface_name = router.get_external_device_name(port['id']) self._assert_no_ip_addresses_on_interface(router.ha_namespace, @@ -607,13 +621,15 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): utils.wait_until_true( lambda: self._metadata_proxy_exists(self.agent.conf, router)) self._assert_internal_devices(router) - self._assert_dvr_external_device(router) - self._assert_dvr_gateway(router) - self._assert_dvr_floating_ips(router, snat_bound_fip=snat_bound_fip) - self._assert_snat_chains(router) - self._assert_floating_ip_chains(router, snat_bound_fip=snat_bound_fip) + self._assert_dvr_external_device(router, enable_gw) + self._assert_dvr_gateway(router, enable_gw) + self._assert_dvr_floating_ips(router, snat_bound_fip=snat_bound_fip, + enable_gw=enable_gw) + self._assert_snat_chains(router, enable_gw=enable_gw) + self._assert_floating_ip_chains(router, snat_bound_fip=snat_bound_fip, + enable_gw=enable_gw) self._assert_metadata_chains(router) - self._assert_rfp_fpr_mtu(router, custom_mtu) + self._assert_rfp_fpr_mtu(router, custom_mtu, enable_gw=enable_gw) if enable_snat: if (ip_version == lib_constants.IP_VERSION_6 or dual_stack): ip_versions = [lib_constants.IP_VERSION_4, @@ -623,8 +639,9 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): snat_ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name( router.router_id) self._assert_onlink_subnet_routes( - router, ip_versions, snat_ns_name) - self._assert_extra_routes(router, namespace=snat_ns_name) + router, ip_versions, snat_ns_name, enable_gw=enable_gw) + self._assert_extra_routes(router, namespace=snat_ns_name, + enable_gw=enable_gw) # During normal operation, a router-gateway-clear followed by # a router delete results in two notifications to the agent. This @@ -633,7 +650,8 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): # that the L3 agent is robust enough to handle that case and delete # the router correctly. self._delete_router(self.agent, router.router_id) - self._assert_fip_namespace_deleted(ext_gateway_port) + self._assert_fip_namespace_deleted(ext_gateway_port, + enable_gw=enable_gw) self._assert_router_does_not_exist(router) self._assert_snat_namespace_does_not_exist(router) @@ -666,8 +684,13 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): } return fip_agent_gw_port_info - def _assert_dvr_external_device(self, router): + def _assert_dvr_external_device(self, router, enable_gw): external_port = router.get_ex_gw_port() + if not external_port: + if not enable_gw: + return + self.fail('GW port is enabled but not present in the router') + snat_ns_name = dvr_snat_ns.SnatNamespace.get_snat_ns_name( router.router_id) @@ -694,12 +717,12 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): else: self.fail("Agent not configured for dvr or dvr_snat") - def _assert_dvr_gateway(self, router): + def _assert_dvr_gateway(self, router, enable_gw): gateway_expected_in_snat_namespace = ( self.agent.conf.agent_mode == 'dvr_snat' ) if gateway_expected_in_snat_namespace: - self._assert_dvr_snat_gateway(router) + self._assert_dvr_snat_gateway(router, enable_gw=enable_gw) self._assert_removal_of_already_deleted_gateway_device(router) snat_namespace_should_not_exist = ( @@ -708,10 +731,15 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): if snat_namespace_should_not_exist: self._assert_snat_namespace_does_not_exist(router) - def _assert_dvr_snat_gateway(self, router): + def _assert_dvr_snat_gateway(self, router, enable_gw=True): namespace = dvr_snat_ns.SnatNamespace.get_snat_ns_name( router.router_id) external_port = router.get_ex_gw_port() + if not external_port: + if not enable_gw: + return + self.fail('GW port is enabled but not present in the router') + external_device_name = router.get_external_device_name( external_port['id']) external_device = ip_lib.IPDevice(external_device_name, @@ -736,7 +764,8 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): router.router_id) self.assertFalse(self._namespace_exists(namespace)) - def _assert_dvr_floating_ips(self, router, snat_bound_fip=False): + def _assert_dvr_floating_ips(self, router, snat_bound_fip=False, + enable_gw=True): # in the fip namespace: # Check that the fg- (floatingip_agent_gateway) # is created with the ip address of the external gateway port @@ -744,6 +773,9 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): self.assertTrue(floating_ips) # We need to fetch the floatingip agent gateway port info # from the router_info + if not enable_gw: + return + floating_agent_gw_port = ( router.router[lib_constants.FLOATINGIP_AGENT_INTF_KEY]) self.assertTrue(floating_agent_gw_port) @@ -1037,7 +1069,11 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): dst=str(not_expected_neighbor)) self.assertEqual([], neighbor) - def _assert_rfp_fpr_mtu(self, router, expected_mtu=1500): + def _assert_rfp_fpr_mtu(self, router, expected_mtu=1500, enable_gw=True): + if not enable_gw: + self.assertIsNone(router.fip_ns) + return + dev_mtu = self.get_device_mtu( router.router_id, router.fip_ns.get_rtr_ext_device_name, router.ns_name) @@ -2040,7 +2076,12 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): self.assertIsNone(fg_device.route.get_gateway()) def _assert_fip_namespace_deleted(self, ext_gateway_port, - assert_ovs_interface=True): + assert_ovs_interface=True, + enable_gw=True): + if not enable_gw: + self.assertEqual({}, ext_gateway_port) + return + ext_net_id = ext_gateway_port['network_id'] fip_ns = self.agent.get_fip_ns(ext_net_id) fip_ns.unsubscribe = mock.Mock()