From 0d89fd7f265730308a394727bb879e0a8d37d004 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 2 Jul 2014 00:39:59 -0400 Subject: [PATCH 1/4] Add code for resumable downloads from S3 --- endpoints/registry.py | 13 +++++++++++-- storage/basestorage.py | 3 +++ storage/s3.py | 3 +++ 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/endpoints/registry.py b/endpoints/registry.py index 8fd54e19c..ded3f8915 100644 --- a/endpoints/registry.py +++ b/endpoints/registry.py @@ -101,6 +101,13 @@ def get_image_layer(namespace, repository, image_id, headers): profile.debug('Looking up repo image') repo_image = model.get_repo_image(namespace, repository, image_id) + # Add the Accept-Ranges header if the storage engine supports resumable + # downloads. + extra_headers = {} + + if store.get_supports_resumable_downloads(): + extra_headers['Accept-Ranges'] = 'bytes'; + profile.debug('Looking up the layer path') try: path = store.image_layer_path(repo_image.storage.uuid) @@ -110,10 +117,12 @@ def get_image_layer(namespace, repository, image_id, headers): if direct_download_url: profile.debug('Returning direct download URL') - return redirect(direct_download_url) + resp = redirect(direct_download_url) + resp.headers = dict(headers, **extra_headers) + return resp profile.debug('Streaming layer data') - return Response(store.stream_read(repo_image.storage.locations, path), headers=headers) + return Response(store.stream_read(repo_image.storage.locations, path), headers=dict(headers, **extra_headers)) except (IOError, AttributeError): profile.debug('Image not found') abort(404, 'Image %(image_id)s not found', issue='unknown-image', diff --git a/storage/basestorage.py b/storage/basestorage.py index b554845b4..94bdec615 100644 --- a/storage/basestorage.py +++ b/storage/basestorage.py @@ -57,6 +57,9 @@ class BaseStorage(StoragePaths): def get_direct_download_url(self, path, expires_in=60): return None + def get_supports_resumable_downloads(self): + return False + def get_content(self, path): raise NotImplementedError diff --git a/storage/s3.py b/storage/s3.py index 1747d34dc..add4ed9dc 100644 --- a/storage/s3.py +++ b/storage/s3.py @@ -83,6 +83,9 @@ class S3Storage(BaseStorage): key.set_contents_from_string(content, encrypt_key=True) return path + def get_supports_resumable_downloads(self): + return True + def get_direct_download_url(self, path, expires_in=60): self._initialize_s3() path = self._init_path(path) From 0ac6312c8ac553d3a7992cbf3128ee630eefffa5 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 2 Jul 2014 21:18:46 -0400 Subject: [PATCH 2/4] Get resumable downloads working in a way that the Docker CLI will actually understand. Also rename the method to conform the the name used in the Docker source code. --- endpoints/registry.py | 45 ++++++++++++++++++++++++++++------- storage/basestorage.py | 2 +- storage/distributedstorage.py | 1 + storage/s3.py | 2 +- 4 files changed, 40 insertions(+), 10 deletions(-) diff --git a/endpoints/registry.py b/endpoints/registry.py index ded3f8915..f650398ec 100644 --- a/endpoints/registry.py +++ b/endpoints/registry.py @@ -88,6 +88,43 @@ def set_cache_headers(f): return wrapper +@registry.route('/images//layer', methods=['HEAD']) +@process_auth +@extract_namespace_repo_from_session +@require_completion +@set_cache_headers +def head_image_layer(namespace, repository, image_id, headers): + permission = ReadRepositoryPermission(namespace, repository) + + profile.debug('Checking repo permissions') + if permission.can() or model.repository_is_public(namespace, repository): + profile.debug('Looking up repo image') + repo_image = model.get_repo_image(namespace, repository, image_id) + + # Add the Accept-Ranges header if the storage engine supports resumeable + # downloads. + extra_headers = {} + + profile.debug('Looking up the layer path') + try: + path = store.image_layer_path(repo_image.storage.uuid) + + if store.get_supports_resumeable_downloads(repo_image.storage.locations, path): + profile.debug('Storage supports resumeable downloads') + extra_headers['Accept-Ranges'] = 'bytes'; + + resp = make_response('') + resp.headers.extend(extra_headers) + return resp + + except (IOError, AttributeError): + profile.debug('Image not found') + abort(404, 'Image %(image_id)s not found', issue='unknown-image', + image_id=image_id) + + abort(403) + + @registry.route('/images//layer', methods=['GET']) @process_auth @extract_namespace_repo_from_session @@ -101,13 +138,6 @@ def get_image_layer(namespace, repository, image_id, headers): profile.debug('Looking up repo image') repo_image = model.get_repo_image(namespace, repository, image_id) - # Add the Accept-Ranges header if the storage engine supports resumable - # downloads. - extra_headers = {} - - if store.get_supports_resumable_downloads(): - extra_headers['Accept-Ranges'] = 'bytes'; - profile.debug('Looking up the layer path') try: path = store.image_layer_path(repo_image.storage.uuid) @@ -118,7 +148,6 @@ def get_image_layer(namespace, repository, image_id, headers): if direct_download_url: profile.debug('Returning direct download URL') resp = redirect(direct_download_url) - resp.headers = dict(headers, **extra_headers) return resp profile.debug('Streaming layer data') diff --git a/storage/basestorage.py b/storage/basestorage.py index 94bdec615..a6b2c581c 100644 --- a/storage/basestorage.py +++ b/storage/basestorage.py @@ -57,7 +57,7 @@ class BaseStorage(StoragePaths): def get_direct_download_url(self, path, expires_in=60): return None - def get_supports_resumable_downloads(self): + def get_supports_resumeable_downloads(self, path): return False def get_content(self, path): diff --git a/storage/distributedstorage.py b/storage/distributedstorage.py index 796abdc2b..9941f0fa5 100644 --- a/storage/distributedstorage.py +++ b/storage/distributedstorage.py @@ -39,3 +39,4 @@ class DistributedStorage(StoragePaths): list_directory = _location_aware(BaseStorage.list_directory) exists = _location_aware(BaseStorage.exists) remove = _location_aware(BaseStorage.remove) + get_supports_resumeable_downloads = _location_aware(BaseStorage.get_supports_resumeable_downloads) diff --git a/storage/s3.py b/storage/s3.py index add4ed9dc..1f9c30211 100644 --- a/storage/s3.py +++ b/storage/s3.py @@ -83,7 +83,7 @@ class S3Storage(BaseStorage): key.set_contents_from_string(content, encrypt_key=True) return path - def get_supports_resumable_downloads(self): + def get_supports_resumeable_downloads(self, path): return True def get_direct_download_url(self, path, expires_in=60): From e850d17e296812e44101e8fd248930c8f881a563 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Thu, 3 Jul 2014 17:18:14 -0400 Subject: [PATCH 3/4] Handle potential NPE in the registry code --- endpoints/registry.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/endpoints/registry.py b/endpoints/registry.py index f650398ec..bd78e6055 100644 --- a/endpoints/registry.py +++ b/endpoints/registry.py @@ -412,6 +412,11 @@ def put_image_json(namespace, repository, image_id): profile.debug('Looking up repo image') repo_image = model.get_repo_image(namespace, repository, image_id) + if not repo_image: + profile.debug('Image not found') + abort(404, 'Image %(image_id)s not found', issue='unknown-image', + image_id=image_id) + uuid = repo_image.storage.uuid if image_id != data['id']: From af46d3d455f7dc53322a486b2c0a5ad08c3d22bd Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Mon, 7 Jul 2014 16:21:27 -0400 Subject: [PATCH 4/4] Remove path from get_supports_resumeable_download --- endpoints/registry.py | 31 +++++++++++++------------------ storage/basestorage.py | 2 +- storage/s3.py | 2 +- 3 files changed, 15 insertions(+), 20 deletions(-) diff --git a/endpoints/registry.py b/endpoints/registry.py index bd78e6055..cc2342fe4 100644 --- a/endpoints/registry.py +++ b/endpoints/registry.py @@ -100,27 +100,22 @@ def head_image_layer(namespace, repository, image_id, headers): if permission.can() or model.repository_is_public(namespace, repository): profile.debug('Looking up repo image') repo_image = model.get_repo_image(namespace, repository, image_id) - - # Add the Accept-Ranges header if the storage engine supports resumeable - # downloads. - extra_headers = {} - - profile.debug('Looking up the layer path') - try: - path = store.image_layer_path(repo_image.storage.uuid) - - if store.get_supports_resumeable_downloads(repo_image.storage.locations, path): - profile.debug('Storage supports resumeable downloads') - extra_headers['Accept-Ranges'] = 'bytes'; - - resp = make_response('') - resp.headers.extend(extra_headers) - return resp - - except (IOError, AttributeError): + if not repo_image: profile.debug('Image not found') abort(404, 'Image %(image_id)s not found', issue='unknown-image', image_id=image_id) + + extra_headers = {} + + # Add the Accept-Ranges header if the storage engine supports resumeable + # downloads. + if store.get_supports_resumeable_downloads(repo_image.storage.locations): + profile.debug('Storage supports resumeable downloads') + extra_headers['Accept-Ranges'] = 'bytes'; + + resp = make_response('') + resp.headers.extend(extra_headers) + return resp abort(403) diff --git a/storage/basestorage.py b/storage/basestorage.py index a6b2c581c..2d3727a5b 100644 --- a/storage/basestorage.py +++ b/storage/basestorage.py @@ -57,7 +57,7 @@ class BaseStorage(StoragePaths): def get_direct_download_url(self, path, expires_in=60): return None - def get_supports_resumeable_downloads(self, path): + def get_supports_resumeable_downloads(self): return False def get_content(self, path): diff --git a/storage/s3.py b/storage/s3.py index 1f9c30211..12c2373de 100644 --- a/storage/s3.py +++ b/storage/s3.py @@ -83,7 +83,7 @@ class S3Storage(BaseStorage): key.set_contents_from_string(content, encrypt_key=True) return path - def get_supports_resumeable_downloads(self, path): + def get_supports_resumeable_downloads(self): return True def get_direct_download_url(self, path, expires_in=60):