openstack
/
neutron
Code Issues Proposed changes

Call unbind_snat_servicenode from schedule router 

Refactor to move the call to plugin.unbind_snat_servicenode from
schedule_snat_router to _schedule_router.  This is a move to pave the
way for removing hints from schedule router.

Partial-bug: #1353266
Partial-bug: #1356639
Co-Authored-By: Swaminathan Vasudevan <swaminathan.vasudevan@hp.com>

Change-Id: I062d8cc3cb870bbaa033d5b107a7dd868dfafa19
This commit is contained in:
Michael Smith 2014-09-02 17:07:04 +00:00 committed by Carl Baldwin
parent f31a56a06c
commit 671e3ffa46
3 changed files with 75 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
neutron/db/l3_dvrscheduler_db.py
View File

@ -18,6 +18,7 @@ import random
import sqlalchemy as sa
from sqlalchemy import orm
from sqlalchemy.orm import exc
from sqlalchemy.orm import joinedload
from neutron.common import constants as q_const
from neutron.common import utils as n_utils
@ -278,28 +279,34 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
LOG.debug('Removed binding for router %(router_id)s and '
'agent %(id)s', {'router_id': router_id, 'id': agent_id})
def schedule_snat_router(self, context, router_id, sync_router, gw_exists):
def get_snat_bindings(self, context, router_ids):
""" Retrieves the dvr snat bindings for a router."""
if not router_ids:
return []
query = context.session.query(CentralizedSnatL3AgentBinding)
query = query.options(joinedload('l3_agent')).filter(
CentralizedSnatL3AgentBinding.router_id.in_(router_ids))
return query.all()
def schedule_snat_router(self, context, router_id, sync_router):
"""Schedule the snat router on l3 service agent."""
if gw_exists:
binding = (context.session.
query(CentralizedSnatL3AgentBinding).
filter_by(router_id=router_id).first())
if binding:
l3_agent_id = binding.l3_agent_id
l3_agent = binding.l3_agent
LOG.debug('SNAT Router %(router_id)s has already been '
'hosted by L3 agent '
'%(l3_agent_id)s', {'router_id': router_id,
'l3_agent_id': l3_agent_id})
self.bind_dvr_router_servicenode(context, router_id, l3_agent)
return
active_l3_agents = self.get_l3_agents(context, active=True)
if not active_l3_agents:
LOG.warn(_('No active L3 agents'))
return
snat_candidates = self.get_snat_candidates(sync_router,
active_l3_agents)
if snat_candidates:
self.bind_snat_servicenode(context, router_id, snat_candidates)
else:
self.unbind_snat_servicenode(context, router_id)
binding = (context.session.
query(CentralizedSnatL3AgentBinding).
filter_by(router_id=router_id).first())
if binding:
l3_agent_id = binding.l3_agent_id
l3_agent = binding.l3_agent
LOG.debug('SNAT Router %(router_id)s has already been '
'hosted by L3 agent '
'%(l3_agent_id)s', {'router_id': router_id,
'l3_agent_id': l3_agent_id})
self.bind_dvr_router_servicenode(context, router_id, l3_agent)
return
active_l3_agents = self.get_l3_agents(context, active=True)
if not active_l3_agents:
LOG.warn(_('No active L3 agents found for SNAT'))
return
snat_candidates = self.get_snat_candidates(sync_router,
active_l3_agents)
if snat_candidates:
self.bind_snat_servicenode(context, router_id, snat_candidates)

22
neutron/scheduler/l3_agent_scheduler.py
View File

@ -200,15 +200,27 @@ class L3Scheduler(object):
def _schedule_router(self, plugin, context, router_id,
candidates=None, hints=None):
sync_router = plugin.get_router(context, router_id)
if (hints and 'gw_exists' in hints
and sync_router.get('distributed', False)):
plugin.schedule_snat_router(
context, router_id, sync_router, hints['gw_exists'])
router_distributed = sync_router.get('distributed', False)
if router_distributed:
# For Distributed routers check for SNAT Binding before
# calling the schedule_snat_router
snat_bindings = plugin.get_snat_bindings(context, [router_id])
router_gw_exists = (hints and 'gw_exists' in hints
and hints['gw_exists'])
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)
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)
candidates = candidates or self.get_candidates(
plugin, context, sync_router)
if not candidates:
return
if sync_router.get('distributed', False):
if router_distributed:
for chosen_agent in candidates:
self.bind_router(context, router_id, chosen_agent)
else:

44
neutron/tests/unit/test_l3_schedulers.py
View File

@ -364,10 +364,13 @@ class L3SchedulerTestCase(l3_agentschedulers_db.L3AgentSchedulerDbMixin,
'distributed': True
}
plugin.get_router.return_value = sync_router
with mock.patch.object(scheduler, 'bind_router'):
scheduler._schedule_router(
plugin, self.adminContext,
'foo_router_id', None)
with contextlib.nested(
mock.patch.object(scheduler, 'bind_router'),
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.get_l3_agents_hosting_routers(
@ -382,12 +385,14 @@ class L3SchedulerTestCase(l3_agentschedulers_db.L3AgentSchedulerDbMixin,
sync_router = {'id': 'foo_router_id',
'distributed': True}
plugin.get_router.return_value = sync_router
with mock.patch.object(scheduler, 'bind_router'):
scheduler._schedule_router(
plugin, self.adminContext,
'foo_router_id', None)
with contextlib.nested(
mock.patch.object(scheduler, 'bind_router'),
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),
@ -406,10 +411,13 @@ class L3SchedulerTestCase(l3_agentschedulers_db.L3AgentSchedulerDbMixin,
}
}
plugin.get_router.return_value = sync_router
with mock.patch.object(scheduler, 'bind_router'):
scheduler._schedule_router(
plugin, self.adminContext,
'foo_router_id', None)
with contextlib.nested(
mock.patch.object(scheduler, 'bind_router'),
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.get_l3_agents_hosting_routers(
@ -877,7 +885,7 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase,
mock_agents.return_value = [agent]
mock_candidates.return_value = [agent]
self.dut.schedule_snat_router(
self.adminContext, 'foo_router_id', router, True)
self.adminContext, 'foo_router_id', router)
self.assertFalse(mock_dvr.called)
def test_schedule_router_unbind_snat_servicenode_negativetest(self):
@ -887,11 +895,13 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase,
}
with contextlib.nested(
mock.patch.object(self.dut, 'get_router'),
mock.patch.object(self.dut, 'unbind_snat_servicenode')) as (
mock_rd, mock_unbind):
mock.patch.object(self.dut, 'get_snat_bindings'),
mock.patch.object(self.dut, 'unbind_snat_servicenode')
) as (mock_rd, mock_snat_bind, mock_unbind):
mock_rd.return_value = router
mock_snat_bind.return_value = False
self.dut.schedule_snat_router(
self.adminContext, 'foo_router_id', router, True)
self.adminContext, 'foo_router_id', router)
self.assertFalse(mock_unbind.called)
def test_schedule_snat_router_with_snat_candidates(self):
@ -910,7 +920,7 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase,
mock_agents.return_value = [agent]
mock_candidates.return_value = [agent]
self.dut.schedule_snat_router(
self.adminContext, 'foo_router_id', mock.ANY, True)
self.adminContext, 'foo_router_id', mock.ANY)
mock_bind.assert_called_once_with(
self.adminContext, 'foo_router_id', [agent])