fix(fluxcd): let helm-controller uninstall before the hook unfences

The pre-delete hook lifted flux-plunger's suspension of a release being
deleted and removed its fence in the same step. helm-controller runs the
uninstall of a release being deleted once it is no longer suspended, but
when its own finalizer is already gone the fence is all that keeps the
object, so removing the fence at once deleted the release before
helm-controller could reconcile it, without an uninstall. The hook now
keeps such a fence while helm-controller handles the generation the
lifted suspension wrote, until shortly before its retry window closes,
and names a release it still had to let go of as before.

A release being deleted under a suspension another manager also holds
was reported as cleanly released, although helm-controller skips the
uninstall of a suspended release; it is now named as deleted without
one and counted with the others.

Assisted-by: LLM
Signed-off-by: Alexey Artamonov <aleksei.artamonov@aenix.io>
This commit is contained in:
Alexey Artamonov committed 2026-10-13 15:09:00 +03:00
1 parent 7b84919db3
commit af50c4a6b4
3 files changed
+145 -21

No files matched your search

+77 -2
View File
@@ -89,6 +89,9 @@ fences_left() {
# * <ns>@<name>.rv resourceVersion, bumped by every write;
# * <ns>@<name>.suspended spec.suspend is true;
# * <ns>@<name>.deleting the release is being deleted;
# * <ns>@<name>.hcdelay helm-controller handles the release, once it is no
# longer suspended, after this many reads of its state:
# the uninstall runs and the generation is observed;
# * <ns>@<name>.owned flux-plunger's Apply entry owns spec.suspend,
# holding what a correct ownership template prints;
# * <ns>@<name>.coowned another manager also owns spec.suspend at true;
@@ -132,7 +135,10 @@ HELMFIN=finalizers.fluxcd.io
# The one predicate that tells a release helm-controller has not yet handled in
# its unsuspended form: being deleted, no longer held by its finalizer, and its
# latest generation not yet observed.
UNHANDLED='{{if .metadata.deletionTimestamp}}{{$held := false}}{{range .metadata.finalizers}}{{if eq . "finalizers.fluxcd.io"}}{{$held = true}}{{end}}{{end}}{{if and (not $held) (ne (printf "%v" .status.observedGeneration) (printf "%v" .metadata.generation))}}'
# A release being deleted under a suspension, which helm-controller skips the
# uninstall of, is told apart first.
SUSPENDED_DELETING='{{if .metadata.deletionTimestamp}}{{if .spec.suspend}}suspended{{else}}'
UNHANDLED='{{$held := false}}{{range .metadata.finalizers}}{{if eq . "finalizers.fluxcd.io"}}{{$held = true}}{{end}}{{end}}{{if and (not $held) (ne (printf "%v" .status.observedGeneration) (printf "%v" .metadata.generation))}}'
# In the hook's pod kubectl finds the API server only through the in-cluster
# config, which client-go stops falling back to once a client override such as
# --request-timeout is set: the call then goes to localhost:8080 and fails.
@@ -263,8 +269,15 @@ if [ "$verb" = get ]; then
case "$tmpl" in *'ne .manager "flux-plunger"'*) [ -e "$rel.coowned" ] && echo other ;; esac
exit 0 ;;
*deletionTimestamp*)
case "$tmpl" in *"$UNHANDLED"*) ;; *) exit 0 ;; esac
[ -e "$rel.deleting" ] || exit 0
case "$tmpl" in *"$SUSPENDED_DELETING"*) if [ -e "$rel.suspended" ]; then printf 'suspended'; exit 0; fi ;; esac
case "$tmpl" in *"$UNHANDLED"*) ;; *) exit 0 ;; esac
# helm-controller handles an unsuspended release being deleted after a
# number of reads: it runs the uninstall and records the generation.
if [ ! -e "$rel.suspended" ] && [ -s "$rel.hcdelay" ]; then
n=$(cat "$rel.hcdelay")
if [ "$n" -le 0 ]; then cp "$rel.gen" "$rel.obs"; else echo $((n - 1)) > "$rel.hcdelay"; fi
fi
grep -q -x "$HELMFIN" "$rel.fin" && exit 0
[ "$(cat "$rel.gen")" = "$(cat "$rel.obs")" ] && exit 0
printf 'unhandled'
@@ -447,6 +460,7 @@ run_hook() {
# Being deleted, helm-controller has handled its current generation.
add_release "$STATE" tenant-c app-c "$FENCE"
touch "$STATE/tenant-c@app-c.deleting"
rm "$STATE/tenant-c@app-c.suspended"
# Its uninstall completes mid-sweep, and the release is gone when read.
add_release "$STATE" tenant-d app-d "$FENCE"
touch "$STATE/tenant-d@app-d.vanish"
@@ -460,6 +474,67 @@ run_hook() {
grep -q -x "released $FENCE from tenant-c/app-c" "$tmp/out"
grep -q -x "released $FENCE from tenant-d/app-d" "$tmp/out"
grep -q -x 'released 4 of 4 fenced HelmReleases, 1 without an uninstall' "$tmp/out"
[ "$(cat "$CLOCK")" -lt "$DEADLINE" ]
rm -rf "$tmp"
}
@test "a release being deleted under another manager's suspension is named as deleted without an uninstall" {
tmp=$(mktemp -d)
prepare "$tmp"
# helm-controller skips the uninstall of a suspended release; the hook leaves a
# suspension another manager also holds, so this release goes without one.
add_release "$STATE" tenant-x app-x "$FENCE"
touch "$STATE/tenant-x@app-x.deleting"
echo yes > "$STATE/tenant-x@app-x.owned"
touch "$STATE/tenant-x@app-x.coowned"
run_hook "$tmp"
[ "$status" -eq 0 ]
[ "$(fences_left "$STATE")" -eq 0 ]
grep -q -x "released $FENCE from tenant-x/app-x without an uninstall: another manager holds its suspension" "$tmp/out"
grep -q -x 'released 1 of 1 fenced HelmReleases, 1 without an uninstall' "$tmp/out"
rm -rf "$tmp"
}
@test "the wait for helm-controller ends early enough that the last fences still get their retries" {
tmp=$(mktemp -d)
prepare "$tmp"
# helm-controller never handles this one, and the first removals of its fence
# fail: the wait must leave the removal time to be retried.
add_release "$STATE" tenant-a app-a "$FENCE"
touch "$STATE/tenant-a@app-a.deleting"
echo yes > "$STATE/tenant-a@app-a.owned"
echo 2 > "$STATE/tenant-a@app-a.patch.fail"
run_hook "$tmp"
[ "$status" -eq 0 ]
[ "$(fences_left "$STATE")" -eq 0 ]
grep -q -x "released $FENCE from tenant-a/app-a without an uninstall: helm-controller had not handled it" "$tmp/out"
[ "$(cat "$CLOCK")" -lt "$DEADLINE" ]
rm -rf "$tmp"
}
@test "a release helm-controller handles while the hook waits keeps its fence until the uninstall has run" {
tmp=$(mktemp -d)
prepare "$tmp"
# helm-controller dropped its finalizer on the suspended delete. Once the hook
# lifts flux-plunger's suspension, helm-controller runs the uninstall a few
# seconds later; removing the fence before that deletes the release without it.
add_release "$STATE" tenant-a app-a "$FENCE"
touch "$STATE/tenant-a@app-a.deleting"
echo yes > "$STATE/tenant-a@app-a.owned"
echo 3 > "$STATE/tenant-a@app-a.hcdelay"
run_hook "$tmp"
[ "$status" -eq 0 ]
[ "$(fences_left "$STATE")" -eq 0 ]
[ "$(cat "$STATE/tenant-a@app-a.obs")" = "$(cat "$STATE/tenant-a@app-a.gen")" ]
grep -q -x "released $FENCE from tenant-a/app-a" "$tmp/out"
grep -q -x 'released 1 of 1 fenced HelmReleases, 0 without an uninstall' "$tmp/out"
[ "$(cat "$CLOCK")" -lt "$DEADLINE" ]
rm -rf "$tmp"
}
+1 -1
View File
@@ -6,7 +6,7 @@ Recovers a HelmRelease stuck with `has no deployed releases`: it suspends the re
While a release is suspended for recovery it also carries the finalizer `flux-plunger.cozystack.io/uninstall-fence`. helm-controller skips the Helm uninstall of a suspended HelmRelease that is deleted, so without the fence a delete arriving during recovery would leave the release's resources behind. With it, flux-plunger lifts its suspension, helm-controller runs the uninstall, and flux-plunger then removes the fence. A delete held this way for more than ten minutes raises an `UninstallFenceHeld` Warning on the HelmRelease.
When flux-plunger stops it gives up the fences it holds itself, within its termination grace period (`shutdownGracePeriodSeconds`, 90 by default, accepted between 40 and 140): its manager is given 10 seconds to stop, and the drain 25 seconds less than the grace period. The next flux-plunger starts while the old one is still terminating, on a rollout as on an eviction or a node drain, so a single writer is kept by leader election: the old pod holds its Lease through the drain without giving it up, and the Lease outlasts the drain, so a new pod waits for it to run out, a little under two minutes at the default, before it acts. A recovery in progress is aborted: the suspension and the fence go together, and the next flux-plunger starts the recovery again. A release being deleted has flux-plunger's suspension lifted and keeps the fence until helm-controller has run its uninstall, which the drain waits for; one helm-controller has not handled when the grace period runs out keeps its fence. Uninstalling this chart then releases whatever is left: a pre-delete hook stops flux-plunger, waits out its grace period, and lifts any remaining suspension and fence without waiting again. It lifts a suspension only where flux-plunger is the sole manager holding it, which is the outcome flux-plunger's own relinquish has, so a suspension another manager also holds stays; it does so with a patch of `spec.suspend` alone rather than a `kubectl apply`, which on a release last written by a client-side apply would take over and strip the fields that apply set. A release it unfences that was being deleted, that helm-controller's own finalizer no longer held and whose latest generation helm-controller had not observed, is deleted without an uninstall and is named as such. The Job's 240-second deadline sits under the 300 seconds helm-controller allows a pre-delete hook by default. A Deployment that cannot be read or scaled down (flux-plunger would keep running and could fence behind the sweep) ends the hook before any fence is touched, a listing that never succeeds ends it before any fence is touched too, and a fence the hook cannot remove ends it after the sweep; either way the hook exits non-zero, which fails the uninstall and makes helm-controller retry it, hook included. A flux-plunger Deployment that is already gone has nothing to stop, and a flux-plunger pod that is still there after its grace period, typically one stuck `Terminating` on a `NotReady` node, is named and the sweep goes on, since failing on it would fail every retry until the node returns or the pod is force-deleted; because such a pod may still be fencing, the fences are listed again after the sweep, and any found fails the hook. Its report, `released N of M fenced HelmReleases, K without an uninstall` and a line per release, stays readable for ten minutes after the Job finishes, success included: `kubectl logs --namespace cozy-fluxcd job/flux-plunger-uninstall-fence-cleanup`.
When flux-plunger stops it gives up the fences it holds itself, within its termination grace period (`shutdownGracePeriodSeconds`, 90 by default, accepted between 40 and 140): its manager is given 10 seconds to stop, and the drain 25 seconds less than the grace period. The next flux-plunger starts while the old one is still terminating, on a rollout as on an eviction or a node drain, so a single writer is kept by leader election: the old pod holds its Lease through the drain without giving it up, and the Lease outlasts the drain, so a new pod waits for it to run out, a little under two minutes at the default, before it acts. A recovery in progress is aborted: the suspension and the fence go together, and the next flux-plunger starts the recovery again. A release being deleted has flux-plunger's suspension lifted and keeps the fence until helm-controller has run its uninstall, which the drain waits for; one helm-controller has not handled when the grace period runs out keeps its fence. Uninstalling this chart then releases whatever is left: a pre-delete hook stops flux-plunger, waits out its grace period, and lifts any remaining suspension and fence. It lifts a suspension only where flux-plunger is the sole manager holding it, which is the outcome flux-plunger's own relinquish has, so a suspension another manager also holds stays; it does so with a patch of `spec.suspend` alone rather than a `kubectl apply`, which on a release last written by a client-side apply would take over and strip the fields that apply set. A release being deleted that only the fence still holds keeps it while helm-controller, which sees the generation the lifted suspension wrote, runs the uninstall, until a little before the hook's retry window closes; one helm-controller has not handled by then is deleted without an uninstall, and so is one whose suspension another manager holds, since helm-controller skips the uninstall of a suspended release, and the report names each as such. The Job's 240-second deadline sits under the 300 seconds helm-controller allows a pre-delete hook by default. A Deployment that cannot be read or scaled down (flux-plunger would keep running and could fence behind the sweep) ends the hook before any fence is touched, a listing that never succeeds ends it before any fence is touched too, and a fence the hook cannot remove ends it after the sweep; either way the hook exits non-zero, which fails the uninstall and makes helm-controller retry it, hook included. A flux-plunger Deployment that is already gone has nothing to stop, and a flux-plunger pod that is still there after its grace period, typically one stuck `Terminating` on a `NotReady` node, is named and the sweep goes on, since failing on it would fail every retry until the node returns or the pod is force-deleted; because such a pod may still be fencing, the fences are listed again after the sweep, and any found fails the hook. Its report, `released N of M fenced HelmReleases, K without an uninstall` and a line per release, stays readable for ten minutes after the Job finishes, success included: `kubectl logs --namespace cozy-fluxcd job/flux-plunger-uninstall-fence-cleanup`.
If flux-plunger is merely stopped (scaled to zero) rather than uninstalled, or killed without a graceful stop, a fenced release that is deleted and that helm-controller has not uninstalled stays `Terminating` until flux-plunger runs again. To release one by hand, after confirming its uninstall has run or is not wanted, remove the finalizer at the position `INDEX` it has in `metadata.finalizers`; the `test` operation makes the patch fail rather than remove another finalizer if the list has changed:
@@ -6,7 +6,9 @@
being deleted before lifting that fence. This hook is the backstop for what the
drain could not finish, so that removing the chart never leaves a release that
tenant teardown waits on forever: it stops flux-plunger, waits out the drain,
then lifts any suspension and fence that remain, without waiting again.
then lifts any suspension and fence that remain, keeping the fence of a release
being deleted only while helm-controller, now that the release is unsuspended,
runs its uninstall.
Two bounds apply. helm-controller runs a pre-delete hook under the release's
uninstall timeout, 300s by default (the operator sets none), counted from when
@@ -327,16 +329,53 @@ spec:
exit 1
fi
# read_state <ns> <name>: how helm-controller stands with a release
# being deleted. "suspended": it skips the uninstall of a suspended
# release, and the suspension is another manager's. "unhandled":
# its own finalizer no longer holds the release and it has not
# observed the latest generation, so the fence is all that keeps
# the release until it runs the uninstall. Empty otherwise.
read_state() {
kubectl --namespace "$1" get "$hr" "$2" --ignore-not-found --output go-template='{{`{{if .metadata.deletionTimestamp}}{{if .spec.suspend}}suspended{{else}}{{$held := false}}{{range .metadata.finalizers}}{{if eq . "finalizers.fluxcd.io"}}{{$held = true}}{{end}}{{end}}{{if and (not $held) (ne (printf "%v" .status.observedGeneration) (printf "%v" .metadata.generation))}}unhandled{{end}}{{end}}{{end}}`}}' || echo unknown
}
# report_release <ref> <state>: remove the fence and name what that
# leaves the release to.
report_release() {
rr_ns=${1%%/*}
rr_name=${1#*/}
if ! release_fence "$rr_ns" "$rr_name"; then
echo "NOT released $1: the fence could not be removed" >&2
return
fi
released=$((released + 1))
case "$2" in
unhandled)
unhandled=$((unhandled + 1))
echo "released $fence from $1 without an uninstall: helm-controller had not handled it" ;;
suspended)
unhandled=$((unhandled + 1))
echo "released $fence from $1 without an uninstall: another manager holds its suspension" ;;
unknown)
echo "released $fence from $1; whether helm-controller had handled it could not be read" ;;
*)
echo "released $fence from $1" ;;
esac
}
# What is left is what the drain could not finish: flux-plunger did
# not stop gracefully, or helm-controller did not uninstall a release
# being deleted within the grace period. Each loses flux-plunger's
# suspension and the fence without waiting any longer. One being
# deleted that helm-controller's own finalizer no longer holds, and
# whose latest generation it has not observed, is then deleted
# without an uninstall, and is named as such.
# suspension and then its fence. A release being deleted that only
# the fence still holds keeps it while helm-controller, which sees
# the generation the lifted suspension wrote, runs the uninstall,
# until a little before the retry window closes; one it has not
# handled by then is deleted without an uninstall, and is named as
# such.
total=0
released=0
unhandled=0
pending=
for ref in $fenced; do
total=$((total + 1))
hns=${ref%%/*}
@@ -352,21 +391,31 @@ spec:
echo "NOT released $ref: flux-plunger's suspension could not be lifted" >&2
continue ;;
esac
state=$(kubectl --namespace "$hns" get "$hr" "$name" --ignore-not-found --output go-template='{{`{{if .metadata.deletionTimestamp}}{{$held := false}}{{range .metadata.finalizers}}{{if eq . "finalizers.fluxcd.io"}}{{$held = true}}{{end}}{{end}}{{if and (not $held) (ne (printf "%v" .status.observedGeneration) (printf "%v" .metadata.generation))}}unhandled{{end}}{{end}}`}}') || state=unknown
if ! release_fence "$hns" "$name"; then
echo "NOT released $ref: the fence could not be removed" >&2
state=$(read_state "$hns" "$name")
if [ "$state" = unhandled ]; then
pending="$pending $ref"
continue
fi
released=$((released + 1))
case "$state" in
unhandled)
unhandled=$((unhandled + 1))
echo "released $fence from $ref without an uninstall: helm-controller had not handled it" ;;
unknown)
echo "released $fence from $ref; whether helm-controller had handled it could not be read" ;;
*)
echo "released $fence from $ref" ;;
esac
report_release "$ref" "$state"
done
# The last fences go well before the window closes, so their
# removal still has retries.
wait_until=$(( retry_until - 30 ))
while [ -n "$pending" ] && [ "$(date +%s)" -lt "$wait_until" ]; do
sleep 2
still=
for ref in $pending; do
state=$(read_state "${ref%%/*}" "${ref#*/}")
if [ "$state" = unhandled ]; then
still="$still $ref"
else
report_release "$ref" "$state"
fi
done
pending=$still
done
for ref in $pending; do
report_release "$ref" unhandled
done
echo "released $released of $total fenced HelmReleases, $unhandled without an uninstall"