Retry on routerport delete race

After deleting the router interfaces, another concurrent
process (e.g. HA port creation) could happen before the actual
router is deleted, which would lead to an SQLAlchemy error due
to the relationship being violated.

This patch fixes the issue by bumping the router revision before
deleting the router to check for concurrent interface additions.
A race will trigger a staledataerror which will be retried by
the decorator

Closes-Bug: #1655281
Change-Id: I465e9a2f9b216991afa26c16271854fb88068006

neutron/db/l3_db.py

@@ -550,6 +550,10 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
             registry.notify(resources.ROUTER, events.PRECOMMIT_DELETE,
                             self, context=context, router_db=router,
                             router_id=id)
+            # we bump the revision even though we are about to delete to throw
+            # staledataerror if something snuck in with a new interface
+            router.bump_revision()
+            context.session.flush()
             context.session.delete(router)
         registry.notify(resources.ROUTER, events.AFTER_DELETE, self,
                         context=context, router_id=id, original=original)

neutron/tests/unit/db/test_l3_hamode_db.py

@@ -25,7 +25,10 @@ import testtools
 
 from neutron.agent.common import utils as agent_utils
 from neutron.api.rpc.handlers import l3_rpc
+from neutron.callbacks import events
 from neutron.callbacks import exceptions as c_exc
+from neutron.callbacks import registry
+from neutron.callbacks import resources
 from neutron.common import constants as n_const
 from neutron import context
 from neutron.db import agents_db
@@ -39,6 +42,7 @@ from neutron.extensions import l3_ext_ha_mode
 from neutron.extensions import portbindings
 from neutron.extensions import providernet
 from neutron.scheduler import l3_agent_scheduler
+from neutron.services.revisions import revision_plugin
 from neutron.tests.common import helpers
 from neutron.tests.unit import testlib_api
 
@@ -246,6 +250,24 @@ class L3HATestCase(L3HATestFramework):
         router = self._create_router(ha=None)
         self.assertTrue(router['ha'])
 
+    def test_ha_interface_concurrent_create_on_delete(self):
+        # this test depends on protection from the revision plugin so
+        # we have to initialize it
+        revision_plugin.RevisionPlugin()
+        router = self._create_router(ha=True)
+
+        def jam_in_interface(*args, **kwargs):
+            ctx = context.get_admin_context()
+            net = self.plugin._ensure_vr_id_and_network(
+                ctx, self.plugin._get_router(ctx, router['id']))
+            self.plugin.add_ha_port(
+                ctx, router['id'], net.network_id, router['tenant_id'])
+            registry.unsubscribe(jam_in_interface, resources.ROUTER,
+                                 events.PRECOMMIT_DELETE)
+        registry.subscribe(jam_in_interface, resources.ROUTER,
+                           events.PRECOMMIT_DELETE)
+        self.plugin.delete_router(self.admin_ctx, router['id'])
+
     def test_ha_router_delete_with_distributed(self):
         router = self._create_router(ha=True, distributed=True)
         self.plugin.delete_router(self.admin_ctx, router['id'])

neutron/tests/unit/extensions/test_l3.py

@@ -53,6 +53,7 @@ from neutron.extensions import external_net
 from neutron.extensions import l3
 from neutron.extensions import portbindings
 from neutron.plugins.ml2 import config
+from neutron.services.revisions import revision_plugin
 from neutron.tests import base
 from neutron.tests.common import helpers
 from neutron.tests import fake_notifier
@@ -1031,6 +1032,23 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
                 with self.subnet(network=n) as s:
                     self._test_router_add_interface_subnet(r, s)
 
+    def test_router_delete_race_with_interface_add(self):
+        # this test depends on protection from the revision plugin so
+        # we have to initialize it
+        revision_plugin.RevisionPlugin()
+        with self.router() as r, self.subnet() as s:
+
+            def jam_in_interface(*args, **kwargs):
+                self._router_interface_action('add', r['router']['id'],
+                                              s['subnet']['id'], None)
+                # unsubscribe now that the evil is done
+                registry.unsubscribe(jam_in_interface, resources.ROUTER,
+                                     events.PRECOMMIT_DELETE)
+            registry.subscribe(jam_in_interface, resources.ROUTER,
+                               events.PRECOMMIT_DELETE)
+            self._delete('routers', r['router']['id'],
+                         expected_code=exc.HTTPConflict.code)
+
     def test_router_add_interface_ipv6_subnet(self):
         """Test router-interface-add for valid ipv6 subnets.