Make git tagger the default for both dev and run

Fixes GoogleContainerTools#425 Signed-off-by: David Gageot <david@gageot.net>
dgageot · Apr 27, 2018 · c7592d1 · c7592d1
1 parent 48b267c
commit c7592d1
Show file tree

Hide file tree

Showing 9 changed files with 25 additions and 68 deletions.
diff --git a/cmd/skaffold/app/cmd/cmd.go b/cmd/skaffold/app/cmd/cmd.go
@@ -89,7 +89,7 @@ func SetUpLogs(out io.Writer, level string) error {
 	return nil
 }
 
-func readConfiguration(filename string, dev bool) (*config.SkaffoldConfig, error) {
+func readConfiguration(filename string) (*config.SkaffoldConfig, error) {
 	buf, err := util.ReadConfiguration(filename)
 	if err != nil {
 		return nil, errors.Wrap(err, "read skaffold config")
@@ -104,7 +104,7 @@ func readConfiguration(filename string, dev bool) (*config.SkaffoldConfig, error
 		return nil, errors.New("Config version out of date: run `skaffold fix`")
 	}
 
-	cfg, err := config.GetConfig(buf, true, dev)
+	cfg, err := config.GetConfig(buf, true)
 	if err != nil {
 		return nil, errors.Wrap(err, "parsing skaffold config")
 	}
@@ -121,12 +121,12 @@ func readConfiguration(filename string, dev bool) (*config.SkaffoldConfig, error
 	return latestConfig, nil
 }
 
-func runSkaffold(out io.Writer, dev bool, filename string, action func(context.Context, *runner.SkaffoldRunner) error) error {
+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, dev)
+	config, err := readConfiguration(filename)
 	if err != nil {
 		return errors.Wrap(err, "reading configuration")
 	}

diff --git a/cmd/skaffold/app/cmd/dev.go b/cmd/skaffold/app/cmd/dev.go
@@ -31,7 +31,7 @@ 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, true, filename, func(ctx context.Context, r *runner.SkaffoldRunner) error {
+			return runSkaffold(out, filename, func(ctx context.Context, r *runner.SkaffoldRunner) error {
 				return r.Dev(ctx)
 			})
 		},

diff --git a/cmd/skaffold/app/cmd/fix.go b/cmd/skaffold/app/cmd/fix.go
@@ -42,7 +42,7 @@ func NewCmdFix(out io.Writer) *cobra.Command {
 			if err != nil {
 				logrus.Errorf("fix: %s", err)
 			}
-			cfg, err := config.GetConfig(contents, false, true)
+			cfg, err := config.GetConfig(contents, false)
 			if err != nil {
 				logrus.Error(err)
 				return

diff --git a/cmd/skaffold/app/cmd/run.go b/cmd/skaffold/app/cmd/run.go
@@ -31,7 +31,7 @@ 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, false, filename, func(ctx context.Context, r *runner.SkaffoldRunner) error {
+			return runSkaffold(out, filename, func(ctx context.Context, r *runner.SkaffoldRunner) error {
 				return r.Run(ctx)
 			})
 		},

diff --git a/pkg/skaffold/config/config_test.go b/pkg/skaffold/config/config_test.go
@@ -65,47 +65,22 @@ func TestParseConfig(t *testing.T) {
 	var tests = []struct {
 		description string
 		config      string
-		dev         bool
 		expected    util.VersionedConfig
 		badReader   bool
 		shouldErr   bool
 	}{
 		{
-			description: "Minimal config for dev",
+			description: "Minimal config",
 			config:      minimalConfig,
-			dev:         true,
-			expected: config(
-				withLocalBuild(
-					withTagPolicy(v1alpha2.TagPolicy{ShaTagger: &v1alpha2.ShaTagger{}}),
-				),
-			),
-		},
-		{
-			description: "Minimal config for run",
-			config:      minimalConfig,
-			dev:         false,
 			expected: config(
 				withLocalBuild(
 					withTagPolicy(v1alpha2.TagPolicy{GitTagger: &v1alpha2.GitTagger{}}),
 				),
 			),
 		},
 		{
-			description: "Simple config for dev",
+			description: "Simple config",
 			config:      simpleConfig,
-			dev:         true,
-			expected: config(
-				withLocalBuild(
-					withTagPolicy(v1alpha2.TagPolicy{ShaTagger: &v1alpha2.ShaTagger{}}),
-					withDockerArtifact("example", ".", "Dockerfile"),
-				),
-				withDeploy("example"),
-			),
-		},
-		{
-			description: "Simple config for run",
-			config:      simpleConfig,
-			dev:         false,
 			expected: config(
 				withLocalBuild(
 					withTagPolicy(v1alpha2.TagPolicy{GitTagger: &v1alpha2.GitTagger{}}),
@@ -115,22 +90,8 @@ func TestParseConfig(t *testing.T) {
 			),
 		},
 		{
-			description: "Complete config for dev",
-			config:      completeConfig,
-			dev:         true,
-			expected: config(
-				withGCBBuild("ID",
-					withTagPolicy(v1alpha2.TagPolicy{ShaTagger: &v1alpha2.ShaTagger{}}),
-					withDockerArtifact("image1", "./examples/app1", "Dockerfile.dev"),
-					withBazelArtifact("image2", "./examples/app2", "//:example.tar"),
-				),
-				withDeploy("example"),
-			),
-		},
-		{
-			description: "Complete config for run",
+			description: "Complete config",
 			config:      completeConfig,
-			dev:         false,
 			expected: config(
 				withGCBBuild("ID",
 					withTagPolicy(v1alpha2.TagPolicy{ShaTagger: &v1alpha2.ShaTagger{}}),
@@ -149,7 +110,7 @@ func TestParseConfig(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.description, func(t *testing.T) {
-			cfg, err := GetConfig([]byte(test.config), true, test.dev)
+			cfg, err := GetConfig([]byte(test.config), true)
 			testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expected, cfg)
 		})
 	}

diff --git a/pkg/skaffold/config/util.go b/pkg/skaffold/config/util.go
@@ -27,22 +27,22 @@ import (
 // Ordered list of all schema versions
 var Versions = []string{v1alpha1.Version, v1alpha2.Version}
 
-var schemaVersions map[string]func([]byte, bool, bool) (util.VersionedConfig, error) = map[string]func([]byte, bool, bool) (util.VersionedConfig, error){
-	v1alpha1.Version: func(contents []byte, useDefault bool, dev bool) (util.VersionedConfig, error) {
+var schemaVersions = map[string]func([]byte, bool) (util.VersionedConfig, error){
+	v1alpha1.Version: func(contents []byte, useDefault bool) (util.VersionedConfig, error) {
 		config := new(v1alpha1.SkaffoldConfig)
-		err := config.Parse(contents, useDefault, dev)
+		err := config.Parse(contents, useDefault)
 		return config, err
 	},
-	v1alpha2.Version: func(contents []byte, useDefault bool, dev bool) (util.VersionedConfig, error) {
+	v1alpha2.Version: func(contents []byte, useDefault bool) (util.VersionedConfig, error) {
 		config := new(v1alpha2.SkaffoldConfig)
-		err := config.Parse(contents, useDefault, dev)
+		err := config.Parse(contents, useDefault)
 		return config, err
 	},
 }
 
-func GetConfig(contents []byte, useDefault bool, dev bool) (util.VersionedConfig, error) {
+func GetConfig(contents []byte, useDefault bool) (util.VersionedConfig, error) {
 	for _, version := range Versions {
-		if cfg, err := schemaVersions[version](contents, useDefault, dev); err == nil {
+		if cfg, err := schemaVersions[version](contents, useDefault); err == nil {
 			// successfully parsed, but make sure versions match
 			if cfg.GetVersion() == version {
 				return cfg, err

diff --git a/pkg/skaffold/schema/util/util.go b/pkg/skaffold/schema/util/util.go
@@ -18,7 +18,7 @@ package util
 
 type VersionedConfig interface {
 	GetVersion() string
-	Parse([]byte, bool, bool) error
+	Parse([]byte, bool) error
 }
 
 type Config interface {

diff --git a/pkg/skaffold/schema/v1alpha1/config.go b/pkg/skaffold/schema/v1alpha1/config.go
@@ -128,9 +128,9 @@ var defaultRunSkaffoldConfig = &SkaffoldConfig{
 // Parse reads from an io.Reader and unmarshals the result into a SkaffoldConfig.
 // The default config argument provides default values for the config,
 // which can be overridden if present in the config file.
-func (config *SkaffoldConfig) Parse(contents []byte, useDefault bool, mode bool) error {
+func (config *SkaffoldConfig) Parse(contents []byte, useDefault bool) error {
 	if useDefault {
-		*config = *config.getDefaultForMode(mode)
+		*config = *config.getDefaultForMode(false)
 	} else {
 		*config = SkaffoldConfig{}
 	}

diff --git a/pkg/skaffold/schema/v1alpha2/config.go b/pkg/skaffold/schema/v1alpha2/config.go
@@ -178,16 +178,12 @@ func defaultToDockerArtifacts(cfg *SkaffoldConfig) {
 	}
 }
 
-func setDefaultTagger(cfg *SkaffoldConfig, dev bool) {
+func setDefaultTagger(cfg *SkaffoldConfig) {
 	if cfg.Build.TagPolicy != (TagPolicy{}) {
 		return
 	}
 
-	if dev {
-		cfg.Build.TagPolicy = TagPolicy{ShaTagger: &ShaTagger{}}
-	} else {
-		cfg.Build.TagPolicy = TagPolicy{GitTagger: &GitTagger{}}
-	}
+	cfg.Build.TagPolicy = TagPolicy{GitTagger: &GitTagger{}}
 }
 
 func setDefaultDockerfiles(cfg *SkaffoldConfig) {
@@ -248,14 +244,14 @@ func profilesByName(profiles []Profile) map[string]Profile {
 	return byName
 }
 
-func (config *SkaffoldConfig) Parse(contents []byte, useDefaults bool, mode bool) error {
+func (config *SkaffoldConfig) Parse(contents []byte, useDefaults bool) error {
 	if err := yaml.Unmarshal(contents, config); err != nil {
 		return err
 	}
 	if useDefaults {
 		defaultToLocalBuild(config)
 		defaultToDockerArtifacts(config)
-		setDefaultTagger(config, mode)
+		setDefaultTagger(config)
 		setDefaultDockerfiles(config)
 		setDefaultWorkspaces(config)
 	}