From 111a248523b60700c34b2b29e5ef1370fd1ed1fb Mon Sep 17 00:00:00 2001 From: Yusuke Kuoka Date: Mon, 10 Oct 2022 12:46:17 +0000 Subject: [PATCH] fixup! Fix preapply hook behavior Signed-off-by: Yusuke Kuoka --- pkg/app/app.go | 14 +++++++++----- .../testapply/delete_bar_when_bar_needs_foo/log | 7 +++++++ .../testapply/delete_bar_when_foo_needs_bar/log | 7 +++++++ .../delete_foo_and_bar_when_bar_needs_foo/log | 7 +++++++ .../delete_foo_and_bar_when_foo_needs_bar/log | 7 +++++++ .../testapply/delete_foo_when_bar_needs_foo/log | 7 +++++++ .../testapply/delete_foo_when_foo_needs_bar/log | 7 +++++++ .../helm2_upgrade_when_tns1/foo_needs_tns2/bar/log | 7 +++++++ pkg/app/testdata/testapply/install/log | 7 +++++++ pkg/app/testdata/testapply/smoke/log | 13 +++++++++++++ .../log | 7 +++++++ .../testapply/upgrade_when_bar_needs_foo/log | 7 +++++++ .../log | 7 +++++++ .../testapply/upgrade_when_foo_needs_bar/log | 7 +++++++ .../upgrade_when_ns1/foo_needs_ns2/bar/log | 7 +++++++ .../upgrade_when_ns2/bar_needs_ns1/foo/log | 7 +++++++ .../testapply_2/deduplicate_by_--selector/log | 5 +++++ .../log | 5 +++++ .../log | 3 +++ .../hooks_are_run_on_to-be-uninstalled_release/log | 6 +++--- pkg/state/state_run.go | 1 + 21 files changed, 137 insertions(+), 8 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index cd550f10..a09b3047 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -960,13 +960,17 @@ func withDAG(templated *state.HelmState, helm helmexec.Interface, logger *zap.Su return false, []error{err} } - return withBatches(templated, batches, helm, logger, converge) + return withBatches(opts.Purpose, templated, batches, helm, logger, converge) } -func withBatches(templated *state.HelmState, batches [][]state.Release, helm helmexec.Interface, logger *zap.SugaredLogger, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) (bool, []error) { +func withBatches(purpose string, templated *state.HelmState, batches [][]state.Release, helm helmexec.Interface, logger *zap.SugaredLogger, converge func(*state.HelmState, helmexec.Interface) (bool, []error)) (bool, []error) { numBatches := len(batches) - logger.Debugf("processing %d groups of releases in this order:\n%s", numBatches, printBatches(batches)) + if purpose == "" { + purpose = "processing" + } + + logger.Debugf("%s %d groups of releases in this order:\n%s", purpose, numBatches, printBatches(batches)) any := false @@ -983,7 +987,7 @@ func withBatches(templated *state.HelmState, batches [][]state.Release, helm hel releaseIds = append(releaseIds, state.ReleaseToID(&release)) } - logger.Debugf("processing releases in group %d/%d: %s", i+1, numBatches, strings.Join(releaseIds, ", ")) + logger.Debugf("%s releases in group %d/%d: %s", purpose, i+1, numBatches, strings.Join(releaseIds, ", ")) batchSt := *templated batchSt.Releases = targets @@ -1360,7 +1364,7 @@ Do you really want to apply? st.Releases = selectedAndNeededReleases if !interactive || interactive && r.askForConfirmation(confMsg) { - if _, preapplyErrors := withDAG(st, helm, a.Logger, state.PlanOptions{Reverse: true, SelectedReleases: toDelete, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { + if _, preapplyErrors := withDAG(st, helm, a.Logger, state.PlanOptions{Purpose: "invoking preapply hooks for", Reverse: true, SelectedReleases: toApplyWithNeeds, SkipNeeds: true}, a.WrapWithoutSelector(func(subst *state.HelmState, helm helmexec.Interface) []error { for _, r := range subst.Releases { release := r if _, err := st.TriggerPreapplyEvent(&release, "apply"); err != nil { diff --git a/pkg/app/testdata/testapply/delete_bar_when_bar_needs_foo/log b/pkg/app/testdata/testapply/delete_bar_when_bar_needs_foo/log index fb2a598c..b5920210 100644 --- a/pkg/app/testdata/testapply/delete_bar_when_bar_needs_foo/log +++ b/pkg/app/testdata/testapply/delete_bar_when_bar_needs_foo/log @@ -38,6 +38,13 @@ Affected releases are: bar (stable/mychart2) DELETED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default//bar +2 default//foo + +invoking preapply hooks for releases in group 1/2: default//bar +invoking preapply hooks for releases in group 2/2: default//foo processing 1 groups of releases in this order: GROUP RELEASES 1 default//bar diff --git a/pkg/app/testdata/testapply/delete_bar_when_foo_needs_bar/log b/pkg/app/testdata/testapply/delete_bar_when_foo_needs_bar/log index 759c97c6..58646152 100644 --- a/pkg/app/testdata/testapply/delete_bar_when_foo_needs_bar/log +++ b/pkg/app/testdata/testapply/delete_bar_when_foo_needs_bar/log @@ -38,6 +38,13 @@ Affected releases are: bar (stable/mychart2) DELETED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default//foo +2 default//bar + +invoking preapply hooks for releases in group 1/2: default//foo +invoking preapply hooks for releases in group 2/2: default//bar processing 1 groups of releases in this order: GROUP RELEASES 1 default//bar diff --git a/pkg/app/testdata/testapply/delete_foo_and_bar_when_bar_needs_foo/log b/pkg/app/testdata/testapply/delete_foo_and_bar_when_bar_needs_foo/log index 6e283d18..a5c9aefc 100644 --- a/pkg/app/testdata/testapply/delete_foo_and_bar_when_bar_needs_foo/log +++ b/pkg/app/testdata/testapply/delete_foo_and_bar_when_bar_needs_foo/log @@ -40,6 +40,13 @@ Affected releases are: bar (stable/mychart2) DELETED foo (stable/mychart1) DELETED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default//bar +2 default//foo + +invoking preapply hooks for releases in group 1/2: default//bar +invoking preapply hooks for releases in group 2/2: default//foo processing 2 groups of releases in this order: GROUP RELEASES 1 default//bar diff --git a/pkg/app/testdata/testapply/delete_foo_and_bar_when_foo_needs_bar/log b/pkg/app/testdata/testapply/delete_foo_and_bar_when_foo_needs_bar/log index fc2d2d79..79fd557b 100644 --- a/pkg/app/testdata/testapply/delete_foo_and_bar_when_foo_needs_bar/log +++ b/pkg/app/testdata/testapply/delete_foo_and_bar_when_foo_needs_bar/log @@ -40,6 +40,13 @@ Affected releases are: bar (stable/mychart2) DELETED foo (stable/mychart1) DELETED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default//foo +2 default//bar + +invoking preapply hooks for releases in group 1/2: default//foo +invoking preapply hooks for releases in group 2/2: default//bar processing 2 groups of releases in this order: GROUP RELEASES 1 default//foo diff --git a/pkg/app/testdata/testapply/delete_foo_when_bar_needs_foo/log b/pkg/app/testdata/testapply/delete_foo_when_bar_needs_foo/log index cd549569..44cf1b57 100644 --- a/pkg/app/testdata/testapply/delete_foo_when_bar_needs_foo/log +++ b/pkg/app/testdata/testapply/delete_foo_when_bar_needs_foo/log @@ -38,6 +38,13 @@ Affected releases are: bar (stable/mychart2) UPDATED foo (stable/mychart1) DELETED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default//bar +2 default//foo + +invoking preapply hooks for releases in group 1/2: default//bar +invoking preapply hooks for releases in group 2/2: default//foo processing 1 groups of releases in this order: GROUP RELEASES 1 default//foo diff --git a/pkg/app/testdata/testapply/delete_foo_when_foo_needs_bar/log b/pkg/app/testdata/testapply/delete_foo_when_foo_needs_bar/log index a5885a58..a03c04c3 100644 --- a/pkg/app/testdata/testapply/delete_foo_when_foo_needs_bar/log +++ b/pkg/app/testdata/testapply/delete_foo_when_foo_needs_bar/log @@ -38,6 +38,13 @@ Affected releases are: bar (stable/mychart2) UPDATED foo (stable/mychart1) DELETED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default//foo +2 default//bar + +invoking preapply hooks for releases in group 1/2: default//foo +invoking preapply hooks for releases in group 2/2: default//bar processing 1 groups of releases in this order: GROUP RELEASES 1 default//foo diff --git a/pkg/app/testdata/testapply/helm2_upgrade_when_tns1/foo_needs_tns2/bar/log b/pkg/app/testdata/testapply/helm2_upgrade_when_tns1/foo_needs_tns2/bar/log index 22ae35df..e954f517 100644 --- a/pkg/app/testdata/testapply/helm2_upgrade_when_tns1/foo_needs_tns2/bar/log +++ b/pkg/app/testdata/testapply/helm2_upgrade_when_tns1/foo_needs_tns2/bar/log @@ -44,6 +44,13 @@ Affected releases are: bar (stable/mychart2) UPDATED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default/tns1/foo +2 default/tns2/bar + +invoking preapply hooks for releases in group 1/2: default/tns1/foo +invoking preapply hooks for releases in group 2/2: default/tns2/bar processing 2 groups of releases in this order: GROUP RELEASES 1 default/tns2/bar diff --git a/pkg/app/testdata/testapply/install/log b/pkg/app/testdata/testapply/install/log index c43ac830..431bbf69 100644 --- a/pkg/app/testdata/testapply/install/log +++ b/pkg/app/testdata/testapply/install/log @@ -41,6 +41,13 @@ Affected releases are: baz (stable/mychart3) UPDATED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default//foo +2 default//baz, default//bar + +invoking preapply hooks for releases in group 1/2: default//foo +invoking preapply hooks for releases in group 2/2: default//baz, default//bar processing 2 groups of releases in this order: GROUP RELEASES 1 default//baz, default//bar diff --git a/pkg/app/testdata/testapply/smoke/log b/pkg/app/testdata/testapply/smoke/log index 7a27d168..4f491886 100644 --- a/pkg/app/testdata/testapply/smoke/log +++ b/pkg/app/testdata/testapply/smoke/log @@ -135,6 +135,19 @@ Affected releases are: logging (charts/fluent-bit) UPDATED servicemesh (charts/istio) UPDATED +invoking preapply hooks for 5 groups of releases in this order: +GROUP RELEASES +1 default//frontend-v1, default//frontend-v2, default//frontend-v3 +2 default//backend-v1, default//backend-v2 +3 default//anotherbackend +4 default//database, default//servicemesh +5 default//logging, default//front-proxy + +invoking preapply hooks for releases in group 1/5: default//frontend-v1, default//frontend-v2, default//frontend-v3 +invoking preapply hooks for releases in group 2/5: default//backend-v1, default//backend-v2 +invoking preapply hooks for releases in group 3/5: default//anotherbackend +invoking preapply hooks for releases in group 4/5: default//database, default//servicemesh +invoking preapply hooks for releases in group 5/5: default//logging, default//front-proxy processing 2 groups of releases in this order: GROUP RELEASES 1 default//frontend-v1 diff --git a/pkg/app/testdata/testapply/upgrade_when_bar_needs_foo,_with_ns_override/log b/pkg/app/testdata/testapply/upgrade_when_bar_needs_foo,_with_ns_override/log index 341fcf6a..4c800ebf 100644 --- a/pkg/app/testdata/testapply/upgrade_when_bar_needs_foo,_with_ns_override/log +++ b/pkg/app/testdata/testapply/upgrade_when_bar_needs_foo,_with_ns_override/log @@ -36,6 +36,13 @@ Affected releases are: bar (stable/mychart2) UPDATED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default/testNamespace/bar +2 default/testNamespace/foo + +invoking preapply hooks for releases in group 1/2: default/testNamespace/bar +invoking preapply hooks for releases in group 2/2: default/testNamespace/foo processing 2 groups of releases in this order: GROUP RELEASES 1 default/testNamespace/foo diff --git a/pkg/app/testdata/testapply/upgrade_when_bar_needs_foo/log b/pkg/app/testdata/testapply/upgrade_when_bar_needs_foo/log index 151fefaa..b3a32c95 100644 --- a/pkg/app/testdata/testapply/upgrade_when_bar_needs_foo/log +++ b/pkg/app/testdata/testapply/upgrade_when_bar_needs_foo/log @@ -36,6 +36,13 @@ Affected releases are: bar (stable/mychart2) UPDATED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default//bar +2 default//foo + +invoking preapply hooks for releases in group 1/2: default//bar +invoking preapply hooks for releases in group 2/2: default//foo processing 2 groups of releases in this order: GROUP RELEASES 1 default//foo diff --git a/pkg/app/testdata/testapply/upgrade_when_foo_needs_bar,_with_ns_override/log b/pkg/app/testdata/testapply/upgrade_when_foo_needs_bar,_with_ns_override/log index eb1a7796..ef4a3f62 100644 --- a/pkg/app/testdata/testapply/upgrade_when_foo_needs_bar,_with_ns_override/log +++ b/pkg/app/testdata/testapply/upgrade_when_foo_needs_bar,_with_ns_override/log @@ -36,6 +36,13 @@ Affected releases are: bar (stable/mychart2) UPDATED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default/testNamespace/foo +2 default/testNamespace/bar + +invoking preapply hooks for releases in group 1/2: default/testNamespace/foo +invoking preapply hooks for releases in group 2/2: default/testNamespace/bar processing 2 groups of releases in this order: GROUP RELEASES 1 default/testNamespace/bar diff --git a/pkg/app/testdata/testapply/upgrade_when_foo_needs_bar/log b/pkg/app/testdata/testapply/upgrade_when_foo_needs_bar/log index e77cda0d..a8a99ba8 100644 --- a/pkg/app/testdata/testapply/upgrade_when_foo_needs_bar/log +++ b/pkg/app/testdata/testapply/upgrade_when_foo_needs_bar/log @@ -36,6 +36,13 @@ Affected releases are: bar (stable/mychart2) UPDATED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default//foo +2 default//bar + +invoking preapply hooks for releases in group 1/2: default//foo +invoking preapply hooks for releases in group 2/2: default//bar processing 2 groups of releases in this order: GROUP RELEASES 1 default//bar diff --git a/pkg/app/testdata/testapply/upgrade_when_ns1/foo_needs_ns2/bar/log b/pkg/app/testdata/testapply/upgrade_when_ns1/foo_needs_ns2/bar/log index cb2a3ea4..7e99abab 100644 --- a/pkg/app/testdata/testapply/upgrade_when_ns1/foo_needs_ns2/bar/log +++ b/pkg/app/testdata/testapply/upgrade_when_ns1/foo_needs_ns2/bar/log @@ -40,6 +40,13 @@ Affected releases are: bar (stable/mychart2) UPDATED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default/ns1/foo +2 default/ns2/bar + +invoking preapply hooks for releases in group 1/2: default/ns1/foo +invoking preapply hooks for releases in group 2/2: default/ns2/bar processing 2 groups of releases in this order: GROUP RELEASES 1 default/ns2/bar diff --git a/pkg/app/testdata/testapply/upgrade_when_ns2/bar_needs_ns1/foo/log b/pkg/app/testdata/testapply/upgrade_when_ns2/bar_needs_ns1/foo/log index 721d8989..059c3d06 100644 --- a/pkg/app/testdata/testapply/upgrade_when_ns2/bar_needs_ns1/foo/log +++ b/pkg/app/testdata/testapply/upgrade_when_ns2/bar_needs_ns1/foo/log @@ -40,6 +40,13 @@ Affected releases are: bar (stable/mychart2) UPDATED foo (stable/mychart1) UPDATED +invoking preapply hooks for 2 groups of releases in this order: +GROUP RELEASES +1 default/ns2/bar +2 default/ns1/foo + +invoking preapply hooks for releases in group 1/2: default/ns2/bar +invoking preapply hooks for releases in group 2/2: default/ns1/foo processing 2 groups of releases in this order: GROUP RELEASES 1 default/ns1/foo diff --git a/pkg/app/testdata/testapply_2/deduplicate_by_--selector/log b/pkg/app/testdata/testapply_2/deduplicate_by_--selector/log index 24ad3af8..1caa2793 100644 --- a/pkg/app/testdata/testapply_2/deduplicate_by_--selector/log +++ b/pkg/app/testdata/testapply_2/deduplicate_by_--selector/log @@ -53,6 +53,11 @@ merged environment: &{default map[] map[]} Affected releases are: foo (incubator/raw) UPDATED +invoking preapply hooks for 1 groups of releases in this order: +GROUP RELEASES +1 default/default/foo + +invoking preapply hooks for releases in group 1/1: default/default/foo processing 1 groups of releases in this order: GROUP RELEASES 1 default/default/foo diff --git a/pkg/app/testdata/testapply_2/select_single_release_from_helmfile_with_two_duplicates/log b/pkg/app/testdata/testapply_2/select_single_release_from_helmfile_with_two_duplicates/log index 21dfd3f8..31ee4bbc 100644 --- a/pkg/app/testdata/testapply_2/select_single_release_from_helmfile_with_two_duplicates/log +++ b/pkg/app/testdata/testapply_2/select_single_release_from_helmfile_with_two_duplicates/log @@ -57,6 +57,11 @@ merged environment: &{default map[] map[]} Affected releases are: foo (incubator/raw) UPDATED +invoking preapply hooks for 1 groups of releases in this order: +GROUP RELEASES +1 default/default/foo + +invoking preapply hooks for releases in group 1/1: default/default/foo processing 1 groups of releases in this order: GROUP RELEASES 1 default/default/foo diff --git a/pkg/app/testdata/testapply_hooks/hooks_are_not_run_on_alreadyd_uninstalled_release/log b/pkg/app/testdata/testapply_hooks/hooks_are_not_run_on_alreadyd_uninstalled_release/log index ac333ffe..75c88b6b 100644 --- a/pkg/app/testdata/testapply_hooks/hooks_are_not_run_on_alreadyd_uninstalled_release/log +++ b/pkg/app/testdata/testapply_hooks/hooks_are_not_run_on_alreadyd_uninstalled_release/log @@ -5,6 +5,9 @@ hook[prepare] logs | hook[preapply] logs | foo hook[preapply] logs | +hook[preapply] logs | bar +hook[preapply] logs | + hook[presync] logs | foo hook[presync] logs | diff --git a/pkg/app/testdata/testapply_hooks/hooks_are_run_on_to-be-uninstalled_release/log b/pkg/app/testdata/testapply_hooks/hooks_are_run_on_to-be-uninstalled_release/log index c79da665..0f285254 100644 --- a/pkg/app/testdata/testapply_hooks/hooks_are_run_on_to-be-uninstalled_release/log +++ b/pkg/app/testdata/testapply_hooks/hooks_are_run_on_to-be-uninstalled_release/log @@ -2,15 +2,15 @@ hook[prepare] logs | foo hook[prepare] logs | +hook[preapply] logs | foo +hook[preapply] logs | + hook[preapply] logs | bar hook[preapply] logs | hook[presync] logs | bar hook[presync] logs | -hook[preapply] logs | foo -hook[preapply] logs | - hook[presync] logs | foo hook[presync] logs | diff --git a/pkg/state/state_run.go b/pkg/state/state_run.go index 0500a1e7..3a89197f 100644 --- a/pkg/state/state_run.go +++ b/pkg/state/state_run.go @@ -100,6 +100,7 @@ func (st *HelmState) iterateOnReleases(helm helmexec.Interface, concurrency int, } type PlanOptions struct { + Purpose string Reverse bool IncludeNeeds bool IncludeTransitiveNeeds bool