x/vmware-nsx
Code Issues Proposed changes

Call unbind_snat_servicenode from schedule router

Browse Source 
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 38b5d74309
commit 1e04e36b36
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 import sqlalchemy as sa
from sqlalchemy import orm from sqlalchemy import orm
from sqlalchemy.orm import exc from sqlalchemy.orm import exc
from sqlalchemy.orm import joinedload
from neutron.common import constants as q_const from neutron.common import constants as q_const
from neutron.common import utils as n_utils 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 ' LOG.debug('Removed binding for router %(router_id)s and '
'agent %(id)s', {'router_id': router_id, 'id': agent_id}) '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.""" """Schedule the snat router on l3 service agent."""
if gw_exists: binding = (context.session.
binding = (context.session. query(CentralizedSnatL3AgentBinding).
query(CentralizedSnatL3AgentBinding). filter_by(router_id=router_id).first())
filter_by(router_id=router_id).first()) if binding:
if binding: l3_agent_id = binding.l3_agent_id
l3_agent_id = binding.l3_agent_id l3_agent = binding.l3_agent
l3_agent = binding.l3_agent LOG.debug('SNAT Router %(router_id)s has already been '
LOG.debug('SNAT Router %(router_id)s has already been ' 'hosted by L3 agent '
'hosted by L3 agent ' '%(l3_agent_id)s', {'router_id': router_id,
'%(l3_agent_id)s', {'router_id': router_id, 'l3_agent_id': l3_agent_id})
'l3_agent_id': l3_agent_id}) self.bind_dvr_router_servicenode(context, router_id, l3_agent)
self.bind_dvr_router_servicenode(context, router_id, l3_agent) return
return active_l3_agents = self.get_l3_agents(context, active=True)
active_l3_agents = self.get_l3_agents(context, active=True) if not active_l3_agents:
if not active_l3_agents: LOG.warn(_('No active L3 agents found for SNAT'))
LOG.warn(_('No active L3 agents')) return
return snat_candidates = self.get_snat_candidates(sync_router,
snat_candidates = self.get_snat_candidates(sync_router, active_l3_agents)
active_l3_agents) if snat_candidates:
if snat_candidates: self.bind_snat_servicenode(context, router_id, snat_candidates)
self.bind_snat_servicenode(context, router_id, snat_candidates)
else:
self.unbind_snat_servicenode(context, router_id)

22
neutron/scheduler/l3_agent_scheduler.py
View File

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

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

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