Skip to content

Commit

Permalink
Fix console output for internal Jib tasks/goals (#3880)
Browse files Browse the repository at this point in the history
  • Loading branch information
@TadCordle
TadCordle authored Mar 26, 2020
1 parent eaf5b88 commit b06cdee
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 35 deletions.
19 changes: 12 additions & 7 deletions pkg/skaffold/build/jib/gradle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,18 @@ func getDependenciesGradle(ctx context.Context, workspace string, a *latest.JibA
}

func getCommandGradle(ctx context.Context, workspace string, a *latest.JibArtifact) exec.Cmd {
args := append(gradleArgsFunc(a, "_jibSkaffoldFilesV2", MinimumJibGradleVersion), "-q")
args := append(gradleArgsFunc(a, "_jibSkaffoldFilesV2", MinimumJibGradleVersion), "-q", "--console=plain")
return GradleCommand.CreateCommand(ctx, workspace, args)
}

func getSyncMapCommandGradle(ctx context.Context, workspace string, a *latest.JibArtifact) *exec.Cmd {
cmd := GradleCommand.CreateCommand(ctx, workspace, gradleBuildArgsFunc("_jibSkaffoldSyncMap", a, true, MinimumJibMavenVersionForSync))
cmd := GradleCommand.CreateCommand(ctx, workspace, gradleBuildArgsFunc("_jibSkaffoldSyncMap", a, true, false, MinimumJibMavenVersionForSync))
return &cmd
}

// GenerateGradleBuildArgs generates the arguments to Gradle for building the project as an image.
func GenerateGradleBuildArgs(task string, imageName string, a *latest.JibArtifact, skipTests bool, insecureRegistries map[string]bool) []string {
args := gradleBuildArgsFunc(task, a, skipTests, MinimumJibGradleVersion)
args := gradleBuildArgsFunc(task, a, skipTests, true, 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")
Expand All @@ -109,10 +109,15 @@ func GenerateGradleBuildArgs(task string, imageName string, a *latest.JibArtifac
}

// Do not use directly, use gradleBuildArgsFunc
func gradleBuildArgs(task string, a *latest.JibArtifact, skipTests bool, minimumVersion string) []string {
// disable jib's rich progress footer; we could use `--console=plain`
// but it also disables colour which can be helpful
args := []string{"-Djib.console=plain"}
func gradleBuildArgs(task string, a *latest.JibArtifact, skipTests, showColors bool, minimumVersion string) []string {
// Disable jib's rich progress footer on builds. Show colors on normal builds for clearer information,
// but use --console=plain for internal goals to avoid formatting issues
var args []string
if showColors {
args = []string{"-Djib.console=plain"}
} else {
args = []string{"--console=plain"}
}
args = append(args, gradleArgsFunc(a, task, minimumVersion)...)

if skipTests {
Expand Down
28 changes: 21 additions & 7 deletions pkg/skaffold/build/jib/gradle_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,31 +245,31 @@ func TestGetCommandGradle(t *testing.T) {
jibArtifact: latest.JibArtifact{},
filesInWorkspace: []string{},
expectedCmd: func(workspace string) exec.Cmd {
return GradleCommand.CreateCommand(ctx, workspace, []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":_jibSkaffoldFilesV2", "-q"})
return GradleCommand.CreateCommand(ctx, workspace, []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":_jibSkaffoldFilesV2", "-q", "--console=plain"})
},
},
{
description: "gradle default with project",
jibArtifact: latest.JibArtifact{Project: "project"},
filesInWorkspace: []string{},
expectedCmd: func(workspace string) exec.Cmd {
return GradleCommand.CreateCommand(ctx, workspace, []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":project:_jibSkaffoldFilesV2", "-q"})
return GradleCommand.CreateCommand(ctx, workspace, []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":project:_jibSkaffoldFilesV2", "-q", "--console=plain"})
},
},
{
description: "gradle with wrapper",
jibArtifact: latest.JibArtifact{},
filesInWorkspace: []string{"gradlew", "gradlew.cmd"},
expectedCmd: func(workspace string) exec.Cmd {
return GradleCommand.CreateCommand(ctx, workspace, []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":_jibSkaffoldFilesV2", "-q"})
return GradleCommand.CreateCommand(ctx, workspace, []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":_jibSkaffoldFilesV2", "-q", "--console=plain"})
},
},
{
description: "gradle with wrapper and project",
jibArtifact: latest.JibArtifact{Project: "project"},
filesInWorkspace: []string{"gradlew", "gradlew.cmd"},
expectedCmd: func(workspace string) exec.Cmd {
return GradleCommand.CreateCommand(ctx, workspace, []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":project:_jibSkaffoldFilesV2", "-q"})
return GradleCommand.CreateCommand(ctx, workspace, []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":project:_jibSkaffoldFilesV2", "-q", "--console=plain"})
},
},
}
Expand Down Expand Up @@ -375,49 +375,63 @@ func TestGradleBuildArgs(t *testing.T) {
description string
jibArtifact latest.JibArtifact
skipTests bool
showColors bool
expected []string
}{
{
description: "single module",
jibArtifact: latest.JibArtifact{},
skipTests: false,
showColors: true,
expected: []string{"-Djib.console=plain", "fake-gradleArgs-for-testTask"},
},
{
description: "single module skip tests",
jibArtifact: latest.JibArtifact{},
skipTests: true,
showColors: true,
expected: []string{"-Djib.console=plain", "fake-gradleArgs-for-testTask", "-x", "test"},
},
{
description: "single module plain console",
jibArtifact: latest.JibArtifact{},
skipTests: true,
showColors: false,
expected: []string{"--console=plain", "fake-gradleArgs-for-testTask", "-x", "test"},
},
{
description: "single module with extra flags",
jibArtifact: latest.JibArtifact{Flags: []string{"--flag1", "--flag2"}},
skipTests: false,
showColors: true,
expected: []string{"-Djib.console=plain", "fake-gradleArgs-for-testTask", "--flag1", "--flag2"},
},
{
description: "multi module",
jibArtifact: latest.JibArtifact{Project: "module"},
skipTests: false,
showColors: true,
expected: []string{"-Djib.console=plain", "fake-gradleArgs-for-module-for-testTask"},
},
{
description: "single module skip tests",
jibArtifact: latest.JibArtifact{Project: "module"},
skipTests: true,
showColors: true,
expected: []string{"-Djib.console=plain", "fake-gradleArgs-for-module-for-testTask", "-x", "test"},
},
{
description: "multi module with extra flags",
jibArtifact: latest.JibArtifact{Project: "module", Flags: []string{"--flag1", "--flag2"}},
skipTests: false,
showColors: true,
expected: []string{"-Djib.console=plain", "fake-gradleArgs-for-module-for-testTask", "--flag1", "--flag2"},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&gradleArgsFunc, getGradleArgsFuncFake(t, "test-version"))
args := gradleBuildArgs("testTask", &test.jibArtifact, test.skipTests, "test-version")
args := gradleBuildArgs("testTask", &test.jibArtifact, test.skipTests, test.showColors, "test-version")
t.CheckDeepEqual(test.expected, args)
})
}
Expand All @@ -434,8 +448,8 @@ func getGradleArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(*l
}

// check that parameters are actually passed though
func getGradleBuildArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(string, *latest.JibArtifact, bool, string) []string {
return func(task string, a *latest.JibArtifact, skipTests bool, minimumVersion string) []string {
func getGradleBuildArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(string, *latest.JibArtifact, bool, bool, string) []string {
return func(task string, a *latest.JibArtifact, skipTests, showColors bool, minimumVersion string) []string {
t.CheckDeepEqual(expectedMinimumVersion, minimumVersion)

testString := ""
Expand Down
6 changes: 4 additions & 2 deletions pkg/skaffold/build/jib/init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,20 +87,22 @@ type jibJSON struct {
func validate(path string) []ArtifactConfig {
// Determine whether maven or gradle
var builderType PluginType
var executable, wrapper, taskName, searchString string
var executable, wrapper, taskName, searchString, consoleFlag string
switch {
case strings.HasSuffix(path, "pom.xml"):
builderType = JibMaven
executable = "mvn"
wrapper = "mvnw"
searchString = "<artifactId>jib-maven-plugin</artifactId>"
taskName = "jib:_skaffold-init"
consoleFlag = "--batch-mode"
case strings.HasSuffix(path, "build.gradle"), strings.HasSuffix(path, "build.gradle.kts"):
builderType = JibGradle
executable = "gradle"
wrapper = "gradlew"
searchString = "com.google.cloud.tools.jib"
taskName = "_jibSkaffoldInit"
consoleFlag = "--console=plain"
default:
return nil
}
Expand All @@ -114,7 +116,7 @@ func validate(path string) []ArtifactConfig {
if wrapperExecutable, err := util.AbsFile(filepath.Dir(path), wrapper); err == nil {
executable = wrapperExecutable
}
cmd := exec.Command(executable, taskName, "-q")
cmd := exec.Command(executable, taskName, "-q", consoleFlag)
cmd.Dir = filepath.Dir(path)
stdout, err := util.RunCmdOut(cmd)
if err != nil {
Expand Down
12 changes: 6 additions & 6 deletions pkg/skaffold/build/jib/init_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,15 +49,15 @@ func TestValidate(t *testing.T) {
description: "jib string found but not configured",
path: "path/to/build.gradle",
fileContents: "com.google.cloud.tools.jib",
command: "gradle _jibSkaffoldInit -q",
command: "gradle _jibSkaffoldInit -q --console=plain",
stdout: "error",
expectedConfig: nil,
},
{
description: "jib gradle single project",
path: "path/to/build.gradle",
fileContents: "com.google.cloud.tools.jib",
command: "gradle _jibSkaffoldInit -q",
command: "gradle _jibSkaffoldInit -q --console=plain",
stdout: `BEGIN JIB JSON
{"image":"image","project":"project"}
`,
Expand All @@ -69,7 +69,7 @@ func TestValidate(t *testing.T) {
description: "jib gradle-kotlin single project",
path: "path/to/build.gradle.kts",
fileContents: "com.google.cloud.tools.jib",
command: "gradle _jibSkaffoldInit -q",
command: "gradle _jibSkaffoldInit -q --console=plain",
stdout: `BEGIN JIB JSON
{"image":"image","project":"project"}
`,
Expand All @@ -81,7 +81,7 @@ func TestValidate(t *testing.T) {
description: "jib gradle multi-project",
path: "path/to/build.gradle",
fileContents: "com.google.cloud.tools.jib",
command: "gradle _jibSkaffoldInit -q",
command: "gradle _jibSkaffoldInit -q --console=plain",
stdout: `BEGIN JIB JSON
{"image":"image","project":"project1"}
Expand All @@ -97,7 +97,7 @@ BEGIN JIB JSON
description: "jib maven single module",
path: "path/to/pom.xml",
fileContents: "<artifactId>jib-maven-plugin</artifactId>",
command: "mvn jib:_skaffold-init -q",
command: "mvn jib:_skaffold-init -q --batch-mode",
stdout: `BEGIN JIB JSON
{"image":"image","project":"project"}`,
expectedConfig: []ArtifactConfig{
Expand All @@ -108,7 +108,7 @@ BEGIN JIB JSON
description: "jib maven multi-module",
path: "path/to/pom.xml",
fileContents: "<artifactId>jib-maven-plugin</artifactId>",
command: "mvn jib:_skaffold-init -q",
command: "mvn jib:_skaffold-init -q --batch-mode",
stdout: `BEGIN JIB JSON
{"image":"image","project":"project1"}
Expand Down
19 changes: 12 additions & 7 deletions pkg/skaffold/build/jib/maven.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,19 +87,19 @@ func getDependenciesMaven(ctx context.Context, workspace string, a *latest.JibAr

func getCommandMaven(ctx context.Context, workspace string, a *latest.JibArtifact) exec.Cmd {
args := mavenArgsFunc(a, MinimumJibMavenVersion)
args = append(args, "jib:_skaffold-files-v2", "--quiet")
args = append(args, "jib:_skaffold-files-v2", "--quiet", "--batch-mode")

return MavenCommand.CreateCommand(ctx, workspace, args)
}

func getSyncMapCommandMaven(ctx context.Context, workspace string, a *latest.JibArtifact) *exec.Cmd {
cmd := MavenCommand.CreateCommand(ctx, workspace, mavenBuildArgsFunc("_skaffold-sync-map", a, true, MinimumJibMavenVersionForSync))
cmd := MavenCommand.CreateCommand(ctx, workspace, mavenBuildArgsFunc("_skaffold-sync-map", a, true, false, MinimumJibMavenVersionForSync))
return &cmd
}

// GenerateMavenBuildArgs generates the arguments to Maven for building the project as an image.
func GenerateMavenBuildArgs(goal string, imageName string, a *latest.JibArtifact, skipTests bool, insecureRegistries map[string]bool) []string {
args := mavenBuildArgsFunc(goal, a, skipTests, MinimumJibMavenVersion)
args := mavenBuildArgsFunc(goal, a, skipTests, true, 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")
Expand All @@ -110,10 +110,15 @@ func GenerateMavenBuildArgs(goal string, imageName string, a *latest.JibArtifact
}

// Do not use directly, use mavenBuildArgsFunc
func mavenBuildArgs(goal string, a *latest.JibArtifact, skipTests bool, minimumVersion string) []string {
// disable jib's rich progress footer on builds; we could use --batch-mode
// but it also disables colour which can be helpful
args := []string{"-Djib.console=plain"}
func mavenBuildArgs(goal string, a *latest.JibArtifact, skipTests, showColors bool, minimumVersion string) []string {
// Disable jib's rich progress footer on builds. Show colors on normal builds for clearer information,
// but use --batch-mode for internal goals to avoid formatting issues
var args []string
if showColors {
args = []string{"-Djib.console=plain"}
} else {
args = []string{"--batch-mode"}
}
args = append(args, mavenArgsFunc(a, minimumVersion)...)

if skipTests {
Expand Down
24 changes: 18 additions & 6 deletions pkg/skaffold/build/jib/maven_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,23 +240,23 @@ func TestGetCommandMaven(t *testing.T) {
jibArtifact: latest.JibArtifact{},
filesInWorkspace: []string{},
expectedCmd: func(workspace string) exec.Cmd {
return MavenCommand.CreateCommand(ctx, workspace, []string{"fake-mavenArgs", "jib:_skaffold-files-v2", "--quiet"})
return MavenCommand.CreateCommand(ctx, workspace, []string{"fake-mavenArgs", "jib:_skaffold-files-v2", "--quiet", "--batch-mode"})
},
},
{
description: "maven with wrapper",
jibArtifact: latest.JibArtifact{},
filesInWorkspace: []string{"mvnw", "mvnw.bat"},
expectedCmd: func(workspace string) exec.Cmd {
return MavenCommand.CreateCommand(ctx, workspace, []string{"fake-mavenArgs", "jib:_skaffold-files-v2", "--quiet"})
return MavenCommand.CreateCommand(ctx, workspace, []string{"fake-mavenArgs", "jib:_skaffold-files-v2", "--quiet", "--batch-mode"})
},
},
{
description: "maven with multi-modules",
jibArtifact: latest.JibArtifact{Project: "module"},
filesInWorkspace: []string{"mvnw", "mvnw.bat"},
expectedCmd: func(workspace string) exec.Cmd {
return MavenCommand.CreateCommand(ctx, workspace, []string{"fake-mavenArgs-for-module", "jib:_skaffold-files-v2", "--quiet"})
return MavenCommand.CreateCommand(ctx, workspace, []string{"fake-mavenArgs-for-module", "jib:_skaffold-files-v2", "--quiet", "--batch-mode"})
},
},
}
Expand Down Expand Up @@ -340,37 +340,49 @@ func TestMavenBuildArgs(t *testing.T) {
description string
jibArtifact latest.JibArtifact
skipTests bool
showColors bool
expected []string
}{
{
description: "single module",
jibArtifact: latest.JibArtifact{},
skipTests: false,
showColors: true,
expected: []string{"-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"},
},
{
description: "single module plain console",
jibArtifact: latest.JibArtifact{},
skipTests: true,
showColors: false,
expected: []string{"--batch-mode", "fake-mavenArgs", "-DskipTests=true", "prepare-package", "jib:test-goal"},
},
{
description: "multi module",
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"},
},
{
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"},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&mavenArgsFunc, getMavenArgsFuncFake(t, "test-version"))
args := mavenBuildArgs("test-goal", &test.jibArtifact, test.skipTests, "test-version")
args := mavenBuildArgs("test-goal", &test.jibArtifact, test.skipTests, test.showColors, "test-version")
t.CheckDeepEqual(test.expected, args)
})
}
Expand Down Expand Up @@ -424,8 +436,8 @@ func getMavenArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(*la
}
}

func getMavenBuildArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(string, *latest.JibArtifact, bool, string) []string {
return func(goal string, a *latest.JibArtifact, skipTests bool, minimumVersion string) []string {
func getMavenBuildArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(string, *latest.JibArtifact, bool, bool, string) []string {
return func(goal string, a *latest.JibArtifact, skipTests, showColors bool, minimumVersion string) []string {
t.CheckDeepEqual(expectedMinimumVersion, minimumVersion)
testString := ""
if skipTests {
Expand Down

0 comments on commit b06cdee

Please sign in to comment.