ml2: allow retry on retriabable db error by precommit

Db retriable error by precommit can be recovered with upper layer by
retry instead of converting into ML2MechanismDriverError.
Raise retriable db error instead of swallowing it and let upper layer to
retry.
Due to concurrency, db error by precommit is sometimes inevitable.
precommit may issue db transactions depending on mechanism driver, Some
operations on e.g. subnet, routers, touches many other resources (e.g. ip
allocation, ports) in single db transaction as well. And background
operation by mechanism driver (e.g. background sync, monitoring
resources periodically, etc) may touch those db tables with other
order.

Change-Id: Ifc670c1eb5801934bf46fdc23a7721ce4c2cfa6c
Closes-Bug: #1609184
Closes-Bug: #1609149

neutron/plugins/ml2/managers.py

@@ -23,6 +23,7 @@ import six
 import stevedore
 
 from neutron._i18n import _, _LE, _LI, _LW
+from neutron.db import api as db_api
 from neutron.db import segments_db
 from neutron.extensions import external_net
 from neutron.extensions import multiprovidernet as mpnet
@@ -388,21 +389,32 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
                     raise vlantransparent.VlanTransparencyDriverError()
 
     def _call_on_drivers(self, method_name, context,
-                         continue_on_failure=False):
+                         continue_on_failure=False, raise_db_retriable=False):
         """Helper method for calling a method across all mechanism drivers.
 
         :param method_name: name of the method to call
         :param context: context parameter to pass to each method call
         :param continue_on_failure: whether or not to continue to call
         all mechanism drivers once one has raised an exception
+        :param raise_db_retriable: whether or not to treat retriable db
+        exception by mechanism drivers to propagate up to upper layer so
+        that upper layer can handle it or error in ML2 player
         :raises: neutron.plugins.ml2.common.MechanismDriverError
-        if any mechanism driver call fails.
+        if any mechanism driver call fails. or DB retriable error when
+        raise_db_retriable=False. See neutron.db.api.is_retriable for
+        what db exception is retriable
         """
         error = False
         for driver in self.ordered_mech_drivers:
             try:
                 getattr(driver.obj, method_name)(context)
-            except Exception:
+            except Exception as e:
+                if raise_db_retriable and db_api.is_retriable(e):
+                    with excutils.save_and_reraise_exception():
+                        LOG.debug("DB exception raised by Mechanism driver "
+                                  "'%(name)s' in %(method)s",
+                                  {'name': driver.name, 'method': method_name},
+                                  exc_info=e)
                 LOG.exception(
                     _LE("Mechanism driver '%(name)s' failed in %(method)s"),
                     {'name': driver.name, 'method': method_name}
@@ -418,7 +430,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
     def create_network_precommit(self, context):
         """Notify all mechanism drivers during network creation.
 
-        :raises: neutron.plugins.ml2.common.MechanismDriverError
+        :raises: DB retriable error if create_network_precommit raises them
+        See neutron.db.api.is_retriable for what db exception is retriable
+        or neutron.plugins.ml2.common.MechanismDriverError
         if any mechanism driver create_network_precommit call fails.
 
         Called within the database transaction. If a mechanism driver
@@ -427,7 +441,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
         that all mechanism drivers are called in this case.
         """
         self._check_vlan_transparency(context)
-        self._call_on_drivers("create_network_precommit", context)
+        self._call_on_drivers("create_network_precommit", context,
+                              raise_db_retriable=True)
 
     def create_network_postcommit(self, context):
         """Notify all mechanism drivers after network creation.
@@ -446,7 +461,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
     def update_network_precommit(self, context):
         """Notify all mechanism drivers during network update.
 
-        :raises: neutron.plugins.ml2.common.MechanismDriverError
+        :raises: DB retriable error if create_network_precommit raises them
+        See neutron.db.api.is_retriable for what db exception is retriable
+        or neutron.plugins.ml2.common.MechanismDriverError
         if any mechanism driver update_network_precommit call fails.
 
         Called within the database transaction. If a mechanism driver
@@ -454,7 +471,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
         to the caller, triggering a rollback. There is no guarantee
         that all mechanism drivers are called in this case.
         """
-        self._call_on_drivers("update_network_precommit", context)
+        self._call_on_drivers("update_network_precommit", context,
+                              raise_db_retriable=True)
 
     def update_network_postcommit(self, context):
         """Notify all mechanism drivers after network update.
@@ -473,7 +491,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
     def delete_network_precommit(self, context):
         """Notify all mechanism drivers during network deletion.
 
-        :raises: neutron.plugins.ml2.common.MechanismDriverError
+        :raises: DB retriable error if create_network_precommit raises them
+        See neutron.db.api.is_retriable for what db exception is retriable
+        or neutron.plugins.ml2.common.MechanismDriverError
         if any mechanism driver delete_network_precommit call fails.
 
         Called within the database transaction. If a mechanism driver
@@ -481,7 +501,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
         to the caller, triggering a rollback. There is no guarantee
         that all mechanism drivers are called in this case.
         """
-        self._call_on_drivers("delete_network_precommit", context)
+        self._call_on_drivers("delete_network_precommit", context,
+                              raise_db_retriable=True)
 
     def delete_network_postcommit(self, context):
         """Notify all mechanism drivers after network deletion.
@@ -504,7 +525,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
     def create_subnet_precommit(self, context):
         """Notify all mechanism drivers during subnet creation.
 
-        :raises: neutron.plugins.ml2.common.MechanismDriverError
+        :raises: DB retriable error if create_network_precommit raises them
+        See neutron.db.api.is_retriable for what db exception is retriable
+        or neutron.plugins.ml2.common.MechanismDriverError
         if any mechanism driver create_subnet_precommit call fails.
 
         Called within the database transaction. If a mechanism driver
@@ -512,7 +535,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
         to the caller, triggering a rollback. There is no guarantee
         that all mechanism drivers are called in this case.
         """
-        self._call_on_drivers("create_subnet_precommit", context)
+        self._call_on_drivers("create_subnet_precommit", context,
+                              raise_db_retriable=True)
 
     def create_subnet_postcommit(self, context):
         """Notify all mechanism drivers after subnet creation.
@@ -531,7 +555,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
     def update_subnet_precommit(self, context):
         """Notify all mechanism drivers during subnet update.
 
-        :raises: neutron.plugins.ml2.common.MechanismDriverError
+        :raises: DB retriable error if create_network_precommit raises them
+        See neutron.db.api.is_retriable for what db exception is retriable
+        or neutron.plugins.ml2.common.MechanismDriverError
         if any mechanism driver update_subnet_precommit call fails.
 
         Called within the database transaction. If a mechanism driver
@@ -539,7 +565,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
         to the caller, triggering a rollback. There is no guarantee
         that all mechanism drivers are called in this case.
         """
-        self._call_on_drivers("update_subnet_precommit", context)
+        self._call_on_drivers("update_subnet_precommit", context,
+                              raise_db_retriable=True)
 
     def update_subnet_postcommit(self, context):
         """Notify all mechanism drivers after subnet update.
@@ -558,7 +585,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
     def delete_subnet_precommit(self, context):
         """Notify all mechanism drivers during subnet deletion.
 
-        :raises: neutron.plugins.ml2.common.MechanismDriverError
+        :raises: DB retriable error if create_network_precommit raises them
+        See neutron.db.api.is_retriable for what db exception is retriable
+        or neutron.plugins.ml2.common.MechanismDriverError
         if any mechanism driver delete_subnet_precommit call fails.
 
         Called within the database transaction. If a mechanism driver
@@ -566,7 +595,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
         to the caller, triggering a rollback. There is no guarantee
         that all mechanism drivers are called in this case.
         """
-        self._call_on_drivers("delete_subnet_precommit", context)
+        self._call_on_drivers("delete_subnet_precommit", context,
+                              raise_db_retriable=True)
 
     def delete_subnet_postcommit(self, context):
         """Notify all mechanism drivers after subnet deletion.
@@ -589,7 +619,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
     def create_port_precommit(self, context):
         """Notify all mechanism drivers during port creation.
 
-        :raises: neutron.plugins.ml2.common.MechanismDriverError
+        :raises: DB retriable error if create_network_precommit raises them
+        See neutron.db.api.is_retriable for what db exception is retriable
+        or neutron.plugins.ml2.common.MechanismDriverError
         if any mechanism driver create_port_precommit call fails.
 
         Called within the database transaction. If a mechanism driver
@@ -597,7 +629,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
         to the caller, triggering a rollback. There is no guarantee
         that all mechanism drivers are called in this case.
         """
-        self._call_on_drivers("create_port_precommit", context)
+        self._call_on_drivers("create_port_precommit", context,
+                              raise_db_retriable=True)
 
     def create_port_postcommit(self, context):
         """Notify all mechanism drivers of port creation.
@@ -616,7 +649,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
     def update_port_precommit(self, context):
         """Notify all mechanism drivers during port update.
 
-        :raises: neutron.plugins.ml2.common.MechanismDriverError
+        :raises: DB retriable error if create_network_precommit raises them
+        See neutron.db.api.is_retriable for what db exception is retriable
+        or neutron.plugins.ml2.common.MechanismDriverError
         if any mechanism driver update_port_precommit call fails.
 
         Called within the database transaction. If a mechanism driver
@@ -624,7 +659,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
         to the caller, triggering a rollback. There is no guarantee
         that all mechanism drivers are called in this case.
         """
-        self._call_on_drivers("update_port_precommit", context)
+        self._call_on_drivers("update_port_precommit", context,
+                              raise_db_retriable=True)
 
     def update_port_postcommit(self, context):
         """Notify all mechanism drivers after port update.
@@ -643,7 +679,9 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
     def delete_port_precommit(self, context):
         """Notify all mechanism drivers during port deletion.
 
-        :raises: neutron.plugins.ml2.common.MechanismDriverError
+        :raises:DB retriable error if create_network_precommit raises them
+        See neutron.db.api.is_retriable for what db exception is retriable
+        or neutron.plugins.ml2.common.MechanismDriverError
         if any mechanism driver delete_port_precommit call fails.
 
         Called within the database transaction. If a mechanism driver
@@ -651,7 +689,8 @@ class MechanismManager(stevedore.named.NamedExtensionManager):
         to the caller, triggering a rollback. There is no guarantee
         that all mechanism drivers are called in this case.
         """
-        self._call_on_drivers("delete_port_precommit", context)
+        self._call_on_drivers("delete_port_precommit", context,
+                              raise_db_retriable=True)
 
     def delete_port_postcommit(self, context):
         """Notify all mechanism drivers after port deletion.

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

@@ -16,9 +16,14 @@
 
 import mock
 
+from oslo_db import exception as db_exc
+
+from neutron.plugins.ml2.common import exceptions as ml2_exc
+from neutron.plugins.ml2 import config as config
 from neutron.plugins.ml2 import driver_api as api
 from neutron.plugins.ml2 import managers
 from neutron.tests import base
+from neutron.tests.unit.plugins.ml2.drivers import mechanism_test
 
 
 class TestManagers(base.BaseTestCase):
@@ -36,3 +41,39 @@ class TestManagers(base.BaseTestCase):
         bindinglevel.segment_id = 'fake_seg_id1'
         self.assertTrue(manager._check_driver_to_bind(
             'fake_driver', segments_to_bind, binding_levels))
+
+
+class TestMechManager(base.BaseTestCase):
+    def setUp(self):
+        config.cfg.CONF.set_override('mechanism_drivers', ['test'],
+                                     group='ml2')
+        super(TestMechManager, self).setUp()
+        self._manager = managers.MechanismManager()
+
+    def _check_precommit(self, resource, operation):
+        meth_name = "%s_%s_precommit" % (operation, resource)
+        method = getattr(self._manager, meth_name)
+        fake_ctxt = mock.Mock()
+        fake_ctxt.current = {}
+
+        with mock.patch.object(mechanism_test.TestMechanismDriver, meth_name,
+                               side_effect=db_exc.DBDeadlock()):
+            self.assertRaises(db_exc.DBDeadlock, method, fake_ctxt)
+
+        with mock.patch.object(mechanism_test.TestMechanismDriver, meth_name,
+                               side_effect=RuntimeError()):
+            self.assertRaises(ml2_exc.MechanismDriverError, method, fake_ctxt)
+
+    def _check_resource(self, resource):
+        self._check_precommit(resource, 'create')
+        self._check_precommit(resource, 'update')
+        self._check_precommit(resource, 'delete')
+
+    def test_network_precommit(self):
+        self._check_resource('network')
+
+    def test_subnet_precommit(self):
+        self._check_resource('subnet')
+
+    def test_port_precommit(self):
+        self._check_resource('port')