diff --git a/pkg/skaffold/build/local/local.go b/pkg/skaffold/build/local/local.go index 8b97a60ea03..91fabcd287c 100644 --- a/pkg/skaffold/build/local/local.go +++ b/pkg/skaffold/build/local/local.go @@ -33,9 +33,7 @@ import ( // its checksum. It streams build progress to the writer argument. func (b *Builder) Build(ctx context.Context, out io.Writer, tagger tag.Tagger, artifacts []*latest.Artifact) ([]build.Artifact, error) { if b.localCluster { - if _, err := color.Default.Fprintf(out, "Found [%s] context, using local docker daemon.\n", b.kubeContext); err != nil { - return nil, errors.Wrap(err, "writing status") - } + color.Default.Fprintf(out, "Found [%s] context, using local docker daemon.\n", b.kubeContext) } defer b.api.Close() @@ -105,7 +103,7 @@ func (b *Builder) retagAndPush(ctx context.Context, out io.Writer, initialTag st } if b.pushImages { - if err := docker.RunPush(ctx, b.api, newTag, out); err != nil { + if err := docker.RunPush(ctx, out, b.api, newTag); err != nil { return errors.Wrap(err, "pushing") } } diff --git a/pkg/skaffold/build/local/local_test.go b/pkg/skaffold/build/local/local_test.go index d91164979d6..aee0cf20832 100644 --- a/pkg/skaffold/build/local/local_test.go +++ b/pkg/skaffold/build/local/local_test.go @@ -19,7 +19,6 @@ package local import ( "context" "fmt" - "io" "io/ioutil" "testing" @@ -57,8 +56,7 @@ func TestLocalRun(t *testing.T) { var tests = []struct { description string - out io.Writer - api docker.APIClient + api testutil.FakeAPIClient tagger tag.Tagger artifacts []*latest.Artifact expected []build.Artifact @@ -67,90 +65,80 @@ func TestLocalRun(t *testing.T) { }{ { description: "single build", - out: ioutil.Discard, - artifacts: []*latest.Artifact{ - { - ImageName: "gcr.io/test/image", - ArtifactType: latest.ArtifactType{ - DockerArtifact: &latest.DockerArtifact{}, - }, - }, + artifacts: []*latest.Artifact{{ + ImageName: "gcr.io/test/image", + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{}, + }}, }, tagger: &FakeTagger{Out: "gcr.io/test/image:tag"}, - api: testutil.NewFakeImageAPIClient(map[string]string{}, &testutil.FakeImageAPIOptions{}), - expected: []build.Artifact{ - { - ImageName: "gcr.io/test/image", - Tag: "gcr.io/test/image:tag", - }, + expected: []build.Artifact{{ + ImageName: "gcr.io/test/image", + Tag: "gcr.io/test/image:tag", + }}, + }, + { + description: "single build local cluster", + artifacts: []*latest.Artifact{{ + ImageName: "gcr.io/test/image", + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{}, + }}, }, + tagger: &FakeTagger{Out: "gcr.io/test/image:tag"}, + localCluster: true, + expected: []build.Artifact{{ + ImageName: "gcr.io/test/image", + Tag: "gcr.io/test/image:tag", + }}, }, { description: "subset build", - out: ioutil.Discard, tagger: &FakeTagger{Out: "gcr.io/test/image:tag"}, - artifacts: []*latest.Artifact{ - { - ImageName: "gcr.io/test/image", - ArtifactType: latest.ArtifactType{ - DockerArtifact: &latest.DockerArtifact{}, - }, - }, + artifacts: []*latest.Artifact{{ + ImageName: "gcr.io/test/image", + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{}, + }}, }, - api: testutil.NewFakeImageAPIClient(map[string]string{}, &testutil.FakeImageAPIOptions{}), - expected: []build.Artifact{ - { - ImageName: "gcr.io/test/image", - Tag: "gcr.io/test/image:tag", - }, - }, - }, - { - description: "local cluster bad writer", - out: &testutil.BadWriter{}, - shouldErr: true, - localCluster: true, + expected: []build.Artifact{{ + ImageName: "gcr.io/test/image", + Tag: "gcr.io/test/image:tag", + }}, }, { description: "error image build", - out: ioutil.Discard, artifacts: []*latest.Artifact{{}}, - api: testutil.NewFakeImageAPIClient(map[string]string{}, &testutil.FakeImageAPIOptions{ + api: testutil.FakeAPIClient{ ErrImageBuild: true, - }), + }, shouldErr: true, }, { description: "error image tag", - out: ioutil.Discard, artifacts: []*latest.Artifact{{}}, - api: testutil.NewFakeImageAPIClient(map[string]string{}, &testutil.FakeImageAPIOptions{ + api: testutil.FakeAPIClient{ ErrImageTag: true, - }), + }, shouldErr: true, }, { - description: "bad writer", - out: &testutil.BadWriter{}, + description: "unkown artifact type", artifacts: []*latest.Artifact{{}}, - api: testutil.NewFakeImageAPIClient(map[string]string{}, &testutil.FakeImageAPIOptions{}), shouldErr: true, }, { description: "error image inspect", - out: &testutil.BadWriter{}, artifacts: []*latest.Artifact{{}}, - api: testutil.NewFakeImageAPIClient(map[string]string{}, &testutil.FakeImageAPIOptions{ + api: testutil.FakeAPIClient{ ErrImageInspect: true, - }), + }, shouldErr: true, }, { description: "error tagger", - out: ioutil.Discard, artifacts: []*latest.Artifact{{}}, tagger: &FakeTagger{Err: fmt.Errorf("")}, - api: testutil.NewFakeImageAPIClient(map[string]string{}, &testutil.FakeImageAPIOptions{}), shouldErr: true, }, } @@ -159,11 +147,11 @@ func TestLocalRun(t *testing.T) { t.Run(test.description, func(t *testing.T) { l := Builder{ cfg: &latest.LocalBuild{}, - api: test.api, + api: &test.api, localCluster: test.localCluster, } - res, err := l.Build(context.Background(), test.out, test.tagger, test.artifacts) + res, err := l.Build(context.Background(), ioutil.Discard, test.tagger, test.artifacts) testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, res) }) } diff --git a/pkg/skaffold/docker/image.go b/pkg/skaffold/docker/image.go index 9889a42678c..92d9a04b0a8 100644 --- a/pkg/skaffold/docker/image.go +++ b/pkg/skaffold/docker/image.go @@ -32,7 +32,7 @@ import ( "github.com/docker/docker/pkg/term" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" - "github.com/google/go-containerregistry/pkg/v1" + v1 "github.com/google/go-containerregistry/pkg/v1" "github.com/google/go-containerregistry/pkg/v1/remote" "github.com/google/go-containerregistry/pkg/v1/remote/transport" "github.com/pkg/errors" @@ -83,7 +83,7 @@ func StreamDockerMessages(dst io.Writer, src io.Reader) error { return jsonmessage.DisplayJSONMessagesStream(src, dst, fd, false, nil) } -func RunPush(ctx context.Context, cli APIClient, ref string, out io.Writer) error { +func RunPush(ctx context.Context, out io.Writer, cli APIClient, ref string) error { registryAuth, err := encodedRegistryAuth(ctx, cli, DefaultAuthHelper, ref) if err != nil { return errors.Wrapf(err, "getting auth config for %s", ref) diff --git a/pkg/skaffold/docker/image_test.go b/pkg/skaffold/docker/image_test.go index d0880fac0fa..ed48781b9cd 100644 --- a/pkg/skaffold/docker/image_test.go +++ b/pkg/skaffold/docker/image_test.go @@ -18,7 +18,6 @@ package docker import ( "context" - "fmt" "io/ioutil" "os" "testing" @@ -37,91 +36,80 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } -type testImageAPI struct { - description string - imageName string - tagToImageID map[string]string - shouldErr bool - expected string - - testOpts *testutil.FakeImageAPIOptions -} - func TestRunPush(t *testing.T) { - var tests = []testImageAPI{ + var tests = []struct { + description string + imageName string + api testutil.FakeAPIClient + shouldErr bool + }{ { - description: "push", - imageName: "gcr.io/scratchman", - tagToImageID: map[string]string{}, + description: "push", + imageName: "gcr.io/scratchman", }, { - description: "no error pushing non canonical tag", - imageName: "noncanonicalscratchman", - tagToImageID: map[string]string{}, + description: "no error pushing non canonical tag", + imageName: "noncanonicalscratchman", }, { - description: "no error pushing canonical tag", - imageName: "canonical/name", - tagToImageID: map[string]string{}, + description: "no error pushing canonical tag", + imageName: "canonical/name", }, { - description: "stream error", - imageName: "gcr.io/imthescratchman", - tagToImageID: map[string]string{}, - testOpts: &testutil.FakeImageAPIOptions{ - ReturnBody: &testutil.FakeReaderCloser{Err: fmt.Errorf("")}, + description: "stream error", + imageName: "gcr.io/imthescratchman", + api: testutil.FakeAPIClient{ + ErrStream: true, }, shouldErr: true, }, { - description: "image push error", - imageName: "gcr.io/skibabopbadopbop", - tagToImageID: map[string]string{}, - testOpts: &testutil.FakeImageAPIOptions{ + description: "image push error", + imageName: "gcr.io/skibabopbadopbop", + api: testutil.FakeAPIClient{ ErrImagePush: true, }, shouldErr: true, }, } - for _, test := range tests { t.Run(test.description, func(t *testing.T) { - api := testutil.NewFakeImageAPIClient(test.tagToImageID, test.testOpts) - err := RunPush(context.Background(), api, test.imageName, ioutil.Discard) + err := RunPush(context.Background(), ioutil.Discard, &test.api, test.imageName) + testutil.CheckError(t, test.shouldErr, err) }) } } func TestRunBuildArtifact(t *testing.T) { - var tests = []testImageAPI{ + var tests = []struct { + description string + expected string + api testutil.FakeAPIClient + shouldErr bool + }{ { - description: "build", - tagToImageID: map[string]string{}, - expected: "test", + description: "build", + expected: "test", }, { - description: "bad image build", - tagToImageID: map[string]string{}, - testOpts: &testutil.FakeImageAPIOptions{ + description: "bad image build", + api: testutil.FakeAPIClient{ ErrImageBuild: true, }, shouldErr: true, }, { - description: "bad return reader", - tagToImageID: map[string]string{}, - testOpts: &testutil.FakeImageAPIOptions{ - ReturnBody: &testutil.FakeReaderCloser{Err: fmt.Errorf("")}, + description: "bad return reader", + api: testutil.FakeAPIClient{ + ErrStream: true, }, shouldErr: true, }, } for _, test := range tests { t.Run(test.description, func(t *testing.T) { - api := testutil.NewFakeImageAPIClient(test.tagToImageID, test.testOpts) - - err := BuildArtifact(context.Background(), ioutil.Discard, api, ".", &latest.DockerArtifact{}, "finalimage") + err := BuildArtifact(context.Background(), ioutil.Discard, &test.api, ".", &latest.DockerArtifact{}, "finalimage") testutil.CheckError(t, test.shouldErr, err) }) @@ -129,22 +117,27 @@ func TestRunBuildArtifact(t *testing.T) { } func TestDigest(t *testing.T) { - var tests = []testImageAPI{ + var tests = []struct { + description string + imageName string + api testutil.FakeAPIClient + expected string + shouldErr bool + }{ { description: "get digest", imageName: "identifier:latest", - tagToImageID: map[string]string{ - "identifier:latest": "sha256:123abc", + api: testutil.FakeAPIClient{ + TagToImageID: map[string]string{ + "identifier:latest": "sha256:123abc", + }, }, expected: "sha256:123abc", }, { description: "image inspect error", imageName: "test", - tagToImageID: map[string]string{ - "test:latest": "sha256:123abc", - }, - testOpts: &testutil.FakeImageAPIOptions{ + api: testutil.FakeAPIClient{ ErrImageInspect: true, }, shouldErr: true, @@ -152,17 +145,12 @@ func TestDigest(t *testing.T) { { description: "not found", imageName: "somethingelse", - tagToImageID: map[string]string{ - "test:latest": "sha256:123abc", - }, - expected: "", + expected: "", }, } for _, test := range tests { t.Run(test.description, func(t *testing.T) { - api := testutil.NewFakeImageAPIClient(test.tagToImageID, test.testOpts) - - digest, err := Digest(context.Background(), api, test.imageName) + digest, err := Digest(context.Background(), &test.api, test.imageName) testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, digest) }) diff --git a/testutil/fake_image_api.go b/testutil/fake_image_api.go index cde4ad33930..4a6d8eab6a7 100644 --- a/testutil/fake_image_api.go +++ b/testutil/fake_image_api.go @@ -21,6 +21,7 @@ import ( "crypto/rand" "fmt" "io" + "io/ioutil" "strings" "github.com/docker/docker/api/types" @@ -28,52 +29,49 @@ import ( "github.com/docker/docker/registry" ) -type FakeImageAPIClient struct { +type FakeAPIClient struct { *client.Client - tagToImageID map[string]string - opts *FakeImageAPIOptions -} - -type FakeImageAPIOptions struct { + TagToImageID map[string]string ErrImageBuild bool ErrImageInspect bool ErrImageTag bool ErrImagePush bool - - ReturnBody io.ReadCloser + ErrStream bool } -func NewFakeImageAPIClient(initContents map[string]string, opts *FakeImageAPIOptions) *FakeImageAPIClient { - if opts == nil { - opts = &FakeImageAPIOptions{} - } - if opts.ReturnBody == nil { - opts.ReturnBody = FakeReaderCloser{Err: io.EOF} - } - return &FakeImageAPIClient{ - tagToImageID: initContents, - opts: opts, +type errReader struct{} + +func (f errReader) Read([]byte) (int, error) { return 0, fmt.Errorf("") } + +func (f *FakeAPIClient) body() io.ReadCloser { + if f.ErrStream { + return ioutil.NopCloser(&errReader{}) } + return ioutil.NopCloser(strings.NewReader("")) } -func (f *FakeImageAPIClient) ImageBuild(ctx context.Context, context io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) { - if f.opts.ErrImageBuild { +func (f *FakeAPIClient) ImageBuild(_ context.Context, _ io.Reader, options types.ImageBuildOptions) (types.ImageBuildResponse, error) { + if f.ErrImageBuild { return types.ImageBuildResponse{}, fmt.Errorf("") } - for _, tag := range options.Tags { - imageID := "sha256:" + randomID() + if f.TagToImageID == nil { + f.TagToImageID = make(map[string]string) + } + + imageID := "sha256:" + randomID() + f.TagToImageID[imageID] = imageID - f.tagToImageID[tag] = imageID - f.tagToImageID[imageID] = imageID + for _, tag := range options.Tags { + f.TagToImageID[tag] = imageID if !strings.Contains(tag, ":") { - f.tagToImageID[tag+":latest"] = imageID + f.TagToImageID[tag+":latest"] = imageID } } return types.ImageBuildResponse{ - Body: f.opts.ReturnBody, + Body: f.body(), }, nil } @@ -83,43 +81,46 @@ func randomID() string { return fmt.Sprintf("%x", b) } -func (f *FakeImageAPIClient) ImageInspectWithRaw(ctx context.Context, ref string) (types.ImageInspect, []byte, error) { - if f.opts.ErrImageInspect { +func (f *FakeAPIClient) ImageInspectWithRaw(_ context.Context, ref string) (types.ImageInspect, []byte, error) { + if f.ErrImageInspect { return types.ImageInspect{}, nil, fmt.Errorf("") } - imageID, ok := f.tagToImageID[ref] - if !ok { - return types.ImageInspect{}, nil, nil - } - - return types.ImageInspect{ID: imageID}, nil, nil + return types.ImageInspect{ + ID: f.TagToImageID[ref], + }, nil, nil } -func (f *FakeImageAPIClient) ImageTag(ctx context.Context, image, ref string) error { - if f.opts.ErrImageTag { +func (f *FakeAPIClient) ImageTag(_ context.Context, image, ref string) error { + if f.ErrImageTag { return fmt.Errorf("") } - imageID, ok := f.tagToImageID[image] + + imageID, ok := f.TagToImageID[image] if !ok { - return fmt.Errorf("image %s not found. fake registry contents: %s", image, f.tagToImageID) + return fmt.Errorf("image %s not found. fake registry contents: %s", image, f.TagToImageID) } - f.tagToImageID[ref] = imageID + + if f.TagToImageID == nil { + f.TagToImageID = make(map[string]string) + } + f.TagToImageID[ref] = imageID + return nil } -func (f *FakeImageAPIClient) ImagePush(_ context.Context, _ string, _ types.ImagePushOptions) (io.ReadCloser, error) { - var err error - if f.opts.ErrImagePush { - err = fmt.Errorf("") +func (f *FakeAPIClient) ImagePush(context.Context, string, types.ImagePushOptions) (io.ReadCloser, error) { + if f.ErrImagePush { + return nil, fmt.Errorf("") } - return f.opts.ReturnBody, err + + return f.body(), nil } -func (f *FakeImageAPIClient) Info(ctx context.Context) (types.Info, error) { +func (f *FakeAPIClient) Info(context.Context) (types.Info, error) { return types.Info{ IndexServerAddress: registry.IndexServer, }, nil } -func (f *FakeImageAPIClient) Close() error { return nil } +func (f *FakeAPIClient) Close() error { return nil } diff --git a/testutil/util.go b/testutil/util.go index 24764fdeed9..f0721c27916 100644 --- a/testutil/util.go +++ b/testutil/util.go @@ -28,21 +28,6 @@ import ( "github.com/google/go-cmp/cmp" ) -type BadReader struct{} - -func (BadReader) Read([]byte) (int, error) { return 0, errors.New("bad read") } - -type BadWriter struct{} - -func (BadWriter) Write([]byte) (int, error) { return 0, errors.New("bad write") } - -type FakeReaderCloser struct { - Err error -} - -func (f FakeReaderCloser) Close() error { return nil } -func (f FakeReaderCloser) Read([]byte) (int, error) { return 0, f.Err } - func CheckDeepEqual(t *testing.T, expected, actual interface{}) { t.Helper() if diff := cmp.Diff(actual, expected); diff != "" {