add skaffold build --push flag

What is the problem being solved?
Fixes GoogleContainerTools#5667, adding --push flag to skaffold build.  This --push flag overrides any skaffold.yaml "build.local.push: false" configuration for a build allowing easier artifact pushing after local dev without having to modify yaml configuration.

Why is this the best approach?
This approach follows the style and convention for adding a flag that has been used prior for adding a flag used by a single command, similar to GoogleContainerTools#4039. Additionally this approach does not change the skaffold build default push behaviour to be false, instead only taking precendence when the --push flag is set true which is the intended use case.

What other approaches did you consider?
N/A

What side effects will this approach have?
The addition of this single flag should not have side effects on skaffold functionally outside of --push, the default behavior for skaffold build will be preserved w/o --push specified.

What future work remains to be done?
N/A

Author: aaron-prindle

cmd/skaffold/app/cmd/build.go

@@ -55,6 +55,7 @@ func NewCmdBuild() *cobra.Command {
 			{Value: buildFormatFlag, Name: "output", Shorthand: "o", DefValue: defaultBuildFormatTemplate, Usage: "Used in conjunction with --quiet flag. " + buildFormatFlag.Usage()},
 			{Value: &buildOutputFlag, Name: "file-output", DefValue: "", Usage: "Filename to write build images to"},
 			{Value: &opts.DryRun, Name: "dry-run", DefValue: false, Usage: "Don't build images, just compute the tag for each artifact.", IsEnum: true},
+			{Value: &opts.PushImages, Name: "push", DefValue: nil, Usage: "Push the built images to the specified image repository.", IsEnum: true, NoOptDefVal: "true"},
 		}).
 		WithHouseKeepingMessages().
 		NoArgs(doBuild)

docs/content/en/docs/references/cli/_index.md

@@ -202,6 +202,7 @@ Options:
   -o, --output={{json .}}: Used in conjunction with --quiet flag. Format output with go-template. For full struct documentation, see https://godoc.org/github.com/GoogleContainerTools/skaffold/cmd/skaffold/app/flags#BuildOutput
   -p, --profile=[]: Activate profiles by name (prefixed with `-` to disable a profile)
       --profile-auto-activation=true: Set to false to disable profile auto activation
+      --push=: Push the built images to the specified image repository.
   -q, --quiet=false: Suppress the build output and print image built on success. See --output to format output.
       --remote-cache-dir='': Specify the location of the git repositories cache (default $HOME/.skaffold/repos)
       --rpc-http-port=50052: tcp port to expose event REST API over HTTP
@@ -239,6 +240,7 @@ Env vars:
 * `SKAFFOLD_OUTPUT` (same as `--output`)
 * `SKAFFOLD_PROFILE` (same as `--profile`)
 * `SKAFFOLD_PROFILE_AUTO_ACTIVATION` (same as `--profile-auto-activation`)
+* `SKAFFOLD_PUSH` (same as `--push`)
 * `SKAFFOLD_QUIET` (same as `--quiet`)
 * `SKAFFOLD_REMOTE_CACHE_DIR` (same as `--remote-cache-dir`)
 * `SKAFFOLD_RPC_HTTP_PORT` (same as `--rpc-http-port`)

pkg/skaffold/config/options.go

@@ -79,6 +79,7 @@ type SkaffoldOptions struct {
 	DigestSource       string
 	WatchPollInterval  int
 	DefaultRepo        StringOrUndefined
+	PushImages         BoolOrUndefined
 	CustomLabels       []string
 	TargetImages       []string
 	Profiles           []string

pkg/skaffold/runner/build_deploy_test.go

@@ -24,9 +24,11 @@ import (
 
 	"k8s.io/client-go/tools/clientcmd/api"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/client"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
@@ -171,6 +173,22 @@ func TestBuildDryRun(t *testing.T) {
 	})
 }
 
+func TestBuildPushFlag(t *testing.T) {
+	testutil.Run(t, "", func(t *testutil.T) {
+		testBench := &TestBench{}
+		artifacts := []*latest.Artifact{
+			{ImageName: "img1"},
+			{ImageName: "img2"},
+		}
+		runner := createRunner(t, testBench, nil, artifacts, nil)
+		runner.runCtx.Opts.PushImages = config.NewBoolOrUndefined(util.BoolPtr(true))
+
+		_, err := runner.Build(context.Background(), ioutil.Discard, artifacts)
+
+		t.CheckNoError(err)
+	})
+}
+
 func TestDigestSources(t *testing.T) {
 	artifacts := []*latest.Artifact{
 		{ImageName: "img1"},

pkg/skaffold/runner/new.go

@@ -188,10 +188,15 @@ func isImageLocal(runCtx *runcontext.RunContext, imageName string) (bool, error)
 
 	cl := runCtx.GetCluster()
 	var pushImages bool
-	if pipeline.Build.LocalBuild.Push == nil {
+
+	switch {
+	case runCtx.Opts.PushImages.Value() != nil:
+		logrus.Debugf("push value set via skaffold build --push flag, --push=%t", *runCtx.Opts.PushImages.Value())
+		pushImages = *runCtx.Opts.PushImages.Value()
+	case pipeline.Build.LocalBuild.Push == nil:
 		pushImages = cl.PushImages
 		logrus.Debugf("push value not present, defaulting to %t because cluster.PushImages is %t", pushImages, cl.PushImages)
-	} else {
+	default:
 		pushImages = *pipeline.Build.LocalBuild.Push
 	}
 	return !pushImages, nil

pkg/skaffold/runner/new_test.go

@@ -20,6 +20,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kpt"
@@ -238,3 +239,95 @@ func TestGetDefaultDeployer(tOuter *testing.T) {
 		}
 	})
 }
+
+func TestIsImageLocal(t *testing.T) {
+	tests := []struct {
+		description       string
+		pushImagesFlagVal *bool
+		localBuildConfig  *bool
+		expected          bool
+	}{
+		{
+			description:       "skaffold build --push=nil, pipeline.Build.LocalBuild.Push=nil",
+			pushImagesFlagVal: nil,
+			localBuildConfig:  nil,
+			expected:          false,
+		},
+		{
+			description:       "skaffold build --push=nil, pipeline.Build.LocalBuild.Push=false",
+			pushImagesFlagVal: nil,
+			localBuildConfig:  util.BoolPtr(false),
+			expected:          true,
+		},
+		{
+			description:       "skaffold build --push=nil, pipeline.Build.LocalBuild.Push=true",
+			pushImagesFlagVal: nil,
+			localBuildConfig:  util.BoolPtr(true),
+			expected:          false,
+		},
+		{
+			description:       "skaffold build --push=false, pipeline.Build.LocalBuild.Push=nil",
+			pushImagesFlagVal: util.BoolPtr(false),
+			localBuildConfig:  nil,
+			expected:          true,
+		},
+		{
+			description:       "skaffold build --push=false, pipeline.Build.LocalBuild.Push=false",
+			pushImagesFlagVal: util.BoolPtr(false),
+			localBuildConfig:  util.BoolPtr(false),
+			expected:          true,
+		},
+		{
+			description:       "skaffold build --push=false, pipeline.Build.LocalBuild.Push=true",
+			pushImagesFlagVal: util.BoolPtr(false),
+			localBuildConfig:  util.BoolPtr(true),
+			expected:          true,
+		},
+		{
+			description:       "skaffold build --push=true, pipeline.Build.LocalBuild.Push=nil",
+			pushImagesFlagVal: util.BoolPtr(true),
+			localBuildConfig:  nil,
+			expected:          false,
+		},
+		{
+			description:       "skaffold build --push=true, pipeline.Build.LocalBuild.Push=nil",
+			pushImagesFlagVal: util.BoolPtr(true),
+			localBuildConfig:  util.BoolPtr(false),
+			expected:          false,
+		},
+		{
+			description:       "skaffold build --push=true, pipeline.Build.LocalBuild.Push=nil",
+			pushImagesFlagVal: util.BoolPtr(true),
+			localBuildConfig:  util.BoolPtr(true),
+			expected:          false,
+		},
+	}
+	imageName := "testImage"
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			rctx := &runcontext.RunContext{
+				Cluster: config.Cluster{
+					PushImages: true,
+				},
+				Opts: config.SkaffoldOptions{
+					PushImages: config.NewBoolOrUndefined(test.pushImagesFlagVal),
+				},
+				Pipelines: runcontext.NewPipelines([]latest.Pipeline{{
+					Build: latest.BuildConfig{
+						Artifacts: []*latest.Artifact{
+							{ImageName: imageName},
+						},
+						BuildType: latest.BuildType{
+							LocalBuild: &latest.LocalBuild{
+								Push: test.localBuildConfig,
+							},
+						},
+					},
+				}})}
+			output, _ := isImageLocal(rctx, imageName)
+			if output != test.expected {
+				t.Errorf("isImageLocal output was %t, expected: %t", output, test.expected)
+			}
+		})
+	}
+}