Skip to content

Commit

Permalink
Rename unique constraints due to new convention.
Browse files Browse the repository at this point in the history
We have found that current constraint name convention allows us to create
constraints with duplicate names. It happens if we add constraints
in different tables with the same column names (for example, `name`,
`deleted`). In this case we can not create new constraint in mysql due to
database limitation (it requires constraint name to be unique within a
database). To solve this issue unique constraint name convention has
been changed from "uniq_c1_x_c2_x_c3" to "uniq_t0c10c20c3" where `t` is
a table name and columns `c1`, `c2`, `c3` are in UniqueConstraint.
Fixed unique constraints in models description.
Synced db code from oslo (function `_raise_if_duplicate_entry_error()`
modified respectively to new unique constraint name convention).

blueprint db-enforce-unique-keys
Fixes: bug 1182054

Change-Id: I4122dfb910a07a1a423781ebc22e9ce50596a05d
  • Loading branch information
vikt0rs committed Jun 7, 2013
1 parent 7a475d3 commit 64ce647
Show file tree
Hide file tree
Showing 7 changed files with 226 additions and 9 deletions.
@@ -0,0 +1,128 @@
# Copyright 2013 Mirantis Inc.
# All Rights Reserved
#
# 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.
#
# @author: Victor Sergeyev, Mirantis Inc.
#
# vim: tabstop=4 shiftwidth=4 softtabstop=4

from migrate.changeset import UniqueConstraint
from migrate import ForeignKeyConstraint
from sqlalchemy import MetaData, Table

from nova.db.sqlalchemy import utils

UC_DATA = {
# (table_name: ((columns,), old_uc_name_1), (columns,), old_uc_name_2)
"floating_ips": (
(("address", "deleted",), "uniq_address_x_deleted"),
),
"instance_type_projects": (
(("instance_type_id", "project_id", "deleted"),
"uniq_instance_type_id_x_project_id_x_deleted"),
),
"instance_types": (
(("name", "deleted"), "uniq_name_x_deleted"),
(("flavorid", "deleted",), "uniq_flavorid_x_deleted"),
),
"key_pairs": (
(("user_id", "name", "deleted"), "key_pairs_uniq_name_and_user_id"),
),
"networks": (
(("vlan", "deleted",), "uniq_vlan_x_deleted"),
),
"task_log": (
(("task_name", "host", "period_beginning", "period_ending"),
"uniq_task_name_x_host_x_period_beginning_x_period_ending"),
),
}
# some UC names are different for mysql and postgresql
UC_SPEC_DB_DATA = {
# {engine: {table_name: (((columns,), old_uc_name), (...))}}
"sqlite": {
"instance_info_caches": (
(("instance_uuid",), "instance_uuid"),
),
"virtual_interfaces": (
(("address",), "virtual_interfaces_instance_uuid_fkey"),
),
},
"mysql": {
"instance_info_caches": (
(("instance_uuid",), "instance_uuid"),
),
"virtual_interfaces": (
(("address",), "virtual_interfaces_instance_uuid_fkey"),
),
},
"postgresql": {
"instance_info_caches": (
(("instance_uuid",), "instance_info_caches_instance_uuid_key"),
),
"virtual_interfaces": (
(("address",), "virtual_interfaces_address_key"),
),
},
}


constraint_names = {
"instance_info_caches": "instance_info_caches_instance_uuid_fkey",
"virtual_interfaces": "virtual_interfaces_instance_uuid_fkey",
}


def _uc_rename(migrate_engine, upgrade=True):
UC_DATA.update(UC_SPEC_DB_DATA[migrate_engine.name])

meta = MetaData(bind=migrate_engine)

for table in UC_DATA:
t = Table(table, meta, autoload=True)

for columns, old_uc_name in UC_DATA[table]:
new_uc_name = "uniq_{0}0{1}".format(table, "0".join(columns))

if table in constraint_names and migrate_engine.name == "mysql":
instances = Table("instances", meta, autoload=True)

ForeignKeyConstraint(
columns=[t.c.instance_uuid],
refcolumns=[instances.c.uuid],
name=constraint_names[table]
).drop(engine=migrate_engine)

if upgrade:
old_name, new_name = old_uc_name, new_uc_name
else:
old_name, new_name = new_uc_name, old_uc_name

utils.drop_unique_constraint(migrate_engine, table,
old_name, *(columns))
UniqueConstraint(*columns, table=t, name=new_name).create()

if table in constraint_names and migrate_engine.name == "mysql":
ForeignKeyConstraint(
columns=[t.c.instance_uuid],
refcolumns=[instances.c.uuid],
name=constraint_names[table]
).create(engine=migrate_engine)


def upgrade(migrate_engine):
return _uc_rename(migrate_engine, upgrade=True)


def downgrade(migrate_engine):
return _uc_rename(migrate_engine, upgrade=False)
43 changes: 40 additions & 3 deletions nova/db/sqlalchemy/models.py
Expand Up @@ -246,6 +246,10 @@ class InstanceInfoCache(BASE, NovaBase):
Represents a cache of information about an instance
"""
__tablename__ = 'instance_info_caches'
__table_args__ = (
schema.UniqueConstraint(
"instance_uuid",
name="uniq_instance_info_caches0instance_uuid"),)
id = Column(Integer, primary_key=True, autoincrement=True)

# text column used for storing a json object of network data for api
Expand All @@ -262,6 +266,14 @@ class InstanceInfoCache(BASE, NovaBase):
class InstanceTypes(BASE, NovaBase):
"""Represent possible instance_types or flavor of VM offered."""
__tablename__ = "instance_types"

__table_args__ = (
schema.UniqueConstraint("flavorid", "deleted",
name="uniq_instance_types0flavorid0deleted"),
schema.UniqueConstraint("name", "deleted",
name="uniq_instance_types0name0deleted")
)

id = Column(Integer, primary_key=True)
name = Column(String(255))
memory_mb = Column(Integer)
Expand Down Expand Up @@ -552,7 +564,10 @@ class ProviderFirewallRule(BASE, NovaBase):
class KeyPair(BASE, NovaBase):
"""Represents a public key pair for ssh."""
__tablename__ = 'key_pairs'
__table_args__ = (schema.UniqueConstraint("name", "user_id"), )
__table_args__ = (
schema.UniqueConstraint("name", "user_id", "deleted",
name="uniq_key_pairs0user_id0name0deleted"),
)
id = Column(Integer, primary_key=True)

name = Column(String(255))
Expand Down Expand Up @@ -591,8 +606,11 @@ class Migration(BASE, NovaBase):
class Network(BASE, NovaBase):
"""Represents a network."""
__tablename__ = 'networks'
__table_args__ = (schema.UniqueConstraint("vpn_public_address",
"vpn_public_port"), )
__table_args__ = (
schema.UniqueConstraint("vlan", "deleted",
name="uniq_networks0vlan0deleted"),
)

id = Column(Integer, primary_key=True)
label = Column(String(255))

Expand Down Expand Up @@ -628,6 +646,10 @@ class Network(BASE, NovaBase):
class VirtualInterface(BASE, NovaBase):
"""Represents a virtual interface on an instance."""
__tablename__ = 'virtual_interfaces'
__table_args__ = (
schema.UniqueConstraint("address",
name="unique_virtual_interfaces0address"),
)
id = Column(Integer, primary_key=True)
address = Column(String(255), unique=True)
network_id = Column(Integer, nullable=False)
Expand Down Expand Up @@ -669,6 +691,10 @@ class FixedIp(BASE, NovaBase):
class FloatingIp(BASE, NovaBase):
"""Represents a floating ip that dynamically forwards to a fixed ip."""
__tablename__ = 'floating_ips'
__table_args__ = (
schema.UniqueConstraint("address", "deleted",
name="uniq_floating_ips0address0deleted"),
)
id = Column(Integer, primary_key=True)
address = Column(types.IPAddress())
fixed_ip_id = Column(Integer, nullable=True)
Expand Down Expand Up @@ -757,6 +783,11 @@ class InstanceSystemMetadata(BASE, NovaBase):
class InstanceTypeProjects(BASE, NovaBase):
"""Represent projects associated instance_types."""
__tablename__ = "instance_type_projects"
__table_args__ = (schema.UniqueConstraint(
"instance_type_id", "project_id", "deleted",
name="uniq_instance_type_projects0instance_type_id0project_id0deleted"
),
)
id = Column(Integer, primary_key=True)
instance_type_id = Column(Integer, ForeignKey('instance_types.id'),
nullable=False)
Expand Down Expand Up @@ -983,6 +1014,12 @@ class InstanceIdMapping(BASE, NovaBase):
class TaskLog(BASE, NovaBase):
"""Audit log for background periodic tasks."""
__tablename__ = 'task_log'
__table_args__ = (
schema.UniqueConstraint(
'task_name', 'host', 'period_beginning', 'period_ending',
name="uniq_task_log0task_name0host0period_beginning0period_ending"
),
)
id = Column(Integer, primary_key=True, nullable=False, autoincrement=True)
task_name = Column(String(255), nullable=False)
state = Column(String(255), nullable=False)
Expand Down
7 changes: 4 additions & 3 deletions nova/openstack/common/db/sqlalchemy/session.py
Expand Up @@ -386,14 +386,15 @@ def _raise_if_duplicate_entry_error(integrity_error, engine_name):
"""

def get_columns_from_uniq_cons_or_name(columns):
# note(boris-42): UniqueConstraint name convention: "uniq_c1_x_c2_x_c3"
# means that columns c1, c2, c3 are in UniqueConstraint.
# note(vsergeyev): UniqueConstraint name convention: "uniq_t0c10c2"
# where `t` it is table name, `0` it is delimiter and
# columns `c1`, `c2` are in UniqueConstraint.
uniqbase = "uniq_"
if not columns.startswith(uniqbase):
if engine_name == "postgresql":
return [columns[columns.index("_") + 1:columns.rindex("_")]]
return [columns]
return columns[len(uniqbase):].split("_x_")
return columns[len(uniqbase):].split("0")[1:]

if engine_name not in ["mysql", "sqlite", "postgresql"]:
return
Expand Down
4 changes: 2 additions & 2 deletions nova/tests/db/test_db_api.py
Expand Up @@ -4273,8 +4273,8 @@ def test_virtual_interface_get_by_address_data_error_exception(self):
"i.nv.ali.ip")

def test_virtual_interface_get_by_uuid(self):
vifs = [self._create_virt_interface({}),
self._create_virt_interface({})]
vifs = [self._create_virt_interface({"address": "address_1"}),
self._create_virt_interface({"address": "address_2"})]
for vif in vifs:
real_vif = db.virtual_interface_get_by_uuid(self.ctxt, vif['uuid'])
self._assertEqualObjects(vif, real_vif)
Expand Down
48 changes: 48 additions & 0 deletions nova/tests/db/test_migrations.py
Expand Up @@ -1416,6 +1416,54 @@ def _check_184(self, engine, data):
self.assertTrue(db_utils.check_shadow_table(engine, 'floating_ips'))
self.assertTrue(db_utils.check_shadow_table(engine, 'console_pools'))

def _unique_constraint_check_migrate_185(self, engine, check=True):
"""Test check unique constraint behavior. It should be the same before
and after migration because we changed their names only."""

data_list = [
("floating_ips", {'address': '10.12.14.16', 'deleted': 0}),
("instance_info_caches", {'instance_uuid': 'm161-uuid1'}),
('instance_type_projects', {'instance_type_id': 1,
'project_id': '116', 'deleted': 0}),
('instance_types', {'flavorid': "flavorid_12", 'deleted': 0,
'memory_mb': 64, 'vcpus': 10, 'swap': 100}),
('instance_types', {'name': "name_123", 'deleted': 0,
'memory_mb': 128, 'vcpus': 11, 'swap': 300}),
('key_pairs', {'user_id': 1, 'name': "name_qwer", 'deleted': 0}),
('networks', {'vlan': '123', 'deleted': 0}),
('task_log', {'task_name': 'task_123', 'host': 'localhost',
'period_beginning': datetime.datetime(2013, 02, 11),
'period_ending': datetime.datetime(2015, 01, 01),
'state': 'state_1', 'message': 'msg_1'}),
('virtual_interfaces', {'address': '192.168.0.0'})
]

for table_name, data in data_list:
table = db_utils.get_table(engine, table_name)
if not check:
table.insert().values(data).execute()
else:
# we replace values for some columns because they don't
# belong to unique constraint
if table_name == "instance_types":
for key in ("memory_mb", "vcpus", "swap"):
data[key] = data[key] * 2
if table_name == "task_log":
data["message"] = 'msg_2'
data["state"] = 'state_2'

self.assertRaises(sqlalchemy.exc.IntegrityError,
table.insert().execute, data)

def _pre_upgrade_185(self, engine):
self._unique_constraint_check_migrate_185(engine, False)

def check_185(self, engine):
self._unique_constraint_check_migrate_185(engine)

def _post_downgrade_185(self, engine):
self._unique_constraint_check_migrate_185(engine)


class TestBaremetalMigrations(BaseMigrationTestCase, CommonTestsMixIn):
"""Test sqlalchemy-migrate migrations."""
Expand Down
2 changes: 1 addition & 1 deletion nova/tests/utils.py
Expand Up @@ -58,7 +58,7 @@ def get_test_instance_type(context=None):
try:
instance_type_ref = nova.db.instance_type_create(context,
test_instance_type)
except exception.InstanceTypeExists:
except (exception.InstanceTypeExists, exception.InstanceTypeIdExists):
instance_type_ref = nova.db.instance_type_get_by_name(context,
'kinda.big')
return instance_type_ref
Expand Down
3 changes: 3 additions & 0 deletions nova/tests/virt/libvirt/test_libvirt.py
Expand Up @@ -2708,6 +2708,7 @@ def fake_get_info(instance):
instance_ref = self.test_instance
instance_ref['image_ref'] = ''
instance_ref['root_device_name'] = '/dev/vda'
instance_ref['uuid'] = uuidutils.generate_uuid()
instance = db.instance_create(self.context, instance_ref)

conn.spawn(self.context, instance, None, [], None,
Expand All @@ -2719,6 +2720,7 @@ def fake_get_info(instance):
instance_ref = self.test_instance
instance_ref['image_ref'] = 'my_fake_image'
instance_ref['root_device_name'] = '/dev/vda'
instance_ref['uuid'] = uuidutils.generate_uuid()
instance = db.instance_create(self.context, instance_ref)

conn.spawn(self.context, instance, None, [], None,
Expand All @@ -2728,6 +2730,7 @@ def fake_get_info(instance):

# Booted from an image
instance_ref['image_ref'] = 'my_fake_image'
instance_ref['uuid'] = uuidutils.generate_uuid()
instance = db.instance_create(self.context, instance_ref)
conn.spawn(self.context, instance, None, [], None)
self.assertTrue(self.cache_called_for_disk)
Expand Down

0 comments on commit 64ce647

Please sign in to comment.