Merge pull request #840 from dgageot/shell-out-docker-build

Shell out docker build

cmd/skaffold/app/cmd/docker/deps.go

@@ -19,7 +19,6 @@ package docker
 import (
 	"io"
 	"path/filepath"
-	"strings"
 
 	cmdutil "github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/cmd/util"
 	"github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags"
@@ -63,7 +62,7 @@ func runDeps(out io.Writer, filename, dockerfile, context string) error {
 		return errors.Wrap(err, "parsing skaffold config")
 	}
 	// normalize the provided dockerfile path WRT to the context
-	normalizedPath, err := normalizeDockerfilePath(dockerfile, context)
+	normalizedPath, err := docker.NormalizeDockerfilePath(context, dockerfile)
 	if err != nil {
 		return errors.Wrap(err, "normalizing dockerfile path")
 	}
@@ -79,22 +78,13 @@ func runDeps(out io.Writer, filename, dockerfile, context string) error {
 	return nil
 }
 
-func normalizeDockerfilePath(dockerfile, context string) (string, error) {
-	if !filepath.IsAbs(dockerfile) {
-		if !strings.HasPrefix(dockerfile, context) {
-			dockerfile = filepath.Join(context, dockerfile)
-		}
-	}
-	return filepath.Abs(dockerfile)
-}
-
 func getBuildArgsForDockerfile(config *config.SkaffoldConfig, dockerfile string) map[string]*string {
 	var err error
 	for _, artifact := range config.Build.Artifacts {
 		if artifact.DockerArtifact != nil {
 			artifactPath := artifact.DockerArtifact.DockerfilePath
 			if artifact.Workspace != "" {
-				artifactPath, err = normalizeDockerfilePath(artifactPath, artifact.Workspace)
+				artifactPath, err = docker.NormalizeDockerfilePath(artifact.Workspace, artifactPath)
 				if err != nil {
 					logrus.Warnf("normalizing artifact dockerfile path: %s\n", err.Error())
 				}

examples/annotated-skaffold.yaml

@@ -62,27 +62,35 @@ build:
 # This next section is where you'll put your specific builder configuration.
   # Valid builders are `local`, `googleCloudBuild` and `kaniko.
   # Defaults to `local: {}`
-  # Example
+
+  # Pushing the images can be skipped. If no value is specified, it'll default to
+  # `true` on minikube or Docker for Desktop, for even faster build and deploy cycles.
+  # `false` on other types of kubernetes clusters that require pushing the images.
+  # skaffold defers to your ~/.docker/config for authentication information.
+  # If you're using Google Container Registry, make sure that you have gcloud and
+  # docker-credentials-helper-gcr configured correctly.
+  #
+  # By default, the local builder connects to the Docker daemon with Go code to build
+  # images. If `useDockerCLI` is set, skaffold will simply shell out to the docker CLI.
+  # `useBuildkit` can also be set to activate the experimental BuildKit feature.
+  #
   # local:
-    # Pushing the images can be skipped. If no value is specified, it'll default to
-    # `true` on minikube or Docker for Desktop, for even faster build and deploy cycles.
-    # `false` on other types of kubernetes clusters that require pushing the images.
-    # Skaffold defers to your ~/.docker/config for authentication information.
-    # If you're using Google Container Registry, make sure that you have gcloud and
-    # docker-credentials-helper-gcr configured correctly.
-    # skipPush: true
+  #   skipPush: true
+  #   useDockerCLI: false
+  #   useBuildkit: false
 
   # Docker artifacts can be built on Google Container Builder. The projectId then needs
   # to be provided and the currently logged user should be given permissions to trigger
   # new builds on GCB.
+  #
   #  googleCloudBuild:
   #   projectId: YOUR_PROJECT
 
   # Docker artifacts can be built on a Kubernetes cluster with Kaniko.
   # Sources will be sent to a GCS bucket whose name is provided.
   # Kaniko also needs access to a service account to push the final image.
   # See https://github.com/GoogleContainerTools/kaniko#running-kaniko-in-a-kubernetes-cluster
-  # Example
+  #
   # kaniko:
   #   gcsBucket: k8s-skaffold
   #   pullSecret: /a/secret/path/serviceaccount.json

pkg/skaffold/build/kaniko/run.go

@@ -56,6 +56,14 @@ func runKaniko(ctx context.Context, out io.Writer, artifact *v1alpha2.Artifact,
 	pods := client.CoreV1().Pods(cfg.Namespace)
 
 	imageDst := fmt.Sprintf("%s:%s", artifact.ImageName, initialTag)
+	args := []string{
+		fmt.Sprintf("--dockerfile=%s", dockerfilePath),
+		fmt.Sprintf("--context=gs://%s/%s", cfg.GCSBucket, tarName),
+		fmt.Sprintf("--destination=%s", imageDst),
+		fmt.Sprintf("-v=%s", logrus.GetLevel().String()),
+	}
+	args = append(args, docker.GetBuildArgs(artifact.DockerArtifact)...)
+
 	p, err := pods.Create(&v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			GenerateName: "kaniko",
@@ -68,36 +76,25 @@ func runKaniko(ctx context.Context, out io.Writer, artifact *v1alpha2.Artifact,
 					Name:            kanikoContainerName,
 					Image:           constants.DefaultKanikoImage,
 					ImagePullPolicy: v1.PullIfNotPresent,
-					Args: addBuildArgs([]string{
-						fmt.Sprintf("--dockerfile=%s", dockerfilePath),
-						fmt.Sprintf("--context=gs://%s/%s", cfg.GCSBucket, tarName),
-						fmt.Sprintf("--destination=%s", imageDst),
-						fmt.Sprintf("-v=%s", logrus.GetLevel().String()),
-					}, artifact),
-					VolumeMounts: []v1.VolumeMount{
-						{
-							Name:      constants.DefaultKanikoSecretName,
-							MountPath: "/secret",
-						},
-					},
-					Env: []v1.EnvVar{
-						{
-							Name:  "GOOGLE_APPLICATION_CREDENTIALS",
-							Value: "/secret/kaniko-secret",
-						},
-					},
+					Args:            args,
+					VolumeMounts: []v1.VolumeMount{{
+						Name:      constants.DefaultKanikoSecretName,
+						MountPath: "/secret",
+					}},
+					Env: []v1.EnvVar{{
+						Name:  "GOOGLE_APPLICATION_CREDENTIALS",
+						Value: "/secret/kaniko-secret",
+					}},
 				},
 			},
-			Volumes: []v1.Volume{
-				{
-					Name: constants.DefaultKanikoSecretName,
-					VolumeSource: v1.VolumeSource{
-						Secret: &v1.SecretVolumeSource{
-							SecretName: cfg.PullSecretName,
-						},
+			Volumes: []v1.Volume{{
+				Name: constants.DefaultKanikoSecretName,
+				VolumeSource: v1.VolumeSource{
+					Secret: &v1.SecretVolumeSource{
+						SecretName: cfg.PullSecretName,
 					},
 				},
-			},
+			}},
 			RestartPolicy: v1.RestartPolicyNever,
 		},
 	})
@@ -162,19 +159,3 @@ func gcsDelete(ctx context.Context, bucket, path string) error {
 
 	return c.Bucket(bucket).Object(path).Delete(ctx)
 }
-
-func addBuildArgs(args []string, artifact *v1alpha2.Artifact) []string {
-	if artifact.DockerArtifact == nil {
-		return args
-	}
-
-	if len(artifact.DockerArtifact.BuildArgs) == 0 {
-		return args
-	}
-
-	for k, v := range artifact.DockerArtifact.BuildArgs {
-		args = append(args, fmt.Sprintf("--build-arg=%s=%s", k, *v))
-	}
-
-	return args
-}

pkg/skaffold/build/local/docker.go

@@ -20,6 +20,8 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"os"
+	"os/exec"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
@@ -31,14 +33,38 @@ import (
 func (b *Builder) buildDocker(ctx context.Context, out io.Writer, workspace string, a *v1alpha2.DockerArtifact) (string, error) {
 	initialTag := util.RandomID()
 
-	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 {
-		return "", errors.Wrap(err, "running build")
+	if b.cfg.UseDockerCLI || b.cfg.UseBuildkit {
+		dockerfilePath, err := docker.NormalizeDockerfilePath(workspace, a.DockerfilePath)
+		if err != nil {
+			return "", errors.Wrap(err, "normalizing dockerfile path")
+		}
+
+		args := []string{"build", workspace, "--file", dockerfilePath, "-t", initialTag}
+		args = append(args, docker.GetBuildArgs(a)...)
+		for _, from := range a.CacheFrom {
+			args = append(args, "--cache-from", from)
+		}
+
+		cmd := exec.Command("docker", args...)
+		if b.cfg.UseBuildkit {
+			cmd.Env = append(os.Environ(), "DOCKER_BUILDKIT=1")
+		}
+		cmd.Stdout = out
+		cmd.Stderr = out
+
+		if err := util.RunCmd(cmd); err != nil {
+			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 {
+			return "", errors.Wrap(err, "running build")
+		}
 	}
 
 	return fmt.Sprintf("%s:latest", initialTag), nil

pkg/skaffold/build/local/local_test.go

@@ -183,6 +183,7 @@ func TestLocalRun(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
 			l := Builder{
+				cfg:          test.config,
 				api:          test.api,
 				localCluster: test.localCluster,
 			}

pkg/skaffold/build/local/types.go

@@ -29,6 +29,8 @@ import (
 
 // Builder uses the host docker daemon to build and tag the image.
 type Builder struct {
+	cfg *v1alpha2.LocalBuild
+
 	api          docker.APIClient
 	localCluster bool
 	pushImages   bool
@@ -54,6 +56,7 @@ func NewBuilder(cfg *v1alpha2.LocalBuild, kubeContext string) (*Builder, error)
 	}
 
 	return &Builder{
+		cfg:          cfg,
 		kubeContext:  kubeContext,
 		api:          api,
 		localCluster: localCluster,

pkg/skaffold/docker/context.go

@@ -19,12 +19,26 @@ package docker
 import (
 	"context"
 	"io"
+	"path/filepath"
+	"strings"
 
 	cstorage "cloud.google.com/go/storage"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/pkg/errors"
 )
 
+// NormalizeDockerfilePath returns the absolute path to the dockerfile.
+func NormalizeDockerfilePath(context, dockerfile string) (string, error) {
+	if filepath.IsAbs(dockerfile) {
+		return dockerfile, nil
+	}
+
+	if !strings.HasPrefix(dockerfile, context) {
+		dockerfile = filepath.Join(context, dockerfile)
+	}
+	return filepath.Abs(dockerfile)
+}
+
 func CreateDockerTarContext(buildArgs map[string]*string, w io.Writer, context, dockerfilePath string) error {
 	paths, err := GetDependencies(buildArgs, context, dockerfilePath)
 	if err != nil {

pkg/skaffold/docker/image.go

@@ -18,9 +18,12 @@ package docker
 
 import (
 	"context"
+	"fmt"
 	"io"
 	"net/http"
+	"sort"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/docker/docker/api/types"
 	"github.com/docker/docker/client"
 	"github.com/docker/docker/pkg/jsonmessage"
@@ -163,3 +166,27 @@ func RemoteDigest(identifier string) (string, error) {
 
 	return h.String(), nil
 }
+
+// GetBuildArgs gives the build args flags for docker build.
+func GetBuildArgs(a *v1alpha2.DockerArtifact) []string {
+	var args []string
+
+	var keys []string
+	for k := range a.BuildArgs {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
+
+	for _, k := range keys {
+		args = append(args, "--build-arg")
+
+		v := a.BuildArgs[k]
+		if v == nil {
+			args = append(args, k)
+		} else {
+			args = append(args, fmt.Sprintf("%s=%s", k, *v))
+		}
+	}
+
+	return args
+}

pkg/skaffold/docker/image_test.go

@@ -25,8 +25,11 @@ import (
 	"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"
 )
 
 func TestMain(m *testing.M) {
@@ -175,3 +178,20 @@ func TestDigest(t *testing.T) {
 		})
 	}
 }
+
+func TestGetBuildArgs(t *testing.T) {
+	artifact := &v1alpha2.DockerArtifact{
+		BuildArgs: map[string]*string{
+			"key1": util.StringPtr("value1"),
+			"key2": nil,
+		},
+	}
+
+	arg := GetBuildArgs(artifact)
+	expected := []string{"--build-arg", "key1=value1", "--build-arg", "key2"}
+
+	if diff := cmp.Diff(arg, expected); diff != "" {
+		t.Errorf("%T differ (-got, +want): %s", expected, diff)
+		return
+	}
+}

pkg/skaffold/docker/parse.go

@@ -168,9 +168,9 @@ func readDockerfile(workspace, absDockerfilePath string, buildArgs map[string]*s
 }
 
 func GetDependencies(buildArgs map[string]*string, workspace, dockerfilePath string) ([]string, error) {
-	absDockerfilePath := dockerfilePath
-	if !filepath.IsAbs(dockerfilePath) {
-		absDockerfilePath = filepath.Join(workspace, dockerfilePath)
+	absDockerfilePath, err := NormalizeDockerfilePath(workspace, dockerfilePath)
+	if err != nil {
+		return nil, errors.Wrap(err, "normalizing dockerfile path")
 	}
 
 	deps, err := readDockerfile(workspace, absDockerfilePath, buildArgs)

pkg/skaffold/schema/v1alpha2/config.go

@@ -79,7 +79,9 @@ type BuildType struct {
 // LocalBuild contains the fields needed to do a build on the local docker daemon
 // and optionally push to a repository.
 type LocalBuild struct {
-	SkipPush *bool `yaml:"skipPush"`
+	SkipPush     *bool `yaml:"skipPush"`
+	UseDockerCLI bool  `yaml:"useDockerCLI"`
+	UseBuildkit  bool  `yaml:"useBuildkit"`
 }
 
 // GoogleCloudBuild contains the fields needed to do a remote build on

pkg/skaffold/util/util.go

@@ -127,6 +127,12 @@ func BoolPtr(b bool) *bool {
 	return &o
 }
 
+// StringPtr returns a pointer to a string
+func StringPtr(s string) *string {
+	o := s
+	return &o
+}
+
 func ReadConfiguration(filename string) ([]byte, error) {
 	switch {
 	case filename == "":