From 812c8099a6761b93850dd8f185be79b7b498fe03 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 20 May 2015 17:12:40 -0700 Subject: [PATCH] Decouple redis dependency from blob descriptor cache Ensure that clients can use the blob descriptor cache provider without needing the redis package. Signed-off-by: Stephen J Day --- docs/client/repository.go | 3 ++- docs/handlers/app.go | 7 +++--- docs/handlers/app_test.go | 4 ++-- docs/storage/blob_test.go | 8 +++---- docs/storage/cache/cache.go | 11 ++++------ docs/storage/cache/{ => memory}/memory.go | 11 +++++----- docs/storage/cache/memory/memory_test.go | 13 +++++++++++ docs/storage/cache/memory_test.go | 9 -------- docs/storage/cache/{ => redis}/redis.go | 22 +++++++++---------- docs/storage/cache/{ => redis}/redis_test.go | 5 +++-- .../storage/cache/{cache_test.go => suite.go} | 6 ++--- docs/storage/manifeststore_test.go | 4 ++-- 12 files changed, 53 insertions(+), 50 deletions(-) rename docs/storage/cache/{ => memory}/memory.go (93%) create mode 100644 docs/storage/cache/memory/memory_test.go delete mode 100644 docs/storage/cache/memory_test.go rename docs/storage/cache/{ => redis}/redis.go (93%) rename docs/storage/cache/{ => redis}/redis_test.go (88%) rename docs/storage/cache/{cache_test.go => suite.go} (95%) diff --git a/docs/client/repository.go b/docs/client/repository.go index 180d6472..d43ac0db 100644 --- a/docs/client/repository.go +++ b/docs/client/repository.go @@ -18,6 +18,7 @@ import ( "github.com/docker/distribution/registry/api/v2" "github.com/docker/distribution/registry/client/transport" "github.com/docker/distribution/registry/storage/cache" + "github.com/docker/distribution/registry/storage/cache/memory" ) // NewRepository creates a new Repository for the given repository name and base URL @@ -66,7 +67,7 @@ func (r *repository) Blobs(ctx context.Context) distribution.BlobStore { name: r.Name(), ub: r.ub, client: r.client, - statter: cache.NewCachedBlobStatter(cache.NewInMemoryBlobDescriptorCacheProvider(), statter), + statter: cache.NewCachedBlobStatter(memory.NewInMemoryBlobDescriptorCacheProvider(), statter), } } diff --git a/docs/handlers/app.go b/docs/handlers/app.go index 22c0b6de..1d58e945 100644 --- a/docs/handlers/app.go +++ b/docs/handlers/app.go @@ -18,7 +18,8 @@ import ( registrymiddleware "github.com/docker/distribution/registry/middleware/registry" repositorymiddleware "github.com/docker/distribution/registry/middleware/repository" "github.com/docker/distribution/registry/storage" - "github.com/docker/distribution/registry/storage/cache" + memorycache "github.com/docker/distribution/registry/storage/cache/memory" + rediscache "github.com/docker/distribution/registry/storage/cache/redis" storagedriver "github.com/docker/distribution/registry/storage/driver" "github.com/docker/distribution/registry/storage/driver/factory" storagemiddleware "github.com/docker/distribution/registry/storage/driver/middleware" @@ -114,10 +115,10 @@ func NewApp(ctx context.Context, configuration configuration.Configuration) *App if app.redis == nil { panic("redis configuration required to use for layerinfo cache") } - app.registry = storage.NewRegistryWithDriver(app, app.driver, cache.NewRedisBlobDescriptorCacheProvider(app.redis)) + app.registry = storage.NewRegistryWithDriver(app, app.driver, rediscache.NewRedisBlobDescriptorCacheProvider(app.redis)) ctxu.GetLogger(app).Infof("using redis blob descriptor cache") case "inmemory": - app.registry = storage.NewRegistryWithDriver(app, app.driver, cache.NewInMemoryBlobDescriptorCacheProvider()) + app.registry = storage.NewRegistryWithDriver(app, app.driver, memorycache.NewInMemoryBlobDescriptorCacheProvider()) ctxu.GetLogger(app).Infof("using inmemory blob descriptor cache") default: if v != "" { diff --git a/docs/handlers/app_test.go b/docs/handlers/app_test.go index 03ea0c9c..fd1c486c 100644 --- a/docs/handlers/app_test.go +++ b/docs/handlers/app_test.go @@ -13,7 +13,7 @@ import ( "github.com/docker/distribution/registry/auth" _ "github.com/docker/distribution/registry/auth/silly" "github.com/docker/distribution/registry/storage" - "github.com/docker/distribution/registry/storage/cache" + memorycache "github.com/docker/distribution/registry/storage/cache/memory" "github.com/docker/distribution/registry/storage/driver/inmemory" "golang.org/x/net/context" ) @@ -30,7 +30,7 @@ func TestAppDispatcher(t *testing.T) { Context: ctx, router: v2.Router(), driver: driver, - registry: storage.NewRegistryWithDriver(ctx, driver, cache.NewInMemoryBlobDescriptorCacheProvider()), + registry: storage.NewRegistryWithDriver(ctx, driver, memorycache.NewInMemoryBlobDescriptorCacheProvider()), } server := httptest.NewServer(app) router := v2.Router() diff --git a/docs/storage/blob_test.go b/docs/storage/blob_test.go index 6843922a..114e686f 100644 --- a/docs/storage/blob_test.go +++ b/docs/storage/blob_test.go @@ -12,7 +12,7 @@ import ( "github.com/docker/distribution" "github.com/docker/distribution/context" "github.com/docker/distribution/digest" - "github.com/docker/distribution/registry/storage/cache" + "github.com/docker/distribution/registry/storage/cache/memory" "github.com/docker/distribution/registry/storage/driver/inmemory" "github.com/docker/distribution/testutil" ) @@ -35,7 +35,7 @@ func TestSimpleBlobUpload(t *testing.T) { ctx := context.Background() imageName := "foo/bar" driver := inmemory.New() - registry := NewRegistryWithDriver(ctx, driver, cache.NewInMemoryBlobDescriptorCacheProvider()) + registry := NewRegistryWithDriver(ctx, driver, memory.NewInMemoryBlobDescriptorCacheProvider()) repository, err := registry.Repository(ctx, imageName) if err != nil { t.Fatalf("unexpected error getting repo: %v", err) @@ -148,7 +148,7 @@ func TestSimpleBlobRead(t *testing.T) { ctx := context.Background() imageName := "foo/bar" driver := inmemory.New() - registry := NewRegistryWithDriver(ctx, driver, cache.NewInMemoryBlobDescriptorCacheProvider()) + registry := NewRegistryWithDriver(ctx, driver, memory.NewInMemoryBlobDescriptorCacheProvider()) repository, err := registry.Repository(ctx, imageName) if err != nil { t.Fatalf("unexpected error getting repo: %v", err) @@ -252,7 +252,7 @@ func TestLayerUploadZeroLength(t *testing.T) { ctx := context.Background() imageName := "foo/bar" driver := inmemory.New() - registry := NewRegistryWithDriver(ctx, driver, cache.NewInMemoryBlobDescriptorCacheProvider()) + registry := NewRegistryWithDriver(ctx, driver, memory.NewInMemoryBlobDescriptorCacheProvider()) repository, err := registry.Repository(ctx, imageName) if err != nil { t.Fatalf("unexpected error getting repo: %v", err) diff --git a/docs/storage/cache/cache.go b/docs/storage/cache/cache.go index e7471c27..79e6d9c8 100644 --- a/docs/storage/cache/cache.go +++ b/docs/storage/cache/cache.go @@ -6,7 +6,6 @@ import ( "fmt" "github.com/docker/distribution" - "github.com/docker/distribution/digest" ) // BlobDescriptorCacheProvider provides repository scoped @@ -17,12 +16,10 @@ type BlobDescriptorCacheProvider interface { RepositoryScoped(repo string) (distribution.BlobDescriptorService, error) } -func validateDigest(dgst digest.Digest) error { - return dgst.Validate() -} - -func validateDescriptor(desc distribution.Descriptor) error { - if err := validateDigest(desc.Digest); err != nil { +// ValidateDescriptor provides a helper function to ensure that caches have +// common criteria for admitting descriptors. +func ValidateDescriptor(desc distribution.Descriptor) error { + if err := desc.Digest.Validate(); err != nil { return err } diff --git a/docs/storage/cache/memory.go b/docs/storage/cache/memory/memory.go similarity index 93% rename from docs/storage/cache/memory.go rename to docs/storage/cache/memory/memory.go index 125c11fb..cdd9abe8 100644 --- a/docs/storage/cache/memory.go +++ b/docs/storage/cache/memory/memory.go @@ -1,4 +1,4 @@ -package cache +package memory import ( "sync" @@ -7,6 +7,7 @@ import ( "github.com/docker/distribution/context" "github.com/docker/distribution/digest" "github.com/docker/distribution/registry/api/v2" + "github.com/docker/distribution/registry/storage/cache" ) type inMemoryBlobDescriptorCacheProvider struct { @@ -17,7 +18,7 @@ type inMemoryBlobDescriptorCacheProvider struct { // NewInMemoryBlobDescriptorCacheProvider returns a new mapped-based cache for // storing blob descriptor data. -func NewInMemoryBlobDescriptorCacheProvider() BlobDescriptorCacheProvider { +func NewInMemoryBlobDescriptorCacheProvider() cache.BlobDescriptorCacheProvider { return &inMemoryBlobDescriptorCacheProvider{ global: newMapBlobDescriptorCache(), repositories: make(map[string]*mapBlobDescriptorCache), @@ -117,7 +118,7 @@ func newMapBlobDescriptorCache() *mapBlobDescriptorCache { } func (mbdc *mapBlobDescriptorCache) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { - if err := validateDigest(dgst); err != nil { + if err := dgst.Validate(); err != nil { return distribution.Descriptor{}, err } @@ -133,11 +134,11 @@ func (mbdc *mapBlobDescriptorCache) Stat(ctx context.Context, dgst digest.Digest } func (mbdc *mapBlobDescriptorCache) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error { - if err := validateDigest(dgst); err != nil { + if err := dgst.Validate(); err != nil { return err } - if err := validateDescriptor(desc); err != nil { + if err := cache.ValidateDescriptor(desc); err != nil { return err } diff --git a/docs/storage/cache/memory/memory_test.go b/docs/storage/cache/memory/memory_test.go new file mode 100644 index 00000000..3bae7ccb --- /dev/null +++ b/docs/storage/cache/memory/memory_test.go @@ -0,0 +1,13 @@ +package memory + +import ( + "testing" + + "github.com/docker/distribution/registry/storage/cache" +) + +// TestInMemoryBlobInfoCache checks the in memory implementation is working +// correctly. +func TestInMemoryBlobInfoCache(t *testing.T) { + cache.CheckBlobDescriptorCache(t, NewInMemoryBlobDescriptorCacheProvider()) +} diff --git a/docs/storage/cache/memory_test.go b/docs/storage/cache/memory_test.go deleted file mode 100644 index 9f2ce460..00000000 --- a/docs/storage/cache/memory_test.go +++ /dev/null @@ -1,9 +0,0 @@ -package cache - -import "testing" - -// TestInMemoryBlobInfoCache checks the in memory implementation is working -// correctly. -func TestInMemoryBlobInfoCache(t *testing.T) { - checkBlobDescriptorCache(t, NewInMemoryBlobDescriptorCacheProvider()) -} diff --git a/docs/storage/cache/redis.go b/docs/storage/cache/redis/redis.go similarity index 93% rename from docs/storage/cache/redis.go rename to docs/storage/cache/redis/redis.go index 1f3727f0..29bbe3bc 100644 --- a/docs/storage/cache/redis.go +++ b/docs/storage/cache/redis/redis.go @@ -1,13 +1,13 @@ -package cache +package redis import ( "fmt" - "github.com/docker/distribution/registry/api/v2" - "github.com/docker/distribution" "github.com/docker/distribution/context" "github.com/docker/distribution/digest" + "github.com/docker/distribution/registry/api/v2" + "github.com/docker/distribution/registry/storage/cache" "github.com/garyburd/redigo/redis" ) @@ -31,11 +31,9 @@ type redisBlobDescriptorService struct { // request objects, we can change this to a connection. } -var _ BlobDescriptorCacheProvider = &redisBlobDescriptorService{} - // NewRedisBlobDescriptorCacheProvider returns a new redis-based // BlobDescriptorCacheProvider using the provided redis connection pool. -func NewRedisBlobDescriptorCacheProvider(pool *redis.Pool) BlobDescriptorCacheProvider { +func NewRedisBlobDescriptorCacheProvider(pool *redis.Pool) cache.BlobDescriptorCacheProvider { return &redisBlobDescriptorService{ pool: pool, } @@ -55,7 +53,7 @@ func (rbds *redisBlobDescriptorService) RepositoryScoped(repo string) (distribut // Stat retrieves the descriptor data from the redis hash entry. func (rbds *redisBlobDescriptorService) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { - if err := validateDigest(dgst); err != nil { + if err := dgst.Validate(); err != nil { return distribution.Descriptor{}, err } @@ -89,11 +87,11 @@ func (rbds *redisBlobDescriptorService) stat(ctx context.Context, conn redis.Con // hash. A hash is used here since we may store unrelated fields about a layer // in the future. func (rbds *redisBlobDescriptorService) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error { - if err := validateDigest(dgst); err != nil { + if err := dgst.Validate(); err != nil { return err } - if err := validateDescriptor(desc); err != nil { + if err := cache.ValidateDescriptor(desc); err != nil { return err } @@ -134,7 +132,7 @@ var _ distribution.BlobDescriptorService = &repositoryScopedRedisBlobDescriptorS // forwards the descriptor request to the global blob store. If the media type // differs for the repository, we override it. func (rsrbds *repositoryScopedRedisBlobDescriptorService) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) { - if err := validateDigest(dgst); err != nil { + if err := dgst.Validate(); err != nil { return distribution.Descriptor{}, err } @@ -170,11 +168,11 @@ func (rsrbds *repositoryScopedRedisBlobDescriptorService) Stat(ctx context.Conte } func (rsrbds *repositoryScopedRedisBlobDescriptorService) SetDescriptor(ctx context.Context, dgst digest.Digest, desc distribution.Descriptor) error { - if err := validateDigest(dgst); err != nil { + if err := dgst.Validate(); err != nil { return err } - if err := validateDescriptor(desc); err != nil { + if err := cache.ValidateDescriptor(desc); err != nil { return err } diff --git a/docs/storage/cache/redis_test.go b/docs/storage/cache/redis/redis_test.go similarity index 88% rename from docs/storage/cache/redis_test.go rename to docs/storage/cache/redis/redis_test.go index 65c2fd3a..ed6944a1 100644 --- a/docs/storage/cache/redis_test.go +++ b/docs/storage/cache/redis/redis_test.go @@ -1,4 +1,4 @@ -package cache +package redis import ( "flag" @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/docker/distribution/registry/storage/cache" "github.com/garyburd/redigo/redis" ) @@ -46,5 +47,5 @@ func TestRedisBlobDescriptorCacheProvider(t *testing.T) { t.Fatalf("unexpected error flushing redis db: %v", err) } - checkBlobDescriptorCache(t, NewRedisBlobDescriptorCacheProvider(pool)) + cache.CheckBlobDescriptorCache(t, NewRedisBlobDescriptorCacheProvider(pool)) } diff --git a/docs/storage/cache/cache_test.go b/docs/storage/cache/suite.go similarity index 95% rename from docs/storage/cache/cache_test.go rename to docs/storage/cache/suite.go index e923367a..ceefab97 100644 --- a/docs/storage/cache/cache_test.go +++ b/docs/storage/cache/suite.go @@ -8,10 +8,10 @@ import ( "github.com/docker/distribution/digest" ) -// checkBlobDescriptorCache takes a cache implementation through a common set +// CheckBlobDescriptorCache takes a cache implementation through a common set // of operations. If adding new tests, please add them here so new -// implementations get the benefit. -func checkBlobDescriptorCache(t *testing.T, provider BlobDescriptorCacheProvider) { +// implementations get the benefit. This should be used for unit tests. +func CheckBlobDescriptorCache(t *testing.T, provider BlobDescriptorCacheProvider) { ctx := context.Background() checkBlobDescriptorCacheEmptyRepository(t, ctx, provider) diff --git a/docs/storage/manifeststore_test.go b/docs/storage/manifeststore_test.go index 59f174b3..3422985a 100644 --- a/docs/storage/manifeststore_test.go +++ b/docs/storage/manifeststore_test.go @@ -10,7 +10,7 @@ import ( "github.com/docker/distribution/context" "github.com/docker/distribution/digest" "github.com/docker/distribution/manifest" - "github.com/docker/distribution/registry/storage/cache" + "github.com/docker/distribution/registry/storage/cache/memory" "github.com/docker/distribution/registry/storage/driver" "github.com/docker/distribution/registry/storage/driver/inmemory" "github.com/docker/distribution/testutil" @@ -29,7 +29,7 @@ type manifestStoreTestEnv struct { func newManifestStoreTestEnv(t *testing.T, name, tag string) *manifestStoreTestEnv { ctx := context.Background() driver := inmemory.New() - registry := NewRegistryWithDriver(ctx, driver, cache.NewInMemoryBlobDescriptorCacheProvider()) + registry := NewRegistryWithDriver(ctx, driver, memory.NewInMemoryBlobDescriptorCacheProvider()) repo, err := registry.Repository(ctx, name) if err != nil {