fix check_ports_exist_on_l3agent in no subnet case

If no subnets attached to the given router, this check
should return False.

Currently, if no subnets attached to given router, the
following process in this method will fetch all ports
to continue its checking, consider those ports are not
related to the given router, the following checking
should be invalid.

To issue #1378066, after running "router-gateway-clear",
_schedule_router will be triggered, and the invalid
checking will make processing in get_candidates believe
that all l3-agents are valid to schedule this router,
and finally, invalid records are inserted into table
RouterL3AgentBindings.

Closes-Bug: #1378066
Closes-Bug: #1417908
Closes-Bug: #1420032

Change-Id: If96d866c831330cca68a5fe5a0f27f178bbf40a6

diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py
index c0142b22b98965d3948996806f2d342fcd666025..2d5d9b4da91898bce5b1bcee5f6bc7855e00ae98 100644 (file)
--- a/neutron/db/l3_agentschedulers_db.py
+++ b/neutron/db/l3_agentschedulers_db.py
@@ -384,6 +384,8 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
         ports on the host, running the input l3agent.
         """
         subnet_ids = self.get_subnet_ids_on_router(context, router_id)
+        if not subnet_ids:
+            return False
 
         core_plugin = manager.NeutronManager.get_plugin()
         filter = {'fixed_ips': {'subnet_id': subnet_ids}}

diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py
index f3ba89e80b08bdd291fcefd57e93b3bd21b0ff7e..ef548a167e145b8264b49f86661341524dc74c51 100644 (file)
--- a/neutron/tests/unit/test_l3_schedulers.py
+++ b/neutron/tests/unit/test_l3_schedulers.py
@@ -667,33 +667,45 @@ class L3SchedulerTestBaseMixin(object):
 
     def test_check_ports_exist_on_l3agent_no_subnets(self):
         l3_agent, router = self._prepare_check_ports_exist_tests()
-        # no subnets
-        val = self.check_ports_exist_on_l3agent(self.adminContext,
-                                                l3_agent, router['id'])
-        self.assertFalse(val)
+        with mock.patch.object(manager.NeutronManager,
+                               'get_plugin') as getp:
+            getp.return_value = self.plugin
+            # no subnets
+            self.get_subnet_ids_on_router.return_value = set()
+            val = self.check_ports_exist_on_l3agent(self.adminContext,
+                                                    l3_agent, router['id'])
+            self.assertFalse(self.plugin.get_ports.called)
+            self.assertFalse(val)
 
     def test_check_ports_exist_on_l3agent_no_subnet_match(self):
         l3_agent, router = self._prepare_check_ports_exist_tests()
-        # no matching subnet
-        self.plugin.get_subnet_ids_on_router = mock.Mock(
-            return_value=[str(uuid.uuid4())])
-        val = self.check_ports_exist_on_l3agent(self.adminContext,
-                                                l3_agent, router['id'])
-        self.assertFalse(val)
+        with mock.patch.object(manager.NeutronManager,
+                               'get_plugin') as getp:
+            getp.return_value = self.plugin
+            # no matching subnet
+            self.get_subnet_ids_on_router.return_value = [str(uuid.uuid4())]
+            val = self.check_ports_exist_on_l3agent(self.adminContext,
+                                                    l3_agent, router['id'])
+            self.assertTrue(self.plugin.get_ports.called)
+            self.assertFalse(val)
 
     def test_check_ports_exist_on_l3agent_subnet_match(self):
         l3_agent, router = self._prepare_check_ports_exist_tests()
-        # matching subnet
-        port = {'subnet_id': str(uuid.uuid4()),
-                'binding:host_id': 'host_1',
-                'device_owner': 'compute:',
-                'id': 1234}
-        self.plugin.get_ports.return_value = [port]
-        self.plugin.get_subnet_ids_on_router = mock.Mock(
-            return_value=[port['subnet_id']])
-        val = self.check_ports_exist_on_l3agent(self.adminContext,
-                                                l3_agent, router['id'])
-        self.assertTrue(val)
+        with mock.patch.object(manager.NeutronManager,
+                               'get_plugin') as getp:
+            getp.return_value = self.plugin
+            # matching subnet
+            port = {'subnet_id': str(uuid.uuid4()),
+                    'binding:host_id': 'host_1',
+                    'device_owner': 'compute:',
+                    'id': 1234}
+            self.plugin.get_ports.return_value = [port]
+            self.get_subnet_ids_on_router = mock.Mock(
+                return_value=[port['subnet_id']])
+            val = self.check_ports_exist_on_l3agent(self.adminContext,
+                                                    l3_agent, router['id'])
+            self.assertTrue(self.plugin.get_ports.called)
+            self.assertTrue(val)
 
 
 class L3SchedulerTestCase(l3_agentschedulers_db.L3AgentSchedulerDbMixin,