From a8c715ab0fae82563252e815a12ddeb37ee5cc23 Mon Sep 17 00:00:00 2001 From: Kevin Benton Date: Thu, 24 Dec 2015 01:37:10 -0800 Subject: [PATCH] Don't call add_ha_port inside a transaction Calling add_ha_port inside a transaction will break the delete_port error recovery logic. This patch prevents the scheduler from doing that. It also adds a note to add_ha_port and a runtime check to prevent the function from working with an active transaction. Change-Id: I39e2bb70527a8ff4a47668f44abb81d0fede3786 Closes-Bug: #1529037 --- neutron/db/l3_agentschedulers_db.py | 7 +++---- neutron/db/l3_hamode_db.py | 9 +++++++++ neutron/scheduler/l3_agent_scheduler.py | 4 ++-- neutron/tests/unit/db/test_l3_hamode_db.py | 5 +++++ 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py index 5ee651f79..58dad2696 100644 --- a/neutron/db/l3_agentschedulers_db.py +++ b/neutron/db/l3_agentschedulers_db.py @@ -251,11 +251,10 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, router = self.get_router(context, router_id) agent = self._get_agent(context, agent_id) self.validate_agent_router_combination(context, agent, router) - if self.check_agent_router_scheduling_needed( - context, agent, router): - self.create_router_to_agent_binding(context, agent, router) - else: + if not self.check_agent_router_scheduling_needed( + context, agent, router): return + self.create_router_to_agent_binding(context, agent, router) l3_notifier = self.agent_notifiers.get(constants.AGENT_TYPE_L3) if l3_notifier: diff --git a/neutron/db/l3_hamode_db.py b/neutron/db/l3_hamode_db.py index 01e979151..df7ef7f94 100644 --- a/neutron/db/l3_hamode_db.py +++ b/neutron/db/l3_hamode_db.py @@ -318,6 +318,15 @@ class L3_HA_NAT_db_mixin(l3_dvr_db.L3_NAT_with_dvr_db_mixin, return portbinding def add_ha_port(self, context, router_id, network_id, tenant_id): + # NOTE(kevinbenton): we have to block any ongoing transactions because + # our exception handling will try to delete the port using the normal + # core plugin API. If this function is called inside of a transaction + # the exception will mangle the state, cause the delete call to fail, + # and end up relying on the DB rollback to remove the port instead of + # proper delete_port call. + if context.session.is_active: + raise RuntimeError(_('add_ha_port cannot be called inside of a ' + 'transaction.')) args = {'tenant_id': '', 'network_id': network_id, 'admin_state_up': True, diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index eb6f4a1bd..488d9744f 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/neutron/scheduler/l3_agent_scheduler.py @@ -292,9 +292,9 @@ class L3Scheduler(object): tenant_id, agent): """Creates and binds a new HA port for this agent.""" ha_network = plugin.get_ha_network(context, tenant_id) + port_binding = plugin.add_ha_port(context.elevated(), router_id, + ha_network.network.id, tenant_id) with context.session.begin(subtransactions=True): - port_binding = plugin.add_ha_port(context.elevated(), router_id, - ha_network.network.id, tenant_id) port_binding.l3_agent_id = agent['id'] self.bind_router(context, router_id, agent) diff --git a/neutron/tests/unit/db/test_l3_hamode_db.py b/neutron/tests/unit/db/test_l3_hamode_db.py index 8e22257ee..128015a5e 100644 --- a/neutron/tests/unit/db/test_l3_hamode_db.py +++ b/neutron/tests/unit/db/test_l3_hamode_db.py @@ -422,6 +422,11 @@ class L3HATestCase(L3HATestFramework): self.assertNotEqual(ha0, ha1) + def test_add_ha_port_subtransactions_blocked(self): + with self.admin_ctx.session.begin(): + self.assertRaises(RuntimeError, self.plugin.add_ha_port, + self.admin_ctx, 'id', 'id', 'id') + def test_add_ha_port_binding_failure_rolls_back_port(self): router = self._create_router() device_filter = {'device_id': [router['id']]} -- 2.45.2