Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure working JVM before enabling Jib actions to avoid hangs #5725

Merged
merged 10 commits into from
May 3, 2021

Conversation

briandealwis
Copy link
Member

@briandealwis briandealwis commented Apr 26, 2021

Fixes: #5493
Fixes: #5380

Description
macOS provides facades for the common Java executables (e.g., java, javac) that attempt to find and invoke the corresponding commands from a suitable Java VM (normally found in /Library/Java/JavaVirtualMachines). When no Java VM is available, these facades error:

$ java -version
The operation couldn’t be completed. Unable to locate a Java Runtime.
Please visit http://www.java.com for information on installing Java.

But using a Maven Wrapper script (mvnw) instead results in a hang! Tracing through the script shows:

$ sh -x mvnw
...
WRAPPER_LAUNCHER=org.apache.maven.wrapper.MavenWrapperMain
+ exec /usr/bin/java -Xmx1g -classpath /Users/bdealwis/Projects/Skaffold/repo-skaffold/integration/examples/jib/.mvn/wrapper/maven-wrapper.jar -Dmaven.home=/Users/bdealwis/Projects/Skaffold/repo-skaffold/integration/examples -Dmaven.multiModuleProjectDirectory=/Users/bdealwis/Projects/Skaffold/repo-skaffold/integration/examples/jib org.apache.maven.wrapper.MavenWrapperMain

This PR adds a check that a working JVM exists before using Jib.

@codecov
Copy link

codecov bot commented Apr 26, 2021

Codecov Report

Merging #5725 (36bfafa) into master (91c4480) will decrease coverage by 0.03%.
The diff coverage is 35.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5725      +/-   ##
==========================================
- Coverage   70.70%   70.67%   -0.04%     
==========================================
  Files         428      429       +1     
  Lines       16199    16213      +14     
==========================================
+ Hits        11454    11459       +5     
- Misses       3900     3907       +7     
- Partials      845      847       +2     
Impacted Files Coverage Δ
pkg/skaffold/build/jib/init.go 83.05% <0.00%> (-4.45%) ⬇️
pkg/skaffold/build/jib/jib.go 69.46% <0.00%> (-1.08%) ⬇️
pkg/skaffold/build/jib/jvm.go 55.55% <55.55%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 91c4480...36bfafa. Read the comment docs.

Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good! Just have one question

@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 27, 2021
Copy link
Contributor

@MarlonGamez MarlonGamez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, just noticed a tiny inconsistency

Co-authored-by: Marlon Gamez <marlongamez@google.com>
@briandealwis briandealwis enabled auto-merge (squash) April 27, 2021 17:24
@briandealwis
Copy link
Member Author

I hit hanging-tests again this morning when testing a different change on my M1!

PTAL @MarlonGamez: I tweaked the implementation again and this is now working on all platforms.

@briandealwis briandealwis merged commit 9a89e8f into GoogleContainerTools:master May 3, 2021
@briandealwis briandealwis deleted the jibcheck branch May 3, 2021 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants