Output logs in color for parallel builds

cmd/skaffold/app/cmd/cmd.go

@@ -70,7 +70,7 @@ func NewSkaffoldCommand(out, err io.Writer) *cobra.Command {
 
 			opts.Command = cmd.Use
 
-			color.SetupColors(out, defaultColor, forceColors)
+			out = color.SetupColors(out, defaultColor, forceColors)
 			cmd.Root().SetOutput(out)
 
 			// Setup logs

pkg/skaffold/build/bazel/build.go

@@ -26,6 +26,7 @@ import (
 	"path/filepath"
 	"strings"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
@@ -55,6 +56,12 @@ func (b *Builder) buildTar(ctx context.Context, out io.Writer, workspace string,
 	args = append(args, a.BuildArgs...)
 	args = append(args, a.BuildTarget)
 
+	if color.IsColorable(out) {
+		args = append(args, "--color=yes")
+	} else {
+		args = append(args, "--color=no")
+	}
+
 	// FIXME: is it possible to apply b.skipTests?
 	cmd := exec.CommandContext(ctx, "bazel", args...)
 	cmd.Dir = workspace

pkg/skaffold/build/bazel/build_test.go

@@ -30,7 +30,7 @@ import (
 func TestBuildBazel(t *testing.T) {
 	testutil.Run(t, "", func(t *testutil.T) {
 		t.NewTempDir().Mkdir("bin").Chdir()
-		t.Override(&util.DefaultExecCommand, testutil.CmdRun("bazel build //:app.tar").AndRunOut("bazel info bazel-bin", "bin"))
+		t.Override(&util.DefaultExecCommand, testutil.CmdRun("bazel build //:app.tar --color=no").AndRunOut("bazel info bazel-bin", "bin"))
 		testutil.CreateFakeImageTar("bazel:app", "bin/app.tar")
 
 		artifact := &latest.Artifact{

pkg/skaffold/build/gcb/jib.go

@@ -41,15 +41,15 @@ func (b *Builder) jibBuildSpec(artifact *latest.Artifact, tag string) (cloudbuil
 			Steps: []*cloudbuild.BuildStep{{
 				Name:       b.MavenImage,
 				Entrypoint: "sh",
-				Args:       fixHome("mvn", jib.GenerateMavenBuildArgs("build", tag, artifact.JibArtifact, b.skipTests, true, artifact.Dependencies, b.artifactStore, b.cfg.GetInsecureRegistries())),
+				Args:       fixHome("mvn", jib.GenerateMavenBuildArgs(nil, "build", tag, artifact.JibArtifact, b.skipTests, true, artifact.Dependencies, b.artifactStore, b.cfg.GetInsecureRegistries())),
 			}},
 		}, nil
 	case jib.JibGradle:
 		return cloudbuild.Build{
 			Steps: []*cloudbuild.BuildStep{{
 				Name:       b.GradleImage,
 				Entrypoint: "sh",
-				Args:       fixHome("gradle", jib.GenerateGradleBuildArgs("jib", tag, artifact.JibArtifact, b.skipTests, true, artifact.Dependencies, b.artifactStore, b.cfg.GetInsecureRegistries())),
+				Args:       fixHome("gradle", jib.GenerateGradleBuildArgs(nil, "jib", tag, artifact.JibArtifact, b.skipTests, true, artifact.Dependencies, b.artifactStore, b.cfg.GetInsecureRegistries())),
 			}},
 		}, nil
 	default:

pkg/skaffold/build/gcb/jib_test.go

@@ -36,12 +36,12 @@ func TestJibMavenBuildSpec(t *testing.T) {
 		{
 			description:  "skip tests",
 			skipTests:    true,
-			expectedArgs: []string{"-c", "mvn -Duser.home=$$HOME -Djib.console=plain jib:_skaffold-fail-if-jib-out-of-date -Djib.requiredVersion=" + jib.MinimumJibMavenVersion + " --non-recursive -DskipTests=true prepare-package jib:build -Dimage=img"},
+			expectedArgs: []string{"-c", "mvn -Duser.home=$$HOME --batch-mode jib:_skaffold-fail-if-jib-out-of-date -Djib.requiredVersion=" + jib.MinimumJibMavenVersion + " --non-recursive -DskipTests=true prepare-package jib:build -Dimage=img"},
 		},
 		{
 			description:  "do not skip tests",
 			skipTests:    false,
-			expectedArgs: []string{"-c", "mvn -Duser.home=$$HOME -Djib.console=plain jib:_skaffold-fail-if-jib-out-of-date -Djib.requiredVersion=" + jib.MinimumJibMavenVersion + " --non-recursive prepare-package jib:build -Dimage=img"},
+			expectedArgs: []string{"-c", "mvn -Duser.home=$$HOME --batch-mode jib:_skaffold-fail-if-jib-out-of-date -Djib.requiredVersion=" + jib.MinimumJibMavenVersion + " --non-recursive prepare-package jib:build -Dimage=img"},
 		},
 	}
 	for _, test := range tests {
@@ -83,12 +83,12 @@ func TestJibGradleBuildSpec(t *testing.T) {
 		{
 			description:  "skip tests",
 			skipTests:    true,
-			expectedArgs: []string{"-c", "gradle -Duser.home=$$HOME -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion=" + jib.MinimumJibGradleVersion + " :jib -x test --image=img"},
+			expectedArgs: []string{"-c", "gradle -Duser.home=$$HOME --console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion=" + jib.MinimumJibGradleVersion + " :jib -x test --image=img"},
 		},
 		{
 			description:  "do not skip tests",
 			skipTests:    false,
-			expectedArgs: []string{"-c", "gradle -Duser.home=$$HOME -Djib.console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion=" + jib.MinimumJibGradleVersion + " :jib --image=img"},
+			expectedArgs: []string{"-c", "gradle -Duser.home=$$HOME --console=plain _skaffoldFailIfJibOutOfDate -Djib.requiredVersion=" + jib.MinimumJibGradleVersion + " :jib --image=img"},
 		},
 	}
 	for _, test := range tests {

pkg/skaffold/build/jib/gradle.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/sirupsen/logrus"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
@@ -43,7 +44,7 @@ const MinimumJibGradleVersionForSync = "2.0.0"
 var GradleCommand = util.CommandWrapper{Executable: "gradle", Wrapper: "gradlew"}
 
 func (b *Builder) buildJibGradleToDocker(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, deps []*latest.ArtifactDependency, tag string) (string, error) {
-	args := GenerateGradleBuildArgs("jibDockerBuild", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
+	args := GenerateGradleBuildArgs(out, "jibDockerBuild", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
 	if err := b.runGradleCommand(ctx, out, workspace, args); err != nil {
 		return "", err
 	}
@@ -52,7 +53,7 @@ func (b *Builder) buildJibGradleToDocker(ctx context.Context, out io.Writer, wor
 }
 
 func (b *Builder) buildJibGradleToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, deps []*latest.ArtifactDependency, tag string) (string, error) {
-	args := GenerateGradleBuildArgs("jib", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
+	args := GenerateGradleBuildArgs(out, "jib", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
 	if err := b.runGradleCommand(ctx, out, workspace, args); err != nil {
 		return "", err
 	}
@@ -97,8 +98,8 @@ func getSyncMapCommandGradle(ctx context.Context, workspace string, a *latest.Ji
 }
 
 // GenerateGradleBuildArgs generates the arguments to Gradle for building the project as an image.
-func GenerateGradleBuildArgs(task string, imageName string, a *latest.JibArtifact, skipTests, pushImages bool, deps []*latest.ArtifactDependency, r ArtifactResolver, insecureRegistries map[string]bool) []string {
-	args := gradleBuildArgsFunc(task, a, skipTests, true, MinimumJibGradleVersion)
+func GenerateGradleBuildArgs(out io.Writer, task string, imageName string, a *latest.JibArtifact, skipTests, pushImages bool, deps []*latest.ArtifactDependency, r ArtifactResolver, insecureRegistries map[string]bool) []string {
+	args := gradleBuildArgsFunc(task, a, skipTests, color.IsColorable(out), MinimumJibGradleVersion)
 	if insecure, err := isOnInsecureRegistry(imageName, insecureRegistries); err == nil && insecure {
 		// jib doesn't support marking specific registries as insecure
 		args = append(args, "-Djib.allowInsecureRegistries=true")

pkg/skaffold/build/jib/gradle_test.go

@@ -393,7 +393,7 @@ func TestGenerateGradleBuildArgs(t *testing.T) {
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersion))
-			command := GenerateGradleBuildArgs("testTask", test.image, &test.in, test.skipTests, test.pushImages, test.deps, test.r, test.insecureRegistries)
+			command := GenerateGradleBuildArgs(nil, "testTask", test.image, &test.in, test.skipTests, test.pushImages, test.deps, test.r, test.insecureRegistries)
 			t.CheckDeepEqual(test.out, command)
 		})
 	}

pkg/skaffold/build/jib/maven.go

@@ -24,6 +24,7 @@ import (
 
 	"github.com/sirupsen/logrus"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
@@ -43,7 +44,7 @@ const MinimumJibMavenVersionForSync = "2.0.0"
 var MavenCommand = util.CommandWrapper{Executable: "mvn", Wrapper: "mvnw"}
 
 func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, deps []*latest.ArtifactDependency, tag string) (string, error) {
-	args := GenerateMavenBuildArgs("dockerBuild", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
+	args := GenerateMavenBuildArgs(out, "dockerBuild", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
 	if err := b.runMavenCommand(ctx, out, workspace, args); err != nil {
 		return "", err
 	}
@@ -52,7 +53,7 @@ func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, work
 }
 
 func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, deps []*latest.ArtifactDependency, tag string) (string, error) {
-	args := GenerateMavenBuildArgs("build", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
+	args := GenerateMavenBuildArgs(out, "build", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
 	if err := b.runMavenCommand(ctx, out, workspace, args); err != nil {
 		return "", err
 	}
@@ -98,8 +99,8 @@ func getSyncMapCommandMaven(ctx context.Context, workspace string, a *latest.Jib
 }
 
 // GenerateMavenBuildArgs generates the arguments to Maven for building the project as an image.
-func GenerateMavenBuildArgs(goal string, imageName string, a *latest.JibArtifact, skipTests, pushImages bool, deps []*latest.ArtifactDependency, r ArtifactResolver, insecureRegistries map[string]bool) []string {
-	args := mavenBuildArgsFunc(goal, a, skipTests, true, MinimumJibMavenVersion)
+func GenerateMavenBuildArgs(out io.Writer, goal string, imageName string, a *latest.JibArtifact, skipTests, pushImages bool, deps []*latest.ArtifactDependency, r ArtifactResolver, insecureRegistries map[string]bool) []string {
+	args := mavenBuildArgsFunc(goal, a, skipTests, color.IsColorable(out), MinimumJibMavenVersion)
 	if insecure, err := isOnInsecureRegistry(imageName, insecureRegistries); err == nil && insecure {
 		// jib doesn't support marking specific registries as insecure
 		args = append(args, "-Djib.allowInsecureRegistries=true")
@@ -118,7 +119,7 @@ func mavenBuildArgs(goal string, a *latest.JibArtifact, skipTests, showColors bo
 	// but use --batch-mode for internal goals to avoid formatting issues
 	var args []string
 	if showColors {
-		args = []string{"-Djib.console=plain"}
+		args = []string{"-Dstyle.color=always", "-Djansi.passthrough=true", "-Djib.console=plain"}
 	} else {
 		args = []string{"--batch-mode"}
 	}

pkg/skaffold/build/jib/maven_test.go

@@ -380,7 +380,7 @@ func TestGenerateMavenBuildArgs(t *testing.T) {
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			t.Override(&mavenBuildArgsFunc, getMavenBuildArgsFuncFake(t, MinimumJibMavenVersion))
-			args := GenerateMavenBuildArgs("test-goal", test.image, &test.a, test.skipTests, test.pushImages, test.deps, test.r, test.insecureRegistries)
+			args := GenerateMavenBuildArgs(nil, "test-goal", test.image, &test.a, test.skipTests, test.pushImages, test.deps, test.r, test.insecureRegistries)
 			t.CheckDeepEqual(test.out, args)
 		})
 	}
@@ -399,14 +399,14 @@ func TestMavenBuildArgs(t *testing.T) {
 			jibArtifact: latest.JibArtifact{},
 			skipTests:   false,
 			showColors:  true,
-			expected:    []string{"-Djib.console=plain", "fake-mavenArgs", "prepare-package", "jib:test-goal"},
+			expected:    []string{"-Dstyle.color=always", "-Djansi.passthrough=true", "-Djib.console=plain", "fake-mavenArgs", "prepare-package", "jib:test-goal"},
 		},
 		{
 			description: "single module skip tests",
 			jibArtifact: latest.JibArtifact{},
 			skipTests:   true,
 			showColors:  true,
-			expected:    []string{"-Djib.console=plain", "fake-mavenArgs", "-DskipTests=true", "prepare-package", "jib:test-goal"},
+			expected:    []string{"-Dstyle.color=always", "-Djansi.passthrough=true", "-Djib.console=plain", "fake-mavenArgs", "-DskipTests=true", "prepare-package", "jib:test-goal"},
 		},
 		{
 			description: "single module plain console",
@@ -420,14 +420,14 @@ func TestMavenBuildArgs(t *testing.T) {
 			jibArtifact: latest.JibArtifact{Project: "module"},
 			skipTests:   false,
 			showColors:  true,
-			expected:    []string{"-Djib.console=plain", "fake-mavenArgs-for-module", "package", "jib:test-goal", "-Djib.containerize=module"},
+			expected:    []string{"-Dstyle.color=always", "-Djansi.passthrough=true", "-Djib.console=plain", "fake-mavenArgs-for-module", "package", "jib:test-goal", "-Djib.containerize=module"},
 		},
 		{
 			description: "single module skip tests",
 			jibArtifact: latest.JibArtifact{Project: "module"},
 			skipTests:   true,
 			showColors:  true,
-			expected:    []string{"-Djib.console=plain", "fake-mavenArgs-for-module", "-DskipTests=true", "package", "jib:test-goal", "-Djib.containerize=module"},
+			expected:    []string{"-Dstyle.color=always", "-Djansi.passthrough=true", "-Djib.console=plain", "fake-mavenArgs-for-module", "-DskipTests=true", "package", "jib:test-goal", "-Djib.containerize=module"},
 		},
 	}
 	for _, test := range tests {

pkg/skaffold/build/result.go

@@ -25,6 +25,7 @@ import (
 
 	"github.com/sirupsen/logrus"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 )
 
@@ -58,11 +59,16 @@ func (l *logAggregatorImpl) GetWriter() (io.Writer, func(), error) {
 		return nil, nil, err
 	}
 	r, w := io.Pipe()
+
+	writer := io.Writer(w)
+	if color.IsColorable(l.out) {
+		writer = color.NewWriter(writer)
+	}
 	ch := make(chan string, buffSize)
 	l.messages <- ch
 	// write the build output to a buffered channel.
 	go l.writeToChannel(r, ch)
-	return w, func() { w.Close() }, nil
+	return writer, func() { w.Close() }, nil
 }
 
 func (l *logAggregatorImpl) PrintInOrder(ctx context.Context) {

pkg/skaffold/color/formatter.go

@@ -36,7 +36,7 @@ func init() {
 }
 
 // SetupColors enables/disables coloured output.
-func SetupColors(out io.Writer, defaultColor int, forceColors bool) {
+func SetupColors(out io.Writer, defaultColor int, forceColors bool) io.Writer {
 	_, isTerm := util.IsTerminal(out)
 	useColors := isTerm || forceColors
 	if useColors {
@@ -62,13 +62,22 @@ func SetupColors(out io.Writer, defaultColor int, forceColors bool) {
 		37: White,
 		0:  None,
 	}[defaultColor]
+
+	if useColors {
+		return NewWriter(out)
+	}
+	return out
 }
 
 // Color can be used to format text so it can be printed to the terminal in color.
 type Color struct {
 	color *colors.Color
 }
 
+type colorableWriter struct {
+	io.Writer
+}
+
 var (
 	// LightRed can format text to be displayed to the terminal in light red.
 	LightRed = Color{color: colors.New(colors.FgHiRed)}
@@ -103,7 +112,7 @@ var (
 
 // Fprintln outputs the result to out, followed by a newline.
 func (c Color) Fprintln(out io.Writer, a ...interface{}) {
-	if c.color == nil {
+	if c.color == nil || !IsColorable(out) {
 		fmt.Fprintln(out, a...)
 		return
 	}
@@ -113,10 +122,23 @@ func (c Color) Fprintln(out io.Writer, a ...interface{}) {
 
 // Fprintf outputs the result to out.
 func (c Color) Fprintf(out io.Writer, format string, a ...interface{}) {
-	if c.color == nil {
+	if c.color == nil || !IsColorable(out) {
 		fmt.Fprintf(out, format, a...)
 		return
 	}
 
 	fmt.Fprint(out, c.color.Sprintf(format, a...))
 }
+
+func NewWriter(out io.Writer) io.Writer {
+	return colorableWriter{out}
+}
+
+func IsColorable(out io.Writer) bool {
+	switch out.(type) {
+	case colorableWriter:
+		return true
+	default:
+		return false
+	}
+}

pkg/skaffold/color/formatter_test.go

@@ -32,8 +32,8 @@ func TestFprintln(t *testing.T) {
 	defer func() { SetupColors(nil, DefaultColorCode, false) }()
 	var b bytes.Buffer
 
-	SetupColors(&b, 0, true)
-	Green.Fprintln(&b, "2", "less", "chars!")
+	cw := SetupColors(&b, 0, true)
+	Green.Fprintln(cw, "2", "less", "chars!")
 
 	compareText(t, "\033[32m2 less chars!\033[0m\n", b.String())
 }
@@ -42,50 +42,50 @@ func TestFprintf(t *testing.T) {
 	defer func() { SetupColors(nil, DefaultColorCode, false) }()
 	var b bytes.Buffer
 
-	SetupColors(&b, 0, true)
-	Green.Fprintf(&b, "It's been %d %s", 1, "week")
+	cw := SetupColors(&b, 0, true)
+	Green.Fprintf(cw, "It's been %d %s", 1, "week")
 
 	compareText(t, "\033[32mIt's been 1 week\033[0m", b.String())
 }
 
 func TestFprintlnNoTTY(t *testing.T) {
 	var b bytes.Buffer
 
-	SetupColors(&b, 0, false)
-	Green.Fprintln(&b, "2", "less", "chars!")
+	cw := SetupColors(&b, 0, false)
+	Green.Fprintln(cw, "2", "less", "chars!")
 
 	compareText(t, "2 less chars!\n", b.String())
 }
 
 func TestFprintfNoTTY(t *testing.T) {
 	var b bytes.Buffer
 
-	SetupColors(&b, 0, false)
-	Green.Fprintf(&b, "It's been %d %s", 1, "week")
+	cw := SetupColors(&b, 0, false)
+	Green.Fprintf(cw, "It's been %d %s", 1, "week")
 
 	compareText(t, "It's been 1 week", b.String())
 }
 
 func TestFprintlnDefaultColor(t *testing.T) {
 	var b bytes.Buffer
 
-	SetupColors(&b, 34, true)
-	Default.Fprintln(&b, "2", "less", "chars!")
+	cw := SetupColors(&b, 34, true)
+	Default.Fprintln(cw, "2", "less", "chars!")
 	compareText(t, "\033[34m2 less chars!\033[0m\n", b.String())
 }
 
 func TestFprintlnChangeDefaultToNone(t *testing.T) {
 	var b bytes.Buffer
 
-	SetupColors(&b, 0, true)
-	Default.Fprintln(&b, "2", "less", "chars!")
+	cw := SetupColors(&b, 0, true)
+	Default.Fprintln(cw, "2", "less", "chars!")
 	compareText(t, "2 less chars!\n", b.String())
 }
 
 func TestFprintlnChangeDefaultToUnknown(t *testing.T) {
 	var b bytes.Buffer
 
-	SetupColors(&b, -1, true)
-	Default.Fprintln(&b, "2", "less", "chars!")
+	cw := SetupColors(&b, -1, true)
+	Default.Fprintln(cw, "2", "less", "chars!")
 	compareText(t, "2 less chars!\n", b.String())
 }