From 51e67ab7f513b78001b3391d57456bd1a0591d07 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Wed, 13 Dec 2017 16:27:46 -0500 Subject: [PATCH] Fix get_blob_path to not make any database calls and add a test This will be supported by caching, hopefully removing the need to hit the database when the blob object is cached --- data/model/storage.py | 17 +++++++++++----- endpoints/v2/models_interface.py | 2 +- endpoints/v2/models_pre_oci.py | 10 ++++++---- endpoints/v2/test/test_models_pre_oci.py | 25 ++++++++++++++++++++++++ 4 files changed, 44 insertions(+), 10 deletions(-) create mode 100644 endpoints/v2/test/test_models_pre_oci.py diff --git a/data/model/storage.py b/data/model/storage.py index eef439d39..6a4ff291e 100644 --- a/data/model/storage.py +++ b/data/model/storage.py @@ -248,12 +248,19 @@ def get_storage_by_uuid(storage_uuid): def get_layer_path(storage_record): """ Returns the path in the storage engine to the layer data referenced by the storage row. """ - store = config.store - if not storage_record.cas_path: - logger.debug('Serving layer from legacy v1 path') - return store.v1_image_layer_path(storage_record.uuid) + return get_layer_path_for_storage(storage_record.uuid, storage_record.cas_path, + storage_record.content_checksum) - return store.blob_path(storage_record.content_checksum) + +def get_layer_path_for_storage(storage_uuid, cas_path, content_checksum): + """ Returns the path in the storage engine to the layer data referenced by the storage + information. """ + store = config.store + if not cas_path: + logger.debug('Serving layer from legacy v1 path for storage %s', storage_uuid) + return store.v1_image_layer_path(storage_uuid) + + return store.blob_path(content_checksum) def lookup_repo_storages_by_content_checksum(repo, checksums): diff --git a/endpoints/v2/models_interface.py b/endpoints/v2/models_interface.py index 66118a95f..bb6bb8bf7 100644 --- a/endpoints/v2/models_interface.py +++ b/endpoints/v2/models_interface.py @@ -41,7 +41,7 @@ class BlobUpload( """ -class Blob(namedtuple('Blob', ['uuid', 'digest', 'size', 'locations'])): +class Blob(namedtuple('Blob', ['uuid', 'digest', 'size', 'locations', 'cas_path'])): """ Blob represents an opaque binary blob saved to the storage system. """ diff --git a/endpoints/v2/models_pre_oci.py b/endpoints/v2/models_pre_oci.py index fa2e7dc49..0f2352716 100644 --- a/endpoints/v2/models_pre_oci.py +++ b/endpoints/v2/models_pre_oci.py @@ -211,7 +211,8 @@ class PreOCIModel(DockerRegistryV2DataInterface): uuid=blob_record.uuid, digest=blob_digest, size=blob_upload.byte_count, - locations=[blob_upload.location_name],) + locations=[blob_upload.location_name], + cas_path=blob_record.cas_path) def lookup_blobs_by_digest(self, namespace_name, repo_name, digests): def _blob_view(blob_record): @@ -219,6 +220,7 @@ class PreOCIModel(DockerRegistryV2DataInterface): uuid=blob_record.uuid, digest=blob_record.content_checksum, size=blob_record.image_size, + cas_path=blob_record.cas_path, locations=None, # Note: Locations is None in this case. ) @@ -235,7 +237,8 @@ class PreOCIModel(DockerRegistryV2DataInterface): uuid=blob_record.uuid, digest=digest, size=blob_record.image_size, - locations=blob_record.locations,) + locations=blob_record.locations, + cas_path=blob_record.cas_path) except model.BlobDoesNotExist: return None @@ -254,8 +257,7 @@ class PreOCIModel(DockerRegistryV2DataInterface): label.media_type) def get_blob_path(self, blob): - blob_record = model.storage.get_storage_by_uuid(blob.uuid) - return model.storage.get_layer_path(blob_record) + return model.storage.get_layer_path_for_storage(blob.uuid, blob.cas_path, blob.digest) def set_manifest_expires_after(self, namespace_name, repo_name, digest, expires_after_sec): try: diff --git a/endpoints/v2/test/test_models_pre_oci.py b/endpoints/v2/test/test_models_pre_oci.py new file mode 100644 index 000000000..c0e232939 --- /dev/null +++ b/endpoints/v2/test/test_models_pre_oci.py @@ -0,0 +1,25 @@ +import hashlib + +from playhouse.test_utils import assert_query_count + +from data import model +from data.database import ImageStorageLocation +from endpoints.v2.models_pre_oci import data_model +from test.fixtures import * + +def test_get_blob_path(initialized_db): + # Add a blob. + digest = 'sha256:' + hashlib.sha256("a").hexdigest() + location = ImageStorageLocation.get(name='local_us') + db_blob = model.blob.store_blob_record_and_temp_link('devtable', 'simple', digest, location, 1, + 10000000) + + with assert_query_count(1): + blob = data_model.get_blob_by_digest('devtable', 'simple', digest) + assert blob.uuid == db_blob.uuid + + # The blob tuple should have everything get_blob_path needs, so there should be no queries. + with assert_query_count(0): + assert data_model.get_blob_path(blob) + + assert data_model.get_blob_path(blob) == model.storage.get_layer_path(db_blob)