From f63e3595c4e2aede7168fb1e65dbe7190308adce Mon Sep 17 00:00:00 2001 From: Edward Hope-Morley Date: Fri, 14 Aug 2020 17:44:54 +0100 Subject: [PATCH] Ensure fip ip rules deleted when fip removed The information needed to delete ip rules associated with fips is held in memory between add and remove so a restart of the l3-agent results in any fips that existed before the restart having their ip rules persist after the fips are removed. This patch enures that an agent restart reloads this information so that ip rules associated with a fip are correctly removed when the fip is removed. Change-Id: If656a703c996ccc7719b1b09d793c5bbdfd6f3c1 Closes-Bug: #1891673 (cherry picked from commit 5eca44bfa850e6e75c9974ae7711b87764628253) (cherry picked from commit 8ba796ea7ff28a815996ffeaf3c4dc39df1edcfb) (cherry picked from commit 84d38f342bcad6537971d732a4961334a5890f3b) (cherry picked from commit f28788f77798e6e1e64ac9f60a82b99b52546f8f) (cherry picked from commit 1eb5b54776d2194319528712399439c54c5320d7) --- neutron/agent/l3/dvr_fip_ns.py | 3 ++ neutron/agent/l3/dvr_local_router.py | 27 +++++++++++++++- neutron/tests/unit/agent/l3/test_agent.py | 5 ++- .../unit/agent/l3/test_dvr_local_router.py | 31 ++++++++++++++++++- neutron/tests/unit/agent/linux/test_pd.py | 8 +++-- 5 files changed, 69 insertions(+), 5 deletions(-) diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index 037d2691a3d..9d66823d001 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -102,6 +102,9 @@ class FipNamespace(namespaces.Namespace): self._subscribers.discard(external_net_id) return not self.has_subscribers() + def lookup_rule_priority(self, floating_ip): + return self._rule_priorities.lookup(floating_ip) + def allocate_rule_priority(self, floating_ip): return self._rule_priorities.allocate(floating_ip) diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index 477256d188f..d131f6c26ad 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -48,6 +48,27 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): self.fip_ns = None self._pending_arp_set = set() + self.load_used_fip_information() + + def load_used_fip_information(self): + """Some information needed to remove a floating ip e.g. it's associated + ip rule priorities, are stored in memory to avoid extra db lookups. + Since this is lost on agent restart we need to reload them. + """ + ex_gw_port = self.get_ex_gw_port() + if ex_gw_port: + fip_ns = self.agent.get_fip_ns(ex_gw_port['network_id']) + + for fip in self.get_floating_ips(): + floating_ip = fip['floating_ip_address'] + fixed_ip = fip['fixed_ip_address'] + rule_pr = fip_ns.lookup_rule_priority(floating_ip) + if rule_pr: + self.floating_ips_dict[floating_ip] = (fixed_ip, rule_pr) + else: + LOG.error("Rule priority not found for floating ip %s", + floating_ip) + def migrate_centralized_floating_ip(self, fip, interface_name, device): # Remove the centralized fip first and then add fip to the host ip_cidr = common_utils.ip_to_cidr(fip['floating_ip_address']) @@ -163,7 +184,11 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): table=dvr_fip_ns.FIP_RT_TBL, priority=rule_pr) self.fip_ns.deallocate_rule_priority(floating_ip) - #TODO(rajeev): Handle else case - exception/log? + else: + LOG.error("Unable to find necessary information to complete " + "removal of floating ip rules for %s - will require " + "manual cleanup (see LP 1891673 for details).", + floating_ip) def floating_ip_removed_dist(self, fip_cidr): """Remove floating IP from FIP namespace.""" diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py index 1519b206d70..fa8ee2d0f3f 100644 --- a/neutron/tests/unit/agent/l3/test_agent.py +++ b/neutron/tests/unit/agent/l3/test_agent.py @@ -1150,8 +1150,11 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework): self.assertIsNone(res_ip) self.assertTrue(log_error.called) + @mock.patch.object(dvr_router.DvrEdgeRouter, 'load_used_fip_information') @mock.patch.object(dvr_router_base.LOG, 'error') - def test_get_snat_port_for_internal_port_ipv6_same_port(self, log_error): + def test_get_snat_port_for_internal_port_ipv6_same_port(self, + log_error, + load_used_fips): router = l3_test_common.prepare_router_data(ip_version=4, enable_snat=True, num_internal_ports=1) diff --git a/neutron/tests/unit/agent/l3/test_dvr_local_router.py b/neutron/tests/unit/agent/l3/test_dvr_local_router.py index 34d37e8e8ca..e1e807e84e7 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -156,7 +156,9 @@ class TestDvrRouterOperations(base.BaseTestCase): kwargs['router'] = router kwargs['agent_conf'] = self.conf kwargs['interface_driver'] = mock.Mock() - return dvr_router.DvrLocalRouter(HOSTNAME, **kwargs) + with mock.patch.object(dvr_router.DvrLocalRouter, + 'load_used_fip_information'): + return dvr_router.DvrLocalRouter(HOSTNAME, **kwargs) def _set_ri_kwargs(self, agent, router_id, router): self.ri_kwargs['agent'] = agent @@ -224,6 +226,33 @@ class TestDvrRouterOperations(base.BaseTestCase): self.assertTrue( ri.fip_ns.create_rtr_2_fip_link.called) + def test_load_used_fip_information(self): + router = mock.MagicMock() + with mock.patch.object(dvr_router.DvrLocalRouter, + 'get_floating_ips') as mock_get_floating_ips: + with mock.patch.object(dvr_router.DvrLocalRouter, + 'get_ex_gw_port') as mock_ext_port: + mock_ext_port.return_value = {'network_id': _uuid()} + fip = {'id': _uuid(), + 'host': HOSTNAME, + 'floating_ip_address': '15.1.2.3', + 'fixed_ip_address': '192.168.0.1', + 'floating_network_id': _uuid(), + 'port_id': _uuid()} + fip_ns = mock.MagicMock() + fip_ns.lookup_rule_priority.return_value = 1234 + mock_get_floating_ips.return_value = [fip] + mock_agent = mock.MagicMock() + mock_agent.get_fip_ns.return_value = fip_ns + kwargs = {'agent': mock_agent, + 'router_id': _uuid(), + 'router': mock.Mock(), + 'agent_conf': self.conf, + 'interface_driver': mock.Mock()} + router = dvr_router.DvrLocalRouter(HOSTNAME, **kwargs) + self.assertEqual({'15.1.2.3': ('192.168.0.1', 1234)}, + router.floating_ips_dict) + def test_get_floating_ips_dvr(self): router = mock.MagicMock() router.get.return_value = [{'host': HOSTNAME}, diff --git a/neutron/tests/unit/agent/linux/test_pd.py b/neutron/tests/unit/agent/linux/test_pd.py index 525cca65e08..7ac8bb69035 100644 --- a/neutron/tests/unit/agent/linux/test_pd.py +++ b/neutron/tests/unit/agent/linux/test_pd.py @@ -46,7 +46,9 @@ class TestPrefixDelegation(tests_base.DietTestCase): pd_router = l3_agent.pd.routers.get(router.router_id) self.assertEqual(ns_name, pd_router.get('ns_name')) - def test_add_update_dvr_edge_router(self): + @mock.patch.object(dvr_edge_router.DvrEdgeRouter, + 'load_used_fip_information') + def test_add_update_dvr_edge_router(self, load_used_fip_info): l3_agent = mock.Mock() l3_agent.pd.routers = {} router_id = '1' @@ -59,7 +61,9 @@ class TestPrefixDelegation(tests_base.DietTestCase): ns_name = ri.snat_namespace.name self._test_add_update_pd(l3_agent, ri, ns_name) - def test_add_update_dvr_local_router(self): + @mock.patch.object(dvr_local_router.DvrLocalRouter, + 'load_used_fip_information') + def test_add_update_dvr_local_router(self, load_used_fip_info): l3_agent = mock.Mock() l3_agent.pd.routers = {} router_id = '1'