DVR: fix router rescheduling on agent side

The patch makes L3 agent aware of possible SNAT role
rescheduling to/from it.
The gist is to compare gw_port host change.
If it was changed and agent is not on target host then
it needs to clear snat namespace if one exists. If agent
is on target host it needs to create snat namespace from
scratch if it doesn't exist.

Host field was excluded from gw_port comparison on
agent side as part of HA Router feature implementation.
This code was moved to corresponding module.

Closes-Bug: #1472205
Change-Id: I840bded9eb547df014c6fb2b4cbfe4a876b9b878

diff --git a/neutron/agent/l3/dvr_edge_router.py b/neutron/agent/l3/dvr_edge_router.py
index b68af5cdecfbbf350e19a49dac2dafda3ec3f25f..ea8c7130920cbfb2887ea8b4b82079b674f57c72 100644 (file)
--- a/neutron/agent/l3/dvr_edge_router.py
+++ b/neutron/agent/l3/dvr_edge_router.py
@@ -40,17 +40,27 @@ class DvrEdgeRouter(dvr_local_router.DvrLocalRouter):
         if not self._is_this_snat_host():
             # no centralized SNAT gateway for this node/agent
             LOG.debug("not hosting snat for router: %s", self.router['id'])
+            if self.snat_namespace:
+                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)
             return
 
-        self._external_gateway_added(ex_gw_port,
-                                     interface_name,
-                                     self.snat_namespace.name,
-                                     preserve_ips=[])
+        if not self.snat_namespace:
+            # SNAT might be rescheduled to this agent; need to process like
+            # newly created gateway
+            return self.external_gateway_added(ex_gw_port, interface_name)
+        else:
+            self._external_gateway_added(ex_gw_port,
+                                        interface_name,
+                                        self.snat_namespace.name,
+                                        preserve_ips=[])
 
     def external_gateway_removed(self, ex_gw_port, interface_name):
         super(DvrEdgeRouter, self).external_gateway_removed(ex_gw_port,
                                                             interface_name)
-        if not self._is_this_snat_host():
+        if not self._is_this_snat_host() and not self.snat_namespace:
             # no centralized SNAT gateway for this node/agent
             LOG.debug("not hosting snat for router: %s", self.router['id'])
             return

diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py
index e7b7b5020af362481975052fe0e0cc8e5ba4c148..33d750d300d8db6aabf8f705014aa43dda4ab27e 100644 (file)
--- a/neutron/agent/l3/ha_router.py
+++ b/neutron/agent/l3/ha_router.py
@@ -333,6 +333,16 @@ class HaRouter(router.RouterInfo):
         self.ha_state = state
         callback(self.router_id, state)
 
+    @staticmethod
+    def _gateway_ports_equal(port1, port2):
+        def _get_filtered_dict(d, ignore):
+            return {k: v for k, v in d.items() if k not in ignore}
+
+        keys_to_ignore = set(['binding:host_id'])
+        port1_filtered = _get_filtered_dict(port1, keys_to_ignore)
+        port2_filtered = _get_filtered_dict(port2, keys_to_ignore)
+        return port1_filtered == port2_filtered
+
     def external_gateway_added(self, ex_gw_port, interface_name):
         self._plug_external_gateway(ex_gw_port, interface_name, self.ns_name)
         self._add_gateway_vip(ex_gw_port, interface_name)

diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py
index 8b25f0a6a33761d97f2921109db74a17f9c72875..81bca38775afdc1d5cfeef732d3276c54ff5e17e 100644 (file)
--- a/neutron/agent/l3/router_info.py
+++ b/neutron/agent/l3/router_info.py
@@ -15,7 +15,6 @@
 import netaddr
 
 from oslo_log import log as logging
-import six
 
 from neutron.agent.l3 import namespaces
 from neutron.agent.linux import ip_lib
@@ -479,6 +478,10 @@ class RouterInfo(object):
                            namespace=self.ns_name,
                            prefix=EXTERNAL_DEV_PREFIX)
 
+    @staticmethod
+    def _gateway_ports_equal(port1, port2):
+        return port1 == port2
+
     def _process_external_gateway(self, ex_gw_port):
         # TODO(Carl) Refactor to clarify roles of ex_gw_port vs self.ex_gw_port
         ex_gw_port_id = (ex_gw_port and ex_gw_port['id'] or
@@ -488,19 +491,9 @@ class RouterInfo(object):
         if ex_gw_port_id:
             interface_name = self.get_external_device_name(ex_gw_port_id)
         if ex_gw_port:
-            def _gateway_ports_equal(port1, port2):
-                def _get_filtered_dict(d, ignore):
-                    return dict((k, v) for k, v in six.iteritems(d)
-                                if k not in ignore)
-
-                keys_to_ignore = set(['binding:host_id'])
-                port1_filtered = _get_filtered_dict(port1, keys_to_ignore)
-                port2_filtered = _get_filtered_dict(port2, keys_to_ignore)
-                return port1_filtered == port2_filtered
-
             if not self.ex_gw_port:
                 self.external_gateway_added(ex_gw_port, interface_name)
-            elif not _gateway_ports_equal(ex_gw_port, self.ex_gw_port):
+            elif not self._gateway_ports_equal(ex_gw_port, self.ex_gw_port):
                 self.external_gateway_updated(ex_gw_port, interface_name)
         elif not ex_gw_port and self.ex_gw_port:
             self.external_gateway_removed(self.ex_gw_port, interface_name)

diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py
index b59c9cc632db933507571868b78cc575890946b9..b4921692d0f990416082dbc4ae49de93983ae851 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_agent.py
+++ b/neutron/tests/unit/agent/l3/test_agent.py
@@ -578,41 +578,60 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
     def test_external_gateway_updated_dual_stack(self):
         self._test_external_gateway_updated(dual_stack=True)
 
-    def _test_ext_gw_updated_dvr_agent_mode(self, host,
-                                            agent_mode, expected_call_count):
+    def _test_ext_gw_updated_dvr_edge_router(self, host_match,
+                                             snat_hosted_before=True):
+        """
+        Helper to test external gw update for edge router on dvr_snat agent
+
+        :param host_match: True if new gw host should be the same as agent host
+        :param snat_hosted_before: True if agent has already been hosting
+        snat for the router
+        """
         router = l3_test_common.prepare_router_data(num_internal_ports=2)
-        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
-        ri = dvr_router.DvrEdgeRouter(agent,
+        ri = dvr_router.DvrEdgeRouter(mock.Mock(),
                                       HOSTNAME,
                                       router['id'],
                                       router,
                                       **self.ri_kwargs)
-        ri.create_snat_namespace()
+        if snat_hosted_before:
+            ri.create_snat_namespace()
+            snat_ns_name = ri.snat_namespace.name
+        else:
+            self.assertIsNone(ri.snat_namespace)
+
         interface_name, ex_gw_port = l3_test_common.prepare_ext_gw_test(self,
                                                                         ri)
         ri._external_gateway_added = mock.Mock()
 
-        # test agent mode = dvr (compute node)
-        router['gw_port_host'] = host
-        agent.conf.agent_mode = agent_mode
+        router['gw_port_host'] = ri.host if host_match else (ri.host + 'foo')
 
         ri.external_gateway_updated(ex_gw_port, interface_name)
-        # no gateway should be added on dvr node
-        self.assertEqual(expected_call_count,
-                         ri._external_gateway_added.call_count)
-
-    def test_ext_gw_updated_dvr_agent_mode(self):
-        # no gateway should be added on dvr node
-        self._test_ext_gw_updated_dvr_agent_mode('any-foo', 'dvr', 0)
-
-    def test_ext_gw_updated_dvr_snat_agent_mode_no_host(self):
-        # no gateway should be added on dvr_snat node without host match
-        self._test_ext_gw_updated_dvr_agent_mode('any-foo', 'dvr_snat', 0)
-
-    def test_ext_gw_updated_dvr_snat_agent_mode_host(self):
-        # gateway should be added on dvr_snat node
-        self._test_ext_gw_updated_dvr_agent_mode(HOSTNAME,
-                                                 'dvr_snat', 1)
+        if not host_match:
+            self.assertFalse(ri._external_gateway_added.called)
+            if snat_hosted_before:
+                # host mismatch means that snat was rescheduled to another
+                # agent, hence need to verify that gw port was unplugged and
+                # snat namespace was deleted
+                self.mock_driver.unplug.assert_called_with(
+                    interface_name,
+                    bridge=self.conf.external_network_bridge,
+                    namespace=snat_ns_name,
+                    prefix=l3_agent.EXTERNAL_DEV_PREFIX)
+                self.assertIsNone(ri.snat_namespace)
+        else:
+            if not snat_hosted_before:
+                self.assertIsNotNone(ri.snat_namespace)
+            self.assertTrue(ri._external_gateway_added.called)
+
+    def test_ext_gw_updated_dvr_edge_router(self):
+        self._test_ext_gw_updated_dvr_edge_router(host_match=True)
+
+    def test_ext_gw_updated_dvr_edge_router_host_mismatch(self):
+        self._test_ext_gw_updated_dvr_edge_router(host_match=False)
+
+    def test_ext_gw_updated_dvr_dvr_edge_router_snat_rescheduled(self):
+        self._test_ext_gw_updated_dvr_edge_router(host_match=True,
+                                                  snat_hosted_before=False)
 
     def test_agent_add_external_gateway(self):
         router = l3_test_common.prepare_router_data(num_internal_ports=2)