Skip to content

Commit

Permalink
Merge pull request #1424 from dgageot/simplify-fake-docker-api
Browse files Browse the repository at this point in the history 
Simplify fake docker api
  • Loading branch information
@dgageot
dgageot authored Dec 27, 2018
2 parents 48ca967 + a1f7cbc commit 38a3404
Show file tree
Hide file tree
Showing 6 changed files with 142 additions and 182 deletions.
6 changes: 2 additions & 4 deletions pkg/skaffold/build/local/local.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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")
}
}
Expand Down
96 changes: 42 additions & 54 deletions pkg/skaffold/build/local/local_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package local
import (
"context"
"fmt"
"io"
"io/ioutil"
"testing"

Expand Down Expand Up @@ -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
Expand All @@ -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,
},
}
Expand All @@ -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)
})
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/docker/image.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
110 changes: 49 additions & 61 deletions pkg/skaffold/docker/image_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package docker

import (
"context"
"fmt"
"io/ioutil"
"os"
"testing"
Expand All @@ -37,132 +36,121 @@ 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)
})
}
}

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,
},
{
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)
})
Expand Down
Loading

0 comments on commit 38a3404

Please sign in to comment.