feat: Add needs: [NS/NAME] for controlling installation/deletion order declaratively (#914)

Introduces DAG-aware installation/deletion ordering to Helmfile.

`needs` controls the order of the installation/deletion of the release:

```yaml
relesaes:
- name: somerelease
  needs:
  - [TILLER_NAMESPACE/][NAMESPACE/]anotherelease
```

All the releases listed under `needs` are installed before(or deleted after) the release itself.

For the following example, `helmfile [sync|apply]` installs releases in this order:

1. logging
2. servicemesh
3. myapp1 and myapp2

```yaml
  - name: myapp1
    chart: charts/myapp
    needs:
    - servicemesh
    - logging
  - name: myapp2
    chart: charts/myapp
    needs:
    - servicemesh
    - logging
  - name: servicemesh
    chart: charts/istio
    needs:
    - logging
  - name: logging
    chart: charts/fluentd
```

Note that all the releases in a same group is installed concurrently. That is, myapp1 and myapp2 are installed concurrently.

On `helmdile [delete|destroy]`, deleations happen in the reverse order.

That is, `myapp1` and `myapp2` are deleted first, then `servicemesh`, and finally `logging`.

Resolves #715

pkg/state/state.go

@@ -20,6 +20,7 @@ import (
 	"github.com/roboll/helmfile/pkg/helmexec"
 	"github.com/roboll/helmfile/pkg/remote"
 	"github.com/roboll/helmfile/pkg/tmpl"
+	"github.com/variantdev/dag/pkg/dag"
 
 	"regexp"
 
@@ -144,6 +145,8 @@ type ReleaseSpec struct {
 	// MissingFileHandler is set to either "Error" or "Warn". "Error" instructs helmfile to fail when unable to find a values or secrets file. When "Warn", it prints the file and continues.
 	// The default value for MissingFileHandler is "Error".
 	MissingFileHandler *string `yaml:"missingFileHandler,omitempty"`
+	// Needs is the [TILLER_NS/][NS/]NAME representations of releases that this release depends on.
+	Needs []string `yaml:"needs,omitempty"`
 
 	// Hooks is a list of extension points paired with operations, that are executed in specific points of the lifecycle of releases defined in helmfile
 	Hooks []event.Hook `yaml:"hooks,omitempty"`
@@ -408,12 +411,89 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme
 		return prepErrs
 	}
 
+	availableIds := make([]string, len(preps))
+	idToPrep := map[string]syncPrepareResult{}
+
+	d := dag.New()
+	for i, p := range preps {
+		r := p.release
+
+		id := releaseToID(r)
+
+		idToPrep[id] = p
+
+		availableIds[i] = id
+
+		d.Add(id, dag.Dependencies(r.Needs))
+	}
+
+	plan, err := d.Plan()
+	if err != nil {
+		return []error{err}
+	}
+
+	groupsTotal := len(plan)
+
+	st.logger.Debugf("syncing %d groups of releases in this order: %s", groupsTotal, plan)
+
+	for id, prep := range idToPrep {
+		for _, need := range prep.release.Needs {
+			if _, ok := idToPrep[need]; !ok {
+				return []error{fmt.Errorf("%q needs %q, but it must be one of %s", id, need, strings.Join(availableIds, ", "))}
+			}
+		}
+	}
+
+	for groupIndex, dagNodesInGroup := range plan {
+		var idsInGroup []string
+		var prepsInGroup []syncPrepareResult
+
+		for _, node := range dagNodesInGroup {
+			prepareResult, ok := idToPrep[node.Id]
+			if !ok {
+				panic(fmt.Sprintf("[bug] no release found for dag node id %q", node.Id))
+			}
+			prepsInGroup = append(prepsInGroup, prepareResult)
+			idsInGroup = append(idsInGroup, node.Id)
+		}
+
+		st.logger.Debugf("syncing releases in group %d/%d: %s", groupIndex+1, groupsTotal, strings.Join(idsInGroup, ", "))
+
+		errs := st.syncReleaseGroup(affectedReleases, helm, prepsInGroup)
+		if len(errs) > 0 {
+			return errs
+		}
+	}
+
+	return nil
+}
+
+func releaseToID(r *ReleaseSpec) string {
+	var id string
+
+	tns := r.TillerNamespace
+	if tns != "" {
+		id += tns + "/"
+	}
+
+	ns := r.Namespace
+	if ns != "" {
+		id += ns + "/"
+	}
+
+	id += r.Name
+
+	return id
+}
+
+func (st *HelmState) syncReleaseGroup(affectedReleases *AffectedReleases, helm helmexec.Interface, preps []syncPrepareResult) []error {
 	errs := []error{}
 	jobQueue := make(chan *syncPrepareResult, len(preps))
 	results := make(chan syncResult, len(preps))
+	concurrency := len(preps)
 
 	st.scatterGather(
-		workerLimit,
+		concurrency,
 		len(preps),
 		func() {
 			for i := 0; i < len(preps); i++ {
@@ -1010,8 +1090,9 @@ func (st *HelmState) ReleaseStatuses(helm helmexec.Interface, workerLimit int) [
 }
 
 // DeleteReleases wrapper for executing helm delete on the releases
+// This function traverses the DAG of the releases in the reverse order, so that the releases that are NOT depended by any others are deleted first.
 func (st *HelmState) DeleteReleases(affectedReleases *AffectedReleases, helm helmexec.Interface, concurrency int, purge bool) []error {
-	return st.scatterGatherReleases(helm, concurrency, func(release ReleaseSpec, workerIndex int) error {
+	return st.dagAwareReverseIterateOnReleases(helm, concurrency, func(release ReleaseSpec, workerIndex int) error {
 		if !release.Desired() {
 			return nil
 		}

pkg/state/state_run.go

@@ -2,9 +2,11 @@ package state
 
 import (
 	"fmt"
+	"strings"
 	"sync"
 
 	"github.com/roboll/helmfile/pkg/helmexec"
+	"github.com/variantdev/dag/pkg/dag"
 )
 
 type result struct {
@@ -51,9 +53,14 @@ func (st *HelmState) scatterGather(concurrency int, items int, produceInputs fun
 
 func (st *HelmState) scatterGatherReleases(helm helmexec.Interface, concurrency int,
 	do func(ReleaseSpec, int) error) []error {
+
+	return st.iterateOnReleases(helm, concurrency, st.Releases, do)
+}
+
+func (st *HelmState) iterateOnReleases(helm helmexec.Interface, concurrency int, inputs []ReleaseSpec,
+	do func(ReleaseSpec, int) error) []error {
 	var errs []error
 
-	inputs := st.Releases
 	inputsSize := len(inputs)
 
 	releases := make(chan ReleaseSpec)
@@ -96,3 +103,52 @@ func (st *HelmState) scatterGatherReleases(helm helmexec.Interface, concurrency
 
 	return nil
 }
+
+func (st *HelmState) dagAwareReverseIterateOnReleases(helm helmexec.Interface, concurrency int,
+	do func(ReleaseSpec, int) error) []error {
+
+	idToRelease := map[string]ReleaseSpec{}
+
+	preps := st.Releases
+
+	d := dag.New()
+	for _, r := range preps {
+
+		id := releaseToID(&r)
+
+		idToRelease[id] = r
+
+		d.Add(id, dag.Dependencies(r.Needs))
+	}
+
+	plan, err := d.Plan()
+	if err != nil {
+		return []error{err}
+	}
+
+	groupsTotal := len(plan)
+
+	st.logger.Debugf("processing %d groups of releases in this order: %s", groupsTotal, plan)
+
+	for groupIndex := len(plan) - 1; groupIndex >= 0; groupIndex-- {
+		dagNodesInGroup := plan[groupIndex]
+
+		var idsInGroup []string
+		var releasesInGroup []ReleaseSpec
+
+		for _, node := range dagNodesInGroup {
+			releasesInGroup = append(releasesInGroup, idToRelease[node.Id])
+			idsInGroup = append(idsInGroup, node.Id)
+		}
+
+		st.logger.Debugf("processing releases in group %d/%d: %s", groupIndex+1, groupsTotal, strings.Join(idsInGroup, ", "))
+
+		errs := st.iterateOnReleases(helm, concurrency, releasesInGroup, do)
+
+		if len(errs) > 0 {
+			return errs
+		}
+	}
+
+	return nil
+}

pkg/state/state_test.go

@@ -867,10 +867,11 @@ func TestHelmState_SyncRepos(t *testing.T) {
 
 func TestHelmState_SyncReleases(t *testing.T) {
 	tests := []struct {
-		name         string
-		releases     []ReleaseSpec
-		helm         *mockHelmExec
-		wantReleases []mockRelease
+		name          string
+		releases      []ReleaseSpec
+		helm          *mockHelmExec
+		wantReleases  []mockRelease
+		wantErrorMsgs []string
 	}{
 		{
 			name: "normal release",
@@ -961,6 +962,106 @@ func TestHelmState_SyncReleases(t *testing.T) {
 			helm:         &mockHelmExec{},
 			wantReleases: []mockRelease{{"releaseName", []string{"--set", "foo.bar[0]={A,B}"}}},
 		},
+		{
+			name: "foo needs bar",
+			releases: []ReleaseSpec{
+				{
+					Name:  "foo",
+					Chart: "charts/foo",
+					Needs: []string{
+						"bar",
+					},
+				},
+				{
+					Name:  "bar",
+					Chart: "charts/bar",
+				},
+			},
+			helm:         &mockHelmExec{},
+			wantReleases: []mockRelease{{"bar", []string{}}, {"foo", []string{}}},
+		},
+		{
+			name: "bar needs foo",
+			releases: []ReleaseSpec{
+				{
+					Name:  "foo",
+					Chart: "charts/foo",
+				},
+				{
+					Name:  "bar",
+					Chart: "charts/bar",
+					Needs: []string{
+						"foo",
+					},
+				},
+			},
+			helm:         &mockHelmExec{},
+			wantReleases: []mockRelease{{"foo", []string{}}, {"bar", []string{}}},
+		},
+		{
+			name: "ns2/bar needs ns1/foo",
+			releases: []ReleaseSpec{
+				{
+					Name:      "foo",
+					Namespace: "ns1",
+					Chart:     "charts/foo",
+				},
+				{
+					Name:      "bar",
+					Namespace: "ns2",
+					Chart:     "charts/bar",
+					Needs: []string{
+						"ns1/foo",
+					},
+				},
+			},
+			helm:         &mockHelmExec{},
+			wantReleases: []mockRelease{{"foo", []string{"--namespace", "ns1"}}, {"bar", []string{"--namespace", "ns2"}}},
+		},
+		{
+			name: "tillerns1/ns1/foo needs tillerns2/ns2/bar",
+			releases: []ReleaseSpec{
+				{
+					Name:            "foo",
+					Chart:           "charts/foo",
+					Namespace:       "ns1",
+					TillerNamespace: "tillerns1",
+					Needs: []string{
+						"tillerns2/ns2/bar",
+					},
+				},
+				{
+					Name:            "bar",
+					Namespace:       "ns2",
+					TillerNamespace: "tillerns2",
+					Chart:           "charts/bar",
+				},
+			},
+			helm:         &mockHelmExec{},
+			wantReleases: []mockRelease{{"bar", []string{"--tiller-namespace", "tillerns2", "--namespace", "ns2"}}, {"foo", []string{"--tiller-namespace", "tillerns1", "--namespace", "ns1"}}},
+		},
+		{
+			name: "tillerns1/ns1/foo needs tillerns2/ns2/bar",
+			releases: []ReleaseSpec{
+				{
+					Name:            "foo",
+					Chart:           "charts/foo",
+					Namespace:       "ns1",
+					TillerNamespace: "tillerns1",
+					Needs: []string{
+						"bar",
+					},
+				},
+				{
+					Name:            "bar",
+					Namespace:       "ns2",
+					TillerNamespace: "tillerns2",
+					Chart:           "charts/bar",
+				},
+			},
+			helm:          &mockHelmExec{},
+			wantErrorMsgs: []string{`"tillerns1/ns1/foo" needs "bar", but it must be one of tillerns1/ns1/foo, tillerns2/ns2/bar`},
+		},
 	}
 	for i := range tests {
 		tt := tests[i]
@@ -970,7 +1071,23 @@ func TestHelmState_SyncReleases(t *testing.T) {
 				logger:      logger,
 				valsRuntime: valsRuntime,
 			}
-			if _ = state.SyncReleases(&AffectedReleases{}, tt.helm, []string{}, 1); !reflect.DeepEqual(tt.helm.releases, tt.wantReleases) {
+			if errs := state.SyncReleases(&AffectedReleases{}, tt.helm, []string{}, 1); errs != nil && len(errs) > 0 {
+				if len(errs) != len(tt.wantErrorMsgs) {
+					t.Fatalf("Unexpected errors: %v\nExpected: %v", errs, tt.wantErrorMsgs)
+				}
+				var mismatch int
+				for i := range tt.wantErrorMsgs {
+					expected := tt.wantErrorMsgs[i]
+					actual := errs[i].Error()
+					if !reflect.DeepEqual(actual, expected) {
+						t.Errorf("Unexpected error: expected=%v, got=%v", expected, actual)
+					}
+				}
+				if mismatch > 0 {
+					t.Fatalf("%d unexpected errors detected", mismatch)
+				}
+			}
+			if !reflect.DeepEqual(tt.helm.releases, tt.wantReleases) {
 				t.Errorf("HelmState.SyncReleases() for [%s] = %v, want %v", tt.name, tt.helm.releases, tt.wantReleases)
 			}
 		})