From: lzklibj Date: Wed, 28 Oct 2015 09:02:11 +0000 (+0800) Subject: fix get_ha_sync_data_for_host for non-dvr agent X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=294324d6970980e8f95f441727204c66478b4684;p=openstack-build%2Fneutron-build.git fix get_ha_sync_data_for_host for non-dvr agent 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 --- diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py index a12f7840f..3a2b21fa3 100644 --- a/neutron/db/l3_agentschedulers_db.py +++ b/neutron/db/l3_agentschedulers_db.py @@ -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) diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py index ada1c799a..826d4b8a5 100644 --- a/neutron/db/l3_dvrscheduler_db.py +++ b/neutron/db/l3_dvrscheduler_db.py @@ -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, diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py index 20b9c07d8..14d07b96a 100644 --- a/neutron/db/l3_hamode_db.py +++ b/neutron/db/l3_hamode_db.py @@ -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: diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index aec0cc8f3..a416c31b8 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/neutron/tests/unit/db/test_l3_hamode_db.py @@ -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): diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py index db991108a..a13dc2a1a 100644 --- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py +++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py @@ -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)