Cobra context (#3842)

* Update cobra to 0.6.0 Signed-off-by: David Gageot <david@gageot.net> * Simplify context handling Signed-off-by: David Gageot <david@gageot.net>
GoogleContainerTools · Mar 23, 2020 · cd02979 · cd02979
1 parent 08680df
commit cd02979
Show file tree

Hide file tree

Showing 38 changed files with 374 additions and 120 deletions.
diff --git a/cmd/skaffold/app/cmd/build.go b/cmd/skaffold/app/cmd/build.go
@@ -55,7 +55,7 @@ func NewCmdBuild() *cobra.Command {
 			f.VarP(buildFormatFlag, "output", "o", "Used in conjunction with --quiet flag. "+buildFormatFlag.Usage())
 			f.StringVar(&buildOutputFlag, "file-output", "", "Filename to write build images to")
 		}).
-		NoArgs(cancelWithCtrlC(context.Background(), doBuild))
+		NoArgs(doBuild)
 }
 
 func doBuild(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/commands.go b/cmd/skaffold/app/cmd/commands.go
@@ -17,6 +17,7 @@ limitations under the License.
 package cmd
 
 import (
+	"context"
 	"fmt"
 	"io"
 
@@ -32,8 +33,8 @@ type Builder interface {
 	WithFlags(adder func(*pflag.FlagSet)) Builder
 	WithCommonFlags() Builder
 	Hidden() Builder
-	ExactArgs(argCount int, action func(io.Writer, []string) error) *cobra.Command
-	NoArgs(action func(io.Writer) error) *cobra.Command
+	ExactArgs(argCount int, action func(context.Context, io.Writer, []string) error) *cobra.Command
+	NoArgs(action func(context.Context, io.Writer) error) *cobra.Command
 }
 
 type builder struct {
@@ -82,18 +83,18 @@ func (b *builder) Hidden() Builder {
 	return b
 }
 
-func (b *builder) ExactArgs(argCount int, action func(io.Writer, []string) error) *cobra.Command {
+func (b *builder) ExactArgs(argCount int, action func(context.Context, io.Writer, []string) error) *cobra.Command {
 	b.cmd.Args = cobra.ExactArgs(argCount)
 	b.cmd.RunE = func(_ *cobra.Command, args []string) error {
-		return action(b.cmd.OutOrStdout(), args)
+		return action(b.cmd.Context(), b.cmd.OutOrStdout(), args)
 	}
 	return &b.cmd
 }
 
-func (b *builder) NoArgs(action func(io.Writer) error) *cobra.Command {
+func (b *builder) NoArgs(action func(context.Context, io.Writer) error) *cobra.Command {
 	b.cmd.Args = cobra.NoArgs
 	b.cmd.RunE = func(*cobra.Command, []string) error {
-		return action(b.cmd.OutOrStdout())
+		return action(b.cmd.Context(), b.cmd.OutOrStdout())
 	}
 	return &b.cmd
 }
diff --git a/cmd/skaffold/app/cmd/commands_test.go b/cmd/skaffold/app/cmd/commands_test.go
@@ -18,6 +18,7 @@ package cmd
 
 import (
 	"bytes"
+	"context"
 	"errors"
 	"fmt"
 	"io"
@@ -70,7 +71,7 @@ func TestNewCmdExactArgs(t *testing.T) {
 }
 
 func TestNewCmdError(t *testing.T) {
-	cmd := NewCmd("").NoArgs(func(out io.Writer) error {
+	cmd := NewCmd("").NoArgs(func(ctx context.Context, out io.Writer) error {
 		return errors.New("expected error")
 	})
 
@@ -82,7 +83,7 @@ func TestNewCmdError(t *testing.T) {
 func TestNewCmdOutput(t *testing.T) {
 	var buf bytes.Buffer
 
-	cmd := NewCmd("").ExactArgs(1, func(out io.Writer, args []string) error {
+	cmd := NewCmd("").ExactArgs(1, func(ctx context.Context, out io.Writer, args []string) error {
 		fmt.Fprintf(out, "test output: %v\n", args)
 		return nil
 	})

diff --git a/cmd/skaffold/app/cmd/config/list.go b/cmd/skaffold/app/cmd/config/list.go
@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"context"
 	"fmt"
 	"io"
 
@@ -26,7 +27,7 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
 )
 
-func List(out io.Writer) error {
+func List(ctx context.Context, out io.Writer) error {
 	var configYaml []byte
 	if showAll {
 		cfg, err := config.ReadConfigFile(configFile)

diff --git a/cmd/skaffold/app/cmd/config/list_test.go b/cmd/skaffold/app/cmd/config/list_test.go
@@ -18,6 +18,7 @@ package config
 
 import (
 	"bytes"
+	"context"
 	"strings"
 	"testing"
 
@@ -163,7 +164,7 @@ kubeContexts:
 
 			buf := &bytes.Buffer{}
 			// list values
-			err := List(buf)
+			err := List(context.Background(), buf)
 			t.CheckNoError(err)
 
 			if test.expectedOutput != "" && !strings.HasSuffix(buf.String(), test.expectedOutput) {

diff --git a/cmd/skaffold/app/cmd/config/set.go b/cmd/skaffold/app/cmd/config/set.go
@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"context"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -40,7 +41,7 @@ type cfgStruct struct {
 	idx   []int
 }
 
-func Set(out io.Writer, args []string) error {
+func Set(ctx context.Context, out io.Writer, args []string) error {
 	if err := setConfigValue(args[0], args[1]); err != nil {
 		return err
 	}

diff --git a/cmd/skaffold/app/cmd/config/set_test.go b/cmd/skaffold/app/cmd/config/set_test.go
@@ -17,6 +17,7 @@ limitations under the License.
 package config
 
 import (
+	"context"
 	"io/ioutil"
 	"testing"
 
@@ -243,7 +244,7 @@ func TestSetAndUnsetConfig(t *testing.T) {
 			}
 
 			// set specified value
-			err := Set(ioutil.Discard, []string{test.key, test.value})
+			err := Set(context.Background(), ioutil.Discard, []string{test.key, test.value})
 			actualConfig, cfgErr := config.ReadConfigFile(cfg)
 			t.CheckNoError(cfgErr)
 			t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expectedSetCfg, actualConfig)
@@ -254,7 +255,7 @@ func TestSetAndUnsetConfig(t *testing.T) {
 			}
 
 			// unset the value
-			err = Unset(ioutil.Discard, []string{test.key})
+			err = Unset(context.Background(), ioutil.Discard, []string{test.key})
 			newConfig, cfgErr := config.ReadConfigFile(cfg)
 			t.CheckNoError(cfgErr)
 

diff --git a/cmd/skaffold/app/cmd/config/unset.go b/cmd/skaffold/app/cmd/config/unset.go
@@ -17,11 +17,12 @@ limitations under the License.
 package config
 
 import (
+	"context"
 	"fmt"
 	"io"
 )
 
-func Unset(out io.Writer, args []string) error {
+func Unset(ctx context.Context, out io.Writer, args []string) error {
 	if err := unsetConfigValue(args[0]); err != nil {
 		return err
 	}

diff --git a/cmd/skaffold/app/cmd/credits/export.go b/cmd/skaffold/app/cmd/credits/export.go
@@ -17,6 +17,7 @@ limitations under the License.
 package credits
 
 import (
+	"context"
 	"io"
 	"io/ioutil"
 	"log"
@@ -33,7 +34,7 @@ import (
 var Path string
 
 // Export writes all the licenses and credit files to the `Path` folder.
-func Export(out io.Writer) error {
+func Export(ctx context.Context, out io.Writer) error {
 	statikFS, err := statik.FS()
 	if err != nil {
 		return errors.Wrap(err, "opening embedded filesystem")

diff --git a/cmd/skaffold/app/cmd/debug.go b/cmd/skaffold/app/cmd/debug.go
@@ -32,7 +32,7 @@ func NewCmdDebug() *cobra.Command {
 		WithDescription("[beta] Run a pipeline in debug mode").
 		WithLongDescription("Similar to `dev`, but configures the pipeline for debugging.").
 		WithCommonFlags().
-		NoArgs(cancelWithCtrlC(context.Background(), doDebug))
+		NoArgs(doDebug)
 }
 
 func doDebug(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/delete.go b/cmd/skaffold/app/cmd/delete.go
@@ -31,7 +31,7 @@ func NewCmdDelete() *cobra.Command {
 	return NewCmd("delete").
 		WithDescription("Delete the deployed application").
 		WithCommonFlags().
-		NoArgs(cancelWithCtrlC(context.Background(), doDelete))
+		NoArgs(doDelete)
 }
 
 func doDelete(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/deploy.go b/cmd/skaffold/app/cmd/deploy.go
@@ -49,7 +49,7 @@ func NewCmdDeploy() *cobra.Command {
 			f.VarP(&buildOutputFile, "build-artifacts", "a", `Filepath containing build output.
 E.g. build.out created by running skaffold build --quiet -o "{{json .}}" > build.out`)
 		}).
-		NoArgs(cancelWithCtrlC(context.Background(), doDeploy))
+		NoArgs(doDeploy)
 }
 
 func doDeploy(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/dev.go b/cmd/skaffold/app/cmd/dev.go
@@ -45,7 +45,7 @@ func NewCmdDev() *cobra.Command {
 			f.StringSliceVarP(&opts.TargetImages, "watch-image", "w", nil, "Choose which artifacts to watch. Artifacts with image names that contain the expression will be watched only. Default is to watch sources for all artifacts")
 			f.IntVarP(&opts.WatchPollInterval, "watch-poll-interval", "i", 1000, "Interval (in ms) between two checks for file changes")
 		}).
-		NoArgs(cancelWithCtrlC(context.Background(), doDev))
+		NoArgs(doDev)
 }
 
 func doDev(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/diagnose.go b/cmd/skaffold/app/cmd/diagnose.go
@@ -36,7 +36,7 @@ func NewCmdDiagnose() *cobra.Command {
 	return NewCmd("diagnose").
 		WithDescription("Run a diagnostic on Skaffold").
 		WithCommonFlags().
-		NoArgs(cancelWithCtrlC(context.Background(), doDiagnose))
+		NoArgs(doDiagnose)
 }
 
 func doDiagnose(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/find_configs.go b/cmd/skaffold/app/cmd/find_configs.go
@@ -17,6 +17,7 @@ limitations under the License.
 package cmd
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -50,7 +51,7 @@ func NewCmdFindConfigs() *cobra.Command {
 		NoArgs(doFindConfigs)
 }
 
-func doFindConfigs(out io.Writer) error {
+func doFindConfigs(_ context.Context, out io.Writer) error {
 	pathToVersion, err := findConfigs(directory)
 	if err != nil {
 		return err

diff --git a/cmd/skaffold/app/cmd/fix.go b/cmd/skaffold/app/cmd/fix.go
@@ -17,6 +17,7 @@ limitations under the License.
 package cmd
 
 import (
+	"context"
 	"io"
 	"io/ioutil"
 
@@ -41,7 +42,7 @@ func NewCmdFix() *cobra.Command {
 		NoArgs(doFix)
 }
 
-func doFix(out io.Writer) error {
+func doFix(_ context.Context, out io.Writer) error {
 	return fix(out, opts.ConfigurationFile, overwrite)
 }
 

diff --git a/cmd/skaffold/app/cmd/generate_pipeline.go b/cmd/skaffold/app/cmd/generate_pipeline.go
@@ -41,7 +41,7 @@ func NewCmdGeneratePipeline() *cobra.Command {
 		WithFlags(func(f *pflag.FlagSet) {
 			f.StringSliceVar(&configFiles, "config-files", nil, "Select additional files whose artifacts to use when generating pipeline.")
 		}).
-		NoArgs(cancelWithCtrlC(context.Background(), doGeneratePipeline))
+		NoArgs(doGeneratePipeline)
 }
 
 func doGeneratePipeline(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/init.go b/cmd/skaffold/app/cmd/init.go
@@ -72,7 +72,7 @@ func NewCmdInit() *cobra.Command {
 			f.BoolVar(&enableManifestGeneration, "XXenableManifestGeneration", false, "")
 			f.MarkHidden("XXenableManifestGeneration")
 		}).
-		NoArgs(cancelWithCtrlC(context.Background(), doInit))
+		NoArgs(doInit)
 }
 
 func doInit(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/render.go b/cmd/skaffold/app/cmd/render.go
@@ -43,7 +43,7 @@ func NewCmdRender() *cobra.Command {
 			f.BoolVar(&showBuild, "loud", false, "Show the build logs and output")
 			f.StringVar(&renderOutputPath, "output", "", "file to write rendered manifests to")
 		}).
-		NoArgs(cancelWithCtrlC(context.Background(), doRender))
+		NoArgs(doRender)
 }
 
 func doRender(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/run.go b/cmd/skaffold/app/cmd/run.go
@@ -36,7 +36,7 @@ func NewCmdRun() *cobra.Command {
 		WithExample("Build, test, deploy and tail the logs", "run --tail").
 		WithExample("Run with a given profile", "run -p <profile>").
 		WithCommonFlags().
-		NoArgs(cancelWithCtrlC(context.Background(), doRun))
+		NoArgs(doRun)
 }
 
 func doRun(ctx context.Context, out io.Writer) error {

diff --git a/cmd/skaffold/app/cmd/schema.go b/cmd/skaffold/app/cmd/schema.go
@@ -17,6 +17,7 @@ limitations under the License.
 package cmd
 
 import (
+	"context"
 	"io"
 
 	"github.com/spf13/cobra"
@@ -46,7 +47,7 @@ func NewCmdSchemaGet() *cobra.Command {
 	return NewCmd("get").
 		WithDescription("Print a given skaffold.yaml's json schema").
 		WithExample("Print the schema in version `skaffold/v1`", "schema get skaffold/v1").
-		ExactArgs(1, func(out io.Writer, args []string) error {
+		ExactArgs(1, func(_ context.Context, out io.Writer, args []string) error {
 			version := args[0]
 			return schema.Print(out, version)
 		})

diff --git a/cmd/skaffold/app/cmd/schema/list.go b/cmd/skaffold/app/cmd/schema/list.go
@@ -17,14 +17,15 @@ limitations under the License.
 package schema
 
 import (
+	"context"
 	"fmt"
 	"io"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema"
 )
 
 // List prints to `out` all supported schema versions.
-func List(out io.Writer) error {
+func List(_ context.Context, out io.Writer) error {
 	for _, version := range schema.SchemaVersions {
 		fmt.Fprintln(out, version.APIVersion)
 	}

diff --git a/cmd/skaffold/app/cmd/schema/list_test.go b/cmd/skaffold/app/cmd/schema/list_test.go
@@ -18,6 +18,7 @@ package schema
 
 import (
 	"bytes"
+	"context"
 	"strings"
 	"testing"
 
@@ -29,7 +30,7 @@ func TestList(t *testing.T) {
 	testutil.Run(t, "", func(t *testutil.T) {
 		var out bytes.Buffer
 
-		err := List(&out)
+		err := List(context.Background(), &out)
 
 		versions := out.String()
 		t.CheckNoError(err)

diff --git a/cmd/skaffold/app/cmd/version.go b/cmd/skaffold/app/cmd/version.go
@@ -17,6 +17,7 @@ limitations under the License.
 package cmd
 
 import (
+	"context"
 	"io"
 
 	"github.com/spf13/cobra"
@@ -37,6 +38,6 @@ func NewCmdVersion() *cobra.Command {
 		NoArgs(doVersion)
 }
 
-func doVersion(out io.Writer) error {
+func doVersion(_ context.Context, out io.Writer) error {
 	return versionFlag.Template().Execute(out, version.Get())
 }
diff --git a/cmd/skaffold/app/cmd/signals.go → cmd/skaffold/app/signals.go b/cmd/skaffold/app/cmd/signals.go → cmd/skaffold/app/signals.go
@@ -14,26 +14,15 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cmd
+package app
 
 import (
 	"context"
-	"io"
 	"os"
 	"os/signal"
 	"syscall"
 )
 
-func cancelWithCtrlC(ctx context.Context, action func(context.Context, io.Writer) error) func(io.Writer) error {
-	return func(out io.Writer) error {
-		ctx, cancel := context.WithCancel(ctx)
-		defer cancel()
-
-		catchCtrlC(cancel)
-		return action(ctx, out)
-	}
-}
-
 func catchCtrlC(cancel context.CancelFunc) {
 	signals := make(chan os.Signal, 1)
 	signal.Notify(signals,