From 2ec1c935eec1eec8e4bf6ceb52aa746118c7bea5 Mon Sep 17 00:00:00 2001 From: Aaron Prindle Date: Thu, 15 Apr 2021 11:59:52 -0700 Subject: [PATCH] add skaffold build --push flag What is the problem being solved? Fixes #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 #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 | 1 + docs/content/en/docs/references/cli/_index.md | 2 ++ pkg/skaffold/config/options.go | 1 + pkg/skaffold/runner/build_deploy_test.go | 18 ++++++++++++++++++ pkg/skaffold/runner/new.go | 9 +++++++-- 5 files changed, 29 insertions(+), 2 deletions(-) diff --git a/cmd/skaffold/app/cmd/build.go b/cmd/skaffold/app/cmd/build.go index 5b21c4cd026..a25c3f20ff5 100644 --- a/cmd/skaffold/app/cmd/build.go +++ b/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) diff --git a/docs/content/en/docs/references/cli/_index.md b/docs/content/en/docs/references/cli/_index.md index 7bbfe4b95a0..c7e41e80f82 100644 --- a/docs/content/en/docs/references/cli/_index.md +++ b/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`) diff --git a/pkg/skaffold/config/options.go b/pkg/skaffold/config/options.go index bb80c2afe82..93338e947c9 100644 --- a/pkg/skaffold/config/options.go +++ b/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 diff --git a/pkg/skaffold/runner/build_deploy_test.go b/pkg/skaffold/runner/build_deploy_test.go index 62df92f46a9..cb4da7f980e 100644 --- a/pkg/skaffold/runner/build_deploy_test.go +++ b/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"}, diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index 75e06d7a9b3..529b9127eb4 100644 --- a/pkg/skaffold/runner/new.go +++ b/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