Improve Maven/Jib multimodule builds between Minikube and remote clusters by briandealwis · Pull Request #2122 · GoogleContainerTools/skaffold

briandealwis · 2019-05-14T17:46:50Z

Jib for Maven ~~1.3.0~~ 1.4.0 will allow restricting Jib's image-building goals to a specific artifact. This ability means there is no additional package-binding step required for Skaffold-JibMaven projects. Maven/Jib multi-module project will need updating, and these changes are not compatible with previous versions of Jib.

This PR reworks the Jib support to:

check for a minimum Jib version (~~1.3.0~~ 1.4.0)
remove the previous package-goal checks
add documentation

Projects using older versions of Jib will be prompted to update to Jib ~~1.3.0~~ 1.4.0. For versions prior to 1.4.0, the build will fail due to a new goal introduced to check the required version:

$ skaffold dev
FATA[0002] watching files for artifact gcr.io/k8s-skaffold/skaffold-jib: listing files: listing files: getting jibMaven dependencies: initial Jib dependency refresh failed: failed to get Jib dependencies: Running [/Users/bsd/Projects/GPE/repo-skaffold/integration/testdata/jib/mvnw jib:_skaffold-fail-if-jib-out-of-date -Djib.requiredVersion=[1.4.0,) -Djib.maven-plugin-version=1.3.0 --non-recursive jib:_skaffold-files-v2 --quiet]: stdout [ERROR] Could not find goal '_skaffold-fail-if-jib-out-of-date' in plugin com.google.cloud.tools:jib-maven-plugin:1.3.0 among available goals _skaffold-files, _skaffold-files-v2, _skaffold-init, _skaffold-package-goals, build, buildTar, dockerBuild -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoNotFoundException
, stderr: , err: exit status 1: exit status 1

For later versions of Jib, when we bump this required version, they will see failures like:

FATA[0002] watching files for artifact gcr.io/k8s-skaffold/skaffold-jib: listing files: listing files: getting jibMaven dependencies: initial Jib dependency refresh failed: failed to get Jib dependencies: Running [/Users/bsd/Projects/GPE/repo-skaffold/integration/testdata/jib/mvnw jib:_skaffold-fail-if-jib-out-of-date -Djib.requiredVersion=(1.4.0,) -q --non-recursive jib:_skaffold-files-v2 --quiet]: stdout [ERROR] Failed to execute goal com.google.cloud.tools:jib-maven-plugin:1.4.0:_skaffold-fail-if-jib-out-of-date (default-cli) on project hello-spring-boot: Jib plugin version is 1.4.0 but is required to be (1.4.0,) -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException
, stderr: , err: exit status 1: exit status 1

Technically, only Maven multi-module projects require updating to 1.4.0, but for simplicity we're bumping the required version for Gradle too. If a user is willing to switch to a newer version of Skaffold, switching to a newer Jib should not present a problem either.

briandealwis · 2019-05-14T17:47:19Z

PR will break since 1.3.0 is not released.

briandealwis · 2019-05-14T17:48:04Z

To test: I bumped the jib-maven-plugin version to 1.3.0-SNAPSHOT and installed locally.

CHANGELOG.md

examples/jib-multimodule/pom.xml

tejal29 · 2019-05-16T17:39:31Z

The integration test is still failing
--- FAIL: TestRun/jib_in_googlecloudbuild (9.30s) run_test.go:124: skaffold run: exit status 1, Generating tags... - gcr.io/k8s-skaffold/skaffold-jib -> gcr.io/k8s-skaffold/skaffold-jib:v0.29.0-11-gd116eff Tags generated in 18.462803ms Starting build... Building [gcr.io/k8s-skaffold/skaffold-jib]...
Not sure if this is regarding the jib version. Don't see "requires Jib v%s+", minJibVersion". But again, it could be we are not collecting the output correctly.

Thanks
Tejal

briandealwis · 2019-05-16T18:16:44Z

Thanks for taking a look @tejal29 but the tests won't work until we release Jib 1.3.0. I'll update this PR when that happens.

- move example changes to integration/examples - adopt new testutil changes in changes

briandealwis · 2019-06-10T18:25:12Z

Jib 1.3.0 has been released and this is now ready for review.
PTAL @GoogleContainerTools/java-tools-build

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

pkg/skaffold/jib/jib_maven_test.go

loosebazooka · 2019-06-10T18:46:53Z

pkg/skaffold/jib/jib_test.go

+		{"1.2.0", true},
+		{"1.2.0-SNAPSHOT", true},
+		{"1.3.0", false},
+		{"1.3.0-SNAPSHOT", false},


will it work with our potential RCs?

Hmm no. Will fix.

CHANGELOG.md

loosebazooka · 2019-06-11T20:22:15Z

integration/testdata/jib/pom.xml


    <properties>
        <java.version>1.8</java.version>
+        <jib.maven-plugin-version>1.3.0</jib.maven-plugin-version>


should this (and other instances of this) be defined in a parent?

oh jeepers where would we define it?

I don't know. Maybe don't do this. Just wondering.

Could be worth doing in the integration tests but the examples should be standalone. I'll leave it for now: these values were hard-coded previously and making them properties makes testing easier.

loosebazooka

Looks fine to me, should we have a go person go over it?

But also, maybe we should do a jib team experiment on a local build of this. Testing out all scenarios since we expect some breakages for existing users?

briandealwis · 2019-07-18T21:40:42Z

Jib 1.4.0 has been released with the new requiredVersion functionality. I added a test to verify that a build will fail when the Jib version is too old. PTAL.

CHANGELOG.md

chanseokoh · 2019-07-19T14:43:49Z

Do you think the Kokoro failure is related?

     --- FAIL: TestRun/jib_in_googlecloudbuild (9.12s)
        run_test.go:130: skaffold run: exit status 1, Generating tags...
             - gcr.io/k8s-skaffold/skaffold-jib -> gcr.io/k8s-skaffold/skaffold-jib:v0.33.0-151-gde5bc34
            Tags generated in 21.036902ms
            Starting build...
            Building [gcr.io/k8s-skaffold/skaffold-jib]...

briandealwis · 2019-07-19T15:08:26Z

I wouldn't have thought so, but that test output is not helpful :-(

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

CHANGELOG.md

integration/testdata/jib/skaffold.yaml

pkg/skaffold/build/local/jib_maven_test.go

pkg/skaffold/jib/jib.go

chanseokoh · 2019-07-19T15:57:29Z

pkg/skaffold/jib/jib_gradle.go

 )

+// Skaffold-Jib depends on functionality introduced after Jib-Gradle 1.4.0
+const MinimumJibGradleVersion = "(1.4.0,)"


Huh, shouldn't this be [1.4.0,) (i.e., includes 1.4.0)? But tests are not failing??

pkg/skaffold/jib/jib_gradle_test.go

chanseokoh · 2019-07-19T16:04:27Z

pkg/skaffold/jib/jib_maven.go

 )

+// Skaffold-Jib depends on functionality introduced after Jib-Maven 1.4.0
+const MinimumJibMavenVersion = "(1.4.0,)"


briandealwis · 2019-07-19T16:16:24Z

Thanks for the details look @chanseokoh — you caught a bunch of things. The GCB failure is related: the version range should be left-closed (:blush:).

briandealwis · 2019-07-23T19:32:54Z

PTAL @balopat @dgageot

integration/build_test.go

dgageot · 2019-07-24T09:55:07Z

integration/build_test.go

+				t.Skip("skipping test that is not gcp only")
+			}
+
+			err := skaffold.Build(test.args...).WithConfig(test.filename).InDir(test.dir).Run(t)


I think .RunOrFail does that

RunOrFail fails the test on error, whereas the error is a sign of success here.

I updated this test to check for expected error text as it would spuriously pass if (say) the wrong skaffold version was picked up ("Your Skaffold version might be too old").

pkg/skaffold/jib/jib_test.go

integration/build_test.go

briandealwis added 3 commits May 14, 2019 12:19

Check minimum Jib version

2ae6221

Improve Maven/Jib multimodule builds

67cf35b

Add documentation

fc58ef7

briandealwis requested a review from TadCordle May 14, 2019 17:46

googlebot added the cla: yes label May 14, 2019

chanseokoh reviewed May 15, 2019

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

examples/jib-multimodule/pom.xml Outdated Show resolved Hide resolved

briandealwis added 3 commits May 15, 2019 13:56

review nits

803be22

Jib 1.0.2 -> 1.3.0

c4dda08

integration test too

d116eff

briandealwis added the build/jib label May 15, 2019

Merge remote-tracking branch 'origin/master' into HEAD

043e941

- move example changes to integration/examples - adopt new testutil changes in changes

briandealwis marked this pull request as ready for review June 10, 2019 18:24

briandealwis requested review from balopat, dgageot, loosebazooka, nkubala, priyawadhwa and tejal29 as code owners June 10, 2019 18:24

loosebazooka reviewed Jun 10, 2019

View reviewed changes

Fix checkJibVersion to support RCs

83127c1

chanseokoh previously approved these changes Jun 11, 2019

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

loosebazooka reviewed Jun 11, 2019

View reviewed changes

Remove stray conflict marker

d52c3cf

loosebazooka reviewed Jun 12, 2019

View reviewed changes

briandealwis added the kokoro:run runs the kokoro jobs on a PR label Jun 13, 2019

briandealwis added 2 commits July 18, 2019 17:03

merge with HEAD

a544ab3

Bump minimum Jib versions to 1.4.0, and add test to check too-old

c8fa4f6

briandealwis removed the !! blocked !! this issue/PR is blocked by another issue label Jul 18, 2019

chanseokoh reviewed Jul 18, 2019

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

briandealwis added 2 commits July 18, 2019 20:04

Update CHANGELOG.md

dd16386

fix tests

de5bc34

chanseokoh reviewed Jul 19, 2019

View reviewed changes

briandealwis added 8 commits July 19, 2019 12:44

back out accidental commit

9384ea3

remove accidentally reintroduced version headers

ccd52ce

Fix version bounds

d784215

Doc nits

ead1a95

update comment

7d2bc5a

where are these versions coming from!?

a86d686

revert change to use .Wrapf

b6ce9d9

happy lint

4c0ef77

chanseokoh approved these changes Jul 22, 2019

View reviewed changes

merge with HEAD

1daa2c2

dgageot suggested changes Jul 24, 2019

View reviewed changes

briandealwis added 2 commits July 24, 2019 09:23

review nits

a09ef50

Improve TestExpectedFailure to look for specific error message

7851f7b

dgageot suggested changes Jul 24, 2019

View reviewed changes

integration/build_test.go Show resolved Hide resolved

Mark build test as non-GCP specific

f32f8df

dgageot approved these changes Jul 24, 2019

View reviewed changes

dgageot merged commit 7013ed2 into GoogleContainerTools:master Jul 24, 2019

briandealwis deleted the i1876-jibmm branch November 11, 2019 20:24

Conversation

briandealwis commented May 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

briandealwis commented May 14, 2019

Uh oh!

briandealwis commented May 14, 2019

Uh oh!

Uh oh!

Uh oh!

tejal29 commented May 16, 2019

Uh oh!

briandealwis commented May 16, 2019

Uh oh!

briandealwis commented Jun 10, 2019

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

loosebazooka left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briandealwis commented Jul 18, 2019

Uh oh!

Uh oh!

chanseokoh commented Jul 19, 2019

Uh oh!

briandealwis commented Jul 19, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

briandealwis commented Jul 19, 2019

Uh oh!

briandealwis commented Jul 23, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

briandealwis commented May 14, 2019 •

edited

Loading

loosebazooka left a comment •

edited

Loading