From dafa61bd46b7eacbc708d17a3fa492de971d6dd2 Mon Sep 17 00:00:00 2001
From: armando-migliaccio <armamig@gmail.com>
Date: Sat, 12 Sep 2015 12:07:35 -0700
Subject: [PATCH] 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                 | 19 +++++++++++-----
 neutron/tests/unit/db/test_l3_dvr_db.py | 30 +++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/neutron/db/l3_dvr_db.py b/neutron/db/l3_dvr_db.py
index e1619b245..c7f78547b 100644
--- 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 0dc5330a4..97a559089 100644
--- 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'])
-- 
2.45.2