From a821ad2b0137c5adde3aed1fdd8ebe8c15ca8b49 Mon Sep 17 00:00:00 2001 From: Matt Jibson Date: Tue, 1 Sep 2015 15:53:32 -0400 Subject: [PATCH] Return an error on failed S3 uploads The previous change to this file didn't raise the error up to stream_write, and so the complete_upload function still ran because the loop was only broken. It errored because the data was already canceled. This is better than what we had before, which was to silently fail but report success (even internally to ourselves!) on bad image upload. This means we discovered a bug where a user could have failed during image upload, but quay would write that image to the repository, potentially writing broken images to S3. --- endpoints/v1/registry.py | 6 +++++- storage/cloud.py | 3 ++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/endpoints/v1/registry.py b/endpoints/v1/registry.py index 7267fa0df..e1838730d 100644 --- a/endpoints/v1/registry.py +++ b/endpoints/v1/registry.py @@ -252,7 +252,11 @@ def put_image_layer(namespace, repository, image_id): # Stream write the data to storage. with database.CloseForLongOperation(app.config): - store.stream_write(repo_image.storage.locations, layer_path, sr) + try: + store.stream_write(repo_image.storage.locations, layer_path, sr) + except IOError: + logger.exception('Exception when writing image data') + abort(520, 'Image %(image_id)s could not be written. Please try again.', image_id=image_id) # Append the computed checksum. csums = [] diff --git a/storage/cloud.py b/storage/cloud.py index 89ebf53aa..d333d0358 100644 --- a/storage/cloud.py +++ b/storage/cloud.py @@ -167,7 +167,8 @@ class _CloudStorage(BaseStorage): except IOError: app.metric_queue.put('MultipartUploadFailure', 1) mp.cancel_upload() - break + raise + app.metric_queue.put('MultipartUploadSuccess', 1) mp.complete_upload()