From: lzklibj <lzklibj@cn.ibm.com>
Date: Thu, 26 Feb 2015 05:19:07 +0000 (-0800)
Subject: fix check_ports_exist_on_l3agent in no subnet case
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=e99f6e00cfd397bb74d44c9462dfcfa545dbed8c;p=openstack-build%2Fneutron-build.git

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 c0142b22b..2d5d9b4da 100644
--- 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 f3ba89e80..ef548a167 100644
--- 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,