Getting feedback into account

Signed-off-by: David Gageot <david@gageot.net>

cmd/skaffold/app/cmd/build.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"io"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
 	"github.com/spf13/cobra"
 )
 
@@ -31,11 +30,20 @@ func NewCmdBuild(out io.Writer) *cobra.Command {
 		Short: "Builds the artifacts",
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runSkaffold(out, filename, func(ctx context.Context, r *runner.SkaffoldRunner) error {
-				return r.Build(ctx)
-			})
+			return build(out, filename)
 		},
 	}
 	AddRunDevFlags(cmd)
 	return cmd
 }
+
+func build(out io.Writer, filename string) error {
+	ctx := context.Background()
+
+	runner, err := NewRunner(out, filename)
+	if err != nil {
+		return err
+	}
+
+	return runner.Build(ctx)
+}

cmd/skaffold/app/cmd/cmd.go

@@ -17,7 +17,6 @@ limitations under the License.
 package cmd
 
 import (
-	"context"
 	"io"
 
 	yaml "gopkg.in/yaml.v2"
@@ -90,6 +89,21 @@ func SetUpLogs(out io.Writer, level string) error {
 	return nil
 }
 
+func NewRunner(out io.Writer, filename string) (*runner.SkaffoldRunner, error) {
+	config, err := readConfiguration(filename)
+	if err != nil {
+		return nil, errors.Wrap(err, "reading configuration")
+	}
+
+	opts.Output = out
+	r, err := runner.NewForConfig(opts, config)
+	if err != nil {
+		return nil, errors.Wrap(err, "getting skaffold config")
+	}
+
+	return r, nil
+}
+
 func readConfiguration(filename string) (*config.SkaffoldConfig, error) {
 	buf, err := util.ReadConfiguration(filename)
 	if err != nil {
@@ -121,25 +135,3 @@ func readConfiguration(filename string) (*config.SkaffoldConfig, error) {
 
 	return latestConfig, nil
 }
-
-func runSkaffold(out io.Writer, filename string, action func(context.Context, *runner.SkaffoldRunner) error) error {
-	ctx := context.Background()
-
-	opts.Output = out
-
-	config, err := readConfiguration(filename)
-	if err != nil {
-		return errors.Wrap(err, "reading configuration")
-	}
-
-	r, err := runner.NewForConfig(opts, config)
-	if err != nil {
-		return errors.Wrap(err, "getting skaffold config")
-	}
-
-	if err := action(ctx, r); err != nil {
-		return errors.Wrap(err, "running skaffold steps")
-	}
-
-	return nil
-}

cmd/skaffold/app/cmd/dev.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"io"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
 	"github.com/spf13/cobra"
 )
 
@@ -31,12 +30,21 @@ func NewCmdDev(out io.Writer) *cobra.Command {
 		Short: "Runs a pipeline file in development mode",
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runSkaffold(out, filename, func(ctx context.Context, r *runner.SkaffoldRunner) error {
-				return r.Dev(ctx)
-			})
+			return dev(out, filename)
 		},
 	}
 	AddRunDevFlags(cmd)
 	AddDevFlags(cmd)
 	return cmd
 }
+
+func dev(out io.Writer, filename string) error {
+	ctx := context.Background()
+
+	runner, err := NewRunner(out, filename)
+	if err != nil {
+		return err
+	}
+
+	return runner.Build(ctx)
+}

cmd/skaffold/app/cmd/run.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"io"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner"
 	"github.com/spf13/cobra"
 )
 
@@ -31,13 +30,22 @@ func NewCmdRun(out io.Writer) *cobra.Command {
 		Short: "Runs a pipeline file",
 		Args:  cobra.NoArgs,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			return runSkaffold(out, filename, func(ctx context.Context, r *runner.SkaffoldRunner) error {
-				return r.Run(ctx)
-			})
+			return run(out, filename)
 		},
 	}
 	AddRunDevFlags(cmd)
 
 	cmd.Flags().StringVarP(&opts.CustomTag, "tag", "t", "", "The optional custom tag to use for images which overrides the current Tagger configuration")
 	return cmd
 }
+
+func run(out io.Writer, filename string) error {
+	ctx := context.Background()
+
+	runner, err := NewRunner(out, filename)
+	if err != nil {
+		return err
+	}
+
+	return runner.Run(ctx)
+}

pkg/skaffold/runner/runner.go

@@ -24,8 +24,6 @@ import (
 	"syscall"
 	"time"
 
-	"golang.org/x/sync/errgroup"
-
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
@@ -34,10 +32,10 @@ import (
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/watch"
-	clientgo "k8s.io/client-go/kubernetes"
-
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"golang.org/x/sync/errgroup"
+	clientgo "k8s.io/client-go/kubernetes"
 )
 
 // SkaffoldRunner is responsible for running the skaffold build and deploy pipeline.
@@ -68,24 +66,22 @@ func NewForConfig(opts *config.SkaffoldOptions, cfg *config.SkaffoldConfig) (*Sk
 	if err != nil {
 		return nil, errors.Wrap(err, "parsing skaffold build config")
 	}
+
 	deployer, err := getDeployer(&cfg.Deploy, kubeContext)
 	if err != nil {
 		return nil, errors.Wrap(err, "parsing skaffold deploy config")
 	}
-	tagger, err := newTaggerForConfig(cfg.Build.TagPolicy)
+
+	tagger, err := getTagger(cfg.Build.TagPolicy, opts.CustomTag)
 	if err != nil {
 		return nil, errors.Wrap(err, "parsing skaffold tag config")
 	}
-	customTag := opts.CustomTag
-	if customTag != "" {
-		tagger = &tag.CustomTag{
-			Tag: customTag,
-		}
-	}
+
 	client, err := kubernetesClient()
 	if err != nil {
 		return nil, errors.Wrap(err, "getting k8s client")
 	}
+
 	return &SkaffoldRunner{
 		config:         cfg,
 		Builder:        builder,
@@ -98,7 +94,7 @@ func NewForConfig(opts *config.SkaffoldOptions, cfg *config.SkaffoldConfig) (*Sk
 }
 
 func getBuilder(cfg *v1alpha2.BuildConfig, kubeContext string) (build.Builder, error) {
-	if cfg != nil && cfg.LocalBuild != nil {
+	if cfg.LocalBuild != nil {
 		logrus.Debugf("Using builder: local")
 		return build.NewLocalBuilder(cfg, kubeContext)
 	}
@@ -125,7 +121,13 @@ func getDeployer(cfg *v1alpha2.DeployConfig, kubeContext string) (deploy.Deploye
 	return nil, fmt.Errorf("Unknown deployer for config %+v", cfg)
 }
 
-func newTaggerForConfig(t v1alpha2.TagPolicy) (tag.Tagger, error) {
+func getTagger(t v1alpha2.TagPolicy, customTag string) (tag.Tagger, error) {
+	if customTag != "" {
+		return &tag.CustomTag{
+			Tag: customTag,
+		}, nil
+	}
+
 	if t.EnvTemplateTagger != nil {
 		return tag.NewEnvTemplateTagger(t.EnvTemplateTagger.Template)
 	}
@@ -150,22 +152,22 @@ func (r *SkaffoldRunner) Build(ctx context.Context) error {
 	return err
 }
 
+// Run runs the skaffold build and deploy pipeline.
+func (r *SkaffoldRunner) Run(ctx context.Context) error {
+	_, _, err := r.buildAndDeploy(ctx, r.config.Build.Artifacts, nil)
+	return err
+}
+
 // Dev watches for changes and runs the skaffold build and deploy
 // pipeline until interrrupted by the user.
 func (r *SkaffoldRunner) Dev(ctx context.Context) error {
 	if r.opts.Cleanup {
-		return cleanUpOnCtrlC(ctx, r.dev, r.cleanup)
+		return cleanUpOnCtrlC(ctx, r.watchBuildDeploy, r.cleanup)
 	}
-	return r.dev(ctx)
-}
-
-// Run runs the skaffold build and deploy pipeline.
-func (r *SkaffoldRunner) Run(ctx context.Context) error {
-	_, _, err := r.buildAndDeploy(ctx, r.config.Build.Artifacts, nil)
-	return err
+	return r.watchBuildDeploy(ctx)
 }
 
-func (r *SkaffoldRunner) dev(ctx context.Context) error {
+func (r *SkaffoldRunner) watchBuildDeploy(ctx context.Context) error {
 	artifacts := r.config.Build.Artifacts
 
 	var err error