Add WithClient to daemon.Image (#674)

Fixes #139

Rather than wrapping the docker client options (as proposed in #139),
this just allows you to pass in an entire client implementation. This
makes testing a nicer, since we can pass in a MockImageSaver, and
callers can configure the client however they'd like without us having
to keep up with the docker client dependency.

By default, we'll still use the same options (client.FromEnv) to reach
the daemon (for compatibility).

pkg/v1/daemon/image.go

@@ -36,6 +36,7 @@ var _ v1.Image = (*image)(nil)
 type imageOpener struct {
 	ref      name.Reference
 	buffered bool
+	client   Client
 }
 
 // ImageOption is a functional option for Image.
@@ -45,14 +46,16 @@ func (i *imageOpener) Open() (v1.Image, error) {
 	var opener tarball.Opener
 	var err error
 	if i.buffered {
-		opener, err = bufferedOpener(i.ref)
+		opener, err = i.bufferedOpener(i.ref)
 	} else {
-		opener, err = unbufferedOpener(i.ref)
+		opener, err = i.unbufferedOpener(i.ref)
 	}
 	if err != nil {
 		return nil, err
 	}
 
+	i.client.NegotiateAPIVersion(context.Background())
+
 	tb, err := tarball.Image(opener, nil)
 	if err != nil {
 		return nil, err
@@ -63,33 +66,13 @@ func (i *imageOpener) Open() (v1.Image, error) {
 	return img, nil
 }
 
-// ImageSaver is an interface for testing.
-type ImageSaver interface {
-	ImageSave(context.Context, []string) (io.ReadCloser, error)
+func (i *imageOpener) saveImage(ref name.Reference) (io.ReadCloser, error) {
+	return i.client.ImageSave(context.Background(), []string{ref.Name()})
 }
 
-// This is a variable so we can override in tests.
-var getImageSaver = func() (ImageSaver, error) {
-	cli, err := client.NewClientWithOpts(client.FromEnv)
-	if err != nil {
-		return nil, err
-	}
-	cli.NegotiateAPIVersion(context.Background())
-	return cli, nil
-}
-
-func saveImage(ref name.Reference) (io.ReadCloser, error) {
-	cli, err := getImageSaver()
-	if err != nil {
-		return nil, err
-	}
-
-	return cli.ImageSave(context.Background(), []string{ref.Name()})
-}
-
-func bufferedOpener(ref name.Reference) (tarball.Opener, error) {
+func (i *imageOpener) bufferedOpener(ref name.Reference) (tarball.Opener, error) {
 	// Store the tarball in memory and return a new reader into the bytes each time we need to access something.
-	rc, err := saveImage(ref)
+	rc, err := i.saveImage(ref)
 	if err != nil {
 		return nil, err
 	}
@@ -106,10 +89,10 @@ func bufferedOpener(ref name.Reference) (tarball.Opener, error) {
 	}, nil
 }
 
-func unbufferedOpener(ref name.Reference) (tarball.Opener, error) {
+func (i *imageOpener) unbufferedOpener(ref name.Reference) (tarball.Opener, error) {
 	// To avoid storing the tarball in memory, do a save every time we need to access something.
 	return func() (io.ReadCloser, error) {
-		return saveImage(ref)
+		return i.saveImage(ref)
 	}, nil
 }
 
@@ -126,5 +109,14 @@ func Image(ref name.Reference, options ...ImageOption) (v1.Image, error) {
 			return nil, err
 		}
 	}
+
+	if i.client == nil {
+		var err error
+		i.client, err = client.NewClientWithOpts(client.FromEnv)
+		if err != nil {
+			return nil, err
+		}
+	}
+
 	return i.Open()
 }

pkg/v1/daemon/image_test.go

@@ -29,47 +29,40 @@ import (
 var imagePath = "../tarball/testdata/test_image_1.tar"
 
 type MockImageSaver struct {
+	Client
 	path string
 }
 
+func (m *MockImageSaver) NegotiateAPIVersion(ctx context.Context) {}
+
 func (m *MockImageSaver) ImageSave(_ context.Context, _ []string) (io.ReadCloser, error) {
 	return os.Open(m.path)
 }
 
-func init() {
-	getImageSaver = func() (ImageSaver, error) {
-		return &MockImageSaver{path: imagePath}, nil
-	}
-}
-
 func TestImage(t *testing.T) {
-	img, err := tarball.ImageFromPath(imagePath, nil)
-	if err != nil {
-		t.Fatalf("error loading test image: %s", err)
-	}
-
-	tag, err := name.NewTag("unused", name.WeakValidation)
-	if err != nil {
-		t.Fatalf("error creating test name: %s", err)
-	}
+	for _, opts := range [][]ImageOption{{
+		WithBufferedOpener(),
+		WithClient(&MockImageSaver{path: imagePath}),
+	}, {
+		WithUnbufferedOpener(),
+		WithClient(&MockImageSaver{path: imagePath}),
+	}} {
+		img, err := tarball.ImageFromPath(imagePath, nil)
+		if err != nil {
+			t.Fatalf("error loading test image: %s", err)
+		}
 
-	runTest := func(buffered bool) {
-		var bufferedOption ImageOption
-		if buffered {
-			bufferedOption = WithBufferedOpener()
-		} else {
-			bufferedOption = WithUnbufferedOpener()
+		tag, err := name.NewTag("unused", name.WeakValidation)
+		if err != nil {
+			t.Fatalf("error creating test name: %s", err)
 		}
-		dmn, err := Image(tag, bufferedOption)
+
+		dmn, err := Image(tag, opts...)
 		if err != nil {
 			t.Errorf("Error loading daemon image: %s", err)
 		}
 		if err := compare.Images(img, dmn); err != nil {
 			t.Errorf("compare.Images: %v", err)
 		}
 	}
-
-	runTest(false)
-	runTest(true)
-
 }

pkg/v1/daemon/options.go

@@ -14,6 +14,13 @@
 
 package daemon
 
+import (
+	"context"
+	"io"
+
+	"github.com/docker/docker/api/types"
+)
+
 // WithBufferedOpener buffers the image.
 func WithBufferedOpener() ImageOption {
 	return func(i *imageOpener) error {
@@ -32,3 +39,22 @@ func (i *imageOpener) setBuffered(buffer bool) error {
 	i.buffered = buffer
 	return nil
 }
+
+// WithClient is a functional option to allow injecting a docker client.
+//
+// By default, github.com/docker/docker/client.FromEnv is used.
+func WithClient(client Client) ImageOption {
+	return func(i *imageOpener) error {
+		i.client = client
+		return nil
+	}
+}
+
+// Client represents the subset of a docker client that the daemon
+// package uses.
+type Client interface {
+	NegotiateAPIVersion(ctx context.Context)
+	ImageSave(context.Context, []string) (io.ReadCloser, error)
+	ImageLoad(context.Context, io.Reader, bool) (types.ImageLoadResponse, error)
+	ImageTag(context.Context, string, string) error
+}