]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Eliminate unnecessary indirection in L3 agent
authorCarl Baldwin <carl.baldwin@hp.com>
Fri, 17 Oct 2014 04:16:49 +0000 (04:16 +0000)
committerCarl Baldwin <carl.baldwin@hp.com>
Thu, 20 Nov 2014 23:29:45 +0000 (23:29 +0000)
These indirections serve no useful purpose.  Normally, I wouldn't
bother to change them but these make other refactoring efforts a bit
more of a pain.

Change-Id: Ia73c93eb2eaa81d5772b3e0178fdf18f0c275b4c

neutron/agent/l3_agent.py
neutron/tests/unit/test_l3_agent.py

index 3120f3c3c74176c64f354e2c6e7c39e7463fd223..912a029f285c58489257c50ec4dc263883680bac 100644 (file)
@@ -620,17 +620,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         ns_to_ignore = self._get_routers_namespaces(router_ids)
 
         ns_to_destroy = router_namespaces - ns_to_ignore
-        self._destroy_stale_router_namespaces(ns_to_destroy)
-
-    def _destroy_stale_router_namespaces(self, router_namespaces):
-        """Destroys the stale router namespaces
-
-        The argumenet router_namespaces is a list of stale router namespaces
-
-        As some stale router namespaces may not be able to be deleted, only
-        one attempt will be made to delete them.
-        """
-        for ns in router_namespaces:
+        for ns in ns_to_destroy:
             try:
                 self._destroy_namespace(ns)
             except RuntimeError:
@@ -1845,18 +1835,11 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         while True:
             pool.spawn_n(self._process_router_update)
 
-    def _router_ids(self):
-        if not self.conf.use_namespaces:
-            return [self.conf.router_id]
-
     @periodic_task.periodic_task
     def periodic_sync_routers_task(self, context):
-        self._sync_routers_task(context)
-
-    def _sync_routers_task(self, context):
         if self.services_sync:
             super(L3NATAgent, self).process_services_sync(context)
-        LOG.debug("Starting _sync_routers_task - fullsync:%s",
+        LOG.debug("Starting periodic_sync_routers_task - fullsync:%s",
                   self.fullsync)
         if not self.fullsync:
             return
@@ -1869,10 +1852,12 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
         prev_router_ids = set(self.router_info)
 
         try:
-            router_ids = self._router_ids()
             timestamp = timeutils.utcnow()
-            routers = self.plugin_rpc.get_routers(
-                context, router_ids)
+            if self.conf.use_namespaces:
+                routers = self.plugin_rpc.get_routers(context)
+            else:
+                routers = self.plugin_rpc.get_routers(context,
+                                                      [self.conf.router_id])
 
             LOG.debug('Processing :%r', routers)
             for r in routers:
@@ -1882,7 +1867,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback,
                                       timestamp=timestamp)
                 self._queue.add(update)
             self.fullsync = False
-            LOG.debug("_sync_routers_task successfully completed")
+            LOG.debug("periodic_sync_routers_task successfully completed")
         except messaging.MessagingException:
             LOG.exception(_LE("Failed synchronizing routers due to RPC error"))
             self.fullsync = True
index b4e63d604ada7e8ec2614b902048ab046b592ae5..6cc20b5e278c9c9f750f2d2bc7c3b7c5847d983f 100644 (file)
@@ -415,18 +415,18 @@ class TestBasicRouterOperations(base.BaseTestCase):
 
         return agent, ri, port
 
-    def test__sync_routers_task_raise_exception(self):
+    def test_periodic_sync_routers_task_raise_exception(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         self.plugin_api.get_routers.side_effect = Exception()
         with mock.patch.object(agent, '_cleanup_namespaces') as f:
-            agent._sync_routers_task(agent.context)
+            agent.periodic_sync_routers_task(agent.context)
         self.assertFalse(f.called)
 
-    def test__sync_routers_task_call_clean_stale_namespaces(self):
+    def test_periodic_sync_routers_task_call_clean_stale_namespaces(self):
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
         self.plugin_api.get_routers.return_value = []
         with mock.patch.object(agent, '_cleanup_namespaces') as f:
-            agent._sync_routers_task(agent.context)
+            agent.periodic_sync_routers_task(agent.context)
         self.assertTrue(f.called)
 
     def test_router_info_create(self):
@@ -1516,8 +1516,8 @@ vrrp_instance VR_1 {
             # The unexpected exception has been fixed manually
             internal_network_added.side_effect = None
 
-            # _sync_routers_task finds out that _rpc_loop failed to process the
-            # router last time, it will retry in the next run.
+            # periodic_sync_routers_task finds out that _rpc_loop failed to
+            # process the router last time, it will retry in the next run.
             agent.process_router(ri)
             # We were able to add the port to ri.internal_ports
             self.assertIn(
@@ -1547,8 +1547,8 @@ vrrp_instance VR_1 {
             # The unexpected exception has been fixed manually
             internal_net_removed.side_effect = None
 
-            # _sync_routers_task finds out that _rpc_loop failed to process the
-            # router last time, it will retry in the next run.
+            # periodic_sync_routers_task finds out that _rpc_loop failed to
+            # process the router last time, it will retry in the next run.
             agent.process_router(ri)
             # We were able to remove the port from ri.internal_ports
             self.assertNotIn(
@@ -1867,7 +1867,7 @@ vrrp_instance VR_1 {
 
         self.conf.set_override('router_id', '1234')
         agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
-        self.assertEqual(['1234'], agent._router_ids())
+        self.assertEqual('1234', agent.conf.router_id)
         self.assertFalse(agent._clean_stale_namespaces)
 
     def test_process_router_if_compatible_with_no_ext_net_in_conf(self):