DVR: SNAT redirect rules should be removed only on Gateway clear

SNAT redirect rules should be removed only on gateway clear and
not on gateway move or gateway reschedule to a different agent.

This would make the snat node unreachable by the dvr service
ports.

Closes-Bug: #1611964
(cherry picked from commit 82c134d6e06fb47ffc923880ed597bddb7a06013)

Conflicts:
	neutron/agent/l3/dvr_local_router.py
	neutron/tests/functional/agent/l3/test_dvr_router.py

Change-Id: Icec7eabfd06825bf3b4c48f06cf5b88e0a5af06d

3 files changed, 42 insertions, 6 deletions

diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py
index a067bb436a..85622d1556 100644
--- a/neutron/agent/l3/dvr_local_router.py
+++ b/neutron/agent/l3/dvr_local_router.py
@@ -424,12 +424,17 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase):
             to_fip_interface_name = (
                 self.get_external_device_interface_name(ex_gw_port))
             self.process_floating_ip_addresses(to_fip_interface_name)
-        for p in self.internal_ports:
-            # NOTE: When removing the gateway port, pass in the snat_port
-            # cache along with the current ports.
-            gateway = self.get_snat_port_for_internal_port(p, self.snat_ports)
-            internal_interface = self.get_internal_device_name(p['id'])
-            self._snat_redirect_remove(gateway, p, internal_interface)
+        # NOTE:_snat_redirect_remove should be only called when the
+        # gateway is cleared and should not be called when the gateway
+        # is moved or rescheduled.
+        if not self.router.get('gw_port'):
+            for p in self.internal_ports:
+                # NOTE: When removing the gateway port, pass in the snat_port
+                # cache along with the current ports.
+                gateway = self.get_snat_port_for_internal_port(
+                    p, self.snat_ports)
+                internal_interface = self.get_internal_device_name(p['id'])
+                self._snat_redirect_remove(gateway, p, internal_interface)
 
     def _handle_router_snat_rules(self, ex_gw_port, interface_name):
         pass
diff --git a/neutron/tests/functional/agent/test_l3_agent.py b/neutron/tests/functional/agent/test_l3_agent.py
index 879645fe38..38de8be39c 100644
--- a/neutron/tests/functional/agent/test_l3_agent.py
+++ b/neutron/tests/functional/agent/test_l3_agent.py
@@ -1549,6 +1549,34 @@ class TestDvrRouter(L3AgentTestFramework):
         self.assertFalse(self._fixed_ip_rule_exists(router_ns, fixed_ip))
         self.assertTrue(self._fixed_ip_rule_exists(router_ns, new_fixed_ip))
 
+    def test_dvr_gateway_move_does_not_remove_redirect_rules(self):
+        """Test to validate snat redirect rules not cleared with snat move."""
+        self.agent.conf.agent_mode = 'dvr_snat'
+        router_info = self.generate_dvr_router_info(enable_snat=True)
+        router_info[l3_constants.FLOATINGIP_KEY] = []
+        router_info[l3_constants.FLOATINGIP_AGENT_INTF_KEY] = []
+        router1 = self.manage_router(self.agent, router_info)
+        router1.router['gw_port_host'] = ""
+        self.agent._process_updated_router(router1.router)
+        router_updated = self.agent.router_info[router1.router['id']]
+        self.assertTrue(self._namespace_exists(router_updated.ns_name))
+        ns_ipr = ip_lib.IPRule(namespace=router1.ns_name)
+        ip4_rules_list = ns_ipr.rule.list_rules(l3_constants.IP_VERSION_4)
+        self.assertEqual(4, len(ip4_rules_list))
+        # IPRule list should have 4 entries.
+        # Three entries from 'default', 'main' and 'local' table.
+        # The remaining 1 is for the router interfaces(csnat port).
+        default_rules_list_count = 0
+        interface_rules_list_count = 0
+        for ip_rule in ip4_rules_list:
+            tbl_index = ip_rule['table']
+            if tbl_index in ['local', 'default', 'main']:
+                default_rules_list_count = default_rules_list_count + 1
+            else:
+                interface_rules_list_count = interface_rules_list_count + 1
+        self.assertEqual(3, default_rules_list_count)
+        self.assertEqual(1, interface_rules_list_count)
+
     def _get_fixed_ip_rule_priority(self, namespace, fip):
         iprule = ip_lib.IPRule(namespace)
         lines = iprule.rule._as_root([4], ['show']).splitlines()
diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py
index 17f05e2e82..ca8bfb2685 100644
--- a/neutron/tests/unit/agent/l3/test_agent.py
+++ b/neutron/tests/unit/agent/l3/test_agent.py
@@ -558,6 +558,7 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
             ri.get_snat_port_for_internal_port = mock.Mock(
                 return_value=sn_port)
             ri._snat_redirect_remove = mock.Mock()
+            ri.router['gw_port'] = ""
             ri.external_gateway_removed(ex_gw_port, interface_name)
             if not router.get('distributed'):
                 self.mock_driver.unplug.assert_called_once_with(
@@ -647,8 +648,10 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
             interface_name, ex_gw_port = l3_test_common.prepare_ext_gw_test(
                 self, ri)
             router['gw_port_host'] = ''
+            ri._snat_redirect_remove = mock.Mock()
             ri.external_gateway_updated(ex_gw_port, interface_name)
             if router['gw_port_host'] != ri.host:
+                self.assertFalse(ri._snat_redirect_remove.called)
                 self.assertEqual(1, snat_ns_delete.call_count)
 
     @mock.patch.object(namespaces.Namespace, 'delete')