Avoid full_sync in l3_agent for router updates

While processing a router update in _process_router_update method,
if an exception occurs, we try to do a full_sync.

We only need to re-sync the router whose update failed.

Addressed a TODO in the same method, which falls in similar lines.

Change-Id: I7c43a508adf46d8524f1cc48b83f1e1c276a2de0
Closes-Bug: #1494682

diff --git a/neutron/agent/l3/agent.py b/neutron/agent/l3/agent.py
index 834d1723a1606d4dad4b992f38634897a2d1bd31..e71e51d87201135984c9bf979875e198596ec7cd 100644 (file)
--- a/neutron/agent/l3/agent.py
+++ b/neutron/agent/l3/agent.py
@@ -437,6 +437,12 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         ri.process(self)
         registry.notify(resources.ROUTER, events.AFTER_UPDATE, self, router=ri)
 
+    def _resync_router(self, router_update,
+                       priority=queue.PRIORITY_SYNC_ROUTERS_TASK):
+        router_update.timestamp = timeutils.utcnow()
+        router_update.priority = priority
+        self._queue.add(router_update)
+
     def _process_router_update(self):
         for rp, update in self._queue.each_update_to_next_router():
             LOG.debug("Starting router update for %s, action %s, priority %s",
@@ -454,7 +460,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
                 except Exception:
                     msg = _LE("Failed to fetch router information for '%s'")
                     LOG.exception(msg, update.id)
-                    self.fullsync = True
+                    self._resync_router(update)
                     continue
 
                 if routers:
@@ -463,10 +469,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
             if not router:
                 removed = self._safe_router_removed(update.id)
                 if not removed:
-                    # TODO(Carl) Stop this fullsync non-sense.  Just retry this
-                    # one router by sticking the update at the end of the queue
-                    # at a lower priority.
-                    self.fullsync = True
+                    self._resync_router(update)
                 else:
                     # need to update timestamp of removed router in case
                     # there are older events for the same router in the
@@ -488,7 +491,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
             except Exception:
                 msg = _LE("Failed to process compatible router '%s'")
                 LOG.exception(msg, update.id)
-                self.fullsync = True
+                self._resync_router(update)
                 continue
 
             LOG.debug("Finished a router update for %s", update.id)

diff --git a/neutron/tests/unit/agent/l3/test_agent.py b/neutron/tests/unit/agent/l3/test_agent.py
index 6453d831afe18ffa3c8b4084647453c03e8c2c59..22576d70914b3072bddb1c16040e1bacda57d5b7 100644 (file)
--- a/neutron/tests/unit/agent/l3/test_agent.py
+++ b/neutron/tests/unit/agent/l3/test_agent.py
@@ -1793,36 +1793,36 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         self.conf.set_override('router_id', '1234')
         self._configure_metadata_proxy()
 
-    def test_process_routers_update_rpc_timeout_on_get_routers(self):
+    def _test_process_routers_update_rpc_timeout(self, ext_net_call=False,
+                                                 ext_net_call_failed=False):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         agent.fullsync = False
         agent._process_router_if_compatible = mock.Mock()
-        self.plugin_api.get_routers.side_effect = (
-            oslo_messaging.MessagingTimeout)
+        if ext_net_call_failed:
+            agent._process_router_if_compatible.side_effect = (
+                oslo_messaging.MessagingTimeout)
         agent._queue = mock.Mock()
+        agent._resync_router = mock.Mock()
         update = mock.Mock()
         update.router = None
         agent._queue.each_update_to_next_router.side_effect = [
             [(None, update)]]
-
         agent._process_router_update()
-        self.assertTrue(agent.fullsync)
-        self.assertFalse(agent._process_router_if_compatible.called)
+        self.assertFalse(agent.fullsync)
+        self.assertEqual(ext_net_call,
+                         agent._process_router_if_compatible.called)
+        agent._resync_router.assert_called_with(update)
 
-    def test_process_routers_update_rpc_timeout_on_get_ext_net(self):
-        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
-        agent.fullsync = False
-        agent._process_router_if_compatible = mock.Mock()
-        agent._process_router_if_compatible.side_effect = (
+    def test_process_routers_update_rpc_timeout_on_get_routers(self):
+        self.plugin_api.get_routers.side_effect = (
             oslo_messaging.MessagingTimeout)
-        agent._queue = mock.Mock()
-        agent._queue.each_update_to_next_router.side_effect = [
-            [(None, mock.Mock())]]
+        self._test_process_routers_update_rpc_timeout()
 
-        agent._process_router_update()
-        self.assertTrue(agent.fullsync)
+    def test_process_routers_update_rpc_timeout_on_get_ext_net(self):
+        self._test_process_routers_update_rpc_timeout(ext_net_call=True,
+                                                      ext_net_call_failed=True)
 
-    def test_process_routers_update_router_deleted(self):
+    def _test_process_routers_update_router_deleted(self, error=False):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         agent._queue = mock.Mock()
         update = mock.Mock()
@@ -1833,11 +1833,26 @@ class TestBasicRouterOperations(BasicRouterOperationsFramework):
         router_processor = mock.Mock()
         agent._queue.each_update_to_next_router.side_effect = [
             [(router_processor, update)]]
+        agent._resync_router = mock.Mock()
+        if error:
+            agent._safe_router_removed = mock.Mock()
+            agent._safe_router_removed.return_value = False
         agent._process_router_update()
-        router_info.delete.assert_called_once_with(agent)
-        self.assertFalse(agent.router_info)
-        router_processor.fetched_and_processed.assert_called_once_with(
-            update.timestamp)
+        if error:
+            self.assertFalse(router_processor.fetched_and_processed.called)
+            agent._resync_router.assert_called_with(update)
+        else:
+            router_info.delete.assert_called_once_with(agent)
+            self.assertFalse(agent.router_info)
+            self.assertFalse(agent._resync_router.called)
+            router_processor.fetched_and_processed.assert_called_once_with(
+                update.timestamp)
+
+    def test_process_routers_update_router_deleted_success(self):
+        self._test_process_routers_update_router_deleted()
+
+    def test_process_routers_update_router_deleted_error(self):
+        self._test_process_routers_update_router_deleted(True)
 
     def test_process_router_if_compatible_with_no_ext_net_in_conf(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)