Merge pull request #542 from stevvooe/decouple-redis-from-cache

Decouple redis dependency from blob descriptor cache

notifications/listener_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/distribution/registry/storage"
-	"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"
 	"github.com/docker/libtrust"
@@ -18,7 +18,7 @@ import (
 
 func TestListener(t *testing.T) {
 	ctx := context.Background()
-	registry := storage.NewRegistryWithDriver(ctx, inmemory.New(), cache.NewInMemoryBlobDescriptorCacheProvider())
+	registry := storage.NewRegistryWithDriver(ctx, inmemory.New(), memory.NewInMemoryBlobDescriptorCacheProvider())
 	tl := &testListener{
 		ops: make(map[string]int),
 	}

registry/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),
 	}
 }
 

registry/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 != "" {

registry/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()

registry/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)

registry/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 {
+// ValidateDescriptor provides a helper function to ensure that caches have
-	return dgst.Validate()
+// common criteria for admitting descriptors.
-}
+func ValidateDescriptor(desc distribution.Descriptor) error {
-
+	if err := desc.Digest.Validate(); err != nil {
-func validateDescriptor(desc distribution.Descriptor) error {
-	if err := validateDigest(desc.Digest); err != nil {
 		return err
 	}
 

registry/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
 	}
 

registry/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())
+}

registry/storage/cache/memory_test.go

@@ -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())
-}

registry/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
 	}
 

registry/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))
 }

registry/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.
+// implementations get the benefit. This should be used for unit tests.
-func checkBlobDescriptorCache(t *testing.T, provider BlobDescriptorCacheProvider) {
+func CheckBlobDescriptorCache(t *testing.T, provider BlobDescriptorCacheProvider) {
 	ctx := context.Background()
 
 	checkBlobDescriptorCacheEmptyRepository(t, ctx, provider)

registry/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 {