From b7ad4b6606569ee6e207262f05eac5d64724e268 Mon Sep 17 00:00:00 2001 From: Stephen Gran Date: Sun, 17 Nov 2013 11:35:29 +0000 Subject: [PATCH] Enforce unique constraint on neutron pool members Neutron loadbalancer pool members should not be allowed to have duplicate address/port tuples in the same pool. Change-Id: Ie52c6033217ec05ee4f59bcf8a0e4167c7b13663 Closes-Bug: #1251867 --- neutron/db/loadbalancer/loadbalancer_db.py | 57 +++++++++------- .../e197124d4b9_add_unique_constrain.py | 65 +++++++++++++++++++ neutron/extensions/loadbalancer.py | 5 ++ .../db/loadbalancer/test_db_loadbalancer.py | 19 ++++++ .../drivers/radware/test_plugin_driver.py | 1 + 5 files changed, 125 insertions(+), 22 deletions(-) create mode 100644 neutron/db/migration/alembic_migrations/versions/e197124d4b9_add_unique_constrain.py diff --git a/neutron/db/loadbalancer/loadbalancer_db.py b/neutron/db/loadbalancer/loadbalancer_db.py index b056562d9d..18e8d39f3f 100644 --- a/neutron/db/loadbalancer/loadbalancer_db.py +++ b/neutron/db/loadbalancer/loadbalancer_db.py @@ -97,6 +97,10 @@ class Member(model_base.BASEV2, models_v2.HasId, models_v2.HasTenant, models_v2.HasStatusDescription): """Represents a v2 neutron loadbalancer member.""" + __table_args__ = ( + sa.schema.UniqueConstraint('pool_id', 'address', 'protocol_port', + name='uniq_member0pool_id0address0port'), + ) pool_id = sa.Column(sa.String(36), sa.ForeignKey("pools.id"), nullable=False) address = sa.Column(sa.String(64), nullable=False) @@ -668,31 +672,40 @@ class LoadBalancerPluginDb(LoadBalancerPluginBase, v = member['member'] tenant_id = self._get_tenant_id_for_create(context, v) - with context.session.begin(subtransactions=True): - # ensuring that pool exists - self._get_resource(context, Pool, v['pool_id']) - - member_db = Member(id=uuidutils.generate_uuid(), - tenant_id=tenant_id, - pool_id=v['pool_id'], - address=v['address'], - protocol_port=v['protocol_port'], - weight=v['weight'], - admin_state_up=v['admin_state_up'], - status=constants.PENDING_CREATE) - context.session.add(member_db) - - return self._make_member_dict(member_db) + try: + with context.session.begin(subtransactions=True): + # ensuring that pool exists + self._get_resource(context, Pool, v['pool_id']) + member_db = Member(id=uuidutils.generate_uuid(), + tenant_id=tenant_id, + pool_id=v['pool_id'], + address=v['address'], + protocol_port=v['protocol_port'], + weight=v['weight'], + admin_state_up=v['admin_state_up'], + status=constants.PENDING_CREATE) + context.session.add(member_db) + return self._make_member_dict(member_db) + except exception.DBDuplicateEntry: + raise loadbalancer.MemberExists( + address=v['address'], + port=v['protocol_port'], + pool=v['pool_id']) def update_member(self, context, id, member): v = member['member'] - with context.session.begin(subtransactions=True): - member_db = self._get_resource(context, Member, id) - self.assert_modification_allowed(member_db) - if v: - member_db.update(v) - - return self._make_member_dict(member_db) + try: + with context.session.begin(subtransactions=True): + member_db = self._get_resource(context, Member, id) + self.assert_modification_allowed(member_db) + if v: + member_db.update(v) + return self._make_member_dict(member_db) + except exception.DBDuplicateEntry: + raise loadbalancer.MemberExists( + address=member_db['address'], + port=member_db['protocol_port'], + pool=member_db['pool_id']) def delete_member(self, context, id): with context.session.begin(subtransactions=True): diff --git a/neutron/db/migration/alembic_migrations/versions/e197124d4b9_add_unique_constrain.py b/neutron/db/migration/alembic_migrations/versions/e197124d4b9_add_unique_constrain.py new file mode 100644 index 0000000000..897c8aac2a --- /dev/null +++ b/neutron/db/migration/alembic_migrations/versions/e197124d4b9_add_unique_constrain.py @@ -0,0 +1,65 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 +# +# Copyright 2013 OpenStack Foundation +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. +# + +"""add unique constraint to members + +Revision ID: e197124d4b9 +Revises: havana +Create Date: 2013-11-17 10:09:37.728903 + +""" + +# revision identifiers, used by Alembic. +revision = 'e197124d4b9' +down_revision = 'havana' + +# Change to ['*'] if this migration applies to all plugins + +migration_for_plugins = [ + 'neutron.services.loadbalancer.plugin.LoadBalancerPlugin', + 'neutron.plugins.nicira.NeutronServicePlugin.NvpAdvancedPlugin', +] + +from alembic import op + +from neutron.db import migration + + +CONSTRAINT_NAME = 'uniq_member0pool_id0address0port' +TABLE_NAME = 'members' + + +def upgrade(active_plugins=None, options=None): + if not migration.should_run(active_plugins, migration_for_plugins): + return + + op.create_unique_constraint( + name=CONSTRAINT_NAME, + source=TABLE_NAME, + local_cols=['pool_id', 'address', 'protocol_port'] + ) + + +def downgrade(active_plugins=None, options=None): + if not migration.should_run(active_plugins, migration_for_plugins): + return + + op.drop_constraint( + name=CONSTRAINT_NAME, + tablename=TABLE_NAME, + type='unique' + ) diff --git a/neutron/extensions/loadbalancer.py b/neutron/extensions/loadbalancer.py index d2dde8adb7..37a7fb36fd 100644 --- a/neutron/extensions/loadbalancer.py +++ b/neutron/extensions/loadbalancer.py @@ -76,6 +76,11 @@ class ProtocolMismatch(qexception.BadRequest): "pool protocol %(pool_proto)s") +class MemberExists(qexception.NeutronException): + message = _("Member with address %(address)s and port %(port)s " + "already present in pool %(pool)s") + + RESOURCE_ATTRIBUTE_MAP = { 'vips': { 'id': {'allow_post': False, 'allow_put': False, diff --git a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py index db5a522b3e..f1f2a64207 100644 --- a/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py +++ b/neutron/tests/unit/db/loadbalancer/test_db_loadbalancer.py @@ -819,6 +819,25 @@ class TestLoadBalancer(LoadBalancerPluginDbTestCase): self.assertIn(member2['member']['id'], pool_update['pool']['members']) + def test_create_same_member_in_same_pool_raises_member_exists(self): + with self.subnet(): + with self.pool(name="pool1") as pool: + pool_id = pool['pool']['id'] + with self.member(address='192.168.1.100', + protocol_port=80, + pool_id=pool_id): + member_data = { + 'address': '192.168.1.100', + 'protocol_port': 80, + 'weight': 1, + 'admin_state_up': True, + 'pool_id': pool_id + } + self.assertRaises(loadbalancer.MemberExists, + self.plugin.create_member, + context.get_admin_context(), + {'member': member_data}) + def test_update_member(self): with self.pool(name="pool1") as pool1: with self.pool(name="pool2") as pool2: diff --git a/neutron/tests/unit/services/loadbalancer/drivers/radware/test_plugin_driver.py b/neutron/tests/unit/services/loadbalancer/drivers/radware/test_plugin_driver.py index 153ca66d6f..6a0b0b8492 100644 --- a/neutron/tests/unit/services/loadbalancer/drivers/radware/test_plugin_driver.py +++ b/neutron/tests/unit/services/loadbalancer/drivers/radware/test_plugin_driver.py @@ -293,6 +293,7 @@ class TestLoadBalancerPlugin(TestLoadBalancerPluginBase): self.member(pool_id=pool['pool']['id'], no_delete=True), self.member(pool_id=pool['pool']['id'], + address='192.168.1.101', no_delete=True), self.health_monitor(no_delete=True), self.vip(pool=pool, subnet=subnet, no_delete=True)