From 30b9e6f4052e72d3642c5a9d6480baf1652c415e Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 11 Jan 2019 15:09:43 +0100 Subject: [PATCH] Clear error message for unsupported artifact on GCB Also prepare the support for other artifacts types Signed-off-by: David Gageot --- pkg/skaffold/build/gcb/cloud_build.go | 6 +- pkg/skaffold/build/gcb/desc.go | 44 +++++++----- pkg/skaffold/build/gcb/desc_test.go | 80 ++++++--------------- pkg/skaffold/build/gcb/docker.go | 43 +++++++++++ pkg/skaffold/build/gcb/docker_test.go | 100 ++++++++++++++++++++++++++ 5 files changed, 196 insertions(+), 77 deletions(-) create mode 100644 pkg/skaffold/build/gcb/docker.go create mode 100644 pkg/skaffold/build/gcb/docker_test.go diff --git a/pkg/skaffold/build/gcb/cloud_build.go b/pkg/skaffold/build/gcb/cloud_build.go index 7ba5649e6a5..f4e27e29c25 100644 --- a/pkg/skaffold/build/gcb/cloud_build.go +++ b/pkg/skaffold/build/gcb/cloud_build.go @@ -85,12 +85,16 @@ func (b *Builder) buildArtifactWithCloudBuild(ctx context.Context, out io.Writer return "", errors.Wrap(err, "checking bucket is in correct project") } + desc, err := b.buildDescription(artifact, cbBucket, buildObject) + if err != nil { + return "", errors.Wrap(err, "could not create build description") + } + color.Default.Fprintf(out, "Pushing code to gs://%s/%s\n", cbBucket, buildObject) if err := docker.UploadContextToGCS(ctx, artifact.Workspace, artifact.DockerArtifact, cbBucket, buildObject); err != nil { return "", errors.Wrap(err, "uploading source tarball") } - desc := b.buildDescription(artifact, cbBucket, buildObject) call := cbclient.Projects.Builds.Create(projectID, desc) op, err := call.Context(ctx).Do() if err != nil { diff --git a/pkg/skaffold/build/gcb/desc.go b/pkg/skaffold/build/gcb/desc.go index 4c62fbd708d..1858ed4177d 100644 --- a/pkg/skaffold/build/gcb/desc.go +++ b/pkg/skaffold/build/gcb/desc.go @@ -17,30 +17,19 @@ limitations under the License. package gcb import ( - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" + "errors" + "fmt" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" cloudbuild "google.golang.org/api/cloudbuild/v1" ) -func (b *Builder) buildDescription(artifact *latest.Artifact, bucket, object string) *cloudbuild.Build { - var steps []*cloudbuild.BuildStep - - for _, cacheFrom := range artifact.DockerArtifact.CacheFrom { - steps = append(steps, &cloudbuild.BuildStep{ - Name: b.DockerImage, - Args: []string{"pull", cacheFrom}, - }) +func (b *Builder) buildDescription(artifact *latest.Artifact, bucket, object string) (*cloudbuild.Build, error) { + steps, err := b.buildSteps(artifact) + if err != nil { + return nil, err } - args := append([]string{"build", "--tag", artifact.ImageName, "-f", artifact.DockerArtifact.DockerfilePath}) - args = append(args, docker.GetBuildArgs(artifact.DockerArtifact)...) - args = append(args, ".") - - steps = append(steps, &cloudbuild.BuildStep{ - Name: b.DockerImage, - Args: args, - }) - return &cloudbuild.Build{ LogsBucket: bucket, Source: &cloudbuild.Source{ @@ -56,5 +45,24 @@ func (b *Builder) buildDescription(artifact *latest.Artifact, bucket, object str MachineType: b.MachineType, }, Timeout: b.Timeout, + }, nil +} + +func (b *Builder) buildSteps(artifact *latest.Artifact) ([]*cloudbuild.BuildStep, error) { + switch { + case artifact.DockerArtifact != nil: + return b.dockerBuildSteps(artifact.ImageName, artifact.DockerArtifact), nil + + case artifact.BazelArtifact != nil: + return nil, errors.New("skaffold can't build a bazel artifact with Google Cloud Build") + + case artifact.JibMavenArtifact != nil: + return nil, errors.New("skaffold can't build a jib maven artifact with Google Cloud Build") + + case artifact.JibGradleArtifact != nil: + return nil, errors.New("skaffold can't build a jib gradle artifact with Google Cloud Build") + + default: + return nil, fmt.Errorf("undefined artifact type: %+v", artifact.ArtifactType) } } diff --git a/pkg/skaffold/build/gcb/desc_test.go b/pkg/skaffold/build/gcb/desc_test.go index 6f5a97f8491..5b6e4064613 100644 --- a/pkg/skaffold/build/gcb/desc_test.go +++ b/pkg/skaffold/build/gcb/desc_test.go @@ -20,86 +20,50 @@ import ( "testing" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/testutil" - cloudbuild "google.golang.org/api/cloudbuild/v1" ) -func TestBuildDescription(t *testing.T) { +func TestBuildJibMavenDescriptionFail(t *testing.T) { artifact := &latest.Artifact{ - ImageName: "nginx", ArtifactType: latest.ArtifactType{ - DockerArtifact: &latest.DockerArtifact{ - DockerfilePath: "Dockerfile", - BuildArgs: map[string]*string{ - "arg1": util.StringPtr("value1"), - "arg2": nil, - }, - }, + JibMavenArtifact: &latest.JibMavenArtifact{}, }, } builder := Builder{ - GoogleCloudBuild: &latest.GoogleCloudBuild{ - DockerImage: "docker/docker", - DiskSizeGb: 100, - MachineType: "n1-standard-1", - Timeout: "10m", - }, + GoogleCloudBuild: &latest.GoogleCloudBuild{}, } - desc := builder.buildDescription(artifact, "bucket", "object") + _, err := 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", + testutil.CheckError(t, true, err) +} + +func TestBuildJibGradleDescriptionFail(t *testing.T) { + artifact := &latest.Artifact{ + ArtifactType: latest.ArtifactType{ + JibGradleArtifact: &latest.JibGradleArtifact{}, }, - Timeout: "10m", } - testutil.CheckDeepEqual(t, expected, *desc) + builder := Builder{ + GoogleCloudBuild: &latest.GoogleCloudBuild{}, + } + _, err := builder.buildDescription(artifact, "bucket", "object") + + testutil.CheckError(t, true, err) } -func TestPullCacheFrom(t *testing.T) { +func TestBuildBazelDescriptionFail(t *testing.T) { artifact := &latest.Artifact{ - ImageName: "nginx", ArtifactType: latest.ArtifactType{ - DockerArtifact: &latest.DockerArtifact{ - DockerfilePath: "Dockerfile", - CacheFrom: []string{"from/image1", "from/image2"}, - }, + BazelArtifact: &latest.BazelArtifact{}, }, } builder := Builder{ - GoogleCloudBuild: &latest.GoogleCloudBuild{ - DockerImage: "docker/docker", - }, + GoogleCloudBuild: &latest.GoogleCloudBuild{}, } - desc := builder.buildDescription(artifact, "bucket", "object") - - expected := []*cloudbuild.BuildStep{{ - Name: "docker/docker", - Args: []string{"pull", "from/image1"}, - }, { - Name: "docker/docker", - Args: []string{"pull", "from/image2"}, - }, { - Name: "docker/docker", - Args: []string{"build", "--tag", "nginx", "-f", "Dockerfile", "--cache-from", "from/image1", "--cache-from", "from/image2", "."}, - }} + _, err := builder.buildDescription(artifact, "bucket", "object") - testutil.CheckDeepEqual(t, expected, desc.Steps) + testutil.CheckError(t, true, err) } diff --git a/pkg/skaffold/build/gcb/docker.go b/pkg/skaffold/build/gcb/docker.go new file mode 100644 index 00000000000..ab75015ec4b --- /dev/null +++ b/pkg/skaffold/build/gcb/docker.go @@ -0,0 +1,43 @@ +/* +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 ( + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" + cloudbuild "google.golang.org/api/cloudbuild/v1" +) + +func (b *Builder) dockerBuildSteps(imageName string, artifact *latest.DockerArtifact) []*cloudbuild.BuildStep { + var steps []*cloudbuild.BuildStep + + for _, cacheFrom := range artifact.CacheFrom { + steps = append(steps, &cloudbuild.BuildStep{ + Name: b.DockerImage, + Args: []string{"pull", cacheFrom}, + }) + } + + args := append([]string{"build", "--tag", imageName, "-f", artifact.DockerfilePath}) + args = append(args, docker.GetBuildArgs(artifact)...) + args = append(args, ".") + + return append(steps, &cloudbuild.BuildStep{ + Name: b.DockerImage, + Args: args, + }) +} diff --git a/pkg/skaffold/build/gcb/docker_test.go b/pkg/skaffold/build/gcb/docker_test.go new file mode 100644 index 00000000000..8f6c2e3675a --- /dev/null +++ b/pkg/skaffold/build/gcb/docker_test.go @@ -0,0 +1,100 @@ +/* +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/latest" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" + "github.com/GoogleContainerTools/skaffold/testutil" + cloudbuild "google.golang.org/api/cloudbuild/v1" +) + +func TestDockerBuildDescription(t *testing.T) { + artifact := &latest.Artifact{ + ImageName: "nginx", + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{ + DockerfilePath: "Dockerfile", + BuildArgs: map[string]*string{ + "arg1": util.StringPtr("value1"), + "arg2": nil, + }, + }, + }, + } + + builder := Builder{ + GoogleCloudBuild: &latest.GoogleCloudBuild{ + DockerImage: "docker/docker", + DiskSizeGb: 100, + MachineType: "n1-standard-1", + Timeout: "10m", + }, + } + desc, err := 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, err, expected, *desc) +} + +func TestPullCacheFrom(t *testing.T) { + artifact := &latest.DockerArtifact{ + DockerfilePath: "Dockerfile", + CacheFrom: []string{"from/image1", "from/image2"}, + } + + builder := Builder{ + GoogleCloudBuild: &latest.GoogleCloudBuild{ + DockerImage: "docker/docker", + }, + } + steps := builder.dockerBuildSteps("nginx2", artifact) + + expected := []*cloudbuild.BuildStep{{ + Name: "docker/docker", + Args: []string{"pull", "from/image1"}, + }, { + Name: "docker/docker", + Args: []string{"pull", "from/image2"}, + }, { + Name: "docker/docker", + Args: []string{"build", "--tag", "nginx2", "-f", "Dockerfile", "--cache-from", "from/image1", "--cache-from", "from/image2", "."}, + }} + + testutil.CheckDeepEqual(t, expected, steps) +}