From 393d484e07e53496ff068d14047c97d28febd03a Mon Sep 17 00:00:00 2001
From: Hemanth Nakkina <hemanth.nakkina@canonical.com>
Date: Fri, 12 Feb 2021 17:44:38 +0530
Subject: [PATCH] Fix deletion of rfp interfaces when router is re-enabled

1. When dvr router is disabled and enabled back, the rfp
interfaces are deleted on the nodes without snat-* namespace.
This is due to creation of snat namespace during router
initialization stage on all the nodes. At later stages, since
the gw_port_host is not bounded on this node, external gateway
is removed which triggers removal of rfp interfaces and snat
namespace.
Create snat namespace only on the nodes where gw_port_host is
bounded.

2. In case of DVR SNAT, when the l3 agent is rescheduled to
another node, the rfp interfaces on qrouter-* namespace are
removed. Instead of calling external_gateway_removed() which
further deletes the rfp interfaces, the qg-, sg- interfaces
need to be unplugged and snat namespace need to be deleted.

Closes-Bug: #1894843
Change-Id: Ic35c2f9bceacec8eeba67a2b1ea0cd0b0ffc72fe
---
 neutron/agent/l3/dvr_edge_router.py           | 10 ++--
 .../functional/agent/l3/test_dvr_router.py    | 48 +++++++++++++++++++
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py
index 6a92f0a0e68..29a5a575e4d 100644
--- a/neutron/agent/l3/dvr_edge_router.py
+++ b/neutron/agent/l3/dvr_edge_router.py
@@ -71,8 +71,10 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
             if self.snat_namespace.exists():
                 LOG.debug("SNAT was rescheduled to host %s. Clearing snat "
                           "namespace.", self.router.get('gw_port_host'))
-                return self.external_gateway_removed(
-                    ex_gw_port, interface_name)
+                self.driver.unplug(interface_name,
+                                   namespace=self.snat_namespace.name,
+                                   prefix=router.EXTERNAL_DEV_PREFIX)
+                self.snat_namespace.delete()
             return
 
         if not self.snat_namespace.exists():
@@ -185,8 +187,8 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
         # TODO(mlavalle): in the near future, this method should contain the
         # code in the L3 agent that creates a gateway for a dvr. The first step
         # is to move the creation of the snat namespace here
-        self.snat_namespace.create()
-        return self.snat_namespace
+        if self._is_this_snat_host():
+            self.snat_namespace.create()
 
     def _get_snat_int_device_name(self, port_id):
         long_name = lib_constants.SNAT_INT_DEV_PREFIX + port_id
diff --git a/neutron/tests/functional/agent/l3/test_dvr_router.py b/neutron/tests/functional/agent/l3/test_dvr_router.py
index d6593668538..f3dab467990 100644
--- a/neutron/tests/functional/agent/l3/test_dvr_router.py
+++ b/neutron/tests/functional/agent/l3/test_dvr_router.py
@@ -781,6 +781,54 @@ class TestDvrRouter(DvrRouterTestFramework, framework.L3AgentTestFramework):
             self._assert_iptables_rules_exist(
                 iptables_mgr, 'nat', expected_rules)
 
+    def test_dvr_router_fip_associations_exist_when_router_reenabled(self):
+        """Test to validate the fip associations when router is re-enabled.
+
+        This test validates the fip associations when the router is disabled
+        and enabled back again. This test is specifically for the host where
+        snat namespace is not created or gateway port is binded on other host.
+        """
+        self.agent.conf.agent_mode = 'dvr_snat'
+        router_info = self.generate_dvr_router_info(enable_snat=True)
+        # Ensure agent does not create snat namespace by changing gw_port_host
+        router_info['gw_port_host'] = 'agent2'
+        router_info_copy = copy.deepcopy(router_info)
+        router1 = self.manage_router(self.agent, router_info)
+
+        fip_ns_name = router1.fip_ns.name
+        self.assertTrue(self._namespace_exists(router1.fip_ns.name))
+
+        # Simulate disable router
+        self.agent._safe_router_removed(router1.router['id'])
+        self.assertFalse(self._namespace_exists(router1.ns_name))
+        self.assertTrue(self._namespace_exists(fip_ns_name))
+
+        # Simulated enable router
+        router_updated = self.manage_router(self.agent, router_info_copy)
+        self._assert_dvr_floating_ips(router_updated)
+
+    def test_dvr_router_fip_associations_exist_when_snat_removed(self):
+        """Test to validate the fip associations when snat is removed.
+
+        This test validates the fip associations when the snat is removed from
+        the agent. The fip associations should exist when the snat is moved to
+        another l3 agent.
+        """
+        self.agent.conf.agent_mode = 'dvr_snat'
+        router_info = self.generate_dvr_router_info(enable_snat=True)
+        router_info_copy = copy.deepcopy(router_info)
+        router1 = self.manage_router(self.agent, router_info)
+
+        # Remove gateway port host and the binding host_id to simulate
+        # removal of snat from l3 agent
+        router_info_copy['gw_port_host'] = ''
+        router_info_copy['gw_port']['binding:host_id'] = ''
+        router_info_copy['gw_port']['binding:vif_type'] = 'unbound'
+        router_info_copy['gw_port']['binding:vif_details'] = {}
+        self.agent._process_updated_router(router_info_copy)
+        router_updated = self.agent.router_info[router1.router['id']]
+        self._assert_dvr_floating_ips(router_updated)
+
     def test_dvr_router_with_ha_for_fip_disassociation(self):
         """Test to validate the fip rules are deleted in dvr_snat_ha router.