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

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])

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'])