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

diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py
index 5ee651f794e16953bf72e8eefd324b0216adea9d..58dad2696a982feddcf3435535dedd059cecf251 100644 (file)
--- 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 01e9791514ee6ee14b85c2f5b1f0bd5911392dd5..df7ef7f94b46d8f759e570147537a8487d78551e 100644 (file)
--- 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 eb6f4a1bd87da7bc7cfe701316c43d06f2818125..488d9744f20f5ee9f55c915a8992c0f8af4ea0f7 100644 (file)
--- 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 8e22257eeb2269ab3301f9dcca57246f68be0543..128015a5e2e254cb326cb51bfb53001fb02461d8 100644 (file)
--- 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']]}