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

customizable jib feature minimum requirements #3628

Merged
merged 1 commit into from
Feb 3, 2020
Merged

customizable jib feature minimum requirements #3628

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
15 changes: 8 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 @@ -38,6 +38,7 @@ var (

// Skaffold-Jib depends on functionality introduced with Jib-Gradle 1.4.0
const MinimumJibGradleVersion = "1.4.0"
const MinimumJibGradleVersionForSync = "2.0.0"

// GradleCommand stores Gradle executable and wrapper name
var GradleCommand = util.CommandWrapper{Executable: "gradle", Wrapper: "gradlew"}
Expand Down Expand Up @@ -87,18 +88,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"), "-q")
args := append(gradleArgsFunc(a, "_jibSkaffoldFilesV2", MinimumJibGradleVersion), "-q")
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))
cmd := GradleCommand.CreateCommand(ctx, workspace, gradleBuildArgsFunc("_jibSkaffoldSyncMap", a, true, MinimumJibMavenVersionForSync))
tejal29 marked this conversation as resolved.
Show resolved Hide resolved
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)
args := gradleBuildArgsFunc(task, a, skipTests, 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,11 +110,11 @@ func GenerateGradleBuildArgs(task string, imageName string, a *latest.JibArtifac
}

// Do not use directly, use gradleBuildArgsFunc
func gradleBuildArgs(task string, a *latest.JibArtifact, skipTests bool) []string {
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"}
args = append(args, gradleArgsFunc(a, task)...)
args = append(args, gradleArgsFunc(a, task, minimumVersion)...)

if skipTests {
args = append(args, "-x", "test")
Expand All @@ -123,8 +124,8 @@ func gradleBuildArgs(task string, a *latest.JibArtifact, skipTests bool) []strin
}

// Do not use directly, use gradleArgsFunc
func gradleArgs(a *latest.JibArtifact, task string) []string {
args := []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion}
func gradleArgs(a *latest.JibArtifact, task string, minimumVersion string) []string {
args := []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + minimumVersion}
if a.Project == "" {
return append(args, ":"+task)
}
Expand Down
55 changes: 32 additions & 23 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 @@ -71,7 +71,7 @@ func TestBuildJibGradleToDocker(t *testing.T) {
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.NewTempDir().Touch("build.gradle").Chdir()
t.Override(&gradleBuildArgsFunc, gradleBuildArgsFuncFake)
t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersion))
t.Override(&util.DefaultExecCommand, test.commands)
api := (&testutil.FakeAPIClient{}).Add("img:tag", "imageID")
localDocker := docker.NewLocalDaemon(api, nil, false, nil)
Expand Down Expand Up @@ -130,7 +130,7 @@ func TestBuildJibGradleToRegistry(t *testing.T) {
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.NewTempDir().Touch("build.gradle").Chdir()
t.Override(&gradleBuildArgsFunc, gradleBuildArgsFuncFake)
t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersion))
t.Override(&util.DefaultExecCommand, test.commands)
t.Override(&docker.RemoteDigest, func(identifier string, _ map[string]bool) (string, error) {
if identifier == "img:tag" {
Expand Down Expand Up @@ -219,7 +219,9 @@ func TestGetDependenciesGradle(t *testing.T) {
))

// Change build file mod time
os.Chtimes(build, test.modTime, test.modTime)
if err := os.Chtimes(build, test.modTime, test.modTime); err != nil {
t.Fatal(err)
}

deps, err := getDependenciesGradle(ctx, tmpDir.Root(), &latest.JibArtifact{Project: "gradle-test"})
if test.err != nil {
Expand Down Expand Up @@ -313,7 +315,7 @@ func TestGetSyncMapCommandGradle(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&gradleBuildArgsFunc, gradleBuildArgsFuncFake)
t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersionForSync))
cmd := getSyncMapCommandGradle(ctx, test.workspace, &test.jibArtifact)
expectedCmd := test.expectedCmd(test.workspace)
t.CheckDeepEqual(expectedCmd.Path, cmd.Path)
Expand All @@ -340,7 +342,7 @@ func TestGenerateGradleBuildArgs(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&gradleBuildArgsFunc, gradleBuildArgsFuncFake)
t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersion))
command := GenerateGradleBuildArgs("testTask", test.image, &test.in, test.skipTests, test.insecureRegistries)
t.CheckDeepEqual(test.out, command)
})
Expand All @@ -356,16 +358,16 @@ func TestGradleArgs(t *testing.T) {
{
description: "single module",
jibArtifact: latest.JibArtifact{},
expected: []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":testTask"},
expected: []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=test-version", ":testTask"},
},
{
description: "multi module",
jibArtifact: latest.JibArtifact{Project: "module"},
expected: []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=" + MinimumJibGradleVersion, ":module:testTask"},
expected: []string{"_skaffoldFailIfJibOutOfDate", "-Djib.requiredVersion=test-version", ":module:testTask"},
},
}
for _, test := range tests {
args := gradleArgs(&test.jibArtifact, "testTask")
args := gradleArgs(&test.jibArtifact, "testTask", "test-version")
testutil.CheckDeepEqual(t, test.expected, args)
}
}
Expand Down Expand Up @@ -416,29 +418,36 @@ func TestGradleBuildArgs(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&gradleArgsFunc, gradleArgsFuncFake)
args := gradleBuildArgs("testTask", &test.jibArtifact, test.skipTests)
t.Override(&gradleArgsFunc, getGradleArgsFuncFake(t, "test-version"))
args := gradleBuildArgs("testTask", &test.jibArtifact, test.skipTests, "test-version")
t.CheckDeepEqual(test.expected, args)
})
}
}

func gradleArgsFuncFake(a *latest.JibArtifact, task string) []string {
if a.Project == "" {
return []string{"fake-gradleArgs-for-" + task}
func getGradleArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(*latest.JibArtifact, string, string) []string {
return func(a *latest.JibArtifact, task string, minimumVersion string) []string {
t.CheckDeepEqual(expectedMinimumVersion, minimumVersion)
if a.Project == "" {
return []string{"fake-gradleArgs-for-" + task}
}
return []string{"fake-gradleArgs-for-" + a.Project + "-for-" + task}
}
return []string{"fake-gradleArgs-for-" + a.Project + "-for-" + task}
}

// check that parameters are actually passed though
func gradleBuildArgsFuncFake(task string, a *latest.JibArtifact, skipTests bool) []string {
testString := ""
if skipTests {
testString = "-skipTests"
}

if a.Project == "" {
return []string{"fake-gradleBuildArgs-for-" + task + testString}
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 {
t.CheckDeepEqual(expectedMinimumVersion, minimumVersion)

testString := ""
if skipTests {
testString = "-skipTests"
}

if a.Project == "" {
return []string{"fake-gradleBuildArgs-for-" + task + testString}
}
return []string{"fake-gradleBuildArgs-for-" + a.Project + "-for-" + task + testString}
}
return []string{"fake-gradleBuildArgs-for-" + a.Project + "-for-" + task + testString}
}
15 changes: 8 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 @@ -37,6 +37,7 @@ var (

// Skaffold-Jib depends on functionality introduced with Jib-Maven 1.4.0
const MinimumJibMavenVersion = "1.4.0"
const MinimumJibMavenVersionForSync = "2.0.0"

// MavenCommand stores Maven executable and wrapper name
var MavenCommand = util.CommandWrapper{Executable: "mvn", Wrapper: "mvnw"}
Expand Down Expand Up @@ -85,20 +86,20 @@ 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)
args := mavenArgsFunc(a, MinimumJibMavenVersion)
args = append(args, "jib:_skaffold-files-v2", "--quiet")

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))
cmd := MavenCommand.CreateCommand(ctx, workspace, mavenBuildArgsFunc("_skaffold-sync-map", a, true, 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)
args := mavenBuildArgsFunc(goal, a, skipTests, 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 @@ -109,11 +110,11 @@ func GenerateMavenBuildArgs(goal string, imageName string, a *latest.JibArtifact
}

// Do not use directly, use mavenBuildArgsFunc
func mavenBuildArgs(goal string, a *latest.JibArtifact, skipTests bool) []string {
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"}
args = append(args, mavenArgsFunc(a)...)
args = append(args, mavenArgsFunc(a, minimumVersion)...)

if skipTests {
args = append(args, "-DskipTests=true")
Expand All @@ -130,8 +131,8 @@ func mavenBuildArgs(goal string, a *latest.JibArtifact, skipTests bool) []string
}

// Do not use directly, use mavenArgsFunc
func mavenArgs(a *latest.JibArtifact) []string {
args := []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion}
func mavenArgs(a *latest.JibArtifact, minimumVersion string) []string {
args := []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + minimumVersion}
args = append(args, a.Flags...)

if a.Project == "" {
Expand Down
69 changes: 36 additions & 33 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 @@ -70,7 +70,7 @@ func TestBuildJibMavenToDocker(t *testing.T) {

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&mavenBuildArgsFunc, mavenBuildArgsFuncFake)
t.Override(&mavenBuildArgsFunc, getMavenBuildArgsFuncFake(t, MinimumJibMavenVersion))
t.NewTempDir().Touch("pom.xml").Chdir()
t.Override(&util.DefaultExecCommand, test.commands)
api := (&testutil.FakeAPIClient{}).Add("img:tag", "imageID")
Expand Down Expand Up @@ -104,16 +104,12 @@ func TestBuildJibMavenToRegistry(t *testing.T) {
{
description: "build",
artifact: &latest.JibArtifact{},
commands: testutil.CmdRun(
"mvn fake-mavenBuildArgs-for-build -Dimage=img:tag",
),
commands: testutil.CmdRun("mvn fake-mavenBuildArgs-for-build -Dimage=img:tag"),
},
{
description: "build with module",
artifact: &latest.JibArtifact{Project: "module"},
commands: testutil.CmdRun(
"mvn fake-mavenBuildArgs-for-module-for-build -Dimage=img:tag",
),
commands: testutil.CmdRun("mvn fake-mavenBuildArgs-for-module-for-build -Dimage=img:tag"),
},
{
description: "fail build",
Expand All @@ -129,7 +125,7 @@ func TestBuildJibMavenToRegistry(t *testing.T) {

for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&mavenBuildArgsFunc, mavenBuildArgsFuncFake)
t.Override(&mavenBuildArgsFunc, getMavenBuildArgsFuncFake(t, MinimumJibMavenVersion))
t.NewTempDir().Touch("pom.xml").Chdir()
t.Override(&util.DefaultExecCommand, test.commands)
t.Override(&docker.RemoteDigest, func(identifier string, _ map[string]bool) (string, error) {
Expand Down Expand Up @@ -219,7 +215,9 @@ func TestGetDependenciesMaven(t *testing.T) {
))

// Change build file mod time
os.Chtimes(build, test.modTime, test.modTime)
if err := os.Chtimes(build, test.modTime, test.modTime); err != nil {
t.Fatal(err)
}

deps, err := getDependenciesMaven(ctx, tmpDir.Root(), &latest.JibArtifact{Project: "maven-test"})
if test.err != nil {
Expand Down Expand Up @@ -266,7 +264,7 @@ func TestGetCommandMaven(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&mavenArgsFunc, mavenArgsFuncFake)
t.Override(&mavenArgsFunc, getMavenArgsFuncFake(t, MinimumJibMavenVersion))
tmpDir := t.NewTempDir().
Touch(test.filesInWorkspace...)

Expand Down Expand Up @@ -305,7 +303,7 @@ func TestGetSyncMapCommandMaven(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&mavenBuildArgsFunc, mavenBuildArgsFuncFake)
t.Override(&mavenBuildArgsFunc, getMavenBuildArgsFuncFake(t, MinimumJibMavenVersionForSync))
cmd := getSyncMapCommandMaven(ctx, test.workspace, &test.jibArtifact)
expectedCmd := test.expectedCmd(test.workspace)
t.CheckDeepEqual(expectedCmd.Path, cmd.Path)
Expand All @@ -332,7 +330,7 @@ func TestGenerateMavenBuildArgs(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&mavenBuildArgsFunc, mavenBuildArgsFuncFake)
t.Override(&mavenBuildArgsFunc, getMavenBuildArgsFuncFake(t, MinimumJibMavenVersion))
args := GenerateMavenBuildArgs("test-goal", test.image, &test.a, test.skipTests, test.insecureRegistries)
t.CheckDeepEqual(test.out, args)
})
Expand Down Expand Up @@ -373,8 +371,8 @@ func TestMavenBuildArgs(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&mavenArgsFunc, mavenArgsFuncFake)
args := mavenBuildArgs("test-goal", &test.jibArtifact, test.skipTests)
t.Override(&mavenArgsFunc, getMavenArgsFuncFake(t, "test-version"))
args := mavenBuildArgs("test-goal", &test.jibArtifact, test.skipTests, "test-version")
t.CheckDeepEqual(test.expected, args)
})
}
Expand All @@ -389,51 +387,56 @@ func TestMavenArgs(t *testing.T) {
{
description: "single module",
jibArtifact: latest.JibArtifact{},
expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion, "--non-recursive"},
expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=test-version", "--non-recursive"},
},
{
description: "single module with extra flags",
jibArtifact: latest.JibArtifact{
Flags: []string{"--flag1", "--flag2"},
},
expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion, "--flag1", "--flag2", "--non-recursive"},
expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=test-version", "--flag1", "--flag2", "--non-recursive"},
},
{
description: "multi module",
jibArtifact: latest.JibArtifact{Project: "module"},
expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion, "--projects", "module", "--also-make"},
expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=test-version", "--projects", "module", "--also-make"},
},
{
description: "multi module with extra falgs",
jibArtifact: latest.JibArtifact{
Project: "module",
Flags: []string{"--flag1", "--flag2"},
},
expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=" + MinimumJibMavenVersion, "--flag1", "--flag2", "--projects", "module", "--also-make"},
expected: []string{"jib:_skaffold-fail-if-jib-out-of-date", "-Djib.requiredVersion=test-version", "--flag1", "--flag2", "--projects", "module", "--also-make"},
},
}
for _, test := range tests {
args := mavenArgs(&test.jibArtifact)
args := mavenArgs(&test.jibArtifact, "test-version")
testutil.CheckDeepEqual(t, test.expected, args)
}
}

func mavenArgsFuncFake(a *latest.JibArtifact) []string {
if a.Project == "" {
return []string{"fake-mavenArgs"}
func getMavenArgsFuncFake(t *testutil.T, expectedMinimumVersion string) func(*latest.JibArtifact, string) []string {
return func(a *latest.JibArtifact, minimumVersion string) []string {
t.CheckDeepEqual(expectedMinimumVersion, minimumVersion)
if a.Project == "" {
return []string{"fake-mavenArgs"}
}
return []string{"fake-mavenArgs-for-" + a.Project}
}
return []string{"fake-mavenArgs-for-" + a.Project}
}

// check that parameters are actually passed though
func mavenBuildArgsFuncFake(goal string, a *latest.JibArtifact, skipTests bool) []string {
testString := ""
if skipTests {
testString = "-skipTests"
}

if a.Project == "" {
return []string{"fake-mavenBuildArgs-for-" + goal + testString}
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 {
t.CheckDeepEqual(expectedMinimumVersion, minimumVersion)
testString := ""
if skipTests {
testString = "-skipTests"
}

if a.Project == "" {
return []string{"fake-mavenBuildArgs-for-" + goal + testString}
}
return []string{"fake-mavenBuildArgs-for-" + a.Project + "-for-" + goal + testString}
}
return []string{"fake-mavenBuildArgs-for-" + a.Project + "-for-" + goal + testString}
}
Loading