From bdae32630e144865e8c7fa065ee4c7c92e2bf0ea Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 4 Mar 2019 16:47:46 -0500 Subject: [PATCH] Fix V1 push for layers already uploaded --- data/model/blob.py | 13 ++++++++++--- data/model/test/test_model_blob.py | 6 ++++-- endpoints/v1/registry.py | 5 ----- test/registry/registry_tests.py | 21 +++++++++++++++++++++ 4 files changed, 35 insertions(+), 10 deletions(-) diff --git a/data/model/blob.py b/data/model/blob.py index fcd5caefa..fcfd512f6 100644 --- a/data/model/blob.py +++ b/data/model/blob.py @@ -55,16 +55,23 @@ def store_blob_record_and_temp_link(namespace, repo_name, blob_digest, location_ """ Store a record of the blob and temporarily link it to the specified repository. """ assert blob_digest + assert byte_count is not None with db_transaction(): try: storage = ImageStorage.get(content_checksum=blob_digest) - storage.image_size = byte_count + save_changes = False - if uncompressed_byte_count is not None: + if storage.image_size is None: + storage.image_size = byte_count + save_changes = True + + if storage.uncompressed_size is None and uncompressed_byte_count is not None: storage.uncompressed_size = uncompressed_byte_count + save_changes = True - storage.save() + if save_changes: + storage.save() ImageStoragePlacement.get(storage=storage, location=location_obj) except ImageStorage.DoesNotExist: diff --git a/data/model/test/test_model_blob.py b/data/model/test/test_model_blob.py index 343a3c72c..b6053b353 100644 --- a/data/model/test/test_model_blob.py +++ b/data/model/test/test_model_blob.py @@ -21,8 +21,10 @@ def test_store_blob(initialized_db): blob_storage2 = model.blob.store_blob_record_and_temp_link(ADMIN_ACCESS_USER, REPO, digest, location, 2048, 0, 6000) assert blob_storage2.id == blob_storage.id - assert blob_storage2.image_size == 2048 - assert blob_storage2.uncompressed_size == 6000 + + # The sizes should be unchanged. + assert blob_storage2.image_size == 1024 + assert blob_storage2.uncompressed_size == 5000 # Add a new digest, ensure it has a new record. otherdigest = 'anotherdigest' diff --git a/endpoints/v1/registry.py b/endpoints/v1/registry.py index ff5db6bf5..0a75bff4a 100644 --- a/endpoints/v1/registry.py +++ b/endpoints/v1/registry.py @@ -161,11 +161,6 @@ def put_image_layer(namespace, repository, image_id): if repository_ref is None: abort(403) - logger.debug('Retrieving image') - legacy_image = registry_model.get_legacy_image(repository_ref, image_id) - if legacy_image is not None and not legacy_image.uploading: - exact_abort(409, 'Image already exists') - logger.debug('Checking for image in manifest builder') builder = lookup_manifest_builder(repository_ref, session.get('manifest_builder'), store) if builder is None: diff --git a/test/registry/registry_tests.py b/test/registry/registry_tests.py index 55c43fe86..5116e9915 100644 --- a/test/registry/registry_tests.py +++ b/test/registry/registry_tests.py @@ -53,6 +53,27 @@ def test_empty_layer(pusher, puller, images_with_empty_layer, liveserver_session credentials=credentials) +def test_empty_layer_push_again(pusher, puller, images_with_empty_layer, liveserver_session, + app_reloader): + """ Test: Push and pull of an image with an empty layer to a new repository and then push it + again. """ + credentials = ('devtable', 'password') + + # Push a new repository. + pusher.push(liveserver_session, 'devtable', 'newrepo', 'latest', images_with_empty_layer, + credentials=credentials) + + # Pull the repository to verify. + puller.pull(liveserver_session, 'devtable', 'newrepo', 'latest', images_with_empty_layer, + credentials=credentials) + + # Push to the repository again, to ensure everything is skipped properly. + options = ProtocolOptions() + options.skip_head_checks = True + pusher.push(liveserver_session, 'devtable', 'newrepo', 'latest', images_with_empty_layer, + credentials=credentials, options=options) + + def test_multi_layer_images_push_pull(pusher, puller, multi_layer_images, liveserver_session, app_reloader): """ Test: Basic push and pull of a multi-layered image to a new repository. """