diff --git a/cmd/root.go b/cmd/root.go index 7b6eee6a..b1b3a779 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -131,6 +131,7 @@ func setGlobalOptionsForRootCmd(fs *pflag.FlagSet, globalOptions *config.GlobalO fs.StringArrayVar(&globalOptions.StateValuesFile, "state-values-file", nil, "specify state values in a YAML file. Used to override .Values within the helmfile template (not values template).") fs.BoolVar(&globalOptions.SkipDeps, "skip-deps", false, `skip running "helm repo update" and "helm dependency build"`) fs.BoolVar(&globalOptions.SkipRefresh, "skip-refresh", false, `skip running "helm repo update"`) + fs.BoolVar(&globalOptions.AllowFailedReleases, "allow-failed-releases", false, `continue preparing charts for other releases when chart preparation fails for a release; report failures at the end`) fs.BoolVar(&globalOptions.StripArgsValuesOnExitError, "strip-args-values-on-exit-error", true, `Strip the potential secret values of the helm command args contained in a helmfile error message`) fs.BoolVar(&globalOptions.DisableForceUpdate, "disable-force-update", false, `do not force helm repos to update when executing "helm repo add" (Helm 3 only)`) fs.BoolVar(&globalOptions.EnforcePluginVerification, "enforce-plugin-verification", false, `fail plugin installation if verification is not supported (for security purposes)`) diff --git a/pkg/app/app.go b/pkg/app/app.go index 4e68cfa1..05844f61 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -173,6 +173,7 @@ func (a *App) Diff(c DiffConfigProvider) error { prepErr := run.WithPreparedCharts("diff", state.ChartPrepareOptions{ SkipRepos: c.SkipRefresh() || c.SkipDeps(), SkipRefresh: c.SkipRefresh(), + AllowFailedReleases: c.AllowFailedReleases(), SkipDeps: c.SkipDeps(), SkipSchemaValidation: c.SkipSchemaValidation(), IncludeCRDs: &includeCRDs, @@ -246,6 +247,7 @@ func (a *App) Template(c TemplateConfigProvider) error { prepErr := run.WithPreparedCharts("template", state.ChartPrepareOptions{ SkipRepos: c.SkipRefresh() || c.SkipDeps(), SkipRefresh: c.SkipRefresh(), + AllowFailedReleases: c.AllowFailedReleases(), SkipDeps: c.SkipDeps(), SkipSchemaValidation: c.SkipSchemaValidation(), IncludeCRDs: &includeCRDs, @@ -274,11 +276,12 @@ func (a *App) Template(c TemplateConfigProvider) error { func (a *App) WriteValues(c WriteValuesConfigProvider) error { return a.ForEachState(func(run *Run) (ok bool, errs []error) { prepErr := run.WithPreparedCharts("write-values", state.ChartPrepareOptions{ - SkipRepos: c.SkipRefresh() || c.SkipDeps(), - SkipRefresh: c.SkipRefresh(), - SkipDeps: c.SkipDeps(), - SkipCleanup: c.SkipCleanup(), - Concurrency: c.Concurrency(), + SkipRepos: c.SkipRefresh() || c.SkipDeps(), + SkipRefresh: c.SkipRefresh(), + AllowFailedReleases: c.AllowFailedReleases(), + SkipDeps: c.SkipDeps(), + SkipCleanup: c.SkipCleanup(), + Concurrency: c.Concurrency(), }, func() []error { ok, errs = a.writeValues(run, c) return errs @@ -329,6 +332,7 @@ func (a *App) Lint(c LintConfigProvider) error { ForceDownload: true, SkipRepos: c.SkipRefresh() || c.SkipDeps(), SkipRefresh: c.SkipRefresh(), + AllowFailedReleases: c.AllowFailedReleases(), SkipDeps: c.SkipDeps(), SkipCleanup: c.SkipCleanup(), Concurrency: c.Concurrency(), @@ -447,13 +451,14 @@ func (a *App) Fetch(c FetchConfigProvider) error { } prepErr := run.WithPreparedCharts("pull", state.ChartPrepareOptions{ - ForceDownload: true, - SkipRefresh: c.SkipRefresh(), - SkipRepos: c.SkipRefresh() || c.SkipDeps(), - SkipDeps: c.SkipDeps(), - OutputDir: c.OutputDir(), - OutputDirTemplate: c.OutputDirTemplate(), - Concurrency: c.Concurrency(), + ForceDownload: true, + SkipRefresh: c.SkipRefresh(), + AllowFailedReleases: c.AllowFailedReleases(), + SkipRepos: c.SkipRefresh() || c.SkipDeps(), + SkipDeps: c.SkipDeps(), + OutputDir: c.OutputDir(), + OutputDirTemplate: c.OutputDirTemplate(), + Concurrency: c.Concurrency(), }, func() []error { if c.WriteOutput() { for i := range run.state.Releases { @@ -504,6 +509,7 @@ func (a *App) Sync(c SyncConfigProvider) error { prepErr := run.WithPreparedCharts("sync", state.ChartPrepareOptions{ SkipRepos: c.SkipRefresh() || c.SkipDeps(), SkipRefresh: c.SkipRefresh(), + AllowFailedReleases: c.AllowFailedReleases(), SkipDeps: c.SkipDeps(), SkipSchemaValidation: c.SkipSchemaValidation(), Wait: c.Wait(), @@ -562,6 +568,7 @@ func (a *App) Apply(c ApplyConfigProvider) error { prepErr := run.WithPreparedCharts("apply", state.ChartPrepareOptions{ SkipRepos: c.SkipRefresh() || c.SkipDeps(), SkipRefresh: c.SkipRefresh(), + AllowFailedReleases: c.AllowFailedReleases(), SkipDeps: c.SkipDeps(), SkipSchemaValidation: c.SkipSchemaValidation(), Wait: c.Wait(), @@ -628,12 +635,13 @@ func (a *App) Destroy(c DestroyConfigProvider) error { return a.ForEachState(func(run *Run) (ok bool, errs []error) { if !c.SkipCharts() { err := run.WithPreparedCharts("destroy", state.ChartPrepareOptions{ - SkipRepos: c.SkipRefresh() || c.SkipDeps(), - SkipRefresh: c.SkipRefresh(), - SkipDeps: c.SkipDeps(), - Concurrency: c.Concurrency(), - DeleteWait: c.DeleteWait(), - DeleteTimeout: c.DeleteTimeout(), + SkipRepos: c.SkipRefresh() || c.SkipDeps(), + SkipRefresh: c.SkipRefresh(), + AllowFailedReleases: c.AllowFailedReleases(), + SkipDeps: c.SkipDeps(), + Concurrency: c.Concurrency(), + DeleteWait: c.DeleteWait(), + DeleteTimeout: c.DeleteTimeout(), }, func() []error { ok, errs = a.delete(run, true, c) return errs @@ -657,10 +665,11 @@ func (a *App) Test(c TestConfigProvider) error { } err := run.WithPreparedCharts("test", state.ChartPrepareOptions{ - SkipRepos: c.SkipRefresh() || c.SkipDeps(), - SkipRefresh: c.SkipRefresh(), - SkipDeps: c.SkipDeps(), - Concurrency: c.Concurrency(), + SkipRepos: c.SkipRefresh() || c.SkipDeps(), + SkipRefresh: c.SkipRefresh(), + AllowFailedReleases: c.AllowFailedReleases(), + SkipDeps: c.SkipDeps(), + Concurrency: c.Concurrency(), }, func() []error { errs = a.test(run, c) return errs @@ -678,9 +687,10 @@ func (a *App) PrintDAGState(c DAGConfigProvider) error { var err error return a.ForEachState(func(run *Run) (ok bool, errs []error) { err = run.WithPreparedCharts("show-dag", state.ChartPrepareOptions{ - SkipRepos: true, - SkipDeps: true, - Concurrency: 2, + SkipRepos: true, + AllowFailedReleases: false, + SkipDeps: true, + Concurrency: 2, }, func() []error { err = a.dag(run) if err != nil { @@ -695,9 +705,10 @@ func (a *App) PrintDAGState(c DAGConfigProvider) error { func (a *App) PrintState(c StateConfigProvider) error { return a.ForEachState(func(run *Run) (_ bool, errs []error) { err := run.WithPreparedCharts("build", state.ChartPrepareOptions{ - SkipRepos: true, - SkipDeps: true, - Concurrency: 2, + SkipRepos: true, + AllowFailedReleases: false, + SkipDeps: true, + Concurrency: 2, }, func() []error { if c.EmbedValues() { for i := range run.state.Releases { @@ -768,9 +779,10 @@ func (a *App) ListReleases(c ListConfigProvider) error { if !c.SkipCharts() { prepErr := run.WithPreparedCharts("list", state.ChartPrepareOptions{ - SkipRepos: true, - SkipDeps: true, - Concurrency: 2, + SkipRepos: true, + AllowFailedReleases: true, + SkipDeps: true, + Concurrency: 2, }, func() []error { rel, err := a.list(run) if err != nil { diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 19233d9d..78f0cda8 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -2369,6 +2369,7 @@ type configImpl struct { skipTests bool skipSchemaValidation bool skipRefresh bool + allowFailedReleases bool skipNeeds bool includeNeeds bool @@ -2416,6 +2417,10 @@ func (c configImpl) SkipRefresh() bool { return c.skipRefresh } +func (c configImpl) AllowFailedReleases() bool { + return c.allowFailedReleases +} + func (c configImpl) SkipNeeds() bool { return c.skipNeeds } @@ -2508,6 +2513,7 @@ type applyConfig struct { skipCRDs bool skipDeps bool skipRefresh bool + allowFailedReleases bool skipNeeds bool includeNeeds bool includeTransitiveNeeds bool @@ -2609,6 +2615,10 @@ func (a applyConfig) SkipRefresh() bool { return a.skipRefresh } +func (a applyConfig) AllowFailedReleases() bool { + return a.allowFailedReleases +} + func (a applyConfig) SkipNeeds() bool { return a.skipNeeds } diff --git a/pkg/app/config.go b/pkg/app/config.go index 02e4996c..7faa68d7 100644 --- a/pkg/app/config.go +++ b/pkg/app/config.go @@ -18,6 +18,7 @@ type ConfigProvider interface { RepoRetry() int SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool SequentialHelmfiles() bool FileOrDir() string @@ -62,6 +63,7 @@ type ApplyConfigProvider interface { SkipCRDs() bool SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool Wait() bool WaitRetries() int WaitForJobs() bool @@ -126,6 +128,7 @@ type SyncConfigProvider interface { SkipCRDs() bool SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool Wait() bool WaitRetries() int WaitForJobs() bool @@ -174,6 +177,7 @@ type DiffConfigProvider interface { SkipCRDs() bool SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool IncludeTests() bool @@ -231,6 +235,7 @@ type DestroyConfigProvider interface { SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool SkipCharts() bool DeleteWait() bool DeleteTimeout() int @@ -246,6 +251,7 @@ type TestConfigProvider interface { SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool Timeout() int Cleanup() bool Logs() bool @@ -260,6 +266,7 @@ type LintConfigProvider interface { Set() []string SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool SkipCleanup() bool DAGConfig @@ -277,6 +284,7 @@ type UnittestConfigProvider interface { DebugPlugin() bool SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool SkipCleanup() bool DAGConfig @@ -287,6 +295,7 @@ type UnittestConfigProvider interface { type FetchConfigProvider interface { SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool OutputDir() string OutputDirTemplate() string WriteOutput() bool @@ -306,6 +315,7 @@ type TemplateConfigProvider interface { Validate() bool SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool SkipCleanup() bool SkipTests() bool OutputDir() string @@ -333,6 +343,7 @@ type WriteValuesConfigProvider interface { OutputFileTemplate() string SkipDeps() bool SkipRefresh() bool + AllowFailedReleases() bool SkipCleanup() bool IncludeTransitiveNeeds() bool diff --git a/pkg/app/destroy_test.go b/pkg/app/destroy_test.go index 1a136230..cde27941 100644 --- a/pkg/app/destroy_test.go +++ b/pkg/app/destroy_test.go @@ -39,6 +39,7 @@ type destroyConfig struct { interactive bool skipDeps bool skipRefresh bool + allowFailedReleases bool logger *zap.SugaredLogger includeTransitiveNeeds bool skipCharts bool @@ -78,6 +79,10 @@ func (d destroyConfig) SkipRefresh() bool { return d.skipRefresh } +func (d destroyConfig) AllowFailedReleases() bool { + return d.allowFailedReleases +} + func (d destroyConfig) IncludeTransitiveNeeds() bool { return d.includeTransitiveNeeds } diff --git a/pkg/app/diff_test.go b/pkg/app/diff_test.go index 2b6a9204..81c2d364 100644 --- a/pkg/app/diff_test.go +++ b/pkg/app/diff_test.go @@ -25,6 +25,7 @@ type diffConfig struct { skipCRDs bool skipDeps bool skipRefresh bool + allowFailedReleases bool includeTests bool skipNeeds bool includeNeeds bool @@ -87,6 +88,10 @@ func (a diffConfig) SkipRefresh() bool { return a.skipRefresh } +func (a diffConfig) AllowFailedReleases() bool { + return a.allowFailedReleases +} + func (a diffConfig) IncludeTests() bool { return a.includeTests } diff --git a/pkg/app/run.go b/pkg/app/run.go index 323380a7..93a40c66 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -56,7 +56,12 @@ func (r *Run) prepareChartsIfNeeded(helmfileCommand string, dir string, concurre releaseToChart, errs := r.state.PrepareCharts(r.helm, dir, concurrency, helmfileCommand, opts) if len(errs) > 0 { - return nil, fmt.Errorf("%v", errs) + if !opts.AllowFailedReleases { + // abort on first error + return nil, fmt.Errorf("%v", errs) + } + // return partial results with errors for the failed ones + return releaseToChart, &MultiError{Errors: errs} } return releaseToChart, nil @@ -113,9 +118,16 @@ func (r *Run) WithPreparedCharts(helmfileCommand string, opts state.ChartPrepare // remain available for the entire operation lifecycle. See issue #1799. defer r.state.CleanupChartifyTempDirs() - releaseToChart, err := r.prepareChartsIfNeeded(helmfileCommand, dir, opts.Concurrency, opts) - if err != nil { - return err + releaseToChart, prepareErr := r.prepareChartsIfNeeded(helmfileCommand, dir, opts.Concurrency, opts) + // IMPORTANT: on opts.AllowFailedReleases: do not abort only on error here, just forward it to the caller in order to allow for partial results + // Only in case prepareCharts failed with a general error and returned to processable release abort anyways + if prepareErr != nil && releaseToChart == nil { + return prepareErr + } + if !opts.AllowFailedReleases { + if prepareErr != nil { + return prepareErr + } } for i := range r.state.Releases { @@ -125,7 +137,7 @@ func (r *Run) WithPreparedCharts(helmfileCommand string, opts state.ChartPrepare Namespace: rel.Namespace, KubeContext: rel.KubeContext, } - if chart := releaseToChart[key]; chart != rel.Chart { + if chart, ok := releaseToChart[key]; ok && chart != rel.Chart { // The chart has been downloaded and modified by Helmfile (and chartify under the hood). // We let the later step use the modified version of the chart, located under the `chart` variable, // instead of the original chart path. @@ -146,8 +158,28 @@ func (r *Run) WithPreparedCharts(helmfileCommand string, opts state.ChartPrepare } } - _, err = r.state.TriggerGlobalCleanupEvent(helmfileCommand, firstErr) - return err + _, cleanupErr := r.state.TriggerGlobalCleanupEvent(helmfileCommand, firstErr) + if !opts.AllowFailedReleases { + // return directly on first error + return cleanupErr + } else { + // merge the two errors into a single error output + var merged []error + if prepareErr != nil { + if me, ok := prepareErr.(*MultiError); ok { + merged = append(merged, me.Errors...) + } else { + merged = append(merged, prepareErr) + } + } + if cleanupErr != nil { + merged = append(merged, fmt.Errorf("error during global cleanup event: %w", cleanupErr)) + } + if len(merged) > 0 { + return &MultiError{Errors: merged} + } + return nil + } } func (r *Run) Deps(c DepsConfigProvider) []error { diff --git a/pkg/config/global.go b/pkg/config/global.go index 6391b863..ce18e4b7 100644 --- a/pkg/config/global.go +++ b/pkg/config/global.go @@ -35,6 +35,8 @@ type GlobalOptions struct { SkipDeps bool // SkipRefresh is true if the running "helm repo update" should be skipped SkipRefresh bool + // AllowFailedReleases is true if partial errors during release processing are allowed + AllowFailedReleases bool // StripArgsValuesOnExitError is true if the ARGS output on exit error should be suppressed StripArgsValuesOnExitError bool // DisableForceUpdate is true if force updating repos is not desirable when executing "helm repo add" (Helm 3) @@ -264,6 +266,11 @@ func (g *GlobalImpl) SkipRefresh() bool { return g.GlobalOptions.SkipRefresh } +// AllowFailedReleases returns true if partial errors during release processing are allowed +func (g *GlobalImpl) AllowFailedReleases() bool { + return g.GlobalOptions.AllowFailedReleases +} + // StripArgsValuesOnExitError return if the ARGS output on exit error should be suppressed func (g *GlobalImpl) StripArgsValuesOnExitError() bool { return g.GlobalOptions.StripArgsValuesOnExitError diff --git a/pkg/state/issue_2616_test.go b/pkg/state/issue_2616_test.go new file mode 100644 index 00000000..8cd864d2 --- /dev/null +++ b/pkg/state/issue_2616_test.go @@ -0,0 +1,114 @@ +package state + +import ( + "sync" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/helmfile/helmfile/pkg/exectest" +) + +// TestAllowFailedReleasesFlag_Enabled verifies that the AllowFailedReleases flag +// works as intended with one release that should work and another release, that +// will fail. +// The test makes one release fail by having an oci with latest version, which will always error +func TestAllowFailedReleasesFlag_Enabled(t *testing.T) { + resetChartCacheForTest() + helmfileContent := []byte(` +repositories: +- name: stable + url: kubernetes-charts.storage.googleapis.com + oci: true + +releases: + - name: grafana + namespace: grafana + chart: stable/grafana + - name: error + namespace: failed + chart: oci://example.com/chart/error + version: latest +`) + + logger := zap.NewExample().Sugar() + + st, err := createFromYaml(helmfileContent, "example/path/to/helmfile.yaml", DefaultEnv, logger) + require.NoError(t, err) + + tempDir := t.TempDir() + + helm := &exectest.Helm{ + ChartsMutex: &sync.Mutex{}, + } + + // OutputDirTemplate includes .OutputDir so charts go into tempDir (auto-cleaned + // by t.TempDir), not the global cache or CWD. + opts := ChartPrepareOptions{ + Concurrency: 1, + OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", + AllowFailedReleases: true, + } + + releaseToChart, errs := st.PrepareCharts(helm, tempDir, 1, "apply", opts) + require.NotEmpty(t, errs, "PrepareCharts should return an error") + + // Verify only the error chart is not prepared + assert.Contains(t, releaseToChart, PrepareChartKey{Name: "grafana", Namespace: "grafana"}, + "grafana chart should be prepared") + assert.NotContains(t, releaseToChart, PrepareChartKey{Name: "error", Namespace: "failed"}, + "error chart should not be prepared") +} + +// TestAllowFailedReleasesFlag_Disabled verifies that the AllowFailedReleases flag +// has no unintended side effects and the default abort on error still works. +// The test makes one release fail by having an oci with latest version, which will always error +func TestAllowFailedReleasesFlag_Disabled(t *testing.T) { + resetChartCacheForTest() + cleanupLeakedChartDirs(t, "grafana", "error") + helmfileContent := []byte(` +repositories: +- name: stable + url: kubernetes-charts.storage.googleapis.com + oci: true + +releases: + - name: grafana + namespace: grafana + chart: stable/grafana + - name: error + namespace: failed + chart: oci://example.com/chart/error + version: latest +`) + + logger := zap.NewExample().Sugar() + + st, err := createFromYaml(helmfileContent, "example/path/to/helmfile.yaml", DefaultEnv, logger) + require.NoError(t, err) + + tempDir := t.TempDir() + + helm := &exectest.Helm{ + ChartsMutex: &sync.Mutex{}, + } + + // OutputDirTemplate includes .OutputDir so charts go into tempDir (auto-cleaned + // by t.TempDir), not the global cache or CWD. + opts := ChartPrepareOptions{ + Concurrency: 1, + OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", + // PrefetchSharedRemoteCharts: true, + } + + releaseToChart, errs := st.PrepareCharts(helm, tempDir, 1, "apply", opts) + require.NotEmpty(t, errs, "PrepareCharts should return an error") + + // Verify both releases are not prepared + assert.NotContains(t, releaseToChart, PrepareChartKey{Name: "grafana", Namespace: "grafana"}, + "grafana chart should not be prepared") + assert.NotContains(t, releaseToChart, PrepareChartKey{Name: "error", Namespace: "failed"}, + "error chart should not be prepared") +} diff --git a/pkg/state/state.go b/pkg/state/state.go index 1ca72ecf..88854458 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -1534,6 +1534,7 @@ type ChartPrepareOptions struct { SkipRepos bool SkipDeps bool SkipRefresh bool + AllowFailedReleases bool SkipResolve bool SkipCleanup bool // SkipSchemaValidation configures chartify to pass --skip-schema-validation to helm-template run by it. @@ -2341,7 +2342,13 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre if downloadRes.err != nil { errs = append(errs, downloadRes.err) - return + + if !opts.AllowFailedReleases { + return + } else { + // continue processing other releases even if one fails + continue + } } func() { prepareChartInfoMutex.Lock() @@ -2362,16 +2369,22 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre ) if len(errs) > 0 { - return nil, errs + if !opts.AllowFailedReleases { + return nil, errs + } + // else if AllowFailedReleases: return partial results along with errs. } if len(builds) > 0 { if err := st.runHelmDepBuilds(helm, concurrency, builds, opts); err != nil { - return nil, []error{err} + if !opts.AllowFailedReleases { + return nil, []error{err} + } + errs = append(errs, err) } } - return prepareChartInfo, nil + return prepareChartInfo, errs } // nolint: unparam