-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add support for building projects using jib #1073
Add support for building projects using jib #1073
Conversation
// Maven and Gradle projects often provide a wrapper to ensure a particular | ||
// builder version is used. This function tries to resolve a wrapper | ||
// or otherwise resolves the builder executable. | ||
func findBuilder(builderExecutable string, wrapperScriptName string, workspace string) ([]string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why return an slice of strings? only one string is returned, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to add some logs here to help debug a setup.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Windows the wrappers are batch files, and need to be run with an explicit "cmd.exe /c wrapper.bat"
Codecov Report
@@ Coverage Diff @@
## jib_skaffold #1073 +/- ##
================================================
- Coverage 43.25% 43.22% -0.04%
================================================
Files 78 81 +3
Lines 3299 3357 +58
================================================
+ Hits 1427 1451 +24
- Misses 1739 1773 +34
Partials 133 133
Continue to review full report at Codecov.
|
PTAL @GoogleContainerTools/container-tools |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good, left organizational/nits comments.
I think we should merge #1058 first and then reuse what we can from there here.
pkg/skaffold/build/local/jib.go
Outdated
) | ||
|
||
func (b *Builder) buildJibMaven(_ /*ctx*/ context.Context, _ /*out*/ io.Writer, _ /*workspace*/ string, _ /*a*/ *v1alpha3.JibMavenArtifact) (string, error) { | ||
return "", errors.New("buildJibMaven is unimplemented") | ||
func (b *Builder) buildJibMaven(ctx context.Context, out io.Writer, workspace string, a *v1alpha3.JibMavenArtifact) (string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should pull out maven related stuff to jib_maven.go
and gradle methods to jib_gradle.go
?
Sorry @briandealwis this needs to be rebased |
I left a few comments on things that could be fixed in another PR. Is tested the feature and it works! |
Adds support for building projects using jib for maven or gradle. The implementation invokes
jib:dockerBuild
(Maven) /:jibDockerBuild
(Gradle) to build to the local docker daemon using a predictable image name derived from theworkspace
andmodule
/project
settings.Currently the internal image names are of the form
jib<workspace>
if not project/module is specified and theworkspace
is a valid image namejib<workspace_project
if project/module is specified and theworkspace_project
is a valid image namejib__<hex-sha1>
where<hex_sha1>
is the sha1 of<workspace>
or<workspace>_<project>
(if a project/module is specified)But maybe these should have a fixed image name of
jib_skaffold
and instead specify the above as an image tag.Supports:
Does not support: