Remove duplication

Signed-off-by: David Gageot <david@gageot.net>

pkg/skaffold/build/bazel.go

@@ -61,20 +61,19 @@ func buildImageTag(buildTarget string) string {
 	return fmt.Sprintf(":%s", imageTag)
 }
 
-func (l *LocalBuilder) buildBazel(ctx context.Context, out io.Writer, a *v1alpha2.Artifact) (string, error) {
-	cmd := exec.Command("bazel", "build", a.BazelArtifact.BuildTarget)
-	cmd.Dir = a.Workspace
+func (l *LocalBuilder) buildBazel(ctx context.Context, out io.Writer, workspace string, a *v1alpha2.BazelArtifact) (string, error) {
+	cmd := exec.Command("bazel", "build", a.BuildTarget)
+	cmd.Dir = workspace
 	cmd.Stdout = out
 	cmd.Stderr = out
 	if err := cmd.Run(); err != nil {
 		return "", errors.Wrap(err, "running command")
 	}
 
-	tarPath := buildTarPath(a.BazelArtifact.BuildTarget)
+	tarPath := buildTarPath(a.BuildTarget)
+	imageTag := buildImageTag(a.BuildTarget)
 
-	imageTag := buildImageTag(a.BazelArtifact.BuildTarget)
-
-	imageTar, err := os.Open(filepath.Join(a.Workspace, "bazel-bin", tarPath))
+	imageTar, err := os.Open(filepath.Join(workspace, "bazel-bin", tarPath))
 	if err != nil {
 		return "", errors.Wrap(err, "opening image tarball")
 	}

pkg/skaffold/build/local.go

@@ -20,14 +20,13 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"os"
-	"path/filepath"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
+	"github.com/docker/docker/api/types"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -76,14 +75,14 @@ func (l *LocalBuilder) Labels() map[string]string {
 }
 
 func (l *LocalBuilder) runBuildForArtifact(ctx context.Context, out io.Writer, artifact *v1alpha2.Artifact) (string, error) {
-	if artifact.DockerArtifact != nil {
-		return l.buildDocker(ctx, out, artifact)
+	switch {
+	case artifact.DockerArtifact != nil:
+		return l.buildDocker(ctx, out, artifact.Workspace, artifact.DockerArtifact)
+	case artifact.BazelArtifact != nil:
+		return l.buildBazel(ctx, out, artifact.Workspace, artifact.BazelArtifact)
+	default:
+		return "", fmt.Errorf("undefined artifact type: %+v", artifact.ArtifactType)
 	}
-	if artifact.BazelArtifact != nil {
-		return l.buildBazel(ctx, out, artifact)
-	}
-
-	return "", fmt.Errorf("undefined artifact type: %+v", artifact.ArtifactType)
 }
 
 // Build runs a docker build on the host and tags the resulting image with
@@ -141,24 +140,14 @@ func (l *LocalBuilder) Build(ctx context.Context, out io.Writer, tagger tag.Tagg
 	return builds, nil
 }
 
-func (l *LocalBuilder) buildDocker(ctx context.Context, out io.Writer, a *v1alpha2.Artifact) (string, error) {
-	// Add a sanity check to check if the dockerfile exists before running the build
-	if _, err := os.Stat(filepath.Join(a.Workspace, a.DockerArtifact.DockerfilePath)); err != nil {
-		if os.IsNotExist(err) {
-			return "", fmt.Errorf("Could not find dockerfile: %s", a.DockerArtifact.DockerfilePath)
-		}
-		return "", errors.Wrap(err, "stat dockerfile")
-	}
-
+func (l *LocalBuilder) buildDocker(ctx context.Context, out io.Writer, workspace string, a *v1alpha2.DockerArtifact) (string, error) {
 	initialTag := util.RandomID()
-	err := docker.RunBuild(ctx, l.api, &docker.BuildOptions{
-		ImageName:   initialTag,
-		Dockerfile:  a.DockerArtifact.DockerfilePath,
-		ContextDir:  a.Workspace,
-		ProgressBuf: out,
-		BuildBuf:    out,
-		BuildArgs:   a.DockerArtifact.BuildArgs,
-		CacheFrom:   a.DockerArtifact.CacheFrom,
+
+	err := docker.RunBuild(ctx, out, l.api, workspace, types.ImageBuildOptions{
+		Tags:       []string{initialTag},
+		Dockerfile: a.DockerfilePath,
+		BuildArgs:  a.BuildArgs,
+		CacheFrom:  a.CacheFrom,
 	})
 	if err != nil {
 		return "", errors.Wrap(err, "running build")

pkg/skaffold/docker/image.go

@@ -18,8 +18,11 @@ package docker
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net/http"
+	"os"
+	"path/filepath"
 
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/api/types/filters"
@@ -36,51 +39,42 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-type BuildOptions struct {
-	ImageName   string
-	Dockerfile  string
-	ContextDir  string
-	ProgressBuf io.Writer
-	BuildBuf    io.Writer
-	BuildArgs   map[string]*string
-	CacheFrom   []string
-}
-
 // RunBuild performs a docker build and returns nothing
-func RunBuild(ctx context.Context, cli APIClient, opts *BuildOptions) error {
-	logrus.Debugf("Running docker build: context: %s, dockerfile: %s", opts.ContextDir, opts.Dockerfile)
+func RunBuild(ctx context.Context, out io.Writer, cli APIClient, workspace string, opts types.ImageBuildOptions) error {
+	logrus.Debugf("Running docker build: context: %s, dockerfile: %s", workspace, opts.Dockerfile)
+
+	// Add a sanity check to check if the dockerfile exists before running the build
+	if _, err := os.Stat(filepath.Join(workspace, opts.Dockerfile)); err != nil {
+		if os.IsNotExist(err) {
+			return fmt.Errorf("Could not find dockerfile: %s", opts.Dockerfile)
+		}
+		return errors.Wrap(err, "stat dockerfile")
+	}
 
 	// Like `docker build`, we ignore the errors
 	// See https://github.com/docker/cli/blob/75c1bb1f33d7cedbaf48404597d5bf9818199480/cli/command/image/build.go#L364
 	authConfigs, _ := DefaultAuthHelper.GetAllAuthConfigs()
-
-	imageBuildOpts := types.ImageBuildOptions{
-		Tags:        []string{opts.ImageName},
-		Dockerfile:  opts.Dockerfile,
-		BuildArgs:   opts.BuildArgs,
-		AuthConfigs: authConfigs,
-		CacheFrom:   opts.CacheFrom,
-	}
+	opts.AuthConfigs = authConfigs
 
 	buildCtx, buildCtxWriter := io.Pipe()
 	go func() {
-		err := CreateDockerTarContext(buildCtxWriter, opts.Dockerfile, opts.ContextDir)
+		err := CreateDockerTarContext(buildCtxWriter, opts.Dockerfile, workspace)
 		if err != nil {
 			buildCtxWriter.CloseWithError(errors.Wrap(err, "creating docker context"))
 			return
 		}
 		buildCtxWriter.Close()
 	}()
 
-	progressOutput := streamformatter.NewProgressOutput(opts.ProgressBuf)
+	progressOutput := streamformatter.NewProgressOutput(out)
 	body := progress.NewProgressReader(buildCtx, progressOutput, 0, "", "Sending build context to Docker daemon")
 
-	resp, err := cli.ImageBuild(ctx, body, imageBuildOpts)
+	resp, err := cli.ImageBuild(ctx, body, opts)
 	if err != nil {
 		return errors.Wrap(err, "docker build")
 	}
 	defer resp.Body.Close()
-	return StreamDockerMessages(opts.BuildBuf, resp.Body)
+	return StreamDockerMessages(out, resp.Body)
 }
 
 // StreamDockerMessages streams formatted json output from the docker daemon

pkg/skaffold/docker/image_test.go

@@ -20,10 +20,13 @@ import (
 	"bytes"
 	"context"
 	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/testutil"
+	"github.com/docker/docker/api/types"
 )
 
 func TestMain(m *testing.M) {
@@ -116,11 +119,17 @@ func TestRunBuild(t *testing.T) {
 	}
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
+			tmp, cleanup := testutil.TempDir(t)
+			defer cleanup()
+
+			ioutil.WriteFile(filepath.Join(tmp, "Dockerfile"), []byte{}, os.ModePerm)
+
 			api := testutil.NewFakeImageAPIClient(test.tagToImageID, test.testOpts)
-			err := RunBuild(context.Background(), api, &BuildOptions{
+			err := RunBuild(context.Background(), ioutil.Discard, api, tmp, types.ImageBuildOptions{
 				Dockerfile: "Dockerfile",
-				ImageName:  "finalimage",
+				Tags:       []string{"finalimage"},
 			})
+
 			testutil.CheckError(t, test.shouldErr, err)
 		})
 	}