From 4a96b16dcb837c8085b0fb895ee6052cdde054ca Mon Sep 17 00:00:00 2001 From: Eugene Nikanorov Date: Thu, 21 Mar 2013 17:19:30 +0400 Subject: [PATCH] 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 | 65 +++++++++------- quantum/extensions/loadbalancer.py | 4 + .../db/loadbalancer/test_db_loadbalancer.py | 75 ++++++++++++++++++- 3 files changed, 115 insertions(+), 29 deletions(-) diff --git a/quantum/db/loadbalancer/loadbalancer_db.py b/quantum/db/loadbalancer/loadbalancer_db.py index a2557f308..dfe5eec84 100644 --- a/quantum/db/loadbalancer/loadbalancer_db.py +++ b/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) - context.session.flush() + try: + 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) - # 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 - if new_pool['tenant_id'] != vip_db['tenant_id']: - raise q_exc.NotAuthorized() - # validate that the pool has same protocol - if new_pool['protocol'] != vip_db['protocol']: - raise loadbalancer.ProtocolMismatch( - vip_proto=vip_db['protocol'], - pool_proto=new_pool['protocol']) - - if vip_db['pool_id']: - old_pool = self._get_resource( - context, - Pool, - vip_db['pool_id'] - ) - old_pool['vip_id'] = None - - new_pool['vip_id'] = vip_db['id'] + try: + # in case new pool already has a vip + # update will raise integrity error at first query + old_pool_id = vip_db['pool_id'] + vip_db.update(v) + # 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 + if new_pool['tenant_id'] != vip_db['tenant_id']: + raise q_exc.NotAuthorized() + # validate that the pool has same protocol + if new_pool['protocol'] != vip_db['protocol']: + raise loadbalancer.ProtocolMismatch( + vip_proto=vip_db['protocol'], + pool_proto=new_pool['protocol']) + + if old_pool_id: + old_pool = self._get_resource( + context, + Pool, + old_pool_id + ) + old_pool['vip_id'] = None + + 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) diff --git a/quantum/extensions/loadbalancer.py b/quantum/extensions/loadbalancer.py index f984441bf..6c7c22b49 100644 --- a/quantum/extensions/loadbalancer.py +++ b/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") diff --git a/quantum/tests/unit/db/loadbalancer/test_db_loadbalancer.py b/quantum/tests/unit/db/loadbalancer/test_db_loadbalancer.py index 5305d0541..3ce24362d 100644 --- a/quantum/tests/unit/db/loadbalancer/test_db_loadbalancer.py +++ b/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', -- 2.45.2