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.

Sign up for GitHub

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

Jump to bottom

Fix console output for internal Jib tasks/goals #3880

Merged
merged 1 commit into from
Mar 26, 2020
Merged

Fix console output for internal Jib tasks/goals #3880

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 12 additions & 7 deletions pkg/skaffold/build/jib/gradle.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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