Add a composite unique key to floatingip table in Neutron database
This patch set avoids associating multiple floating IPs to one fixed_ ip_address on one internal port when commands are executed concurrently by adding a composite unique key between floating_network_id, fixed_port _id and fixed_ip_address in floatingips table in Neutron database. This implies the following 2 use cases are allowed/supported in Neutron: 1. one port_id with different fixed_ip_address(es) can associate with floating_ip_address(es) from the same floating_network_id [1] 2. same fixed_ip_address can associate to same floating_network_id, as long as they are on different ports, because different internal networks could be using the same IP ranges. [1] https://bugs.launchpad.net/neutron/+bug/1057844 Change-Id: Ie8f3ec1b23c14f36992886510c3114cf956769d4 Closes-Bug: #1534445
This commit is contained in:
Lujin Luo
parent
dc6508aae2
commit
a9c3b7ef08
|
|
@ -142,6 +142,12 @@ class FloatingIP(standard_attr.HasStandardAttributes, model_base.BASEV2,
|
|||
last_known_router_id = sa.Column(sa.String(36))
|
||||
status = sa.Column(sa.String(16))
|
||||
router = orm.relationship(Router, backref='floating_ips')
|
||||
__table_args__ = (
|
||||
sa.UniqueConstraint(
|
||||
floating_network_id, fixed_port_id, fixed_ip_address,
|
||||
name=('uniq_floatingips0floatingnetworkid'
|
||||
'0fixedportid0fixedipaddress')),
|
||||
model_base.BASEV2.__table_args__,)
|
||||
|
||||
|
||||
class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
||||
|
|
|
|||
|
|
@ -1 +1 @@
|
|||
67daae611b6e
|
||||
6b461a21bcfc
|
||||
|
|
|
|||
|
|
@ -0,0 +1,72 @@
|
|||
# Copyright 2016 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.
|
||||
#
|
||||
|
||||
"""uniq_floatingips0floating_network_id0fixed_port_id0fixed_ip_addr
|
||||
|
||||
Revision ID: 6b461a21bcfc
|
||||
Revises: 67daae611b6e
|
||||
Create Date: 2016-06-03 16:00:38.273324
|
||||
|
||||
"""
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision = '6b461a21bcfc'
|
||||
down_revision = '67daae611b6e'
|
||||
|
||||
from alembic import op
|
||||
from neutron_lib import exceptions
|
||||
import sqlalchemy as sa
|
||||
|
||||
from neutron._i18n import _
|
||||
|
||||
|
||||
floatingips = sa.Table(
|
||||
'floatingips', sa.MetaData(),
|
||||
sa.Column('floating_network_id', sa.String(36)),
|
||||
sa.Column('fixed_port_id', sa.String(36)),
|
||||
sa.Column('fixed_ip_address', sa.String(64)))
|
||||
|
||||
|
||||
class DuplicateFloatingIPforOneFixedIP(exceptions.Conflict):
|
||||
message = _("Duplicate Floating IPs were created for fixed IP "
|
||||
"addresse(s) %(fixed_ip_address)s. Database cannot "
|
||||
"be upgraded. Please remove all duplicate Floating "
|
||||
"IPs before upgrading the database.")
|
||||
|
||||
|
||||
def upgrade():
|
||||
op.create_unique_constraint(
|
||||
'uniq_floatingips0floatingnetworkid0fixedportid0fixedipaddress',
|
||||
'floatingips',
|
||||
['floating_network_id', 'fixed_port_id', 'fixed_ip_address'])
|
||||
|
||||
|
||||
def check_sanity(connection):
|
||||
res = get_duplicate_floating_ip_for_one_fixed_ip(connection)
|
||||
if res:
|
||||
raise DuplicateFloatingIPforOneFixedIP(fixed_ip_address=",".join(res))
|
||||
|
||||
|
||||
def get_duplicate_floating_ip_for_one_fixed_ip(connection):
|
||||
insp = sa.engine.reflection.Inspector.from_engine(connection)
|
||||
if 'floatingips' not in insp.get_table_names():
|
||||
return []
|
||||
session = sa.orm.Session(bind=connection.connect())
|
||||
query = (session.query(floatingips.c.fixed_ip_address)
|
||||
.group_by(floatingips.c.floating_network_id,
|
||||
floatingips.c.fixed_port_id,
|
||||
floatingips.c.fixed_ip_address)
|
||||
.having(sa.func.count() > 1)).all()
|
||||
return [q[0] for q in query]
|
||||
|
|
@ -393,6 +393,29 @@ class TestSanityCheck(testlib_api.SqlTestCaseLight):
|
|||
self.assertRaises(script.DuplicatePortRecordinRouterPortdatabase,
|
||||
script.check_sanity, conn)
|
||||
|
||||
def test_check_sanity_6b461a21bcfc(self):
|
||||
floatingips = sqlalchemy.Table(
|
||||
'floatingips', sqlalchemy.MetaData(),
|
||||
sqlalchemy.Column('floating_network_id', sqlalchemy.String(36)),
|
||||
sqlalchemy.Column('fixed_port_id', sqlalchemy.String(36)),
|
||||
sqlalchemy.Column('fixed_ip_address', sqlalchemy.String(64)))
|
||||
|
||||
with self.engine.connect() as conn:
|
||||
floatingips.create(conn)
|
||||
conn.execute(floatingips.insert(), [
|
||||
{'floating_network_id': '12345',
|
||||
'fixed_port_id': '1234567',
|
||||
'fixed_ip_address': '12345678'},
|
||||
{'floating_network_id': '12345',
|
||||
'fixed_port_id': '1234567',
|
||||
'fixed_ip_address': '12345678'}
|
||||
])
|
||||
script_dir = alembic_script.ScriptDirectory.from_config(
|
||||
self.alembic_config)
|
||||
script = script_dir.get_revision("6b461a21bcfc").module
|
||||
self.assertRaises(script.DuplicateFloatingIPforOneFixedIP,
|
||||
script.check_sanity, conn)
|
||||
|
||||
|
||||
class TestWalkDowngrade(oslotest_base.BaseTestCase):
|
||||
|
||||
|
|
|
|||
|
|
@ -2217,6 +2217,121 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
|
|||
self.assertEqual(ip_address,
|
||||
body['floatingip']['fixed_ip_address'])
|
||||
|
||||
def test_floatingip_update_same_fixed_ip_same_port(self):
|
||||
with self.subnet() as private_sub:
|
||||
ip_range = list(netaddr.IPNetwork(private_sub['subnet']['cidr']))
|
||||
fixed_ip = [{'ip_address': str(ip_range[-3])}]
|
||||
with self.port(subnet=private_sub, fixed_ips=fixed_ip) as p:
|
||||
with self.router() as r:
|
||||
with self.subnet(cidr='11.0.0.0/24') as public_sub:
|
||||
self._set_net_external(
|
||||
public_sub['subnet']['network_id'])
|
||||
self._add_external_gateway_to_router(
|
||||
r['router']['id'],
|
||||
public_sub['subnet']['network_id'])
|
||||
self._router_interface_action(
|
||||
'add', r['router']['id'],
|
||||
private_sub['subnet']['id'], None)
|
||||
fip1 = self._make_floatingip(
|
||||
self.fmt,
|
||||
public_sub['subnet']['network_id'])
|
||||
fip2 = self._make_floatingip(
|
||||
self.fmt,
|
||||
public_sub['subnet']['network_id'])
|
||||
# 1. Update floating IP 1 with port_id and fixed_ip
|
||||
body_1 = self._update(
|
||||
'floatingips', fip1['floatingip']['id'],
|
||||
{'floatingip': {'port_id': p['port']['id'],
|
||||
'fixed_ip_address': str(ip_range[-3])}
|
||||
})
|
||||
self.assertEqual(str(ip_range[-3]),
|
||||
body_1['floatingip']['fixed_ip_address'])
|
||||
self.assertEqual(p['port']['id'],
|
||||
body_1['floatingip']['port_id'])
|
||||
# 2. Update floating IP 2 with port_id and fixed_ip
|
||||
# mock out the sequential check
|
||||
plugin = 'neutron.db.l3_db.L3_NAT_dbonly_mixin'
|
||||
check_get = mock.patch(
|
||||
plugin + '._check_and_get_fip_assoc',
|
||||
fip=fip2, floating_db=mock.ANY,
|
||||
return_value=(p['port']['id'], str(ip_range[-3]),
|
||||
r['router']['id']))
|
||||
check_and_get = check_get.start()
|
||||
# do regular _check_and_get_fip_assoc() after skip
|
||||
check_and_get.side_effect = check_get.stop()
|
||||
self._update(
|
||||
'floatingips', fip2['floatingip']['id'],
|
||||
{'floatingip':
|
||||
{'port_id': p['port']['id'],
|
||||
'fixed_ip_address': str(ip_range[-3])
|
||||
}}, exc.HTTPConflict.code)
|
||||
body = self._show('floatingips',
|
||||
fip2['floatingip']['id'])
|
||||
self.assertIsNone(
|
||||
body['floatingip']['fixed_ip_address'])
|
||||
self.assertIsNone(
|
||||
body['floatingip']['port_id'])
|
||||
|
||||
def test_create_multiple_floatingips_same_fixed_ip_same_port(self):
|
||||
'''This tests that if multiple API requests arrive to create
|
||||
floating IPs on same external network to same port with one
|
||||
fixed ip, the latter API requests would be blocked at
|
||||
database side.
|
||||
'''
|
||||
with self.router() as r:
|
||||
with self.subnet(cidr='11.0.0.0/24') as public_sub:
|
||||
self._set_net_external(public_sub['subnet']['network_id'])
|
||||
self._add_external_gateway_to_router(
|
||||
r['router']['id'],
|
||||
public_sub['subnet']['network_id'])
|
||||
|
||||
with self.subnet() as private_sub:
|
||||
ip_range = list(netaddr.IPNetwork(
|
||||
private_sub['subnet']['cidr']))
|
||||
fixed_ips = [{'ip_address': str(ip_range[-3])},
|
||||
{'ip_address': str(ip_range[-2])}]
|
||||
|
||||
self._router_interface_action(
|
||||
'add', r['router']['id'],
|
||||
private_sub['subnet']['id'], None)
|
||||
|
||||
with self.port(subnet=private_sub,
|
||||
fixed_ips=fixed_ips) as p:
|
||||
# 1. Create floating IP 1
|
||||
fip1 = self._make_floatingip(
|
||||
self.fmt,
|
||||
public_sub['subnet']['network_id'],
|
||||
p['port']['id'],
|
||||
fixed_ip=str(ip_range[-3]))
|
||||
# 2. Create floating IP 2
|
||||
# mock out the sequential check
|
||||
plugin = 'neutron.db.l3_db.L3_NAT_dbonly_mixin'
|
||||
check_get = mock.patch(
|
||||
plugin + '._check_and_get_fip_assoc',
|
||||
fip=mock.ANY, floating_db=mock.ANY,
|
||||
return_value=(p['port']['id'], str(ip_range[-3]),
|
||||
r['router']['id']))
|
||||
check_and_get = check_get.start()
|
||||
# do regular _check_and_get_fip_assoc() after skip
|
||||
check_and_get.side_effect = check_get.stop()
|
||||
self._make_floatingip(
|
||||
self.fmt,
|
||||
public_sub['subnet']['network_id'],
|
||||
p['port']['id'],
|
||||
fixed_ip=str(ip_range[-3]),
|
||||
http_status=exc.HTTPConflict.code)
|
||||
# Test that floating IP 1 is successfully created
|
||||
body = self._show('floatingips',
|
||||
fip1['floatingip']['id'])
|
||||
self.assertEqual(
|
||||
body['floatingip']['port_id'],
|
||||
fip1['floatingip']['port_id'])
|
||||
|
||||
self._delete('ports', p['port']['id'])
|
||||
# Test that port has been successfully deleted.
|
||||
body = self._show('ports', p['port']['id'],
|
||||
expected_code=exc.HTTPNotFound.code)
|
||||
|
||||
def test_first_floatingip_associate_notification(self):
|
||||
with self.port() as p:
|
||||
private_sub = {'subnet': {'id':
|
||||
|
|
|
|||
Loading…
Reference in New Issue