From 236e408272bcb9b8e957524864e571b5afdc4623 Mon Sep 17 00:00:00 2001 From: Oleg Bondarev Date: Tue, 7 Jul 2015 12:02:58 +0300 Subject: [PATCH] DVR: fix router scheduling Fix scheduling of DVR routers to not stop scheduling once csnat portion was scheduled. See bug report for failing scenario. This partially reverts commit 3794b4a83e68041e24b715135f0ccf09a5631178 and fixes bug 1374473 by moving csnat scheduling after general dvr router scheduling, so double binding does not happen. Closes-Bug: #1472163 Related-Bug: #1374473 Change-Id: I57c06e2be732e47b6cce7c724f6b255ea2d8fa32 --- neutron/scheduler/l3_agent_scheduler.py | 20 +++++------ neutron/tests/unit/extensions/test_agent.py | 7 ++++ neutron/tests/unit/extensions/test_l3.py | 5 +-- .../openvswitch/agent/test_agent_scheduler.py | 33 ++++++++++++++++--- .../unit/scheduler/test_l3_agent_scheduler.py | 25 ++++++++++---- 5 files changed, 67 insertions(+), 23 deletions(-) diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index 223a54874..ae7adbf38 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/neutron/scheduler/l3_agent_scheduler.py @@ -232,8 +232,16 @@ class L3Scheduler(object): def _schedule_router(self, plugin, context, router_id, candidates=None): sync_router = plugin.get_router(context, router_id) + candidates = candidates or self._get_candidates( + plugin, context, sync_router) + if not candidates: + return + router_distributed = sync_router.get('distributed', False) if router_distributed: + for chosen_agent in candidates: + self.bind_router(context, router_id, chosen_agent) + # For Distributed routers check for SNAT Binding before # calling the schedule_snat_router snat_bindings = plugin.get_snat_bindings(context, [router_id]) @@ -241,21 +249,13 @@ class L3Scheduler(object): if not snat_bindings and router_gw_exists: # If GW exists for DVR routers and no SNAT binding # call the schedule_snat_router - return plugin.schedule_snat_router( + plugin.schedule_snat_router( context, router_id, sync_router) - if not router_gw_exists and snat_bindings: + elif not router_gw_exists and snat_bindings: # If DVR router and no Gateway but SNAT Binding exists then # call the unbind_snat_servicenode to unbind the snat service # from agent plugin.unbind_snat_servicenode(context, router_id) - return - candidates = candidates or self._get_candidates( - plugin, context, sync_router) - if not candidates: - return - if router_distributed: - for chosen_agent in candidates: - self.bind_router(context, router_id, chosen_agent) elif sync_router.get('ha', False): chosen_agents = self._bind_ha_router(plugin, context, router_id, candidates) diff --git a/neutron/tests/unit/extensions/test_agent.py b/neutron/tests/unit/extensions/test_agent.py index 79397a5fe..21fa1022b 100644 --- a/neutron/tests/unit/extensions/test_agent.py +++ b/neutron/tests/unit/extensions/test_agent.py @@ -118,6 +118,13 @@ class AgentDBTestMixIn(object): return res + def _register_dvr_agents(self): + dvr_snat_agent = helpers.register_l3_agent( + host=L3_HOSTA, agent_mode=constants.L3_AGENT_MODE_DVR_SNAT) + dvr_agent = helpers.register_l3_agent( + host=L3_HOSTB, agent_mode=constants.L3_AGENT_MODE_DVR) + return [dvr_snat_agent, dvr_agent] + class AgentDBTestCase(AgentDBTestMixIn, test_db_base_plugin_v2.NeutronDbPluginV2TestCase): diff --git a/neutron/tests/unit/extensions/test_l3.py b/neutron/tests/unit/extensions/test_l3.py index ba391320c..77e55682b 100644 --- a/neutron/tests/unit/extensions/test_l3.py +++ b/neutron/tests/unit/extensions/test_l3.py @@ -40,6 +40,7 @@ from neutron.db import l3_agentschedulers_db from neutron.db import l3_attrs_db from neutron.db import l3_db from neutron.db import l3_dvr_db +from neutron.db import l3_dvrscheduler_db from neutron.extensions import external_net from neutron.extensions import l3 from neutron.extensions import portbindings @@ -301,8 +302,8 @@ class TestL3NatServicePlugin(common_db_mixin.CommonDbMixin, # A L3 routing with L3 agent scheduling service plugin class for tests with # plugins that delegate away L3 routing functionality class TestL3NatAgentSchedulingServicePlugin(TestL3NatServicePlugin, - l3_agentschedulers_db. - L3AgentSchedulerDbMixin): + l3_dvrscheduler_db. + L3_DVRsch_db_mixin): supported_extension_aliases = ["router", "l3_agent_scheduler"] diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py index dc406d800..e512b102f 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py @@ -240,9 +240,8 @@ class OvsAgentSchedulerTestCaseBase(test_l3.L3NatTestCaseMixin, # the global attribute map attributes.RESOURCE_ATTRIBUTE_MAP.update( agent.RESOURCE_ATTRIBUTE_MAP) - self.l3agentscheduler_dbMinxin = ( - manager.NeutronManager.get_service_plugins().get( - service_constants.L3_ROUTER_NAT)) + self.l3plugin = manager.NeutronManager.get_service_plugins().get( + service_constants.L3_ROUTER_NAT) self.l3_notify_p = mock.patch( 'neutron.extensions.l3agentscheduler.notify') self.patched_l3_notify = self.l3_notify_p.start() @@ -967,11 +966,37 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase): res = router_req.get_response(self.ext_api) router = self.deserialize(self.fmt, res) l3agents = ( - self.l3agentscheduler_dbMinxin.get_l3_agents_hosting_routers( + self.l3plugin.get_l3_agents_hosting_routers( self.adminContext, [router['router']['id']])) self._delete('routers', router['router']['id']) self.assertEqual(0, len(l3agents)) + def test_dvr_router_scheduling_to_all_needed_agents(self): + self._register_dvr_agents() + with self.subnet() as s: + net_id = s['subnet']['network_id'] + self._set_net_external(net_id) + + router = {'name': 'router1', + 'external_gateway_info': {'network_id': net_id}, + 'admin_state_up': True, + 'distributed': True} + r = self.l3plugin.create_router(self.adminContext, + {'router': router}) + with mock.patch.object( + self.l3plugin, + 'check_ports_exist_on_l3agent') as ports_exist: + # emulating dvr serviceable ports exist on compute node + ports_exist.return_value = True + self.l3plugin.schedule_router( + self.adminContext, r['id']) + + l3agents = self._list_l3_agents_hosting_router(r['id']) + self.assertEqual(2, len(l3agents['agents'])) + self.assertEqual({'dvr', 'dvr_snat'}, + set([a['configurations']['agent_mode'] for a in + l3agents['agents']])) + def test_router_sync_data(self): with self.subnet() as s1,\ self.subnet(cidr='10.0.2.0/24') as s2,\ diff --git a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py index a8f5c15dd..8b4c546a9 100644 --- a/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py +++ b/neutron/tests/unit/scheduler/test_l3_agent_scheduler.py @@ -489,12 +489,18 @@ class L3SchedulerTestBaseMixin(object): sync_router = {'id': 'foo_router_id', 'distributed': True} plugin.get_router.return_value = sync_router - with mock.patch.object(plugin, 'get_snat_bindings', return_value=True): - scheduler._schedule_router( - plugin, self.adminContext, 'foo_router_id', None) + with mock.patch.object( + plugin, 'get_snat_bindings', return_value=True),\ + mock.patch.object(scheduler, 'bind_router'): + scheduler._schedule_router( + plugin, self.adminContext, 'foo_router_id', None) expected_calls = [ mock.call.get_router(mock.ANY, 'foo_router_id'), - mock.call.unbind_snat_servicenode(mock.ANY, 'foo_router_id'), + mock.call.get_l3_agents_hosting_routers( + mock.ANY, ['foo_router_id'], admin_state_up=True), + mock.call.get_l3_agents(mock.ANY, active=True), + mock.call.get_l3_agent_candidates(mock.ANY, sync_router, [agent]), + mock.call.unbind_snat_servicenode(mock.ANY, 'foo_router_id') ] plugin.assert_has_calls(expected_calls) @@ -510,11 +516,16 @@ class L3SchedulerTestBaseMixin(object): } plugin.get_router.return_value = sync_router with mock.patch.object( - plugin, 'get_snat_bindings', return_value=False): - scheduler._schedule_router( - plugin, self.adminContext, 'foo_router_id', None) + plugin, 'get_snat_bindings', return_value=False),\ + mock.patch.object(scheduler, 'bind_router'): + scheduler._schedule_router( + plugin, self.adminContext, 'foo_router_id', None) expected_calls = [ mock.call.get_router(mock.ANY, 'foo_router_id'), + mock.call.get_l3_agents_hosting_routers( + mock.ANY, ['foo_router_id'], admin_state_up=True), + mock.call.get_l3_agents(mock.ANY, active=True), + mock.call.get_l3_agent_candidates(mock.ANY, sync_router, [agent]), mock.call.schedule_snat_router( mock.ANY, 'foo_router_id', sync_router), ] -- 2.45.2