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 commit3794b4a83e
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 (cherry picked from commit236e408272
) Conflicts: neutron/scheduler/l3_agent_scheduler.py neutron/tests/unit/extensions/test_agent.py neutron/tests/unit/plugins/openvswitch/test_agent_scheduler.py Change-Id: I57c06e2be732e47b6cce7c724f6b255ea2d8fa32
This commit is contained in:
parent
1578dcbb8d
commit
bc43214239
|
@ -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)
|
||||
|
|
|
@ -160,7 +160,8 @@ class AgentDBTestMixIn(object):
|
|||
return [dhcp_host]
|
||||
|
||||
def _register_one_l3_agent(self, host=L3_HOSTA, internal_only=True,
|
||||
ext_net_id='', ext_bridge=''):
|
||||
ext_net_id='', ext_bridge='',
|
||||
agent_mode=constants.L3_AGENT_MODE_LEGACY):
|
||||
l3 = {
|
||||
'binary': 'neutron-l3-agent',
|
||||
'host': host,
|
||||
|
@ -171,6 +172,7 @@ class AgentDBTestMixIn(object):
|
|||
'external_network_bridge': ext_bridge,
|
||||
'gateway_external_network_id': ext_net_id,
|
||||
'interface_driver': 'interface_driver',
|
||||
'agent_mode': agent_mode,
|
||||
},
|
||||
'agent_type': constants.AGENT_TYPE_L3}
|
||||
callback = agents_db.AgentExtRpcCallback()
|
||||
|
@ -178,6 +180,13 @@ class AgentDBTestMixIn(object):
|
|||
agent_state={'agent_state': l3},
|
||||
time=timeutils.strtime())
|
||||
|
||||
def _register_dvr_agents(self):
|
||||
dvr_snat_agent = self._register_one_l3_agent(
|
||||
host=L3_HOSTA, agent_mode=constants.L3_AGENT_MODE_DVR_SNAT)
|
||||
dvr_agent = self._register_one_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):
|
||||
|
|
|
@ -39,6 +39,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
|
||||
|
@ -300,8 +301,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"]
|
||||
|
||||
|
|
|
@ -230,9 +230,8 @@ class OvsAgentSchedulerTestCaseBase(test_l3.L3NatTestCaseMixin,
|
|||
attributes.RESOURCE_ATTRIBUTE_MAP.update(
|
||||
agent.RESOURCE_ATTRIBUTE_MAP)
|
||||
self.addCleanup(self.restore_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()
|
||||
|
@ -1079,11 +1078,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 contextlib.nested(
|
||||
self.subnet(),
|
||||
|
|
|
@ -536,12 +536,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)
|
||||
|
||||
|
@ -557,11 +563,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),
|
||||
]
|
||||
|
|
Loading…
Reference in New Issue