]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
fix get_ha_sync_data_for_host for non-dvr agent
authorlzklibj <lzklibj@cn.ibm.com>
Wed, 28 Oct 2015 09:02:11 +0000 (17:02 +0800)
committerlzklibj <lzklibj@cn.ibm.com>
Fri, 15 Jan 2016 08:47:57 +0000 (16:47 +0800)
get_ha_sync_data_for_host will call _get_dvr_sync_data
even given agent is not in DVR agent_mode.
_get_dvr_sync_data has additional processing for dvr
like:
    dvr_router_ids = set(router['id'] for router in routers
                         if is_distributed_router(router))
    floating_ip_port_ids = [fip['port_id'] for fip in floating_ips
                                if fip['router_id'] in dvr_router_ids]
which should only work for DVR scenario.

That will increase neutron-server processing time for
non-DVR scenario. Adding logic to judge whether given
agent is in DVR agent_mode, call get_sync_data directly.

Closes-Bug: #1510796
Change-Id: I5572e19b7cd8b2ea63fde4463705ab1c56fe5e68

neutron/db/l3_agentschedulers_db.py
neutron/db/l3_dvrscheduler_db.py
neutron/db/l3_hamode_db.py
neutron/tests/unit/db/test_l3_hamode_db.py
neutron/tests/unit/scheduler/test_l3_agent_scheduler.py

index a12f7840f1c686751bee8ce02222a49283ec0af3..3a2b21fa303fdc0ce0ef97a9986490cab386132b 100644 (file)
@@ -364,7 +364,7 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase,
                                                router_ids):
         if n_utils.is_extension_supported(self,
                                           constants.L3_HA_MODE_EXT_ALIAS):
-            return self.get_ha_sync_data_for_host(context, host,
+            return self.get_ha_sync_data_for_host(context, host, agent,
                                                   router_ids=router_ids,
                                                   active=True)
 
index ada1c799aae75b715d6f73b2c47e99f38e9a759b..826d4b8a545f448370b4033ba3b390c6edcd6da9 100644 (file)
@@ -383,7 +383,7 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
     def _get_active_l3_agent_routers_sync_data(self, context, host, agent,
                                                router_ids):
         if n_utils.is_extension_supported(self, n_const.L3_HA_MODE_EXT_ALIAS):
-            return self.get_ha_sync_data_for_host(context, host,
+            return self.get_ha_sync_data_for_host(context, host, agent,
                                                   router_ids=router_ids,
                                                   active=True)
         return self._get_dvr_sync_data(context, host, agent,
index 20b9c07d83d65219c8644e1f5e6fd558f5de24fc..14d07b96ad1f484bf57a2a08689b1c6e31fce552 100644 (file)
@@ -588,14 +588,14 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin,
 
         return list(routers_dict.values())
 
-    def get_ha_sync_data_for_host(self, context, host=None, router_ids=None,
-                                  active=None):
-        if n_utils.is_extension_supported(self,
-                                          constants.L3_DISTRIBUTED_EXT_ALIAS):
+    def get_ha_sync_data_for_host(self, context, host, agent,
+                                  router_ids=None, active=None):
+        agent_mode = self._get_agent_mode(agent)
+        dvr_agent_mode = (agent_mode in [constants.L3_AGENT_MODE_DVR_SNAT,
+                                         constants.L3_AGENT_MODE_DVR])
+        if (dvr_agent_mode and n_utils.is_extension_supported(
+                self, constants.L3_DISTRIBUTED_EXT_ALIAS)):
             # DVR has to be handled differently
-            agent = self._get_agent_by_type_and_host(context,
-                                                     constants.AGENT_TYPE_L3,
-                                                     host)
             sync_data = self._get_dvr_sync_data(context, host, agent,
                                                 router_ids, active)
         else:
index aec0cc8f3e312cc1ffa0be487fbf876c5e787a57..a416c31b8fcaf00c79e3a9d36b86e20e327e69df 100644 (file)
@@ -59,6 +59,7 @@ class L3HATestFramework(testlib_api.SqlTestCase):
         cfg.CONF.set_override('allow_overlapping_ips', True)
 
         self.plugin = FakeL3PluginWithAgents()
+        self.plugin.router_scheduler = l3_agent_scheduler.ChanceScheduler()
         self.agent1 = helpers.register_l3_agent()
         self.agent2 = helpers.register_l3_agent(
             'host_2', constants.L3_AGENT_MODE_DVR_SNAT)
@@ -95,9 +96,8 @@ class L3HATestFramework(testlib_api.SqlTestCase):
 
     def _bind_router(self, router_id):
         with self.admin_ctx.session.begin(subtransactions=True):
-            scheduler = l3_agent_scheduler.ChanceScheduler()
             agents_db = self.plugin.get_agents_db(self.admin_ctx)
-            scheduler._bind_ha_router_to_agents(
+            self.plugin.router_scheduler._bind_ha_router_to_agents(
                 self.plugin,
                 self.admin_ctx,
                 router_id,
@@ -287,11 +287,30 @@ class L3HATestCase(L3HATestFramework):
         self.assertEqual(0, len(bound_agents))
         self.assertEqual(2, mock_manager.call_count)
 
+    def test_get_ha_sync_data_for_host_with_non_dvr_agent(self):
+        with mock.patch.object(self.plugin,
+                               '_get_dvr_sync_data') as mock_get_sync:
+            self.plugin.supported_extension_aliases = ['dvr', 'l3-ha']
+            self.plugin.get_ha_sync_data_for_host(self.admin_ctx,
+                                                  self.agent1['host'],
+                                                  self.agent1)
+            self.assertFalse(mock_get_sync.called)
+
+    def test_get_ha_sync_data_for_host_with_dvr_agent(self):
+        with mock.patch.object(self.plugin,
+                               '_get_dvr_sync_data') as mock_get_sync:
+            self.plugin.supported_extension_aliases = ['dvr', 'l3-ha']
+            self.plugin.get_ha_sync_data_for_host(self.admin_ctx,
+                                                  self.agent2['host'],
+                                                  self.agent2)
+            self.assertTrue(mock_get_sync.called)
+
     def test_l3_agent_routers_query_interface(self):
         router = self._create_router()
         self._bind_router(router['id'])
         routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx,
-                                                        self.agent1['host'])
+                                                        self.agent1['host'],
+                                                        self.agent1)
         self.assertEqual(1, len(routers))
         router = routers[0]
 
@@ -318,8 +337,10 @@ class L3HATestCase(L3HATestFramework):
             ha_network1['network_id'], ha_network2['network_id'])
 
     def _deployed_router_change_ha_flag(self, to_ha):
-        self._create_router(ha=not to_ha)
-        routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx)
+        router1 = self._create_router(ha=not to_ha)
+        self._bind_router(router1['id'])
+        routers = self.plugin.get_ha_sync_data_for_host(
+            self.admin_ctx, self.agent1['host'], self.agent1)
         router = routers[0]
         interface = router.get(constants.HA_INTERFACE_KEY)
         if to_ha:
@@ -328,7 +349,9 @@ class L3HATestCase(L3HATestFramework):
             self.assertIsNotNone(interface)
 
         self._migrate_router(router['id'], to_ha)
-        routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx)
+        self.plugin.schedule_router(self.admin_ctx, router1['id'])
+        routers = self.plugin.get_ha_sync_data_for_host(
+            self.admin_ctx, self.agent1['host'], self.agent1)
         router = routers[0]
         interface = router.get(constants.HA_INTERFACE_KEY)
         if to_ha:
@@ -353,9 +376,12 @@ class L3HATestCase(L3HATestFramework):
         self.assertTrue(self.notif_m.called)
 
     def test_unique_vr_id_between_routers(self):
-        self._create_router()
-        self._create_router()
-        routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx)
+        router1 = self._create_router()
+        router2 = self._create_router()
+        self._bind_router(router1['id'])
+        self._bind_router(router2['id'])
+        routers = self.plugin.get_ha_sync_data_for_host(
+            self.admin_ctx, self.agent1['host'], self.agent1)
         self.assertEqual(2, len(routers))
         self.assertNotEqual(routers[0]['ha_vr_id'], routers[1]['ha_vr_id'])
 
@@ -365,9 +391,12 @@ class L3HATestCase(L3HATestFramework):
 
     @mock.patch('neutron.db.l3_hamode_db.VR_ID_RANGE', new=set(range(1, 2)))
     def test_vr_id_unique_range_per_tenant(self):
-        self._create_router()
-        self._create_router(tenant_id=_uuid())
-        routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx)
+        router1 = self._create_router()
+        router2 = self._create_router(tenant_id=_uuid())
+        self._bind_router(router1['id'])
+        self._bind_router(router2['id'])
+        routers = self.plugin.get_ha_sync_data_for_host(
+            self.admin_ctx, self.agent1['host'], self.agent1)
         self.assertEqual(2, len(routers))
         self.assertEqual(routers[0]['ha_vr_id'], routers[1]['ha_vr_id'])
 
@@ -415,9 +444,12 @@ class L3HATestCase(L3HATestFramework):
         self.assertEqual(allocs_before, allocs_after)
 
     def test_one_ha_router_one_not(self):
-        self._create_router(ha=False)
-        self._create_router()
-        routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx)
+        router1 = self._create_router(ha=False)
+        router2 = self._create_router()
+        self._bind_router(router1['id'])
+        self._bind_router(router2['id'])
+        routers = self.plugin.get_ha_sync_data_for_host(
+            self.admin_ctx, self.agent1['host'], self.agent1)
 
         ha0 = routers[0]['ha']
         ha1 = routers[1]['ha']
@@ -522,8 +554,8 @@ class L3HATestCase(L3HATestFramework):
         router2 = self._create_router()
         self._bind_router(router2['id'])
 
-        routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx,
-                                                        self.agent1['host'])
+        routers = self.plugin.get_ha_sync_data_for_host(
+            self.admin_ctx, self.agent1['host'], self.agent1)
         for router in routers:
             self.assertEqual('standby', router[constants.HA_ROUTER_STATE_KEY])
 
@@ -532,8 +564,8 @@ class L3HATestCase(L3HATestFramework):
         self.plugin.update_routers_states(
             self.admin_ctx, states, self.agent1['host'])
 
-        routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx,
-                                                        self.agent1['host'])
+        routers = self.plugin.get_ha_sync_data_for_host(
+            self.admin_ctx, self.agent1['host'], self.agent1)
         for router in routers:
             self.assertEqual(states[router['id']],
                              router[constants.HA_ROUTER_STATE_KEY])
@@ -549,8 +581,8 @@ class L3HATestCase(L3HATestFramework):
         self.plugin._set_router_states(
             self.admin_ctx, bindings, {router1['id']: 'active',
                                        router2['id']: 'active'})
-        routers = self.plugin.get_ha_sync_data_for_host(self.admin_ctx,
-                                                        self.agent1['host'])
+        routers = self.plugin.get_ha_sync_data_for_host(
+            self.admin_ctx, self.agent1['host'], self.agent1)
         self.assertEqual('active', routers[0][constants.HA_ROUTER_STATE_KEY])
 
     def test_exclude_dvr_agents_for_ha_candidates(self):
index db991108ad5cac21ab40659aac5315fc27c3bedd..a13dc2a1a336959f5b236049e36eaa8bf17ec856 100644 (file)
@@ -1646,7 +1646,7 @@ class L3HAChanceSchedulerTestCase(L3HATestCaseMixin):
         for agent in agents:
             sync_data = self.plugin.get_ha_sync_data_for_host(
                 self.adminContext, router_ids=[router['id']],
-                host=agent.host)
+                host=agent.host, agent=agent)
             self.assertEqual(1, len(sync_data))
             interface = sync_data[0][constants.HA_INTERFACE_KEY]
             self.assertIsNotNone(interface)