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
(cherry picked from commit a8c715ab0f)

neutron/db/l3_agentschedulers_db.py

@@ -249,11 +249,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:

neutron/db/l3_hamode_db.py

@@ -314,6 +314,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,

neutron/scheduler/l3_agent_scheduler.py

@@ -289,9 +289,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)
 

neutron/tests/unit/db/test_l3_hamode_db.py

@@ -432,6 +432,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']]}