Merge pull request #3369 from quay/fix-broken-manifest-builder

Fix broken manifest builder

data/registry_model/manifestbuilder.py

@@ -7,13 +7,15 @@ from collections import namedtuple
 from flask import session
 
 from data import model
-from data.database import db_transaction
+from data.database import db_transaction, ImageStorage
 from data.registry_model import registry_model
+from image.docker.schema2 import EMPTY_LAYER_BLOB_DIGEST
 
 logger = logging.getLogger(__name__)
 
 ManifestLayer = namedtuple('ManifestLayer', ['layer_id', 'v1_metadata_string', 'db_id'])
-_BuilderState = namedtuple('_BuilderState', ['builder_id', 'images', 'tags', 'checksums'])
+_BuilderState = namedtuple('_BuilderState', ['builder_id', 'images', 'tags', 'checksums',
+                                             'temp_storages'])
 
 _SESSION_KEY = '__manifestbuilder'
 
@@ -23,7 +25,7 @@ def create_manifest_builder(repository_ref, storage):
       and returns it. Returns None if the builder could not be constructed.
   """
   builder_id = str(uuid.uuid4())
-  builder = _ManifestBuilder(repository_ref, _BuilderState(builder_id, {}, {}, {}), storage)
+  builder = _ManifestBuilder(repository_ref, _BuilderState(builder_id, {}, {}, {}, []), storage)
   builder._save_to_session()
   return builder
 
@@ -111,10 +113,6 @@ class _ManifestBuilder(object):
                                                       location_name)
       model.tag.create_temporary_hidden_tag(repository, created, temp_tag_expiration)
 
-      # Mark the image as uploading.
-      created.storage.uploading = True
-      created.storage.save()
-
       # Save its V1 metadata.
       command_list = v1_metadata.get('container_config', {}).get('Cmd', None)
       command = json.dumps(command_list) if command_list else None
@@ -155,7 +153,9 @@ class _ManifestBuilder(object):
       existing_storage = repo_image.storage
       repo_image.storage = blob._db_id
       repo_image.save()
-      existing_storage.delete_instance(recursive=True)
+
+      if existing_storage.uploading:
+        self._builder_state.temp_storages.append(existing_storage.id)
 
     self._builder_state.checksums[layer.layer_id] = computed_checksums
     self._save_to_session()
@@ -198,6 +198,15 @@ class _ManifestBuilder(object):
         and it is expected manifest builders will eventually time out if unused for an
         extended period of time.
     """
+    temp_storages = self._builder_state.temp_storages
+    for storage_id in temp_storages:
+      try:
+        storage = ImageStorage.get(id=storage_id)
+        if storage.uploading and storage.content_checksum != EMPTY_LAYER_BLOB_DIGEST:
+          storage.delete_instance()
+      except ImageStorage.DoesNotExist:
+        pass
+
     session.pop(_SESSION_KEY, None)
 
   def _save_to_session(self):

data/registry_model/shared.py

@@ -434,12 +434,24 @@ class SharedModel:
 
     # Add the leaf layer
     builder.add_layer(legacy_image_row.storage.content_checksum, legacy_image_row.v1_json_metadata)
+    if legacy_image_row.storage.uploading:
+      logger.error('Cannot add an uploading storage row: %s', legacy_image_row.storage.id)
+      return None
 
     for parent_image in parents:
+      if parent_image.storage.uploading:
+        logger.error('Cannot add an uploading storage row: %s', legacy_image_row.storage.id)
+        return None
+
       builder.add_layer(parent_image.storage.content_checksum, parent_image.v1_json_metadata)
 
     # Sign the manifest with our signing key.
-    return builder.build(docker_v2_signing_key)
+    try:
+      return builder.build(docker_v2_signing_key)
+    except ManifestException as me:
+      logger.exception('Got exception when trying to build manifest for legacy image %s',
+                       legacy_image_row)
+      return None
 
   def _get_shared_storage(self, blob_digest):
     """ Returns an ImageStorage row for the blob digest if it is a globally shared storage. """