Merge "Add a unique key to port_id in routerports table"
This commit is contained in:
commit
f80bd6d121
|
@ -20,6 +20,7 @@ import netaddr
|
||||||
from neutron_lib.api import validators
|
from neutron_lib.api import validators
|
||||||
from neutron_lib import constants as l3_constants
|
from neutron_lib import constants as l3_constants
|
||||||
from neutron_lib import exceptions as n_exc
|
from neutron_lib import exceptions as n_exc
|
||||||
|
from oslo_db import exception as db_exc
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
from oslo_utils import excutils
|
from oslo_utils import excutils
|
||||||
from oslo_utils import uuidutils
|
from oslo_utils import uuidutils
|
||||||
|
@ -75,7 +76,8 @@ class RouterPort(model_base.BASEV2):
|
||||||
port_id = sa.Column(
|
port_id = sa.Column(
|
||||||
sa.String(36),
|
sa.String(36),
|
||||||
sa.ForeignKey('ports.id', ondelete="CASCADE"),
|
sa.ForeignKey('ports.id', ondelete="CASCADE"),
|
||||||
primary_key=True)
|
primary_key=True,
|
||||||
|
unique=True)
|
||||||
revises_on_change = ('router', )
|
revises_on_change = ('router', )
|
||||||
# The port_type attribute is redundant as the port table already specifies
|
# The port_type attribute is redundant as the port table already specifies
|
||||||
# it in DEVICE_OWNER.However, this redundancy enables more efficient
|
# it in DEVICE_OWNER.However, this redundancy enables more efficient
|
||||||
|
@ -792,6 +794,7 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
||||||
with p_utils.delete_port_on_error(self._core_plugin,
|
with p_utils.delete_port_on_error(self._core_plugin,
|
||||||
context, port['id']) as delmgr:
|
context, port['id']) as delmgr:
|
||||||
delmgr.delete_on_error = cleanup_port
|
delmgr.delete_on_error = cleanup_port
|
||||||
|
try:
|
||||||
with context.session.begin(subtransactions=True):
|
with context.session.begin(subtransactions=True):
|
||||||
router_port = RouterPort(
|
router_port = RouterPort(
|
||||||
port_id=port['id'],
|
port_id=port['id'],
|
||||||
|
@ -799,6 +802,21 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase,
|
||||||
port_type=device_owner
|
port_type=device_owner
|
||||||
)
|
)
|
||||||
context.session.add(router_port)
|
context.session.add(router_port)
|
||||||
|
except db_exc.DBDuplicateEntry:
|
||||||
|
qry = context.session.query(RouterPort).filter_by(
|
||||||
|
port_id=port['id']).one()
|
||||||
|
existing_router_id = qry['router_id']
|
||||||
|
raise n_exc.PortInUse(net_id=port['network_id'],
|
||||||
|
port_id=port['id'],
|
||||||
|
device_id=existing_router_id)
|
||||||
|
# Update owner after actual process again in order to
|
||||||
|
# make sure the records in routerports table and ports
|
||||||
|
# table are consistent.
|
||||||
|
else:
|
||||||
|
self._core_plugin.update_port(
|
||||||
|
context, port['id'], {'port': {
|
||||||
|
'device_id': router.id,
|
||||||
|
'device_owner': device_owner}})
|
||||||
|
|
||||||
gw_ips = []
|
gw_ips = []
|
||||||
gw_network_id = None
|
gw_network_id = None
|
||||||
|
|
|
@ -17,6 +17,7 @@ from neutron_lib.api import validators
|
||||||
from neutron_lib import constants as const
|
from neutron_lib import constants as const
|
||||||
from neutron_lib import exceptions as n_exc
|
from neutron_lib import exceptions as n_exc
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
|
from oslo_db import exception as db_exc
|
||||||
from oslo_log import helpers as log_helper
|
from oslo_log import helpers as log_helper
|
||||||
from oslo_log import log as logging
|
from oslo_log import log as logging
|
||||||
from oslo_utils import excutils
|
from oslo_utils import excutils
|
||||||
|
@ -330,6 +331,7 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
||||||
port['fixed_ips'][-1]['subnet_id'])
|
port['fixed_ips'][-1]['subnet_id'])
|
||||||
|
|
||||||
delmgr.delete_on_error = cleanup_port
|
delmgr.delete_on_error = cleanup_port
|
||||||
|
try:
|
||||||
with context.session.begin(subtransactions=True):
|
with context.session.begin(subtransactions=True):
|
||||||
router_port = l3_db.RouterPort(
|
router_port = l3_db.RouterPort(
|
||||||
port_id=port['id'],
|
port_id=port['id'],
|
||||||
|
@ -337,6 +339,21 @@ class L3_NAT_with_dvr_db_mixin(l3_db.L3_NAT_db_mixin,
|
||||||
port_type=device_owner
|
port_type=device_owner
|
||||||
)
|
)
|
||||||
context.session.add(router_port)
|
context.session.add(router_port)
|
||||||
|
except db_exc.DBDuplicateEntry:
|
||||||
|
qry = context.session.query(l3_db.RouterPort).filter_by(
|
||||||
|
port_id=port['id']).one()
|
||||||
|
existing_router_id = qry['router_id']
|
||||||
|
raise n_exc.PortInUse(net_id=port['network_id'],
|
||||||
|
port_id=port['id'],
|
||||||
|
device_id=existing_router_id)
|
||||||
|
# Update owner after actual process again in order to
|
||||||
|
# make sure the records in routerports table and ports
|
||||||
|
# table are consistent.
|
||||||
|
else:
|
||||||
|
self._core_plugin.update_port(
|
||||||
|
context, port['id'], {'port': {
|
||||||
|
'device_id': router.id,
|
||||||
|
'device_owner': device_owner}})
|
||||||
|
|
||||||
# NOTE: For IPv6 additional subnets added to the same
|
# NOTE: For IPv6 additional subnets added to the same
|
||||||
# network we need to update the CSNAT port with respective
|
# network we need to update the CSNAT port with respective
|
||||||
|
|
|
@ -1 +1 @@
|
||||||
3d0e74aa7d37
|
030a959ceafa
|
||||||
|
|
|
@ -0,0 +1,69 @@
|
||||||
|
# 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_routerports0port_id
|
||||||
|
|
||||||
|
Revision ID: 030a959ceafa
|
||||||
|
Revises: 3d0e74aa7d37
|
||||||
|
Create Date: 2016-06-21 11:33:13.043879
|
||||||
|
|
||||||
|
"""
|
||||||
|
|
||||||
|
# revision identifiers, used by Alembic.
|
||||||
|
revision = '030a959ceafa'
|
||||||
|
down_revision = '3d0e74aa7d37'
|
||||||
|
|
||||||
|
from alembic import op
|
||||||
|
import sqlalchemy as sa
|
||||||
|
|
||||||
|
from neutron._i18n import _
|
||||||
|
|
||||||
|
from neutron.common import exceptions
|
||||||
|
|
||||||
|
routerports = sa.Table(
|
||||||
|
'routerports', sa.MetaData(),
|
||||||
|
sa.Column('router_id', sa.String(36)),
|
||||||
|
sa.Column('port_id', sa.String(36)),
|
||||||
|
sa.Column('port_type', sa.String(255)))
|
||||||
|
|
||||||
|
|
||||||
|
class DuplicatePortRecordinRouterPortdatabase(exceptions.Conflict):
|
||||||
|
message = _("Duplicate port(s) %(port_id)s records exist in routerports "
|
||||||
|
"database. Database cannot be upgraded. Please remove all "
|
||||||
|
"duplicated records before upgrading the database.")
|
||||||
|
|
||||||
|
|
||||||
|
def upgrade():
|
||||||
|
op.create_unique_constraint(
|
||||||
|
'uniq_routerports0port_id',
|
||||||
|
'routerports',
|
||||||
|
['port_id'])
|
||||||
|
|
||||||
|
|
||||||
|
def check_sanity(connection):
|
||||||
|
res = get_duplicate_port_records_in_routerport_database(connection)
|
||||||
|
if res:
|
||||||
|
raise DuplicatePortRecordinRouterPortdatabase(port_id=",".join(res))
|
||||||
|
|
||||||
|
|
||||||
|
def get_duplicate_port_records_in_routerport_database(connection):
|
||||||
|
insp = sa.engine.reflection.Inspector.from_engine(connection)
|
||||||
|
if 'routerports' not in insp.get_table_names():
|
||||||
|
return []
|
||||||
|
session = sa.orm.Session(bind=connection.connect())
|
||||||
|
query = (session.query(routerports.c.port_id)
|
||||||
|
.group_by(routerports.c.port_id)
|
||||||
|
.having(sa.func.count() > 1)).all()
|
||||||
|
return [q[0] for q in query]
|
|
@ -378,6 +378,27 @@ class TestSanityCheck(testlib_api.SqlTestCaseLight):
|
||||||
self.assertRaises(script.DuplicateL3HARouterAgentPortBinding,
|
self.assertRaises(script.DuplicateL3HARouterAgentPortBinding,
|
||||||
script.check_sanity, conn)
|
script.check_sanity, conn)
|
||||||
|
|
||||||
|
def test_check_sanity_030a959ceafa(self):
|
||||||
|
routerports = sqlalchemy.Table(
|
||||||
|
'routerports', sqlalchemy.MetaData(),
|
||||||
|
sqlalchemy.Column('router_id', sqlalchemy.String(36)),
|
||||||
|
sqlalchemy.Column('port_id', sqlalchemy.String(36)),
|
||||||
|
sqlalchemy.Column('port_type', sqlalchemy.String(255)))
|
||||||
|
|
||||||
|
with self.engine.connect() as conn:
|
||||||
|
routerports.create(conn)
|
||||||
|
conn.execute(routerports.insert(), [
|
||||||
|
{'router_id': '1234', 'port_id': '12345',
|
||||||
|
'port_type': '123'},
|
||||||
|
{'router_id': '12343', 'port_id': '12345',
|
||||||
|
'port_type': '1232'}
|
||||||
|
])
|
||||||
|
script_dir = alembic_script.ScriptDirectory.from_config(
|
||||||
|
self.alembic_config)
|
||||||
|
script = script_dir.get_revision("030a959ceafa").module
|
||||||
|
self.assertRaises(script.DuplicatePortRecordinRouterPortdatabase,
|
||||||
|
script.check_sanity, conn)
|
||||||
|
|
||||||
|
|
||||||
class TestWalkDowngrade(oslotest_base.BaseTestCase):
|
class TestWalkDowngrade(oslotest_base.BaseTestCase):
|
||||||
|
|
||||||
|
|
|
@ -1241,6 +1241,35 @@ class L3NatTestCaseBase(L3NatTestCaseMixin):
|
||||||
None,
|
None,
|
||||||
p['port']['id'])
|
p['port']['id'])
|
||||||
|
|
||||||
|
def test_router_add_interface_dup_port(self):
|
||||||
|
'''This tests that if multiple routers add one port as their
|
||||||
|
interfaces. Only the first router's interface would be added
|
||||||
|
to this port. All the later requests would return exceptions.
|
||||||
|
'''
|
||||||
|
with self.router() as r1, self.router() as r2, self.network() as n:
|
||||||
|
with self.subnet(network=n) as s:
|
||||||
|
with self.port(subnet=s) as p:
|
||||||
|
self._router_interface_action('add',
|
||||||
|
r1['router']['id'],
|
||||||
|
None,
|
||||||
|
p['port']['id'])
|
||||||
|
# mock out the sequential check
|
||||||
|
plugin = 'neutron.db.l3_db.L3_NAT_dbonly_mixin'
|
||||||
|
with mock.patch(plugin + '._check_router_port',
|
||||||
|
port_id=p['port']['id'],
|
||||||
|
device_id=r2['router']['id']) as checkport:
|
||||||
|
checkport.return_value = p['port']
|
||||||
|
self._router_interface_action('add',
|
||||||
|
r2['router']['id'],
|
||||||
|
None,
|
||||||
|
p['port']['id'],
|
||||||
|
exc.HTTPConflict.code)
|
||||||
|
# clean-up
|
||||||
|
self._router_interface_action('remove',
|
||||||
|
r1['router']['id'],
|
||||||
|
None,
|
||||||
|
p['port']['id'])
|
||||||
|
|
||||||
def _assert_body_port_id_and_update_port(self, body, mock_update_port,
|
def _assert_body_port_id_and_update_port(self, body, mock_update_port,
|
||||||
port_id, device_id):
|
port_id, device_id):
|
||||||
self.assertNotIn('port_id', body)
|
self.assertNotIn('port_id', body)
|
||||||
|
|
Loading…
Reference in New Issue