public_git/helmfile
1
0
Fork 0
mirror of https://github.com/helmfile/helmfile.git synced 2026-10-20 13:48:22 +02:00
Code Issues Projects Releases Wiki Activity

feat: support Helm 4 --rollback-on-failure alongside deprecated --atomic (#2722)

Browse Source 
* feat: support Helm 4 --rollback-on-failure alongside deprecated --atomic (#2712)

Helm 4 renamed the `--atomic` flag to `--rollback-on-failure`
(helm/helm#13629). The old flag still works under Helm 4 but is
deprecated (prints a warning) and slated for removal in Helm 5.

Add a `rollbackOnFailure` key to both `helmDefaults` (HelmSpec) and
`releases[]` (ReleaseSpec) that emits `--rollback-on-failure`. It
requires Helm 4+ (errors otherwise) and is mutually exclusive with
`atomic`.

Additionally, when the resolved Helm binary is v4+, an existing
`atomic: true` now emits `--rollback-on-failure` instead of `--atomic`,
so users are migrated off the deprecated flag automatically without any
config change. On older Helm, `atomic: true` continues to emit
`--atomic`.

Updated the spew-based values-ID hashes in temp_test.go that change
whenever ReleaseSpec gains a field (same approach as the --force-conflicts
change in #2480).

Closes #2712.

Signed-off-by: yxxhero <aiopsclub@163.com>

* test: add integration test for rollback-on-failure / atomic migration (#2712)

Covers the end-to-end plumbing that unit tests cannot (real helm version
detection + cluster deploy) via test/integration/run.sh:

  1. atomic: true parses, deploys a ConfigMap, and emits the version-correct
     flag: --rollback-on-failure on Helm 4 (auto-migration of the deprecated
     --atomic) and --atomic on Helm 3.
  2. rollbackOnFailure: true emits --rollback-on-failure on Helm 4 and is
     rejected with a clear Helm-4-required error on Helm 3.

Flag assertions grep the `exec: helm upgrade --install` lines logged under
--debug, matching flags as standalone tokens so the release name
"issue-2712-atomic" cannot be confused with the "--atomic" flag.

Verified locally against Helm 4.2.3: both atomic:true and
rollbackOnFailure:true emit --rollback-on-failure with no --atomic.

Signed-off-by: yxxhero <aiopsclub@163.com>

---------

Signed-off-by: yxxhero <aiopsclub@163.com>
This commit is contained in:
yxxhero
2026-07-31 22:22:31 +08:00
committed by GitHub
parent 4a538536af
commit 7f748fec1b
8 changed files with 306 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/configuration.md
+7 -1
View File
@@ -246,8 +246,12 @@ releases:
# Defines the strategy to use when updating. Possible value is:
# - "reinstallIfForbidden": Performs an uninstall before the update only if the update is forbidden (e.g., due to permission issues or conflicts).
updateStrategy: ""
# restores previous state in case of failed release (default false)
# restores previous state in case of failed release (default false).
# On Helm 4+ this emits --rollback-on-failure (the successor to the deprecated --atomic).
atomic: true
# restores previous state on a failed release via the Helm 4 --rollback-on-failure flag
# (default false). Requires Helm 4 or greater. Mutually exclusive with atomic.
rollbackOnFailure: false
# when true, cleans up any new resources created during a failed release (default false)
cleanupOnFail: false
# --kube-context to be passed to helm commands
@@ -437,6 +441,8 @@ The following `helmDefaults` fields are also available but not shown in the exam
| `enableDNS` | bool | false | Enable DNS lookups when rendering templates |
| `skipCRDs` | bool | false | Skip CRDs during installation |
| `skipRefresh` | bool | false | Skip running `helm dependency up` |
| `atomic` | bool | false | Restore previous state on a failed install/upgrade. On Helm 4+ emits `--rollback-on-failure` (the successor to the deprecated `--atomic`); on older Helm emits `--atomic` |
| `rollbackOnFailure` | bool | false | Restore previous state on a failed install/upgrade via the Helm 4 `--rollback-on-failure` flag. Requires Helm 4 or greater. Mutually exclusive with `atomic` |
| `forceConflicts` | bool | false | Force server-side apply changes against conflicts (Helm 4 only) |
| `takeOwnership` | bool | false | Take ownership of existing resources |
| `serverSide` | string | | Controls the helm 4 `--server-side` flag. Must be `"true"`, `"false"`, or `"auto"` (Helm 4 only) |
pkg/state/state.go
+25 -1
View File
@@ -239,6 +239,9 @@ type HelmSpec struct {
Force bool `yaml:"force"`
// Atomic, when set to true, restore previous state in case of a failed install/upgrade attempt
Atomic bool `yaml:"atomic"`
// RollbackOnFailure, when set to true, restores previous state on a failed install/upgrade via the
// Helm 4 --rollback-on-failure flag (the successor to the deprecated --atomic flag). Requires Helm 4 or greater.
RollbackOnFailure bool `yaml:"rollbackOnFailure"`
// CleanupOnFail, when set to true, the --cleanup-on-fail helm flag is passed to the upgrade command
CleanupOnFail bool `yaml:"cleanupOnFail,omitempty"`
// ForceConflicts, when set to true, force server-side apply changes against conflicts (Helm 4 only)
@@ -363,6 +366,9 @@ type ReleaseSpec struct {
UpdateStrategy string `yaml:"updateStrategy,omitempty"`
// Atomic, when set to true, restore previous state in case of a failed install/upgrade attempt
Atomic *bool `yaml:"atomic,omitempty"`
// RollbackOnFailure, when set to true, restores previous state on a failed install/upgrade via the
// Helm 4 --rollback-on-failure flag (the successor to the deprecated --atomic flag). Requires Helm 4 or greater.
RollbackOnFailure *bool `yaml:"rollbackOnFailure,omitempty"`
// CleanupOnFail, when set to true, the --cleanup-on-fail helm flag is passed to the upgrade command
CleanupOnFail *bool `yaml:"cleanupOnFail,omitempty"`
// ForceConflicts, when set to true, force server-side apply changes against conflicts (Helm 4 only)
@@ -4002,8 +4008,26 @@ func (st *HelmState) flagsForUpgrade(helm helmexec.Interface, release *ReleaseSp
flags = append(flags, "--recreate-pods")
}
if release.Atomic != nil && *release.Atomic || release.Atomic == nil && st.HelmDefaults.Atomic {
atomicEnabled := (release.Atomic != nil && *release.Atomic) || (release.Atomic == nil && st.HelmDefaults.Atomic)
rollbackOnFailureEnabled := (release.RollbackOnFailure != nil && *release.RollbackOnFailure) || (release.RollbackOnFailure == nil && st.HelmDefaults.RollbackOnFailure)
if atomicEnabled && rollbackOnFailureEnabled {
return nil, nil, fmt.Errorf("atomic and rollbackOnFailure are mutually exclusive (check both releases[].atomic/rollbackOnFailure and helmDefaults.atomic/rollbackOnFailure)")
}
// rollbackOnFailure maps to the Helm 4 --rollback-on-failure flag and is unsupported on older Helm.
if rollbackOnFailureEnabled && !helm.IsHelm4() {
return nil, nil, fmt.Errorf("rollbackOnFailure requires Helm 4 or greater (set via releases[].rollbackOnFailure or helmDefaults.rollbackOnFailure)")
}
// Helm 4 renamed --atomic to --rollback-on-failure; the former is deprecated (prints a warning)
// and slated for removal in Helm 5. Emit the new flag on Helm 4+ for both `atomic: true` and
// `rollbackOnFailure: true` so users migrate off it automatically; on older Helm, `atomic: true`
// still emits --atomic. See issue #2712.
if atomicEnabled && !helm.IsHelm4() {
flags = append(flags, "--atomic")
} else if atomicEnabled || rollbackOnFailureEnabled {
flags = append(flags, "--rollback-on-failure")
}
if release.CleanupOnFail != nil && *release.CleanupOnFail || release.CleanupOnFail == nil && st.HelmDefaults.CleanupOnFail {
pkg/state/state_test.go
+151 -3
View File
@@ -646,8 +646,10 @@ func TestHelmState_flagsForUpgrade(t *testing.T) {
{
name: "atomic",
defaults: HelmSpec{
Atomic: false,
Atomic: false,
CreateNamespace: &disable,
},
version: semver.MustParse("3.10.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
@@ -661,11 +663,33 @@ func TestHelmState_flagsForUpgrade(t *testing.T) {
"--namespace", "test-namespace",
},
},
{
name: "atomic-helm4",
defaults: HelmSpec{
Atomic: false,
CreateNamespace: &disable,
},
version: semver.MustParse("4.0.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
Atomic: &enable,
Name: "test-charts",
Namespace: "test-namespace",
},
want: []string{
"--version", "0.1",
"--rollback-on-failure",
"--namespace", "test-namespace",
},
},
{
name: "atomic-override-default",
defaults: HelmSpec{
Atomic: true,
Atomic: true,
CreateNamespace: &disable,
},
version: semver.MustParse("3.10.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
@@ -681,8 +705,10 @@ func TestHelmState_flagsForUpgrade(t *testing.T) {
{
name: "atomic-from-default",
defaults: HelmSpec{
Atomic: true,
Atomic: true,
CreateNamespace: &disable,
},
version: semver.MustParse("3.10.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
@@ -695,6 +721,128 @@ func TestHelmState_flagsForUpgrade(t *testing.T) {
"--namespace", "test-namespace",
},
},
{
name: "atomic-from-default-helm4",
defaults: HelmSpec{
Atomic: true,
CreateNamespace: &disable,
},
version: semver.MustParse("4.0.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
Name: "test-charts",
Namespace: "test-namespace",
},
want: []string{
"--version", "0.1",
"--rollback-on-failure",
"--namespace", "test-namespace",
},
},
{
name: "rollback-on-failure",
defaults: HelmSpec{
CreateNamespace: &disable,
},
version: semver.MustParse("4.0.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
RollbackOnFailure: &enable,
Name: "test-charts",
Namespace: "test-namespace",
},
want: []string{
"--version", "0.1",
"--rollback-on-failure",
"--namespace", "test-namespace",
},
},
{
name: "rollback-on-failure-from-default",
defaults: HelmSpec{
RollbackOnFailure: true,
CreateNamespace: &disable,
},
version: semver.MustParse("4.0.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
Name: "test-charts",
Namespace: "test-namespace",
},
want: []string{
"--version", "0.1",
"--rollback-on-failure",
"--namespace", "test-namespace",
},
},
{
name: "rollback-on-failure-override-default",
defaults: HelmSpec{
RollbackOnFailure: true,
CreateNamespace: &disable,
},
version: semver.MustParse("4.0.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
RollbackOnFailure: &disable,
Name: "test-charts",
Namespace: "test-namespace",
},
want: []string{
"--version", "0.1",
"--namespace", "test-namespace",
},
},
{
name: "rollback-on-failure-helm3-error",
defaults: HelmSpec{
CreateNamespace: &disable,
},
version: semver.MustParse("3.10.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
RollbackOnFailure: &enable,
Name: "test-charts",
Namespace: "test-namespace",
},
wantErr: "rollbackOnFailure requires Helm 4 or greater (set via releases[].rollbackOnFailure or helmDefaults.rollbackOnFailure)",
},
{
name: "rollback-on-failure-from-default-helm3-error",
defaults: HelmSpec{
RollbackOnFailure: true,
CreateNamespace: &disable,
},
version: semver.MustParse("3.10.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
Name: "test-charts",
Namespace: "test-namespace",
},
wantErr: "rollbackOnFailure requires Helm 4 or greater (set via releases[].rollbackOnFailure or helmDefaults.rollbackOnFailure)",
},
{
name: "atomic-and-rollback-on-failure-mutually-exclusive-helm4",
defaults: HelmSpec{
CreateNamespace: &disable,
},
version: semver.MustParse("4.0.0"),
release: &ReleaseSpec{
Chart: "test/chart",
Version: "0.1",
Atomic: &enable,
RollbackOnFailure: &enable,
Name: "test-charts",
Namespace: "test-namespace",
},
wantErr: "atomic and rollbackOnFailure are mutually exclusive (check both releases[].atomic/rollbackOnFailure and helmDefaults.atomic/rollbackOnFailure)",
},
{
name: "cleanup-on-fail",
defaults: HelmSpec{
pkg/state/temp_test.go
+6 -6
View File
@@ -38,39 +38,39 @@ func TestGenerateID(t *testing.T) {
run(testcase{
subject: "baseline",
release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
want: "foo-values-6765d87f7c",
want: "foo-values-5bcd864488",
})
run(testcase{
subject: "different bytes content",
release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
data: []byte(`{"k":"v"}`),
want: "foo-values-6d7db5f8b",
want: "foo-values-59cd566bbc",
})
run(testcase{
subject: "different map content",
release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"},
data: map[string]any{"k": "v"},
want: "foo-values-5c55f66dd",
want: "foo-values-58bcc85765",
})
run(testcase{
subject: "different chart",
release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"},
want: "foo-values-7665888bf4",
want: "foo-values-7796c46c49",
})
run(testcase{
subject: "different name",
release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"},
want: "bar-values-bc6f974bd",
want: "bar-values-5b5f6f54cc",
})
run(testcase{
subject: "specific ns",
release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"},
want: "myns-foo-values-76f8c7c596",
want: "myns-foo-values-547578788f",
})
for id, n := range ids {
test/integration/run.sh
+1
View File
@@ -151,6 +151,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes
. ${dir}/test-cases/issue-1172-selector-required-env.sh
. ${dir}/test-cases/kubedog-tracking.sh
. ${dir}/test-cases/lookup.sh
. ${dir}/test-cases/issue-2712-rollback-on-failure.sh
# ALL DONE -----------------------------------------------------------------------------------------------------------
test/integration/test-cases/issue-2712-rollback-on-failure.sh
+88
View File
@@ -0,0 +1,88 @@
# Issue #2712: Support Helm 4 --rollback-on-failure alongside deprecated --atomic
# https://github.com/helmfile/helmfile/issues/2712
#
# End-to-end coverage:
# 1. atomic: true is accepted (no unknown-field parse error), deploys, and emits the
# version-correct flag: --rollback-on-failure on Helm 4 (auto-migration of the
# deprecated --atomic), --atomic on Helm 3.
# 2. rollbackOnFailure: true emits --rollback-on-failure on Helm 4 and is rejected
# with a clear Helm-4-required error on Helm 3.
#
# The flag-emission assertions inspect the `exec: helm upgrade --install ...` lines
# that helmfile logs under --debug (see pkg/helmexec/exec.go exec helper).
issue_2712_input_dir="${cases_dir}/issue-2712-rollback-on-failure/input"
issue_2712_tmp=$(mktemp -d)
cleanup_issue_2712() {
rm -rf "${issue_2712_tmp}"
}
trap cleanup_issue_2712 EXIT
# Succeeds when the captured --debug log contains a `helm upgrade --install` command
# carrying the given flag. The flag is matched as a standalone argument (surrounded by
# whitespace) so a release name such as "issue-2712-atomic" can never be confused with
# the "--atomic" flag. $1 = log file, $2 = flag token (e.g. --rollback-on-failure).
issue_2712_assert_upgrade_flag() {
grep -E "upgrade --install" "$1" | grep -qE "(^|[[:space:]])${2}([[:space:]]|$)"
}
# --- Test 1: atomic: true is accepted, deploys, and emits the version-correct flag ---
if [ "${HELMFILE_HELM4}" = "1" ]; then
test_start "issue-2712 atomic migrates to --rollback-on-failure (Helm 4)"
else
test_start "issue-2712 atomic emits --atomic (Helm 3)"
fi
info "Syncing release with atomic: true"
if ! ${helmfile} --debug -f "${issue_2712_input_dir}/atomic.yaml" sync > "${issue_2712_tmp}/atomic.log" 2>&1; then
cat "${issue_2712_tmp}/atomic.log"
fail "helmfile sync with atomic: true should succeed"
fi
info "Verifying ConfigMap issue-2712-atomic-cm was applied"
${kubectl} get configmap issue-2712-atomic-cm > /dev/null \
|| fail "ConfigMap issue-2712-atomic-cm should exist after sync"
if [ "${HELMFILE_HELM4}" = "1" ]; then
issue_2712_assert_upgrade_flag "${issue_2712_tmp}/atomic.log" "--rollback-on-failure" \
|| { cat "${issue_2712_tmp}/atomic.log"; fail "Helm 4 should migrate atomic:true to --rollback-on-failure"; }
if issue_2712_assert_upgrade_flag "${issue_2712_tmp}/atomic.log" "--atomic"; then
fail "Helm 4 should not emit the deprecated --atomic flag for atomic:true"
fi
test_pass "issue-2712 atomic migrates to --rollback-on-failure (Helm 4)"
else
issue_2712_assert_upgrade_flag "${issue_2712_tmp}/atomic.log" "--atomic" \
|| { cat "${issue_2712_tmp}/atomic.log"; fail "Helm 3 should emit --atomic for atomic:true"; }
test_pass "issue-2712 atomic emits --atomic (Helm 3)"
fi
# --- Test 2: rollbackOnFailure behavior is version-gated ---
if [ "${HELMFILE_HELM4}" = "1" ]; then
test_start "issue-2712 rollbackOnFailure emits --rollback-on-failure (Helm 4)"
info "Syncing release with rollbackOnFailure: true"
if ! ${helmfile} --debug -f "${issue_2712_input_dir}/rollback.yaml" sync > "${issue_2712_tmp}/rollback.log" 2>&1; then
cat "${issue_2712_tmp}/rollback.log"
fail "helmfile sync with rollbackOnFailure: true should succeed on Helm 4"
fi
${kubectl} get configmap issue-2712-rollback-cm > /dev/null \
|| fail "ConfigMap issue-2712-rollback-cm should exist after sync"
issue_2712_assert_upgrade_flag "${issue_2712_tmp}/rollback.log" "--rollback-on-failure" \
|| { cat "${issue_2712_tmp}/rollback.log"; fail "rollbackOnFailure:true should emit --rollback-on-failure on Helm 4"; }
test_pass "issue-2712 rollbackOnFailure emits --rollback-on-failure (Helm 4)"
else
test_start "issue-2712 rollbackOnFailure rejected on Helm 3"
info "Syncing release with rollbackOnFailure: true (expected to fail)"
if ${helmfile} -f "${issue_2712_input_dir}/rollback.yaml" sync > "${issue_2712_tmp}/rollback.log" 2>&1; then
cat "${issue_2712_tmp}/rollback.log"
fail "helmfile sync with rollbackOnFailure: true should fail on Helm 3"
fi
grep -q "rollbackOnFailure requires Helm 4" "${issue_2712_tmp}/rollback.log" \
|| { cat "${issue_2712_tmp}/rollback.log"; fail "expected 'rollbackOnFailure requires Helm 4' error on Helm 3"; }
test_pass "issue-2712 rollbackOnFailure rejected on Helm 3"
fi
# --- Cleanup ---
info "Cleaning up issue-2712 releases"
${helmfile} -f "${issue_2712_input_dir}/atomic.yaml" destroy > /dev/null 2>&1 || true
${helmfile} -f "${issue_2712_input_dir}/rollback.yaml" destroy > /dev/null 2>&1 || true
test/integration/test-cases/issue-2712-rollback-on-failure/input/atomic.yaml
+14
View File
@@ -0,0 +1,14 @@
releases:
- name: issue-2712-atomic
chart: ../../../charts/raw
atomic: true
values:
- templates:
- |
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ .Release.Name }}-cm
namespace: {{ .Release.Namespace }}
data:
version: "1"
test/integration/test-cases/issue-2712-rollback-on-failure/input/rollback.yaml
+14
View File
@@ -0,0 +1,14 @@
releases:
- name: issue-2712-rollback
chart: ../../../charts/raw
rollbackOnFailure: true
values:
- templates:
- |
apiVersion: v1
kind: ConfigMap
metadata:
name: {{ .Release.Name }}-cm
namespace: {{ .Release.Namespace }}
data:
version: "1"