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

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: true, Usage: "Push the built images to the specified image repository.", IsEnum: true},
 		}).
 		WithHouseKeepingMessages().
 		NoArgs(doBuild)

cmd/skaffold/app/cmd/init_test.go

@@ -157,7 +157,7 @@ func TestFlagsToConfigVersion(t *testing.T) {
 
 			// we ignore Skaffold options
 			test.expectedConfig.Opts = capturedConfig.Opts
-			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedConfig, capturedConfig, cmp.AllowUnexported(cfg.StringOrUndefined{}))
+			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedConfig, capturedConfig, cmp.AllowUnexported(cfg.StringOrUndefined{}, cfg.BoolOrUndefined{}))
 		})
 	}
 }

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=true: 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/config/types.go

@@ -17,9 +17,48 @@ limitations under the License.
 package config
 
 import (
+	"strconv"
+
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
+// BoolOrUndefined holds the value of a flag of type `bool`,
+// that's by default `undefined`.
+// We use this instead of just `bool` to differentiate `undefined`
+// and `[true|false]` bool values.
+type BoolOrUndefined struct {
+	value *bool
+}
+
+func (s *BoolOrUndefined) Type() string {
+	return "bool"
+}
+
+func (s *BoolOrUndefined) Value() *bool {
+	return s.value
+}
+
+func (s *BoolOrUndefined) Set(v string) error {
+	val, err := strconv.ParseBool(v)
+	if err != nil {
+		return err
+	}
+	s.value = &val
+	return nil
+}
+
+func (s *BoolOrUndefined) SetNil() error {
+	s.value = nil
+	return nil
+}
+
+func (s *BoolOrUndefined) String() string {
+	if s.value == nil {
+		return ""
+	}
+	return strconv.FormatBool(*s.value)
+}
+
 // StringOrUndefined holds the value of a flag of type `string`,
 // that's by default `undefined`.
 // We use this instead of just `string` to differentiate `undefined`

pkg/skaffold/config/types_test.go

@@ -84,6 +84,63 @@ func TestStringOrUndefined(t *testing.T) {
 	}
 }
 
+func TestBoolOrUndefinedUsage(t *testing.T) {
+	var output bytes.Buffer
+
+	cmd := &cobra.Command{}
+	cmd.Flags().Var(&BoolOrUndefined{}, "flag", "use it like this")
+	cmd.SetOutput(&output)
+	cmd.Usage()
+
+	testutil.CheckDeepEqual(t, "Usage:\n\nFlags:\n      --flag   use it like this\n", output.String())
+}
+
+func TestBoolOrUndefined_SetNil(t *testing.T) {
+	var s BoolOrUndefined
+	s.Set("true")
+	testutil.CheckDeepEqual(t, "true", s.String())
+	s.SetNil()
+	testutil.CheckDeepEqual(t, "", s.String())
+	testutil.CheckDeepEqual(t, (*bool)(nil), s.value)
+	testutil.CheckDeepEqual(t, (*bool)(nil), s.Value())
+}
+
+func TestBoolOrUndefined(t *testing.T) {
+	tests := []struct {
+		description string
+		args        []string
+		expected    *bool
+	}{
+		{
+			description: "undefined",
+			args:        []string{},
+			expected:    nil,
+		},
+		{
+			description: "set true",
+			args:        []string{"--flag=true"},
+			expected:    util.BoolPtr(true),
+		},
+		{
+			description: "set false",
+			args:        []string{"--flag=false"},
+			expected:    util.BoolPtr(false),
+		},
+	}
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			var flag BoolOrUndefined
+
+			cmd := &cobra.Command{}
+			cmd.Flags().Var(&flag, "flag", "")
+			cmd.SetArgs(test.args)
+			cmd.Execute()
+
+			t.CheckDeepEqual(test.expected, flag.value)
+		})
+	}
+}
+
 func TestMuted(t *testing.T) {
 	tests := []struct {
 		phases                  []string

pkg/skaffold/runner/build_deploy_test.go

@@ -24,6 +24,7 @@ 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"
@@ -171,6 +172,23 @@ 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)
+		val := config.BoolOrUndefined{}
+		val.Set("true")
+		runner.runCtx.Opts.PushImages = val
+		_, 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")
+		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