Simplify docker related code.

Now that `skaffold docker` commands are removed, the code
can be simplified.

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

pkg/skaffold/bazel/bazel.go

@@ -35,9 +35,10 @@ func query(target string) string {
 }
 
 // GetDependencies finds the sources dependencies for the given bazel artifact.
-func GetDependencies(a *v1alpha2.Artifact) ([]string, error) {
-	cmd := exec.Command("bazel", "query", query(a.BazelArtifact.BuildTarget), "--noimplicit_deps", "--order_output=no")
-	cmd.Dir = a.Workspace
+// All paths are relative to the workspace.
+func GetDependencies(workspace string, a *v1alpha2.BazelArtifact) ([]string, error) {
+	cmd := exec.Command("bazel", "query", query(a.BuildTarget), "--noimplicit_deps", "--order_output=no")
+	cmd.Dir = workspace
 	stdout, err := util.RunCmdOut(cmd)
 	if err != nil {
 		return nil, errors.Wrap(err, "getting bazel dependencies")
@@ -59,7 +60,7 @@ func GetDependencies(a *v1alpha2.Artifact) ([]string, error) {
 		deps = append(deps, depToPath(l))
 	}
 
-	if _, err := os.Stat(filepath.Join(a.Workspace, "WORKSPACE")); err == nil {
+	if _, err := os.Stat(filepath.Join(workspace, "WORKSPACE")); err == nil {
 		deps = append(deps, "WORKSPACE")
 	}
 

pkg/skaffold/build/gcb/container_builder.go

@@ -87,7 +87,7 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, tagger tag.T
 	}
 
 	fmt.Fprintf(out, "Pushing code to gs://%s/%s\n", cbBucket, buildObject)
-	if err := docker.UploadContextToGCS(ctx, artifact.Workspace, artifact.DockerArtifact.DockerfilePath, cbBucket, buildObject); err != nil {
+	if err := docker.UploadContextToGCS(ctx, artifact.Workspace, artifact.DockerArtifact, cbBucket, buildObject); err != nil {
 		return "", errors.Wrap(err, "uploading source tarball")
 	}
 

pkg/skaffold/build/kaniko/run.go

@@ -44,7 +44,7 @@ func runKaniko(ctx context.Context, out io.Writer, artifact *v1alpha2.Artifact,
 
 	initialTag := util.RandomID()
 	tarName := fmt.Sprintf("context-%s.tar.gz", initialTag)
-	if err := docker.UploadContextToGCS(ctx, artifact.Workspace, dockerfilePath, cfg.GCSBucket, tarName); err != nil {
+	if err := docker.UploadContextToGCS(ctx, artifact.Workspace, artifact.DockerArtifact, cfg.GCSBucket, tarName); err != nil {
 		return "", errors.Wrap(err, "uploading tar to gcs")
 	}
 	defer gcsDelete(ctx, cfg.GCSBucket, tarName)

pkg/skaffold/build/local/docker.go

@@ -26,7 +26,6 @@ import (
 	"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"
 )
 
@@ -56,13 +55,7 @@ func (b *Builder) buildDocker(ctx context.Context, out io.Writer, workspace stri
 			return "", errors.Wrap(err, "running build")
 		}
 	} else {
-		err := docker.RunBuild(ctx, out, b.api, workspace, types.ImageBuildOptions{
-			Tags:       []string{initialTag},
-			Dockerfile: a.DockerfilePath,
-			BuildArgs:  a.BuildArgs,
-			CacheFrom:  a.CacheFrom,
-		})
-		if err != nil {
+		if err := docker.BuildArtifact(ctx, out, b.api, workspace, a, initialTag); err != nil {
 			return "", errors.Wrap(err, "running build")
 		}
 	}

pkg/skaffold/docker/context.go

@@ -23,6 +23,7 @@ import (
 	"strings"
 
 	cstorage "cloud.google.com/go/storage"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/pkg/errors"
 )
@@ -39,37 +40,41 @@ func NormalizeDockerfilePath(context, dockerfile string) (string, error) {
 	return filepath.Abs(dockerfile)
 }
 
-func CreateDockerTarContext(buildArgs map[string]*string, w io.Writer, context, dockerfilePath string) error {
-	paths, err := GetDependencies(buildArgs, context, dockerfilePath)
+func CreateDockerTarContext(w io.Writer, workspace string, a *v1alpha2.DockerArtifact) error {
+	paths, err := GetDependencies(workspace, a)
 	if err != nil {
 		return errors.Wrap(err, "getting relative tar paths")
 	}
-	if err := util.CreateTar(w, context, paths); err != nil {
+
+	if err := util.CreateTar(w, workspace, paths); err != nil {
 		return errors.Wrap(err, "creating tar gz")
 	}
+
 	return nil
 }
 
-func CreateDockerTarGzContext(buildArgs map[string]*string, w io.Writer, context, dockerfilePath string) error {
-	paths, err := GetDependencies(buildArgs, context, dockerfilePath)
+func CreateDockerTarGzContext(w io.Writer, workspace string, a *v1alpha2.DockerArtifact) error {
+	paths, err := GetDependencies(workspace, a)
 	if err != nil {
 		return errors.Wrap(err, "getting relative tar paths")
 	}
-	if err := util.CreateTarGz(w, context, paths); err != nil {
+
+	if err := util.CreateTarGz(w, workspace, paths); err != nil {
 		return errors.Wrap(err, "creating tar gz")
 	}
+
 	return nil
 }
 
-func UploadContextToGCS(ctx context.Context, context, dockerfilePath, bucket, objectName string) error {
+func UploadContextToGCS(ctx context.Context, workspace string, a *v1alpha2.DockerArtifact, bucket, objectName string) error {
 	c, err := cstorage.NewClient(ctx)
 	if err != nil {
 		return err
 	}
 	defer c.Close()
 
 	w := c.Bucket(bucket).Object(objectName).NewWriter(ctx)
-	if err := CreateDockerTarGzContext(map[string]*string{}, w, context, dockerfilePath); err != nil {
+	if err := CreateDockerTarGzContext(w, workspace, a); err != nil {
 		return errors.Wrap(err, "uploading targz to google storage")
 	}
 	return w.Close()

pkg/skaffold/docker/context_test.go

@@ -24,6 +24,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
@@ -36,6 +37,11 @@ func TestDockerContext(t *testing.T) {
 		RetrieveImage = retrieveImage
 	}()
 
+	artifact := &v1alpha2.DockerArtifact{
+		DockerfilePath: "Dockerfile",
+		BuildArgs:      map[string]*string{},
+	}
+
 	os.Mkdir(filepath.Join(tmpDir, "files"), 0750)
 	ioutil.WriteFile(filepath.Join(tmpDir, "files", "ignored.txt"), []byte(""), 0644)
 	ioutil.WriteFile(filepath.Join(tmpDir, "files", "included.txt"), []byte(""), 0644)
@@ -45,7 +51,7 @@ func TestDockerContext(t *testing.T) {
 	ioutil.WriteFile(filepath.Join(tmpDir, "alsoignored.txt"), []byte(""), 0644)
 	reader, writer := io.Pipe()
 	go func() {
-		err := CreateDockerTarContext(map[string]*string{}, writer, tmpDir, "Dockerfile")
+		err := CreateDockerTarContext(writer, tmpDir, artifact)
 		if err != nil {
 			writer.CloseWithError(err)
 		} else {

pkg/skaffold/docker/image.go

@@ -39,18 +39,17 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-// RunBuild performs a docker build and returns nothing
-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)
+// BuildArtifact performs a docker build and returns nothing
+func BuildArtifact(ctx context.Context, out io.Writer, cli APIClient, workspace string, a *v1alpha2.DockerArtifact, initialTag string) error {
+	logrus.Debugf("Running docker build: context: %s, dockerfile: %s", workspace, a.DockerfilePath)
 
 	// Like `docker build`, we ignore the errors
 	// See https://github.com/docker/cli/blob/75c1bb1f33d7cedbaf48404597d5bf9818199480/cli/command/image/build.go#L364
 	authConfigs, _ := DefaultAuthHelper.GetAllAuthConfigs()
-	opts.AuthConfigs = authConfigs
 
 	buildCtx, buildCtxWriter := io.Pipe()
 	go func() {
-		err := CreateDockerTarContext(opts.BuildArgs, buildCtxWriter, workspace, opts.Dockerfile)
+		err := CreateDockerTarContext(buildCtxWriter, workspace, a)
 		if err != nil {
 			buildCtxWriter.CloseWithError(errors.Wrap(err, "creating docker context"))
 			return
@@ -61,11 +60,18 @@ func RunBuild(ctx context.Context, out io.Writer, cli APIClient, workspace strin
 	progressOutput := streamformatter.NewProgressOutput(out)
 	body := progress.NewProgressReader(buildCtx, progressOutput, 0, "", "Sending build context to Docker daemon")
 
-	resp, err := cli.ImageBuild(ctx, body, opts)
+	resp, err := cli.ImageBuild(ctx, body, types.ImageBuildOptions{
+		Tags:        []string{initialTag},
+		Dockerfile:  a.DockerfilePath,
+		BuildArgs:   a.BuildArgs,
+		CacheFrom:   a.CacheFrom,
+		AuthConfigs: authConfigs,
+	})
 	if err != nil {
 		return errors.Wrap(err, "docker build")
 	}
 	defer resp.Body.Close()
+
 	return StreamDockerMessages(out, resp.Body)
 }
 

pkg/skaffold/docker/image_test.go

@@ -22,13 +22,11 @@ import (
 	"fmt"
 	"io/ioutil"
 	"os"
-	"path/filepath"
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
-	"github.com/docker/docker/api/types"
 	"github.com/google/go-cmp/cmp"
 )
 
@@ -96,7 +94,7 @@ func TestRunPush(t *testing.T) {
 	}
 }
 
-func TestRunBuild(t *testing.T) {
+func TestRunBuildArtifact(t *testing.T) {
 	var tests = []testImageAPI{
 		{
 			description:  "build",
@@ -122,16 +120,9 @@ 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(), ioutil.Discard, api, tmp, types.ImageBuildOptions{
-				Dockerfile: "Dockerfile",
-				Tags:       []string{"finalimage"},
-			})
+
+			err := BuildArtifact(context.Background(), ioutil.Discard, api, ".", &v1alpha2.DockerArtifact{}, "finalimage")
 
 			testutil.CheckError(t, test.shouldErr, err)
 		})

pkg/skaffold/docker/parse.go

@@ -26,6 +26,7 @@ import (
 	"strings"
 	"sync"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/docker/docker/builder/dockerignore"
 	"github.com/docker/docker/pkg/fileutils"
 	"github.com/google/go-containerregistry/pkg/v1"
@@ -167,13 +168,15 @@ func readDockerfile(workspace, absDockerfilePath string, buildArgs map[string]*s
 	return deps, nil
 }
 
-func GetDependencies(buildArgs map[string]*string, workspace, dockerfilePath string) ([]string, error) {
-	absDockerfilePath, err := NormalizeDockerfilePath(workspace, dockerfilePath)
+// GetDependencies finds the sources dependencies for the given docker artifact.
+// All paths are relative to the workspace.
+func GetDependencies(workspace string, a *v1alpha2.DockerArtifact) ([]string, error) {
+	absDockerfilePath, err := NormalizeDockerfilePath(workspace, a.DockerfilePath)
 	if err != nil {
 		return nil, errors.Wrap(err, "normalizing dockerfile path")
 	}
 
-	deps, err := readDockerfile(workspace, absDockerfilePath, buildArgs)
+	deps, err := readDockerfile(workspace, absDockerfilePath, a.BuildArgs)
 	if err != nil {
 		return nil, err
 	}
@@ -225,8 +228,8 @@ func GetDependencies(buildArgs map[string]*string, workspace, dockerfilePath str
 	}
 
 	// Always add dockerfile even if it's .dockerignored. The daemon will need it anyways.
-	if !filepath.IsAbs(dockerfilePath) {
-		files[dockerfilePath] = true
+	if !filepath.IsAbs(a.DockerfilePath) {
+		files[a.DockerfilePath] = true
 	} else {
 		files[absDockerfilePath] = true
 	}

pkg/skaffold/docker/parse_test.go

@@ -23,6 +23,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 	"github.com/google/go-containerregistry/pkg/v1"
 )
@@ -307,7 +308,11 @@ func TestGetDependencies(t *testing.T) {
 				ioutil.WriteFile(filepath.Join(workspace, ".dockerignore"), []byte(test.ignore), 0644)
 			}
 
-			deps, err := GetDependencies(test.buildArgs, workspace, "Dockerfile")
+			deps, err := GetDependencies(workspace, &v1alpha2.DockerArtifact{
+				BuildArgs:      test.buildArgs,
+				DockerfilePath: "Dockerfile",
+			})
+
 			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, deps)
 		})
 	}

pkg/skaffold/watch/artifacts.go

@@ -18,11 +18,13 @@ package watch
 
 import (
 	"context"
+	"fmt"
 	"path/filepath"
 	"strings"
 	"time"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/bazel"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -112,7 +114,7 @@ func matchesAny(artifact *v1alpha2.Artifact, files []string) (bool, error) {
 		return false, nil
 	}
 
-	deps, err := build.DependenciesForArtifact(artifact)
+	deps, err := dependenciesForArtifact(artifact)
 	if err != nil {
 		return false, errors.Wrap(err, "getting dependencies for artifact")
 	}
@@ -128,6 +130,20 @@ func matchesAny(artifact *v1alpha2.Artifact, files []string) (bool, error) {
 	return false, nil
 }
 
+// All paths are relative to the workspace.
+func dependenciesForArtifact(a *v1alpha2.Artifact) ([]string, error) {
+	switch {
+	case a.DockerArtifact != nil:
+		return docker.GetDependencies(a.Workspace, a.DockerArtifact)
+
+	case a.BazelArtifact != nil:
+		return bazel.GetDependencies(a.Workspace, a.BazelArtifact)
+
+	default:
+		return nil, fmt.Errorf("undefined artifact type: %+v", a.ArtifactType)
+	}
+}
+
 // inWorkspace filters out the paths that are not in the artifact's workspace.
 func inWorkspace(artifact *v1alpha2.Artifact, files []string) ([]string, error) {
 	var inWorkspace []string