From 411af3a965b3851f71a24779a4d556ad5a045570 Mon Sep 17 00:00:00 2001 From: Ivar Lazzaro Date: Wed, 16 Sep 2015 17:14:28 -0700 Subject: [PATCH] remove unique constraint from ES ip address mapping Verifying the uniqueness in software to allow for empty address to be set. Change-Id: If359130db372a29374020a4f82c683bb5bbb6ef3 Closes-Bug: 1496642 --- .../neutron/db/grouppolicy/group_policy_db.py | 11 +++-- ...573886_es_ip_allocation_fix_constraints.py | 45 +++++++++++++++++++ .../alembic_migrations/versions/HEAD | 2 +- gbpservice/neutron/extensions/group_policy.py | 5 +++ .../db/grouppolicy/test_group_policy_db.py | 17 +++++++ .../services/grouppolicy/test_apic_mapping.py | 13 ++++-- .../grouppolicy/test_resource_mapping.py | 11 ++++- requirements.txt | 2 + 8 files changed, 96 insertions(+), 10 deletions(-) create mode 100644 gbpservice/neutron/db/migration/alembic_migrations/versions/1fadeb573886_es_ip_allocation_fix_constraints.py diff --git a/gbpservice/neutron/db/grouppolicy/group_policy_db.py b/gbpservice/neutron/db/grouppolicy/group_policy_db.py index f6aa28e07..941ceb1c2 100644 --- a/gbpservice/neutron/db/grouppolicy/group_policy_db.py +++ b/gbpservice/neutron/db/grouppolicy/group_policy_db.py @@ -139,9 +139,6 @@ class L2Policy(gquota.GBPQuotaBase, model_base.BASEV2, models_v2.HasId, class ESToL3PAssociation(model_base.BASEV2): """Many to many consuming relation between ESs and L3Ps.""" __tablename__ = 'gp_es_to_l3p_associations' - __table_args__ = ( - sa.UniqueConstraint('external_segment_id', 'allocated_address'), - ) l3_policy_id = sa.Column(sa.String(36), sa.ForeignKey('gp_l3_policies.id'), primary_key=True) external_segment_id = sa.Column( @@ -811,6 +808,14 @@ class GroupPolicyDbPlugin(gpolicy.GroupPolicyPluginBase, l3p_db.external_segments.append(assoc) else: # Create address allocation + existing = context.session.query( + ESToL3PAssociation).filter_by( + external_segment_id=es['id']).filter( + ESToL3PAssociation.allocated_address.in_( + es_dict[es['id']])).all() + if existing: + raise gpolicy.IpAddressOverlappingInExternalSegment( + es_id=es['id']) for ip in es_dict[es['id']]: assoc = ESToL3PAssociation( external_segment_id=es['id'], diff --git a/gbpservice/neutron/db/migration/alembic_migrations/versions/1fadeb573886_es_ip_allocation_fix_constraints.py b/gbpservice/neutron/db/migration/alembic_migrations/versions/1fadeb573886_es_ip_allocation_fix_constraints.py new file mode 100644 index 000000000..219d04117 --- /dev/null +++ b/gbpservice/neutron/db/migration/alembic_migrations/versions/1fadeb573886_es_ip_allocation_fix_constraints.py @@ -0,0 +1,45 @@ +# Copyright 2014 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. +# + + +"""es_ip_allocation_fix_constraints +""" + +# revision identifiers, used by Alembic. +revision = '1fadeb573886' +down_revision = '81d13acfbb80' + +from alembic import op +from neutron.db import migration +from sqlalchemy.engine import reflection + + +def upgrade(active_plugins=None, options=None): + inspector = reflection.Inspector.from_engine(op.get_bind()) + unique_constraints = inspector.get_unique_constraints( + 'gp_es_to_l3p_associations') + for constraint in unique_constraints: + if constraint['column_names'] == ['external_segment_id', + 'allocated_address']: + with migration.remove_fks_from_table( + 'gp_es_to_l3p_associations'): + op.drop_constraint(constraint['name'], + 'gp_es_to_l3p_associations', + 'unique') + break + + +def downgrade(active_plugins=None, options=None): + pass diff --git a/gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD b/gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD index da2776e17..908d20055 100644 --- a/gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD +++ b/gbpservice/neutron/db/migration/alembic_migrations/versions/HEAD @@ -1 +1 @@ -81d13acfbb80 +1fadeb573886 \ No newline at end of file diff --git a/gbpservice/neutron/extensions/group_policy.py b/gbpservice/neutron/extensions/group_policy.py index 7f417a81c..daa96dc75 100644 --- a/gbpservice/neutron/extensions/group_policy.py +++ b/gbpservice/neutron/extensions/group_policy.py @@ -187,6 +187,11 @@ class GroupPolicyInvalidProtocol(nexc.InvalidInput): "representation (0 to 255) are supported.") +class IpAddressOverlappingInExternalSegment(nexc.BadRequest): + message = _("One or more requested IP addresses are already allocated for " + "External Segment %(es_id)s.") + + # Group Policy Values gp_supported_actions = [None, gp_constants.GP_ACTION_ALLOW, gp_constants.GP_ACTION_REDIRECT] diff --git a/gbpservice/neutron/tests/unit/db/grouppolicy/test_group_policy_db.py b/gbpservice/neutron/tests/unit/db/grouppolicy/test_group_policy_db.py index 3cdc1f8a9..3a22095de 100644 --- a/gbpservice/neutron/tests/unit/db/grouppolicy/test_group_policy_db.py +++ b/gbpservice/neutron/tests/unit/db/grouppolicy/test_group_policy_db.py @@ -1315,6 +1315,23 @@ class TestGroupResources(GroupPolicyDbTestCase): expected = set([ptg1_1['id'], ptg1_2['id'], ptg1_3['id']]) self.assertEqual(expected, found) + def test_multiple_l3p_in_es_default_address(self): + es = self.create_external_segment()['external_segment'] + es_dict = {es['id']: []} + self.create_l3_policy(external_segments=es_dict, + expected_res_status=201) + self.create_l3_policy(external_segments=es_dict, + expected_res_status=201) + es_dict = {es['id']: ['172.16.0.1', '172.16.0.2']} + self.create_l3_policy(external_segments=es_dict, + expected_res_status=201) + # First address is overlapping + es_dict = {es['id']: ['172.16.0.1', '172.16.0.3']} + res = self.create_l3_policy(external_segments=es_dict, + expected_res_status=400) + self.assertEqual('IpAddressOverlappingInExternalSegment', + res['NeutronError']['type']) + class TestQuotasForGBP(GroupPolicyDbTestCase): diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py index 5e9510447..5fbbc3a0f 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py @@ -717,9 +717,12 @@ class TestL3Policy(ApicMappingTestCase): l3p = self.create_l3_policy( shared=shared_l3p, tenant_id=es['tenant_id'] if not shared_es else 'another_tenant', - external_segments={es['id']: ['192.168.0.3']}, + external_segments={es['id']: []}, expected_res_status=201)['l3_policy'] + self.assertEqual(1, len(l3p['external_segments'][es['id']])) + self.assertEqual('192.168.0.2', l3p['external_segments'][es['id']][0]) + owner = self.common_tenant if shared_es else es['tenant_id'] l3p_owner = self.common_tenant if shared_l3p else l3p['tenant_id'] mgr = self.driver.apic_manager @@ -734,7 +737,7 @@ class TestL3Policy(ApicMappingTestCase): mgr.ensure_logical_node_profile_created.assert_called_once_with( es['id'], mocked.APIC_EXT_SWITCH, mocked.APIC_EXT_MODULE, - mocked.APIC_EXT_PORT, mocked.APIC_EXT_ENCAP, '192.168.0.3/24', + mocked.APIC_EXT_PORT, mocked.APIC_EXT_ENCAP, '192.168.0.2/24', owner=owner, router_id=APIC_EXTERNAL_RID, transaction=mock.ANY) @@ -780,7 +783,9 @@ class TestL3Policy(ApicMappingTestCase): shared=shared_l3p)['l3_policy'] l3p = self.update_l3_policy( l3p['id'], tenant_id=l3p['tenant_id'], expected_res_status=200, - external_segments={es['id']: ['192.168.0.3']})['l3_policy'] + external_segments={es['id']: []})['l3_policy'] + self.assertEqual(1, len(l3p['external_segments'][es['id']])) + self.assertEqual('192.168.0.2', l3p['external_segments'][es['id']][0]) mgr = self.driver.apic_manager owner = self.common_tenant if shared_es else es['tenant_id'] @@ -795,7 +800,7 @@ class TestL3Policy(ApicMappingTestCase): mgr.ensure_external_routed_network_created.call_args_list) mgr.ensure_logical_node_profile_created.assert_called_once_with( es['id'], mocked.APIC_EXT_SWITCH, mocked.APIC_EXT_MODULE, - mocked.APIC_EXT_PORT, mocked.APIC_EXT_ENCAP, '192.168.0.3/24', + mocked.APIC_EXT_PORT, mocked.APIC_EXT_ENCAP, '192.168.0.2/24', owner=owner, router_id=APIC_EXTERNAL_RID, transaction=mock.ANY) diff --git a/gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py b/gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py index 2cd92b515..4ea3bfeb1 100644 --- a/gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py +++ b/gbpservice/neutron/tests/unit/services/grouppolicy/test_resource_mapping.py @@ -1296,6 +1296,9 @@ class TestL3Policy(ResourceMappingTestCase): l3p = self.create_l3_policy( ip_pool='192.168.0.0/16', expected_res_status=201, external_segments=external_segments) + # IP address is set in the API + self.assertEqual(1, len( + l3p['l3_policy']['external_segments'][es1['id']])) req = self.new_delete_request('l3_policies', l3p['l3_policy']['id']) req.get_response(self.ext_api) @@ -1322,9 +1325,13 @@ class TestL3Policy(ResourceMappingTestCase): ip_pool='192.168.0.0/16')['l3_policy'] for external_segments in [{es1['id']: []}, {es2['id']: []}]: - self.update_l3_policy( + l3p = self.update_l3_policy( l3p['id'], expected_res_status=200, - external_segments=external_segments) + external_segments=external_segments)[ + 'l3_policy'] + self.assertEqual( + 1, len(l3p['external_segments'][ + external_segments.keys()[0]])) # es2 to [es1, es2] external_segments = {es2['id']: [], es1['id']: []} res = self.update_l3_policy( diff --git a/requirements.txt b/requirements.txt index dc488f144..be1f354f4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,3 +3,5 @@ # process, which may cause wedges in the gate later. oslosphinx>=2.5.0,<2.6.0 # Apache-2.0 +oslo.utils<1.4.1 +oslo.i18n==1.5.0