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

Output logs in color for parallel builds #5014

Merged

Conversation

IsaacPD
Copy link
Contributor

@IsaacPD IsaacPD commented Nov 10, 2020

When building in parallel, logs would not show up with colors for builders that had colorful logs.

Also Fixes: #4913
Related: #4990

Description
Wrap the output writer that is passed through skaffold with a "colorful writer" struct. This allows detecting when we should enable printing with colors at any point by checking in color.Fprint(f/ln) if it is a colorful writer.

Because the writer is now wrapped in a struct we are no longer passing os.Stdout raw and so when running commands it will be necessary to force enable/disable coloring with flags when appropriate.

User facing changes
Logs that normally show up in color in a non-parallel build are also in color for builds done in parallel.

Before (master):
Parallel build containing a jib maven artifact
Screen Shot 2020-11-10 at 1 10 47 PM (2)

Running skaffold run --mute-logs=all in skaffold/examples/ruby

...
Starting deploy...
- writing logs to /var/.../skaffold/deploy/2020-11-02_18-49-06.log
...

In /var/.../skaffold/deploy/2020-11-02_18-49-06.log

^[[34mStarting deploy...^[[0m
 - service/ruby configured
 - deployment.apps/ruby configured

The logs contain ANSI escape codes which do not represent the actual content intended to be logged.

After:
Parallel build containing a jib maven artifact
Screen Shot 2020-11-10 at 1 12 28 PM (2)

Running skaffold run --mute-logs=all in skaffold/examples/ruby

...
Starting deploy...
- writing logs to /var/.../skaffold/deploy/2020-11-02_18-53-51.log
...

In /var/.../skaffold/deploy/2020-11-02_18-53-51.log

Starting deploy...
 - service/ruby configured
 - deployment.apps/ruby configured

The only thing logged is the intended output.

@codecov
Copy link

codecov bot commented Nov 10, 2020

Codecov Report

Merging #5014 (44512a4) into master (3a73d83) will increase coverage by 0.06%.
The diff coverage is 82.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5014      +/-   ##
==========================================
+ Coverage   72.11%   72.17%   +0.06%     
==========================================
  Files         365      365              
  Lines       12773    12823      +50     
==========================================
+ Hits         9211     9255      +44     
- Misses       2877     2879       +2     
- Partials      685      689       +4     
Impacted Files Coverage Δ
pkg/skaffold/build/bazel/build.go 67.24% <25.00%> (-3.13%) ⬇️
pkg/skaffold/build/result.go 83.33% <50.00%> (-3.34%) ⬇️
cmd/skaffold/app/cmd/cmd.go 65.89% <100.00%> (ø)
pkg/skaffold/build/gcb/jib.go 84.61% <100.00%> (ø)
pkg/skaffold/build/jib/gradle.go 100.00% <100.00%> (ø)
pkg/skaffold/build/jib/maven.go 100.00% <100.00%> (ø)
pkg/skaffold/color/formatter.go 100.00% <100.00%> (+10.00%) ⬆️
pkg/skaffold/docker/parse.go 84.06% <0.00%> (-1.41%) ⬇️
pkg/skaffold/errors/errors.go 97.36% <0.00%> (+0.49%) ⬆️
pkg/skaffold/docker/dependencies.go 75.00% <0.00%> (+2.69%) ⬆️
... and 1 more

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 3a73d83...44512a4. Read the comment docs.

@gsquared94 gsquared94 self-assigned this Nov 10, 2020
@IsaacPD IsaacPD added kokoro:run runs the kokoro jobs on a PR kokoro:force-run forces a kokoro re-run on a PR and removed kokoro:run runs the kokoro jobs on a PR labels Nov 10, 2020
@kokoro-team kokoro-team removed the kokoro:force-run forces a kokoro re-run on a PR label Nov 11, 2020
@tejal29
Copy link
Member

tejal29 commented Nov 11, 2020

Verified this locally

  1. --mute-logs=false
  2. building jib artifacts in parallel (master VS on branch)

pkg/skaffold/build/gcb/jib.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@gsquared94 gsquared94 left a comment

Choose a reason for hiding this comment

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

lgtm!

@tejal29 tejal29 merged commit 1a90903 into GoogleContainerTools:master Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Output logs from --mute-logs contain ANSI sequences
4 participants