Improve Maven/Jib multimodule builds between Minikube and remote clusters

CHANGELOG.md

@@ -1,3 +1,9 @@
+# v0.XX.X Release - MM/DD/YYYY
+
+*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 and should be removed.
+
 # v0.34.0 Release - 07/19/2019
 
 *Note*: This release comes with a new config version `v1beta13`.

docs/content/en/docs/how-tos/builders/_index.md

@@ -157,6 +157,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.4.0 or later.
+
 ### Configuration
 
 To use Jib, add a `jibMaven` or `jibGradle` field to each artifact you specify in the
@@ -191,22 +193,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.4.0 and should be removed.
+{{% /alert %}}
 
 #### Gradle
 

integration/build_test.go

@@ -179,3 +179,39 @@ 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
+		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")
+			}
+		})
+	}
+}

integration/examples/jib-multimodule/pom.xml

@@ -18,6 +18,7 @@
 
   <properties>
     <java.version>1.8</java.version>
+    <jib.maven-plugin-version>1.4.0</jib.maven-plugin-version>
   </properties>
 
   <dependencies>
@@ -38,7 +39,7 @@
         <plugin>
           <groupId>com.google.cloud.tools</groupId>
           <artifactId>jib-maven-plugin</artifactId>
-          <version>1.4.0</version>
+          <version>${jib.maven-plugin-version}</version>
         </plugin>
       </plugins>
     </pluginManagement>

integration/examples/jib-multimodule/project1/pom.xml

@@ -36,16 +36,6 @@
                         </jvmFlags>
                     </container>
                 </configuration>
-                <executions>
-                    <!-- Skaffold multi-module with Jib requires a jib:xxx goal be bound to the package phase -->
-                    <execution>
-                        <id>default-package</id>
-                        <phase>package</phase>
-                        <goals>
-                            <goal>build</goal>
-                        </goals>
-                    </execution>
-                </executions>
             </plugin>
         </plugins>
     </build>

integration/examples/jib-multimodule/project2/pom.xml

@@ -36,16 +36,6 @@
                         </jvmFlags>
                     </container>
                 </configuration>
-                <executions>
-                    <!-- Skaffold multi-module with Jib requires a jib:xxx goal be bound to the package phase -->
-                    <execution>
-                        <id>default-package</id>
-                        <phase>package</phase>
-                        <goals>
-                            <goal>build</goal>
-                        </goals>
-                    </execution>
-                </executions>
             </plugin>
         </plugins>
     </build>

integration/examples/jib/pom.xml

@@ -9,6 +9,7 @@
 
     <properties>
         <java.version>1.8</java.version>
+        <jib.maven-plugin-version>1.4.0</jib.maven-plugin-version>
     </properties>
 
     <parent>
@@ -34,7 +35,7 @@
             <plugin>
                 <groupId>com.google.cloud.tools</groupId>
                 <artifactId>jib-maven-plugin</artifactId>
-                <version>1.4.0</version>
+                <version>${jib.maven-plugin-version}</version>
                 <configuration>
                     <container>
                         <jvmFlags>

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",

integration/testdata/jib/pom.xml

@@ -9,6 +9,7 @@
 
     <properties>
         <java.version>1.8</java.version>
+        <jib.maven-plugin-version>1.4.0</jib.maven-plugin-version>
     </properties>
 
     <parent>
@@ -34,7 +35,7 @@
             <plugin>
                 <groupId>com.google.cloud.tools</groupId>
                 <artifactId>jib-maven-plugin</artifactId>
-                <version>1.4.0</version>
+                <version>${jib.maven-plugin-version}</version>
                 <configuration>
                     <container>
                         <jvmFlags>

integration/testdata/jib/skaffold.yaml

@@ -7,8 +7,14 @@ 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
+      path: /build/artifacts/0/jibMaven/args
+      value: ['-Djib.maven-plugin-version=1.3.0']

pkg/skaffold/build/gcb/jib_test.go

@@ -19,6 +19,7 @@ package gcb
 import (
 	"testing"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/jib"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 	cloudbuild "google.golang.org/api/cloudbuild/v1"
@@ -29,8 +30,8 @@ func TestJibMavenBuildSteps(t *testing.T) {
 		skipTests bool
 		args      []string
 	}{
-		{false, []string{"-Djib.console=plain", "--non-recursive", "prepare-package", "jib:dockerBuild", "-Dimage=img"}},
-		{true, []string{"-Djib.console=plain", "--non-recursive", "-DskipTests=true", "prepare-package", "jib:dockerBuild", "-Dimage=img"}},
+		{false, []string{"-Djib.console=plain", "jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + jib.MinimumJibMavenVersion, "--non-recursive", "prepare-package", "jib:dockerBuild", "-Dimage=img"}},
+		{true, []string{"-Djib.console=plain", "jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + jib.MinimumJibMavenVersion, "--non-recursive", "-DskipTests=true", "prepare-package", "jib:dockerBuild", "-Dimage=img"}},
 	}
 	for _, test := range tests {
 		artifact := &latest.Artifact{
@@ -63,8 +64,8 @@ func TestJibGradleBuildSteps(t *testing.T) {
 		skipTests bool
 		args      []string
 	}{
-		{false, []string{"-Djib.console=plain", ":jibDockerBuild", "--image=img"}},
-		{true, []string{"-Djib.console=plain", ":jibDockerBuild", "--image=img", "-x", "test"}},
+		{false, []string{"-Djib.console=plain", "_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + jib.MinimumJibGradleVersion, ":jibDockerBuild", "--image=img"}},
+		{true, []string{"-Djib.console=plain", "_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + jib.MinimumJibGradleVersion, ":jibDockerBuild", "--image=img", "-x", "test"}},
 	}
 	for _, test := range tests {
 		artifact := &latest.Artifact{

pkg/skaffold/build/local/jib_gradle_test.go

@@ -22,6 +22,7 @@ import (
 	"testing"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/jib"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
@@ -39,22 +40,22 @@ func TestBuildJibGradleToDocker(t *testing.T) {
 		{
 			description: "build",
 			artifact:    &latest.JibGradleArtifact{},
-			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain :jibDockerBuild --image=img:tag"),
+			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion="+jib.MinimumJibGradleVersion+" :jibDockerBuild --image=img:tag"),
 		},
 		{
 			description: "build with additional flags",
 			artifact:    &latest.JibGradleArtifact{Flags: []string{"--flag1", "--flag2"}},
-			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain :jibDockerBuild --image=img:tag --flag1 --flag2"),
+			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion="+jib.MinimumJibGradleVersion+" :jibDockerBuild --image=img:tag --flag1 --flag2"),
 		},
 		{
 			description: "build with project",
 			artifact:    &latest.JibGradleArtifact{Project: "project"},
-			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain :project:jibDockerBuild --image=img:tag"),
+			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion="+jib.MinimumJibGradleVersion+" :project:jibDockerBuild --image=img:tag"),
 		},
 		{
 			description:   "fail build",
 			artifact:      &latest.JibGradleArtifact{},
-			cmd:           testutil.FakeRunErr(t, "gradle -Djib.console=plain :jibDockerBuild --image=img:tag", errors.New("BUG")),
+			cmd:           testutil.FakeRunErr(t, "gradle -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion="+jib.MinimumJibGradleVersion+" :jibDockerBuild --image=img:tag", errors.New("BUG")),
 			shouldErr:     true,
 			expectedError: "gradle build failed",
 		},
@@ -95,22 +96,22 @@ func TestBuildJibGradleToRegistry(t *testing.T) {
 		{
 			description: "remote build",
 			artifact:    &latest.JibGradleArtifact{},
-			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain :jib --image=img:tag"),
+			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion="+jib.MinimumJibGradleVersion+" :jib --image=img:tag"),
 		},
 		{
 			description: "build with additional flags",
 			artifact:    &latest.JibGradleArtifact{Flags: []string{"--flag1", "--flag2"}},
-			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain :jib --image=img:tag --flag1 --flag2"),
+			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion="+jib.MinimumJibGradleVersion+" :jib --image=img:tag --flag1 --flag2"),
 		},
 		{
 			description: "build with project",
 			artifact:    &latest.JibGradleArtifact{Project: "project"},
-			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain :project:jib --image=img:tag"),
+			cmd:         testutil.FakeRun(t, "gradle -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion="+jib.MinimumJibGradleVersion+" :project:jib --image=img:tag"),
 		},
 		{
 			description:   "fail build",
 			artifact:      &latest.JibGradleArtifact{},
-			cmd:           testutil.FakeRunErr(t, "gradle -Djib.console=plain :jib --image=img:tag", errors.New("BUG")),
+			cmd:           testutil.FakeRunErr(t, "gradle -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion="+jib.MinimumJibGradleVersion+" :jib --image=img:tag", errors.New("BUG")),
 			shouldErr:     true,
 			expectedError: "gradle build failed",
 		},

pkg/skaffold/build/local/jib_maven.go

@@ -36,13 +36,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
@@ -52,13 +45,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
@@ -67,32 +53,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(util.OSEnviron(), b.localDocker.ExtraEnv()...)