Merge pull request #1366 from dgageot/minor-jib-refactoring

Extract push/no-push logic into builder

Author: dgageot

pkg/skaffold/build/local/jib_gradle.go

@@ -28,9 +28,16 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-func (b *Builder) buildJibGradleToDocker(ctx context.Context, out io.Writer, workspace string, a *latest.JibGradleArtifact) (string, error) {
-	skaffoldImage := generateJibImageRef(workspace, a.Project)
-	args := generateGradleArgs("jibDockerBuild", skaffoldImage, a)
+func (b *Builder) buildJibGradle(ctx context.Context, out io.Writer, workspace string, artifact *latest.Artifact) (string, error) {
+	if b.pushImages {
+		return b.buildJibGradleToRegistry(ctx, out, workspace, artifact)
+	}
+	return b.buildJibGradleToDocker(ctx, out, workspace, artifact.JibGradleArtifact)
+}
+
+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)
 
 	if err := runGradleCommand(ctx, out, workspace, args); err != nil {
 		return "", err
@@ -52,16 +59,16 @@ func (b *Builder) buildJibGradleToRegistry(ctx context.Context, out io.Writer, w
 }
 
 // generateGradleArgs generates the arguments to Gradle for building the project as an image called `skaffoldImage`.
-func generateGradleArgs(task string, skaffoldImage string, a *latest.JibGradleArtifact) []string {
+func generateGradleArgs(task string, imageName string, artifact *latest.JibGradleArtifact) []string {
 	var command string
-	if a.Project == "" {
+	if artifact.Project == "" {
 		command = ":" + task
 	} else {
 		// multi-module
-		command = fmt.Sprintf(":%s:%s", a.Project, task)
+		command = fmt.Sprintf(":%s:%s", artifact.Project, task)
 	}
 
-	return []string{command, "--image=" + skaffoldImage}
+	return []string{command, "--image=" + imageName}
 }
 
 func runGradleCommand(ctx context.Context, out io.Writer, workspace string, args []string) error {

pkg/skaffold/build/local/jib_maven.go

@@ -29,16 +29,23 @@ import (
 	"github.com/sirupsen/logrus"
 )
 
-func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, workspace string, a *latest.JibMavenArtifact) (string, error) {
+func (b *Builder) buildJibMaven(ctx context.Context, out io.Writer, workspace string, artifact *latest.Artifact) (string, error) {
+	if b.pushImages {
+		return buildJibMavenToRegistry(ctx, out, workspace, artifact)
+	}
+	return buildJibMavenToDocker(ctx, out, workspace, artifact.JibMavenArtifact)
+}
+
+func buildJibMavenToDocker(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibMavenArtifact) (string, error) {
 	// If this is a multi-module project, we require `package` be bound to jib:dockerBuild
-	if a.Module != "" {
-		if err := verifyJibPackageGoal(ctx, "dockerBuild", workspace, a); err != nil {
+	if artifact.Module != "" {
+		if err := verifyJibPackageGoal(ctx, "dockerBuild", workspace, artifact); err != nil {
 			return "", err
 		}
 	}
 
-	skaffoldImage := generateJibImageRef(workspace, a.Module)
-	args := generateMavenArgs("dockerBuild", skaffoldImage, a)
+	skaffoldImage := generateJibImageRef(workspace, artifact.Module)
+	args := generateMavenArgs("dockerBuild", skaffoldImage, artifact)
 
 	if err := runMavenCommand(ctx, out, workspace, args); err != nil {
 		return "", err
@@ -47,7 +54,7 @@ func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, work
 	return skaffoldImage, nil
 }
 
-func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.Artifact) (string, error) {
+func buildJibMavenToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.Artifact) (string, error) {
 	// If this is a multi-module project, we require `package` be bound to jib:build
 	if artifact.JibMavenArtifact.Module != "" {
 		if err := verifyJibPackageGoal(ctx, "build", workspace, artifact.JibMavenArtifact); err != nil {
@@ -67,30 +74,30 @@ func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, wo
 }
 
 // generateMavenArgs generates the arguments to Maven for building the project as an image called `skaffoldImage`.
-func generateMavenArgs(goal string, skaffoldImage string, a *latest.JibMavenArtifact) []string {
+func generateMavenArgs(goal string, imageName string, artifact *latest.JibMavenArtifact) []string {
 	var command []string
-	if a.Module == "" {
+	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", a.Module, "--also-make", "package"}
+		command = []string{"--projects", artifact.Module, "--also-make", "package"}
 	}
-	command = append(command, "-Dimage="+skaffoldImage)
-	if a.Profile != "" {
-		command = append(command, "--activate-profiles", a.Profile)
+	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, a *latest.JibMavenArtifact) error {
+func verifyJibPackageGoal(ctx context.Context, requiredGoal string, workspace string, artifact *latest.JibMavenArtifact) error {
 	// cannot use --non-recursive
-	command := []string{"--quiet", "--projects", a.Module, "jib:_skaffold-package-goals"}
-	if a.Profile != "" {
-		command = append(command, "--activate-profiles", a.Profile)
+	command := []string{"--quiet", "--projects", artifact.Module, "jib:_skaffold-package-goals"}
+	if artifact.Profile != "" {
+		command = append(command, "--activate-profiles", artifact.Profile)
 	}
 
 	cmd := jib.MavenCommand.CreateCommand(ctx, workspace, command)
@@ -100,7 +107,7 @@ func verifyJibPackageGoal(ctx context.Context, requiredGoal string, workspace st
 		return errors.Wrap(err, "could not obtain jib package goals")
 	}
 	goals := util.NonEmptyLines(stdout)
-	logrus.Debugf("jib bound package goals for %s %s: %v (%d)", workspace, a.Module, goals, len(goals))
+	logrus.Debugf("jib bound package goals for %s %s: %v (%d)", workspace, artifact.Module, goals, len(goals))
 	if len(goals) != 1 {
 		return errors.New("skaffold requires a single jib goal bound to 'package'")
 	}

pkg/skaffold/build/local/local.go

@@ -90,16 +90,10 @@ func (b *Builder) runBuildForArtifact(ctx context.Context, out io.Writer, artifa
 		return b.buildBazel(ctx, out, artifact.Workspace, artifact.BazelArtifact)
 
 	case artifact.JibMavenArtifact != nil:
-		if b.pushImages {
-			return b.buildJibMavenToRegistry(ctx, out, artifact.Workspace, artifact)
-		}
-		return b.buildJibMavenToDocker(ctx, out, artifact.Workspace, artifact.JibMavenArtifact)
+		return b.buildJibMaven(ctx, out, artifact.Workspace, artifact)
 
 	case artifact.JibGradleArtifact != nil:
-		if b.pushImages {
-			return b.buildJibGradleToRegistry(ctx, out, artifact.Workspace, artifact)
-		}
-		return b.buildJibGradleToDocker(ctx, out, artifact.Workspace, artifact.JibGradleArtifact)
+		return b.buildJibGradle(ctx, out, artifact.Workspace, artifact)
 
 	default:
 		return "", fmt.Errorf("undefined artifact type: %+v", artifact.ArtifactType)