feat: allow helmfile to continue on failed releases (#2616)

* feat: allow helmfile to continue on failed releases

Co-authored-by: Peter Honeder <peter.honeder@unwired.at>
Signed-off-by: Niklas Ott <niklas.ott@unwired.at>

* fix: skip failed-prep releases, complete flag wiring, add tests and docs (#64)

Review follow-ups for --allow-failed-releases (#2616):

- Track per-release chart preparation failures in PrepareCharts (returned
  as a map keyed by release) and remove those releases from the state in
  Run.WithPreparedCharts when --allow-failed-releases is set, so a failed
  release is never executed against its original, un-prepared chart
  reference (which could either fail again with a duplicate error or, for
  charts requiring chartify, bypass patches/dependency modifications and
  produce an unintended result). All failures are still reported at the
  end via the aggregated MultiError.
- Complete the release identity on error results from
  prepareChartForRelease so failures are attributed to the correct
  release.
- With --allow-failed-releases, continue building dependencies of the
  remaining charts when 'helm dep build' fails for one release, and skip
  the affected releases during execution.
- Wire --allow-failed-releases into 'helmfile unittest' and 'helmfile
  status'; remove the dead flag wiring for write-values and list (both
  never prepare charts, see commandsSkipChartPrep).
- Simplify control flow (guard clauses, errors.As, drop dead code and
  redundant else branches).
- Add end-to-end coverage in pkg/app/issue_2616_test.go and extend the
  state-level tests; document the flag in docs/cli.md and CHANGELOG.md.

Signed-off-by: yxxhero <11087727+yxxhero@users.noreply.github.com>

---------

Signed-off-by: Niklas Ott <niklas.ott@unwired.at>
Signed-off-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
Co-authored-by: Peter Honeder <peter.honeder@unwired.at>
Co-authored-by: yxxhero <11087727+yxxhero@users.noreply.github.com>
This commit is contained in:
Niklas
2026-09-03 06:26:46 +08:00
committed by GitHub
co-authored by Peter Honeder yxxhero
parent 2e81265f2e
commit 34ead21107
16 changed files with 423 additions and 55 deletions
+34 -20
View File
@@ -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,
+10
View File
@@ -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
}
+11
View File
@@ -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
}
+5
View File
@@ -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
}
+5
View File
@@ -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
}
+109
View File
@@ -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)
})
})
}
}
+58 -11
View File
@@ -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 {