From: armando-migliaccio <armamig@gmail.com>
Date: Sat, 12 Sep 2015 19:07:35 +0000 (-0700)
Subject: Fix BadRequest error on add_router_interface for DVR
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=dafa61bd46b7eacbc708d17a3fa492de971d6dd2;p=openstack-build%2Fneutron-build.git

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