From 4336215636f7ace860f1e499cf5033d12073a44b Mon Sep 17 00:00:00 2001 From: jonjohnsonjr Date: Wed, 19 Feb 2020 10:24:03 -0800 Subject: [PATCH] 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 | 48 ++++++++++++++++--------------------- pkg/v1/daemon/image_test.go | 45 +++++++++++++++------------------- pkg/v1/daemon/options.go | 26 ++++++++++++++++++++ 3 files changed, 65 insertions(+), 54 deletions(-) diff --git a/pkg/v1/daemon/image.go b/pkg/v1/daemon/image.go index f0b216d2a..cb2eb0d96 100644 --- a/pkg/v1/daemon/image.go +++ b/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() } diff --git a/pkg/v1/daemon/image_test.go b/pkg/v1/daemon/image_test.go index 3cbe33cc6..f0b05edd0 100644 --- a/pkg/v1/daemon/image_test.go +++ b/pkg/v1/daemon/image_test.go @@ -29,38 +29,35 @@ 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) } @@ -68,8 +65,4 @@ func TestImage(t *testing.T) { t.Errorf("compare.Images: %v", err) } } - - runTest(false) - runTest(true) - } diff --git a/pkg/v1/daemon/options.go b/pkg/v1/daemon/options.go index 4e03952ee..bb6726bc8 100644 --- a/pkg/v1/daemon/options.go +++ b/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 +}