Add validation for the dvr router l3agent binding

Validates dvr router add/remove cases to the
l3agents running in different dvr modes such
as "dvr_snat" and "dvr" mode.

In the case of distributed virtual routers it
does not make sense to move distributed routers
from one "dvr" node to another "dvr" node.

Also added some unit tests that addresses the
validation of legacy routers to dvr agent and
dvr routers to legacy agent.

Partial-Bug: #1369721

Change-Id: I008dda6abaf25094b11f3730b951e096dd3b7025

diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py
index 5fd3d360caf7be86a64b9528754bd236bf7bace5..4284346a9a7a342c017159fe2eb58abf2275429e 100644 (file)
--- a/neutron/db/l3_agentschedulers_db.py
+++ b/neutron/db/l3_agentschedulers_db.py
@@ -153,20 +153,25 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
           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)
+        :raises: DVRL3CannotAssignToDvrAgent if attempting to assign DVR
+          router from one DVR Agent to another.
         """
         is_distributed = router.get('distributed')
         agent_conf = self.get_configuration_dict(agent)
         agent_mode = agent_conf.get('agent_mode', 'legacy')
-
+        router_type = ('distributed' if is_distributed else 'centralized')
         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'])
+        if agent_mode == 'dvr' and is_distributed:
+            raise l3agentscheduler.DVRL3CannotAssignToDvrAgent(
+                router_type=router_type, router_id=router['id'],
+                agent_id=agent['id'])
 
         is_wrong_type_or_unsuitable_agent = (
             agent['agent_type'] != constants.AGENT_TYPE_L3 or

diff --git a/neutron/extensions/l3agentscheduler.py b/neutron/extensions/l3agentscheduler.py
index d8e27aaf5668ebd0d337367a143666830b4e0ea1..98adc999c16615137ebd43c2c0e85cd69aa08378 100644 (file)
--- a/neutron/extensions/l3agentscheduler.py
+++ b/neutron/extensions/l3agentscheduler.py
@@ -183,6 +183,12 @@ class RouterL3AgentMismatch(exceptions.Conflict):
                 "on %(agent_mode)s L3 agent %(agent_id)s.")
 
 
+class DVRL3CannotAssignToDvrAgent(exceptions.Conflict):
+    message = _("Not allowed to manually assign a %(router_type)s "
+                "router %(router_id)s from an existing DVR node "
+                "to another L3 agent %(agent_id)s.")
+
+
 class L3AgentSchedulerPluginBase(object):
     """REST API to operate the l3 agent scheduler.
 

diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py
index 99b5ebc52c441c79708da83c251820377b43a7d6..3f0e3e649af0784863b2fe253e9628d9ea5573ff 100644 (file)
--- a/neutron/tests/unit/test_l3_schedulers.py
+++ b/neutron/tests/unit/test_l3_schedulers.py
@@ -35,6 +35,7 @@ from neutron.db import l3_db
 from neutron.db import l3_dvrscheduler_db
 from neutron.db import l3_hamode_db
 from neutron.db import l3_hascheduler_db
+from neutron.extensions import l3agentscheduler as l3agent
 from neutron import manager
 from neutron.scheduler import l3_agent_scheduler
 from neutron.tests import base
@@ -293,7 +294,7 @@ class L3SchedulerBaseMixin(object):
         agent_db = self.plugin.get_agents_db(self.adminContext,
                                              filters={'host': [HOST_DVR]})
         self.l3_dvr_agent = agent_db[0]
-
+        self.l3_dvr_agent_id = agent_db[0].id
         callback.report_state(self.adminContext,
                               agent_state={'agent_state': DVR_SNAT_L3_AGENT},
                               time=timeutils.strtime())
@@ -337,7 +338,8 @@ class L3SchedulerTestBaseMixin(object):
 
     def _test_add_router_to_l3_agent(self,
                                      distributed=False,
-                                     already_scheduled=False):
+                                     already_scheduled=False,
+                                     external_gw=None):
         agent_id = self.agent_id1
         agent = self.agent1
         if distributed:
@@ -348,7 +350,7 @@ class L3SchedulerTestBaseMixin(object):
                                    tenant_id=str(uuid.uuid4()),
                                    name='r1')
         router['router']['distributed'] = distributed
-        router['router']['external_gateway_info'] = None
+        router['router']['external_gateway_info'] = external_gw
         if already_scheduled:
             self._test_schedule_bind_router(agent, router)
         with contextlib.nested(
@@ -361,21 +363,97 @@ class L3SchedulerTestBaseMixin(object):
                                         router['router']['id'])
             self.assertNotEqual(already_scheduled, auto_s.called)
 
+    def _create_router_for_l3_agent_dvr_test(self,
+                                             distributed=False,
+                                             external_gw=None):
+        router = self._make_router(self.fmt,
+                                   tenant_id=str(uuid.uuid4()),
+                                   name='r1')
+        router['router']['distributed'] = distributed
+        router['router']['external_gateway_info'] = external_gw
+        return router
+
+    def _prepare_l3_agent_dvr_move_exceptions(self,
+                                              distributed=False,
+                                              external_gw=None,
+                                              agent_id=None,
+                                              expected_exception=None):
+        router = self._create_router_for_l3_agent_dvr_test(
+            distributed=distributed, external_gw=external_gw)
+        with contextlib.nested(
+            mock.patch.object(self, "create_router_to_agent_binding"),
+            mock.patch('neutron.db.l3_db.L3_NAT_db_mixin.get_router',
+                       return_value=router['router'])):
+            self.assertRaises(expected_exception,
+                              self.add_router_to_l3_agent,
+                              self.adminContext, agent_id,
+                              router['router']['id'])
+
+    def test_add_router_to_l3_agent_mismatch_error_dvr_to_legacy(self):
+        self._register_l3_agents()
+        self._prepare_l3_agent_dvr_move_exceptions(
+            distributed=True,
+            agent_id=self.agent_id1,
+            expected_exception=l3agent.RouterL3AgentMismatch)
+
+    def test_add_router_to_l3_agent_mismatch_error_legacy_to_dvr(self):
+        self._register_l3_dvr_agents()
+        self._prepare_l3_agent_dvr_move_exceptions(
+            agent_id=self.l3_dvr_agent_id,
+            expected_exception=l3agent.RouterL3AgentMismatch)
+
+    def test_add_router_to_l3_agent_mismatch_error_dvr_to_dvr(self):
+        self._register_l3_dvr_agents()
+        self._prepare_l3_agent_dvr_move_exceptions(
+            distributed=True,
+            agent_id=self.l3_dvr_agent_id,
+            expected_exception=l3agent.DVRL3CannotAssignToDvrAgent)
+
+    def test_add_router_to_l3_agent_dvr_to_snat(self):
+        external_gw_info = {
+            "network_id": str(uuid.uuid4()),
+            "enable_snat": True
+        }
+        self._register_l3_dvr_agents()
+        agent_id = self.l3_dvr_snat_id
+        agent = self.l3_dvr_snat_agent
+        router = self._create_router_for_l3_agent_dvr_test(
+            distributed=True,
+            external_gw=external_gw_info)
+        with contextlib.nested(
+            mock.patch.object(self, "validate_agent_router_combination"),
+            mock.patch.object(self, "create_router_to_agent_binding"),
+            mock.patch('neutron.db.l3_db.L3_NAT_db_mixin.get_router',
+                       return_value=router['router'])
+        ) as (valid_agent_rtr, rtr_agent_binding, get_rtr):
+
+            self.add_router_to_l3_agent(self.adminContext, agent_id,
+                                        router['router']['id'])
+            rtr_agent_binding.assert_called_once_with(
+                self.adminContext, agent, router['router'])
+
     def test_add_router_to_l3_agent(self):
-        self._test_add_router_to_l3_agent(distributed=False,
-                                          already_scheduled=False)
+        self._test_add_router_to_l3_agent()
 
     def test_add_distributed_router_to_l3_agent(self):
+        external_gw_info = {
+            "network_id": str(uuid.uuid4()),
+            "enable_snat": True
+        }
         self._test_add_router_to_l3_agent(distributed=True,
-                                          already_scheduled=False)
+                                          external_gw=external_gw_info)
 
     def test_add_router_to_l3_agent_already_scheduled(self):
-        self._test_add_router_to_l3_agent(distributed=False,
-                                          already_scheduled=True)
+        self._test_add_router_to_l3_agent(already_scheduled=True)
 
     def test_add_distributed_router_to_l3_agent_already_scheduled(self):
+        external_gw_info = {
+            "network_id": str(uuid.uuid4()),
+            "enable_snat": True
+        }
         self._test_add_router_to_l3_agent(distributed=True,
-                                          already_scheduled=True)
+                                          already_scheduled=True,
+                                          external_gw=external_gw_info)
 
     def _prepare_schedule_dvr_tests(self):
         scheduler = l3_agent_scheduler.ChanceScheduler()