Skip to content

Commit

Permalink
Support recent maven, for Jib on GCB
Browse files Browse the repository at this point in the history 
Fix GoogleContainerTools#2546

Also took the opportunity to improve tests a bit.

Should give builds that are slightly faster since
they will by default use a builder image that's
already cached on GCB.

Signed-off-by: David Gageot <[email protected]>
  • Loading branch information
@dgageot
dgageot committed Jul 25, 2019
1 parent 10e9795 commit 710cce5
Show file tree
Hide file tree
Showing 10 changed files with 182 additions and 117 deletions.
17 changes: 11 additions & 6 deletions pkg/skaffold/build/gcb/cloud_build.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/gcp"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/sources"
Expand Down Expand Up @@ -100,7 +101,7 @@ func (b *Builder) buildArtifactWithCloudBuild(ctx context.Context, out io.Writer
return "", errors.Wrap(err, "uploading source tarball")
}

call := cbclient.Projects.Builds.Create(projectID, desc)
call := cbclient.Projects.Builds.Create(projectID, &desc)
op, err := call.Context(ctx).Do()
if err != nil {
return "", errors.Wrap(err, "could not create build")
Expand Down Expand Up @@ -138,7 +139,7 @@ watch:
switch cb.Status {
case StatusQueued, StatusWorking, StatusUnknown:
case StatusSuccess:
digest, err = getDigest(cb)
digest, err = getDigest(cb, tag)
if err != nil {
return "", errors.Wrap(err, "getting image id from finished build")
}
Expand Down Expand Up @@ -174,11 +175,15 @@ func getBuildID(op *cloudbuild.Operation) (string, error) {
return buildMeta.Build.Id, nil
}

func getDigest(b *cloudbuild.Build) (string, error) {
if b.Results == nil || len(b.Results.Images) == 0 {
return "", errors.New("build failed")
func getDigest(b *cloudbuild.Build, defaultToTag string) (string, error) {
if b.Results != nil && len(b.Results.Images) == 1 {
return b.Results.Images[0].Digest, nil
}
return b.Results.Images[0].Digest, nil

// The build steps pushed the image directly like when we use Jib.
// Retrieve the digest for that tag.
// TODO(dgageot): I don't think GCB could push to an insecure registry.
return docker.RemoteDigest(defaultToTag, nil)
}

func (b *Builder) getLogs(ctx context.Context, offset int64, bucket, objectName string) (io.ReadCloser, error) {
Expand Down
54 changes: 35 additions & 19 deletions pkg/skaffold/build/gcb/desc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,48 +24,64 @@ import (
cloudbuild "google.golang.org/api/cloudbuild/v1"
)

func (b *Builder) buildDescription(artifact *latest.Artifact, tag, bucket, object string) (*cloudbuild.Build, error) {
tags := []string{tag}

steps, err := b.buildSteps(artifact, tag)
if err != nil {
return nil, err
}

return &cloudbuild.Build{
func (b *Builder) baseDescription(bucket, object string) cloudbuild.Build {
return cloudbuild.Build{
LogsBucket: bucket,
Source: &cloudbuild.Source{
StorageSource: &cloudbuild.StorageSource{
Bucket: bucket,
Object: object,
},
},
Steps: steps,
Images: tags,
Options: &cloudbuild.BuildOptions{
DiskSizeGb: b.DiskSizeGb,
MachineType: b.MachineType,
},
Timeout: b.Timeout,
}, nil
}
}

func (b *Builder) buildSteps(artifact *latest.Artifact, tag string) ([]*cloudbuild.BuildStep, error) {
func (b *Builder) buildDescription(artifact *latest.Artifact, tag, bucket, object string) (cloudbuild.Build, error) {
base := b.baseDescription(bucket, object)

switch {
case artifact.DockerArtifact != nil:
return b.dockerBuildSteps(artifact.DockerArtifact, tag)
return b.dockerBuildDescription(artifact.DockerArtifact, tag, base)

case artifact.BazelArtifact != nil:
return nil, errors.New("skaffold can't build a bazel artifact with Google Cloud Build")
return cloudbuild.Build{}, errors.New("skaffold can't build a bazel artifact with Google Cloud Build")

// TODO: build multiple tagged images with jib in GCB (priyawadhwa@)
// TODO: build multiple tagged images with jib in GCB (priyawadhwa@)
case artifact.JibMavenArtifact != nil:
return b.jibMavenBuildSteps(artifact.JibMavenArtifact, tag), nil
return b.jibMavenBuildDescription(artifact.JibMavenArtifact, tag, base), nil

case artifact.JibGradleArtifact != nil:
return b.jibGradleBuildSteps(artifact.JibGradleArtifact, tag), nil
return b.jibGradleBuildDescription(artifact.JibGradleArtifact, tag, base), nil

default:
return nil, fmt.Errorf("undefined artifact type: %+v", artifact.ArtifactType)
return cloudbuild.Build{}, fmt.Errorf("undefined artifact type: %+v", artifact.ArtifactType)
}

// steps, err := b.buildSteps(artifact, tag)
// if err != nil {
// return nil, err
// }

// return &cloudbuild.Build{
// LogsBucket: bucket,
// Source: &cloudbuild.Source{
// StorageSource: &cloudbuild.StorageSource{
// Bucket: bucket,
// Object: object,
// },
// },
// Steps: steps,
// // Images: []string{tag},
// Options: &cloudbuild.BuildOptions{
// DiskSizeGb: b.DiskSizeGb,
// MachineType: b.MachineType,
// },
// Timeout: b.Timeout,
// Tags: []string{"skaffold"},
// }, nil
}
54 changes: 36 additions & 18 deletions pkg/skaffold/build/gcb/desc_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,32 +19,50 @@ package gcb
import (
"testing"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestBuildBazelDescriptionFail(t *testing.T) {
artifact := &latest.Artifact{
ArtifactType: latest.ArtifactType{
BazelArtifact: &latest.BazelArtifact{},
func TestBuildDescriptionFail(t *testing.T) {
tests := []struct {
description string
artifact *latest.Artifact
}{
{
description: "bazel",
artifact: &latest.Artifact{
ArtifactType: latest.ArtifactType{
BazelArtifact: &latest.BazelArtifact{},
},
},
},
{
description: "unknown",
artifact: &latest.Artifact{},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
builder := newBuilder(latest.GoogleCloudBuild{})

builder := Builder{
GoogleCloudBuild: &latest.GoogleCloudBuild{},
}
_, err := builder.buildDescription(artifact, "tag", "bucket", "object")

testutil.CheckError(t, true, err)
}
_, err := builder.buildDescription(test.artifact, "tag", "bucket", "object")

func TestBuildUnknownDescriptionFail(t *testing.T) {
artifact := &latest.Artifact{}

builder := Builder{
GoogleCloudBuild: &latest.GoogleCloudBuild{},
t.CheckError(true, err)
})
}
_, err := builder.buildDescription(artifact, "tag", "bucket", "object")
}

testutil.CheckError(t, true, err)
func newBuilder(gcb latest.GoogleCloudBuild) *Builder {
return NewBuilder(&runcontext.RunContext{
Opts: &config.SkaffoldOptions{},
Cfg: &latest.Pipeline{
Build: latest.BuildConfig{
BuildType: latest.BuildType{
GoogleCloudBuild: &gcb,
},
},
},
})
}
13 changes: 9 additions & 4 deletions pkg/skaffold/build/gcb/docker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
cloudbuild "google.golang.org/api/cloudbuild/v1"
)

func (b *Builder) dockerBuildSteps(artifact *latest.DockerArtifact, tag string) ([]*cloudbuild.BuildStep, error) {
func (b *Builder) dockerBuildDescription(artifact *latest.DockerArtifact, tag string, base cloudbuild.Build) (cloudbuild.Build, error) {
var steps []*cloudbuild.BuildStep

for _, cacheFrom := range artifact.CacheFrom {
Expand All @@ -40,13 +40,18 @@ func (b *Builder) dockerBuildSteps(artifact *latest.DockerArtifact, tag string)

ba, err := docker.GetBuildArgs(artifact)
if err != nil {
return nil, errors.Wrap(err, "getting docker build args")
return cloudbuild.Build{}, errors.Wrap(err, "getting docker build args")
}
args = append(args, ba...)
args = append(args, ".")

return append(steps, &cloudbuild.BuildStep{
steps = append(steps, &cloudbuild.BuildStep{
Name: b.DockerImage,
Args: args,
}), nil
})

base.Steps = steps
base.Images = []string{tag}

return base, nil
}
28 changes: 12 additions & 16 deletions pkg/skaffold/build/gcb/docker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,12 @@ func TestDockerBuildDescription(t *testing.T) {
},
}

builder := Builder{
GoogleCloudBuild: &latest.GoogleCloudBuild{
DockerImage: "docker/docker",
DiskSizeGb: 100,
MachineType: "n1-standard-1",
Timeout: "10m",
},
}
builder := newBuilder(latest.GoogleCloudBuild{
DockerImage: "docker/docker",
DiskSizeGb: 100,
MachineType: "n1-standard-1",
Timeout: "10m",
})
desc, err := builder.buildDescription(artifact, "nginx", "bucket", "object")

expected := cloudbuild.Build{
Expand All @@ -68,7 +66,7 @@ func TestDockerBuildDescription(t *testing.T) {
Timeout: "10m",
}

testutil.CheckErrorAndDeepEqual(t, false, err, expected, *desc)
testutil.CheckErrorAndDeepEqual(t, false, err, expected, desc)
}

func TestPullCacheFrom(t *testing.T) {
Expand All @@ -77,12 +75,10 @@ func TestPullCacheFrom(t *testing.T) {
CacheFrom: []string{"from/image1", "from/image2"},
}

builder := Builder{
GoogleCloudBuild: &latest.GoogleCloudBuild{
DockerImage: "docker/docker",
},
}
steps, err := builder.dockerBuildSteps(artifact, "nginx2")
builder := newBuilder(latest.GoogleCloudBuild{
DockerImage: "docker/docker",
})
desc, err := builder.dockerBuildDescription(artifact, "nginx2", cloudbuild.Build{})

expected := []*cloudbuild.BuildStep{{
Name: "docker/docker",
Expand All @@ -97,5 +93,5 @@ func TestPullCacheFrom(t *testing.T) {
Args: []string{"build", "--tag", "nginx2", "-f", "Dockerfile", "--cache-from", "from/image1", "--cache-from", "from/image2", "."},
}}

testutil.CheckErrorAndDeepEqual(t, false, err, expected, steps)
testutil.CheckErrorAndDeepEqual(t, false, err, expected, desc.Steps)
}
19 changes: 12 additions & 7 deletions pkg/skaffold/build/gcb/jib.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,22 @@ import (
cloudbuild "google.golang.org/api/cloudbuild/v1"
)

// TODO(dgageot): check that `package` is bound to `jib:build`
func (b *Builder) jibMavenBuildSteps(artifact *latest.JibMavenArtifact, tag string) []*cloudbuild.BuildStep {
return []*cloudbuild.BuildStep{{
func (b *Builder) jibMavenBuildDescription(artifact *latest.JibMavenArtifact, tag string, base cloudbuild.Build) cloudbuild.Build {
base.Steps = []*cloudbuild.BuildStep{{
Name: b.MavenImage,
Args: jib.GenerateMavenArgs("dockerBuild", tag, artifact, b.skipTests),
Args: fixHome(jib.GenerateMavenArgs("build", tag, artifact, b.skipTests)),
}}
return base
}

func (b *Builder) jibGradleBuildSteps(artifact *latest.JibGradleArtifact, tag string) []*cloudbuild.BuildStep {
return []*cloudbuild.BuildStep{{
func (b *Builder) jibGradleBuildDescription(artifact *latest.JibGradleArtifact, tag string, base cloudbuild.Build) cloudbuild.Build {
base.Steps = []*cloudbuild.BuildStep{{
Name: b.GradleImage,
Args: jib.GenerateGradleArgs("jibDockerBuild", tag, artifact, b.skipTests),
Args: fixHome(jib.GenerateGradleArgs("jib", tag, artifact, b.skipTests)),
}}
return base
}

func fixHome(args []string) []string {
return append([]string{"-Duser.home=/builder/home"}, args...)
}
Loading

0 comments on commit 710cce5

Please sign in to comment.