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 5eca44bfa8)
(cherry picked from commit 8ba796ea7f)
(cherry picked from commit 84d38f342b)

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)
 

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'])
@@ -161,7 +182,11 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
                                   table=dvr_fip_ns.FIP_RT_TBL,
                                   priority=int(str(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."""

neutron/tests/unit/agent/l3/test_agent.py

@@ -1148,8 +1148,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=lib_constants.IP_VERSION_4, enable_snat=True,
             num_internal_ports=1)

neutron/tests/unit/agent/l3/test_dvr_local_router.py

@@ -155,7 +155,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
@@ -223,6 +225,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},

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'