From db81f1809595184e2a92f6b5ca1b8203876b9b6a Mon Sep 17 00:00:00 2001 From: Anton Bretting Date: Mon, 2 May 2022 23:18:47 +0200 Subject: [PATCH] Only run preapply or presync Signed-off-by: Anton Bretting --- pkg/app/app.go | 2 + pkg/app/app_apply_hooks_test.go | 33 ++++++++- pkg/app/app_apply_test.go | 30 -------- .../apply_release_with_preapply_hook/log | 68 +++++++++++++++++++ .../apply_release_with_preapply_hook#01/log | 5 ++ .../apply_release_with_preapply_hook#02/log | 8 +++ pkg/event/bus.go | 7 ++ pkg/event/bus_test.go | 26 +++---- pkg/state/state.go | 32 ++++++++- 9 files changed, 166 insertions(+), 45 deletions(-) create mode 100644 pkg/app/testdata/testapply_2/apply_release_with_preapply_hook/log create mode 100644 pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#02/log diff --git a/pkg/app/app.go b/pkg/app/app.go index 4af49d56..69217e0b 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -1330,6 +1330,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { for _, hook := range release.Hooks { if slices.Contains(hook.Events, "preapply") { releasesWithPreApply = append(releasesWithPreApply, release) + break } } } @@ -1380,6 +1381,7 @@ Do you really want to apply? a.Logger.Infof("\nRunning preapply hook for %s:", release.Name) if _, err := st.TriggerPreapplyEvent(&release, "apply"); err != nil { syncErrs = append(syncErrs, err) + continue } } diff --git a/pkg/app/app_apply_hooks_test.go b/pkg/app/app_apply_hooks_test.go index eeb74cbb..50af7e2a 100644 --- a/pkg/app/app_apply_hooks_test.go +++ b/pkg/app/app_apply_hooks_test.go @@ -213,7 +213,38 @@ releases: labels: app: test hooks: - - events: ["preapply", "presync"] + - events: ["prepare", "preapply", "presync"] + command: echo + showlogs: true + args: ["foo"] +`, + }, + selectors: []string{"name=foo"}, + upgraded: []exectest.Release{ + {Name: "foo"}, + }, + diffs: map[exectest.DiffKey]error{ + {Name: "foo", Chart: "incubator/raw", Flags: "--kube-contextdefault--namespacedefault--detailed-exitcode"}: helmexec.ExitError{Code: 2}, + }, + error: "", + // as we check for log output, set concurrency to 1 to avoid non-deterministic test result + concurrency: 1, + logLevel: "info", + }) + }) + + t.Run("apply release with preapply hook", func(t *testing.T) { + check(t, testcase{ + files: map[string]string{ + "/path/to/helmfile.yaml": ` +releases: +- name: foo + chart: incubator/raw + namespace: default + labels: + app: test + hooks: + - events: ["presync"] command: echo showlogs: true args: ["foo"] diff --git a/pkg/app/app_apply_test.go b/pkg/app/app_apply_test.go index a3512801..e1e07827 100644 --- a/pkg/app/app_apply_test.go +++ b/pkg/app/app_apply_test.go @@ -1340,34 +1340,4 @@ foo 4 Fri Nov 1 08:40:07 2019 DEPLOYED raw-3.1.0 3.1.0 default concurrency: 1, }) }) - - t.Run("apply release with preapply hook", func(t *testing.T) { - check(t, testcase{ - files: map[string]string{ - "/path/to/helmfile.yaml": ` -releases: -- name: foo - chart: incubator/raw - namespace: default - labels: - app: test - hooks: - - events: ["preapply"] - command: echo - showlogs: true - args: ["foo"] -`, - }, - selectors: []string{"name=foo"}, - upgraded: []exectest.Release{ - {Name: "foo"}, - }, - diffs: map[exectest.DiffKey]error{ - {Name: "foo", Chart: "incubator/raw", Flags: "--kube-contextdefault--namespacedefault--detailed-exitcode"}: helmexec.ExitError{Code: 2}, - }, - error: "", - // as we check for log output, set concurrency to 1 to avoid non-deterministic test result - concurrency: 1, - }) - }) } diff --git a/pkg/app/testdata/testapply_2/apply_release_with_preapply_hook/log b/pkg/app/testdata/testapply_2/apply_release_with_preapply_hook/log new file mode 100644 index 00000000..2961d200 --- /dev/null +++ b/pkg/app/testdata/testapply_2/apply_release_with_preapply_hook/log @@ -0,0 +1,68 @@ +processing file "helmfile.yaml" in directory "." +first-pass rendering starting for "helmfile.yaml.part.0": inherited=&{default map[] map[]}, overrode= +first-pass uses: &{default map[] map[]} +first-pass rendering output of "helmfile.yaml.part.0": + 0: + 1: releases: + 2: - name: foo + 3: chart: incubator/raw + 4: namespace: default + 5: labels: + 6: app: test + 7: hooks: + 8: - events: ["preapply"] + 9: command: echo +10: showlogs: true +11: args: ["foo"] +12: + +first-pass produced: &{default map[] map[]} +first-pass rendering result of "helmfile.yaml.part.0": {default map[] map[]} +vals: +map[] +defaultVals:[] +second-pass rendering result of "helmfile.yaml.part.0": + 0: + 1: releases: + 2: - name: foo + 3: chart: incubator/raw + 4: namespace: default + 5: labels: + 6: app: test + 7: hooks: + 8: - events: ["preapply"] + 9: command: echo +10: showlogs: true +11: args: ["foo"] +12: + +merged environment: &{default map[] map[]} +1 release(s) matching name=foo found in helmfile.yaml + +Affected releases are: + foo (incubator/raw) UPDATED +Releases with preapply hooks: + foo (incubator/raw) + +Running preapply hook for foo: +hook[echo]: stateFilePath=helmfile.yaml, basePath=. +hook[echo]: triggered by event "preapply" + +echo:XlBWj> foo +hook[echo]: foo + + + +hook[preapply] logs | foo +hook[preapply] logs | +processing 1 groups of releases in this order: +GROUP RELEASES +1 default/default/foo + +processing releases in group 1/1: default/default/foo +getting deployed release version failed:unexpected list key: {^foo$ --kube-contextdefault--deleting--deployed--failed--pending} + +UPDATED RELEASES: +NAME CHART VERSION +foo incubator/raw + diff --git a/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#01/log b/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#01/log index 5c8158c0..cbffb336 100644 --- a/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#01/log +++ b/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#01/log @@ -1,8 +1,13 @@ +hook[prepare] logs | foo +hook[prepare] logs | + Running preapply hook for foo: hook[preapply] logs | foo hook[preapply] logs | +hook[echo]: already executed by "preapply", skipping "presync" + UPDATED RELEASES: NAME CHART VERSION diff --git a/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#02/log b/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#02/log new file mode 100644 index 00000000..8f9089e2 --- /dev/null +++ b/pkg/app/testdata/testapply_hooks/apply_release_with_preapply_hook#02/log @@ -0,0 +1,8 @@ + +hook[presync] logs | foo +hook[presync] logs | + +UPDATED RELEASES: +NAME CHART VERSION +foo incubator/raw + diff --git a/pkg/event/bus.go b/pkg/event/bus.go index b3448720..cd0c273a 100644 --- a/pkg/event/bus.go +++ b/pkg/event/bus.go @@ -10,6 +10,8 @@ import ( "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/helmexec" "github.com/helmfile/helmfile/pkg/tmpl" + "go.uber.org/zap" + "golang.org/x/exp/slices" ) type Hook struct { @@ -19,6 +21,7 @@ type Hook struct { Kubectl map[string]string `yaml:"kubectlApply,omitempty"` Args []string `yaml:"args"` ShowLogs bool `yaml:"showlogs"` + Executed bool `yaml:"-"` } type event struct { @@ -71,6 +74,10 @@ func (bus *Bus) Trigger(evt string, evtErr error, context map[string]interface{} } } + if slices.Contains(hook.Events, "preapply") && evt == "presync" && hook.Executed { + bus.Logger.Infof("hook[%s]: already executed by \"preapply\", skipping \"%s\"\n", name, evt) + continue + } if hook.Kubectl != nil { if hook.Command != "" { bus.Logger.Warnf("warn: ignoring command '%s' given within a kubectlApply hook", hook.Command) diff --git a/pkg/event/bus_test.go b/pkg/event/bus_test.go index f2cc2d89..2352fad1 100644 --- a/pkg/event/bus_test.go +++ b/pkg/event/bus_test.go @@ -41,21 +41,21 @@ func TestTrigger(t *testing.T) { }{ { "okhook1", - &Hook{"okhook1", []string{"foo"}, "ok", nil, []string{}, true}, + &Hook{"okhook1", []string{"foo"}, "ok", nil, []string{}, true, false}, "foo", true, "", }, { "okhooké", - &Hook{"okhook2", []string{"foo"}, "ok", nil, []string{}, false}, + &Hook{"okhook2", []string{"foo"}, "ok", nil, []string{}, false, false}, "foo", true, "", }, { "missinghook1", - &Hook{"okhook1", []string{"foo"}, "ok", nil, []string{}, false}, + &Hook{"okhook1", []string{"foo"}, "ok", nil, []string{}, false, false}, "bar", false, "", @@ -69,70 +69,70 @@ func TestTrigger(t *testing.T) { }, { "nghook1", - &Hook{"nghook1", []string{"foo"}, "ng", nil, []string{}, false}, + &Hook{"nghook1", []string{"foo"}, "ng", nil, []string{}, false, false}, "foo", false, "hook[nghook1]: command `ng` failed: cmd failed due to invalid cmd: ng", }, { "nghook2", - &Hook{"nghook2", []string{"foo"}, "ok", nil, []string{"ng"}, false}, + &Hook{"nghook2", []string{"foo"}, "ok", nil, []string{"ng"}, false, false}, "foo", false, "hook[nghook2]: command `ok` failed: cmd failed due to invalid arg: ng", }, { "okkubeapply1", - &Hook{"okkubeapply1", []string{"foo"}, "", map[string]string{"kustomize": "kustodir"}, []string{}, false}, + &Hook{"okkubeapply1", []string{"foo"}, "", map[string]string{"kustomize": "kustodir"}, []string{}, false, false}, "foo", true, "", }, { "okkubeapply2", - &Hook{"okkubeapply2", []string{"foo"}, "", map[string]string{"filename": "resource.yaml"}, []string{}, false}, + &Hook{"okkubeapply2", []string{"foo"}, "", map[string]string{"filename": "resource.yaml"}, []string{}, false, false}, "foo", true, "", }, { "kokubeapply", - &Hook{"kokubeapply", []string{"foo"}, "", map[string]string{"kustomize": "kustodir", "filename": "resource.yaml"}, []string{}, true}, + &Hook{"kokubeapply", []string{"foo"}, "", map[string]string{"kustomize": "kustodir", "filename": "resource.yaml"}, []string{}, true, false}, "foo", false, "hook[kokubeapply]: kustomize & filename cannot be used together", }, { "kokubeapply2", - &Hook{"kokubeapply2", []string{"foo"}, "", map[string]string{}, []string{}, true}, + &Hook{"kokubeapply2", []string{"foo"}, "", map[string]string{}, []string{}, true, false}, "foo", false, "hook[kokubeapply2]: either kustomize or filename must be given", }, { "kokubeapply3", - &Hook{"", []string{"foo"}, "", map[string]string{}, []string{}, true}, + &Hook{"", []string{"foo"}, "", map[string]string{}, []string{}, true, false}, "foo", false, "hook[kubectlApply]: either kustomize or filename must be given", }, { "warnkubeapply1", - &Hook{"warnkubeapply1", []string{"foo"}, "ok", map[string]string{"filename": "resource.yaml"}, []string{}, true}, + &Hook{"warnkubeapply1", []string{"foo"}, "ok", map[string]string{"filename": "resource.yaml"}, []string{}, true, false}, "foo", true, "", }, { "warnkubeapply2", - &Hook{"warnkubeapply2", []string{"foo"}, "", map[string]string{"filename": "resource.yaml"}, []string{"ng"}, true}, + &Hook{"warnkubeapply2", []string{"foo"}, "", map[string]string{"filename": "resource.yaml"}, []string{"ng"}, true, false}, "foo", true, "", }, { "warnkubeapply3", - &Hook{"warnkubeapply3", []string{"foo"}, "ok", map[string]string{"filename": "resource.yaml"}, []string{"ng"}, true}, + &Hook{"warnkubeapply3", []string{"foo"}, "ok", map[string]string{"filename": "resource.yaml"}, []string{"ng"}, true, false}, "foo", true, "", diff --git a/pkg/state/state.go b/pkg/state/state.go index c133905c..48815202 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -18,6 +18,15 @@ import ( "text/template" "github.com/imdario/mergo" + "github.com/variantdev/chartify" + "golang.org/x/exp/slices" + + "github.com/helmfile/helmfile/pkg/environment" + "github.com/helmfile/helmfile/pkg/event" + "github.com/helmfile/helmfile/pkg/helmexec" + "github.com/helmfile/helmfile/pkg/remote" + "github.com/helmfile/helmfile/pkg/tmpl" + "github.com/tatsushid/go-prettytable" "github.com/variantdev/chartify" "github.com/variantdev/vals" @@ -2256,6 +2265,16 @@ func (st *HelmState) TriggerPreapplyEvent(r *ReleaseSpec, helmfileCommand string } func (st *HelmState) triggerReleaseEvent(evt string, evtErr error, r *ReleaseSpec, helmfileCmd string) (bool, error) { + + var stateRelease ReleaseSpec + + for id, release := range st.Releases { + if release.Name == r.Name { + stateRelease = st.Releases[id] + break + } + } + bus := &event.Bus{ Hooks: r.Hooks, StateFilePath: st.FilePath, @@ -2272,7 +2291,18 @@ func (st *HelmState) triggerReleaseEvent(evt string, evtErr error, r *ReleaseSpe "Release": r, "HelmfileCommand": helmfileCmd, } - return bus.Trigger(evt, evtErr, data) + + executed, err := bus.Trigger(evt, evtErr, data) + + if executed { + for id, hook := range stateRelease.Hooks { + if slices.Contains(hook.Events, evt) { + stateRelease.Hooks[id].Executed = true + } + } + } + + return executed, err } // ResolveDeps returns a copy of this helmfile state with the concrete chart version numbers filled in for remote chart dependencies