Skip to content

Commit

Permalink
Merge pull request #1454 from dgageot/jib-show-duplication
Browse files Browse the repository at this point in the history
Show duplication in jib code
  • Loading branch information
dgageot authored Jan 14, 2019
2 parents ae14267 + add22d8 commit db5e1ad
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 76 deletions.
17 changes: 2 additions & 15 deletions pkg/skaffold/build/local/jib_gradle.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
22 changes: 2 additions & 20 deletions pkg/skaffold/build/local/jib_maven.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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:<goal>`
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 {
Expand Down
34 changes: 0 additions & 34 deletions pkg/skaffold/build/local/jib_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
25 changes: 22 additions & 3 deletions pkg/skaffold/jib/jib_gradle.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,29 @@ 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, a *latest.JibGradleArtifact) []string {
var command string
if a.Project == "" {
command = ":" + task
} else {
// multi-module
command = fmt.Sprintf(":%s:%s", a.Project, task)
}

return []string{command, "--image=" + imageName}
}
16 changes: 16 additions & 0 deletions pkg/skaffold/jib/jib_gradle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
25 changes: 23 additions & 2 deletions pkg/skaffold/jib/jib_maven.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -49,9 +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")

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, a *latest.JibMavenArtifact) []string {
var args []string
if a.Profile != "" {
args = append(args, "--activate-profiles", a.Profile)
}
if a.Module == "" {
// single-module project
args = append(args, "--non-recursive")
args = append(args, "prepare-package", "jib:"+goal)
} else {
// multi-module project: we assume `package` is bound to `jib:<goal>`
args = append(args, "--projects", a.Module, "--also-make")
args = append(args, "package")
}
args = append(args, "-Dimage="+imageName)

return MavenCommand.CreateCommand(ctx, workspace, args)
return args
}
22 changes: 20 additions & 2 deletions pkg/skaffold/jib/jib_maven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"})
},
},
{
Expand All @@ -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"})
},
},
{
Expand Down Expand Up @@ -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{"--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{"--activate-profiles", "profile", "--projects", "module", "--also-make", "package", "-Dimage=image"}},
}

for _, tt := range testCases {
args := GenerateMavenArgs("goal", "image", &tt.in)

testutil.CheckDeepEqual(t, tt.out, args)
}
}

0 comments on commit db5e1ad

Please sign in to comment.