Merge "Enforce unique instance uuid in data model"

This commit is contained in:
Jenkins 2014-12-05 11:23:34 +00:00 committed by Gerrit Code Review
commit 5eea055b8b
8 changed files with 462 additions and 1 deletions

View File

@ -918,6 +918,35 @@ class DbCommands(object):
admin_context = context.get_admin_context()
db.archive_deleted_rows(admin_context, max_rows)
@args('--delete', action='store_true', dest='delete',
help='If specified, automatically delete any records found where '
'instance_uuid is NULL.')
def null_instance_uuid_scan(self, delete=False):
"""Lists and optionally deletes database records where
instance_uuid is NULL.
"""
hits = migration.db_null_instance_uuid_scan(delete)
records_found = False
for table_name, records in six.iteritems(hits):
# Don't print anything for 0 hits
if records:
records_found = True
if delete:
print(_("Deleted %(records)d records "
"from table '%(table_name)s'.") %
{'records': records, 'table_name': table_name})
else:
print(_("There are %(records)d records in the "
"'%(table_name)s' table where the uuid or "
"instance_uuid column is NULL. Run this "
"command again with the --delete option after you "
"have backed up any necessary data.") %
{'records': records, 'table_name': table_name})
# check to see if we didn't find anything
if not records_found:
print(_('There were no records found where '
'instance_uuid was NULL.'))
class AgentBuildCommands(object):
"""Class for managing agent builds."""

View File

@ -37,3 +37,23 @@ def db_version():
def db_initial_version():
"""The starting version for the database."""
return IMPL.db_initial_version()
def db_null_instance_uuid_scan(delete=False):
"""Utility for scanning the database to look for NULL instance uuid rows.
Scans the backing nova database to look for table entries where
instances.uuid or instance_uuid columns are NULL (except for the
fixed_ips table since that can contain NULL instance_uuid entries by
design). Dumps the tables that have NULL instance_uuid entries or
optionally deletes them based on usage.
This tool is meant to be used in conjunction with the 267 database
migration script to detect and optionally cleanup NULL instance_uuid
records.
:param delete: If true, delete NULL instance_uuid records found, else
just query to see if they exist for reporting.
:returns: dict of table name to number of hits for NULL instance_uuid rows.
"""
return IMPL.db_null_instance_uuid_scan(delete=delete)

View File

@ -0,0 +1,125 @@
# Copyright 2014 IBM Corp.
#
# 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.
from migrate import UniqueConstraint
from oslo.db.sqlalchemy import utils
from sqlalchemy import MetaData
from sqlalchemy.sql import null
from nova import exception
from nova.i18n import _
from nova.openstack.common import log as logging
LOG = logging.getLogger(__name__)
UC_NAME = 'uniq_instances0uuid'
def scan_for_null_records(table, col_name, check_fkeys):
"""Queries the table looking for NULL instances of the given column.
:param col_name: The name of the column to look for in the table.
:param check_fkeys: If True, check the table for foreign keys back to the
instances table and if not found, return.
:raises: exception.ValidationError: If any records are found.
"""
if col_name in table.columns:
# NOTE(mriedem): filter out tables that don't have a foreign key back
# to the instances table since they could have stale data even if
# instances.uuid wasn't NULL.
if check_fkeys:
fkey_found = False
fkeys = table.c[col_name].foreign_keys or []
for fkey in fkeys:
if fkey.column.table.name == 'instances':
fkey_found = True
if not fkey_found:
return
records = len(list(
table.select().where(table.c[col_name] == null()).execute()
))
if records:
msg = _("There are %(records)d records in the "
"'%(table_name)s' table where the uuid or "
"instance_uuid column is NULL. These must be "
"manually cleaned up before the migration will pass. "
"Consider running the "
"'nova-manage db null_instance_uuid_scan' command.") % (
{'records': records, 'table_name': table.name})
raise exception.ValidationError(detail=msg)
def process_null_records(meta, scan=True):
"""Scans the database for null instance_uuid records for processing.
:param meta: sqlalchemy.MetaData object, assumes tables are reflected.
:param scan: If True, does a query and fails the migration if NULL instance
uuid entries found. If False, makes instances.uuid
non-nullable.
"""
if scan:
for table in reversed(meta.sorted_tables):
# NOTE(mriedem): There is a periodic task in the network manager
# that calls nova.db.api.fixed_ip_disassociate_all_by_timeout which
# will set fixed_ips.instance_uuid to None by design, so we have to
# skip the fixed_ips table otherwise we'll wipeout the pool of
# fixed IPs.
if table.name not in ('fixed_ips', 'shadow_fixed_ips'):
scan_for_null_records(table, 'instance_uuid', check_fkeys=True)
for table_name in ('instances', 'shadow_instances'):
table = meta.tables[table_name]
if scan:
scan_for_null_records(table, 'uuid', check_fkeys=False)
else:
# The record is gone so make the uuid column non-nullable.
table.columns.uuid.alter(nullable=False)
def upgrade(migrate_engine):
# NOTE(mriedem): We're going to load up all of the tables so we can find
# any with an instance_uuid column since those may be foreign keys back
# to the instances table and we want to cleanup those records first. We
# have to do this explicitly because the foreign keys in nova aren't
# defined with cascading deletes.
meta = MetaData(migrate_engine)
meta.reflect(migrate_engine)
# Scan the database first and fail if any NULL records found.
process_null_records(meta, scan=True)
# Now run the alter statements.
process_null_records(meta, scan=False)
# Create a unique constraint on instances.uuid for foreign keys.
instances = meta.tables['instances']
UniqueConstraint('uuid', table=instances, name=UC_NAME).create()
# NOTE(mriedem): We now have a unique index on instances.uuid from the
# 216_havana migration and a unique constraint on the same column, which
# is redundant but should not be a big performance penalty. We should
# clean this up in a later (separate) migration since it involves dropping
# any ForeignKeys on the instances.uuid column due to some index rename
# issues in older versions of MySQL. That is beyond the scope of this
# migration.
def downgrade(migrate_engine):
# drop the unique constraint on instances.uuid
UniqueConstraint('uuid',
table=utils.get_table(migrate_engine, 'instances'),
name=UC_NAME).drop()
# We can't bring the deleted records back but we can make uuid nullable.
for table_name in ('instances', 'shadow_instances'):
table = utils.get_table(migrate_engine, table_name)
table.columns.uuid.alter(nullable=True)

View File

@ -19,7 +19,9 @@ import os
from migrate import exceptions as versioning_exceptions
from migrate.versioning import api as versioning_api
from migrate.versioning.repository import Repository
from oslo.db.sqlalchemy import utils as db_utils
import sqlalchemy
from sqlalchemy.sql import null
from nova.db.sqlalchemy import api as db_session
from nova import exception
@ -70,6 +72,75 @@ def db_initial_version():
return INIT_VERSION
def _process_null_records(table, col_name, check_fkeys, delete=False):
"""Queries the database and optionally deletes the NULL records.
:param table: sqlalchemy.Table object.
:param col_name: The name of the column to check in the table.
:param check_fkeys: If True, check the table for foreign keys back to the
instances table and if not found, return.
:param delete: If true, run a delete operation on the table, else just
query for number of records that match the NULL column.
:returns: The number of records processed for the table and column.
"""
records = 0
if col_name in table.columns:
# NOTE(mriedem): filter out tables that don't have a foreign key back
# to the instances table since they could have stale data even if
# instances.uuid wasn't NULL.
if check_fkeys:
fkey_found = False
fkeys = table.c[col_name].foreign_keys or []
for fkey in fkeys:
if fkey.column.table.name == 'instances':
fkey_found = True
if not fkey_found:
return 0
if delete:
records = table.delete().where(
table.c[col_name] == null()
).execute().rowcount
else:
records = len(list(
table.select().where(table.c[col_name] == null()).execute()
))
return records
def db_null_instance_uuid_scan(delete=False):
"""Scans the database for NULL instance_uuid records.
:param delete: If true, delete NULL instance_uuid records found, else
just query to see if they exist for reporting.
:returns: dict of table name to number of hits for NULL instance_uuid rows.
"""
engine = get_engine()
meta = sqlalchemy.MetaData(bind=engine)
# NOTE(mriedem): We're going to load up all of the tables so we can find
# any with an instance_uuid column since those may be foreign keys back
# to the instances table and we want to cleanup those records first. We
# have to do this explicitly because the foreign keys in nova aren't
# defined with cascading deletes.
meta.reflect(engine)
# Keep track of all of the tables that had hits in the query.
processed = {}
for table in reversed(meta.sorted_tables):
# Ignore the fixed_ips table by design.
if table.name not in ('fixed_ips', 'shadow_fixed_ips'):
processed[table.name] = _process_null_records(
table, 'instance_uuid', check_fkeys=True, delete=delete)
# Now process the *instances tables.
for table_name in ('instances', 'shadow_instances'):
table = db_utils.get_table(engine, table_name)
processed[table.name] = _process_null_records(
table, 'uuid', check_fkeys=False, delete=delete)
return processed
def db_version_control(version=None):
repository = _find_migrate_repo()
versioning_api.version_control(get_engine(), repository, version)

View File

@ -169,6 +169,7 @@ class Instance(BASE, NovaBase):
'host', 'node', 'deleted'),
Index('instances_host_deleted_cleaned_idx',
'host', 'deleted', 'cleaned'),
schema.UniqueConstraint('uuid', name='uniq_instances0uuid'),
)
injected_files = []
@ -258,7 +259,7 @@ class Instance(BASE, NovaBase):
os_type = Column(String(255))
architecture = Column(String(255))
vm_mode = Column(String(255))
uuid = Column(String(36))
uuid = Column(String(36), nullable=False)
root_device_name = Column(String(255))
default_ephemeral_device = Column(String(255))

View File

@ -43,12 +43,15 @@ from oslo.db.sqlalchemy import test_base
from oslo.db.sqlalchemy import test_migrations
from oslo.db.sqlalchemy import utils as oslodbutils
import sqlalchemy
from sqlalchemy.engine import reflection
import sqlalchemy.exc
from sqlalchemy.sql import null
from nova.db import migration
from nova.db.sqlalchemy import migrate_repo
from nova.db.sqlalchemy import migration as sa_migration
from nova.db.sqlalchemy import utils as db_utils
from nova import exception
from nova.i18n import _
from nova import test
from nova.tests.unit import conf_fixture
@ -442,6 +445,102 @@ class NovaMigrationsCheckers(test_migrations.WalkVersionsMixin):
def _post_downgrade_266(self, engine):
self.assertTableNotExists(engine, 'tags')
def _pre_upgrade_267(self, engine):
# Create a fixed_ips row with a null instance_uuid (if not already
# there) to make sure that's not deleted.
fixed_ips = oslodbutils.get_table(engine, 'fixed_ips')
fake_fixed_ip = {'id': 1}
fixed_ips.insert().execute(fake_fixed_ip)
# Create an instance record with a valid (non-null) UUID so we make
# sure we don't do something stupid and delete valid records.
instances = oslodbutils.get_table(engine, 'instances')
fake_instance = {'id': 1, 'uuid': 'fake-non-null-uuid'}
instances.insert().execute(fake_instance)
# Add a null instance_uuid entry for the volumes table
# since it doesn't have a foreign key back to the instances table.
volumes = oslodbutils.get_table(engine, 'volumes')
fake_volume = {'id': '9c3c317e-24db-4d57-9a6f-96e6d477c1da'}
volumes.insert().execute(fake_volume)
def _check_267(self, engine, data):
# Make sure the column is non-nullable and the UC exists.
fixed_ips = oslodbutils.get_table(engine, 'fixed_ips')
self.assertTrue(fixed_ips.c.instance_uuid.nullable)
fixed_ip = fixed_ips.select(fixed_ips.c.id == 1).execute().first()
self.assertIsNone(fixed_ip.instance_uuid)
instances = oslodbutils.get_table(engine, 'instances')
self.assertFalse(instances.c.uuid.nullable)
inspector = reflection.Inspector.from_engine(engine)
constraints = inspector.get_unique_constraints('instances')
constraint_names = [constraint['name'] for constraint in constraints]
self.assertIn('uniq_instances0uuid', constraint_names)
# Make sure the instances record with the valid uuid is still there.
instance = instances.select(instances.c.id == 1).execute().first()
self.assertIsNotNone(instance)
# Check that the null entry in the volumes table is still there since
# we skipped tables that don't have FK's back to the instances table.
volumes = oslodbutils.get_table(engine, 'volumes')
self.assertTrue(volumes.c.instance_uuid.nullable)
volume = fixed_ips.select(
volumes.c.id == '9c3c317e-24db-4d57-9a6f-96e6d477c1da'
).execute().first()
self.assertIsNone(volume.instance_uuid)
def _post_downgrade_267(self, engine):
# Make sure the UC is gone and the column is nullable again.
instances = oslodbutils.get_table(engine, 'instances')
self.assertTrue(instances.c.uuid.nullable)
inspector = reflection.Inspector.from_engine(engine)
constraints = inspector.get_unique_constraints('instances')
constraint_names = [constraint['name'] for constraint in constraints]
self.assertNotIn('uniq_instances0uuid', constraint_names)
def test_migration_267(self):
# This is separate from test_walk_versions so we can test the case
# where there are non-null instance_uuid entries in the database which
# cause the 267 migration to fail.
engine = self.migrate_engine
self.migration_api.version_control(
engine, self.REPOSITORY, self.INIT_VERSION)
self.migration_api.upgrade(engine, self.REPOSITORY, 266)
# Create a consoles record with a null instance_uuid so
# we can test that the upgrade fails if that entry is found.
# NOTE(mriedem): We use the consoles table since that's the only table
# created in the 216 migration with a ForeignKey created on the
# instance_uuid table for sqlite.
consoles = oslodbutils.get_table(engine, 'consoles')
fake_console = {'id': 1}
consoles.insert().execute(fake_console)
# NOTE(mriedem): We handle the 267 migration where we expect to
# hit a ValidationError on the consoles table to have
# a null instance_uuid entry
ex = self.assertRaises(exception.ValidationError,
self.migration_api.upgrade,
engine, self.REPOSITORY, 267)
self.assertIn("There are 1 records in the "
"'consoles' table where the uuid or "
"instance_uuid column is NULL.",
ex.kwargs['detail'])
# Remove the consoles entry with the null instance_uuid column.
rows = consoles.delete().where(
consoles.c['instance_uuid'] == null()).execute().rowcount
self.assertEqual(1, rows)
# Now run the 267 upgrade again.
self.migration_api.upgrade(engine, self.REPOSITORY, 267)
# Make sure the consoles entry with the null instance_uuid
# was deleted.
console = consoles.select(consoles.c.id == 1).execute().first()
self.assertIsNone(console)
class TestNovaMigrationsSQLite(NovaMigrationsCheckers,
test_base.DbTestCase):

View File

@ -0,0 +1,85 @@
# Copyright 2014 IBM Corp.
#
# 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.
from migrate import UniqueConstraint
from oslo.db.sqlalchemy import utils as db_utils
from nova.db.sqlalchemy import api as db_api
from nova.db.sqlalchemy import migration
from nova import test
class TestNullInstanceUuidScanDB(test.TestCase):
# NOTE(mriedem): Copied from the 267 database migration.
def downgrade(self, migrate_engine):
UniqueConstraint('uuid',
table=db_utils.get_table(migrate_engine, 'instances'),
name='uniq_instances0uuid').drop()
for table_name in ('instances', 'shadow_instances'):
table = db_utils.get_table(migrate_engine, table_name)
table.columns.uuid.alter(nullable=True)
def setUp(self):
super(TestNullInstanceUuidScanDB, self).setUp()
self.engine = db_api.get_engine()
# When this test runs, we've already run the schema migration to make
# instances.uuid non-nullable, so we have to alter the table here
# so we can test against a real database.
self.downgrade(self.engine)
# Now create fake entries in the fixed_ips, consoles and
# instances table where (instance_)uuid is None for testing.
for table_name in ('fixed_ips', 'instances', 'consoles'):
table = db_utils.get_table(self.engine, table_name)
fake_record = {'id': 1}
table.insert().execute(fake_record)
def test_db_null_instance_uuid_scan_readonly(self):
results = migration.db_null_instance_uuid_scan(delete=False)
self.assertEqual(1, results.get('instances'))
self.assertEqual(1, results.get('consoles'))
# The fixed_ips table should be ignored.
self.assertNotIn('fixed_ips', results)
# Now pick a random table with an instance_uuid column and show it's
# in the results but with 0 hits.
self.assertEqual(0, results.get('instance_info_caches'))
# Make sure nothing was deleted.
for table_name in ('fixed_ips', 'instances', 'consoles'):
table = db_utils.get_table(self.engine, table_name)
record = table.select(table.c.id == 1).execute().first()
self.assertIsNotNone(record)
def test_db_null_instance_uuid_scan_delete(self):
results = migration.db_null_instance_uuid_scan(delete=True)
self.assertEqual(1, results.get('instances'))
self.assertEqual(1, results.get('consoles'))
# The fixed_ips table should be ignored.
self.assertNotIn('fixed_ips', results)
# Now pick a random table with an instance_uuid column and show it's
# in the results but with 0 hits.
self.assertEqual(0, results.get('instance_info_caches'))
# Make sure fixed_ips wasn't touched, but instances and instance_faults
# records were deleted.
fixed_ips = db_utils.get_table(self.engine, 'fixed_ips')
record = fixed_ips.select(fixed_ips.c.id == 1).execute().first()
self.assertIsNotNone(record)
consoles = db_utils.get_table(self.engine, 'consoles')
record = consoles.select(consoles.c.id == 1).execute().first()
self.assertIsNone(record)
instances = db_utils.get_table(self.engine, 'instances')
record = instances.select(instances.c.id == 1).execute().first()
self.assertIsNone(record)

View File

@ -22,6 +22,7 @@ import mock
from nova.cmd import manage
from nova import context
from nova import db
from nova.db import migration
from nova import exception
from nova.i18n import _
from nova import test
@ -336,6 +337,36 @@ class DBCommandsTestCase(test.TestCase):
def test_archive_deleted_rows_negative(self):
self.assertEqual(1, self.commands.archive_deleted_rows(-1))
@mock.patch.object(migration, 'db_null_instance_uuid_scan',
return_value={'foo': 0})
def test_null_instance_uuid_scan_no_records_found(self, mock_scan):
self.useFixture(fixtures.MonkeyPatch('sys.stdout',
StringIO.StringIO()))
self.commands.null_instance_uuid_scan()
self.assertIn("There were no records found", sys.stdout.getvalue())
@mock.patch.object(migration, 'db_null_instance_uuid_scan',
return_value={'foo': 1, 'bar': 0})
def _test_null_instance_uuid_scan(self, mock_scan, delete):
self.useFixture(fixtures.MonkeyPatch('sys.stdout',
StringIO.StringIO()))
self.commands.null_instance_uuid_scan(delete)
output = sys.stdout.getvalue()
if delete:
self.assertIn("Deleted 1 records from table 'foo'.", output)
self.assertNotIn("Deleted 0 records from table 'bar'.", output)
else:
self.assertIn("1 records in the 'foo' table", output)
self.assertNotIn("0 records in the 'bar' table", output)
self.assertNotIn("There were no records found", output)
def test_null_instance_uuid_scan_readonly(self):
self._test_null_instance_uuid_scan(delete=False)
def test_null_instance_uuid_scan_delete(self):
self._test_null_instance_uuid_scan(delete=True)
class ServiceCommandsTestCase(test.TestCase):
def setUp(self):