From 31f97b5f98ab56d93ea7d6a11d3d79c2d7722a50 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 27 Sep 2021 16:22:45 +0000 Subject: [PATCH] [DVR] Check if SNAT iptables manager is initialized Check if SNAT iptables manager is initialized before processing the IP NAT rules. If the router never had an external GW port, the DVR GW in the SNAT namespace has not been created and the SNAT iptables manager has not been initialized. In this case, the IP NAT rules for centralized FIPs (to be applied on the SNAT namespace) cannot be set. Conflicts: neutron/tests/functional/agent/l3/framework.py Closes-Bug: #1945215 Change-Id: I426602514805d728f8cd78e42f2b0979b2101089 (cherry picked from commit f18edfdf450179f6bc8a47f3b143f2701bd93e0e) (cherry picked from commit b9143c37e06567b1ce50f9e858c9289984bb8e4b) --- neutron/agent/l3/dvr_edge_router.py | 5 +- .../tests/functional/agent/l3/framework.py | 38 +++++---- .../functional/agent/l3/test_dvr_router.py | 83 ++++++++++++++----- 3 files changed, 90 insertions(+), 36 deletions(-) diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py index d8ba684b40b..8a1dc7955ba 100644 --- a/neutron/agent/l3/dvr_edge_router.py +++ b/neutron/agent/l3/dvr_edge_router.py @@ -375,7 +375,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 90481e47fb4..968e53dd508 100644 --- a/neutron/tests/functional/agent/l3/framework.py +++ b/neutron/tests/functional/agent/l3/framework.py @@ -522,19 +522,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 @@ -561,7 +564,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.get_routing_table(4, namespace=namespace) @@ -569,10 +572,11 @@ class L3AgentTestFramework(base.BaseSudoTestCase): 'destination': route['destination']} 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: @@ -580,7 +584,13 @@ class L3AgentTestFramework(base.BaseSudoTestCase): namespace=ns_name) routes.extend(_routes) routes = set(route['destination'] 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 1d2dd1e436e..a2477c9792d 100644 --- a/neutron/tests/functional/agent/l3/test_dvr_router.py +++ b/neutron/tests/functional/agent/l3/test_dvr_router.py @@ -216,6 +216,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] @@ -542,7 +551,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. @@ -558,12 +568,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 @@ -581,7 +592,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, @@ -608,13 +622,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, @@ -624,8 +640,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 @@ -634,7 +651,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) @@ -667,8 +685,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) @@ -695,12 +718,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 = ( @@ -709,10 +732,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, @@ -737,7 +765,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 @@ -745,6 +774,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) @@ -1026,7 +1058,11 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework): self.assertNotEqual([], neighbor) self.assertEqual(expected_neighbor, neighbor[0]['dst']) - 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) @@ -2022,7 +2058,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()