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 3794b4a83e
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
This commit is contained in:
parent
985f6ed968
commit
236e408272
@ -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)
|
||||
|
@ -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):
|
||||
|
@ -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"]
|
||||
|
||||
|
@ -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,\
|
||||
|
@ -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),
|
||||
]
|
||||
|
Loading…
Reference in New Issue
Block a user