Merge pull request #3189 from quay/fix-v22-phase1-migration
Fix the V22 phase 1 migrations to use new tables for mapping rather than editing existing tables
This commit is contained in:
Joseph Schorr
GitHub
commit
92743ca57b
7 changed files with 128 additions and 57 deletions
|
|
@ -504,7 +504,7 @@ class User(BaseModel):
|
|||
RepositoryNotification, OAuthAuthorizationCode,
|
||||
RepositoryActionCount, TagManifestLabel,
|
||||
TeamSync, RepositorySearchScore,
|
||||
DeletedNamespace} | appr_classes | v22_classes
|
||||
DeletedNamespace} | appr_classes | v22_classes | transition_classes
|
||||
delete_instance_filtered(self, User, delete_nullable, skip_transitive_deletes)
|
||||
|
||||
|
||||
|
|
@ -652,7 +652,7 @@ class Repository(BaseModel):
|
|||
# are cleaned up directly
|
||||
skip_transitive_deletes = {RepositoryTag, RepositoryBuild, RepositoryBuildTrigger, BlobUpload,
|
||||
Image, TagManifest, TagManifestLabel, Label, DerivedStorageForImage,
|
||||
RepositorySearchScore} | appr_classes | v22_classes
|
||||
RepositorySearchScore} | appr_classes | v22_classes | transition_classes
|
||||
|
||||
delete_instance_filtered(self, Repository, delete_nullable, skip_transitive_deletes)
|
||||
|
||||
|
|
@ -1433,10 +1433,13 @@ class TagManifest(BaseModel):
|
|||
digest = CharField(index=True)
|
||||
json_data = TextField()
|
||||
|
||||
# Note: `manifest` will be back-filled by a worker and may not be present
|
||||
# currently.
|
||||
manifest = ForeignKeyField(Manifest, null=True, index=True)
|
||||
broken = BooleanField(null=True, index=True)
|
||||
|
||||
class TagManifestToManifest(BaseModel):
|
||||
""" NOTE: Only used for the duration of the migrations. """
|
||||
tag_manifest = ForeignKeyField(TagManifest, index=True, unique=True)
|
||||
manifest = ForeignKeyField(Manifest, index=True, unique=True)
|
||||
broken = BooleanField(index=True, default=False)
|
||||
|
||||
|
||||
class TagManifestLabel(BaseModel):
|
||||
""" TO BE DEPRECATED: Mapping from a tag manifest to a label.
|
||||
|
|
@ -1445,11 +1448,6 @@ class TagManifestLabel(BaseModel):
|
|||
annotated = ForeignKeyField(TagManifest, index=True)
|
||||
label = ForeignKeyField(Label)
|
||||
|
||||
# Note: `manifest_label` will be back-filled by a worker and may not be present
|
||||
# currently.
|
||||
manifest_label = ForeignKeyField(ManifestLabel, null=True, index=True)
|
||||
broken_manifest = BooleanField(null=True, index=True)
|
||||
|
||||
class Meta:
|
||||
database = db
|
||||
read_slaves = (read_slave,)
|
||||
|
|
@ -1458,11 +1456,24 @@ class TagManifestLabel(BaseModel):
|
|||
)
|
||||
|
||||
|
||||
class TagManifestLabelMap(BaseModel):
|
||||
""" NOTE: Only used for the duration of the migrations. """
|
||||
tag_manifest = ForeignKeyField(TagManifest, index=True)
|
||||
manifest = ForeignKeyField(Manifest, null=True, index=True)
|
||||
|
||||
label = ForeignKeyField(Label, index=True)
|
||||
|
||||
tag_manifest_label = ForeignKeyField(TagManifestLabel, index=True)
|
||||
manifest_label = ForeignKeyField(ManifestLabel, null=True, index=True)
|
||||
|
||||
broken_manifest = BooleanField(index=True, default=False)
|
||||
|
||||
|
||||
appr_classes = set([ApprTag, ApprTagKind, ApprBlobPlacementLocation, ApprManifestList,
|
||||
ApprManifestBlob, ApprBlob, ApprManifestListManifest, ApprManifest,
|
||||
ApprBlobPlacement])
|
||||
v22_classes = set([Manifest, ManifestLabel, ManifestBlob, ManifestLegacyImage])
|
||||
transition_classes = set([TagManifestToManifest, TagManifestLabelMap])
|
||||
|
||||
is_model = lambda x: inspect.isclass(x) and issubclass(x, BaseModel) and x is not BaseModel
|
||||
all_models = [model[1] for model in inspect.getmembers(sys.modules[__name__], is_model)]
|
||||
|
|
|
|||
|
|
@ -1,13 +1,13 @@
|
|||
"""Add V2_2 data models for Manifest, ManifestBlob and ManifestLegacyImage
|
||||
|
||||
Revision ID: 7734c7584421
|
||||
Revision ID: 9093adccc784
|
||||
Revises: 6c21e2cfb8b6
|
||||
Create Date: 2018-07-31 13:26:02.850353
|
||||
Create Date: 2018-08-06 16:07:50.222749
|
||||
|
||||
"""
|
||||
|
||||
# revision identifiers, used by Alembic.
|
||||
revision = '7734c7584421'
|
||||
revision = '9093adccc784'
|
||||
down_revision = '6c21e2cfb8b6'
|
||||
|
||||
from alembic import op
|
||||
|
|
@ -75,17 +75,39 @@ def upgrade(tables, tester):
|
|||
op.create_index('manifestlegacyimage_image_id', 'manifestlegacyimage', ['image_id'], unique=False)
|
||||
op.create_index('manifestlegacyimage_manifest_id', 'manifestlegacyimage', ['manifest_id'], unique=True)
|
||||
op.create_index('manifestlegacyimage_repository_id', 'manifestlegacyimage', ['repository_id'], unique=False)
|
||||
|
||||
op.add_column(u'tagmanifest', sa.Column('broken', sa.Boolean(), nullable=True))
|
||||
op.add_column(u'tagmanifest', sa.Column('manifest_id', sa.Integer(), nullable=True))
|
||||
op.create_index('tagmanifest_broken', 'tagmanifest', ['broken'], unique=False)
|
||||
op.create_index('tagmanifest_manifest_id', 'tagmanifest', ['manifest_id'], unique=False)
|
||||
op.create_foreign_key(op.f('fk_tagmanifest_manifest_id_manifest'), 'tagmanifest', 'manifest', ['manifest_id'], ['id'])
|
||||
op.add_column(u'tagmanifestlabel', sa.Column('broken_manifest', sa.Boolean(), nullable=True))
|
||||
op.add_column(u'tagmanifestlabel', sa.Column('manifest_label_id', sa.Integer(), nullable=True))
|
||||
op.create_index('tagmanifestlabel_broken_manifest', 'tagmanifestlabel', ['broken_manifest'], unique=False)
|
||||
op.create_index('tagmanifestlabel_manifest_label_id', 'tagmanifestlabel', ['manifest_label_id'], unique=False)
|
||||
op.create_foreign_key(op.f('fk_tagmanifestlabel_manifest_label_id_manifestlabel'), 'tagmanifestlabel', 'manifestlabel', ['manifest_label_id'], ['id'])
|
||||
op.create_table('tagmanifesttomanifest',
|
||||
sa.Column('id', sa.Integer(), nullable=False),
|
||||
sa.Column('tag_manifest_id', sa.Integer(), nullable=False),
|
||||
sa.Column('manifest_id', sa.Integer(), nullable=False),
|
||||
sa.Column('broken', sa.Boolean(), nullable=False, server_default=sa.sql.expression.false()),
|
||||
sa.ForeignKeyConstraint(['manifest_id'], ['manifest.id'], name=op.f('fk_tagmanifesttomanifest_manifest_id_manifest')),
|
||||
sa.ForeignKeyConstraint(['tag_manifest_id'], ['tagmanifest.id'], name=op.f('fk_tagmanifesttomanifest_tag_manifest_id_tagmanifest')),
|
||||
sa.PrimaryKeyConstraint('id', name=op.f('pk_tagmanifesttomanifest'))
|
||||
)
|
||||
op.create_index('tagmanifesttomanifest_broken', 'tagmanifesttomanifest', ['broken'], unique=False)
|
||||
op.create_index('tagmanifesttomanifest_manifest_id', 'tagmanifesttomanifest', ['manifest_id'], unique=True)
|
||||
op.create_index('tagmanifesttomanifest_tag_manifest_id', 'tagmanifesttomanifest', ['tag_manifest_id'], unique=True)
|
||||
op.create_table('tagmanifestlabelmap',
|
||||
sa.Column('id', sa.Integer(), nullable=False),
|
||||
sa.Column('tag_manifest_id', sa.Integer(), nullable=False),
|
||||
sa.Column('manifest_id', sa.Integer(), nullable=True),
|
||||
sa.Column('label_id', sa.Integer(), nullable=False),
|
||||
sa.Column('tag_manifest_label_id', sa.Integer(), nullable=False),
|
||||
sa.Column('manifest_label_id', sa.Integer(), nullable=True),
|
||||
sa.Column('broken_manifest', sa.Boolean(), nullable=False, server_default=sa.sql.expression.false()),
|
||||
sa.ForeignKeyConstraint(['label_id'], ['label.id'], name=op.f('fk_tagmanifestlabelmap_label_id_label')),
|
||||
sa.ForeignKeyConstraint(['manifest_id'], ['manifest.id'], name=op.f('fk_tagmanifestlabelmap_manifest_id_manifest')),
|
||||
sa.ForeignKeyConstraint(['manifest_label_id'], ['manifestlabel.id'], name=op.f('fk_tagmanifestlabelmap_manifest_label_id_manifestlabel')),
|
||||
sa.ForeignKeyConstraint(['tag_manifest_id'], ['tagmanifest.id'], name=op.f('fk_tagmanifestlabelmap_tag_manifest_id_tagmanifest')),
|
||||
sa.ForeignKeyConstraint(['tag_manifest_label_id'], ['tagmanifestlabel.id'], name=op.f('fk_tagmanifestlabelmap_tag_manifest_label_id_tagmanifestlabel')),
|
||||
sa.PrimaryKeyConstraint('id', name=op.f('pk_tagmanifestlabelmap'))
|
||||
)
|
||||
op.create_index('tagmanifestlabelmap_broken_manifest', 'tagmanifestlabelmap', ['broken_manifest'], unique=False)
|
||||
op.create_index('tagmanifestlabelmap_label_id', 'tagmanifestlabelmap', ['label_id'], unique=False)
|
||||
op.create_index('tagmanifestlabelmap_manifest_id', 'tagmanifestlabelmap', ['manifest_id'], unique=False)
|
||||
op.create_index('tagmanifestlabelmap_manifest_label_id', 'tagmanifestlabelmap', ['manifest_label_id'], unique=False)
|
||||
op.create_index('tagmanifestlabelmap_tag_manifest_id', 'tagmanifestlabelmap', ['tag_manifest_id'], unique=False)
|
||||
op.create_index('tagmanifestlabelmap_tag_manifest_label_id', 'tagmanifestlabelmap', ['tag_manifest_label_id'], unique=False)
|
||||
# ### end Alembic commands ###
|
||||
|
||||
for media_type in DOCKER_SCHEMA1_CONTENT_TYPES:
|
||||
|
|
@ -121,8 +143,19 @@ def upgrade(tables, tester):
|
|||
('repository_id', tester.TestDataType.Foreign('repository')),
|
||||
])
|
||||
|
||||
tester.populate_column('tagmanifest', 'manifest_id', tester.TestDataType.Foreign('manifest'))
|
||||
tester.populate_column('tagmanifestlabel', 'manifest_label_id', tester.TestDataType.Foreign('manifestlabel'))
|
||||
tester.populate_table('tagmanifesttomanifest', [
|
||||
('manifest_id', tester.TestDataType.Foreign('manifest')),
|
||||
('tag_manifest_id', tester.TestDataType.Foreign('tagmanifest')),
|
||||
])
|
||||
|
||||
tester.populate_table('tagmanifestlabelmap', [
|
||||
('manifest_id', tester.TestDataType.Foreign('manifest')),
|
||||
('tag_manifest_id', tester.TestDataType.Foreign('tagmanifest')),
|
||||
('tag_manifest_label_id', tester.TestDataType.Foreign('tagmanifestlabel')),
|
||||
('manifest_label_id', tester.TestDataType.Foreign('manifestlabel')),
|
||||
('label_id', tester.TestDataType.Foreign('label')),
|
||||
])
|
||||
|
||||
# ### end population of test data ### #
|
||||
|
||||
|
||||
|
|
@ -135,17 +168,8 @@ def downgrade(tables, tester):
|
|||
mediatype.c.name == op.inline_literal(media_type)))
|
||||
|
||||
# ### commands auto generated by Alembic - please adjust! ###
|
||||
op.drop_constraint(op.f('fk_tagmanifestlabel_manifest_label_id_manifestlabel'), 'tagmanifestlabel', type_='foreignkey')
|
||||
op.drop_index('tagmanifestlabel_manifest_label_id', table_name='tagmanifestlabel')
|
||||
op.drop_index('tagmanifestlabel_broken_manifest', table_name='tagmanifestlabel')
|
||||
op.drop_column(u'tagmanifestlabel', 'manifest_label_id')
|
||||
op.drop_column(u'tagmanifestlabel', 'broken_manifest')
|
||||
op.drop_constraint(op.f('fk_tagmanifest_manifest_id_manifest'), 'tagmanifest', type_='foreignkey')
|
||||
op.drop_index('tagmanifest_manifest_id', table_name='tagmanifest')
|
||||
op.drop_index('tagmanifest_broken', table_name='tagmanifest')
|
||||
op.drop_column(u'tagmanifest', 'manifest_id')
|
||||
op.drop_column(u'tagmanifest', 'broken')
|
||||
|
||||
op.drop_table('tagmanifestlabelmap')
|
||||
op.drop_table('tagmanifesttomanifest')
|
||||
op.drop_table('manifestlegacyimage')
|
||||
op.drop_table('manifestlabel')
|
||||
op.drop_table('manifestblob')
|
||||
|
|
@ -3,7 +3,7 @@ import logging
|
|||
from cachetools import lru_cache
|
||||
|
||||
from data.database import (Label, TagManifestLabel, MediaType, LabelSourceType, db_transaction,
|
||||
ManifestLabel)
|
||||
ManifestLabel, TagManifestLabelMap, TagManifestToManifest)
|
||||
from data.model import InvalidLabelKeyException, InvalidMediaTypeException, DataModelException
|
||||
from data.text import prefix_search
|
||||
from util.validation import validate_label_key
|
||||
|
|
@ -69,11 +69,20 @@ def create_manifest_label(tag_manifest, key, value, source_type_name, media_type
|
|||
|
||||
with db_transaction():
|
||||
label = Label.create(key=key, value=value, source_type=source_type_id, media_type=media_type_id)
|
||||
TagManifestLabel.create(annotated=tag_manifest, label=label,
|
||||
repository=tag_manifest.tag.repository)
|
||||
if tag_manifest.manifest is not None:
|
||||
ManifestLabel.create(manifest=tag_manifest.manifest, label=label,
|
||||
repository=tag_manifest.tag.repository)
|
||||
tag_manifest_label = TagManifestLabel.create(annotated=tag_manifest, label=label,
|
||||
repository=tag_manifest.tag.repository)
|
||||
try:
|
||||
mapping_row = TagManifestToManifest.get(tag_manifest=tag_manifest)
|
||||
if mapping_row.manifest:
|
||||
manifest_label = ManifestLabel.create(manifest=mapping_row.manifest, label=label,
|
||||
repository=tag_manifest.tag.repository)
|
||||
TagManifestLabelMap.create(manifest_label=manifest_label,
|
||||
tag_manifest_label=tag_manifest_label,
|
||||
label=label,
|
||||
manifest=mapping_row.manifest,
|
||||
tag_manifest=tag_manifest)
|
||||
except TagManifestToManifest.DoesNotExist:
|
||||
pass
|
||||
|
||||
return label
|
||||
|
||||
|
|
@ -120,6 +129,11 @@ def delete_manifest_label(label_uuid, tag_manifest):
|
|||
raise DataModelException('Cannot delete immutable label')
|
||||
|
||||
# Delete the mapping records and label.
|
||||
(TagManifestLabelMap
|
||||
.delete()
|
||||
.where(TagManifestLabelMap.label == label)
|
||||
.execute())
|
||||
|
||||
deleted_count = TagManifestLabel.delete().where(TagManifestLabel.label == label).execute()
|
||||
if deleted_count != 1:
|
||||
logger.warning('More than a single label deleted for matching label %s', label_uuid)
|
||||
|
|
|
|||
|
|
@ -10,7 +10,8 @@ from data.model import (image, db_transaction, DataModelException, _basequery,
|
|||
from data.database import (RepositoryTag, Repository, Image, ImageStorage, Namespace, TagManifest,
|
||||
RepositoryNotification, Label, TagManifestLabel, get_epoch_timestamp,
|
||||
db_for_update, Manifest, ManifestLabel, ManifestBlob,
|
||||
ManifestLegacyImage)
|
||||
ManifestLegacyImage, TagManifestToManifest,
|
||||
TagManifestLabelMap)
|
||||
from util.timedeltastring import convert_to_timedelta
|
||||
|
||||
|
||||
|
|
@ -358,11 +359,22 @@ def _delete_tags(repo, query_modifier=None):
|
|||
.join(RepositoryTag)
|
||||
.where(RepositoryTag.id << tags_to_delete))
|
||||
tag_manifest_ids_to_delete = [tagmanifest.id for tagmanifest in tag_manifests_to_delete]
|
||||
manifest_ids_to_delete = [tagmanifest.manifest_id for tagmanifest in tag_manifests_to_delete
|
||||
if tagmanifest.manifest is not None]
|
||||
|
||||
# Find all the new-style manifests to delete, if any.
|
||||
tmt_query = (TagManifestToManifest
|
||||
.select()
|
||||
.where(TagManifestToManifest.tag_manifest << tag_manifests_to_delete,
|
||||
TagManifestToManifest.broken == False))
|
||||
|
||||
manifest_ids_to_delete = [tmt.manifest_id for tmt in tmt_query]
|
||||
num_deleted_manifests = 0
|
||||
if len(tag_manifest_ids_to_delete) > 0:
|
||||
# Delete tag manifest -> manifest mapping entries.
|
||||
(TagManifestToManifest
|
||||
.delete()
|
||||
.where(TagManifestToManifest.tag_manifest << tag_manifest_ids_to_delete)
|
||||
.execute())
|
||||
|
||||
# Find the set of IDs for all the labels to delete.
|
||||
manifest_labels_query = (TagManifestLabel
|
||||
.select()
|
||||
|
|
@ -371,6 +383,11 @@ def _delete_tags(repo, query_modifier=None):
|
|||
label_ids = [manifest_label.label_id for manifest_label in manifest_labels_query]
|
||||
|
||||
# Delete all the mapping entries for labels.
|
||||
(TagManifestLabelMap
|
||||
.delete()
|
||||
.where(TagManifestLabelMap.tag_manifest << tag_manifest_ids_to_delete)
|
||||
.execute())
|
||||
|
||||
(TagManifestLabel
|
||||
.delete()
|
||||
.where(TagManifestLabel.repository == repo,
|
||||
|
|
@ -608,8 +625,9 @@ def _create_manifest(tag, manifest):
|
|||
ManifestBlob.create(manifest=manifest_row, repository=tag.repository, blob=image_storage,
|
||||
blob_index=index)
|
||||
|
||||
return TagManifest.create(tag=tag, digest=manifest.digest, json_data=manifest.bytes,
|
||||
manifest=manifest_row)
|
||||
tag_manifest = TagManifest.create(tag=tag, digest=manifest.digest, json_data=manifest.bytes)
|
||||
TagManifestToManifest.create(tag_manifest=tag_manifest, manifest=manifest_row)
|
||||
return tag_manifest
|
||||
|
||||
|
||||
def load_tag_manifest(namespace, repo_name, tag_name):
|
||||
|
|
|
|||
|
|
@ -11,7 +11,7 @@ from playhouse.test_utils import assert_query_count
|
|||
|
||||
from data import model, database
|
||||
from data.database import (Image, ImageStorage, DerivedStorageForImage, Label, TagManifestLabel,
|
||||
ApprBlob, Manifest, TagManifest)
|
||||
ApprBlob, Manifest, TagManifest, TagManifestToManifest)
|
||||
from image.docker.schema1 import DockerSchema1ManifestBuilder
|
||||
from test.fixtures import *
|
||||
|
||||
|
|
@ -162,7 +162,7 @@ def _get_dangling_label_count():
|
|||
|
||||
def _get_dangling_manifest_count():
|
||||
manifest_ids = set([current.id for current in Manifest.select()])
|
||||
referenced_by_tag_manifest = set([manifest.manifest_id for manifest in TagManifest.select()])
|
||||
referenced_by_tag_manifest = set([tmt.manifest_id for tmt in TagManifestToManifest.select()])
|
||||
return len(manifest_ids - referenced_by_tag_manifest)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -9,7 +9,7 @@ from mock import patch
|
|||
|
||||
from app import docker_v2_signing_key
|
||||
from data.database import (Image, RepositoryTag, ImageStorage, Repository, Manifest, ManifestBlob,
|
||||
ManifestLegacyImage)
|
||||
ManifestLegacyImage, TagManifestToManifest)
|
||||
from data.model.repository import create_repository
|
||||
from data.model.tag import (list_active_repo_tags, create_or_update_tag, delete_tag,
|
||||
get_matching_tags, _tag_alive, get_matching_tags_for_images,
|
||||
|
|
@ -237,12 +237,14 @@ def test_store_tag_manifest(initialized_db):
|
|||
tag_manifest, _ = store_tag_manifest('devtable', 'simple', 'sometag', manifest)
|
||||
|
||||
# Ensure we have the new-model expected rows.
|
||||
assert tag_manifest.manifest is not None
|
||||
assert tag_manifest.manifest.manifest_bytes == manifest.bytes
|
||||
assert tag_manifest.manifest.digest == str(manifest.digest)
|
||||
mapping_row = TagManifestToManifest.get(tag_manifest=tag_manifest)
|
||||
|
||||
assert mapping_row.manifest is not None
|
||||
assert mapping_row.manifest.manifest_bytes == manifest.bytes
|
||||
assert mapping_row.manifest.digest == str(manifest.digest)
|
||||
|
||||
blob_rows = {m.blob_id for m in
|
||||
ManifestBlob.select().where(ManifestBlob.manifest == tag_manifest.manifest)}
|
||||
ManifestBlob.select().where(ManifestBlob.manifest == mapping_row.manifest)}
|
||||
assert blob_rows == {s.id for s in storages}
|
||||
|
||||
assert ManifestLegacyImage.get(manifest=tag_manifest.manifest).image == tag_manifest.tag.image
|
||||
assert ManifestLegacyImage.get(manifest=mapping_row.manifest).image == tag_manifest.tag.image
|
||||
|
|
|
|||
|
|
@ -128,6 +128,8 @@ def test_create_repository(repo_name, expected_status, client):
|
|||
def test_get_repo(has_tag_manifest, client, initialized_db):
|
||||
with client_with_identity('devtable', client) as cl:
|
||||
if not has_tag_manifest:
|
||||
database.TagManifestLabelMap.delete().execute()
|
||||
database.TagManifestToManifest.delete().execute()
|
||||
database.TagManifestLabel.delete().execute()
|
||||
database.TagManifest.delete().execute()
|
||||
|
||||
|
|
|
|||
Reference in a new issue