diff --git a/integration/testdata/diagnose/multi-config/diagnose.tmpl b/integration/testdata/diagnose/multi-config/diagnose.tmpl index d6e64e61f69..b3d0205636d 100644 --- a/integration/testdata/diagnose/multi-config/diagnose.tmpl +++ b/integration/testdata/diagnose/multi-config/diagnose.tmpl @@ -10,8 +10,7 @@ build: dockerfile: Dockerfile tagPolicy: gitCommit: {} - local: - concurrency: 1 + local: {} deploy: kubectl: manifests: @@ -31,8 +30,7 @@ build: dockerfile: Dockerfile tagPolicy: gitCommit: {} - local: - concurrency: 1 + local: {} deploy: kubectl: manifests: @@ -50,8 +48,7 @@ build: dockerfile: Dockerfile tagPolicy: gitCommit: {} - local: - concurrency: 1 + local: {} deploy: kubectl: manifests: diff --git a/integration/testdata/diagnose/temp-config/diagnose.tmpl b/integration/testdata/diagnose/temp-config/diagnose.tmpl index 5dbba2eb2d0..278c618cb48 100644 --- a/integration/testdata/diagnose/temp-config/diagnose.tmpl +++ b/integration/testdata/diagnose/temp-config/diagnose.tmpl @@ -8,8 +8,7 @@ build: dockerfile: Dockerfile tagPolicy: gitCommit: {} - local: - concurrency: 1 + local: {} deploy: kubectl: manifests: diff --git a/pkg/skaffold/build/build.go b/pkg/skaffold/build/build.go index 414ed06fb14..676f0400a88 100644 --- a/pkg/skaffold/build/build.go +++ b/pkg/skaffold/build/build.go @@ -51,7 +51,7 @@ type PipelineBuilder interface { PostBuild(ctx context.Context, out io.Writer) error // Concurrency specifies the max number of builds that can run at any one time. If concurrency is 0, then all builds can run in parallel. - Concurrency() int + Concurrency() *int // Prune removes images built in this pipeline Prune(context.Context, io.Writer) error diff --git a/pkg/skaffold/build/builder_mux.go b/pkg/skaffold/build/builder_mux.go index ab7853e209d..c423095bbd3 100644 --- a/pkg/skaffold/build/builder_mux.go +++ b/pkg/skaffold/build/builder_mux.go @@ -22,6 +22,7 @@ import ( "io" "reflect" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/graph" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/hooks" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/output/log" @@ -65,20 +66,29 @@ func NewBuilderMux(cfg Config, store ArtifactStore, builder func(p latestV1.Pipe if cfg.BuildConcurrency() >= 0 { minConcurrency = cfg.BuildConcurrency() - } else { - concurrency := b.Concurrency() - // set mux concurrency to be the minimum of all builders' concurrency. (concurrency = 0 means unlimited) - switch { - case minConcurrency < 0: - minConcurrency = concurrency - log.Entry(context.TODO()).Infof("build concurrency first set to %d parsed from %s[%d]", minConcurrency, reflect.TypeOf(b).String(), i) - case concurrency > 0 && (minConcurrency == 0 || concurrency < minConcurrency): - minConcurrency = concurrency - log.Entry(context.TODO()).Infof("build concurrency updated to %d parsed from %s[%d]", minConcurrency, reflect.TypeOf(b).String(), i) - default: - log.Entry(context.TODO()).Infof("build concurrency value %d parsed from %s[%d] is ignored since it's not less than previously set value %d", concurrency, reflect.TypeOf(b).String(), i, minConcurrency) - } + continue } + + if b.Concurrency() == nil { + continue + } + concurrency := *b.Concurrency() + + // set mux concurrency to be the minimum of all builders' concurrency. (concurrency = 0 means unlimited) + switch { + case minConcurrency < 0: + minConcurrency = concurrency + log.Entry(context.TODO()).Infof("build concurrency first set to %d parsed from %s[%d]", minConcurrency, reflect.TypeOf(b).String(), i) + case concurrency > 0 && (minConcurrency == 0 || concurrency < minConcurrency): + minConcurrency = concurrency + log.Entry(context.TODO()).Infof("build concurrency updated to %d parsed from %s[%d]", minConcurrency, reflect.TypeOf(b).String(), i) + default: + log.Entry(context.TODO()).Infof("build concurrency value %d parsed from %s[%d] is ignored since it's not less than previously set value %d", concurrency, reflect.TypeOf(b).String(), i, minConcurrency) + } + } + if minConcurrency < 0 { + minConcurrency = constants.DefaultLocalConcurrency // set default concurrency to 1. + log.Entry(context.TODO()).Infof("build concurrency set to default value of %d", minConcurrency) } log.Entry(context.TODO()).Infof("final build concurrency value is %d", minConcurrency) diff --git a/pkg/skaffold/build/builder_mux_test.go b/pkg/skaffold/build/builder_mux_test.go index 49516ff8ce4..6a848c7e9f9 100644 --- a/pkg/skaffold/build/builder_mux_test.go +++ b/pkg/skaffold/build/builder_mux_test.go @@ -121,7 +121,7 @@ func (m *mockPipelineBuilder) Build(ctx context.Context, out io.Writer, artifact func (m *mockPipelineBuilder) PostBuild(ctx context.Context, out io.Writer) error { return nil } -func (m *mockPipelineBuilder) Concurrency() int { return m.concurrency } +func (m *mockPipelineBuilder) Concurrency() *int { return util.IntPtr(m.concurrency) } func (m *mockPipelineBuilder) Prune(context.Context, io.Writer) error { return nil } diff --git a/pkg/skaffold/build/cluster/cluster.go b/pkg/skaffold/build/cluster/cluster.go index c265307d8d1..242abf2ab72 100644 --- a/pkg/skaffold/build/cluster/cluster.go +++ b/pkg/skaffold/build/cluster/cluster.go @@ -70,8 +70,8 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, artifact *la return build.TagWithDigest(tag, digest), nil } -func (b *Builder) Concurrency() int { - return b.ClusterDetails.Concurrency +func (b *Builder) Concurrency() *int { + return util.IntPtr(b.ClusterDetails.Concurrency) } func (b *Builder) runBuildForArtifact(ctx context.Context, out io.Writer, a *latestV1.Artifact, tag string, platforms platform.Matcher) (string, error) { diff --git a/pkg/skaffold/build/gcb/cloud_build.go b/pkg/skaffold/build/gcb/cloud_build.go index 383dbd92059..ce69b616efb 100644 --- a/pkg/skaffold/build/gcb/cloud_build.go +++ b/pkg/skaffold/build/gcb/cloud_build.go @@ -44,6 +44,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/platform" latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/sources" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/proto/v1" ) @@ -65,8 +66,8 @@ func (b *Builder) PostBuild(_ context.Context, _ io.Writer) error { return nil } -func (b *Builder) Concurrency() int { - return b.GoogleCloudBuild.Concurrency +func (b *Builder) Concurrency() *int { + return util.IntPtr(b.GoogleCloudBuild.Concurrency) } func (b *Builder) buildArtifactWithCloudBuild(ctx context.Context, out io.Writer, artifact *latestV1.Artifact, tag string, platform platform.Matcher) (string, error) { diff --git a/pkg/skaffold/build/local/local.go b/pkg/skaffold/build/local/local.go index df70a42ab85..bbf40f327af 100644 --- a/pkg/skaffold/build/local/local.go +++ b/pkg/skaffold/build/local/local.go @@ -58,12 +58,7 @@ func (b *Builder) PostBuild(ctx context.Context, _ io.Writer) error { return nil } -func (b *Builder) Concurrency() int { - if b.local.Concurrency == nil { - return 0 - } - return *b.local.Concurrency -} +func (b *Builder) Concurrency() *int { return b.local.Concurrency } func (b *Builder) PushImages() bool { return b.pushImages diff --git a/pkg/skaffold/parser/config_test.go b/pkg/skaffold/parser/config_test.go index bd9c33854e1..913a723cbb0 100644 --- a/pkg/skaffold/parser/config_test.go +++ b/pkg/skaffold/parser/config_test.go @@ -70,16 +70,11 @@ func createCfg(name string, imageName string, workspace string, requires []lates Artifacts: []*latestV1.Artifact{{ImageName: imageName, ArtifactType: latestV1.ArtifactType{ DockerArtifact: &latestV1.DockerArtifact{DockerfilePath: "Dockerfile"}}, Workspace: workspace}}, TagPolicy: latestV1.TagPolicy{ GitTagger: &latestV1.GitTagger{}}, BuildType: latestV1.BuildType{ - LocalBuild: &latestV1.LocalBuild{Concurrency: concurrency()}, + LocalBuild: &latestV1.LocalBuild{}, }}, Deploy: latestV1.DeployConfig{Logs: latestV1.LogsConfig{Prefix: "container"}}}, } } -func concurrency() *int { - c := 1 - return &c -} - type document struct { path string configs []mockCfg diff --git a/pkg/skaffold/schema/defaults/defaults.go b/pkg/skaffold/schema/defaults/defaults.go index c14aa833753..672e04f8daa 100644 --- a/pkg/skaffold/schema/defaults/defaults.go +++ b/pkg/skaffold/schema/defaults/defaults.go @@ -76,13 +76,6 @@ func Set(c *latestV1.SkaffoldConfig) error { } } - withLocalBuild(c, func(lb *latestV1.LocalBuild) { - // don't set build concurrency if there are no artifacts in the current config - if len(c.Build.Artifacts) > 0 { - setDefaultConcurrency(lb) - } - }) - withCloudBuildConfig(c, setDefaultCloudBuildDockerImage, setDefaultCloudBuildMavenImage, @@ -136,20 +129,6 @@ func defaultToKubectlDeploy(c *latestV1.SkaffoldConfig) { c.Deploy.DeployType.KubectlDeploy = &latestV1.KubectlDeploy{} } -func withLocalBuild(c *latestV1.SkaffoldConfig, operations ...func(*latestV1.LocalBuild)) { - if local := c.Build.LocalBuild; local != nil { - for _, operation := range operations { - operation(local) - } - } -} - -func setDefaultConcurrency(local *latestV1.LocalBuild) { - if local.Concurrency == nil { - local.Concurrency = &constants.DefaultLocalConcurrency - } -} - func withCloudBuildConfig(c *latestV1.SkaffoldConfig, operations ...func(*latestV1.GoogleCloudBuild)) { if gcb := c.Build.GoogleCloudBuild; gcb != nil { for _, operation := range operations { diff --git a/pkg/skaffold/schema/defaults/defaults_test.go b/pkg/skaffold/schema/defaults/defaults_test.go index a34d178bef9..94e85f425b4 100644 --- a/pkg/skaffold/schema/defaults/defaults_test.go +++ b/pkg/skaffold/schema/defaults/defaults_test.go @@ -319,7 +319,6 @@ func TestSetDefaultsOnLocalBuild(t *testing.T) { err = Set(cfg2) testutil.CheckError(t, false, err) SetDefaultDeployer(cfg2) - testutil.CheckDeepEqual(t, 1, *cfg2.Build.LocalBuild.Concurrency) } func TestSetPortForwardLocalPort(t *testing.T) { diff --git a/pkg/skaffold/schema/versions_test.go b/pkg/skaffold/schema/versions_test.go index 03a2a548506..ce03cc9dd36 100644 --- a/pkg/skaffold/schema/versions_test.go +++ b/pkg/skaffold/schema/versions_test.go @@ -493,9 +493,6 @@ func withLocalBuild(ops ...func(*latestV1.BuildConfig)) func(*latestV1.SkaffoldC for _, op := range ops { op(&b) } - if len(b.Artifacts) > 0 { - b.LocalBuild.Concurrency = &constants.DefaultLocalConcurrency - } cfg.Build = b } }