Improve Cloud Build builder

deploy/cloudbuild-release.yaml

@@ -1,4 +1,4 @@
-# using default substitutions, provided by Google Container Builder
+# using default substitutions, provided by Google Cloud Build
 # see: https://cloud.google.com/container-builder/docs/configuring-builds/substitute-variable-values#using_default_substitutions
 steps:
 

deploy/cloudbuild.yaml

@@ -1,4 +1,4 @@
-# using default substitutions, provided by Google Container Builder
+# using default substitutions, provided by Google Cloud Build
 # see: https://cloud.google.com/container-builder/docs/configuring-builds/substitute-variable-values#using_default_substitutions
 steps:
 

docs/quickstart-gke.adoc

@@ -39,7 +39,7 @@ sed -i -e s#k8s-skaffold#${GOOGLE_CLOUD_PROJECT}#g skaffold.yaml
 sed -i -e s#k8s-skaffold#${GOOGLE_CLOUD_PROJECT}#g k8s-pod.yaml
 
 . Take a look at the contents of `skaffold.yaml`.
-You'll notice a profile named `gcb` that will be using Google Container Builder to build
+You'll notice a profile named `gcb` that will be using Google Cloud Build to build
    and push your image.
 The deploy section is configured to use kubectl to apply the Kubernetes manifests.
 [source,shell]

examples/annotated-skaffold.yaml

@@ -79,12 +79,19 @@ build:
   #   useDockerCLI: false
   #   useBuildkit: false
 
-  # Docker artifacts can be built on Google Container Builder. The projectId then needs
+  # Docker artifacts can be built on Google Cloud Build. The projectId then needs
   # to be provided and the currently logged user should be given permissions to trigger
-  # new builds on GCB.
-  #
+  # new builds on Cloud Build.
+  # If the projectId is not provided, Skaffold will try to guess it from the image name.
+  # For eg. If the artifact image name is gcr.io/myproject/image, then Skaffold will use
+  # the `myproject` GCP project.
+  # All the other parameters are also optional. The default values are listed here:
   #  googleCloudBuild:
   #   projectId: YOUR_PROJECT
+  #   diskSizeGb: 200
+  #   machineType: "N1_HIGHCPU_8"|"N1_HIGHCPU_32"
+  #   timeout: 10000S
+  #   dockerImage: gcr.io/cloud-builders/docker
 
   # Docker artifacts can be built on a Kubernetes cluster with Kaniko.
   # Sources will be sent to a GCS bucket whose name is provided.
@@ -162,8 +169,5 @@ profiles:
   - name: gcb
     build:
       googleCloudBuild:
-        # Google Cloud Build's project id
         projectId: k8s-skaffold
-        # diskSizeGb: 200
-        # machineType: "N1_HIGHCPU_8"|"N1_HIGHCPU_32"
 

pkg/skaffold/build/gcb/container_builder.go → pkg/skaffold/build/gcb/cloud_build.go

@@ -40,7 +40,7 @@ import (
 	"google.golang.org/api/iterator"
 )
 
-// Build builds a list of artifacts with GCB.
+// Build builds a list of artifacts with Google Cloud Build.
 func (b *Builder) Build(ctx context.Context, out io.Writer, tagger tag.Tagger, artifacts []*v1alpha2.Artifact) ([]build.Artifact, error) {
 	return build.InParallel(ctx, out, tagger, artifacts, b.buildArtifact)
 }
@@ -63,15 +63,6 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, tagger tag.T
 	}
 	defer c.Close()
 
-	// need to format build args as strings to pass to container builder docker
-	var buildArgs []string
-	for k, v := range artifact.DockerArtifact.BuildArgs {
-		if v != nil {
-			buildArgs = append(buildArgs, []string{"--build-arg", fmt.Sprintf("%s=%s", k, *v)}...)
-		}
-	}
-	logrus.Debugf("Build args: %s", buildArgs)
-
 	projectID, err := b.guessProjectID(artifact)
 	if err != nil {
 		return "", errors.Wrap(err, "getting projectID")
@@ -92,28 +83,8 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, tagger tag.T
 		return "", errors.Wrap(err, "uploading source tarball")
 	}
 
-	args := append([]string{"build", "--tag", artifact.ImageName, "-f", artifact.DockerArtifact.DockerfilePath}, buildArgs...)
-	args = append(args, ".")
-	call := cbclient.Projects.Builds.Create(projectID, &cloudbuild.Build{
-		LogsBucket: cbBucket,
-		Source: &cloudbuild.Source{
-			StorageSource: &cloudbuild.StorageSource{
-				Bucket: cbBucket,
-				Object: buildObject,
-			},
-		},
-		Steps: []*cloudbuild.BuildStep{
-			{
-				Name: "gcr.io/cloud-builders/docker",
-				Args: args,
-			},
-		},
-		Images: []string{artifact.ImageName},
-		Options: &cloudbuild.BuildOptions{
-			DiskSizeGb:  b.DiskSizeGb,
-			MachineType: b.MachineType,
-		},
-	})
+	desc := b.buildDescription(artifact, cbBucket, buildObject)
+	call := cbclient.Projects.Builds.Create(projectID, desc)
 	op, err := call.Context(ctx).Do()
 	if err != nil {
 		return "", errors.Wrap(err, "could not create build")
@@ -187,6 +158,32 @@ watch:
 	return newTag, nil
 }
 
+func (b *Builder) buildDescription(artifact *v1alpha2.Artifact, bucket, object string) *cloudbuild.Build {
+	args := append([]string{"build", "--tag", artifact.ImageName, "-f", artifact.DockerArtifact.DockerfilePath})
+	args = append(args, docker.GetBuildArgs(artifact.DockerArtifact)...)
+	args = append(args, ".")
+
+	return &cloudbuild.Build{
+		LogsBucket: bucket,
+		Source: &cloudbuild.Source{
+			StorageSource: &cloudbuild.StorageSource{
+				Bucket: bucket,
+				Object: object,
+			},
+		},
+		Steps: []*cloudbuild.BuildStep{{
+			Name: b.DockerImage,
+			Args: args,
+		}},
+		Images: []string{artifact.ImageName},
+		Options: &cloudbuild.BuildOptions{
+			DiskSizeGb:  b.DiskSizeGb,
+			MachineType: b.MachineType,
+		},
+		Timeout: b.Timeout,
+	}
+}
+
 func getBuildID(op *cloudbuild.Operation) (string, error) {
 	if op.Metadata == nil {
 		return "", errors.New("missing Metadata in operation")
@@ -219,6 +216,7 @@ func (b *Builder) getLogs(ctx context.Context, offset int64, bucket, objectName
 	if err != nil {
 		if gerr, ok := err.(*googleapi.Error); ok {
 			switch gerr.Code {
+			// case http.
 			case 404, 416, 429, 503:
 				logrus.Debugf("Status Code: %d, %s", gerr.Code, gerr.Body)
 				return nil, nil

pkg/skaffold/build/gcb/cloud_build_test.go

@@ -0,0 +1,73 @@
+/*
+Copyright 2018 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package gcb
+
+import (
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
+	"github.com/GoogleContainerTools/skaffold/testutil"
+	cloudbuild "google.golang.org/api/cloudbuild/v1"
+)
+
+func TestBuildDescription(t *testing.T) {
+	artifact := &v1alpha2.Artifact{
+		ImageName: "nginx",
+		ArtifactType: v1alpha2.ArtifactType{
+			DockerArtifact: &v1alpha2.DockerArtifact{
+				DockerfilePath: "Dockerfile",
+				BuildArgs: map[string]*string{
+					"arg1": util.StringPtr("value1"),
+					"arg2": nil,
+				},
+			},
+		},
+	}
+
+	builder := Builder{
+		GoogleCloudBuild: &v1alpha2.GoogleCloudBuild{
+			DockerImage: "docker/docker",
+			DiskSizeGb:  100,
+			MachineType: "n1-standard-1",
+			Timeout:     "10m",
+		},
+	}
+	desc := builder.buildDescription(artifact, "bucket", "object")
+
+	expected := cloudbuild.Build{
+		LogsBucket: "bucket",
+		Source: &cloudbuild.Source{
+			StorageSource: &cloudbuild.StorageSource{
+				Bucket: "bucket",
+				Object: "object",
+			},
+		},
+		Steps: []*cloudbuild.BuildStep{{
+			Name: "docker/docker",
+			Args: []string{"build", "--tag", "nginx", "-f", "Dockerfile", "--build-arg", "arg1=value1", "--build-arg", "arg2", "."},
+		}},
+		Images: []string{artifact.ImageName},
+		Options: &cloudbuild.BuildOptions{
+			DiskSizeGb:  100,
+			MachineType: "n1-standard-1",
+		},
+		Timeout: "10m",
+	}
+
+	testutil.CheckErrorAndDeepEqual(t, false, nil, expected, *desc)
+}

pkg/skaffold/build/gcb/types.go

@@ -52,21 +52,21 @@ const (
 	RetryDelay = 1 * time.Second
 )
 
-// Builder builds artifacts with GCB.
+// Builder builds artifacts with Google Cloud Build.
 type Builder struct {
 	*v1alpha2.GoogleCloudBuild
 }
 
-// NewBuilder creates a new Builder that builds artifacts with GCB.
+// NewBuilder creates a new Builder that builds artifacts with Google Cloud Build.
 func NewBuilder(cfg *v1alpha2.GoogleCloudBuild) *Builder {
 	return &Builder{
 		GoogleCloudBuild: cfg,
 	}
 }
 
-// Labels are labels specific to GCB builder.
+// Labels are labels specific to Google Cloud Build.
 func (b *Builder) Labels() map[string]string {
 	return map[string]string{
-		constants.Labels.Builder: "google-cloud-builder",
+		constants.Labels.Builder: "google-cloud-build",
 	}
 }

pkg/skaffold/config/config_test.go

@@ -120,7 +120,7 @@ func TestParseConfig(t *testing.T) {
 			description: "Complete config",
 			config:      completeConfig,
 			expected: config(
-				withGCBBuild("ID",
+				withGoogleCloudBuild("ID",
 					withTagPolicy(v1alpha2.TagPolicy{ShaTagger: &v1alpha2.ShaTagger{}}),
 					withDockerArtifact("image1", "./examples/app1", "Dockerfile.dev"),
 					withBazelArtifact("image2", "./examples/app2", "//:example.tar"),
@@ -179,9 +179,12 @@ func withLocalBuild(ops ...func(*v1alpha2.BuildConfig)) func(*SkaffoldConfig) {
 	}
 }
 
-func withGCBBuild(id string, ops ...func(*v1alpha2.BuildConfig)) func(*SkaffoldConfig) {
+func withGoogleCloudBuild(id string, ops ...func(*v1alpha2.BuildConfig)) func(*SkaffoldConfig) {
 	return func(cfg *SkaffoldConfig) {
-		b := v1alpha2.BuildConfig{BuildType: v1alpha2.BuildType{GoogleCloudBuild: &v1alpha2.GoogleCloudBuild{ProjectID: id}}}
+		b := v1alpha2.BuildConfig{BuildType: v1alpha2.BuildType{GoogleCloudBuild: &v1alpha2.GoogleCloudBuild{
+			ProjectID:   id,
+			DockerImage: "gcr.io/cloud-builders/docker",
+		}}}
 		for _, op := range ops {
 			op(&b)
 		}

pkg/skaffold/config/profile_test.go

@@ -75,7 +75,9 @@ func TestApplyProfiles(t *testing.T) {
 						},
 					},
 					BuildType: v1alpha2.BuildType{
-						GoogleCloudBuild: &v1alpha2.GoogleCloudBuild{},
+						GoogleCloudBuild: &v1alpha2.GoogleCloudBuild{
+							DockerImage: "gcr.io/cloud-builders/docker",
+						},
 					},
 					TagPolicy: v1alpha2.TagPolicy{
 						GitTagger: &v1alpha2.GitTagger{},

pkg/skaffold/constants/constants.go

@@ -51,6 +51,8 @@ const (
 	DefaultKanikoTimeout    = "20m"
 
 	UpdateCheckEnvironmentVariable = "SKAFFOLD_UPDATE_CHECK"
+
+	DefaultCloudBuildDockerImage = "gcr.io/cloud-builders/docker"
 )
 
 var DefaultKubectlManifests = []string{"k8s/*.yaml"}

pkg/skaffold/schema/v1alpha2/config.go

@@ -85,11 +85,13 @@ type LocalBuild struct {
 }
 
 // GoogleCloudBuild contains the fields needed to do a remote build on
-// Google Container Builder.
+// Google Cloud Build.
 type GoogleCloudBuild struct {
 	ProjectID   string `yaml:"projectId"`
 	DiskSizeGb  int64  `yaml:"diskSizeGb,omitempty"`
 	MachineType string `yaml:"machineType,omitempty"`
+	Timeout     string `yaml:"timeout,omitempty"`
+	DockerImage string `yaml:"dockerImage,omitempty"`
 }
 
 // KanikoBuild contains the fields needed to do a on-cluster build using

pkg/skaffold/schema/v1alpha2/defaults.go

@@ -28,6 +28,7 @@ import (
 
 func (c *SkaffoldConfig) setDefaultValues() error {
 	c.defaultToLocalBuild()
+	c.setDefaultCloudBuildDockerImage()
 	c.setDefaultTagger()
 	c.setDefaultKustomizePath()
 	c.setDefaultKubectlManifests()
@@ -57,6 +58,17 @@ func (c *SkaffoldConfig) defaultToLocalBuild() {
 	c.Build.BuildType.LocalBuild = &LocalBuild{}
 }
 
+func (c *SkaffoldConfig) setDefaultCloudBuildDockerImage() {
+	cloudBuild := c.Build.BuildType.GoogleCloudBuild
+	if cloudBuild == nil {
+		return
+	}
+
+	if cloudBuild.DockerImage == "" {
+		cloudBuild.DockerImage = constants.DefaultCloudBuildDockerImage
+	}
+}
+
 func (c *SkaffoldConfig) setDefaultTagger() {
 	if c.Build.TagPolicy != (TagPolicy{}) {
 		return