From 09cad21208ca1745d2859aabeb43c5b028f227b6 Mon Sep 17 00:00:00 2001 From: Oleg Bondarev Date: Thu, 20 Aug 2015 12:02:55 +0300 Subject: [PATCH] DVR: make sure snat portion is always scheduled when needed commit 236e408272bcb9b8e957524864e571b5afdc4623 introduced a regression where if router without external gateway was already scheduled to all dvr_snat agents, then when adding external gateway to the router, snat portion scheduling was skipped. The patch fixes regression and adds corresponding unit (functional in fact) test. Closes-Bug: #1486627 Change-Id: Iad7e53bd57836f257d7110bc054d58029484ab99 --- neutron/scheduler/l3_agent_scheduler.py | 11 +++-- .../openvswitch/agent/test_agent_scheduler.py | 42 +++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index ae7adbf38ea..e41a34968d7 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/neutron/scheduler/l3_agent_scheduler.py @@ -234,11 +234,8 @@ class L3Scheduler(object): 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: + chosen_agent = None + if sync_router.get('distributed', False): for chosen_agent in candidates: self.bind_router(context, router_id, chosen_agent) @@ -249,13 +246,15 @@ 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( + chosen_agent = plugin.schedule_snat_router( context, router_id, sync_router) 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) + elif not candidates: + return elif sync_router.get('ha', False): chosen_agents = self._bind_ha_router(plugin, context, router_id, candidates) diff --git a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py index a250f402841..1633afd39b6 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py +++ b/neutron/tests/unit/plugins/ml2/drivers/openvswitch/agent/test_agent_scheduler.py @@ -1021,6 +1021,48 @@ class OvsAgentSchedulerTestCase(OvsAgentSchedulerTestCaseBase): set([a['configurations']['agent_mode'] for a in l3agents['agents']])) + def test_dvr_router_snat_scheduling_late_ext_gw_add(self): + """Test snat scheduling for the case when dvr router is already + scheduled to all dvr_snat agents and then external gateway is added. + """ + helpers.register_l3_agent( + host=L3_HOSTA, agent_mode=constants.L3_AGENT_MODE_DVR_SNAT) + helpers.register_l3_agent( + host=L3_HOSTB, agent_mode=constants.L3_AGENT_MODE_DVR_SNAT) + with self.subnet() as s_int,\ + self.subnet(cidr='20.0.0.0/24') as s_ext: + net_id = s_ext['subnet']['network_id'] + self._set_net_external(net_id) + + router = {'name': 'router1', + 'admin_state_up': True, + 'distributed': True} + r = self.l3plugin.create_router(self.adminContext, + {'router': router}) + # add router interface first + self.l3plugin.add_router_interface(self.adminContext, r['id'], + {'subnet_id': s_int['subnet']['id']}) + # check that router is scheduled to both dvr_snat agents + l3agents = self._list_l3_agents_hosting_router(r['id']) + self.assertEqual(2, len(l3agents['agents'])) + # check that snat is not scheduled as router is not connected to + # external network + snat_agents = self.l3plugin.get_snat_bindings( + self.adminContext, [r['id']]) + self.assertEqual(0, len(snat_agents)) + + # connect router to external network + self.l3plugin.update_router(self.adminContext, r['id'], + {'router': {'external_gateway_info': {'network_id': net_id}}}) + # router should still be scheduled to both dvr_snat agents + l3agents = self._list_l3_agents_hosting_router(r['id']) + self.assertEqual(2, len(l3agents['agents'])) + # now snat portion should be scheduled as router is connected + # to external network + snat_agents = self.l3plugin.get_snat_bindings( + self.adminContext, [r['id']]) + self.assertEqual(1, len(snat_agents)) + def test_dvr_router_csnat_rescheduling(self): helpers.register_l3_agent( host=L3_HOSTA, agent_mode=constants.L3_AGENT_MODE_DVR_SNAT)