From 0b4ab2bc072a43f59cbbafd9ad9f4d8ac38257ee Mon Sep 17 00:00:00 2001 From: Nick Santos Date: Fri, 20 Sep 2019 11:43:31 -0400 Subject: [PATCH] engine: respect the native docker-compose order. Fixes part of issue #2210 (#2223) --- internal/engine/buildcontroller.go | 119 +++++++++++------------- internal/engine/buildcontroller_test.go | 29 ++++++ 2 files changed, 84 insertions(+), 64 deletions(-) diff --git a/internal/engine/buildcontroller.go b/internal/engine/buildcontroller.go index d7e42ec01c..de0d57f1f1 100644 --- a/internal/engine/buildcontroller.go +++ b/internal/engine/buildcontroller.go @@ -40,37 +40,22 @@ func nextTargetToBuild(state store.EngineState) *store.ManifestTarget { return nil } - // unresourced YAML goes first - targets := append([]*store.ManifestTarget{}, state.Targets()...) - findUnresourcedYAMLAndPutItFirst(targets) - - // put no-build manifests next since they're more likely to be - // 1. fast and 2. dependencies of other services (e.g., redis) - sort.Sort(newNoBuildsManifestsFirst(targets)) + targets := state.Targets() - // First, go through all the manifests in order. - // If any of them haven't started yet, build them now. - for _, mt := range targets { - if !mt.State.StartedFirstBuild() { - return mt - } + // If any of the manifest target's haven't been built yet, build them now. + unbuilt := findUnbuiltTargets(targets) + if len(unbuilt) > 0 { + return nextUnbuiltTargetToBuild(unbuilt) } - // Next go through all the manifests, and check: - // 1) all pending file changes, and - // 2) all pending manifest changes - // The earliest one is the one we want. - var choice *store.ManifestTarget - earliest := time.Now() - - // always use a stable iteration order + // Next prioritize builds that crashed and need a rebuilt to have up-to-date code. for _, mt := range targets { - // Always prioritize builds that crashes and have an out-of-sync. if mt.State.NeedsRebuildFromCrash { return mt } } + // Next prioritize builds that are have been manually triggered. if len(state.TriggerQueue) > 0 { mn := state.TriggerQueue[0] mt, ok := state.ManifestTargets[mn] @@ -79,6 +64,19 @@ func nextTargetToBuild(state store.EngineState) *store.ManifestTarget { } } + return earliestPendingAutoTriggerTarget(targets) +} + +// Go through all the manifests, and check: +// 1) all pending file changes, and +// 2) all pending manifest changes +// The earliest one is the one we want. +// +// If no targets are pending, return nil +func earliestPendingAutoTriggerTarget(targets []*store.ManifestTarget) *store.ManifestTarget { + var choice *store.ManifestTarget + earliest := time.Now() + for _, mt := range targets { ok, newTime := mt.State.HasPendingChangesBefore(earliest) if ok { @@ -95,59 +93,52 @@ func nextTargetToBuild(state store.EngineState) *store.ManifestTarget { return choice } -func findUnresourcedYAMLAndPutItFirst(input []*store.ManifestTarget) { - for i := range input { - if input[i].Manifest.ManifestName() == model.UnresourcedYAMLManifestName { - found := input[i] - copy(input[1:], append(input[:i], input[i+1:]...)) - input[0] = found - break - } +// Helper function for ordering targets that have never been built before. +func nextUnbuiltTargetToBuild(unbuilt []*store.ManifestTarget) *store.ManifestTarget { + // unresourced YAML goes first + unresourced := findUnresourcedYAML(unbuilt) + if unresourced != nil { + return unresourced } -} - -type noBuildManifestsFirst struct { - mts []*store.ManifestTarget - origIndexByName map[string]int -} - -var _ sort.Interface = noBuildManifestsFirst{} -func newNoBuildsManifestsFirst(mts []*store.ManifestTarget) *noBuildManifestsFirst { - indexByName := make(map[string]int) - for i, mt := range mts { - indexByName[mt.Manifest.Name.String()] = i - } - return &noBuildManifestsFirst{ - mts: mts, - origIndexByName: indexByName, + // If this is Kubernetes, unbuilt resources go first. + // (If this is Docker Compose, we want to trust the ordering + // that docker-compose put things in.) + deployOnlyK8sTargets := findDeployOnlyK8sManifestTargets(unbuilt) + if len(deployOnlyK8sTargets) > 0 { + return deployOnlyK8sTargets[0] } -} -func (nbmf noBuildManifestsFirst) Len() int { - return len(nbmf.mts) + return unbuilt[0] } -func (nbmf noBuildManifestsFirst) Less(i, j int) bool { - isNoBuild := func(mt *store.ManifestTarget) bool { - return len(mt.Manifest.ImageTargets) == 0 +func findUnbuiltTargets(targets []*store.ManifestTarget) []*store.ManifestTarget { + result := []*store.ManifestTarget{} + for _, target := range targets { + if !target.State.StartedFirstBuild() { + result = append(result, target) + } } + return result +} - a := nbmf.mts[i] - b := nbmf.mts[j] - - nba := isNoBuild(a) - nbb := isNoBuild(b) - - if nba == nbb { - return nbmf.origIndexByName[a.Manifest.Name.String()] < nbmf.origIndexByName[b.Manifest.Name.String()] - } else { - return nba +func findUnresourcedYAML(targets []*store.ManifestTarget) *store.ManifestTarget { + for _, target := range targets { + if target.Manifest.ManifestName() == model.UnresourcedYAMLManifestName { + return target + } } + return nil } -func (nbmf noBuildManifestsFirst) Swap(i, j int) { - nbmf.mts[i], nbmf.mts[j] = nbmf.mts[j], nbmf.mts[i] +func findDeployOnlyK8sManifestTargets(targets []*store.ManifestTarget) []*store.ManifestTarget { + result := []*store.ManifestTarget{} + for _, target := range targets { + if target.Manifest.IsK8s() && len(target.Manifest.ImageTargets) == 0 { + result = append(result, target) + } + } + return result } func nextManifestNameToBuild(state store.EngineState) model.ManifestName { diff --git a/internal/engine/buildcontroller_test.go b/internal/engine/buildcontroller_test.go index aaeef9cbbd..9bac81813f 100644 --- a/internal/engine/buildcontroller_test.go +++ b/internal/engine/buildcontroller_test.go @@ -655,3 +655,32 @@ func TestBuildControllerUnresourcedYAMLFirst(t *testing.T) { } assert.Equal(t, expectedBuildOrder, observedBuildOrder) } + +func TestBuildControllerRespectDockerComposeOrder(t *testing.T) { + f := newTestFixture(t) + defer f.TearDown() + + sancho := NewSanchoFastBuildDCManifest(f) + redis := manifestbuilder.New(f, "redis").WithDockerCompose().Build() + donQuixote := manifestbuilder.New(f, "don-quixote").WithDockerCompose().Build() + manifests := []model.Manifest{redis, sancho, donQuixote} + f.Start(manifests, true) + + var observedBuildOrder []string + for i := 0; i < len(manifests); i++ { + call := f.nextCall() + observedBuildOrder = append(observedBuildOrder, call.dc().Name.String()) + } + + // If these were Kubernetes resources, we would try to deploy don-quixote + // before sancho, because it doesn't have an image build. + // + // But this would be wrong, because Docker Compose has stricter ordering requirements, see: + // https://docs.docker.com/compose/startup-order/ + expectedBuildOrder := []string{ + "redis", + "sancho", + "don-quixote", + } + assert.Equal(t, expectedBuildOrder, observedBuildOrder) +}