Merge "Ensure fip ip rules deleted when fip removed" into stable/ussuri

neutron/agent/l3/dvr_fip_ns.py

@@ -101,6 +101,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

@@ -47,6 +47,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'])
@@ -160,7 +181,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

@@ -1212,8 +1212,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

@@ -154,7 +154,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
@@ -222,6 +224,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'