Integrate context with storage package

This changeset integrates context with the storage package. Debug messages have
been added to exported methods. Existing log messages will now include
contextual details through logger fields to aid in debugging. This integration
focuses on logging and may be followed up with a metric-oriented change in the
future.

Signed-off-by: Stephen J Day <stephen.day@docker.com>

registry/app.go

@@ -227,7 +227,8 @@ func (app *App) dispatcher(dispatch dispatchFunc) http.Handler {
 
 		// decorate the authorized repository with an event bridge.
 		context.Repository = notifications.Listen(
-			context.Repository, app.eventBridge(context, r))
+			app.registry.Repository(context, getName(context)),
+			app.eventBridge(context, r))
 		handler := dispatch(context, r)
 
 		ssrw := &singleStatusResponseWriter{ResponseWriter: w}
@@ -276,9 +277,6 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
 	repo := getName(context)
 
 	if app.accessController == nil {
-		// No access controller, so we simply provide access.
-		context.Repository = app.registry.Repository(repo)
-
 		return nil // access controller is not enabled.
 	}
 
@@ -357,12 +355,11 @@ func (app *App) authorized(w http.ResponseWriter, r *http.Request, context *Cont
 		return err
 	}
 
+	// TODO(stevvooe): This pattern needs to be cleaned up a bit. One context
+	// should be replaced by another, rather than replacing the context on a
+	// mutable object.
 	context.Context = ctx
 
-	// At this point, the request should have access to the repository under
-	// the requested operation. Make is available on the context.
-	context.Repository = app.registry.Repository(repo)
-
 	return nil
 }
 

storage/blobstore.go

@@ -3,10 +3,10 @@ package storage
 import (
 	"fmt"
 
-	"github.com/Sirupsen/logrus"
+	ctxu "github.com/docker/distribution/context"
-
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/storagedriver"
+	"golang.org/x/net/context"
 )
 
 // TODO(stevvooe): Currently, the blobStore implementation used by the
@@ -19,6 +19,7 @@ import (
 // backend links.
 type blobStore struct {
 	*registry
+	ctx context.Context
 }
 
 // exists reports whether or not the path exists. If the driver returns error
@@ -110,7 +111,7 @@ func (bs *blobStore) resolve(path string) (string, error) {
 func (bs *blobStore) put(p []byte) (digest.Digest, error) {
 	dgst, err := digest.FromBytes(p)
 	if err != nil {
-		logrus.Errorf("error digesting content: %v, %s", err, string(p))
+		ctxu.GetLogger(bs.ctx).Errorf("error digesting content: %v, %s", err, string(p))
 		return "", err
 	}
 

storage/layer_test.go

@@ -13,6 +13,7 @@ import (
 	"github.com/docker/distribution/storagedriver"
 	"github.com/docker/distribution/storagedriver/inmemory"
 	"github.com/docker/distribution/testutil"
+	"golang.org/x/net/context"
 )
 
 // TestSimpleLayerUpload covers the layer upload process, exercising common
@@ -30,10 +31,11 @@ func TestSimpleLayerUpload(t *testing.T) {
 		t.Fatalf("error allocating upload store: %v", err)
 	}
 
+	ctx := context.Background()
 	imageName := "foo/bar"
 	driver := inmemory.New()
 	registry := NewRegistryWithDriver(driver)
-	ls := registry.Repository(imageName).Layers()
+	ls := registry.Repository(ctx, imageName).Layers()
 
 	h := sha256.New()
 	rd := io.TeeReader(randomDataReader, h)
@@ -133,10 +135,11 @@ func TestSimpleLayerUpload(t *testing.T) {
 // open, read, seek, read works. More specific edge cases should be covered in
 // other tests.
 func TestSimpleLayerRead(t *testing.T) {
+	ctx := context.Background()
 	imageName := "foo/bar"
 	driver := inmemory.New()
 	registry := NewRegistryWithDriver(driver)
-	ls := registry.Repository(imageName).Layers()
+	ls := registry.Repository(ctx, imageName).Layers()
 
 	randomLayerReader, tarSumStr, err := testutil.CreateRandomTarFile()
 	if err != nil {
@@ -237,10 +240,11 @@ func TestSimpleLayerRead(t *testing.T) {
 
 // TestLayerUploadZeroLength uploads zero-length
 func TestLayerUploadZeroLength(t *testing.T) {
+	ctx := context.Background()
 	imageName := "foo/bar"
 	driver := inmemory.New()
 	registry := NewRegistryWithDriver(driver)
-	ls := registry.Repository(imageName).Layers()
+	ls := registry.Repository(ctx, imageName).Layers()
 
 	upload, err := ls.Upload()
 	if err != nil {

storage/layerstore.go

@@ -4,6 +4,7 @@ import (
 	"time"
 
 	"code.google.com/p/go-uuid/uuid"
+	ctxu "github.com/docker/distribution/context"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/distribution/storagedriver"
@@ -14,6 +15,8 @@ type layerStore struct {
 }
 
 func (ls *layerStore) Exists(digest digest.Digest) (bool, error) {
+	ctxu.GetLogger(ls.repository.ctx).Debug("(*layerStore).Exists")
+
 	// Because this implementation just follows blob links, an existence check
 	// is pretty cheap by starting and closing a fetch.
 	_, err := ls.Fetch(digest)
@@ -31,6 +34,7 @@ func (ls *layerStore) Exists(digest digest.Digest) (bool, error) {
 }
 
 func (ls *layerStore) Fetch(dgst digest.Digest) (Layer, error) {
+	ctxu.GetLogger(ls.repository.ctx).Debug("(*layerStore).Fetch")
 	bp, err := ls.path(dgst)
 	if err != nil {
 		return nil, err
@@ -52,6 +56,7 @@ func (ls *layerStore) Fetch(dgst digest.Digest) (Layer, error) {
 // is already in progress or the layer has already been uploaded, this
 // will return an error.
 func (ls *layerStore) Upload() (LayerUpload, error) {
+	ctxu.GetLogger(ls.repository.ctx).Debug("(*layerStore).Upload")
 
 	// NOTE(stevvooe): Consider the issues with allowing concurrent upload of
 	// the same two layers. Should it be disallowed? For now, we allow both
@@ -89,6 +94,7 @@ func (ls *layerStore) Upload() (LayerUpload, error) {
 // Resume continues an in progress layer upload, returning the current
 // state of the upload.
 func (ls *layerStore) Resume(uuid string) (LayerUpload, error) {
+	ctxu.GetLogger(ls.repository.ctx).Debug("(*layerStore).Resume")
 	startedAtPath, err := ls.repository.registry.pm.path(uploadStartedAtPathSpec{
 		name: ls.repository.Name(),
 		uuid: uuid,

storage/layerupload.go

@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/Sirupsen/logrus"
+	ctxu "github.com/docker/distribution/context"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/storagedriver"
 	"github.com/docker/docker/pkg/tarsum"
@@ -44,6 +45,7 @@ func (luc *layerUploadController) StartedAt() time.Time {
 // contents of the uploaded layer. The checksum should be provided in the
 // format <algorithm>:<hex digest>.
 func (luc *layerUploadController) Finish(digest digest.Digest) (Layer, error) {
+	ctxu.GetLogger(luc.layerStore.repository.ctx).Debug("(*layerUploadController).Finish")
 	canonical, err := luc.validateLayer(digest)
 	if err != nil {
 		return nil, err
@@ -68,6 +70,7 @@ func (luc *layerUploadController) Finish(digest digest.Digest) (Layer, error) {
 
 // Cancel the layer upload process.
 func (luc *layerUploadController) Cancel() error {
+	ctxu.GetLogger(luc.layerStore.repository.ctx).Debug("(*layerUploadController).Cancel")
 	if err := luc.removeResources(); err != nil {
 		return err
 	}

storage/manifeststore.go

@@ -4,6 +4,7 @@ import (
 	"fmt"
 	"strings"
 
+	ctxu "github.com/docker/distribution/context"
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/libtrust"
@@ -77,14 +78,17 @@ var _ ManifestService = &manifestStore{}
 // }
 
 func (ms *manifestStore) Tags() ([]string, error) {
+	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Tags")
 	return ms.tagStore.tags()
 }
 
 func (ms *manifestStore) Exists(tag string) (bool, error) {
+	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Exists")
 	return ms.tagStore.exists(tag)
 }
 
 func (ms *manifestStore) Get(tag string) (*manifest.SignedManifest, error) {
+	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Get")
 	dgst, err := ms.tagStore.resolve(tag)
 	if err != nil {
 		return nil, err
@@ -94,6 +98,8 @@ func (ms *manifestStore) Get(tag string) (*manifest.SignedManifest, error) {
 }
 
 func (ms *manifestStore) Put(tag string, manifest *manifest.SignedManifest) error {
+	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Put")
+
 	// TODO(stevvooe): Add check here to see if the revision is already
 	// present in the repository. If it is, we should merge the signatures, do
 	// a shallow verify (or a full one, doesn't matter) and return an error
@@ -118,6 +124,8 @@ func (ms *manifestStore) Put(tag string, manifest *manifest.SignedManifest) erro
 // semantics in the future, but this will maintain consistency. The underlying
 // blobs are left alone.
 func (ms *manifestStore) Delete(tag string) error {
+	ctxu.GetLogger(ms.repository.ctx).Debug("(*manifestStore).Delete")
+
 	revisions, err := ms.tagStore.revisions(tag)
 	if err != nil {
 		return err

storage/manifeststore_test.go

@@ -6,20 +6,21 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/docker/distribution/testutil"
-
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
 	"github.com/docker/distribution/storagedriver/inmemory"
+	"github.com/docker/distribution/testutil"
 	"github.com/docker/libtrust"
+	"golang.org/x/net/context"
 )
 
 func TestManifestStorage(t *testing.T) {
+	ctx := context.Background()
 	name := "foo/bar"
 	tag := "thetag"
 	driver := inmemory.New()
 	registry := NewRegistryWithDriver(driver)
-	repo := registry.Repository(name)
+	repo := registry.Repository(ctx, name)
 	ms := repo.Manifests()
 
 	exists, err := ms.Exists(tag)

storage/notifications/listener_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/docker/distribution/storagedriver/inmemory"
 	"github.com/docker/distribution/testutil"
 	"github.com/docker/libtrust"
+	"golang.org/x/net/context"
 )
 
 func TestListener(t *testing.T) {
@@ -18,7 +19,8 @@ func TestListener(t *testing.T) {
 	tl := &testListener{
 		ops: make(map[string]int),
 	}
-	repository := Listen(registry.Repository("foo/bar"), tl)
+	ctx := context.Background()
+	repository := Listen(registry.Repository(ctx, "foo/bar"), tl)
 
 	// Now take the registry through a number of operations
 	checkExerciseRepository(t, repository)

storage/registry.go

@@ -1,6 +1,9 @@
 package storage
 
-import "github.com/docker/distribution/storagedriver"
+import (
+	"github.com/docker/distribution/storagedriver"
+	"golang.org/x/net/context"
+)
 
 // registry is the top-level implementation of Registry for use in the storage
 // package. All instances should descend from this object.
@@ -32,8 +35,9 @@ func NewRegistryWithDriver(driver storagedriver.StorageDriver) Registry {
 // Repository returns an instance of the repository tied to the registry.
 // Instances should not be shared between goroutines but are cheap to
 // allocate. In general, they should be request scoped.
-func (reg *registry) Repository(name string) Repository {
+func (reg *registry) Repository(ctx context.Context, name string) Repository {
 	return &repository{
+		ctx:      ctx,
 		registry: reg,
 		name:     name,
 	}
@@ -42,6 +46,7 @@ func (reg *registry) Repository(name string) Repository {
 // repository provides name-scoped access to various services.
 type repository struct {
 	*registry
+	ctx  context.Context
 	name string
 }
 

storage/services.go

@@ -3,6 +3,7 @@ package storage
 import (
 	"github.com/docker/distribution/digest"
 	"github.com/docker/distribution/manifest"
+	"golang.org/x/net/context"
 )
 
 // TODO(stevvooe): These types need to be moved out of the storage package.
@@ -12,7 +13,7 @@ type Registry interface {
 	// Repository should return a reference to the named repository. The
 	// registry may or may not have the repository but should always return a
 	// reference.
-	Repository(name string) Repository
+	Repository(ctx context.Context, name string) Repository
 }
 
 // Repository is a named collection of manifests and layers.