Merge pull request #1114 from balopat/rename_push

skipPush -> push

integration/examples/annotated-skaffold.yaml

@@ -77,7 +77,8 @@ build:
   # `useBuildkit` can also be set to activate the experimental BuildKit feature.
   #
   # local:
-  #   skipPush: true
+  #   false by default for local clusters, true for remote clusters
+  #   push: false
   #   useDockerCLI: false
   #   useBuildkit: false
 

pkg/skaffold/build/local/local_test.go

@@ -80,7 +80,7 @@ func TestLocalRun(t *testing.T) {
 			description: "single build",
 			out:         ioutil.Discard,
 			config: &latest.LocalBuild{
-				SkipPush: util.BoolPtr(false),
+				Push: util.BoolPtr(false),
 			},
 			artifacts: []*latest.Artifact{
 				{
@@ -104,7 +104,7 @@ func TestLocalRun(t *testing.T) {
 			description: "subset build",
 			out:         ioutil.Discard,
 			config: &latest.LocalBuild{
-				SkipPush: util.BoolPtr(true),
+				Push: util.BoolPtr(true),
 			},
 			tagger: &tag.ChecksumTagger{},
 			artifacts: []*latest.Artifact{

pkg/skaffold/build/local/types.go

@@ -48,11 +48,11 @@ func NewBuilder(cfg *latest.LocalBuild, kubeContext string) (*Builder, error) {
 
 	localCluster := kubeContext == constants.DefaultMinikubeContext || kubeContext == constants.DefaultDockerForDesktopContext
 	var pushImages bool
-	if cfg.SkipPush == nil {
-		logrus.Debugf("skipPush value not present. defaulting to cluster default %t (minikube=true, d4d=true, gke=false)", localCluster)
+	if cfg.Push == nil {
 		pushImages = !localCluster
+		logrus.Debugf("push value not present, defaulting to %t because localCluster is %t", pushImages, localCluster)
 	} else {
-		pushImages = !*cfg.SkipPush
+		pushImages = *cfg.Push
 	}
 
 	return &Builder{

pkg/skaffold/schema/latest/config.go

@@ -86,7 +86,7 @@ type BuildType struct {
 // LocalBuild contains the fields needed to do a build on the local docker daemon
 // and optionally push to a repository.
 type LocalBuild struct {
-	SkipPush     *bool `yaml:"skipPush,omitempty"`
+	Push         *bool `yaml:"push,omitempty"`
 	UseDockerCLI bool  `yaml:"useDockerCLI,omitempty"`
 	UseBuildkit  bool  `yaml:"useBuildkit,omitempty"`
 }

pkg/skaffold/schema/v1alpha3/upgrade.go

@@ -38,13 +38,18 @@ func (config *SkaffoldPipeline) Upgrade() (util.VersionedConfig, error) {
 		if err := convert(config.Profiles, &newProfiles); err != nil {
 			return nil, errors.Wrap(err, "converting new profile")
 		}
+		for i, oldProfile := range config.Profiles {
+			convertBuild(oldProfile.Build, newProfiles[i].Build)
+		}
 	}
 
 	// convert Build (should be the same)
 	var newBuild next.BuildConfig
-	if err := convert(config.Build, &newBuild); err != nil {
+	oldBuild := config.Build
+	if err := convert(oldBuild, &newBuild); err != nil {
 		return nil, errors.Wrap(err, "converting new build")
 	}
+	convertBuild(oldBuild, newBuild)
 
 	return &next.SkaffoldPipeline{
 		APIVersion: next.Version,
@@ -55,6 +60,13 @@ func (config *SkaffoldPipeline) Upgrade() (util.VersionedConfig, error) {
 	}, nil
 }
 
+func convertBuild(oldBuild BuildConfig, newBuild next.BuildConfig) {
+	if oldBuild.LocalBuild != nil && oldBuild.LocalBuild.SkipPush != nil {
+		push := !*oldBuild.LocalBuild.SkipPush
+		newBuild.LocalBuild.Push = &push
+	}
+}
+
 func convert(old interface{}, new interface{}) error {
 	o, err := json.Marshal(old)
 	if err != nil {

pkg/skaffold/schema/v1alpha3/upgrade_test.go

@@ -0,0 +1,74 @@
+/*
+Copyright 2018 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package v1alpha3
+
+import (
+	"testing"
+
+	v1alpha4 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
+)
+
+func TestBuildUpgrade(t *testing.T) {
+	old := `apiVersion: skaffold/v1alpha3
+kind: Config
+build:
+  local:	
+    skipPush: false
+profiles:
+  - name: testEnv1
+    build:
+      local:
+        skipPush: true
+  - name: testEnv2
+    build:
+      local:
+        skipPush: false
+`
+	pipeline := NewSkaffoldPipeline()
+	err := pipeline.Parse([]byte(old), true)
+	if err != nil {
+		t.Errorf("unexpected error during parsing old config: %v", err)
+	}
+
+	upgraded, err := pipeline.Upgrade()
+	if err != nil {
+		t.Errorf("unexpected error during upgrade: %v", err)
+	}
+
+	upgradedPipeline := upgraded.(*v1alpha4.SkaffoldPipeline)
+
+	if upgradedPipeline.Build.LocalBuild == nil {
+		t.Errorf("expected build.local to be not nil")
+	}
+	if upgradedPipeline.Build.LocalBuild.Push != nil && !*upgradedPipeline.Build.LocalBuild.Push {
+		t.Errorf("expected build.local.push to be true but it was: %v", *upgradedPipeline.Build.LocalBuild.Push)
+	}
+
+	if upgradedPipeline.Profiles[0].Build.LocalBuild == nil {
+		t.Errorf("expected profiles[0].build.local to be not nil")
+	}
+	if upgradedPipeline.Profiles[0].Build.LocalBuild.Push != nil && *upgradedPipeline.Profiles[0].Build.LocalBuild.Push {
+		t.Errorf("expected profiles[0].build.local.push to be false but it was: %v", *upgradedPipeline.Build.LocalBuild.Push)
+	}
+
+	if upgradedPipeline.Profiles[1].Build.LocalBuild == nil {
+		t.Errorf("expected profiles[1].build.local to be not nil")
+	}
+	if upgradedPipeline.Profiles[1].Build.LocalBuild.Push != nil && !*upgradedPipeline.Profiles[1].Build.LocalBuild.Push {
+		t.Errorf("expected profiles[1].build.local.push to be true but it was: %v", *upgradedPipeline.Build.LocalBuild.Push)
+	}
+}