chore: remove uploading filtering from imagestorage queries (PROJQUAY…

…-1914) (#764)

The "uploading" column is an artifact from depending on writing to the
Image table (see BlobUpload table instead). As of 3.4, Quay no longer
writes to that table, and is only needed until quayio moves away from
Clair v2, after which work to remove "glue" code and fully deprecate
the Image table (amongst other tables) can start.

This is done as a separate commit from the actual migration so that it
can be cherrypicked.

data/model/blob.py

@@ -80,7 +80,6 @@ def store_blob_record_and_temp_link_in_repo(
         except ImageStorage.DoesNotExist:
             storage = ImageStorage.create(
                 content_checksum=blob_digest,
-                uploading=False,
                 image_size=byte_count,
                 uncompressed_size=uncompressed_byte_count,
             )
@@ -217,7 +216,7 @@ def get_shared_blob(digest):
     """
     assert digest
     try:
-        return ImageStorage.get(content_checksum=digest, uploading=False)
+        return ImageStorage.get(content_checksum=digest)
     except ImageStorage.DoesNotExist:
         return None
 
@@ -235,22 +234,18 @@ def get_or_create_shared_blob(digest, byte_data, storage):
     assert storage
 
     try:
-        return ImageStorage.get(content_checksum=digest, uploading=False)
+        return ImageStorage.get(content_checksum=digest)
     except ImageStorage.DoesNotExist:
-        record = ImageStorage.create(
-            image_size=len(byte_data), content_checksum=digest, cas_path=True, uploading=True
-        )
         preferred = storage.preferred_locations[0]
         location_obj = ImageStorageLocation.get(name=preferred)
-        try:
-            storage.put_content([preferred], storage_model.get_layer_path(record), byte_data)
-            ImageStoragePlacement.create(storage=record, location=location_obj)
 
-            record.uploading = False
-            record.save()
-        except:
-            logger.exception("Exception when trying to write special layer %s", digest)
-            record.delete_instance()
-            raise
+        with db_transaction():
+            record = ImageStorage.create(image_size=len(byte_data), content_checksum=digest)
+            try:
+                storage.put_content([preferred], storage_model.get_layer_path(record), byte_data)
+                ImageStoragePlacement.create(storage=record, location=location_obj)
+            except:
+                logger.exception("Exception when trying to write special layer %s", digest)
+                raise
 
-        return record
+            return record

data/model/image.py

@@ -316,7 +316,7 @@ def find_create_or_link_image(
         .join(RepositoryPermission, JOIN.LEFT_OUTER)
         .switch(Repository)
         .join(Namespace, on=(Repository.namespace_user == Namespace.id))
-        .where(ImageStorage.uploading == False, Image.docker_image_id == docker_image_id)
+        .where(Image.docker_image_id == docker_image_id)
     )
 
     existing_image_query = _basequery.filter_to_repos_for_user(
@@ -524,10 +524,8 @@ def get_images_eligible_for_scan(clair_version):
     """
     Returns a query that gives all images eligible for a clair scan.
     """
-    return (
-        get_image_with_storage_and_parent_base()
-        .where(Image.security_indexed_engine < clair_version)
-        .where(ImageStorage.uploading == False)
+    return get_image_with_storage_and_parent_base().where(
+        Image.security_indexed_engine < clair_version
     )
 
 

data/model/oci/blob.py

@@ -25,7 +25,6 @@ def _lookup_blob_uploaded(repository, blob_digest):
             .where(
                 UploadedBlob.repository == repository,
                 ImageStorage.content_checksum == blob_digest,
-                ImageStorage.uploading == False,
             )
             .get()
         )
@@ -41,7 +40,6 @@ def _lookup_blob_in_repository(repository, blob_digest):
             .where(
                 ManifestBlob.repository == repository,
                 ImageStorage.content_checksum == blob_digest,
-                ImageStorage.uploading == False,
             )
             .get()
         )

data/model/oci/manifest.py

@@ -26,7 +26,7 @@
 from data.model.oci.tag import filter_to_alive_tags, create_temporary_tag_if_necessary
 from data.model.oci.label import create_manifest_label
 from data.model.oci.retriever import RepositoryContentRetriever
-from data.model.storage import lookup_repo_storages_by_content_checksum, create_v1_storage
+from data.model.storage import lookup_repo_storages_by_content_checksum
 from data.model.image import lookup_repository_images, get_image, synthesize_v1_image
 from image.docker.schema2 import EMPTY_LAYER_BLOB_DIGEST, EMPTY_LAYER_BYTES
 from image.docker.schema1 import ManifestException
@@ -443,7 +443,6 @@ def _build_blob_map(
             shared_blob = get_or_create_shared_blob(
                 EMPTY_LAYER_BLOB_DIGEST, EMPTY_LAYER_BYTES, storage
             )
-            assert not shared_blob.uploading
             assert shared_blob.content_checksum == EMPTY_LAYER_BLOB_DIGEST
             blob_map[EMPTY_LAYER_BLOB_DIGEST] = shared_blob
 

data/model/storage.py

@@ -255,7 +255,7 @@ def placements_to_filtered_paths_set(placements_list):
 
 
 def create_v1_storage(location_name):
-    storage = ImageStorage.create(cas_path=False, uploading=True)
+    storage = ImageStorage.create(cas_path=False)
     location = get_image_location_for_name(location_name)
     ImageStoragePlacement.create(location=location.id, storage=storage)
     storage.locations = {location_name}
@@ -382,7 +382,6 @@ def _lookup_repo_storages_by_content_checksum(repo, checksums, model_class):
                 ImageStorage.uuid,
                 ImageStorage.cas_path,
                 ImageStorage.uncompressed_size,
-                ImageStorage.uploading,
             )
             .join(model_class)
             .where(model_class.repository == repo, ImageStorage.content_checksum == checksum)

data/model/test/test_gc.py

@@ -615,7 +615,7 @@ def test_image_with_cas(default_tag_policy, initialized_db):
         preferred = storage.preferred_locations[0]
         storage.put_content({preferred}, storage.blob_path(digest), content)
 
-        image_storage = database.ImageStorage.create(content_checksum=digest, uploading=False)
+        image_storage = database.ImageStorage.create(content_checksum=digest)
         location = database.ImageStorageLocation.get(name=preferred)
         database.ImageStoragePlacement.create(location=location, storage=image_storage)
 
@@ -676,8 +676,8 @@ def test_images_shared_cas(default_tag_policy, initialized_db):
         preferred = storage.preferred_locations[0]
         storage.put_content({preferred}, storage.blob_path(digest), content)
 
-        is1 = database.ImageStorage.create(content_checksum=digest, uploading=False)
-        is2 = database.ImageStorage.create(content_checksum=digest, uploading=False)
+        is1 = database.ImageStorage.create(content_checksum=digest)
+        is2 = database.ImageStorage.create(content_checksum=digest)
 
         location = database.ImageStorageLocation.get(name=preferred)
 

data/model/test/test_image_sharing.py

@@ -43,8 +43,6 @@ def createStorage(storage, docker_image_id, repository=REPO, username=ADMIN_ACCE
     image = model.image.find_create_or_link_image(
         docker_image_id, repository_obj, username, {}, preferred
     )
-    image.storage.uploading = False
-    image.storage.save()
     return image.storage
 
 
@@ -282,11 +280,3 @@ def test_org_not_team_member_with_no_access(storage, initialized_db):
     assertDifferentStorage(
         storage, "the-image", first_storage, username=OUTSIDE_ORG_USER, repository=OUTSIDE_ORG_REPO
     )
-
-
-def test_no_link_to_uploading(storage, initialized_db):
-    still_uploading = createStorage(storage, "an-image", repository=PUBLIC_REPO)
-    still_uploading.uploading = True
-    still_uploading.save()
-
-    assertDifferentStorage(storage, "an-image", still_uploading)

data/registry_model/manifestbuilder.py

@@ -150,7 +150,6 @@ def assign_layer_blob(self, layer, blob, computed_checksums):
         Assigns a blob to a layer.
         """
         assert blob
-        assert not blob.uploading
 
         image_metadata = self._builder_state.image_metadata.get(layer.layer_id)
         if image_metadata is None:

data/registry_model/test/test_blobuploader.py

@@ -68,7 +68,6 @@ def test_basic_upload_blob(chunk_count, subchunk, registry_model):
 
     # Check the blob.
     assert blob.compressed_size == len(data)
-    assert not blob.uploading
     assert blob.digest == "sha256:" + hashlib.sha256(data).hexdigest()
 
     # Ensure the blob exists in storage and has the expected data.

data/registry_model/test/test_interface.py

@@ -645,7 +645,6 @@ def test_get_cached_repo_blob(registry_model):
     assert found.uuid == blob.uuid
     assert found.compressed_size == blob.compressed_size
     assert found.uncompressed_size == blob.uncompressed_size
-    assert found.uploading == blob.uploading
     assert found.placements == blob.placements
 
     # Disconnect from the database by overwriting the connection.
@@ -662,7 +661,6 @@ def fail(x, y):
         assert cached.uuid == blob.uuid
         assert cached.compressed_size == blob.compressed_size
         assert cached.uncompressed_size == blob.uncompressed_size
-        assert cached.uploading == blob.uploading
         assert cached.placements == blob.placements
 
         # Try another blob, which should fail since the DB is not connected and the cache

tools/orphans.py

@@ -4,7 +4,6 @@
 
 orphaned = (
     ImageStorage.select()
-    .where(ImageStorage.uploading == False)
     .join(Image, JOIN.LEFT_OUTER)
     .group_by(ImageStorage)
     .having(fn.Count(Image.id) == 0)

util/verifyplacements.py

@@ -33,7 +33,7 @@ def verify_placements():
     encountered = set()
 
     iterator = yield_random_entries(
-        lambda: ImageStorage.select().where(ImageStorage.uploading == False),
+        lambda: ImageStorage.select(),
         ImageStorage.id,
         1000,
         ImageStorage.select(fn.Max(ImageStorage.id)).scalar(),