Merge "Fix argument name mismatch in L3-RPC sync_routers"

neutron/agent/l3_agent.py

@@ -67,11 +67,10 @@ class L3PluginApi(proxy.RpcProxy):
             topic=topic, default_version=self.BASE_RPC_API_VERSION)
         self.host = host
 
-    def get_routers(self, context, fullsync=True, router_ids=None):
+    def get_routers(self, context, router_ids=None):
         """Make a remote process call to retrieve the sync data for routers."""
         return self.call(context,
                          self.make_msg('sync_routers', host=self.host,
-                                       fullsync=fullsync,
                                        router_ids=router_ids),
                          topic=self.topic)
 

neutron/db/agentschedulers_db.py

@@ -123,7 +123,7 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
 
             result = self.auto_schedule_routers(context,
                                                 agent_db.host,
-                                                router_id)
+                                                [router_id])
             if not result:
                 raise l3agentscheduler.RouterSchedulingFailed(
                     router_id=router_id, agent_id=id)
@@ -166,7 +166,7 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
             return {'routers': []}
 
     def list_active_sync_routers_on_active_l3_agent(
-            self, context, host, router_id):
+            self, context, host, router_ids):
         agent = self._get_agent_by_type_and_host(
             context, constants.AGENT_TYPE_L3, host)
         if not agent.admin_state_up:
@@ -174,9 +174,12 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
         query = context.session.query(RouterL3AgentBinding.router_id)
         query = query.filter(
             RouterL3AgentBinding.l3_agent_id == agent.id)
-        if router_id:
-            query = query.filter(RouterL3AgentBinding.router_id == router_id)
 
+        if not router_ids:
+            pass
+        else:
+            query = query.filter(
+                RouterL3AgentBinding.router_id.in_(router_ids))
         router_ids = [item[0] for item in query]
         if router_ids:
             return self.get_sync_data(context, router_ids=router_ids,
@@ -272,10 +275,10 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
             candidates.append(l3_agent)
         return candidates
 
-    def auto_schedule_routers(self, context, host, router_id):
+    def auto_schedule_routers(self, context, host, router_ids):
         if self.router_scheduler:
             return self.router_scheduler.auto_schedule_routers(
-                self, context, host, router_id)
+                self, context, host, router_ids)
 
     def schedule_router(self, context, router):
         if self.router_scheduler:

neutron/db/l3_rpc_base.py

@@ -33,22 +33,22 @@ class L3RpcCallbackMixin(object):
         """Sync routers according to filters to a specific agent.
 
         @param context: contain user information
-        @param kwargs: host, or router_id
+        @param kwargs: host, router_ids
         @return: a list of routers
                  with their interfaces and floating_ips
         """
-        router_id = kwargs.get('router_id')
+        router_ids = kwargs.get('router_ids')
         host = kwargs.get('host')
         context = neutron_context.get_admin_context()
         plugin = manager.NeutronManager.get_plugin()
         if utils.is_extension_supported(
             plugin, constants.L3_AGENT_SCHEDULER_EXT_ALIAS):
             if cfg.CONF.router_auto_schedule:
-                plugin.auto_schedule_routers(context, host, router_id)
+                plugin.auto_schedule_routers(context, host, router_ids)
             routers = plugin.list_active_sync_routers_on_active_l3_agent(
-                context, host, router_id)
+                context, host, router_ids)
         else:
-            routers = plugin.get_sync_data(context, router_id)
+            routers = plugin.get_sync_data(context, router_ids)
         LOG.debug(_("Routers returned to l3 agent:\n %s"),
                   jsonutils.dumps(routers, indent=5))
         return routers

neutron/scheduler/l3_agent_scheduler.py

@@ -36,10 +36,11 @@ class ChanceScheduler(object):
     can be introduced later.
     """
 
-    def auto_schedule_routers(self, plugin, context, host, router_id):
+    def auto_schedule_routers(self, plugin, context, host, router_ids):
         """Schedule non-hosted routers to L3 Agent running on host.
-        If router_id is given, only this router is scheduled
+        If router_ids is given, each router in router_ids is scheduled
-        if it is not hosted yet.
+        if it is not scheduled yet. Otherwise all unscheduled routers
+        are scheduled.
         Don't schedule the routers which are hosted already
         by active l3 agents.
         """
@@ -59,42 +60,45 @@ class ChanceScheduler(object):
             if agents_db.AgentDbMixin.is_agent_down(
                 l3_agent.heartbeat_timestamp):
                 LOG.warn(_('L3 agent %s is not active'), l3_agent.id)
-            # check if the specified router is hosted
+            # check if each of the specified routers is hosted
-            if router_id:
+            if router_ids:
+                unscheduled_router_ids = []
+                for router_id in router_ids:
                     l3_agents = plugin.get_l3_agents_hosting_routers(
                         context, [router_id], admin_state_up=True)
                     if l3_agents:
-                    LOG.debug(_('Router %(router_id)s has already been hosted'
+                        LOG.debug(_('Router %(router_id)s has already been'
-                                ' by L3 agent %(agent_id)s'),
+                                    ' hosted by L3 agent %(agent_id)s'),
                                   {'router_id': router_id,
                                    'agent_id': l3_agents[0]['id']})
+                    else:
+                        unscheduled_router_ids.append(router_id)
+                if not unscheduled_router_ids:
+                    # all (specified) routers are already scheduled
                     return False
-
-            # get the router ids
-            if router_id:
-                router_ids = [(router_id,)]
             else:
                 # get all routers that are not hosted
                 #TODO(gongysh) consider the disabled agent's router
                 stmt = ~exists().where(
                     l3_db.Router.id ==
                     agentschedulers_db.RouterL3AgentBinding.router_id)
-                router_ids = context.session.query(
+                unscheduled_router_ids = [router_id_[0] for router_id_ in
-                    l3_db.Router.id).filter(stmt).all()
+                                          context.session.query(
-            if not router_ids:
+                                              l3_db.Router.id).filter(stmt)]
+                if not unscheduled_router_ids:
                     LOG.debug(_('No non-hosted routers'))
                     return False
 
             # check if the configuration of l3 agent is compatible
             # with the router
-            router_ids = [router_id_[0] for router_id_ in router_ids]
+            routers = plugin.get_routers(
-            routers = plugin.get_routers(context, filters={'id': router_ids})
+                context, filters={'id': unscheduled_router_ids})
             to_removed_ids = []
             for router in routers:
                 candidates = plugin.get_l3_agent_candidates(router, [l3_agent])
                 if not candidates:
                     to_removed_ids.append(router['id'])
-            router_ids = list(set(router_ids) - set(to_removed_ids))
+            router_ids = set(unscheduled_router_ids) - set(to_removed_ids)
             if not router_ids:
                 LOG.warn(_('No routers compatible with L3 agent configuration'
                            ' on host %s'), host)

neutron/tests/unit/openvswitch/test_agent_scheduler.py

@@ -568,10 +568,13 @@ class OvsAgentSchedulerTestCase(test_l3_plugin.L3NatTestCaseMixin,
         with self.router() as router:
             l3_rpc = l3_rpc_base.L3RpcCallbackMixin()
             self._register_agent_states()
-            l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA)
+            ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA)
-            l3_rpc.sync_routers(self.adminContext, host=L3_HOSTB)
+            ret_b = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTB)
             l3_agents = self._list_l3_agents_hosting_router(
                 router['router']['id'])
+            self.assertEqual(1, len(ret_a))
+            self.assertIn(router['router']['id'], [r['id'] for r in ret_a])
+            self.assertFalse(len(ret_b))
         self.assertEqual(1, len(l3_agents['agents']))
         self.assertEqual(L3_HOSTA, l3_agents['agents'][0]['host'])
 
@@ -682,6 +685,75 @@ class OvsAgentSchedulerTestCase(test_l3_plugin.L3NatTestCaseMixin,
         self.assertEqual(1, len(l3_agents_1['agents']))
         self.assertEqual(0, len(l3_agents_2['agents']))
 
+    def test_rpc_sync_routers(self):
+        l3_rpc = l3_rpc_base.L3RpcCallbackMixin()
+        self._register_agent_states()
+
+        # No routers
+        ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA)
+        self.assertEqual(0, len(ret_a))
+
+        with contextlib.nested(self.router(),
+                               self.router(),
+                               self.router()) as routers:
+            router_ids = [r['router']['id'] for r in routers]
+
+            # Get all routers
+            ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA)
+            self.assertEqual(3, len(ret_a))
+            self.assertEqual(set(router_ids), set([r['id'] for r in ret_a]))
+
+            # Get all routers (router_ids=None)
+            ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA,
+                                        router_ids=None)
+            self.assertEqual(3, len(ret_a))
+            self.assertEqual(set(router_ids), set([r['id'] for r in ret_a]))
+
+            # Get router2 only
+            ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA,
+                                        router_ids=[router_ids[1]])
+            self.assertEqual(1, len(ret_a))
+            self.assertIn(router_ids[1], [r['id'] for r in ret_a])
+
+            # Get router1 and router3
+            ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA,
+                                        router_ids=[router_ids[0],
+                                                    router_ids[2]])
+            self.assertEqual(2, len(ret_a))
+            self.assertIn(router_ids[0], [r['id'] for r in ret_a])
+            self.assertIn(router_ids[2], [r['id'] for r in ret_a])
+
+    def test_router_auto_schedule_for_specified_routers(self):
+
+        def _sync_router_with_ids(router_ids, exp_synced, exp_hosted, host_id):
+            ret_a = l3_rpc.sync_routers(self.adminContext, host=L3_HOSTA,
+                                        router_ids=router_ids)
+            self.assertEqual(exp_synced, len(ret_a))
+            for r in router_ids:
+                self.assertIn(r, [r['id'] for r in ret_a])
+            host_routers = self._list_routers_hosted_by_l3_agent(host_id)
+            num_host_routers = len(host_routers['routers'])
+            self.assertEqual(exp_hosted, num_host_routers)
+
+        l3_rpc = l3_rpc_base.L3RpcCallbackMixin()
+        self._register_agent_states()
+        hosta_id = self._get_agent_id(constants.AGENT_TYPE_L3, L3_HOSTA)
+
+        with contextlib.nested(self.router(), self.router(),
+                               self.router(), self.router()) as routers:
+            router_ids = [r['router']['id'] for r in routers]
+            # Sync router1 (router1 is scheduled)
+            _sync_router_with_ids([router_ids[0]], 1, 1, hosta_id)
+            # Sync router1 only (no router is scheduled)
+            _sync_router_with_ids([router_ids[0]], 1, 1, hosta_id)
+            # Schedule router2
+            _sync_router_with_ids([router_ids[1]], 1, 2, hosta_id)
+            # Sync router2 and router4 (router4 is scheduled)
+            _sync_router_with_ids([router_ids[1], router_ids[3]],
+                                  2, 3, hosta_id)
+            # Sync all routers (router3 is scheduled)
+            _sync_router_with_ids(router_ids, 4, 4, hosta_id)
+
     def test_router_schedule_with_candidates(self):
         l3_hosta = {
             'binary': 'neutron-l3-agent',