womax/nova
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "db: De-duplicate list of removed table columns"

Browse Source
This commit is contained in:
Zuul
2021-11-03 09:36:48 +00:00
committed by Gerrit Code Review
parent 2030dd63cd a4d7f70740
commit 9c2d9fd1eb
6 changed files with 131 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files
nova/db/api/migrations/env.py
+14 -35
View File
@@ -32,42 +32,21 @@ target_metadata = models.BASE.metadata
def include_name(name, type_, parent_names):
"""Determine which tables or columns to skip.
This is used when we decide to "delete" a table or column. In this
instance, we will remove the SQLAlchemy model or field but leave the
underlying database table or column in place for a number of releases
after. Once we're sure that there is no code running that contains
references to the old models, we can then remove the underlying table. In
the interim, we must track the discrepancy between models and actual
database data here.
"""
if type_ == 'table':
return name not in models.REMOVED_TABLES
if type_ == 'column':
# NOTE(stephenfin): This is a list of fields that have been removed
# from various SQLAlchemy models but which still exist in the
# underlying tables. Our upgrade policy dictates that we remove fields
# from models at least one cycle before we remove the column from the
# underlying table. Not doing so would prevent us from applying the
# new database schema before rolling out any of the new code since the
# old code could attempt to access data in the removed columns. Alembic
# identifies this temporary mismatch between the models and underlying
# tables and attempts to resolve it. Tell it instead to ignore these
# until we're ready to remove them ourselves.
return (parent_names['table_name'], name) not in {
('build_requests', 'request_spec_id'),
('build_requests', 'user_id'),
('build_requests', 'display_name'),
('build_requests', 'instance_metadata'),
('build_requests', 'progress'),
('build_requests', 'vm_state'),
('build_requests', 'task_state'),
('build_requests', 'image_ref'),
('build_requests', 'access_ip_v4'),
('build_requests', 'access_ip_v6'),
('build_requests', 'info_cache'),
('build_requests', 'security_groups'),
('build_requests', 'config_drive'),
('build_requests', 'key_name'),
('build_requests', 'locked_by'),
('build_requests', 'reservation_id'),
('build_requests', 'launch_index'),
('build_requests', 'hostname'),
('build_requests', 'kernel_id'),
('build_requests', 'ramdisk_id'),
('build_requests', 'root_device_name'),
('build_requests', 'user_data'),
('resource_providers', 'can_host'),
}
return (parent_names['table_name'], name) not in models.REMOVED_COLUMNS
return True
nova/db/api/models.py
+45
View File
@@ -24,6 +24,51 @@ from nova.db import types
LOG = logging.getLogger(__name__)
# NOTE(stephenfin): This is a list of fields that have been removed from
# various SQLAlchemy models but which still exist in the underlying tables. Our
# upgrade policy dictates that we remove fields from models at least one cycle
# before we remove the column from the underlying table. Not doing so would
# prevent us from applying the new database schema before rolling out any of
# the new code since the old code could attempt to access data in the removed
# columns. Alembic identifies this temporary mismatch between the models and
# underlying tables and attempts to resolve it. Tell it instead to ignore these
# until we're ready to remove them ourselves.
REMOVED_COLUMNS = {
('build_requests', 'request_spec_id'),
('build_requests', 'user_id'),
('build_requests', 'display_name'),
('build_requests', 'instance_metadata'),
('build_requests', 'progress'),
('build_requests', 'vm_state'),
('build_requests', 'task_state'),
('build_requests', 'image_ref'),
('build_requests', 'access_ip_v4'),
('build_requests', 'access_ip_v6'),
('build_requests', 'info_cache'),
('build_requests', 'security_groups'),
('build_requests', 'config_drive'),
('build_requests', 'key_name'),
('build_requests', 'locked_by'),
('build_requests', 'reservation_id'),
('build_requests', 'launch_index'),
('build_requests', 'hostname'),
('build_requests', 'kernel_id'),
('build_requests', 'ramdisk_id'),
('build_requests', 'root_device_name'),
('build_requests', 'user_data'),
('resource_providers', 'can_host'),
}
# NOTE(stephenfin): A list of foreign key constraints that were removed when
# the column they were covering was removed.
REMOVED_FKEYS = [
('build_requests', ['request_spec_id']),
]
# NOTE(stephenfin): A list of entire models that have been removed.
REMOVED_TABLES = []
class _NovaAPIBase(models.ModelBase, models.TimestampMixin):
pass
nova/db/main/migrations/env.py
+15 -15
View File
@@ -32,27 +32,27 @@ target_metadata = models.BASE.metadata
def include_name(name, type_, parent_names):
"""Determine which tables or columns to skip.
This is used when we decide to "delete" a table or column. In this
instance, we will remove the SQLAlchemy model or field but leave the
underlying database table or column in place for a number of releases
after. Once we're sure that there is no code running that contains
references to the old models, we can then remove the underlying table. In
the interim, we must track the discrepancy between models and actual
database data here.
"""
if type_ == 'table':
# NOTE(stephenfin): We don't have models corresponding to the various
# shadow tables. Alembic doesn't like this. Tell Alembic to look the
# other way. Good Alembic.
return not name.startswith('shadow_')
if name.startswith('shadow_'):
return False
return name not in models.REMOVED_TABLES
if type_ == 'column':
# NOTE(stephenfin): This is a list of fields that have been removed
# from various SQLAlchemy models but which still exist in the
# underlying tables. Our upgrade policy dictates that we remove fields
# from models at least one cycle before we remove the column from the
# underlying table. Not doing so would prevent us from applying the
# new database schema before rolling out any of the new code since the
# old code could attempt to access data in the removed columns. Alembic
# identifies this temporary mismatch between the models and underlying
# tables and attempts to resolve it. Tell it instead to ignore these
# until we're ready to remove them ourselves.
return (parent_names['table_name'], name) not in {
('instances', 'internal_id'),
('instance_extra', 'vpmems'),
}
return (parent_names['table_name'], name) not in models.REMOVED_COLUMNS
return True
nova/db/main/models.py
+21
View File
@@ -32,6 +32,27 @@ from nova.db import types
CONF = cfg.CONF
# NOTE(stephenfin): This is a list of fields that have been removed from
# various SQLAlchemy models but which still exist in the underlying tables. Our
# upgrade policy dictates that we remove fields from models at least one cycle
# before we remove the column from the underlying table. Not doing so would
# prevent us from applying the new database schema before rolling out any of
# the new code since the old code could attempt to access data in the removed
# columns. Alembic identifies this temporary mismatch between the models and
# underlying tables and attempts to resolve it. Tell it instead to ignore these
# until we're ready to remove them ourselves.
REMOVED_COLUMNS = {
('instances', 'internal_id'),
('instance_extra', 'vpmems'),
}
# NOTE(stephenfin): A list of foreign key constraints that were removed when
# the column they were covering was removed.
REMOVED_FKEYS = []
# NOTE(stephenfin): A list of entire models that have been removed.
REMOVED_TABLES = []
# we don't configure 'cls' since we have models that don't use the
# TimestampMixin
BASE = declarative.declarative_base()
nova/tests/unit/db/api/test_migrations.py
+9 -28
View File
@@ -66,30 +66,17 @@ class NovaModelsMigrationsSync(test_migrations.ModelsMigrationsSync):
if name == 'migrate_version':
return False
# Define a whitelist of tables that will be removed from the DB in
# a later release and don't have a corresponding model anymore.
return name not in models.REMOVED_TABLES
return True
def filter_metadata_diff(self, diff):
# Filter out diffs that shouldn't cause a sync failure.
new_diff = []
# Define a whitelist of ForeignKeys that exist on the model but not in
# the database. They will be removed from the model at a later time.
fkey_whitelist = {'build_requests': ['request_spec_id']}
# Define a whitelist of columns that will be removed from the
# DB at a later release and aren't on a model anymore.
column_whitelist = {
'build_requests': [
'vm_state', 'instance_metadata',
'display_name', 'access_ip_v6', 'access_ip_v4', 'key_name',
'locked_by', 'image_ref', 'progress', 'request_spec_id',
'info_cache', 'user_id', 'task_state', 'security_groups',
'config_drive',
],
'resource_providers': ['can_host'],
}
for element in diff:
if isinstance(element, list):
# modify_nullable is a list
@@ -101,18 +88,12 @@ class NovaModelsMigrationsSync(test_migrations.ModelsMigrationsSync):
fkey = element[1]
tablename = fkey.table.name
column_keys = fkey.column_keys
if (
tablename in fkey_whitelist and
column_keys == fkey_whitelist[tablename]
):
if (tablename, column_keys) in models.REMOVED_FKEYS:
continue
elif element[0] == 'remove_column':
tablename = element[2]
column = element[3]
if (
tablename in column_whitelist and
column.name in column_whitelist[tablename]
):
table = element[2]
column = element[3].name
if (table, column) in models.REMOVED_COLUMNS:
continue
new_diff.append(element)
nova/tests/unit/db/main/test_migrations.py
+27 -20
View File
@@ -84,32 +84,39 @@ class NovaModelsMigrationsSync(test_migrations.ModelsMigrationsSync):
if name == 'migrate_version' or name.startswith('shadow_'):
return False
# Define a whitelist of tables that will be removed from the DB in
# a later release and don't have a corresponding model anymore.
return name not in models.REMOVED_TABLES
return True
def filter_metadata_diff(self, diff):
# Overriding the parent method to decide on certain attributes
# that maybe present in the DB but not in the models.py
# Filter out diffs that shouldn't cause a sync failure.
new_diff = []
def removed_column(element):
# Define a whitelist of columns that would be removed from the
# DB at a later release.
# NOTE(Luyao) The vpmems column was added to the schema in train,
# and removed from the model in train.
column_whitelist = {
'instances': ['internal_id'],
'instance_extra': ['vpmems'],
}
for element in diff:
if isinstance(element, list):
# modify_nullable is a list
new_diff.append(element)
else:
# tuple with action as first element. Different actions have
# different tuple structures.
if element[0] == 'add_fk':
fkey = element[1]
tablename = fkey.table.name
column_keys = fkey.column_keys
if (tablename, column_keys) in models.REMOVED_FKEYS:
continue
if element[0] == 'remove_column':
table = element[2]
column = element[3].name
if (table, column) in models.REMOVED_COLUMNS:
continue
if element[0] != 'remove_column':
return False
new_diff.append(element)
table_name, column = element[2], element[3]
return (
table_name in column_whitelist and
column.name in column_whitelist[table_name]
)
return [element for element in diff if not removed_column(element)]
return new_diff
class TestModelsSyncSQLite(