Block delete_(network|subnet) transactioned calls

These functions assume that they won't be called inside of a
transaction for two reasons. The first is an infinite loop that
will only terminate if it can actually retrieve the latest
information from the database. The second is that ML2's post-commit
operations are expected to occur after data was actually committed
so mechanism drivers that do DB lookups using a different session
may break.

This adds an explicit guard that prevents these two functions from
being called within an active DB session. The rest of the ML2
functions should eventually be converted to this style as well
(see the partial bug).

Related-Bug: #1551958
Partial-Bug: #1540844
Change-Id: I5c00b186585369ef6c8e2b9cb5a43b8bba0e5a7c

doc/source/devref/service_extensions.rst

@@ -49,3 +49,32 @@ reach out to your PTL or a member of the drivers team for approval.
 #. http://git.openstack.org/cgit/openstack/neutron-lbaas/
 #. http://git.openstack.org/cgit/openstack/neutron-vpnaas/
 
+
+Calling the Core Plugin from Services
+-------------------------------------
+
+There are many cases where a service may want to create a resource
+managed by the core plugin (e.g. ports, networks, subnets). This
+can be achieved by importing the Neutron Manager and getting a direct
+reference to the core plugin:
+
+.. code:: python
+   from neutron import manager
+
+   plugin = manager.NeutronManager.get_plugin()
+   plugin.create_port(context, port_dict)
+
+
+However, there is an important caveat. Calls to the core plugin in
+almost every case should not be made inside of an ongoing transaction.
+This is because many plugins (including ML2), can be configured to
+make calls to a backend after creating or modifying an object. If
+the call is made inside of a transaction and the transaction is
+rolled back after the core plugin call, the backend will not be
+notified that the change was undone. This will lead to consistency
+errors between the core plugin and its configured backend(s).
+
+ML2 has a guard against certain methods being called with an active
+DB transaction to help prevent developers from accidentally making
+this mistake. It will raise an error that says explicitly that the
+method should not be called within a transaction.

neutron/plugins/ml2/plugin.py

@@ -13,6 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import functools
+
 from eventlet import greenthread
 from oslo_config import cfg
 from oslo_db import api as oslo_db_api
@@ -91,6 +93,28 @@ SERVICE_PLUGINS_REQUIRED_DRIVERS = {
 }
 
 
+def transaction_guard(f):
+    """Ensures that the context passed in is not in a transaction.
+
+    Many of ML2's methods modifying resources have assumptions that they will
+    not be called inside of a transaction because they perform operations that
+    expect all data to be committed to the database (all postcommit calls).
+    Calling them in a transaction can lead to consistency errors on failures
+    since the ML2 drivers will not be notified of a DB rollback.
+
+    If you receive this error, you must alter your code to handle the fact that
+    ML2 calls have side effects so using transactions to undo on failures is
+    not possible.
+    """
+    @functools.wraps(f)
+    def inner(self, context, *args, **kwargs):
+        if context.session.is_active:
+            raise RuntimeError(_("Method cannot be called within a "
+                                 "transaction."))
+        return f(self, context, *args, **kwargs)
+    return inner
+
+
 class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                 dvr_mac_db.DVRDbMixin,
                 external_net_db.External_net_db_mixin,
@@ -793,6 +817,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                     LOG.exception(_LE("Exception auto-deleting subnet %s"),
                                   subnet_id)
 
+    @transaction_guard
     def delete_network(self, context, id):
         # REVISIT(rkukura) The super(Ml2Plugin, self).delete_network()
         # function is not used because it auto-deletes ports and
@@ -927,6 +952,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         self.mechanism_manager.update_subnet_postcommit(mech_context)
         return updated_subnet
 
+    @transaction_guard
     def delete_subnet(self, context, id):
         # REVISIT(rkukura) The super(Ml2Plugin, self).delete_subnet()
         # function is not used because it deallocates the subnet's addresses

neutron/tests/unit/plugins/ml2/test_plugin.py

@@ -1992,3 +1992,19 @@ class TestMl2PluginCreateUpdateDeletePort(base.BaseTestCase):
             # run the transaction balancing function defined in this test
             plugin.delete_port(self.context, 'fake_id')
             self.assertTrue(self.notify.call_count)
+
+
+class TestTransactionGuard(Ml2PluginV2TestCase):
+    def test_delete_network_guard(self):
+        plugin = ml2_plugin.Ml2Plugin()
+        ctx = context.get_admin_context()
+        with ctx.session.begin(subtransactions=True):
+            with testtools.ExpectedException(RuntimeError):
+                plugin.delete_network(ctx, 'id')
+
+    def test_delete_subnet_guard(self):
+        plugin = ml2_plugin.Ml2Plugin()
+        ctx = context.get_admin_context()
+        with ctx.session.begin(subtransactions=True):
+            with testtools.ExpectedException(RuntimeError):
+                plugin.delete_subnet(ctx, 'id')