Fixes Multiple External Networks issue with DVR
Current L3 agents can support more than one
external network when configured properly.
On DVR routers, router-gateway-set was
returning a 500 error, when two external
networks were configured in the system.
The problem resides in the scheduler where the
bind_router is called twice when the
reschedule_router is called from update_router.
The _schedule_router binds the snat
and the qrouter with the respective agents.
But after scheduling it does not return agent.
And in the case of two external networks, the
get_candidates always returns a valid candidate
to be processed and hence the bind_router is
called twice.
This patch fixes the _schedule_router function
and hence avoids the multiple calls to
bind_router.
This prevents the update_router from failing
and causing the nested rollback for the
transactions.
Conflicts:
neutron/tests/unit/test_l3_schedulers.py
(cherry picked from commit 3794b4a83e
)
Change-Id: I24d44c60a3ea5bbc9e3f44aa5191deff315723ca
Closes-Bug: #1374473
This commit is contained in:
parent
ea63062898
commit
884013c8cd
|
@ -304,3 +304,4 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
|
|||
context, router_id, snat_candidates)
|
||||
self.bind_dvr_router_servicenode(
|
||||
context, router_id, chosen_agent)
|
||||
return chosen_agent
|
||||
|
|
|
@ -237,12 +237,14 @@ 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
|
||||
plugin.schedule_snat_router(context, router_id, sync_router)
|
||||
return plugin.schedule_snat_router(
|
||||
context, router_id, sync_router)
|
||||
if 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:
|
||||
|
|
|
@ -466,18 +466,12 @@ class L3SchedulerTestBaseMixin(object):
|
|||
sync_router = {'id': 'foo_router_id',
|
||||
'distributed': True}
|
||||
plugin.get_router.return_value = sync_router
|
||||
with contextlib.nested(
|
||||
mock.patch.object(scheduler, 'bind_router'),
|
||||
mock.patch.object(plugin, 'get_snat_bindings', return_value=True)):
|
||||
with mock.patch.object(plugin, 'get_snat_bindings', return_value=True):
|
||||
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]),
|
||||
]
|
||||
plugin.assert_has_calls(expected_calls)
|
||||
|
||||
|
@ -492,21 +486,14 @@ class L3SchedulerTestBaseMixin(object):
|
|||
}
|
||||
}
|
||||
plugin.get_router.return_value = sync_router
|
||||
with contextlib.nested(
|
||||
mock.patch.object(scheduler, 'bind_router'),
|
||||
mock.patch.object(
|
||||
plugin, 'get_snat_bindings', return_value=False)
|
||||
):
|
||||
with mock.patch.object(
|
||||
plugin, 'get_snat_bindings', return_value=False):
|
||||
scheduler._schedule_router(
|
||||
plugin, self.adminContext, 'foo_router_id', None)
|
||||
expected_calls = [
|
||||
mock.call.get_router(mock.ANY, 'foo_router_id'),
|
||||
mock.call.schedule_snat_router(
|
||||
mock.ANY, 'foo_router_id', sync_router),
|
||||
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]),
|
||||
]
|
||||
plugin.assert_has_calls(expected_calls)
|
||||
|
||||
|
@ -1242,6 +1229,21 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase,
|
|||
}
|
||||
return agent, router
|
||||
|
||||
def test_schedule_snat_router_return_value(self):
|
||||
agent, router = self._prepare_schedule_snat_tests()
|
||||
with contextlib.nested(
|
||||
mock.patch.object(self.dut, 'get_l3_agents'),
|
||||
mock.patch.object(self.dut, 'get_snat_candidates'),
|
||||
mock.patch.object(self.dut, 'bind_snat_servicenode'),
|
||||
mock.patch.object(self.dut, 'bind_dvr_router_servicenode')
|
||||
) as (mock_gl3, mock_snat_canidates, mock_bind_snat, mock_bind_dvr):
|
||||
mock_snat_canidates.return_value = [agent]
|
||||
mock_bind_snat.return_value = [agent]
|
||||
mock_bind_dvr.return_value = [agent]
|
||||
chosen_agent = self.dut.schedule_snat_router(
|
||||
self.adminContext, 'foo_router_id', router)
|
||||
self.assertEqual(chosen_agent, [agent])
|
||||
|
||||
def test_schedule_router_unbind_snat_servicenode_negativetest(self):
|
||||
router = {
|
||||
'id': 'foo_router_id',
|
||||
|
|
Loading…
Reference in New Issue