From f78ce6782a57d6fd63c6e1dabd6a45cbd5c458cc Mon Sep 17 00:00:00 2001 From: Yoni Shafrir Date: Wed, 21 Jan 2015 08:25:50 +0200 Subject: [PATCH] Removing a router twice from the same agent shouldn't cause an error When we remove a router from an agent that has already been unscheduled from we raise an exception that eventually causes an error. The method '_unbind_router' raises a 'RouterNotHostedByL3Agent' exception on failure. In both cases the actual removal of the router from the same agent has no effect. The solution is to stop raising 'RouterNotHostedByL3Agent' so that _unbind_router() being invoked without error can indicate that the router is no longer bound. This solution matches the behaviour found when trying to schedule a router to the same agent twice. This change is a result of the discussion in: https://review.openstack.org/#/c/144681/2 Closes-Bug: #1406705 Change-Id: I015bfc0fde11ba4f39417e4c134faa8132cb3eac --- neutron/db/l3_agentschedulers_db.py | 8 +------- neutron/extensions/l3agentscheduler.py | 5 ----- neutron/plugins/ml2/plugin.py | 11 ++--------- neutron/tests/unit/ml2/test_ml2_plugin.py | 6 +----- neutron/tests/unit/test_l3_schedulers.py | 14 ++++++++++++++ 5 files changed, 18 insertions(+), 26 deletions(-) diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py index 444a724a697..88da19aed0c 100644 --- a/neutron/db/l3_agentschedulers_db.py +++ b/neutron/db/l3_agentschedulers_db.py @@ -19,7 +19,6 @@ import sqlalchemy as sa from sqlalchemy import func from sqlalchemy import or_ from sqlalchemy import orm -from sqlalchemy.orm import exc from sqlalchemy.orm import joinedload from sqlalchemy import sql @@ -228,12 +227,7 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, query = query.filter( RouterL3AgentBinding.router_id == router_id, RouterL3AgentBinding.l3_agent_id == agent_id) - try: - binding = query.one() - except exc.NoResultFound: - raise l3agentscheduler.RouterNotHostedByL3Agent( - router_id=router_id, agent_id=agent_id) - context.session.delete(binding) + query.delete() def reschedule_router(self, context, router_id, candidates=None): """Reschedule router to a new l3 agent diff --git a/neutron/extensions/l3agentscheduler.py b/neutron/extensions/l3agentscheduler.py index d9beb6e8835..f18a7abdb31 100644 --- a/neutron/extensions/l3agentscheduler.py +++ b/neutron/extensions/l3agentscheduler.py @@ -172,11 +172,6 @@ class RouterReschedulingFailed(exceptions.Conflict): "no eligible l3 agent found.") -class RouterNotHostedByL3Agent(exceptions.Conflict): - message = _("The router %(router_id)s is not hosted" - " by L3 agent %(agent_id)s.") - - class RouterL3AgentMismatch(exceptions.Conflict): message = _("Cannot host %(router_type)s router %(router_id)s " "on %(agent_mode)s L3 agent %(agent_id)s.") diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index 58787aba9f1..7f273a36d26 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -51,7 +51,6 @@ from neutron.db import quota_db # noqa from neutron.db import securitygroups_rpc_base as sg_db_rpc from neutron.extensions import allowedaddresspairs as addr_pair from neutron.extensions import extra_dhcp_opt as edo_ext -from neutron.extensions import l3agentscheduler from neutron.extensions import portbindings from neutron.extensions import providernet as provider from neutron.i18n import _LE, _LI, _LW @@ -1149,14 +1148,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2, l3plugin.dvr_vmarp_table_update(context, port, "del") l3plugin.notify_routers_updated(context, router_ids) for router in removed_routers: - try: - l3plugin.remove_router_from_l3_agent( - context, router['agent_id'], router['router_id']) - except l3agentscheduler.RouterNotHostedByL3Agent: - # router may have been removed by another process - LOG.debug("Router %(id)s not hosted by L3 agent %(agent)s", - {'id': router['router_id'], - 'agent': router['agent_id']}) + l3plugin.remove_router_from_l3_agent( + context, router['agent_id'], router['router_id']) try: # Note that DVR Interface ports will have bindings on # multiple hosts, and so will have multiple mech_contexts, diff --git a/neutron/tests/unit/ml2/test_ml2_plugin.py b/neutron/tests/unit/ml2/test_ml2_plugin.py index 2b07e9eb775..5dec58e49d6 100644 --- a/neutron/tests/unit/ml2/test_ml2_plugin.py +++ b/neutron/tests/unit/ml2/test_ml2_plugin.py @@ -27,7 +27,6 @@ from neutron import context from neutron.db import db_base_plugin_v2 as base_plugin from neutron.db import l3_db from neutron.extensions import external_net as external_net -from neutron.extensions import l3agentscheduler from neutron.extensions import multiprovidernet as mpnet from neutron.extensions import portbindings from neutron.extensions import providernet as pnet @@ -325,10 +324,7 @@ class TestMl2DvrPortsV2(TestMl2PortsV2): device_owner='compute:None'), mock.patch.object(self.l3plugin, 'dvr_deletens_if_no_port', return_value=[ns_to_delete]), - mock.patch.object(self.l3plugin, 'remove_router_from_l3_agent', - side_effect=l3agentscheduler.RouterNotHostedByL3Agent( - router_id=ns_to_delete['router_id'], - agent_id=ns_to_delete['agent_id'])) + mock.patch.object(self.l3plugin, 'remove_router_from_l3_agent') ) as (get_service_plugin, port, dvr_delns_ifno_port, remove_router_from_l3_agent): diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py index 9e603425111..110b3c57df3 100644 --- a/neutron/tests/unit/test_l3_schedulers.py +++ b/neutron/tests/unit/test_l3_schedulers.py @@ -341,6 +341,20 @@ class L3SchedulerTestBaseMixin(object): router['router']['id']) self.assertNotEqual(already_scheduled, auto_s.called) + def test__unbind_router_removes_binding(self): + agent_id = self.agent_id1 + agent = self.agent1 + router = self._make_router(self.fmt, + tenant_id=str(uuid.uuid4()), + name='r1') + self._test_schedule_bind_router(agent, router) + self._unbind_router(self.adminContext, + router['router']['id'], + agent_id) + bindings = self._get_l3_bindings_hosting_routers( + self.adminContext, [router['router']['id']]) + self.assertEqual(0, len(bindings)) + def _create_router_for_l3_agent_dvr_test(self, distributed=False, external_gw=None):