From 27f71d98f8a7b95c65fe7aaf0e4c97e82135b918 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 11 Jan 2019 18:50:23 +0100 Subject: [PATCH 1/2] Move code to jib package Signed-off-by: David Gageot --- pkg/skaffold/build/local/jib_gradle.go | 17 ++----------- pkg/skaffold/build/local/jib_maven.go | 22 ++--------------- pkg/skaffold/build/local/jib_test.go | 34 -------------------------- pkg/skaffold/jib/jib_gradle.go | 13 ++++++++++ pkg/skaffold/jib/jib_gradle_test.go | 16 ++++++++++++ pkg/skaffold/jib/jib_maven.go | 18 ++++++++++++++ pkg/skaffold/jib/jib_maven_test.go | 18 ++++++++++++++ 7 files changed, 69 insertions(+), 69 deletions(-) diff --git a/pkg/skaffold/build/local/jib_gradle.go b/pkg/skaffold/build/local/jib_gradle.go index 0726f7198c3..e536ff3036c 100644 --- a/pkg/skaffold/build/local/jib_gradle.go +++ b/pkg/skaffold/build/local/jib_gradle.go @@ -38,7 +38,7 @@ func (b *Builder) buildJibGradle(ctx context.Context, out io.Writer, workspace s func (b *Builder) buildJibGradleToDocker(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibGradleArtifact) (string, error) { skaffoldImage := generateJibImageRef(workspace, artifact.Project) - args := generateGradleArgs("jibDockerBuild", skaffoldImage, artifact) + args := jib.GenerateGradleArgs("jibDockerBuild", skaffoldImage, artifact) if err := runGradleCommand(ctx, out, workspace, args); err != nil { return "", err @@ -50,7 +50,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.Artifact) (string, error) { initialTag := util.RandomID() skaffoldImage := fmt.Sprintf("%s:%s", artifact.ImageName, initialTag) - args := generateGradleArgs("jib", skaffoldImage, artifact.JibGradleArtifact) + args := jib.GenerateGradleArgs("jib", skaffoldImage, artifact.JibGradleArtifact) if err := runGradleCommand(ctx, out, workspace, args); err != nil { return "", err @@ -59,19 +59,6 @@ func (b *Builder) buildJibGradleToRegistry(ctx context.Context, out io.Writer, w return docker.RemoteDigest(skaffoldImage) } -// generateGradleArgs generates the arguments to Gradle for building the project as an image called `skaffoldImage`. -func generateGradleArgs(task string, imageName string, artifact *latest.JibGradleArtifact) []string { - var command string - if artifact.Project == "" { - command = ":" + task - } else { - // multi-module - command = fmt.Sprintf(":%s:%s", artifact.Project, task) - } - - return []string{command, "--image=" + imageName} -} - func runGradleCommand(ctx context.Context, out io.Writer, workspace string, args []string) error { cmd := jib.GradleCommand.CreateCommand(ctx, workspace, args) cmd.Stdout = out diff --git a/pkg/skaffold/build/local/jib_maven.go b/pkg/skaffold/build/local/jib_maven.go index ccd46e0708b..964c95af51c 100644 --- a/pkg/skaffold/build/local/jib_maven.go +++ b/pkg/skaffold/build/local/jib_maven.go @@ -46,7 +46,7 @@ func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, work } skaffoldImage := generateJibImageRef(workspace, artifact.Module) - args := generateMavenArgs("dockerBuild", skaffoldImage, artifact) + args := jib.GenerateMavenArgs("dockerBuild", skaffoldImage, artifact) if err := runMavenCommand(ctx, out, workspace, args); err != nil { return "", err @@ -65,7 +65,7 @@ func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, wo initialTag := util.RandomID() skaffoldImage := fmt.Sprintf("%s:%s", artifact.ImageName, initialTag) - args := generateMavenArgs("build", skaffoldImage, artifact.JibMavenArtifact) + args := jib.GenerateMavenArgs("build", skaffoldImage, artifact.JibMavenArtifact) if err := runMavenCommand(ctx, out, workspace, args); err != nil { return "", err @@ -74,24 +74,6 @@ func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, wo return docker.RemoteDigest(skaffoldImage) } -// generateMavenArgs generates the arguments to Maven for building the project as an image called `skaffoldImage`. -func generateMavenArgs(goal string, imageName string, artifact *latest.JibMavenArtifact) []string { - var command []string - if artifact.Module == "" { - // single-module project - command = []string{"--non-recursive", "prepare-package", "jib:" + goal} - } else { - // multi-module project: we assume `package` is bound to `jib:` - command = []string{"--projects", artifact.Module, "--also-make", "package"} - } - command = append(command, "-Dimage="+imageName) - if artifact.Profile != "" { - command = append(command, "--activate-profiles", artifact.Profile) - } - - return command -} - // verifyJibPackageGoal verifies that the referenced module has `package` bound to a single jib goal. // It returns `nil` if the goal is matched, and an error if there is a mismatch. func verifyJibPackageGoal(ctx context.Context, requiredGoal string, workspace string, artifact *latest.JibMavenArtifact) error { diff --git a/pkg/skaffold/build/local/jib_test.go b/pkg/skaffold/build/local/jib_test.go index 9f81dc95f86..94fab894b89 100644 --- a/pkg/skaffold/build/local/jib_test.go +++ b/pkg/skaffold/build/local/jib_test.go @@ -25,24 +25,6 @@ import ( "github.com/GoogleContainerTools/skaffold/testutil" ) -func TestGenerateMavenArgs(t *testing.T) { - var testCases = []struct { - in latest.JibMavenArtifact - out []string - }{ - {latest.JibMavenArtifact{}, []string{"--non-recursive", "prepare-package", "jib:goal", "-Dimage=image"}}, - {latest.JibMavenArtifact{Profile: "profile"}, []string{"--non-recursive", "prepare-package", "jib:goal", "-Dimage=image", "--activate-profiles", "profile"}}, - {latest.JibMavenArtifact{Module: "module"}, []string{"--projects", "module", "--also-make", "package", "-Dimage=image"}}, - {latest.JibMavenArtifact{Module: "module", Profile: "profile"}, []string{"--projects", "module", "--also-make", "package", "-Dimage=image", "--activate-profiles", "profile"}}, - } - - for _, tt := range testCases { - args := generateMavenArgs("goal", "image", &tt.in) - - testutil.CheckDeepEqual(t, tt.out, args) - } -} - func TestMavenVerifyJibPackageGoal(t *testing.T) { var testCases = []struct { requiredGoal string @@ -71,22 +53,6 @@ func TestMavenVerifyJibPackageGoal(t *testing.T) { } } -func TestGenerateGradleArgs(t *testing.T) { - var testCases = []struct { - in latest.JibGradleArtifact - out []string - }{ - {latest.JibGradleArtifact{}, []string{":task", "--image=image"}}, - {latest.JibGradleArtifact{Project: "project"}, []string{":project:task", "--image=image"}}, - } - - for _, tt := range testCases { - command := generateGradleArgs("task", "image", &tt.in) - - testutil.CheckDeepEqual(t, tt.out, command) - } -} - func TestGenerateJibImageRef(t *testing.T) { var testCases = []struct { workspace string diff --git a/pkg/skaffold/jib/jib_gradle.go b/pkg/skaffold/jib/jib_gradle.go index b3839eafdfe..bb9548c183d 100644 --- a/pkg/skaffold/jib/jib_gradle.go +++ b/pkg/skaffold/jib/jib_gradle.go @@ -49,3 +49,16 @@ func getCommandGradle(ctx context.Context, workspace string, a *latest.JibGradle } return GradleCommand.CreateCommand(ctx, workspace, args) } + +// GenerateGradleArgs generates the arguments to Gradle for building the project as an image. +func GenerateGradleArgs(task string, imageName string, artifact *latest.JibGradleArtifact) []string { + var command string + if artifact.Project == "" { + command = ":" + task + } else { + // multi-module + command = fmt.Sprintf(":%s:%s", artifact.Project, task) + } + + return []string{command, "--image=" + imageName} +} diff --git a/pkg/skaffold/jib/jib_gradle_test.go b/pkg/skaffold/jib/jib_gradle_test.go index 9428d8dfdc6..b2c2e745083 100644 --- a/pkg/skaffold/jib/jib_gradle_test.go +++ b/pkg/skaffold/jib/jib_gradle_test.go @@ -143,3 +143,19 @@ func TestGetCommandGradle(t *testing.T) { }) } } + +func TestGenerateGradleArgs(t *testing.T) { + var testCases = []struct { + in latest.JibGradleArtifact + out []string + }{ + {latest.JibGradleArtifact{}, []string{":task", "--image=image"}}, + {latest.JibGradleArtifact{Project: "project"}, []string{":project:task", "--image=image"}}, + } + + for _, tt := range testCases { + command := GenerateGradleArgs("task", "image", &tt.in) + + testutil.CheckDeepEqual(t, tt.out, command) + } +} diff --git a/pkg/skaffold/jib/jib_maven.go b/pkg/skaffold/jib/jib_maven.go index 18bb2334663..fcf91ae8a47 100644 --- a/pkg/skaffold/jib/jib_maven.go +++ b/pkg/skaffold/jib/jib_maven.go @@ -55,3 +55,21 @@ func getCommandMaven(ctx context.Context, workspace string, a *latest.JibMavenAr return MavenCommand.CreateCommand(ctx, workspace, args) } + +// GenerateMavenArgs generates the arguments to Maven for building the project as an image. +func GenerateMavenArgs(goal string, imageName string, artifact *latest.JibMavenArtifact) []string { + var command []string + if artifact.Module == "" { + // single-module project + command = []string{"--non-recursive", "prepare-package", "jib:" + goal} + } else { + // multi-module project: we assume `package` is bound to `jib:` + command = []string{"--projects", artifact.Module, "--also-make", "package"} + } + command = append(command, "-Dimage="+imageName) + if artifact.Profile != "" { + command = append(command, "--activate-profiles", artifact.Profile) + } + + return command +} diff --git a/pkg/skaffold/jib/jib_maven_test.go b/pkg/skaffold/jib/jib_maven_test.go index 1da04528a83..44ddb0cd75d 100644 --- a/pkg/skaffold/jib/jib_maven_test.go +++ b/pkg/skaffold/jib/jib_maven_test.go @@ -158,3 +158,21 @@ func TestGetCommandMaven(t *testing.T) { }) } } + +func TestGenerateMavenArgs(t *testing.T) { + var testCases = []struct { + in latest.JibMavenArtifact + out []string + }{ + {latest.JibMavenArtifact{}, []string{"--non-recursive", "prepare-package", "jib:goal", "-Dimage=image"}}, + {latest.JibMavenArtifact{Profile: "profile"}, []string{"--non-recursive", "prepare-package", "jib:goal", "-Dimage=image", "--activate-profiles", "profile"}}, + {latest.JibMavenArtifact{Module: "module"}, []string{"--projects", "module", "--also-make", "package", "-Dimage=image"}}, + {latest.JibMavenArtifact{Module: "module", Profile: "profile"}, []string{"--projects", "module", "--also-make", "package", "-Dimage=image", "--activate-profiles", "profile"}}, + } + + for _, tt := range testCases { + args := GenerateMavenArgs("goal", "image", &tt.in) + + testutil.CheckDeepEqual(t, tt.out, args) + } +} From add22d8744da9ce14aa3d9e0355c69ea4693f7dc Mon Sep 17 00:00:00 2001 From: David Gageot Date: Fri, 11 Jan 2019 18:51:42 +0100 Subject: [PATCH 2/2] Increase code duplication Signed-off-by: David Gageot --- pkg/skaffold/jib/jib_gradle.go | 18 +++++++++++------ pkg/skaffold/jib/jib_maven.go | 31 ++++++++++++++++-------------- pkg/skaffold/jib/jib_maven_test.go | 8 ++++---- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/pkg/skaffold/jib/jib_gradle.go b/pkg/skaffold/jib/jib_gradle.go index bb9548c183d..aff2053e972 100644 --- a/pkg/skaffold/jib/jib_gradle.go +++ b/pkg/skaffold/jib/jib_gradle.go @@ -42,22 +42,28 @@ func GetDependenciesGradle(ctx context.Context, workspace string, a *latest.JibG } func getCommandGradle(ctx context.Context, workspace string, a *latest.JibGradleArtifact) *exec.Cmd { - args := []string{"_jibSkaffoldFiles", "-q"} - if a.Project != "" { + task := "_jibSkaffoldFiles" + + var command string + if a.Project == "" { + command = task + } else { // multi-module - args[0] = fmt.Sprintf(":%s:%s", a.Project, args[0]) + command = fmt.Sprintf(":%s:%s", a.Project, task) } + args := []string{command, "-q"} + return GradleCommand.CreateCommand(ctx, workspace, args) } // GenerateGradleArgs generates the arguments to Gradle for building the project as an image. -func GenerateGradleArgs(task string, imageName string, artifact *latest.JibGradleArtifact) []string { +func GenerateGradleArgs(task string, imageName string, a *latest.JibGradleArtifact) []string { var command string - if artifact.Project == "" { + if a.Project == "" { command = ":" + task } else { // multi-module - command = fmt.Sprintf(":%s:%s", artifact.Project, task) + command = fmt.Sprintf(":%s:%s", a.Project, task) } return []string{command, "--image=" + imageName} diff --git a/pkg/skaffold/jib/jib_maven.go b/pkg/skaffold/jib/jib_maven.go index fcf91ae8a47..887137d6791 100644 --- a/pkg/skaffold/jib/jib_maven.go +++ b/pkg/skaffold/jib/jib_maven.go @@ -40,7 +40,11 @@ func GetDependenciesMaven(ctx context.Context, workspace string, a *latest.JibMa } func getCommandMaven(ctx context.Context, workspace string, a *latest.JibMavenArtifact) *exec.Cmd { - args := []string{"--quiet"} + var args []string + args = append(args, "--quiet") + if a.Profile != "" { + args = append(args, "--activate-profiles", a.Profile) + } if a.Module == "" { // single-module project args = append(args, "--non-recursive") @@ -49,27 +53,26 @@ func getCommandMaven(ctx context.Context, workspace string, a *latest.JibMavenAr args = append(args, "--projects", a.Module, "--also-make") } args = append(args, "jib:_skaffold-files") - if a.Profile != "" { - args = append(args, "--activate-profiles", a.Profile) - } return MavenCommand.CreateCommand(ctx, workspace, args) } // GenerateMavenArgs generates the arguments to Maven for building the project as an image. -func GenerateMavenArgs(goal string, imageName string, artifact *latest.JibMavenArtifact) []string { - var command []string - if artifact.Module == "" { +func GenerateMavenArgs(goal string, imageName string, a *latest.JibMavenArtifact) []string { + var args []string + if a.Profile != "" { + args = append(args, "--activate-profiles", a.Profile) + } + if a.Module == "" { // single-module project - command = []string{"--non-recursive", "prepare-package", "jib:" + goal} + args = append(args, "--non-recursive") + args = append(args, "prepare-package", "jib:"+goal) } else { // multi-module project: we assume `package` is bound to `jib:` - command = []string{"--projects", artifact.Module, "--also-make", "package"} - } - command = append(command, "-Dimage="+imageName) - if artifact.Profile != "" { - command = append(command, "--activate-profiles", artifact.Profile) + args = append(args, "--projects", a.Module, "--also-make") + args = append(args, "package") } + args = append(args, "-Dimage="+imageName) - return command + return args } diff --git a/pkg/skaffold/jib/jib_maven_test.go b/pkg/skaffold/jib/jib_maven_test.go index 44ddb0cd75d..56c9892901b 100644 --- a/pkg/skaffold/jib/jib_maven_test.go +++ b/pkg/skaffold/jib/jib_maven_test.go @@ -104,7 +104,7 @@ func TestGetCommandMaven(t *testing.T) { jibMavenArtifact: latest.JibMavenArtifact{Profile: "profile"}, filesInWorkspace: []string{}, expectedCmd: func(workspace string) *exec.Cmd { - return MavenCommand.CreateCommand(ctx, workspace, []string{"--quiet", "--non-recursive", "jib:_skaffold-files", "--activate-profiles", "profile"}) + return MavenCommand.CreateCommand(ctx, workspace, []string{"--quiet", "--activate-profiles", "profile", "--non-recursive", "jib:_skaffold-files"}) }, }, { @@ -128,7 +128,7 @@ func TestGetCommandMaven(t *testing.T) { jibMavenArtifact: latest.JibMavenArtifact{Profile: "profile"}, filesInWorkspace: []string{"mvnw", "mvnw.bat"}, expectedCmd: func(workspace string) *exec.Cmd { - return MavenCommand.CreateCommand(ctx, workspace, []string{"--quiet", "--non-recursive", "jib:_skaffold-files", "--activate-profiles", "profile"}) + return MavenCommand.CreateCommand(ctx, workspace, []string{"--quiet", "--activate-profiles", "profile", "--non-recursive", "jib:_skaffold-files"}) }, }, { @@ -165,9 +165,9 @@ func TestGenerateMavenArgs(t *testing.T) { out []string }{ {latest.JibMavenArtifact{}, []string{"--non-recursive", "prepare-package", "jib:goal", "-Dimage=image"}}, - {latest.JibMavenArtifact{Profile: "profile"}, []string{"--non-recursive", "prepare-package", "jib:goal", "-Dimage=image", "--activate-profiles", "profile"}}, + {latest.JibMavenArtifact{Profile: "profile"}, []string{"--activate-profiles", "profile", "--non-recursive", "prepare-package", "jib:goal", "-Dimage=image"}}, {latest.JibMavenArtifact{Module: "module"}, []string{"--projects", "module", "--also-make", "package", "-Dimage=image"}}, - {latest.JibMavenArtifact{Module: "module", Profile: "profile"}, []string{"--projects", "module", "--also-make", "package", "-Dimage=image", "--activate-profiles", "profile"}}, + {latest.JibMavenArtifact{Module: "module", Profile: "profile"}, []string{"--activate-profiles", "profile", "--projects", "module", "--also-make", "package", "-Dimage=image"}}, } for _, tt := range testCases {