Skip to content

Commit

Permalink
Merge pull request #3628 from GoogleContainerTools/jib-sync-parts-4
Browse files Browse the repository at this point in the history
customizable jib feature minimum requirements
  • Loading branch information
balopat authored Feb 3, 2020
2 parents 6ba887a + adefa58 commit 2f7ca2d
Show file tree
Hide file tree
Showing 6 changed files with 87 additions and 76 deletions.
15 changes: 8 additions & 7 deletions pkg/skaffold/build/jib/gradle.go
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))
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
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
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
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

0 comments on commit 2f7ca2d

Please sign in to comment.