diff --git a/CHANGELOG.md b/CHANGELOG.md index 5869b1b5..ea93d000 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Added - Add support for `conditionTemplate` and allow `condition` to be set directly to `true` or `false`. +- Add `--allow-failed-releases` global flag to continue preparing charts for the remaining releases when chart preparation fails for a release; failed releases are skipped and all failures are reported at the end (#2616) ## [1.4.1] - 2026-03-03 diff --git a/cmd/root.go b/cmd/root.go index 7b6eee6a..fede132a 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; failed releases are skipped and all failures are reported 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/docs/cli.md b/docs/cli.md index 88bed36e..90f40015 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -550,6 +550,7 @@ The following global flags are also available but not shown in the main help out | Flag | Default | Description | |------|---------|-------------| | `--kubeconfig` | `""` | Use a particular kubeconfig file | +| `--allow-failed-releases` | false | Continue preparing charts for other releases when chart preparation fails for a release; failed releases are skipped and all failures are reported at the end | | `--skip-refresh` | false | Skip running `helm repo update` (lighter than `--skip-deps` which also skips dependency build) | | `--enforce-plugin-verification` | false | Fail plugin installation if verification is not supported | | `--oci-plain-http` | false | Use plain HTTP for OCI registries (required for local/insecure registries in Helm 4) | diff --git a/pkg/app/app.go b/pkg/app/app.go index 4e68cfa1..2d89d2f2 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,6 +276,8 @@ 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{ + // Note: "write-values" never prepares charts (see commandsSkipChartPrep + // in run.go), so AllowFailedReleases does not apply here. SkipRepos: c.SkipRefresh() || c.SkipDeps(), SkipRefresh: c.SkipRefresh(), SkipDeps: c.SkipDeps(), @@ -329,6 +333,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(), @@ -371,6 +376,7 @@ func (a *App) Unittest(c UnittestConfigProvider) 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 +453,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 +511,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 +570,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(), @@ -608,9 +617,10 @@ func (a *App) Apply(c ApplyConfigProvider) error { func (a *App) Status(c StatusesConfigProvider) error { return a.ForEachState(func(run *Run) (ok bool, errs []error) { err := run.WithPreparedCharts("status", state.ChartPrepareOptions{ - SkipRepos: true, - SkipDeps: true, - Concurrency: c.Concurrency(), + SkipRepos: true, + AllowFailedReleases: c.AllowFailedReleases(), + SkipDeps: true, + Concurrency: c.Concurrency(), }, func() []error { ok, errs = a.status(run, c) return errs @@ -628,12 +638,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 +668,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 @@ -768,6 +780,8 @@ func (a *App) ListReleases(c ListConfigProvider) error { if !c.SkipCharts() { prepErr := run.WithPreparedCharts("list", state.ChartPrepareOptions{ + // Note: "list" never prepares charts (see commandsSkipChartPrep in + // run.go), so AllowFailedReleases does not apply here. SkipRepos: true, SkipDeps: true, Concurrency: 2, 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..56238005 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 @@ -341,6 +351,7 @@ type WriteValuesConfigProvider interface { type StatusesConfigProvider interface { Args() string + AllowFailedReleases() bool concurrencyConfig } 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/issue_2616_test.go b/pkg/app/issue_2616_test.go new file mode 100644 index 00000000..ec3466f7 --- /dev/null +++ b/pkg/app/issue_2616_test.go @@ -0,0 +1,109 @@ +package app + +import ( + "path/filepath" + "sync" + "testing" + + "github.com/helmfile/vals" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/zap" + + "github.com/helmfile/helmfile/pkg/exectest" + ffs "github.com/helmfile/helmfile/pkg/filesystem" + "github.com/helmfile/helmfile/pkg/helmexec" +) + +// issue2616HelmfileContent defines two releases: one that is templated fine +// (logging) and one that always fails chart preparation (error), because OCI +// charts do not support the "latest" version tag. +const issue2616HelmfileContent = ` +releases: +- name: logging + chart: incubator/raw + namespace: kube-system + +- name: error + chart: oci://example.com/chart/error + namespace: failed + version: latest +` + +// TestTemplateAllowFailedReleases verifies the behavior of the +// --allow-failed-releases flag end to end for `helmfile template`: +// when chart preparation fails for one release, the remaining releases are +// still templated, the failed release is skipped (i.e. never executed against +// its un-prepared chart reference) and all failures are reported at the end. +func TestTemplateAllowFailedReleases(t *testing.T) { + testcases := []struct { + name string + allowFailedRelease bool + + // wantTemplated contains the releases that must have been passed to + // `helm template`; the release failing chart preparation must never + // appear here. + wantTemplated []exectest.Release + }{ + { + name: "default: abort on chart preparation failure", + allowFailedRelease: false, + wantTemplated: nil, + }, + { + name: "allow-failed-releases: skip failed release, template the rest", + allowFailedRelease: true, + wantTemplated: []exectest.Release{{Name: "logging", Flags: []string{"--kube-context", "default", "--namespace", "kube-system"}}}, + }, + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + var helm = &exectest.Helm{ + FailOnUnexpectedList: true, + FailOnUnexpectedDiff: true, + DiffMutex: &sync.Mutex{}, + ChartsMutex: &sync.Mutex{}, + ReleasesMutex: &sync.Mutex{}, + } + + _ = runWithLogCapture(t, "debug", func(t *testing.T, logger *zap.SugaredLogger) { + t.Helper() + + valsRuntime, err := vals.New(vals.Options{CacheSize: 32}) + require.NoError(t, err) + + files := map[string]string{ + "/path/to/helmfile.yaml": issue2616HelmfileContent, + } + + app := appWithFs(&App{ + OverrideHelmBinary: DefaultHelmBinary, + fs: &ffs.FileSystem{Glob: filepath.Glob}, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + helms: map[helmKey]helmexec.Interface{ + createHelmKey("helm", "default"): helm, + }, + valsRuntime: valsRuntime, + }, files) + + tmplErr := app.Template(applyConfig{ + // if we check log output, concurrency must be 1. otherwise the test becomes non-deterministic. + concurrency: 1, + logger: logger, + allowFailedReleases: tc.allowFailedRelease, + }) + + // The chart preparation failure must be reported in both modes. + require.Error(t, tmplErr) + assert.Contains(t, tmplErr.Error(), "the version for OCI charts should be semver compliant") + + // The release that failed chart preparation must never be executed. + require.Equal(t, tc.wantTemplated, helm.Templated) + }) + }) + } +} diff --git a/pkg/app/run.go b/pkg/app/run.go index 323380a7..40eea495 100644 --- a/pkg/app/run.go +++ b/pkg/app/run.go @@ -1,6 +1,7 @@ package app import ( + "errors" "fmt" "os" "slices" @@ -48,18 +49,23 @@ func (r *Run) askForConfirmation(msg string) bool { // When skipRepos is false, SyncReposOnce still runs normally for all repos. var commandsSkipChartPrep = []string{"write-values", "list"} -func (r *Run) prepareChartsIfNeeded(helmfileCommand string, dir string, concurrency int, opts state.ChartPrepareOptions) (map[state.PrepareChartKey]string, error) { +func (r *Run) prepareChartsIfNeeded(helmfileCommand string, dir string, concurrency int, opts state.ChartPrepareOptions) (map[state.PrepareChartKey]string, map[state.PrepareChartKey]error, error) { // Skip chart preparation for commands that don't need chart pulls if slices.Contains(commandsSkipChartPrep, strings.ToLower(helmfileCommand)) { - return nil, nil + return nil, nil, nil } - releaseToChart, errs := r.state.PrepareCharts(r.helm, dir, concurrency, helmfileCommand, opts) + releaseToChart, failedReleases, 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, nil, fmt.Errorf("%v", errs) + } + // return partial results, along with the per-release errors for the failed ones + return releaseToChart, failedReleases, &MultiError{Errors: errs} } - return releaseToChart, nil + return releaseToChart, failedReleases, nil } func (r *Run) WithPreparedCharts(helmfileCommand string, opts state.ChartPrepareOptions, f func() []error) error { @@ -113,11 +119,25 @@ 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, failedReleases, prepareErr := r.prepareChartsIfNeeded(helmfileCommand, dir, opts.Concurrency, opts) + // A prepare error with no per-release attribution (state parsing, dependency + // resolution, ...) is a general failure: always abort, even with + // opts.AllowFailedReleases, since there are no processable partial results. + // Otherwise, abort only when partial results are not allowed; the failed + // releases are skipped below and their errors are reported at the end. + if prepareErr != nil && (len(failedReleases) == 0 || !opts.AllowFailedReleases) { + return prepareErr } + // Releases whose chart preparation failed are removed from the state, so that + // the operation below never executes them against their original, un-prepared + // chart reference. That would either fail again with a duplicate error or, + // worse, bypass chartify modifications (patches, dependencies) and produce an + // unintended result. Their preparation errors are reported via prepareErr. + releases := r.state.Releases + if len(failedReleases) > 0 { + releases = make([]state.ReleaseSpec, 0, len(r.state.Releases)) + } for i := range r.state.Releases { rel := &r.state.Releases[i] key := state.PrepareChartKey{ @@ -125,7 +145,10 @@ func (r *Run) WithPreparedCharts(helmfileCommand string, opts state.ChartPrepare Namespace: rel.Namespace, KubeContext: rel.KubeContext, } - if chart := releaseToChart[key]; chart != rel.Chart { + if _, failed := failedReleases[key]; failed { + continue + } + 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. @@ -133,7 +156,11 @@ func (r *Run) WithPreparedCharts(helmfileCommand string, opts state.ChartPrepare // if it has been modified or not. rel.ChartPath = chart } + if len(failedReleases) > 0 { + releases = append(releases, *rel) + } } + r.state.Releases = releases r.ReleaseToChart = releaseToChart @@ -146,8 +173,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 cleanupErr + } + + // merge the preparation and cleanup errors into a single error output + var merged []error + if prepareErr != nil { + var me *MultiError + if errors.As(prepareErr, &me) { + 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/issue923_test.go b/pkg/state/issue923_test.go index b8995777..92df0981 100644 --- a/pkg/state/issue923_test.go +++ b/pkg/state/issue923_test.go @@ -86,7 +86,7 @@ releases: OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", } - releaseToChart, errs := st.PrepareCharts(helm, tempDir, 1, "apply", opts) + releaseToChart, _, errs := st.PrepareCharts(helm, tempDir, 1, "apply", opts) require.Empty(t, errs, "PrepareCharts should not return errors") // Verify both releases have prepared charts @@ -152,7 +152,7 @@ releases: OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", } - releaseToChart, errs := st.PrepareCharts(helm, tempDir, 1, "apply", opts) + releaseToChart, _, errs := st.PrepareCharts(helm, tempDir, 1, "apply", opts) require.Empty(t, errs) // argocd-secrets should NOT have a prepared chart diff --git a/pkg/state/issue_2616_test.go b/pkg/state/issue_2616_test.go new file mode 100644 index 00000000..2420a118 --- /dev/null +++ b/pkg/state/issue_2616_test.go @@ -0,0 +1,98 @@ +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" +) + +// issue2616HelmfileContent defines two releases: one that prepares fine +// (grafana) and one that always fails chart preparation (error), because OCI +// charts do not support the "latest" version tag. +var issue2616HelmfileContent = []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 +`) + +// issue2616PrepareCharts runs PrepareCharts for issue2616HelmfileContent. +func issue2616PrepareCharts(t *testing.T, opts ChartPrepareOptions) (map[PrepareChartKey]string, map[PrepareChartKey]error, []error) { + t.Helper() + + resetChartCacheForTest() + + logger := zap.NewExample().Sugar() + + st, err := createFromYaml(issue2616HelmfileContent, "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.OutputDirTemplate = "{{ .OutputDir }}/{{ .Release.Name }}" + + releaseToChart, failedReleases, errs := st.PrepareCharts(helm, tempDir, 1, "apply", opts) + return releaseToChart, failedReleases, errs +} + +// TestAllowFailedReleasesFlag_Enabled verifies that the AllowFailedReleases flag +// works as intended with one release that should work and another release, that +// will fail: the healthy release is prepared and reported in the partial +// results, while the failing one is recorded in failedReleases so that callers +// can skip it during execution. +func TestAllowFailedReleasesFlag_Enabled(t *testing.T) { + releaseToChart, failedReleases, errs := issue2616PrepareCharts(t, ChartPrepareOptions{ + Concurrency: 1, + AllowFailedReleases: true, + }) + + 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") + + // Verify the failed release is attributed per-release, so that the caller + // can skip it instead of executing it against the un-prepared chart + assert.NotContains(t, failedReleases, PrepareChartKey{Name: "grafana", Namespace: "grafana"}, + "grafana should not be marked as failed") + failedKey := PrepareChartKey{Name: "error", Namespace: "failed"} + assert.Contains(t, failedReleases, failedKey, "the failing release should be marked as failed") + assert.Error(t, failedReleases[failedKey]) +} + +// TestAllowFailedReleasesFlag_Disabled verifies that the AllowFailedReleases flag +// has no unintended side effects and the default abort on error still works. +func TestAllowFailedReleasesFlag_Disabled(t *testing.T) { + releaseToChart, failedReleases, errs := issue2616PrepareCharts(t, ChartPrepareOptions{ + Concurrency: 1, + }) + + require.NotEmpty(t, errs, "PrepareCharts should return an error") + + // Verify no partial results are returned on the default abort-on-error path + assert.Nil(t, releaseToChart) + assert.Nil(t, failedReleases) +} diff --git a/pkg/state/issue_2741_test.go b/pkg/state/issue_2741_test.go index e6f888eb..8d862e49 100644 --- a/pkg/state/issue_2741_test.go +++ b/pkg/state/issue_2741_test.go @@ -116,7 +116,7 @@ func TestPrepareChartsPrefetchesSharedRemoteChart(t *testing.T) { OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", } - releaseToChart, errs := st.PrepareCharts(mockHelm, tempDir, 5, "sync", opts) + releaseToChart, _, errs := st.PrepareCharts(mockHelm, tempDir, 5, "sync", opts) require.Empty(t, errs, "PrepareCharts should not return errors") assert.Equal(t, int32(1), mockHelm.fetchCount.Load(), @@ -158,7 +158,7 @@ func TestPrepareChartsSkipsPrefetchForUniqueCharts(t *testing.T) { OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", } - releaseToChart, errs := st.PrepareCharts(mockHelm, tempDir, 3, "sync", opts) + releaseToChart, _, errs := st.PrepareCharts(mockHelm, tempDir, 3, "sync", opts) require.Empty(t, errs, "PrepareCharts should not return errors") assert.Equal(t, int32(0), mockHelm.fetchCount.Load(), @@ -199,7 +199,7 @@ func TestPrepareChartsSkipsPrefetchWhenFetchFlagsDiffer(t *testing.T) { OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", } - releaseToChart, errs := st.PrepareCharts(mockHelm, tempDir, 2, "sync", opts) + releaseToChart, _, errs := st.PrepareCharts(mockHelm, tempDir, 2, "sync", opts) require.Empty(t, errs) assert.Equal(t, int32(0), mockHelm.fetchCount.Load(), @@ -241,7 +241,7 @@ func TestPrepareChartsSkipsPrefetchWhenVerifyEnabled(t *testing.T) { OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", } - releaseToChart, errs := st.PrepareCharts(mockHelm, tempDir, 2, "sync", opts) + releaseToChart, _, errs := st.PrepareCharts(mockHelm, tempDir, 2, "sync", opts) require.Empty(t, errs) assert.Equal(t, int32(0), mockHelm.fetchCount.Load(), @@ -294,7 +294,7 @@ func TestPrepareChartsSkipsPrefetchForUnknownRepoChart(t *testing.T) { OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", } - releaseToChart, errs := st.PrepareCharts(mockHelm, tempDir, 2, "diff", opts) + releaseToChart, _, errs := st.PrepareCharts(mockHelm, tempDir, 2, "diff", opts) require.Empty(t, errs) assert.Equal(t, int32(0), mockHelm.fetchCount.Load(), @@ -367,7 +367,7 @@ func TestPrefetchedSharedChartAllowsConcurrentSyncRelease(t *testing.T) { OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", } - releaseToChart, errs := st.PrepareCharts(fetchHelm, tempDir, 5, "sync", opts) + releaseToChart, _, errs := st.PrepareCharts(fetchHelm, tempDir, 5, "sync", opts) require.Empty(t, errs) require.Equal(t, int32(1), fetchHelm.fetchCount.Load()) @@ -422,7 +422,7 @@ func TestPrefetchedSharedChartAllowsConcurrentDiffRelease(t *testing.T) { OutputDirTemplate: "{{ .OutputDir }}/{{ .Release.Name }}", } - releaseToChart, errs := st.PrepareCharts(fetchHelm, tempDir, 5, "diff", opts) + releaseToChart, _, errs := st.PrepareCharts(fetchHelm, tempDir, 5, "diff", opts) require.Empty(t, errs) require.Equal(t, int32(1), fetchHelm.fetchCount.Load()) diff --git a/pkg/state/run_helm_dep_builds_skip_refresh_test.go b/pkg/state/run_helm_dep_builds_skip_refresh_test.go index cbe5a200..be81160c 100644 --- a/pkg/state/run_helm_dep_builds_skip_refresh_test.go +++ b/pkg/state/run_helm_dep_builds_skip_refresh_test.go @@ -275,7 +275,7 @@ func TestRunHelmDepBuilds_SkipRefreshBehaviors(t *testing.T) { SkipRefresh: tt.optsSkipRefresh, } - err := st.runHelmDepBuilds(helm, 1, builds, opts) + err := st.runHelmDepBuilds(helm, 1, builds, opts, nil) require.NoError(t, err) assert.NotNil(t, helm.buildDepsFlags, "BuildDeps should have been called") @@ -336,7 +336,7 @@ func TestRunHelmDepBuilds_MultipleBuilds(t *testing.T) { opts := ChartPrepareOptions{SkipRefresh: false} - err := st.runHelmDepBuilds(helm, 1, builds, opts) + err := st.runHelmDepBuilds(helm, 1, builds, opts, nil) require.NoError(t, err) assert.True(t, helm.updateRepoCalled, "UpdateRepo should have been called") diff --git a/pkg/state/state.go b/pkg/state/state.go index 1ca72ecf..36d34afc 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. @@ -2207,22 +2208,24 @@ func (st *HelmState) prepareChartForRelease(release *ReleaseSpec, helm helmexec. } // PrepareCharts downloads and prepares all charts for the selected releases. -// Returns the chart paths and any errors encountered. +// It returns the chart paths for the successfully prepared releases, the set of +// releases that failed to prepare (keyed by release, with their errors), and any +// errors encountered. // // Note: OCI chart locks are acquired and released during chart download within this function. // The tempDir cleanup is deferred until after helm operations complete in the caller, // so charts remain available during helm commands even though locks are released. -func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurrency int, helmfileCommand string, opts ChartPrepareOptions) (map[PrepareChartKey]string, []error) { +func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurrency int, helmfileCommand string, opts ChartPrepareOptions) (map[PrepareChartKey]string, map[PrepareChartKey]error, []error) { if !opts.SkipResolve { updated, err := st.ResolveDeps() if err != nil { - return nil, []error{err} + return nil, nil, []error{err} } *st = *updated } selected, err := st.GetSelectedReleases(opts.IncludeTransitiveNeeds) if err != nil { - return nil, []error{err} + return nil, nil, []error{err} } releases := releasesNeedCharts(selected) @@ -2309,6 +2312,12 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre prepareChartInfo := make(map[PrepareChartKey]string, len(releases)) + // Releases that failed to prepare. When opts.AllowFailedReleases is set, + // these releases are excluded from the returned chart paths so that callers + // can skip them instead of executing them against their un-prepared chart + // references. + failedReleases := make(map[PrepareChartKey]error) + errs := []error{} jobQueue := make(chan *ReleaseSpec, len(releases)) @@ -2332,6 +2341,14 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre releaseOpts.ForceDownload = true } result := st.prepareChartForRelease(release, helm, dir, helmfileCommand, releaseOpts, workerIndex) + if result.err != nil { + // Error results returned by prepareChartForRelease may lack the + // release identity. Complete it here, so that the failure can be + // attributed to the correct release in failedReleases below. + result.releaseName = release.Name + result.releaseNamespace = release.Namespace + result.releaseContext = release.KubeContext + } results <- result } }, @@ -2341,7 +2358,15 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre if downloadRes.err != nil { errs = append(errs, downloadRes.err) - return + + if !opts.AllowFailedReleases { + return + } + + // Continue processing the other releases and record which one + // failed, so that the caller can skip it during execution. + failedReleases[chartPrepareResultKey(downloadRes)] = downloadRes.err + continue } func() { prepareChartInfoMutex.Lock() @@ -2361,21 +2386,40 @@ func (st *HelmState) PrepareCharts(helm helmexec.Interface, dir string, concurre }, ) - if len(errs) > 0 { - return nil, errs + if len(errs) > 0 && !opts.AllowFailedReleases { + return nil, nil, errs } if len(builds) > 0 { - if err := st.runHelmDepBuilds(helm, concurrency, builds, opts); err != nil { - return nil, []error{err} + if err := st.runHelmDepBuilds(helm, concurrency, builds, opts, failedReleases); err != nil { + if !opts.AllowFailedReleases { + return nil, nil, []error{err} + } + errs = append(errs, err) } } - return prepareChartInfo, nil + // Drop the chart paths of releases whose `helm dep build` failed (only + // possible with opts.AllowFailedReleases), so that callers skip them too. + for key := range failedReleases { + delete(prepareChartInfo, key) + } + + return prepareChartInfo, failedReleases, errs +} + +// chartPrepareResultKey returns the map key identifying the release a chart +// preparation result belongs to. +func chartPrepareResultKey(r *chartPrepareResult) PrepareChartKey { + return PrepareChartKey{ + Name: r.releaseName, + Namespace: r.releaseNamespace, + KubeContext: r.releaseContext, + } } // nolint: unparam -func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int, builds []*chartPrepareResult, opts ChartPrepareOptions) error { +func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int, builds []*chartPrepareResult, opts ChartPrepareOptions, failedReleases map[PrepareChartKey]error) error { // NOTES: // 1. `helm dep build` fails when it was run concurrency on the same chart. // To avoid that, we run `helm dep build` only once per each local chart. @@ -2399,7 +2443,15 @@ func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int, if anySkipRefresh && !opts.SkipRefresh && !st.HelmDefaults.SkipRefresh && st.NeedsRepoUpdate() { if err := helm.UpdateRepo(); err != nil { - return fmt.Errorf("updating repo: %w", err) + err = fmt.Errorf("updating repo: %w", err) + if opts.AllowFailedReleases { + // None of the dep builds below can be trusted to produce usable + // charts: mark all of them as failed and let the caller skip them. + for _, r := range builds { + failedReleases[chartPrepareResultKey(r)] = err + } + } + return err } } @@ -2428,7 +2480,14 @@ func (st *HelmState) runHelmDepBuilds(helm helmexec.Interface, concurrency int, continue } - return fmt.Errorf("building dependencies of local chart: %w", err) + err = fmt.Errorf("building dependencies of local chart %q for release %q: %w", r.chartName, r.releaseName, err) + if opts.AllowFailedReleases { + // Record the failed release and keep building the remaining charts. + failedReleases[chartPrepareResultKey(r)] = err + continue + } + + return err } }