diff --git a/integration/examples/annotated-skaffold.yaml b/integration/examples/annotated-skaffold.yaml index f7c3e550271..0b35d3af0d5 100644 --- a/integration/examples/annotated-skaffold.yaml +++ b/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 diff --git a/pkg/skaffold/build/local/local_test.go b/pkg/skaffold/build/local/local_test.go index 04d8cbea98f..8c959421f6b 100644 --- a/pkg/skaffold/build/local/local_test.go +++ b/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{ diff --git a/pkg/skaffold/build/local/types.go b/pkg/skaffold/build/local/types.go index d693c849bcc..244ae7a145c 100644 --- a/pkg/skaffold/build/local/types.go +++ b/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{ diff --git a/pkg/skaffold/schema/latest/config.go b/pkg/skaffold/schema/latest/config.go index 5d32e57b180..949bbb43579 100644 --- a/pkg/skaffold/schema/latest/config.go +++ b/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"` } diff --git a/pkg/skaffold/schema/v1alpha3/upgrade.go b/pkg/skaffold/schema/v1alpha3/upgrade.go index 287f2dcf6af..f4cd3f920b4 100644 --- a/pkg/skaffold/schema/v1alpha3/upgrade.go +++ b/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, @@ -54,6 +59,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 { diff --git a/pkg/skaffold/schema/v1alpha3/upgrade_test.go b/pkg/skaffold/schema/v1alpha3/upgrade_test.go new file mode 100644 index 00000000000..a3e522fb05d --- /dev/null +++ b/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) + } +}