cache: add caching for manifest requests (PROJQUAY-6482) (quay#2522)

data/cache/cache_key.py

@@ -67,7 +67,7 @@ def for_security_report(digest, cache_config):
 
     # Security reports don't change often so a longer TTL can be justified.
     cache_ttl = cache_config.get("security_report_cache_ttl", "300s")
-    return CacheKey(f"security_report_{digest}", cache_ttl)
+    return CacheKey(f"security_report__{digest}", cache_ttl)
 
 
 def for_repository_lookup(namespace_name, repo_name, manifest_ref, kind_filter, cache_config):
@@ -85,3 +85,11 @@ def for_repository_lookup(namespace_name, repo_name, manifest_ref, kind_filter,
 
     logger.debug(f"Loading repository lookup from cache_key: {cache_key}")
     return CacheKey(cache_key, cache_ttl)
+
+
+def for_repository_manifest(repository_id, digest, cache_config):
+    """
+    Returns a cache key for the manifest of a repository.
+    """
+    cache_ttl = cache_config.get("repository_manifest_cache_ttl", "300s")
+    return CacheKey("repository_manifest__%s_%s" % (repository_id, digest), cache_ttl)

data/cache/impl.py

@@ -51,6 +51,10 @@ def retrieve(self, cache_key, loader, should_cache=is_not_none):
         """
         pass
 
+    @abstractmethod
+    def invalidate(self, cache_key):
+        pass
+
 
 class DisconnectWrapper(DataModelCache):
     """
@@ -67,6 +71,10 @@ def retrieve(self, cache_key, loader, should_cache=is_not_none):
         with CloseForLongOperation(self.app_config):
             return self.cache.retrieve(cache_key, loader, should_cache)
 
+    def invalidate(self, cache_key):
+        with CloseForLongOperation(self.app_config):
+            return self.cache.invalidate(cache_key)
+
 
 class NoopDataModelCache(DataModelCache):
     """
@@ -76,6 +84,9 @@ class NoopDataModelCache(DataModelCache):
     def retrieve(self, cache_key, loader, should_cache=is_not_none):
         return loader()
 
+    def invalidate(self, cache_key):
+        return
+
 
 class InMemoryDataModelCache(DataModelCache):
     """
@@ -124,6 +135,12 @@ def retrieve(self, cache_key, loader, should_cache=is_not_none):
 
         return result
 
+    def invalidate(self, cache_key):
+        try:
+            del self.cache[cache_key.key]
+        except KeyError:
+            pass
+
 
 _DEFAULT_MEMCACHE_TIMEOUT = 1  # second
 _DEFAULT_MEMCACHE_CONNECT_TIMEOUT = 1  # second
@@ -238,6 +255,14 @@ def retrieve(self, cache_key, loader, should_cache=is_not_none):
 
         return result
 
+    def invalidate(self, cache_key):
+        client = self.client_pool
+        if client is not None:
+            try:
+                client.delete(cache_key.key, True)
+            except:
+                pass
+
 
 class RedisDataModelCache(DataModelCache):
     """
@@ -315,3 +340,7 @@ def retrieve(self, cache_key, loader, should_cache=is_not_none):
             logger.debug("Not caching loaded result for key %s: %s", cache_key.key, result)
 
         return result
+
+    def invalidate(self, cache_key):
+        if self.client is not None:
+            self.client.delete(cache_key.key)

data/cache/redis_cache.py

@@ -32,6 +32,9 @@ def get(self, key, *args, **kwargs):
     def set(self, key, val, *args, **kwargs):
         return self.write_client.set(key, val, *args, **kwargs)
 
+    def delete(self, key, *args, **kwargs):
+        return self.write_client.delete(key, *args, **kwargs)
+
     def __getattr__(self, name):
         return getattr(self.write_client, name, None)
 

data/model/test/test_gc.py

@@ -3,15 +3,13 @@
 import random
 from contextlib import contextmanager
 from datetime import datetime, timedelta
-from test.fixtures import *
-from test.helpers import check_transitive_modifications
 
 import pytest
 from freezegun import freeze_time
 from mock import patch
 from playhouse.test_utils import assert_query_count
 
-from app import docker_v2_signing_key, storage
+from app import docker_v2_signing_key, model_cache, storage
 from data import database, model
 from data.database import (
     ApprBlob,
@@ -32,6 +30,8 @@
 from image.oci.config import OCIConfig
 from image.oci.manifest import OCIManifestBuilder
 from image.shared.schemas import parse_manifest_from_bytes
+from test.fixtures import *
+from test.helpers import check_transitive_modifications
 from util.bytes import Bytes
 
 ADMIN_ACCESS_USER = "devtable"
@@ -84,7 +84,7 @@ def gc_now(repository):
 
 def delete_tag(repository, tag, perform_gc=True, expect_gc=True):
     repo_ref = RepositoryReference.for_repo_obj(repository)
-    registry_model.delete_tag(repo_ref, tag)
+    registry_model.delete_tag(model_cache, repo_ref, tag)
     if perform_gc:
         assert gc_now(repository) == expect_gc
 

data/registry_model/registry_oci_model.py

@@ -41,6 +41,7 @@
 )
 from image.docker.schema2 import EMPTY_LAYER_BLOB_DIGEST, EMPTY_LAYER_BYTES
 from image.shared import ManifestException
+from util.bytes import Bytes
 from util.timedeltastring import convert_to_timedelta
 
 logger = logging.getLogger(__name__)
@@ -163,6 +164,7 @@ def lookup_manifest_by_digest(
             allow_hidden=allow_hidden,
             require_available=require_available,
         )
+
         if manifest is None:
             if raise_on_error:
                 raise model.ManifestDoesNotExist()
@@ -528,7 +530,7 @@ def retarget_tag(
 
             return Tag.for_tag(tag, self._legacy_image_id_handler)
 
-    def delete_tag(self, repository_ref, tag_name):
+    def delete_tag(self, model_cache, repository_ref, tag_name):
         """
         Deletes the latest, *active* tag with the given name in the repository.
         """
@@ -537,16 +539,26 @@ def delete_tag(self, repository_ref, tag_name):
             if deleted_tag is None:
                 return None
 
+            manifest_cache_key = cache_key.for_repository_manifest(
+                deleted_tag.repository.id, deleted_tag.manifest.digest, model_cache.cache_config
+            )
+            model_cache.invalidate(manifest_cache_key)
+
             return Tag.for_tag(deleted_tag, self._legacy_image_id_handler)
 
-    def delete_tags_for_manifest(self, manifest):
+    def delete_tags_for_manifest(self, model_cache, manifest):
         """
         Deletes all tags pointing to the given manifest, making the manifest inaccessible for
         pulling.
 
         Returns the tags (ShallowTag) deleted. Returns None on error.
         """
         with db_disallow_replica_use():
+            manifest_cache_key = cache_key.for_repository_manifest(
+                manifest.repository.id, manifest.digest, model_cache.cache_config
+            )
+            model_cache.invalidate(manifest_cache_key)
+
             deleted_tags = oci.tag.delete_tags_for_manifest(manifest._db_id)
             return [ShallowTag.for_tag(tag) for tag in deleted_tags]
 
@@ -825,6 +837,51 @@ def is_namespace_enabled(self, namespace_name):
         namespace = model.user.get_namespace_user(namespace_name)
         return namespace is not None and namespace.enabled
 
+    def lookup_cached_manifest_by_digest(
+        self,
+        model_cache,
+        repository_ref,
+        manifest_digest,
+        allow_dead=False,
+        allow_hidden=False,
+        require_available=False,
+        raise_on_error=False,
+    ):
+        def load_manifest():
+            manifest = self.lookup_manifest_by_digest(
+                repository_ref,
+                manifest_digest,
+                allow_dead,
+                allow_hidden,
+                require_available,
+                raise_on_error,
+            )
+
+            if manifest:
+                manifest_dict = manifest.asdict()
+                manifest_dict["internal_manifest_bytes"] = manifest_dict[
+                    "internal_manifest_bytes"
+                ].as_unicode()
+                manifest_dict["inputs"]["repository"] = manifest_dict["inputs"][
+                    "repository"
+                ].asdict()
+                manifest_dict["inputs"]["legacy_image_handler"] = None  # TODO(kleesc): Remove
+                manifest_dict["inputs"]["legacy_id_handler"] = None  # TODO(kleesc): Remove
+
+                return manifest_dict
+
+        manifest_cache_key = cache_key.for_repository_manifest(
+            repository_ref.id, manifest_digest, model_cache.cache_config
+        )
+
+        result = model_cache.retrieve(manifest_cache_key, load_manifest)
+        # TODO(kleesc): cleanup this Manifest interface to avoid explicit conversions
+        result["internal_manifest_bytes"] = Bytes.for_string_or_unicode(
+            result["internal_manifest_bytes"]
+        )
+
+        return Manifest.from_dict(result)
+
     def lookup_cached_active_repository_tags(
         self, model_cache, repository_ref, start_pagination_id, limit
     ):

data/registry_model/test/test_interface.py

@@ -11,7 +11,7 @@
 from mock import patch
 from playhouse.test_utils import assert_query_count
 
-from app import docker_v2_signing_key, storage
+from app import docker_v2_signing_key, model_cache, storage
 from data import model
 from data.cache import cache_key
 from data.cache.impl import InMemoryDataModelCache
@@ -368,11 +368,11 @@ def test_delete_tags(repo_namespace, repo_name, via_manifest, registry_model):
     # Delete every tag in the repository.
     for tag in tags:
         if via_manifest:
-            assert registry_model.delete_tag(repository_ref, tag.name)
+            assert registry_model.delete_tag(model_cache, repository_ref, tag.name)
         else:
             manifest = registry_model.get_manifest_for_tag(tag)
             if manifest is not None:
-                registry_model.delete_tags_for_manifest(manifest)
+                registry_model.delete_tags_for_manifest(model_cache, manifest)
 
         # Make sure the tag is no longer found.
         with assert_query_count(1):

endpoints/api/tag.py

@@ -5,7 +5,7 @@
 
 from flask import abort, request
 
-from app import app, docker_v2_signing_key, storage
+from app import app, docker_v2_signing_key, model_cache, storage
 from auth.auth_context import get_authenticated_user
 from data.model import repository as repository_model
 from data.registry_model import registry_model
@@ -248,7 +248,7 @@ def delete(self, namespace, repository, tag):
         if repo_ref is None:
             raise NotFound()
 
-        tag_ref = registry_model.delete_tag(repo_ref, tag)
+        tag_ref = registry_model.delete_tag(model_cache, repo_ref, tag)
         if tag_ref is None:
             raise NotFound()
 

endpoints/v1/tag.py

@@ -3,7 +3,7 @@
 
 from flask import abort, jsonify, make_response, request, session
 
-from app import docker_v2_signing_key, storage
+from app import docker_v2_signing_key, model_cache, storage
 from auth.decorators import process_auth
 from auth.permissions import ModifyRepositoryPermission, ReadRepositoryPermission
 from data.registry_model import registry_model
@@ -129,7 +129,7 @@ def delete_tag(namespace_name, repo_name, tag):
     )
 
     if permission.can() and repository_ref is not None:
-        if not registry_model.delete_tag(repository_ref, tag):
+        if not registry_model.delete_tag(model_cache, repository_ref, tag):
             abort(404)
 
         track_and_log("delete_tag", repository_ref, tag=tag)

endpoints/v2/manifest.py

@@ -158,7 +158,8 @@ def fetch_manifest_by_digest(namespace_name, repo_name, manifest_ref, registry_m
         raise NameUnknown("repository not found")
 
     try:
-        manifest = registry_model.lookup_manifest_by_digest(
+        manifest = registry_model.lookup_cached_manifest_by_digest(
+            model_cache,
             repository_ref,
             manifest_ref,
             raise_on_error=True,
@@ -391,7 +392,7 @@ def delete_manifest_by_digest(namespace_name, repo_name, manifest_ref):
         if manifest is None:
             raise ManifestUnknown()
 
-        tags = registry_model.delete_tags_for_manifest(manifest)
+        tags = registry_model.delete_tags_for_manifest(model_cache, manifest)
         if not tags:
             raise ManifestUnknown()
 

endpoints/v2/test/test_manifest_pullthru.py

@@ -1,12 +1,11 @@
 import unittest
-from test.fixtures import *  # noqa: F401, F403
 from unittest.mock import MagicMock, patch
 
 import pytest
 from flask import url_for
 
 from app import app as realapp
-from app import instance_keys
+from app import instance_keys, model_cache
 from auth.auth_context_type import ValidatedAuthContext
 from data import model
 from data.database import (
@@ -38,6 +37,7 @@
 )
 from image.shared.schemas import parse_manifest_from_bytes
 from proxy.fixtures import *  # noqa: F401, F403
+from test.fixtures import *  # noqa: F401, F403
 from util.bytes import Bytes
 from util.security.registry_jwt import build_context_and_subject, generate_bearer_token
 
@@ -332,6 +332,8 @@ class TestManifestPullThroughStorage:
 
     @pytest.fixture(autouse=True)
     def setup(self, client, app):
+        model_cache.empty_for_testing()
+
         self.client = client
 
         self.user = model.user.get_user("devtable")