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

diff --git a/neutron/db/loadbalancer/loadbalancer_db.py b/neutron/db/loadbalancer/loadbalancer_db.py
index b056562d9d113f1b325c123ad84423db8cc173c1..18e8d39f3fb9f2b2522d318757f97ae688139a3f 100644 (file)
--- 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 (file)
index 0000000..897c8aa
--- /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 d2dde8adb7f968359b8276bd51b98076a3800d79..37a7fb36fd176350803ae9edba4d92cdc41e1d34 100644 (file)
--- 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 db5a522b3e63b398eb7fef4001f18886b9443937..f1f2a64207958bcdd02ad8a21c2960fa92399d24 100644 (file)
--- 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 153ca66d6f841b0d30ee50baec02ee555eb8dcb5..6a0b0b849259241077abe0b07f547cfa6ead51df 100644 (file)
--- 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)