Minor refactoring for add_router_to_l3_agent

This method is more complicated than it needs
to be, and it makes it difficult to target
fixes for it.

Furthermore, this method calls into
auto_schedule_routers, which duplicates some
of the DB calls already made in the above
mentioned method. This refactoring
is done in preparation of the performance
improvement.

Partial-bug: #1356121
Related-Bug: #1358636

Change-Id: I9a0cfa41a5f067949b964d39157def55c40bf9af

diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py
index 8dd2739cb9ce7b004f57d9bde19572385515b73a..1543779de03b27702f7dccc9d662fe185b7d2904 100644 (file)
--- a/neutron/db/l3_agentschedulers_db.py
+++ b/neutron/db/l3_agentschedulers_db.py
@@ -122,55 +122,83 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
                       'dead_time': agent_dead_limit})
             self.reschedule_router(context, binding.router_id)
 
+    def validate_agent_router_combination(self, context, agent, router):
+        """Validate if the router can be correctly assigned to the agent.
+
+        :raises: RouterL3AgentMismatch if attempting to assign DVR router
+          to legacy agent, or centralized router to compute's L3 agents.
+        :raises: InvalidL3Agent if attempting to assign router to an
+          unsuitable agent (disabled, type != L3, incompatible configuration)
+        """
+        is_distributed = router.get('distributed')
+        agent_conf = self.get_configuration_dict(agent)
+        agent_mode = agent_conf.get('agent_mode', 'legacy')
+
+        is_agent_router_types_incompatible = (
+            agent_mode == 'dvr' and not is_distributed
+            or agent_mode == 'legacy' and is_distributed
+        )
+        if is_agent_router_types_incompatible:
+            router_type = ('distributed' if is_distributed else 'centralized')
+            raise l3agentscheduler.RouterL3AgentMismatch(
+                router_type=router_type, router_id=router['id'],
+                agent_mode=agent_mode, agent_id=agent['id'])
+
+        is_wrong_type_or_unsuitable_agent = (
+            agent['agent_type'] != constants.AGENT_TYPE_L3 or
+            not agent['admin_state_up'] or
+            not self.get_l3_agent_candidates(context, router, [agent])
+        )
+        if is_wrong_type_or_unsuitable_agent:
+            raise l3agentscheduler.InvalidL3Agent(id=agent['id'])
+
+    def validate_agent_router_existing_binding(self, context, agent, router):
+        """Validate if the router is already assigned to the agent.
+
+        :raises: RouterHostedByL3Agent if router is already assigned
+        """
+        router_id = router['id']
+        agent_id = agent['id']
+        query = context.session.query(RouterL3AgentBinding)
+        if router.get('distributed'):
+            binding = query.filter_by(router_id=router_id,
+                                      l3_agent_id=agent_id).first()
+            if binding:
+                raise l3agentscheduler.RouterHostedByL3Agent(
+                    router_id=router_id,
+                    agent_id=binding.l3_agent_id)
+        else:
+            try:
+                binding = query.filter_by(router_id=router_id).one()
+
+                raise l3agentscheduler.RouterHostedByL3Agent(
+                    router_id=router_id,
+                    agent_id=binding.l3_agent_id)
+            except exc.NoResultFound:
+                pass
+
+    def create_router_to_agent_binding(self, context, agent, router):
+        """Create router to agent binding."""
+        router_id = router['id']
+        agent_id = agent['id']
+        result = self.auto_schedule_routers(context, agent.host, [router_id])
+        if not result:
+            raise l3agentscheduler.RouterSchedulingFailed(
+                router_id=router_id, agent_id=agent_id)
+
     def add_router_to_l3_agent(self, context, agent_id, router_id):
         """Add a l3 agent to host a router."""
-        router = self.get_router(context, router_id)
-        distributed = router.get('distributed')
         with context.session.begin(subtransactions=True):
-            agent_db = self._get_agent(context, agent_id)
-            agent_conf = self.get_configuration_dict(agent_db)
-            agent_mode = agent_conf.get('agent_mode', 'legacy')
-            if (not distributed and agent_mode == 'dvr' or
-                distributed and agent_mode == 'legacy'):
-                router_type = ('distributed' if distributed else 'centralized')
-                raise l3agentscheduler.RouterL3AgentMismatch(
-                    router_type=router_type, router_id=router_id,
-                    agent_mode=agent_mode, agent_id=agent_id)
-            if (agent_db['agent_type'] != constants.AGENT_TYPE_L3 or
-                not agent_db['admin_state_up'] or
-                not self.get_l3_agent_candidates(context,
-                                                 router,
-                                                 [agent_db])):
-                raise l3agentscheduler.InvalidL3Agent(id=agent_id)
-            query = context.session.query(RouterL3AgentBinding)
-            if distributed:
-                binding = query.filter_by(router_id=router_id,
-                                          l3_agent_id=agent_id).first()
-                if binding:
-                    raise l3agentscheduler.RouterHostedByL3Agent(
-                        router_id=router_id,
-                        agent_id=binding.l3_agent_id)
-            else:
-                try:
-                    binding = query.filter_by(router_id=router_id).one()
-
-                    raise l3agentscheduler.RouterHostedByL3Agent(
-                        router_id=router_id,
-                        agent_id=binding.l3_agent_id)
-                except exc.NoResultFound:
-                    pass
-
-            result = self.auto_schedule_routers(context,
-                                                agent_db.host,
-                                                [router_id])
-            if not result:
-                raise l3agentscheduler.RouterSchedulingFailed(
-                    router_id=router_id, agent_id=agent_id)
+            router = self.get_router(context, router_id)
+            agent = self._get_agent(context, agent_id)
+            self.validate_agent_router_combination(context, agent, router)
+            self.validate_agent_router_existing_binding(context, agent, router)
+            self.create_router_to_agent_binding(context, agent, router)
 
         l3_notifier = self.agent_notifiers.get(constants.AGENT_TYPE_L3)
         if l3_notifier:
             l3_notifier.router_added_to_agent(
-                context, [router_id], agent_db.host)
+                context, [router_id], agent.host)
 
     def remove_router_from_l3_agent(self, context, agent_id, router_id):
         """Remove the router from l3 agent.