From d77c72f82e771659153746bf1d764889d54c90fb Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Sun, 22 Oct 2023 13:57:08 -0300 Subject: [PATCH 1/3] fix: cleanup hooks not receiving error signal Closes #1041 Signed-off-by: Felipe Santos --- pkg/app/app.go | 12 +++++++++--- pkg/state/state.go | 26 ++++++++++++++++++-------- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index eaab62b7..634e380a 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -1552,7 +1552,13 @@ Do you really want to apply? for id := range releasesWithNoChange { r := releasesWithNoChange[id] - if _, err := st.TriggerCleanupEvent(&r, "apply"); err != nil { + + var firstErr error = nil + if len(errs) > 0 { + firstErr = errs[0] + } + + if _, err := st.TriggerCleanupEvent(&r, firstErr, "apply"); err != nil { a.Logger.Warnf("warn: %v\n", err) } } @@ -1601,7 +1607,7 @@ func (a *App) delete(r *Run, purge bool, c DestroyConfigProvider) (bool, []error for id := range releasesWithNoChange { r := releasesWithNoChange[id] - if _, err := st.TriggerCleanupEvent(&r, "delete"); err != nil { + if _, err := st.TriggerCleanupEvent(&r, nil, "delete"); err != nil { a.Logger.Warnf("warn: %v\n", err) } } @@ -1847,7 +1853,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { for id := range releasesWithNoChange { r := releasesWithNoChange[id] - if _, err := st.TriggerCleanupEvent(&r, "sync"); err != nil { + if _, err := st.TriggerCleanupEvent(&r, nil, "sync"); err != nil { a.Logger.Warnf("warn: %v\n", err) } } diff --git a/pkg/state/state.go b/pkg/state/state.go index 0fbff129..11e19522 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -892,7 +892,7 @@ func (st *HelmState) DeleteReleasesForSync(affectedReleases *AffectedReleases, h st.logger.Warnf("warn: %v\n", err) } - if _, err := st.TriggerCleanupEvent(release, "sync"); err != nil { + if _, err := st.TriggerCleanupEvent(release, relErr, "sync"); err != nil { st.logger.Warnf("warn: %v\n", err) } @@ -1018,7 +1018,7 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme } } - if _, err := st.TriggerCleanupEvent(release, "sync"); err != nil { + if _, err := st.TriggerCleanupEvent(release, relErr, "sync"); err != nil { if relErr == nil { relErr = newReleaseFailedError(release, err) } else { @@ -1548,7 +1548,12 @@ func (st *HelmState) TemplateReleases(helm helmexec.Interface, outputDir string, } } - if _, err := st.TriggerCleanupEvent(release, "template"); err != nil { + var firstErr error = nil + if len(errs) > 0 { + firstErr = errs[0] + } + + if _, err := st.TriggerCleanupEvent(release, firstErr, "template"); err != nil { st.logger.Warnf("warn: %v\n", err) } } @@ -1649,7 +1654,7 @@ func (st *HelmState) WriteReleasesValues(helm helmexec.Interface, additionalValu return []error{fmt.Errorf("writing values file %s: %w", outputValuesFile, err)} } - if _, err := st.TriggerCleanupEvent(release, "write-values"); err != nil { + if _, err := st.TriggerCleanupEvent(release, nil, "write-values"); err != nil { st.logger.Warnf("warn: %v\n", err) } } @@ -1724,7 +1729,12 @@ func (st *HelmState) LintReleases(helm helmexec.Interface, additionalValues []st } } - if _, err := st.TriggerCleanupEvent(&release, "lint"); err != nil { + var firstErr error = nil + if len(errs) > 0 { + firstErr = errs[0] + } + + if _, err := st.TriggerCleanupEvent(&release, firstErr, "lint"); err != nil { st.logger.Warnf("warn: %v\n", err) } } @@ -2071,7 +2081,7 @@ func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []st } if triggerCleanupEvents { - if _, err := st.TriggerCleanupEvent(prep.release, "diff"); err != nil { + if _, err := st.TriggerCleanupEvent(prep.release, nil, "diff"); err != nil { st.logger.Warnf("warn: %v\n", err) } } @@ -2425,8 +2435,8 @@ func (st *HelmState) triggerPrepareEvent(r *ReleaseSpec, helmfileCommand string) return st.triggerReleaseEvent("prepare", nil, r, helmfileCommand) } -func (st *HelmState) TriggerCleanupEvent(r *ReleaseSpec, helmfileCommand string) (bool, error) { - return st.triggerReleaseEvent("cleanup", nil, r, helmfileCommand) +func (st *HelmState) TriggerCleanupEvent(r *ReleaseSpec, evtErr error, helmfileCommand string) (bool, error) { + return st.triggerReleaseEvent("cleanup", evtErr, r, helmfileCommand) } func (st *HelmState) triggerPresyncEvent(r *ReleaseSpec, helmfileCommand string) (bool, error) { From 29babda1868c5ba15b4b828611f0d96a3ff00b0e Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Sun, 29 Sep 2024 12:42:01 -0300 Subject: [PATCH 2/3] Revert "fix: cleanup hooks not receiving error signal" This reverts commit d77c72f82e771659153746bf1d764889d54c90fb. Signed-off-by: Felipe Santos --- pkg/app/app.go | 12 +++--------- pkg/state/state.go | 26 ++++++++------------------ 2 files changed, 11 insertions(+), 27 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index 634e380a..eaab62b7 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -1552,13 +1552,7 @@ Do you really want to apply? for id := range releasesWithNoChange { r := releasesWithNoChange[id] - - var firstErr error = nil - if len(errs) > 0 { - firstErr = errs[0] - } - - if _, err := st.TriggerCleanupEvent(&r, firstErr, "apply"); err != nil { + if _, err := st.TriggerCleanupEvent(&r, "apply"); err != nil { a.Logger.Warnf("warn: %v\n", err) } } @@ -1607,7 +1601,7 @@ func (a *App) delete(r *Run, purge bool, c DestroyConfigProvider) (bool, []error for id := range releasesWithNoChange { r := releasesWithNoChange[id] - if _, err := st.TriggerCleanupEvent(&r, nil, "delete"); err != nil { + if _, err := st.TriggerCleanupEvent(&r, "delete"); err != nil { a.Logger.Warnf("warn: %v\n", err) } } @@ -1853,7 +1847,7 @@ func (a *App) sync(r *Run, c SyncConfigProvider) (bool, []error) { for id := range releasesWithNoChange { r := releasesWithNoChange[id] - if _, err := st.TriggerCleanupEvent(&r, nil, "sync"); err != nil { + if _, err := st.TriggerCleanupEvent(&r, "sync"); err != nil { a.Logger.Warnf("warn: %v\n", err) } } diff --git a/pkg/state/state.go b/pkg/state/state.go index 11e19522..0fbff129 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -892,7 +892,7 @@ func (st *HelmState) DeleteReleasesForSync(affectedReleases *AffectedReleases, h st.logger.Warnf("warn: %v\n", err) } - if _, err := st.TriggerCleanupEvent(release, relErr, "sync"); err != nil { + if _, err := st.TriggerCleanupEvent(release, "sync"); err != nil { st.logger.Warnf("warn: %v\n", err) } @@ -1018,7 +1018,7 @@ func (st *HelmState) SyncReleases(affectedReleases *AffectedReleases, helm helme } } - if _, err := st.TriggerCleanupEvent(release, relErr, "sync"); err != nil { + if _, err := st.TriggerCleanupEvent(release, "sync"); err != nil { if relErr == nil { relErr = newReleaseFailedError(release, err) } else { @@ -1548,12 +1548,7 @@ func (st *HelmState) TemplateReleases(helm helmexec.Interface, outputDir string, } } - var firstErr error = nil - if len(errs) > 0 { - firstErr = errs[0] - } - - if _, err := st.TriggerCleanupEvent(release, firstErr, "template"); err != nil { + if _, err := st.TriggerCleanupEvent(release, "template"); err != nil { st.logger.Warnf("warn: %v\n", err) } } @@ -1654,7 +1649,7 @@ func (st *HelmState) WriteReleasesValues(helm helmexec.Interface, additionalValu return []error{fmt.Errorf("writing values file %s: %w", outputValuesFile, err)} } - if _, err := st.TriggerCleanupEvent(release, nil, "write-values"); err != nil { + if _, err := st.TriggerCleanupEvent(release, "write-values"); err != nil { st.logger.Warnf("warn: %v\n", err) } } @@ -1729,12 +1724,7 @@ func (st *HelmState) LintReleases(helm helmexec.Interface, additionalValues []st } } - var firstErr error = nil - if len(errs) > 0 { - firstErr = errs[0] - } - - if _, err := st.TriggerCleanupEvent(&release, firstErr, "lint"); err != nil { + if _, err := st.TriggerCleanupEvent(&release, "lint"); err != nil { st.logger.Warnf("warn: %v\n", err) } } @@ -2081,7 +2071,7 @@ func (st *HelmState) DiffReleases(helm helmexec.Interface, additionalValues []st } if triggerCleanupEvents { - if _, err := st.TriggerCleanupEvent(prep.release, nil, "diff"); err != nil { + if _, err := st.TriggerCleanupEvent(prep.release, "diff"); err != nil { st.logger.Warnf("warn: %v\n", err) } } @@ -2435,8 +2425,8 @@ func (st *HelmState) triggerPrepareEvent(r *ReleaseSpec, helmfileCommand string) return st.triggerReleaseEvent("prepare", nil, r, helmfileCommand) } -func (st *HelmState) TriggerCleanupEvent(r *ReleaseSpec, evtErr error, helmfileCommand string) (bool, error) { - return st.triggerReleaseEvent("cleanup", evtErr, r, helmfileCommand) +func (st *HelmState) TriggerCleanupEvent(r *ReleaseSpec, helmfileCommand string) (bool, error) { + return st.triggerReleaseEvent("cleanup", nil, r, helmfileCommand) } func (st *HelmState) triggerPresyncEvent(r *ReleaseSpec, helmfileCommand string) (bool, error) { From 47b41fbaf69d11ae7f10282a23e3a8b9b55e6b28 Mon Sep 17 00:00:00 2001 From: Felipe Santos Date: Sun, 29 Sep 2024 12:42:43 -0300 Subject: [PATCH 3/3] fix: cleanup hooks not receiving error signal Closes #1041 Signed-off-by: Felipe Santos --- pkg/app/app.go | 53 +++++++++++++++++++++++++++++----------------- pkg/app/run.go | 10 ++++++--- pkg/state/state.go | 4 ++-- 3 files changed, 43 insertions(+), 24 deletions(-) diff --git a/pkg/app/app.go b/pkg/app/app.go index eaab62b7..e7356abb 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -146,8 +146,9 @@ func (a *App) DeprecatedSyncCharts(c DeprecatedChartsConfigProvider) error { SkipRepos: true, SkipDeps: true, Concurrency: 2, - }, func() { + }, func() []error { errs = run.DeprecatedSyncCharts(c) + return errs }) if err != nil { @@ -181,8 +182,9 @@ func (a *App) Diff(c DiffConfigProvider) error { Validate: c.Validate(), Concurrency: c.Concurrency(), IncludeTransitiveNeeds: c.IncludeNeeds(), - }, func() { + }, func() []error { msg, matched, affected, errs = a.diff(run, c) + return errs }) if msg != nil { @@ -249,8 +251,9 @@ func (a *App) Template(c TemplateConfigProvider) error { IncludeTransitiveNeeds: c.IncludeNeeds(), Set: c.Set(), KubeVersion: c.KubeVersion(), - }, func() { + }, func() []error { ok, errs = a.template(run, c) + return errs }) if prepErr != nil { @@ -268,8 +271,9 @@ func (a *App) WriteValues(c WriteValuesConfigProvider) error { SkipDeps: c.SkipDeps(), SkipCleanup: c.SkipCleanup(), Concurrency: c.Concurrency(), - }, func() { + }, func() []error { ok, errs = a.writeValues(run, c) + return errs }) if prepErr != nil { @@ -320,8 +324,9 @@ func (a *App) Lint(c LintConfigProvider) error { SkipCleanup: c.SkipCleanup(), Concurrency: c.Concurrency(), IncludeTransitiveNeeds: c.IncludeNeeds(), - }, func() { + }, func() []error { ok, lintErrs, errs = a.lint(run, c) + return errs }) if prepErr != nil { @@ -355,7 +360,8 @@ func (a *App) Fetch(c FetchConfigProvider) error { OutputDir: c.OutputDir(), OutputDirTemplate: c.OutputDirTemplate(), Concurrency: c.Concurrency(), - }, func() { + }, func() []error { + return nil }) if prepErr != nil { @@ -379,8 +385,9 @@ func (a *App) Sync(c SyncConfigProvider) error { IncludeTransitiveNeeds: c.IncludeNeeds(), Validate: c.Validate(), Concurrency: c.Concurrency(), - }, func() { + }, func() []error { ok, errs = a.sync(run, c) + return errs }) if prepErr != nil { @@ -413,7 +420,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { Validate: c.Validate(), Concurrency: c.Concurrency(), IncludeTransitiveNeeds: c.IncludeNeeds(), - }, func() { + }, func() []error { matched, updated, es := a.apply(run, c) mut.Lock() @@ -421,6 +428,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { mut.Unlock() ok, errs = matched, es + return errs }) if prepErr != nil { @@ -449,8 +457,9 @@ func (a *App) Status(c StatusesConfigProvider) error { SkipRepos: true, SkipDeps: true, Concurrency: c.Concurrency(), - }, func() { + }, func() []error { ok, errs = a.status(run, c) + return errs }) if err != nil { @@ -471,8 +480,9 @@ func (a *App) Delete(c DeleteConfigProvider) error { Concurrency: c.Concurrency(), DeleteWait: c.DeleteWait(), DeleteTimeout: c.DeleteTimeout(), - }, func() { + }, func() []error { ok, errs = a.delete(run, c.Purge(), c) + return errs }) if err != nil { @@ -494,8 +504,9 @@ func (a *App) Destroy(c DestroyConfigProvider) error { Concurrency: c.Concurrency(), DeleteWait: c.DeleteWait(), DeleteTimeout: c.DeleteTimeout(), - }, func() { + }, func() []error { ok, errs = a.delete(run, true, c) + return errs }) if err != nil { errs = append(errs, err) @@ -519,8 +530,9 @@ func (a *App) Test(c TestConfigProvider) error { SkipRepos: c.SkipDeps(), SkipDeps: c.SkipDeps(), Concurrency: c.Concurrency(), - }, func() { + }, func() []error { errs = a.test(run, c) + return errs }) if err != nil { @@ -538,11 +550,12 @@ func (a *App) PrintDAGState(c DAGConfigProvider) error { SkipRepos: true, SkipDeps: true, Concurrency: 2, - }, func() { + }, func() []error { err = a.dag(run) if err != nil { errs = append(errs, err) } + return errs }) return ok, errs }, false, SetFilter(true)) @@ -554,7 +567,7 @@ func (a *App) PrintState(c StateConfigProvider) error { SkipRepos: true, SkipDeps: true, Concurrency: 2, - }, func() { + }, func() []error { if c.EmbedValues() { for i := range run.state.Releases { r := run.state.Releases[i] @@ -562,7 +575,7 @@ func (a *App) PrintState(c StateConfigProvider) error { values, err := run.state.LoadYAMLForEmbedding(&r, r.Values, r.MissingFileHandler, r.ValuesPathPrefix) if err != nil { errs = []error{err} - return + return errs } run.state.Releases[i].Values = values @@ -570,7 +583,7 @@ func (a *App) PrintState(c StateConfigProvider) error { secrets, err := run.state.LoadYAMLForEmbedding(&r, r.Secrets, r.MissingFileHandler, r.ValuesPathPrefix) if err != nil { errs = []error{err} - return + return errs } run.state.Releases[i].Secrets = secrets @@ -580,17 +593,18 @@ func (a *App) PrintState(c StateConfigProvider) error { stateYaml, err := run.state.ToYaml() if err != nil { errs = []error{err} - return + return errs } sourceFile, err := run.state.FullFilePath() if err != nil { errs = []error{err} - return + return errs } fmt.Printf("---\n# Source: %s\n\n%+v", sourceFile, stateYaml) errs = []error{} + return errs }) if err != nil { @@ -626,12 +640,13 @@ func (a *App) ListReleases(c ListConfigProvider) error { SkipRepos: true, SkipDeps: true, Concurrency: 2, - }, func() { + }, func() []error { rel, err := a.list(run) if err != nil { panic(err) } stateReleases = rel + return nil }) } else { stateReleases, err = a.list(run) diff --git a/pkg/app/run.go b/pkg/app/run.go index e29fce2a..a9d77e08 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -55,7 +55,7 @@ func (r *Run) prepareChartsIfNeeded(helmfileCommand string, dir string, concurre return releaseToChart, nil } -func (r *Run) withPreparedCharts(helmfileCommand string, opts state.ChartPrepareOptions, f func()) error { +func (r *Run) withPreparedCharts(helmfileCommand string, opts state.ChartPrepareOptions, f func() []error) error { if r.ReleaseToChart != nil { panic("Run.PrepareCharts can be called only once") } @@ -111,9 +111,13 @@ func (r *Run) withPreparedCharts(helmfileCommand string, opts state.ChartPrepare r.ReleaseToChart = releaseToChart - f() + errs := f() + var firstErr error = nil + if len(errs) > 0 { + firstErr = errs[0] + } - _, err = r.state.TriggerGlobalCleanupEvent(helmfileCommand) + _, err = r.state.TriggerGlobalCleanupEvent(helmfileCommand, firstErr) return err } diff --git a/pkg/state/state.go b/pkg/state/state.go index 0fbff129..32e6fc9e 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -2400,8 +2400,8 @@ func (st *HelmState) TriggerGlobalPrepareEvent(helmfileCommand string) (bool, er return st.triggerGlobalReleaseEvent("prepare", nil, helmfileCommand) } -func (st *HelmState) TriggerGlobalCleanupEvent(helmfileCommand string) (bool, error) { - return st.triggerGlobalReleaseEvent("cleanup", nil, helmfileCommand) +func (st *HelmState) TriggerGlobalCleanupEvent(helmfileCommand string, evtErr error) (bool, error) { + return st.triggerGlobalReleaseEvent("cleanup", evtErr, helmfileCommand) } func (st *HelmState) triggerGlobalReleaseEvent(evt string, evtErr error, helmfileCmd string) (bool, error) {