From 1990ce420168ffd6086d00d9441ff5c8e2a3a988 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 7 Oct 2021 15:57:47 +0100 Subject: [PATCH] db: Add missing foreign keys, indexes to models There were a number of discrepancies between the models and migrations (both sqlalchemy-migrate and alembic-based - they're identical). These need to be addressed. In this patch, we correct a number of discrepancies related to ForeignKey and Index constraints. Most of these take the form of adding the constraint to the model, but we also remove a number of indexes defined on the models but not actually present in the database (since they're not defined in migration). Change-Id: I0b53e2ccc0b03e24b8b6f67e67ea065ab5b85d07 Signed-off-by: Stephen Finucane --- cinder/db/sqlalchemy/api.py | 13 ++-- cinder/db/sqlalchemy/models.py | 81 +++++++++++++++++------ cinder/tests/unit/objects/test_objects.py | 2 + 3 files changed, 72 insertions(+), 24 deletions(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 16618190aee..41e83c935c8 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -1647,11 +1647,14 @@ def volume_data_get_for_project(context, project_id, host=None): skip_internal=False) -VOLUME_DEPENDENT_MODELS = frozenset([models.VolumeMetadata, - models.VolumeAdminMetadata, - models.Transfer, - models.VolumeGlanceMetadata, - models.VolumeAttachment]) +VOLUME_DEPENDENT_MODELS = frozenset([ + models.VolumeMetadata, + models.VolumeAdminMetadata, + models.Snapshot, + models.Transfer, + models.VolumeGlanceMetadata, + models.VolumeAttachment, +]) @require_admin_context diff --git a/cinder/db/sqlalchemy/models.py b/cinder/db/sqlalchemy/models.py index 17d873d1f4f..b530edb0db5 100644 --- a/cinder/db/sqlalchemy/models.py +++ b/cinder/db/sqlalchemy/models.py @@ -66,9 +66,13 @@ class Service(BASE, CinderBase): """Represents a running service on a host.""" __tablename__ = 'services' + __table_args__ = ( + sa.Index('services_uuid_idx', 'uuid', unique=True), + CinderBase.__table_args__, + ) id = sa.Column(sa.Integer, primary_key=True) - uuid = sa.Column(sa.String(36), nullable=True, index=True) + uuid = sa.Column(sa.String(36), nullable=True) cluster_name = sa.Column(sa.String(255), nullable=True) host = sa.Column(sa.String(255)) # , sa.ForeignKey('hosts.id')) binary = sa.Column(sa.String(255)) @@ -233,7 +237,12 @@ class CGSnapshot(BASE, CinderBase): id = sa.Column(sa.String(36), primary_key=True) - consistencygroup_id = sa.Column(sa.String(36), index=True) + consistencygroup_id = sa.Column( + sa.String(36), + sa.ForeignKey('consistencygroups.id'), + nullable=False, + index=True, + ) # TODO(stephenfin): Add nullable=False user_id = sa.Column(sa.String(255)) @@ -258,7 +267,13 @@ class GroupSnapshot(BASE, CinderBase): id = sa.Column(sa.String(36), primary_key=True) - group_id = sa.Column(sa.String(36), nullable=False, index=True) + group_id = sa.Column( + 'group_id', + sa.String(36), + sa.ForeignKey('groups.id'), + nullable=False, + index=True, + ) user_id = sa.Column(sa.String(255)) project_id = sa.Column(sa.String(255)) @@ -336,8 +351,17 @@ class Volume(BASE, CinderBase): source_volid = sa.Column(sa.String(36)) encryption_key_id = sa.Column(sa.String(36)) - consistencygroup_id = sa.Column(sa.String(36), index=True) - group_id = sa.Column(sa.String(36), index=True) + consistencygroup_id = sa.Column( + sa.String(36), + sa.ForeignKey('consistencygroups.id'), + index=True, + ) + group_id = sa.Column( + 'group_id', + sa.String(36), + sa.ForeignKey('groups.id'), + index=True, + ) bootable = sa.Column(sa.Boolean, default=False) multiattach = sa.Column(sa.Boolean, default=False) @@ -362,7 +386,11 @@ class Volume(BASE, CinderBase): primaryjoin='Volume.group_id == Group.id', ) - service_uuid = sa.Column(sa.String(36), index=True) + service_uuid = sa.Column( + sa.String(36), + sa.ForeignKey('services.uuid'), + nullable=True, + ) service = relationship( Service, backref="volumes", @@ -530,7 +558,6 @@ class VolumeTypeProjects(BASE, CinderBase): "volume_type_id", "project_id", "deleted", - name="uniq_volume_type_projects0volume_type_id0project_id0deleted", ), CinderBase.__table_args__, ) @@ -558,12 +585,7 @@ class GroupTypeProjects(BASE, CinderBase): __tablename__ = "group_type_projects" __table_args__ = ( - schema.UniqueConstraint( - "group_type_id", - "project_id", - "deleted", - name="uniq_group_type_projects0group_type_id0project_id0deleted", - ), + sa.UniqueConstraint('group_type_id', 'project_id', 'deleted'), CinderBase.__table_args__, ) @@ -750,7 +772,8 @@ class Quota(BASE, CinderBase): id = sa.Column(sa.Integer, primary_key=True) - project_id = sa.Column(sa.String(255), index=True) + # TODO(stephenfin): Add index=True + project_id = sa.Column(sa.String(255)) resource = sa.Column(sa.String(255), nullable=False) hard_limit = sa.Column(sa.Integer, nullable=True) @@ -784,6 +807,7 @@ class QuotaUsage(BASE, CinderBase): # we do soft deletes and there could be duplicated entries, so we add the # race_preventer field. __table_args__ = ( + sa.Index('quota_usage_project_resource_idx', 'project_id', 'resource'), sa.UniqueConstraint('project_id', 'resource', 'race_preventer'), CinderBase.__table_args__, ) @@ -791,7 +815,8 @@ class QuotaUsage(BASE, CinderBase): id = sa.Column(sa.Integer, primary_key=True) project_id = sa.Column(sa.String(255), index=True) - resource = sa.Column(sa.String(300), index=True) + # TODO(stephenfin): Add index=True + resource = sa.Column(sa.String(255)) in_use = sa.Column(sa.Integer, nullable=False) reserved = sa.Column(sa.Integer, nullable=False) @@ -879,9 +904,22 @@ class Snapshot(BASE, CinderBase): user_id = sa.Column(sa.String(255)) project_id = sa.Column(sa.String(255)) - volume_id = sa.Column(sa.String(36), index=True) - cgsnapshot_id = sa.Column(sa.String(36), index=True) - group_snapshot_id = sa.Column(sa.String(36), index=True) + volume_id = sa.Column( + sa.String(36), + sa.ForeignKey('volumes.id', name='snapshots_volume_id_fkey'), + nullable=False, + index=True, + ) + cgsnapshot_id = sa.Column( + sa.String(36), + sa.ForeignKey('cgsnapshots.id'), + index=True, + ) + group_snapshot_id = sa.Column( + sa.String(36), + sa.ForeignKey('group_snapshots.id'), + index=True, + ) status = sa.Column(sa.String(255)) progress = sa.Column(sa.String(255)) volume_size = sa.Column(sa.Integer) @@ -1160,7 +1198,12 @@ class Worker(BASE, CinderBase): status = sa.Column(sa.String(255), nullable=False) # Service that is currently processing the operation - service_id = sa.Column(sa.Integer, nullable=True, index=True) + service_id = sa.Column( + sa.Integer, + sa.ForeignKey('services.id'), + nullable=True, + index=True, + ) # To prevent claiming and updating races race_preventer = sa.Column(sa.Integer, nullable=False, default=0) diff --git a/cinder/tests/unit/objects/test_objects.py b/cinder/tests/unit/objects/test_objects.py index 1bb905da09d..96b5b8e6a55 100644 --- a/cinder/tests/unit/objects/test_objects.py +++ b/cinder/tests/unit/objects/test_objects.py @@ -129,12 +129,14 @@ class TestObjectVersions(test.TestCase): ('BackupImport', 'user_id'), ('CGSnapshot', 'project_id'), ('CGSnapshot', 'user_id'), + ('CGSnapshot', 'consistencygroup_id'), ('ConsistencyGroup', 'project_id'), ('ConsistencyGroup', 'user_id'), ('Group', 'user_id'), ('Group', 'project_id'), ('GroupType', 'name'), ('Service', 'frozen'), + ('Snapshot', 'volume_id'), ('Snapshot', 'volume_type_id'), ('Volume', 'volume_type_id'), }: