Fix BadRequest error on add_router_interface for DVR

This operation for DVR is made of multiple steps, some of
which are not within the same DB transaction. For this
reason, if a failure occurs, the rollback will be partial.

This inconsistent state leads the retry logic to fail with
BadRequest, because the router is believed to be already
connected to the subnet.

To fix this condition, it is necessary to delete the port
should the DB deadlock occur.

Closes-bug: #1494114

Change-Id: Ia2a73d6f9d1e4746e761ad072d954e64267a3ad1

diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py
index e1619b2456621df7e15e7ccc1a9f4acc6cf7a314..c7f78547b9bf51e0bc5dc3f42da7e4076033cb95 100644 (file)
--- a/neutron/db/l3_dvr_db.py
+++ b/neutron/db/l3_dvr_db.py
@@ -313,6 +313,20 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
                     context, router, interface_info['subnet_id'], device_owner)
 
         if new_port:
+            if router.extra_attributes.distributed and router.gw_port:
+                try:
+                    admin_context = context.elevated()
+                    self._add_csnat_router_interface_port(
+                        admin_context, router, port['network_id'],
+                        port['fixed_ips'][-1]['subnet_id'])
+                except Exception:
+                    with excutils.save_and_reraise_exception():
+                        # we need to preserve the original state prior
+                        # the request by rolling back the port creation
+                        # that led to new_port=True
+                        self._core_plugin.delete_port(
+                            admin_context, port['id'])
+
             with context.session.begin(subtransactions=True):
                 router_port = l3_db.RouterPort(
                     port_id=port['id'],
@@ -321,11 +335,6 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
                 )
                 context.session.add(router_port)
 
-            if router.extra_attributes.distributed and router.gw_port:
-                self._add_csnat_router_interface_port(
-                    context.elevated(), router, port['network_id'],
-                    port['fixed_ips'][-1]['subnet_id'])
-
         router_interface_info = self._make_router_interface_info(
             router_id, port['tenant_id'], port['id'], subnets[-1]['id'],
             [subnet['id'] for subnet in subnets])

diff --git a/neutron/tests/unit/db/test_l3_dvr_db.py b/neutron/tests/unit/db/test_l3_dvr_db.py
index 0dc5330a45d5d5d521a150cd72a8b88299e870cd..97a559089e1df2a80c6d49dddd1cd9849c38ae32 100644 (file)
--- a/neutron/tests/unit/db/test_l3_dvr_db.py
+++ b/neutron/tests/unit/db/test_l3_dvr_db.py
@@ -735,3 +735,33 @@ class L3DvrTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase):
         action = 'del'
         device_owner = l3_const.DEVICE_OWNER_LOADBALANCER
         self._test_dvr_vmarp_table_update(device_owner, action)
+
+    def test_add_router_interface_csnat_ports_failure(self):
+        router_dict = {'name': 'test_router', 'admin_state_up': True,
+                       'distributed': True}
+        router = self._create_router(router_dict)
+        with self.network() as net_ext,\
+                self.subnet() as subnet:
+            ext_net_id = net_ext['network']['id']
+            self.core_plugin.update_network(
+                self.ctx, ext_net_id,
+                {'network': {'router:external': True}})
+            self.mixin.update_router(
+                self.ctx, router['id'],
+                {'router': {'external_gateway_info':
+                            {'network_id': ext_net_id}}})
+            with mock.patch.object(
+                self.mixin, '_add_csnat_router_interface_port') as f:
+                f.side_effect = RuntimeError()
+                self.assertRaises(
+                    RuntimeError,
+                    self.mixin.add_router_interface,
+                    self.ctx, router['id'],
+                    {'subnet_id': subnet['subnet']['id']})
+                filters = {
+                    'device_id': [router['id']],
+                }
+                router_ports = self.core_plugin.get_ports(self.ctx, filters)
+                self.assertEqual(1, len(router_ports))
+                self.assertEqual(l3_const.DEVICE_OWNER_ROUTER_GW,
+                                 router_ports[0]['device_owner'])