Move loadbalancer vip port creation outside of transaction

Currently _create_port_for_vip calls ml2 create_port() method
which includes rpc notification.
That leads to lock wait timeouts in certain cases.

The patch fixes that while making VIP creation process non-atomic.
But that is fine as long until create_vip() returns vip id, it's
not usable from API.

Change-Id: Ie30973de80118a6b022e8c3bb07ca48122ebcd29
Partial-Bug: #1334226

diff --git a/neutron/db/loadbalancer/loadbalancer_db.py b/neutron/db/loadbalancer/loadbalancer_db.py
index eb039d48f7fa22c0bc51749c1419449fbd99ca10..b9ed1a5f179b52d4a928552ccd5a61c0d6c8456d 100644 (file)
--- a/neutron/db/loadbalancer/loadbalancer_db.py
+++ b/neutron/db/loadbalancer/loadbalancer_db.py
@@ -318,25 +318,27 @@ class LoadBalancerPluginDb(loadbalancer.LoadBalancerPluginBase,
             sess_qry.filter_by(vip_id=vip_id).delete()
 
     def _create_port_for_vip(self, context, vip_db, subnet_id, ip_address):
-            # resolve subnet and create port
-            subnet = self._core_plugin.get_subnet(context, subnet_id)
-            fixed_ip = {'subnet_id': subnet['id']}
-            if ip_address and ip_address != attributes.ATTR_NOT_SPECIFIED:
-                fixed_ip['ip_address'] = ip_address
-
-            port_data = {
-                'tenant_id': vip_db.tenant_id,
-                'name': 'vip-' + vip_db.id,
-                'network_id': subnet['network_id'],
-                'mac_address': attributes.ATTR_NOT_SPECIFIED,
-                'admin_state_up': False,
-                'device_id': '',
-                'device_owner': '',
-                'fixed_ips': [fixed_ip]
-            }
-
-            port = self._core_plugin.create_port(context, {'port': port_data})
-            vip_db.port_id = port['id']
+        # resolve subnet and create port
+        subnet = self._core_plugin.get_subnet(context, subnet_id)
+        fixed_ip = {'subnet_id': subnet['id']}
+        if ip_address and ip_address != attributes.ATTR_NOT_SPECIFIED:
+            fixed_ip['ip_address'] = ip_address
+
+        port_data = {
+            'tenant_id': vip_db.tenant_id,
+            'name': 'vip-' + vip_db.id,
+            'network_id': subnet['network_id'],
+            'mac_address': attributes.ATTR_NOT_SPECIFIED,
+            'admin_state_up': False,
+            'device_id': '',
+            'device_owner': '',
+            'fixed_ips': [fixed_ip]
+        }
+
+        port = self._core_plugin.create_port(context, {'port': port_data})
+        vip_db.port_id = port['id']
+        # explicitly sync session with db
+        context.session.flush()
 
     def create_vip(self, context, vip):
         v = vip['vip']
@@ -384,17 +386,21 @@ class LoadBalancerPluginDb(loadbalancer.LoadBalancerPluginBase,
                 context.session.flush()
             except exception.DBDuplicateEntry:
                 raise loadbalancer.VipExists(pool_id=v['pool_id'])
+            if pool:
+                pool['vip_id'] = vip_db['id']
 
+        try:
             # create a port to reserve address for IPAM
+            # do it outside the transaction to avoid rpc calls
             self._create_port_for_vip(
-                context,
-                vip_db,
-                v['subnet_id'],
-                v.get('address')
-            )
-
-            if pool:
-                pool['vip_id'] = vip_db['id']
+                context, vip_db, v['subnet_id'], v.get('address'))
+        except Exception:
+            # catch any kind of exceptions
+            with excutils.save_and_reraise_exception():
+                context.session.delete(vip_db)
+                if pool:
+                    pool['vip_id'] = None
+                context.session.flush()
 
         return self._make_vip_dict(vip_db)
 

diff --git a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py
index 8acb95d2ff001477b4137467267dccc9e6ff5dc3..fec26b77b11e1ee4820243fcc70253bce43b9688 100644 (file)
--- a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py
+++ b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py
@@ -23,12 +23,14 @@ import webob.exc
 
 from neutron.api import extensions
 from neutron.common import config
+from neutron.common import exceptions as n_exc
 from neutron import context
 import neutron.db.l3_db  # noqa
 from neutron.db.loadbalancer import loadbalancer_db as ldb
 from neutron.db import servicetype_db as sdb
 import neutron.extensions
 from neutron.extensions import loadbalancer
+from neutron import manager
 from neutron.plugins.common import constants
 from neutron.services.loadbalancer import (
     plugin as loadbalancer_plugin
@@ -369,6 +371,26 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase):
                 )
             return vip
 
+    def test_create_vip_create_port_fails(self):
+        with self.subnet() as subnet:
+            with self.pool() as pool:
+                lb_plugin = (manager.NeutronManager.
+                             get_instance().
+                             get_service_plugins()[constants.LOADBALANCER])
+                with mock.patch.object(
+                    lb_plugin, '_create_port_for_vip') as cp:
+                    #some exception that can show up in port creation
+                    cp.side_effect = n_exc.IpAddressGenerationFailure(
+                        net_id=subnet['subnet']['network_id'])
+                    self._create_vip(self.fmt, "vip",
+                                     pool['pool']['id'], "HTTP", "80", True,
+                                     subnet_id=subnet['subnet']['id'],
+                                     expected_res_status=409)
+                req = self.new_list_request('vips')
+                res = self.deserialize(self.fmt,
+                                       req.get_response(self.ext_api))
+                self.assertFalse(res['vips'])
+
     def test_create_vip_twice_for_same_pool(self):
         """Test loadbalancer db plugin via extension and directly."""
         with self.subnet() as subnet: