Raise VipExists exception in case Vip is created or updated

for a pool that already has a Vip

fixes bug 1158049

Change-Id: I258a31f5d54c040ed478ab241b0e90f2bd0f69b3

quantum/db/loadbalancer/loadbalancer_db.py

@@ -17,6 +17,7 @@
 
 from oslo.config import cfg
 import sqlalchemy as sa
+from sqlalchemy import exc as sa_exc
 from sqlalchemy import orm
 from sqlalchemy.orm import exc
 from sqlalchemy.sql import expression as expr
@@ -390,8 +391,11 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase):
                     vip_db['id'])
                 vip_db.session_persistence = s_p
 
-            context.session.add(vip_db)
+            try:
-            context.session.flush()
+                context.session.add(vip_db)
+                context.session.flush()
+            except sa_exc.IntegrityError:
+                raise loadbalancer.VipExists(pool_id=v['pool_id'])
 
             # create a port to reserve address for IPAM
             self._create_port_for_vip(
@@ -421,31 +425,38 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase):
                 self._delete_session_persistence(context, id)
 
             if v:
-                vip_db.update(v)
+                try:
-                # If the pool_id is changed, we need to update
+                    # in case new pool already has a vip
-                # the associated pools
+                    # update will raise integrity error at first query
-                if 'pool_id' in v:
+                    old_pool_id = vip_db['pool_id']
-                    new_pool = self._get_resource(context, Pool, v['pool_id'])
+                    vip_db.update(v)
-                    self.assert_modification_allowed(new_pool)
+                    # If the pool_id is changed, we need to update
+                    # the associated pools
+                    if 'pool_id' in v:
+                        new_pool = self._get_resource(context, Pool,
+                                                      v['pool_id'])
+                        self.assert_modification_allowed(new_pool)
 
-                    # check that the pool matches the tenant_id
+                        # check that the pool matches the tenant_id
-                    if new_pool['tenant_id'] != vip_db['tenant_id']:
+                        if new_pool['tenant_id'] != vip_db['tenant_id']:
-                        raise q_exc.NotAuthorized()
+                            raise q_exc.NotAuthorized()
-                    # validate that the pool has same protocol
+                        # validate that the pool has same protocol
-                    if new_pool['protocol'] != vip_db['protocol']:
+                        if new_pool['protocol'] != vip_db['protocol']:
-                        raise loadbalancer.ProtocolMismatch(
+                            raise loadbalancer.ProtocolMismatch(
-                            vip_proto=vip_db['protocol'],
+                                vip_proto=vip_db['protocol'],
-                            pool_proto=new_pool['protocol'])
+                                pool_proto=new_pool['protocol'])
 
-                    if vip_db['pool_id']:
+                        if old_pool_id:
-                        old_pool = self._get_resource(
+                            old_pool = self._get_resource(
-                            context,
+                                context,
-                            Pool,
+                                Pool,
-                            vip_db['pool_id']
+                                old_pool_id
-                        )
+                            )
-                        old_pool['vip_id'] = None
+                            old_pool['vip_id'] = None
 
-                    new_pool['vip_id'] = vip_db['id']
+                        new_pool['vip_id'] = vip_db['id']
+                except sa_exc.IntegrityError:
+                    raise loadbalancer.VipExists(pool_id=v['pool_id'])
 
         return self._make_vip_dict(vip_db)
 

quantum/extensions/loadbalancer.py

@@ -33,6 +33,10 @@ class VipNotFound(qexception.NotFound):
     message = _("Vip %(vip_id)s could not be found")
 
 
+class VipExists(qexception.QuantumException):
+    message = _("Another Vip already exists for pool %(pool_id)s")
+
+
 class PoolNotFound(qexception.NotFound):
     message = _("Pool %(pool_id)s could not be found")
 

quantum/tests/unit/db/loadbalancer/test_db_loadbalancer.py

@@ -22,6 +22,7 @@ import testtools
 from oslo.config import cfg
 import webob.exc
 
+from quantum import context
 from quantum.api.extensions import ExtensionMiddleware
 from quantum.api.extensions import PluginAwareExtensionManager
 from quantum.api.v2 import attributes
@@ -30,6 +31,7 @@ from quantum.common import config
 from quantum.common import exceptions as q_exc
 from quantum.common.test_lib import test_config
 from quantum.db import api as db
+from quantum.db.loadbalancer import loadbalancer_db as ldb
 import quantum.extensions
 from quantum.extensions import loadbalancer
 from quantum.manager import QuantumManager
@@ -75,10 +77,10 @@ class LoadBalancerPluginDbTestCase(test_db_plugin.QuantumDbPluginV2TestCase):
 
         self._subnet_id = "0c798ed8-33ba-11e2-8b28-000c291c4d14"
 
-        plugin = loadbalancer_plugin.LoadBalancerPlugin()
+        self.plugin = loadbalancer_plugin.LoadBalancerPlugin()
         ext_mgr = PluginAwareExtensionManager(
             extensions_path,
-            {constants.LOADBALANCER: plugin}
+            {constants.LOADBALANCER: self.plugin}
         )
         app = config.load_paste_app('extensions_test_app')
         self.ext_api = ExtensionMiddleware(app, ext_mgr=ext_mgr)
@@ -300,6 +302,75 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase):
                 )
             return vip
 
+    def test_create_vip_twice_for_same_pool(self):
+        """ Test loadbalancer db plugin via extension and directly """
+        with self.subnet() as subnet:
+            with self.pool(name="pool1") as pool:
+                with self.vip(name='vip1', subnet=subnet, pool=pool) as vip:
+                    vip_data = {
+                        'name': 'vip1',
+                        'pool_id': pool['pool']['id'],
+                        'description': '',
+                        'protocol_port': 80,
+                        'protocol': 'HTTP',
+                        'connection_limit': -1,
+                        'admin_state_up': True,
+                        'status': 'PENDING_CREATE',
+                        'tenant_id': self._tenant_id,
+                        'session_persistence': ''
+                    }
+                    self.assertRaises(loadbalancer.VipExists,
+                                      self.plugin.create_vip,
+                                      context.get_admin_context(),
+                                      {'vip': vip_data})
+
+    def test_update_vip_raises_vip_exists(self):
+        with self.subnet() as subnet:
+            with contextlib.nested(
+                self.pool(name="pool1"),
+                self.pool(name="pool2")
+            ) as (pool1, pool2):
+                with contextlib.nested(
+                    self.vip(name='vip1', subnet=subnet, pool=pool1),
+                    self.vip(name='vip2', subnet=subnet, pool=pool2)
+                ) as (vip1, vip2):
+                    vip_data = {
+                        'id': vip2['vip']['id'],
+                        'name': 'vip1',
+                        'pool_id': pool1['pool']['id'],
+                    }
+                    self.assertRaises(loadbalancer.VipExists,
+                                      self.plugin.update_vip,
+                                      context.get_admin_context(),
+                                      vip2['vip']['id'],
+                                      {'vip': vip_data})
+
+    def test_update_vip_change_pool(self):
+        with self.subnet() as subnet:
+            with contextlib.nested(
+                self.pool(name="pool1"),
+                self.pool(name="pool2")
+            ) as (pool1, pool2):
+                with self.vip(name='vip1', subnet=subnet, pool=pool1) as vip:
+                    # change vip from pool1 to pool2
+                    vip_data = {
+                        'id': vip['vip']['id'],
+                        'name': 'vip1',
+                        'pool_id': pool2['pool']['id'],
+                    }
+                    ctx = context.get_admin_context()
+                    self.plugin.update_vip(ctx,
+                                           vip['vip']['id'],
+                                           {'vip': vip_data})
+                    db_pool2 = (ctx.session.query(ldb.Pool).
+                                filter_by(id=pool2['pool']['id']).one())
+                    db_pool1 = (ctx.session.query(ldb.Pool).
+                                filter_by(id=pool1['pool']['id']).one())
+                    # check that pool1.vip became None
+                    self.assertIsNone(db_pool1.vip)
+                    # and pool2 got vip
+                    self.assertEqual(db_pool2.vip.id, vip['vip']['id'])
+
     def test_create_vip_with_invalid_values(self):
         invalid = {
             'protocol': 'UNSUPPORTED',