From 2ae6221312489d8f3135f8efb3d990fc7b502c13 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Tue, 14 May 2019 12:19:31 -0400 Subject: [PATCH 01/22] Check minimum Jib version --- pkg/skaffold/jib/jib.go | 37 +++++++++++- pkg/skaffold/jib/jib_gradle_test.go | 8 +-- pkg/skaffold/jib/jib_maven_test.go | 8 +-- pkg/skaffold/jib/jib_test.go | 87 ++++++++++++++++++++++------- 4 files changed, 110 insertions(+), 30 deletions(-) diff --git a/pkg/skaffold/jib/jib.go b/pkg/skaffold/jib/jib.go index 03928389b38..b843e3ae6b8 100644 --- a/pkg/skaffold/jib/jib.go +++ b/pkg/skaffold/jib/jib.go @@ -28,17 +28,24 @@ import ( "time" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" + "github.com/blang/semver" "github.com/karrick/godirwalk" "github.com/pkg/errors" "github.com/sirupsen/logrus" ) +// minJibVersion is the minimum version of Jib required +var minJibVersion = semver.MustParse("1.3.0") + const ( dotDotSlash = ".." + string(filepath.Separator) ) // filesLists contains cached build/input dependencies type filesLists struct { + // BuildDefinitions lists paths to build definitions that trigger a call out to Jib to refresh the pathMap, as well as a rebuild, upon changing + JibVersion string `json:"version"` + // BuildDefinitions lists paths to build definitions that trigger a call out to Jib to refresh the pathMap, as well as a rebuild, upon changing BuildDefinitions []string `json:"build"` @@ -73,7 +80,9 @@ func getDependencies(workspace string, cmd *exec.Cmd, projectName string) ([]str if err := refreshDependencyList(&files, cmd); err != nil { return nil, errors.Wrap(err, "initial Jib dependency refresh failed") } - + if err := checkJibVersion(files.JibVersion); err != nil { + return nil, err + } } else if err := walkFiles(workspace, files.BuildDefinitions, files.Results, func(path string, info os.FileInfo) error { // Walk build files to check for changes if val, ok := files.BuildFileTimes[path]; !ok || info.ModTime() != val { @@ -110,13 +119,13 @@ func getDependencies(workspace string, cmd *exec.Cmd, projectName string) ([]str func refreshDependencyList(files *filesLists, cmd *exec.Cmd) error { stdout, err := util.RunCmdOut(cmd) if err != nil { - return errors.Wrap(err, "failed to get Jib dependencies; it's possible you are using an old version of Jib (Skaffold requires Jib v1.0.2+)") + return errors.Wrapf(err, "failed to get Jib dependencies; it's possible you are using an old version of Jib (Skaffold requires Jib v%s+)", minJibVersion) } // Search for Jib's output JSON. Jib's Maven/Gradle output takes the following form: // ... // BEGIN JIB JSON - // {"build":["/paths","/to","/buildFiles"],"inputs":["/paths","/to","/inputs"],"ignore":["/paths","/to","/ignore"]} + // {"version":"1.3.0","build":["/paths","/to","/buildFiles"],"inputs":["/paths","/to","/inputs"],"ignore":["/paths","/to","/ignore"]} // ... // To parse the output, search for "BEGIN JIB JSON", then unmarshal the next line into the pathMap struct. matches := regexp.MustCompile(`BEGIN JIB JSON\r?\n({.*})`).FindSubmatch(stdout) @@ -230,3 +239,25 @@ func relativize(path string, roots ...string) (string, error) { } return "", errors.New("could not relativize path") } + +// checkJibVersion checks if the provided version is a Maven-style version string +// and is compatible with our minimum supported version. +func checkJibVersion(mavenVersion string) error { + // we assume -SNAPSHOT are just as good as the release + if index := strings.Index(mavenVersion, "-SNAPSHOT"); index >= 0 { + mavenVersion = mavenVersion[0:index] + } + + if len(mavenVersion) == 0 { + return errors.Errorf("requires Jib v%s+", minJibVersion) + } + parsed, err := semver.Parse(mavenVersion) + switch { + case err != nil: + return err + case parsed.LT(minJibVersion): + return errors.Errorf("requires Jib v%s+", minJibVersion) + default: + return nil + } +} diff --git a/pkg/skaffold/jib/jib_gradle_test.go b/pkg/skaffold/jib/jib_gradle_test.go index a89f63ddc58..bc9976ca3d0 100644 --- a/pkg/skaffold/jib/jib_gradle_test.go +++ b/pkg/skaffold/jib/jib_gradle_test.go @@ -65,20 +65,20 @@ func TestGetDependenciesGradle(t *testing.T) { }, { description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}", build, dep1), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}", build, dep1), modTime: time.Unix(0, 0), expected: []string{"build", "dep1"}, }, { // Expected output differs from stdout since build file hasn't change, thus gradle command won't run description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), modTime: time.Unix(0, 0), expected: []string{"build", "dep1"}, }, { description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), modTime: time.Unix(10000, 0), expected: []string{"build", "dep1", "dep2"}, }, @@ -98,7 +98,7 @@ func TestGetDependenciesGradle(t *testing.T) { deps, err := GetDependenciesGradle(ctx, tmpDir.Root(), &latest.JibGradleArtifact{Project: "gradle-test"}) if test.err != nil { - testutil.CheckErrorAndDeepEqual(t, true, err, "getting jibGradle dependencies: initial Jib dependency refresh failed: failed to get Jib dependencies; it's possible you are using an old version of Jib (Skaffold requires Jib v1.0.2+): "+test.err.Error(), err.Error()) + testutil.CheckErrorAndDeepEqual(t, true, err, "getting jibGradle dependencies: initial Jib dependency refresh failed: failed to get Jib dependencies; it's possible you are using an old version of Jib (Skaffold requires Jib v1.3.0+): "+test.err.Error(), err.Error()) } else { testutil.CheckDeepEqual(t, test.expected, deps) } diff --git a/pkg/skaffold/jib/jib_maven_test.go b/pkg/skaffold/jib/jib_maven_test.go index d04b3805b64..1bc2253a84d 100644 --- a/pkg/skaffold/jib/jib_maven_test.go +++ b/pkg/skaffold/jib/jib_maven_test.go @@ -65,20 +65,20 @@ func TestGetDependenciesMaven(t *testing.T) { }, { description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}", build, dep1), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}", build, dep1), modTime: time.Unix(0, 0), expected: []string{"build", "dep1"}, }, { // Expected output differs from stdout since build file hasn't change, thus maven command won't run description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), modTime: time.Unix(0, 0), expected: []string{"build", "dep1"}, }, { description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), modTime: time.Unix(10000, 0), expected: []string{"build", "dep1", "dep2"}, }, @@ -98,7 +98,7 @@ func TestGetDependenciesMaven(t *testing.T) { deps, err := GetDependenciesMaven(ctx, tmpDir.Root(), &latest.JibMavenArtifact{Module: "maven-test"}) if test.err != nil { - testutil.CheckErrorAndDeepEqual(t, true, err, "getting jibMaven dependencies: initial Jib dependency refresh failed: failed to get Jib dependencies; it's possible you are using an old version of Jib (Skaffold requires Jib v1.0.2+): "+test.err.Error(), err.Error()) + testutil.CheckErrorAndDeepEqual(t, true, err, "getting jibMaven dependencies: initial Jib dependency refresh failed: failed to get Jib dependencies; it's possible you are using an old version of Jib (Skaffold requires Jib v1.3.0+): "+test.err.Error(), err.Error()) } else { testutil.CheckDeepEqual(t, test.expected, deps) } diff --git a/pkg/skaffold/jib/jib_test.go b/pkg/skaffold/jib/jib_test.go index 80e73ab3d91..e4f4fcf7b21 100644 --- a/pkg/skaffold/jib/jib_test.go +++ b/pkg/skaffold/jib/jib_test.go @@ -40,40 +40,63 @@ func TestGetDependencies(t *testing.T) { dep3 := tmpDir.Path("dep3") var tests = []struct { - stdout string - expectedDeps []string + name string + stdout string + expectedError bool + expectedDeps []string }{ { - stdout: "BEGIN JIB JSON\n{\"build\":[],\"inputs\":[],\"ignore\":[]}", - expectedDeps: nil, + name: "missing version", + stdout: "BEGIN JIB JSON\n{\"build\":[],\"inputs\":[],\"ignore\":[]}", + expectedError: true, + expectedDeps: nil, }, { - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}\n", dep1, dep2), - expectedDeps: []string{"dep1", "dep2"}, + name: "out of date version", + stdout: "BEGIN JIB JSON\n{\"version\":\"1.0.0\",\"build\":[],\"inputs\":[],\"ignore\":[]}", + expectedError: true, + expectedDeps: nil, }, { - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[],\"inputs\":[\"%s\"],\"ignore\":[]}\n", dep3), - expectedDeps: []string{filepath.FromSlash("dep3/fileA"), filepath.FromSlash("dep3/sub/path/fileB")}, + name: "base case", + stdout: "BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[],\"inputs\":[],\"ignore\":[]}", + expectedError: false, + expectedDeps: nil, }, { - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[],\"inputs\":[\"%s\",\"%s\",\"%s\"],\"ignore\":[]}\n", dep1, dep2, dep3), - expectedDeps: []string{"dep1", "dep2", filepath.FromSlash("dep3/fileA"), filepath.FromSlash("dep3/sub/path/fileB")}, + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}\n", dep1, dep2), + expectedError: false, + expectedDeps: []string{"dep1", "dep2"}, }, { - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[],\"inputs\":[\"%s\",\"%s\",\"nonexistent\",\"%s\"],\"ignore\":[]}\n", dep1, dep2, dep3), - expectedDeps: []string{"dep1", "dep2", filepath.FromSlash("dep3/fileA"), filepath.FromSlash("dep3/sub/path/fileB")}, + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[],\"inputs\":[\"%s\"],\"ignore\":[]}\n", dep3), + expectedError: false, + expectedDeps: []string{filepath.FromSlash("dep3/fileA"), filepath.FromSlash("dep3/sub/path/fileB")}, }, { - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[],\"inputs\":[\"%s\",\"%s\"],\"ignore\":[\"%s\"]}\n", dep1, dep2, dep2), - expectedDeps: []string{"dep1"}, + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[],\"inputs\":[\"%s\",\"%s\",\"%s\"],\"ignore\":[]}\n", dep1, dep2, dep3), + expectedError: false, + expectedDeps: []string{"dep1", "dep2", filepath.FromSlash("dep3/fileA"), filepath.FromSlash("dep3/sub/path/fileB")}, }, { - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[\"%s\",\"%s\"]}\n", dep1, dep3, dep1, dep3), - expectedDeps: nil, + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[],\"inputs\":[\"%s\",\"%s\",\"nonexistent\",\"%s\"],\"ignore\":[]}\n", dep1, dep2, dep3), + expectedError: false, + expectedDeps: []string{"dep1", "dep2", filepath.FromSlash("dep3/fileA"), filepath.FromSlash("dep3/sub/path/fileB")}, }, { - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\",\"%s\",\"%s\"],\"inputs\":[],\"ignore\":[\"%s\"]}\n", dep1, dep2, dep3, tmpDir.Path("dep3/sub/path")), - expectedDeps: []string{"dep1", "dep2", filepath.FromSlash("dep3/fileA")}, + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[],\"inputs\":[\"%s\",\"%s\"],\"ignore\":[\"%s\"]}\n", dep1, dep2, dep2), + expectedError: false, + expectedDeps: []string{"dep1"}, + }, + { + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[\"%s\",\"%s\"]}\n", dep1, dep3, dep1, dep3), + expectedError: false, + expectedDeps: nil, + }, + { + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\",\"%s\",\"%s\"],\"inputs\":[],\"ignore\":[\"%s\"]}\n", dep1, dep2, dep3, tmpDir.Path("dep3/sub/path")), + expectedError: false, + expectedDeps: []string{"dep1", "dep2", filepath.FromSlash("dep3/fileA")}, }, } @@ -90,7 +113,33 @@ func TestGetDependencies(t *testing.T) { results, err := getDependencies(tmpDir.Root(), &exec.Cmd{Args: []string{"ignored"}, Dir: tmpDir.Root()}, "test") - testutil.CheckErrorAndDeepEqual(t, false, err, test.expectedDeps, results) + testutil.CheckErrorAndDeepEqual(t, test.expectedError, err, test.expectedDeps, results) + }) + } +} + +func TestCheckJibVersion(t *testing.T) { + var tests = []struct { + version string + expectedError bool + }{ + {"", true}, + {"-1", true}, + {"1", true}, + {"-SNAPSHOT", true}, + {"abc", true}, + {"1.2", true}, + {"1.2-SNAPSHOT", true}, + {"1.2.0", true}, + {"1.2.0-SNAPSHOT", true}, + {"1.3.0", false}, + {"1.3.0-SNAPSHOT", false}, + {"2.0.0", false}, + } + for _, test := range tests { + t.Run(test.version, func(t *testing.T) { + err := checkJibVersion(test.version) + testutil.CheckError(t, test.expectedError, err) }) } } From 67cf35bc1466b7b9acd94fb4d17635c8a37f4033 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Tue, 14 May 2019 12:45:31 -0400 Subject: [PATCH 02/22] Improve Maven/Jib multimodule builds --- examples/jib-multimodule/pom.xml | 2 +- examples/jib-multimodule/project1/pom.xml | 10 ---- examples/jib-multimodule/project2/pom.xml | 10 ---- pkg/skaffold/build/local/jib_maven.go | 40 ---------------- pkg/skaffold/build/local/jib_maven_test.go | 54 ---------------------- pkg/skaffold/jib/jib_maven.go | 4 +- pkg/skaffold/jib/jib_maven_test.go | 8 ++-- 7 files changed, 7 insertions(+), 121 deletions(-) delete mode 100644 pkg/skaffold/build/local/jib_maven_test.go diff --git a/examples/jib-multimodule/pom.xml b/examples/jib-multimodule/pom.xml index cbf72e93f64..9165d7dde46 100644 --- a/examples/jib-multimodule/pom.xml +++ b/examples/jib-multimodule/pom.xml @@ -38,7 +38,7 @@ com.google.cloud.tools jib-maven-plugin - 1.0.2 + 1.3.0-SNAPSHOT diff --git a/examples/jib-multimodule/project1/pom.xml b/examples/jib-multimodule/project1/pom.xml index 97fd110ffb2..27ba3dbb58c 100644 --- a/examples/jib-multimodule/project1/pom.xml +++ b/examples/jib-multimodule/project1/pom.xml @@ -36,16 +36,6 @@ - - - - default-package - package - - build - - - diff --git a/examples/jib-multimodule/project2/pom.xml b/examples/jib-multimodule/project2/pom.xml index 6cabfe871c6..fc207832807 100644 --- a/examples/jib-multimodule/project2/pom.xml +++ b/examples/jib-multimodule/project2/pom.xml @@ -36,16 +36,6 @@ - - - - default-package - package - - build - - - diff --git a/pkg/skaffold/build/local/jib_maven.go b/pkg/skaffold/build/local/jib_maven.go index e7e34c5addd..33b16e186e0 100644 --- a/pkg/skaffold/build/local/jib_maven.go +++ b/pkg/skaffold/build/local/jib_maven.go @@ -37,13 +37,6 @@ func (b *Builder) buildJibMaven(ctx context.Context, out io.Writer, workspace st } func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibMavenArtifact, tag string) (string, error) { - // If this is a multi-module project, we require `package` be bound to jib:dockerBuild - if artifact.Module != "" { - if err := verifyJibPackageGoal(ctx, "dockerBuild", workspace, artifact); err != nil { - return "", err - } - } - args := jib.GenerateMavenArgs("dockerBuild", tag, artifact, b.skipTests) if err := b.runMavenCommand(ctx, out, workspace, args); err != nil { return "", err @@ -53,13 +46,6 @@ func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, work } func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibMavenArtifact, tag string) (string, error) { - // If this is a multi-module project, we require `package` be bound to jib:build - if artifact.Module != "" { - if err := verifyJibPackageGoal(ctx, "build", workspace, artifact); err != nil { - return "", err - } - } - args := jib.GenerateMavenArgs("build", tag, artifact, b.skipTests) if err := b.runMavenCommand(ctx, out, workspace, args); err != nil { return "", err @@ -68,32 +54,6 @@ func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, wo return docker.RemoteDigest(tag, b.insecureRegistries) } -// verifyJibPackageGoal verifies that the referenced module has `package` bound to a single jib goal. -// It returns `nil` if the goal is matched, and an error if there is a mismatch. -func verifyJibPackageGoal(ctx context.Context, requiredGoal string, workspace string, artifact *latest.JibMavenArtifact) error { - // cannot use --non-recursive - command := []string{"--quiet", "--projects", artifact.Module, "jib:_skaffold-package-goals"} - if artifact.Profile != "" { - command = append(command, "--activate-profiles", artifact.Profile) - } - - cmd := jib.MavenCommand.CreateCommand(ctx, workspace, command) - logrus.Debugf("Looking for jib bound package goals for %s: %s, %v", workspace, cmd.Path, cmd.Args) - stdout, err := util.RunCmdOut(cmd) - if err != nil { - return errors.Wrap(err, "could not obtain jib package goals") - } - goals := util.NonEmptyLines(stdout) - logrus.Debugf("jib bound package goals for %s %s: %v (%d)", workspace, artifact.Module, goals, len(goals)) - if len(goals) != 1 { - return errors.New("skaffold requires a single jib goal bound to 'package'") - } - if goals[0] != requiredGoal { - return errors.Errorf("skaffold `push` setting requires 'package' be bound to 'jib:%s'", requiredGoal) - } - return nil -} - func (b *Builder) runMavenCommand(ctx context.Context, out io.Writer, workspace string, args []string) error { cmd := jib.MavenCommand.CreateCommand(ctx, workspace, args) cmd.Env = append(os.Environ(), b.localDocker.ExtraEnv()...) diff --git a/pkg/skaffold/build/local/jib_maven_test.go b/pkg/skaffold/build/local/jib_maven_test.go deleted file mode 100644 index a887c3e29d3..00000000000 --- a/pkg/skaffold/build/local/jib_maven_test.go +++ /dev/null @@ -1,54 +0,0 @@ -/* -Copyright 2019 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 local - -import ( - "context" - "testing" - - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" - "github.com/GoogleContainerTools/skaffold/testutil" -) - -func TestMavenVerifyJibPackageGoal(t *testing.T) { - var testCases = []struct { - requiredGoal string - mavenOutput string - shouldError bool - }{ - {"xxx", "", true}, // no goals should fail - {"xxx", "\n", true}, // no goals should fail; newline stripped - {"dockerBuild", "dockerBuild", false}, - {"dockerBuild", "dockerBuild\n", false}, // newline stripped - {"dockerBuild", "build\n", true}, - {"dockerBuild", "build\ndockerBuild\n", true}, - } - - defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand) - defer func(previous bool) { util.SkipWrapperCheck = previous }(util.SkipWrapperCheck) - util.SkipWrapperCheck = true - - for _, tt := range testCases { - util.DefaultExecCommand = testutil.NewFakeCmd(t).WithRunOut("mvn --quiet --projects module jib:_skaffold-package-goals", tt.mavenOutput) - - err := verifyJibPackageGoal(context.Background(), tt.requiredGoal, ".", &latest.JibMavenArtifact{Module: "module"}) - if hasError := err != nil; tt.shouldError != hasError { - t.Error("Unexpected return result") - } - } -} diff --git a/pkg/skaffold/jib/jib_maven.go b/pkg/skaffold/jib/jib_maven.go index 2f98daf1fec..96571a87832 100644 --- a/pkg/skaffold/jib/jib_maven.go +++ b/pkg/skaffold/jib/jib_maven.go @@ -62,8 +62,8 @@ func GenerateMavenArgs(goal string, imageName string, a *latest.JibMavenArtifact // single-module project args = append(args, "prepare-package", "jib:"+goal) } else { - // multi-module project: we assume `package` is bound to `jib:` - args = append(args, "package") + // multi-module project: instruct jib to containerize only the given module + args = append(args, "package", "jib:"+goal, "-Djib.containerize=" + a.Module) } args = append(args, "-Dimage="+imageName) diff --git a/pkg/skaffold/jib/jib_maven_test.go b/pkg/skaffold/jib/jib_maven_test.go index 1bc2253a84d..f168f587a60 100644 --- a/pkg/skaffold/jib/jib_maven_test.go +++ b/pkg/skaffold/jib/jib_maven_test.go @@ -202,10 +202,10 @@ func TestGenerateMavenArgs(t *testing.T) { {latest.JibMavenArtifact{}, true, []string{"-Djib.console=plain", "--non-recursive", "-DskipTests=true", "prepare-package", "jib:goal", "-Dimage=image"}}, {latest.JibMavenArtifact{Profile: "profile"}, false, []string{"-Djib.console=plain", "--activate-profiles", "profile", "--non-recursive", "prepare-package", "jib:goal", "-Dimage=image"}}, {latest.JibMavenArtifact{Profile: "profile"}, true, []string{"-Djib.console=plain", "--activate-profiles", "profile", "--non-recursive", "-DskipTests=true", "prepare-package", "jib:goal", "-Dimage=image"}}, - {latest.JibMavenArtifact{Module: "module"}, false, []string{"-Djib.console=plain", "--projects", "module", "--also-make", "package", "-Dimage=image"}}, - {latest.JibMavenArtifact{Module: "module"}, true, []string{"-Djib.console=plain", "--projects", "module", "--also-make", "-DskipTests=true", "package", "-Dimage=image"}}, - {latest.JibMavenArtifact{Module: "module", Profile: "profile"}, false, []string{"-Djib.console=plain", "--activate-profiles", "profile", "--projects", "module", "--also-make", "package", "-Dimage=image"}}, - {latest.JibMavenArtifact{Module: "module", Profile: "profile"}, true, []string{"-Djib.console=plain", "--activate-profiles", "profile", "--projects", "module", "--also-make", "-DskipTests=true", "package", "-Dimage=image"}}, + {latest.JibMavenArtifact{Module: "module"}, false, []string{"-Djib.console=plain", "--projects", "module", "--also-make", "package", "jib:goal", "-Djib.containerize=module", "-Dimage=image"}}, + {latest.JibMavenArtifact{Module: "module"}, true, []string{"-Djib.console=plain", "--projects", "module", "--also-make", "-DskipTests=true", "package", "jib:goal", "-Djib.containerize=module", "-Dimage=image"}}, + {latest.JibMavenArtifact{Module: "module", Profile: "profile"}, false, []string{"-Djib.console=plain", "--activate-profiles", "profile", "--projects", "module", "--also-make", "package", "jib:goal", "-Djib.containerize=module", "-Dimage=image"}}, + {latest.JibMavenArtifact{Module: "module", Profile: "profile"}, true, []string{"-Djib.console=plain", "--activate-profiles", "profile", "--projects", "module", "--also-make", "-DskipTests=true", "package", "jib:goal", "-Djib.containerize=module", "-Dimage=image"}}, } for _, tt := range testCases { From fc58ef704a3c15d1671b33e971a5398d73195ba6 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Tue, 14 May 2019 13:39:36 -0400 Subject: [PATCH 03/22] Add documentation --- CHANGELOG.md | 8 +++++ .../en/docs/how-tos/builders/_index.md | 29 +++++++++---------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab9de9c185b..196429c7175 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,11 @@ +# v0.XX.X Release - + +*Note for Jib users*: +The Jib binding has changed and projects are now required to use +Jib v1.3.0 or later. Maven multi-module projects should no longer +include bindings to `jib:build` and `jib:dockerBuild` foror the +_package_ phase. + # v0.29.0 Release - 05/09/2019 *Note*: This release comes with a new config version `v1beta10`. diff --git a/docs/content/en/docs/how-tos/builders/_index.md b/docs/content/en/docs/how-tos/builders/_index.md index 492e30853d2..ec1336e8377 100755 --- a/docs/content/en/docs/how-tos/builders/_index.md +++ b/docs/content/en/docs/how-tos/builders/_index.md @@ -131,6 +131,8 @@ without a Docker daemon. Skaffold can help build artifacts using Jib; Jib builds the container images and then pushes them to the local Docker daemon or to remote registries as instructed by Skaffold. +Skaffold requires using Jib v1.3.0 or later. + ### Configuration To use Jib, add a `jibMaven` or `jibGradle` field to each artifact you specify in the @@ -165,22 +167,17 @@ each produce a separate container image. To build a Maven multi-module project, first identify the modules that should produce a container image. Then for each such module: - 1. Create a Skaffold `artifact` in the `skaffold.yaml`: - - Set the `artifact`'s `context` field to the root project location. - - Add a `jibMaven` element and set its `module` field to the module's - `:artifactId`, `groupId:artifactId`, or the relative path to the module - _within the project_. - 2. Configure the module's `pom.xml` to bind either `jib:build` or `jib:dockerBuild` to - the `package` phase as appropriate (see below). - -This second step is necessary at the moment as Maven applies plugin goals specified -on the command-line, like `jib:build` or, to all modules and not just the modules -producing container images. -The situation is further complicated as Skaffold speeds up deploys to a local cluster, -such as `minikube`, by building and loading container images directly to the -local cluster's docker daemon (via `jib:dockerBuild` instead of `jib:build`), -thus saving a push and a pull of the image. -We plan to improve this situation [(#1876)](https://github.com/GoogleContainerTools/skaffold/issues/1876). + - Create a Skaffold `artifact` in the `skaffold.yaml`: + - Set the `artifact`'s `context` field to the root project location. + - Add a `jibMaven` element and set its `module` field to the module's + `:artifactId`, `groupId:artifactId`, or the relative path to the module + _within the project_. + +{{% alert title="Updating from earlier versions" %}} +Skaffold had required Maven multi-module projects bind a Jib +`build` or `dockerBuild` goal to the *package* phase. These bindings are +no longer required with Jib 1.3.0 and should be removed. +{{% /alert %}} #### Gradle From 803be228b8b1e48581f1d5602717d3b9e5beda6e Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Wed, 15 May 2019 13:56:17 -0400 Subject: [PATCH 04/22] review nits --- CHANGELOG.md | 5 ++--- examples/jib-multimodule/pom.xml | 3 ++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 196429c7175..2081e266926 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,9 +2,8 @@ *Note for Jib users*: The Jib binding has changed and projects are now required to use -Jib v1.3.0 or later. Maven multi-module projects should no longer -include bindings to `jib:build` and `jib:dockerBuild` foror the -_package_ phase. +Jib v1.3.0 or later. Maven multi-module projects no longer require +binding `jib:build` or `jib:dockerBuild` to the _package_ phase. # v0.29.0 Release - 05/09/2019 diff --git a/examples/jib-multimodule/pom.xml b/examples/jib-multimodule/pom.xml index 9165d7dde46..c94a5c6ddad 100644 --- a/examples/jib-multimodule/pom.xml +++ b/examples/jib-multimodule/pom.xml @@ -18,6 +18,7 @@ 1.8 + 1.2.0 @@ -38,7 +39,7 @@ com.google.cloud.tools jib-maven-plugin - 1.3.0-SNAPSHOT + ${jib.maven-plugin-version} From c4dda08f5b0b4f0847c5c8d5a0f5a33e4d6de08d Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Wed, 15 May 2019 13:59:06 -0400 Subject: [PATCH 05/22] Jib 1.0.2 -> 1.3.0 --- examples/jib-multimodule/pom.xml | 2 +- examples/jib/pom.xml | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/jib-multimodule/pom.xml b/examples/jib-multimodule/pom.xml index c94a5c6ddad..b6bbe0b2de7 100644 --- a/examples/jib-multimodule/pom.xml +++ b/examples/jib-multimodule/pom.xml @@ -18,7 +18,7 @@ 1.8 - 1.2.0 + 1.3.0 diff --git a/examples/jib/pom.xml b/examples/jib/pom.xml index fa3f3ae381f..e479f36567c 100644 --- a/examples/jib/pom.xml +++ b/examples/jib/pom.xml @@ -9,6 +9,7 @@ 1.8 + 1.3.0 @@ -34,7 +35,7 @@ com.google.cloud.tools jib-maven-plugin - 1.0.2 + ${jib.maven-plugin-version} From d116effaf02a12c9baf94f25e83df00a55c98e95 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Wed, 15 May 2019 14:01:08 -0400 Subject: [PATCH 06/22] integration test too --- integration/testdata/jib/pom.xml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integration/testdata/jib/pom.xml b/integration/testdata/jib/pom.xml index fa3f3ae381f..e479f36567c 100644 --- a/integration/testdata/jib/pom.xml +++ b/integration/testdata/jib/pom.xml @@ -9,6 +9,7 @@ 1.8 + 1.3.0 @@ -34,7 +35,7 @@ com.google.cloud.tools jib-maven-plugin - 1.0.2 + ${jib.maven-plugin-version} From 83127c1e7d307c39b816d5365c1892073fefa73b Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Mon, 10 Jun 2019 15:31:51 -0400 Subject: [PATCH 07/22] Fix checkJibVersion to support RCs --- pkg/skaffold/jib/jib.go | 5 +++-- pkg/skaffold/jib/jib_test.go | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/skaffold/jib/jib.go b/pkg/skaffold/jib/jib.go index b843e3ae6b8..a5d696e3ef9 100644 --- a/pkg/skaffold/jib/jib.go +++ b/pkg/skaffold/jib/jib.go @@ -243,8 +243,9 @@ func relativize(path string, roots ...string) (string, error) { // checkJibVersion checks if the provided version is a Maven-style version string // and is compatible with our minimum supported version. func checkJibVersion(mavenVersion string) error { - // we assume -SNAPSHOT are just as good as the release - if index := strings.Index(mavenVersion, "-SNAPSHOT"); index >= 0 { + // semver.Parse doesn't support maven-style version qualifiers + // we assume a version with a qualifiers is just as good as the release + if index := strings.Index(mavenVersion, "-"); index >= 0 { mavenVersion = mavenVersion[0:index] } diff --git a/pkg/skaffold/jib/jib_test.go b/pkg/skaffold/jib/jib_test.go index da3176ed330..90e55e39938 100644 --- a/pkg/skaffold/jib/jib_test.go +++ b/pkg/skaffold/jib/jib_test.go @@ -145,6 +145,7 @@ func TestCheckJibVersion(t *testing.T) { {"1.2.0-SNAPSHOT", true}, {"1.3.0", false}, {"1.3.0-SNAPSHOT", false}, + {"1.4.0-rc1", false}, {"2.0.0", false}, } for _, test := range tests { From d52c3cfee2301bd2fdd7af28ce79e0c24f4f9fc0 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Tue, 11 Jun 2019 21:16:55 -0400 Subject: [PATCH 08/22] Remove stray conflict marker --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d6e66bad1a..c1d8df2e4ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -150,7 +150,6 @@ Huge thanks goes out to all of our contributors for this release: - Priya Wadhwa - Taylor Barrella - Tejal Desai ->>>>>>> origin/master # v0.29.0 Release - 05/09/2019 From c8fa4f633165d3e0f4698a36edb4e1e4f4fba452 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Thu, 18 Jul 2019 17:37:25 -0400 Subject: [PATCH 09/22] Bump minimum Jib versions to 1.4.0, and add test to check too-old --- integration/build_test.go | 37 ++++++++++++++++++++++++++ integration/run_test.go | 4 +++ integration/testdata/jib/skaffold.yaml | 5 ++++ pkg/skaffold/jib/jib_gradle.go | 4 +-- pkg/skaffold/jib/jib_maven.go | 4 +-- 5 files changed, 50 insertions(+), 4 deletions(-) diff --git a/integration/build_test.go b/integration/build_test.go index 3ea17d5acdf..9f871516493 100644 --- a/integration/build_test.go +++ b/integration/build_test.go @@ -179,3 +179,40 @@ func failNowIfError(t *testing.T, err error) { t.Fatal(err) } } + +// TestExpectedBuildFailures verifies that `skaffold build` fails in expected ways +func TestExpectedBuildFailures(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test") + } + + tests := []struct { + description string + dir string + filename string + args []string + env []string + gcpOnly bool + }{ + { + description: "jib is too old", + dir: "testdata/jib", + args: []string{"-p", "old-jib"}, + }, + } + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + if test.gcpOnly && !ShouldRunGCPOnlyTests() { + t.Skip("skipping gcp only test") + } + if !test.gcpOnly && ShouldRunGCPOnlyTests() { + t.Skip("skipping test that is not gcp only") + } + + err := skaffold.Build(test.args...).WithConfig(test.filename).InDir(test.dir).Run(t) + if err == nil { + t.Fatal("expected failure") + } + }) + } +} diff --git a/integration/run_test.go b/integration/run_test.go index 1cb18fddaee..cd98603ddeb 100644 --- a/integration/run_test.go +++ b/integration/run_test.go @@ -99,6 +99,10 @@ func TestRun(t *testing.T) { dir: "testdata/kaniko-microservices", deployments: []string{"leeroy-app", "leeroy-web"}, gcpOnly: true, + }, { + description: "jib", + dir: "testdata/jib", + deployments: []string{"web"}, }, { description: "jib in googlecloudbuild", dir: "testdata/jib", diff --git a/integration/testdata/jib/skaffold.yaml b/integration/testdata/jib/skaffold.yaml index 8819b789575..b4a2f8ab97b 100644 --- a/integration/testdata/jib/skaffold.yaml +++ b/integration/testdata/jib/skaffold.yaml @@ -12,3 +12,8 @@ profiles: - name: gcb build: googleCloudBuild: {} + - name: old-jib + patches: + - op: add + path: /build/artifacts/0/jibMaven/args + value: ['-Djib.maven-plugin-version=1.3.0'] diff --git a/pkg/skaffold/jib/jib_gradle.go b/pkg/skaffold/jib/jib_gradle.go index bac5d4de101..3a59054577a 100644 --- a/pkg/skaffold/jib/jib_gradle.go +++ b/pkg/skaffold/jib/jib_gradle.go @@ -27,8 +27,8 @@ import ( "github.com/sirupsen/logrus" ) -// Skaffold-Jib depends on functionality introduced after Jib-Gradle 1.3.0 -const MinimumJibGradleVersion = "(1.3.0,)" +// Skaffold-Jib depends on functionality introduced after Jib-Gradle 1.4.0 +const MinimumJibGradleVersion = "(1.4.0,)" // GradleCommand stores Gradle executable and wrapper name var GradleCommand = util.CommandWrapper{Executable: "gradle", Wrapper: "gradlew"} diff --git a/pkg/skaffold/jib/jib_maven.go b/pkg/skaffold/jib/jib_maven.go index f2cf2971447..f92c1cfc640 100644 --- a/pkg/skaffold/jib/jib_maven.go +++ b/pkg/skaffold/jib/jib_maven.go @@ -26,8 +26,8 @@ import ( "github.com/sirupsen/logrus" ) -// Skaffold-Jib depends on functionality introduced after Jib-Maven 1.3.0 -const MinimumJibMavenVersion = "(1.3.0,)" +// Skaffold-Jib depends on functionality introduced after Jib-Maven 1.4.0 +const MinimumJibMavenVersion = "(1.4.0,)" // MavenCommand stores Maven executable and wrapper name var MavenCommand = util.CommandWrapper{Executable: "mvn", Wrapper: "mvnw"} From dd163860b6cb43ab991dc0a74b10c293e0c8b1e3 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Thu, 18 Jul 2019 20:04:07 -0400 Subject: [PATCH 10/22] Update CHANGELOG.md --- CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cff4429ee9f..63454399a07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -139,8 +139,6 @@ Huge thanks goes out to all of our contributors for this release: - Valentin Fedoskin - yfei1 ->>>>>>> origin/master - # v0.31.0 Release - 06/06/2019 New Features: From de5bc34bbb0ee87dfac0e4a1ecd2a603bd14d904 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 19 Jul 2019 10:20:25 -0400 Subject: [PATCH 11/22] fix tests --- integration/testdata/jib/skaffold.yaml | 2 +- pkg/skaffold/jib/jib_gradle_test.go | 2 +- pkg/skaffold/jib/jib_maven_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/testdata/jib/skaffold.yaml b/integration/testdata/jib/skaffold.yaml index b4a2f8ab97b..e4c3ae3d519 100644 --- a/integration/testdata/jib/skaffold.yaml +++ b/integration/testdata/jib/skaffold.yaml @@ -16,4 +16,4 @@ profiles: patches: - op: add path: /build/artifacts/0/jibMaven/args - value: ['-Djib.maven-plugin-version=1.3.0'] + value: ['-Djib.maven-plugin-version=1.3.1-SNAPSHOT'] diff --git a/pkg/skaffold/jib/jib_gradle_test.go b/pkg/skaffold/jib/jib_gradle_test.go index 3437b889efe..08d3b5521f7 100644 --- a/pkg/skaffold/jib/jib_gradle_test.go +++ b/pkg/skaffold/jib/jib_gradle_test.go @@ -32,7 +32,7 @@ import ( ) func TestMinimumGradleVersion(t *testing.T) { - testutil.CheckDeepEqual(t, "(1.3.0,)", MinimumJibGradleVersion) + testutil.CheckDeepEqual(t, "(1.4.0,)", MinimumJibGradleVersion) } func TestGradleWrapperDefinition(t *testing.T) { diff --git a/pkg/skaffold/jib/jib_maven_test.go b/pkg/skaffold/jib/jib_maven_test.go index 76de609515d..0ac03e0aaab 100644 --- a/pkg/skaffold/jib/jib_maven_test.go +++ b/pkg/skaffold/jib/jib_maven_test.go @@ -32,7 +32,7 @@ import ( ) func TestMinimumMavenVersion(t *testing.T) { - testutil.CheckDeepEqual(t, "(1.3.0,)", MinimumJibMavenVersion) + testutil.CheckDeepEqual(t, "(1.4.0,)", MinimumJibMavenVersion) } func TestMavenWrapperDefinition(t *testing.T) { From 9384ea3d2d264a7eed9f10b938040d3de2e77c32 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 19 Jul 2019 12:44:22 -0400 Subject: [PATCH 12/22] back out accidental commit --- integration/testdata/jib/skaffold.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration/testdata/jib/skaffold.yaml b/integration/testdata/jib/skaffold.yaml index e4c3ae3d519..b4a2f8ab97b 100644 --- a/integration/testdata/jib/skaffold.yaml +++ b/integration/testdata/jib/skaffold.yaml @@ -16,4 +16,4 @@ profiles: patches: - op: add path: /build/artifacts/0/jibMaven/args - value: ['-Djib.maven-plugin-version=1.3.1-SNAPSHOT'] + value: ['-Djib.maven-plugin-version=1.3.0'] From ccd52ce8a1f2f6b393fbf4e5b54f336f1eeef959 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 19 Jul 2019 12:46:14 -0400 Subject: [PATCH 13/22] remove accidentally reintroduced version headers --- pkg/skaffold/jib/jib.go | 2 +- pkg/skaffold/jib/jib_gradle_test.go | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/skaffold/jib/jib.go b/pkg/skaffold/jib/jib.go index d6d91d9afa1..f8cc09d73fa 100644 --- a/pkg/skaffold/jib/jib.go +++ b/pkg/skaffold/jib/jib.go @@ -115,7 +115,7 @@ func refreshDependencyList(files *filesLists, cmd exec.Cmd) error { // Search for Jib's output JSON. Jib's Maven/Gradle output takes the following form: // ... // BEGIN JIB JSON - // {"version":"1.3.0","build":["/paths","/to","/buildFiles"],"inputs":["/paths","/to","/inputs"],"ignore":["/paths","/to","/ignore"]} + // {"build":["/paths","/to","/buildFiles"],"inputs":["/paths","/to","/inputs"],"ignore":["/paths","/to","/ignore"]} // ... // To parse the output, search for "BEGIN JIB JSON", then unmarshal the next line into the pathMap struct. matches := regexp.MustCompile(`BEGIN JIB JSON\r?\n({.*})`).FindSubmatch(stdout) diff --git a/pkg/skaffold/jib/jib_gradle_test.go b/pkg/skaffold/jib/jib_gradle_test.go index 08d3b5521f7..65901f86a6f 100644 --- a/pkg/skaffold/jib/jib_gradle_test.go +++ b/pkg/skaffold/jib/jib_gradle_test.go @@ -66,20 +66,20 @@ func TestGetDependenciesGradle(t *testing.T) { }, { description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}", build, dep1), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}", build, dep1), modTime: time.Unix(0, 0), expected: []string{"build", "dep1"}, }, { // Expected output differs from stdout since build file hasn't change, thus gradle command won't run description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), modTime: time.Unix(0, 0), expected: []string{"build", "dep1"}, }, { description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), modTime: time.Unix(10000, 0), expected: []string{"build", "dep1", "dep2"}, }, From d784215dbcdf0bfba1d372a4e38ba45c2e11611f Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 19 Jul 2019 12:51:20 -0400 Subject: [PATCH 14/22] Fix version bounds --- pkg/skaffold/jib/jib_gradle.go | 4 ++-- pkg/skaffold/jib/jib_gradle_test.go | 2 +- pkg/skaffold/jib/jib_maven.go | 4 ++-- pkg/skaffold/jib/jib_maven_test.go | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/pkg/skaffold/jib/jib_gradle.go b/pkg/skaffold/jib/jib_gradle.go index 3a59054577a..31664b4878b 100644 --- a/pkg/skaffold/jib/jib_gradle.go +++ b/pkg/skaffold/jib/jib_gradle.go @@ -27,8 +27,8 @@ import ( "github.com/sirupsen/logrus" ) -// Skaffold-Jib depends on functionality introduced after Jib-Gradle 1.4.0 -const MinimumJibGradleVersion = "(1.4.0,)" +// Skaffold-Jib depends on functionality introduced with Jib-Gradle 1.4.0 +const MinimumJibGradleVersion = "[1.4.0,)" // GradleCommand stores Gradle executable and wrapper name var GradleCommand = util.CommandWrapper{Executable: "gradle", Wrapper: "gradlew"} diff --git a/pkg/skaffold/jib/jib_gradle_test.go b/pkg/skaffold/jib/jib_gradle_test.go index 65901f86a6f..56ac26a3ffe 100644 --- a/pkg/skaffold/jib/jib_gradle_test.go +++ b/pkg/skaffold/jib/jib_gradle_test.go @@ -32,7 +32,7 @@ import ( ) func TestMinimumGradleVersion(t *testing.T) { - testutil.CheckDeepEqual(t, "(1.4.0,)", MinimumJibGradleVersion) + testutil.CheckDeepEqual(t, "[1.4.0,)", MinimumJibGradleVersion) } func TestGradleWrapperDefinition(t *testing.T) { diff --git a/pkg/skaffold/jib/jib_maven.go b/pkg/skaffold/jib/jib_maven.go index f92c1cfc640..13cac50651c 100644 --- a/pkg/skaffold/jib/jib_maven.go +++ b/pkg/skaffold/jib/jib_maven.go @@ -26,8 +26,8 @@ import ( "github.com/sirupsen/logrus" ) -// Skaffold-Jib depends on functionality introduced after Jib-Maven 1.4.0 -const MinimumJibMavenVersion = "(1.4.0,)" +// Skaffold-Jib depends on functionality introduced with Jib-Maven 1.4.0 +const MinimumJibMavenVersion = "[1.4.0,)" // MavenCommand stores Maven executable and wrapper name var MavenCommand = util.CommandWrapper{Executable: "mvn", Wrapper: "mvnw"} diff --git a/pkg/skaffold/jib/jib_maven_test.go b/pkg/skaffold/jib/jib_maven_test.go index 0ac03e0aaab..160f8cfab6f 100644 --- a/pkg/skaffold/jib/jib_maven_test.go +++ b/pkg/skaffold/jib/jib_maven_test.go @@ -32,7 +32,7 @@ import ( ) func TestMinimumMavenVersion(t *testing.T) { - testutil.CheckDeepEqual(t, "(1.4.0,)", MinimumJibMavenVersion) + testutil.CheckDeepEqual(t, "[1.4.0,)", MinimumJibMavenVersion) } func TestMavenWrapperDefinition(t *testing.T) { From ead1a953c1d6df6188c574b8b37d312a0fcca8e1 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 19 Jul 2019 12:51:29 -0400 Subject: [PATCH 15/22] Doc nits --- CHANGELOG.md | 2 +- docs/content/en/docs/how-tos/builders/_index.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 63454399a07..98a7dd7d17d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ *Note for Jib users*: The Jib binding has changed and projects are now required to use Jib v1.4.0 or later. Maven multi-module projects no longer require - binding `jib:build` or `jib:dockerBuild` to the _package_ phase. + binding `jib:build` or `jib:dockerBuild` to the _package_ phase and should be removed. # v0.33.0 Release - 07/02/2019 diff --git a/docs/content/en/docs/how-tos/builders/_index.md b/docs/content/en/docs/how-tos/builders/_index.md index 2a7385512b8..7f1cba38edc 100755 --- a/docs/content/en/docs/how-tos/builders/_index.md +++ b/docs/content/en/docs/how-tos/builders/_index.md @@ -157,7 +157,7 @@ without a Docker daemon. Skaffold can help build artifacts using Jib; Jib builds the container images and then pushes them to the local Docker daemon or to remote registries as instructed by Skaffold. -Skaffold requires using Jib v1.3.0 or later. +Skaffold requires using Jib v1.4.0 or later. ### Configuration @@ -202,7 +202,7 @@ produce a container image. Then for each such module: {{% alert title="Updating from earlier versions" %}} Skaffold had required Maven multi-module projects bind a Jib `build` or `dockerBuild` goal to the *package* phase. These bindings are -no longer required with Jib 1.3.0 and should be removed. +no longer required with Jib 1.4.0 and should be removed. {{% /alert %}} #### Gradle From 7d2bc5af1a76951a958a60a93c5da4b093a39aa6 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 19 Jul 2019 13:19:56 -0400 Subject: [PATCH 16/22] update comment --- integration/testdata/jib/skaffold.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/integration/testdata/jib/skaffold.yaml b/integration/testdata/jib/skaffold.yaml index b4a2f8ab97b..507a59e8525 100644 --- a/integration/testdata/jib/skaffold.yaml +++ b/integration/testdata/jib/skaffold.yaml @@ -7,11 +7,12 @@ build: args: - -q -# optional profile to run the jib build on Google Cloud Build profiles: + # optional profile to run the jib build on Google Cloud Build - name: gcb build: googleCloudBuild: {} + # profile to check that an unsupported Jib version fails the build - name: old-jib patches: - op: add From a86d68603015caf68eba3a330eb02a4754406f65 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 19 Jul 2019 13:22:45 -0400 Subject: [PATCH 17/22] where are these versions coming from!? --- pkg/skaffold/jib/jib_maven_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/skaffold/jib/jib_maven_test.go b/pkg/skaffold/jib/jib_maven_test.go index 160f8cfab6f..6f71c47ead5 100644 --- a/pkg/skaffold/jib/jib_maven_test.go +++ b/pkg/skaffold/jib/jib_maven_test.go @@ -66,20 +66,20 @@ func TestGetDependenciesMaven(t *testing.T) { }, { description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}", build, dep1), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}", build, dep1), modTime: time.Unix(0, 0), expected: []string{"build", "dep1"}, }, { // Expected output differs from stdout since build file hasn't change, thus maven command won't run description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), modTime: time.Unix(0, 0), expected: []string{"build", "dep1"}, }, { description: "success", - stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"version\":\"1.3.0\",\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), + stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\", \"%s\"],\"ignore\":[]}", build, dep1, dep2), modTime: time.Unix(10000, 0), expected: []string{"build", "dep1", "dep2"}, }, From b6ce9d9eb256d7d56cba94d4fe78119f619ce031 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 19 Jul 2019 13:25:01 -0400 Subject: [PATCH 18/22] revert change to use .Wrapf --- pkg/skaffold/jib/jib.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/skaffold/jib/jib.go b/pkg/skaffold/jib/jib.go index f8cc09d73fa..04062797718 100644 --- a/pkg/skaffold/jib/jib.go +++ b/pkg/skaffold/jib/jib.go @@ -109,7 +109,7 @@ func getDependencies(workspace string, cmd exec.Cmd, projectName string) ([]stri func refreshDependencyList(files *filesLists, cmd exec.Cmd) error { stdout, err := util.RunCmdOut(&cmd) if err != nil { - return errors.Wrapf(err, "failed to get Jib dependencies") + return errors.Wrap(err, "failed to get Jib dependencies") } // Search for Jib's output JSON. Jib's Maven/Gradle output takes the following form: From 4c0ef770c3e5aae596791cd5b1083e4f761e8c66 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Fri, 19 Jul 2019 15:41:20 -0400 Subject: [PATCH 19/22] happy lint --- integration/build_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/integration/build_test.go b/integration/build_test.go index 9f871516493..2475308d5d8 100644 --- a/integration/build_test.go +++ b/integration/build_test.go @@ -191,7 +191,6 @@ func TestExpectedBuildFailures(t *testing.T) { dir string filename string args []string - env []string gcpOnly bool }{ { From a09ef5059987f240a20f18e8e2fe3463841e4a38 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Wed, 24 Jul 2019 09:23:25 -0400 Subject: [PATCH 20/22] review nits --- integration/build_test.go | 11 +---------- pkg/skaffold/jib/jib_test.go | 22 +++++++++++----------- 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/integration/build_test.go b/integration/build_test.go index 2475308d5d8..0b011ec899d 100644 --- a/integration/build_test.go +++ b/integration/build_test.go @@ -189,9 +189,7 @@ func TestExpectedBuildFailures(t *testing.T) { tests := []struct { description string dir string - filename string args []string - gcpOnly bool }{ { description: "jib is too old", @@ -201,14 +199,7 @@ func TestExpectedBuildFailures(t *testing.T) { } for _, test := range tests { t.Run(test.description, func(t *testing.T) { - if test.gcpOnly && !ShouldRunGCPOnlyTests() { - t.Skip("skipping gcp only test") - } - if !test.gcpOnly && ShouldRunGCPOnlyTests() { - t.Skip("skipping test that is not gcp only") - } - - err := skaffold.Build(test.args...).WithConfig(test.filename).InDir(test.dir).Run(t) + err := skaffold.Build(test.args...).InDir(test.dir).Run(t) if err == nil { t.Fatal("expected failure") } diff --git a/pkg/skaffold/jib/jib_test.go b/pkg/skaffold/jib/jib_test.go index 976ac5ea054..f2ee3a26842 100644 --- a/pkg/skaffold/jib/jib_test.go +++ b/pkg/skaffold/jib/jib_test.go @@ -36,68 +36,68 @@ func TestGetDependencies(t *testing.T) { dep3 := tmpDir.Path("dep3") tests := []struct { - name string + description string stdout string shouldErr bool expectedDeps []string }{ { - name: "empty", + description: "empty", stdout: "", shouldErr: true, expectedDeps: nil, }, { - name: "base case", + description: "base case", stdout: "BEGIN JIB JSON\n{\"build\":[],\"inputs\":[],\"ignore\":[]}", shouldErr: false, expectedDeps: nil, }, { - name: "file in build and file in input", + description: "file in build and file in input", stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[]}\n", dep1, dep2), shouldErr: false, expectedDeps: []string{"dep1", "dep2"}, }, { - name: "dir in input should be expanded", + description: "dir in input should be expanded", stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[],\"inputs\":[\"%s\"],\"ignore\":[]}\n", dep3), shouldErr: false, expectedDeps: []string{filepath.FromSlash("dep3/fileA"), filepath.FromSlash("dep3/sub/path/fileB")}, }, { - name: "files and dir in input should be expanded", + description: "files and dir in input should be expanded", stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[],\"inputs\":[\"%s\",\"%s\",\"%s\"],\"ignore\":[]}\n", dep1, dep2, dep3), shouldErr: false, expectedDeps: []string{"dep1", "dep2", filepath.FromSlash("dep3/fileA"), filepath.FromSlash("dep3/sub/path/fileB")}, }, { - name: "non-existing files should be ignored", + description: "non-existing files should be ignored", stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[],\"inputs\":[\"%s\",\"%s\",\"nonexistent\",\"%s\"],\"ignore\":[]}\n", dep1, dep2, dep3), shouldErr: false, expectedDeps: []string{"dep1", "dep2", filepath.FromSlash("dep3/fileA"), filepath.FromSlash("dep3/sub/path/fileB")}, }, { - name: "ignored files should not be reported", + description: "ignored files should not be reported", stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[],\"inputs\":[\"%s\",\"%s\"],\"ignore\":[\"%s\"]}\n", dep1, dep2, dep2), shouldErr: false, expectedDeps: []string{"dep1"}, }, { - name: "ignored directories should not be reported", + description: "ignored directories should not be reported", stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\"],\"inputs\":[\"%s\"],\"ignore\":[\"%s\",\"%s\"]}\n", dep1, dep3, dep1, dep3), shouldErr: false, expectedDeps: nil, }, { - name: "partial subpaths should be ignored", + description: "partial subpaths should be ignored", stdout: fmt.Sprintf("BEGIN JIB JSON\n{\"build\":[\"%s\",\"%s\",\"%s\"],\"inputs\":[],\"ignore\":[\"%s\"]}\n", dep1, dep2, dep3, tmpDir.Path("dep3/sub/path")), shouldErr: false, expectedDeps: []string{"dep1", "dep2", filepath.FromSlash("dep3/fileA")}, }, } for _, test := range tests { - testutil.Run(t, test.name, func(t *testutil.T) { + testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, t.FakeRunOut("ignored", test.stdout)) results, err := getDependencies(tmpDir.Root(), exec.Cmd{Args: []string{"ignored"}, Dir: tmpDir.Root()}, util.RandomID()) From 7851f7b5bd9562cc5a83a55fa669bf7a51ada0c4 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Wed, 24 Jul 2019 09:52:39 -0400 Subject: [PATCH 21/22] Improve TestExpectedFailure to look for specific error message --- integration/build_test.go | 12 +++++++++--- integration/skaffold/helper.go | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/integration/build_test.go b/integration/build_test.go index 0b011ec899d..d6943f541e4 100644 --- a/integration/build_test.go +++ b/integration/build_test.go @@ -29,6 +29,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/docker/docker/api/types" + "github.com/sirupsen/logrus" ) const imageName = "simple-build:" @@ -190,18 +191,23 @@ func TestExpectedBuildFailures(t *testing.T) { description string dir string args []string + expected string }{ { description: "jib is too old", dir: "testdata/jib", args: []string{"-p", "old-jib"}, + // test string will need to be updated for the jib.requiredVersion error text when moving to Jib > 1.4.0 + expected: "Could not find goal '_skaffold-fail-if-jib-out-of-date' in plugin com.google.cloud.tools:jib-maven-plugin:1.3.0", }, } for _, test := range tests { t.Run(test.description, func(t *testing.T) { - err := skaffold.Build(test.args...).InDir(test.dir).Run(t) - if err == nil { - t.Fatal("expected failure") + if out, err := skaffold.Build(test.args...).InDir(test.dir).RunWithCombinedOutput(t); err == nil { + t.Fatal("expected build to fail") + } else if !strings.Contains(string(out), test.expected) { + logrus.Info("build output: ", string(out)) + t.Fatalf("build failed but for wrong reason") } }) } diff --git a/integration/skaffold/helper.go b/integration/skaffold/helper.go index e259d0fad8e..e89e23cdcc1 100644 --- a/integration/skaffold/helper.go +++ b/integration/skaffold/helper.go @@ -172,6 +172,24 @@ func (b *RunBuilder) Run(t *testing.T) error { return nil } +// RunWithCombinedOutput runs the skaffold command and returns the combined standard output and error. +func (b *RunBuilder) RunWithCombinedOutput(t *testing.T) ([]byte, error) { + t.Helper() + + cmd := b.cmd(context.Background()) + cmd.Stdout, cmd.Stderr = nil, nil + logrus.Infoln(cmd.Args) + + start := time.Now() + out, err := cmd.CombinedOutput() + if err != nil { + return out, errors.Wrapf(err, "skaffold %s", b.command) + } + + logrus.Infoln("Ran in", time.Since(start)) + return out, nil +} + // RunOrFailOutput runs the skaffold command and fails the test // if the command returns an error. // It only returns the standard output. From f32f8df2e589075bfa193cf0e96fe1687fe3a8c5 Mon Sep 17 00:00:00 2001 From: Brian de Alwis Date: Wed, 24 Jul 2019 11:48:23 -0400 Subject: [PATCH 22/22] Mark build test as non-GCP specific --- integration/build_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/integration/build_test.go b/integration/build_test.go index d6943f541e4..8218048b711 100644 --- a/integration/build_test.go +++ b/integration/build_test.go @@ -186,6 +186,9 @@ func TestExpectedBuildFailures(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } + if ShouldRunGCPOnlyTests() { + t.Skip("skipping test that is not gcp only") + } tests := []struct { description string @@ -197,8 +200,8 @@ func TestExpectedBuildFailures(t *testing.T) { description: "jib is too old", dir: "testdata/jib", args: []string{"-p", "old-jib"}, - // test string will need to be updated for the jib.requiredVersion error text when moving to Jib > 1.4.0 expected: "Could not find goal '_skaffold-fail-if-jib-out-of-date' in plugin com.google.cloud.tools:jib-maven-plugin:1.3.0", + // test string will need to be updated for the jib.requiredVersion error text when moving to Jib > 1.4.0 }, } for _, test := range tests {