Merge "Move loadbalancer vip port creation outside of transaction"

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
+        # resolve subnet and create port
-            subnet = self._core_plugin.get_subnet(context, subnet_id)
+        subnet = self._core_plugin.get_subnet(context, subnet_id)
-            fixed_ip = {'subnet_id': subnet['id']}
+        fixed_ip = {'subnet_id': subnet['id']}
-            if ip_address and ip_address != attributes.ATTR_NOT_SPECIFIED:
+        if ip_address and ip_address != attributes.ATTR_NOT_SPECIFIED:
-                fixed_ip['ip_address'] = ip_address
+            fixed_ip['ip_address'] = ip_address
 
-            port_data = {
+        port_data = {
-                'tenant_id': vip_db.tenant_id,
+            'tenant_id': vip_db.tenant_id,
-                'name': 'vip-' + vip_db.id,
+            'name': 'vip-' + vip_db.id,
-                'network_id': subnet['network_id'],
+            'network_id': subnet['network_id'],
-                'mac_address': attributes.ATTR_NOT_SPECIFIED,
+            'mac_address': attributes.ATTR_NOT_SPECIFIED,
-                'admin_state_up': False,
+            'admin_state_up': False,
-                'device_id': '',
+            'device_id': '',
-                'device_owner': '',
+            'device_owner': '',
-                'fixed_ips': [fixed_ip]
+            'fixed_ips': [fixed_ip]
-            }
+        }
 
-            port = self._core_plugin.create_port(context, {'port': port_data})
+        port = self._core_plugin.create_port(context, {'port': port_data})
-            vip_db.port_id = port['id']
+        vip_db.port_id = port['id']
+        # explicitly sync session with db
+        context.session.flush()
 
     def create_vip(self, context, vip):
         v = vip['vip']
@@ -384,18 +386,22 @@ class LoadBalancerPluginDb(loadbalancer.LoadBalancerPluginBase,
                 context.session.flush()
             except exception.DBDuplicateEntry:
                 raise loadbalancer.VipExists(pool_id=v['pool_id'])
-
-            # create a port to reserve address for IPAM
-            self._create_port_for_vip(
-                context,
-                vip_db,
-                v['subnet_id'],
-                v.get('address')
-            )
-
             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'))
+        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)
 
     def update_vip(self, context, id, vip):

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: