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

Configure Jib builds to use plain progress updates #1895

Merged
merged 2 commits into from
Apr 2, 2019

Conversation

briandealwis
Copy link
Member

Fixes GoogleContainerTools/jib#1430

This PR changes Skaffold's Jib binding to invoke Maven and Gradle with -Djib.console=plain to cause Jib to use its impoverished plain progress updater.

Jib provides a rich progress updater that uses ANSI cursor sequences to provide a more GUI-like build monitor. Although we attempt to log these sequence strings as a single output, there can be interference if there is simultaneous output. This rich progress updater is automatically disabled on Jib-Maven when invoked from Skaffold, and this patch explicitly disables Jib's rich progress updater on Jib-Gradle too.

The use of jib.console is a compromise. Although Maven and Gradle provide options to run in a batch mode, doing so disables all colour output which is actually very useful. These batch modes can be enabled from the args field in the skaffold.yaml, and are automatically set when console output is redirected to a log file.

@dgageot
Copy link
Contributor

dgageot commented Mar 29, 2019

For the record, Skaffold will never interleave build outputs.

  • Local builds are always in sequence
  • Remote builds are run in parallel but their output is displayed in sequence.

@codecov-io
Copy link

Codecov Report

Merging #1895 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1895      +/-   ##
==========================================
+ Coverage   51.45%   51.46%   +<.01%     
==========================================
  Files         171      171              
  Lines        7690     7691       +1     
==========================================
+ Hits         3957     3958       +1     
  Misses       3357     3357              
  Partials      376      376
Impacted Files Coverage Δ
pkg/skaffold/jib/jib_maven.go 100% <100%> (ø) ⬆️
pkg/skaffold/jib/jib_gradle.go 100% <100%> (ø) ⬆️

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 6ea8c06...76921d6. Read the comment docs.

@briandealwis
Copy link
Member Author

I was going on this TODO and #1380.

// TODO(dgageot): parallel builds
return build.InSequence(ctx, out, tags, artifacts, b.buildArtifact)

Aside: If Skaffold ever does allow parallel local builds, it would be good to let the builders provide an indication of their support: I'm not sure if Maven and Gradle can really do parallel builds within the same workspace such as for multi-module type builds — I think there may be interference if there are shared project dependencies that needs to be rebuilt.

At the very least, this patch brings consistency between the two jib builders. I'm not sure how Skaffold drains the builder logs, but there is a possibility that the Maven/Gradle side may buffer output.

@briandealwis
Copy link
Member Author

I didn't quite finish that last thought: _ I'm not sure how Skaffold drains the builder logs, but there is a possibility that the Maven/Gradle side may buffer output_ and so the log string may only be partially output in multiple parts rather than as a single output.

@chanseokoh
Copy link
Member

@dgageot this PR is self-contained and good to go anyway.

@nkubala nkubala merged commit 36946d2 into GoogleContainerTools:master Apr 2, 2019
@tejal29 tejal29 mentioned this pull request Apr 12, 2019
@briandealwis briandealwis deleted the jibconsole branch August 30, 2019 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants