mirror of
https://github.com/cozystack/cozystack.git
synced 2026-10-23 03:27:50 +09:00
fix(fluxcd): close the flux-plunger review follow-ups for the hook, drain and Warnings (#4914)
## What this PR does Follow-up to the flux-plunger uninstall fence: the points the last review round raised and left for after the merge, plus four found by a later independent review. None changes the fence's design; each closes a gap in what it reports, how long it waits, or what its documentation says. **Pre-delete hook.** - A release being deleted that only the fence still holds is no longer unfenced in the same step that lifts its suspension. helm-controller runs the uninstall of a release being deleted once it is no longer suspended, but removing the fence at once deleted the object before helm-controller could reconcile it. The hook now keeps that 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 whose state cannot be read keeps its fence for the wait the same way, rather than being let go of on one failed read. - A release being deleted while still suspended, by anyone but flux-plunger, was reported as cleanly released, although helm-controller skips the uninstall of a suspended release. It is now named as deleted without an uninstall and counted with the others. - Each release gets its own share of the retry window, capped at the window's end. One release that could never be read used to take the whole window and leave every release listed after it a single attempt. The share is a cap, not a floor of attempts: a floor would let enough failing releases run the Job past its deadline, which kills it before its report is written. - The hook stops, waits for and lists again only for the flux-plunger of its own release. Another one (a second installation under another release name, changed labels, a binary outside the cluster) is not stopped by the hook, keeps the fences it sets and gives them up itself; the README now says so instead of the hook re-listing unconditionally, which would not close that case and would fail the uninstall for as long as a second installation is recovering anything. - helm-controller's finalizer, which the hook reads to tell whether a release being deleted is still held, is tied to the constant the controller uses, as the fence already was. Two comments are corrected: the retry window counts from the script's start, not the Job's, and kubectl scale's internal conflict retry never runs here. **Shutdown drain.** - A drain deadline of zero or less is refused at startup, and the chart's lowest accepted grace period is held to the same constants as the drain subtraction, so raising the manager's stop bound cannot make the chart render a deadline that is already spent. The timings the README and the values comment state are held to those constants too. - Fences left behind are reported as an error naming each release, not an Info line; a drain that never got a listing answered says so instead of reporting a count of -1; a flux-plunger that never led no longer claims it released every fence. - The drain's Events fold repeats into one counted Event per release and reason, as the manager's recorder does, instead of one object per pass. The fold is a patch, the verb the chart grants on events, and every write is bounded by the drain's deadline. It does not add a per-release Event for fences left at the end: by then the drain's deadline is spent, and writing one would reach past it. - Tests now hold the outcome line, the retry of a release whose fence cannot be given up yet, and the pause between passes, none of which any suite noticed when removed. **Controller.** The `SuspensionNotOwned` and `TerminatingWhileSuspended` Warnings were keyed on the processed-version annotation, which stays on every release flux-plunger ever recovered, so a later restore, migration or operator suspension of a healthy release drew a Warning telling the reader to resume it, and a migration's deliberate suspend-then-delete was called an accident. Both now key on the state a leftover actually freezes the release in, `Ready=False` with `has no deployed releases`, since helm-controller rewrites no condition of a suspended release; the README's candidate query follows. Two comments that claimed more than holds are qualified. **Chart and README.** A `shutdownGracePeriodSeconds` of a million or more now gets the range message rather than being refused as a fraction. The README gives the recovery steps in the order the controller runs them (the annotation is written before the Secret is deleted), and names the window in which a backup restore started during a recovery is unsuspended under it, since the etcd, CNPG and Velero restore steps write `spec.suspend` only when it differs. ### Testing Each change was written test first: a test that builds the binary and runs it with a deadline of zero, helm-unittest for the chart, unit tests with a captured logger and the fake client for the drain's outcome, Events and passes (the Event fold under a client that refuses the update verb, as the chart's role does), table tests for both Warnings, and the bats suite for the hook, whose fake kubectl now models helm-controller handling a release some reads after its suspension is lifted and a state read that fails. Every fix was then reverted on its own, and the suite went red each time. The envtest suites pass unchanged. The README's candidate query was run against HelmReleases read from a live cluster, with a stuck, a healthy, an owned and an unsuspended variant, and lists only the stuck one. ### Screenshots Not applicable: no UI change. ### Downstream repositories The trigger map was walked against the diff: `internal/controller/fluxplunger`, `cmd/flux-plunger`, the `packages/system/flux-plunger` chart (templates, tests, README) and its bats test under `hack/`. The website's FluxPlunger section in `guides/concepts.md` draws the recovery with the Secret deleted before the annotation is written, which is the order before the fence change; it needs the same correction as the chart README. That change is held as a draft until v1.7.0 is released and the website's `next/` is re-initialized: the fence change is not part of release-1.7, so the corrected order would be wrong for v1.7 docs, which are copied from `next/`. No other repository is reached by this diff. - [ ] No downstream repository is affected by this change - [x] [cozystack/website](https://github.com/cozystack/website) - follow-up: https://github.com/cozystack/website/pull/752 (draft, merge after v1.7.0) - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(fluxcd): flux-plunger's pre-delete hook now lets helm-controller run the uninstall of a release being deleted before it removes the release's fence, gives each release its own share of its retries, and reports releases deleted under another manager's suspension; the shutdown drain reports the fences it leaves as errors and refuses a deadline that leaves no time ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Shutdown drain timeouts must be positive, and invalid values now stop startup with a clear error. * Uninstall cleanup waits for Helm to handle releases before removing their fences, while bounding retries and reporting releases that remain incomplete. * Recovery and suspension warnings now reflect the release’s actual failure state, and recovery can retry stale storage cleanup against the same revision. * **Documentation** * Clarified shutdown grace-period requirements, uninstall behavior, and guidance for backup restores and existing suspensions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
12 files changed
+926
-125
No files matched your search
@@ -82,6 +82,11 @@ func main() {
|
||||
|
||||
ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts)))
|
||||
|
||||
if err := fluxplunger.ValidateShutdownDrainTimeout(shutdownDrainTimeout); err != nil {
|
||||
setupLog.Error(err, "invalid flag")
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
// if the enable-http2 flag is false (the default), http/2 should be disabled
|
||||
// due to its vulnerabilities. More specifically, disabling http/2 will
|
||||
// prevent from being vulnerable to the HTTP/2 Stream Cancellation and
|
||||
|
||||
@@ -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;
|
||||
@@ -115,7 +118,8 @@ fences_left() {
|
||||
# for a reason other than pods (wait), the listing
|
||||
# (list), a listing after the first (relist), the
|
||||
# pod listing (pods), a release's ownership read
|
||||
# (<ns>@<name>.ownedread), unsuspend
|
||||
# (<ns>@<name>.ownedread), a release's state read
|
||||
# (<ns>@<name>.stateread), unsuspend
|
||||
# (<ns>@<name>.apply), unsuspend racing another
|
||||
# write (<ns>@<name>.applyconflict) or finalizer patch
|
||||
# (<ns>@<name>.patch);
|
||||
@@ -132,7 +136,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 +270,16 @@ 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
|
||||
if take_fail "$rel.stateread"; then echo "fake: the state read timed out" >&2; exit 1; fi
|
||||
[ -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 +462,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 +476,162 @@ 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: it is still suspended, not by flux-plunger" "$tmp/out"
|
||||
grep -q -x 'released 1 of 1 fenced HelmReleases, 1 without an uninstall' "$tmp/out"
|
||||
rm -rf "$tmp"
|
||||
}
|
||||
|
||||
@test "a release that can never be read does not leave the ones listed after it a single attempt" {
|
||||
tmp=$(mktemp -d)
|
||||
prepare "$tmp"
|
||||
# Listed first, never readable: it must not spend the whole retry window.
|
||||
add_release "$STATE" tenant-a app-a "$FENCE"
|
||||
echo 9999 > "$STATE/tenant-a@app-a.ownedread.fail"
|
||||
# Listed after it, and its fence comes off only on the third try.
|
||||
add_release "$STATE" tenant-z app-z "$FENCE"
|
||||
echo 2 > "$STATE/tenant-z@app-z.patch.fail"
|
||||
|
||||
run_hook "$tmp"
|
||||
|
||||
[ "$status" -ne 0 ]
|
||||
grep -q '^NOT released tenant-a/app-a: it could not be read' "$tmp/out"
|
||||
grep -q -x "released $FENCE from tenant-z/app-z" "$tmp/out"
|
||||
[ "$(cat "$CLOCK")" -lt "$DEADLINE" ]
|
||||
rm -rf "$tmp"
|
||||
}
|
||||
|
||||
@test "releases swept after the retry window closed get one attempt each, so the report is still written" {
|
||||
tmp=$(mktemp -d)
|
||||
prepare "$tmp"
|
||||
# A slow scale-down and the stuck pod's wait take the window to its end; a
|
||||
# share of it per release must not reach past the window for these.
|
||||
echo $((WINDOW - 10)) > "$STATE/scale.slow"
|
||||
touch "$STATE/pods.stuck"
|
||||
for n in a b c; do
|
||||
add_release "$STATE" "tenant-$n" "app-$n" "$FENCE"
|
||||
echo 9999 > "$STATE/tenant-$n@app-$n.ownedread.fail"
|
||||
done
|
||||
|
||||
run_hook "$tmp"
|
||||
|
||||
[ "$status" -ne 0 ]
|
||||
grep -q -x 'released 0 of 3 fenced HelmReleases, 0 without an uninstall' "$tmp/out"
|
||||
[ "$(cat "$CLOCK")" -lt "$DEADLINE" ]
|
||||
rm -rf "$tmp"
|
||||
}
|
||||
|
||||
@test "the listing after the sweep gets the rest of the window, not the last release's share" {
|
||||
tmp=$(mktemp -d)
|
||||
prepare "$tmp"
|
||||
# The pod stays, so the fences are listed again; that listing fails for longer
|
||||
# than one release's share before it answers.
|
||||
echo 1 > "$STATE/wait.fail"
|
||||
touch "$STATE/pods.stuck"
|
||||
add_release "$STATE" tenant-a app-a "$FENCE"
|
||||
echo 20 > "$STATE/relist.fail"
|
||||
|
||||
run_hook "$tmp"
|
||||
|
||||
[ "$status" -eq 0 ]
|
||||
[ "$(fences_left "$STATE")" -eq 0 ]
|
||||
grep -q -x 'released 1 of 1 fenced HelmReleases, 0 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 state read that fails once does not end the wait for helm-controller" {
|
||||
tmp=$(mktemp -d)
|
||||
prepare "$tmp"
|
||||
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"
|
||||
echo 1 > "$STATE/tenant-a@app-a.stateread.fail"
|
||||
|
||||
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"
|
||||
rm -rf "$tmp"
|
||||
}
|
||||
|
||||
@test "a release whose state can never be read is still released inside the deadline, and says so" {
|
||||
tmp=$(mktemp -d)
|
||||
prepare "$tmp"
|
||||
add_release "$STATE" tenant-a app-a "$FENCE"
|
||||
touch "$STATE/tenant-a@app-a.deleting"
|
||||
echo yes > "$STATE/tenant-a@app-a.owned"
|
||||
echo 9999 > "$STATE/tenant-a@app-a.stateread.fail"
|
||||
|
||||
run_hook "$tmp"
|
||||
|
||||
[ "$status" -eq 0 ]
|
||||
[ "$(fences_left "$STATE")" -eq 0 ]
|
||||
grep -q -x "released $FENCE from tenant-a/app-a; whether helm-controller had handled it could not be read" "$tmp/out"
|
||||
# It kept the fence for the whole wait, as one helm-controller had not handled.
|
||||
[ "$(cat "$CLOCK")" -ge $((WINDOW - 30)) ]
|
||||
[ "$(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"
|
||||
}
|
||||
|
||||
|
||||
@@ -6,12 +6,16 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
helmv2 "github.com/fluxcd/helm-controller/api/v2"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
apierrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"k8s.io/apimachinery/pkg/runtime"
|
||||
"k8s.io/apimachinery/pkg/types"
|
||||
ctrl "sigs.k8s.io/controller-runtime"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"
|
||||
@@ -27,6 +31,16 @@ const (
|
||||
ShutdownDrainTimeoutFlag = "shutdown-drain-timeout"
|
||||
)
|
||||
|
||||
// ValidateShutdownDrainTimeout refuses a drain deadline that is already spent
|
||||
// when the drain starts: the drain would leave every fence behind without
|
||||
// listing one.
|
||||
func ValidateShutdownDrainTimeout(d time.Duration) error {
|
||||
if d <= 0 {
|
||||
return fmt.Errorf("--%s must be positive, got %v", ShutdownDrainTimeoutFlag, d)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// DrainFences gives up every fence flux-plunger holds, for a flux-plunger that is
|
||||
// stopping. A live release has its recovery aborted, the suspension and the fence
|
||||
// lifted together, which leaves it as recovery found it for the next flux-plunger
|
||||
@@ -43,8 +57,10 @@ func (r *FluxPlunger) DrainFences(ctx context.Context) (int, error) {
|
||||
if r.APIReader != nil {
|
||||
reader = r.APIReader
|
||||
}
|
||||
// held is what the last listing that succeeded found; -1 until one has.
|
||||
held := -1
|
||||
// left is how many fences the last pass that ran left in place, named in
|
||||
// names; -1 until a listing has succeeded.
|
||||
left := -1
|
||||
var names []string
|
||||
for {
|
||||
list := &helmv2.HelmReleaseList{}
|
||||
if err := reader.List(ctx, list); err != nil {
|
||||
@@ -54,18 +70,17 @@ func (r *FluxPlunger) DrainFences(ctx context.Context) (int, error) {
|
||||
logger.Info("Could not list HelmReleases yet, retrying", "error", err.Error())
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return held, ctx.Err()
|
||||
return left, stillFenced(ctx.Err(), names)
|
||||
case <-time.After(drainPollInterval):
|
||||
}
|
||||
continue
|
||||
}
|
||||
held = 0
|
||||
names = names[:0]
|
||||
for i := range list.Items {
|
||||
hr := &list.Items[i]
|
||||
if !controllerutil.ContainsFinalizer(hr, fenceFinalizer) {
|
||||
continue
|
||||
}
|
||||
held++
|
||||
var err error
|
||||
if hr.DeletionTimestamp.IsZero() {
|
||||
// The relinquishing apply drops every field flux-plunger's manager owns,
|
||||
@@ -78,18 +93,42 @@ func (r *FluxPlunger) DrainFences(ctx context.Context) (int, error) {
|
||||
if err != nil {
|
||||
logger.Info("Could not give up a fence yet, retrying", "helmrelease", client.ObjectKeyFromObject(hr).String(), "error", err.Error())
|
||||
}
|
||||
if stillHolds(ctx, reader, hr) {
|
||||
names = append(names, client.ObjectKeyFromObject(hr).String())
|
||||
}
|
||||
}
|
||||
if held == 0 {
|
||||
left = len(names)
|
||||
if left == 0 {
|
||||
return 0, nil
|
||||
}
|
||||
select {
|
||||
case <-ctx.Done():
|
||||
return held, ctx.Err()
|
||||
return left, stillFenced(ctx.Err(), names)
|
||||
case <-time.After(drainPollInterval):
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// stillHolds reports whether the release still carries the fence after the
|
||||
// drain's step on it. A release that cannot be read, other than one that is
|
||||
// gone, counts as held: naming one too many is safer than naming one too few.
|
||||
func stillHolds(ctx context.Context, reader client.Reader, hr *helmv2.HelmRelease) bool {
|
||||
cur := &helmv2.HelmRelease{}
|
||||
if err := reader.Get(ctx, client.ObjectKeyFromObject(hr), cur); err != nil {
|
||||
return !apierrors.IsNotFound(err)
|
||||
}
|
||||
return controllerutil.ContainsFinalizer(cur, fenceFinalizer)
|
||||
}
|
||||
|
||||
// stillFenced names the releases the last pass left fenced, so the line
|
||||
// reporting a cut-short drain says which releases an operator has to look at.
|
||||
func stillFenced(err error, names []string) error {
|
||||
if len(names) == 0 {
|
||||
return err
|
||||
}
|
||||
return fmt.Errorf("%w; still fenced: %s", err, strings.Join(names, ", "))
|
||||
}
|
||||
|
||||
const (
|
||||
// managerStopTimeout bounds how long the manager may take to stop its
|
||||
// controllers before the drain starts.
|
||||
@@ -140,24 +179,30 @@ type drainingManager interface {
|
||||
// informer cache is frozen and would show fences already gone. Only a
|
||||
// flux-plunger that led drains: one that never did stops while the leader goes
|
||||
// on, and the fences are the leader's. Without leader election Elected is closed
|
||||
// from the start.
|
||||
// from the start. controller-runtime closes Elected only once its
|
||||
// leader-election runnables have started, so a process stopped in between holds
|
||||
// the Lease yet takes the never-led path; it has run no reconcile, so the
|
||||
// fences it skips are ones it inherited, and the next leader takes them over.
|
||||
func RunManager(ctx context.Context, mgr drainingManager, drain time.Duration) error {
|
||||
drainer := &FluxPlunger{
|
||||
Client: mgr.GetClient(),
|
||||
APIReader: mgr.GetAPIReader(),
|
||||
Recorder: apiEventRecorder{c: mgr.GetClient()},
|
||||
}
|
||||
return RunThenDrain(ctx, mgr.Start, func(dctx context.Context) (int, error) {
|
||||
select {
|
||||
case <-mgr.Elected():
|
||||
drainer := &FluxPlunger{
|
||||
Client: mgr.GetClient(),
|
||||
APIReader: mgr.GetAPIReader(),
|
||||
Recorder: newAPIEventRecorder(dctx, mgr.GetClient()),
|
||||
}
|
||||
return drainer.DrainFences(dctx)
|
||||
default:
|
||||
log.FromContext(dctx).Info("Never became leader; leaving the fences to the leader")
|
||||
return 0, nil
|
||||
return 0, errNeverLed
|
||||
}
|
||||
}, drain)
|
||||
}
|
||||
|
||||
// errNeverLed is what the drain of a flux-plunger that never became leader
|
||||
// returns: it drained nothing, and the fences are the leader's.
|
||||
var errNeverLed = errors.New("never became leader; the fences are left to the leader")
|
||||
|
||||
// RunThenDrain runs the manager until it stops, then drains under a deadline of
|
||||
// its own: the context run was given is cancelled by the stop signal, so the drain
|
||||
// cannot be bounded by it. A manager that failed is not drained: a lost Lease
|
||||
@@ -174,9 +219,15 @@ func RunThenDrain(ctx context.Context, run func(context.Context) error, drain fu
|
||||
dctx, cancel := context.WithTimeout(context.Background(), timeout)
|
||||
defer cancel()
|
||||
logger := log.FromContext(ctx)
|
||||
if left, err := drain(dctx); err != nil {
|
||||
logger.Info("Stopped with uninstall fences still held; the next flux-plunger or the chart's pre-delete hook releases them", "held", left, "error", err.Error())
|
||||
} else {
|
||||
left, err := drain(dctx)
|
||||
switch {
|
||||
case errors.Is(err, errNeverLed):
|
||||
logger.Info("Never became leader; leaving the fences to the leader")
|
||||
case err != nil && left < 0:
|
||||
logger.Error(err, "Stopped with no listing of HelmReleases answered; the next flux-plunger or the chart's pre-delete hook releases whatever is fenced")
|
||||
case err != nil:
|
||||
logger.Error(err, "Stopped with uninstall fences still held; the next flux-plunger or the chart's pre-delete hook releases them", "held", left)
|
||||
default:
|
||||
logger.Info("Released every uninstall fence before stopping")
|
||||
}
|
||||
return runErr
|
||||
@@ -184,36 +235,72 @@ func RunThenDrain(ctx context.Context, run func(context.Context) error, drain fu
|
||||
|
||||
// apiEventRecorder writes each Event straight to the apiserver. The drain cannot
|
||||
// use the manager's recorder: the manager stops its recorder provider before
|
||||
// Start returns, after which that recorder delivers nothing.
|
||||
// Start returns, after which that recorder delivers nothing. Like that recorder
|
||||
// it folds repeats of one Event on one object into a single counted Event, since
|
||||
// the drain revisits every fenced release once a second. Every write is bounded
|
||||
// by the drain's context, so none outlives the drain.
|
||||
type apiEventRecorder struct {
|
||||
c client.Client
|
||||
ctx context.Context
|
||||
c client.Client
|
||||
mu sync.Mutex
|
||||
folded map[eventKey]*corev1.Event
|
||||
}
|
||||
|
||||
func (r apiEventRecorder) Event(obj runtime.Object, eventtype, reason, message string) {
|
||||
// eventKey is what makes two Events repeats of one another.
|
||||
type eventKey struct {
|
||||
uid types.UID
|
||||
namespace, name string
|
||||
eventtype, reason, text string
|
||||
}
|
||||
|
||||
// eventWriteTimeout caps one Event write inside the drain's own deadline.
|
||||
const eventWriteTimeout = 5 * time.Second
|
||||
|
||||
func newAPIEventRecorder(ctx context.Context, c client.Client) *apiEventRecorder {
|
||||
return &apiEventRecorder{ctx: ctx, c: c, folded: map[eventKey]*corev1.Event{}}
|
||||
}
|
||||
|
||||
func (r *apiEventRecorder) Event(obj runtime.Object, eventtype, reason, message string) {
|
||||
r.write(obj, eventtype, reason, message)
|
||||
}
|
||||
|
||||
func (r apiEventRecorder) Eventf(obj runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) {
|
||||
func (r *apiEventRecorder) Eventf(obj runtime.Object, eventtype, reason, messageFmt string, args ...interface{}) {
|
||||
r.write(obj, eventtype, reason, fmt.Sprintf(messageFmt, args...))
|
||||
}
|
||||
|
||||
func (r apiEventRecorder) AnnotatedEventf(obj runtime.Object, _ map[string]string, eventtype, reason, messageFmt string, args ...interface{}) {
|
||||
func (r *apiEventRecorder) AnnotatedEventf(obj runtime.Object, _ map[string]string, eventtype, reason, messageFmt string, args ...interface{}) {
|
||||
r.write(obj, eventtype, reason, fmt.Sprintf(messageFmt, args...))
|
||||
}
|
||||
|
||||
func (r apiEventRecorder) write(obj runtime.Object, eventtype, reason, message string) {
|
||||
func (r *apiEventRecorder) write(obj runtime.Object, eventtype, reason, message string) {
|
||||
logger := log.Log.WithName("drain-events")
|
||||
o, ok := obj.(client.Object)
|
||||
if !ok {
|
||||
logger.Info("Cannot record an event for this object", "type", fmt.Sprintf("%T", obj), "reason", reason)
|
||||
return
|
||||
}
|
||||
ctx, cancel := context.WithTimeout(r.ctx, eventWriteTimeout)
|
||||
defer cancel()
|
||||
now := metav1.Now()
|
||||
key := eventKey{uid: o.GetUID(), namespace: o.GetNamespace(), name: o.GetName(), eventtype: eventtype, reason: reason, text: message}
|
||||
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
if ev, seen := r.folded[key]; seen {
|
||||
// A patch, not an update: the chart grants events create and patch only.
|
||||
base := ev.DeepCopy()
|
||||
ev.Count++
|
||||
ev.LastTimestamp = now
|
||||
if err := r.c.Patch(ctx, ev, client.MergeFrom(base)); err != nil {
|
||||
logger.Info("Could not count a repeated event", "reason", reason, "error", err.Error())
|
||||
}
|
||||
return
|
||||
}
|
||||
gvk, err := apiutil.GVKForObject(o, r.c.Scheme())
|
||||
if err != nil {
|
||||
logger.Info("Cannot record an event", "reason", reason, "error", err.Error())
|
||||
return
|
||||
}
|
||||
now := metav1.Now()
|
||||
ev := &corev1.Event{
|
||||
ObjectMeta: metav1.ObjectMeta{GenerateName: o.GetName() + ".", Namespace: o.GetNamespace()},
|
||||
InvolvedObject: corev1.ObjectReference{
|
||||
@@ -231,9 +318,9 @@ func (r apiEventRecorder) write(obj runtime.Object, eventtype, reason, message s
|
||||
LastTimestamp: now,
|
||||
Count: 1,
|
||||
}
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||
defer cancel()
|
||||
if err := r.c.Create(ctx, ev); err != nil {
|
||||
logger.Info("Could not record an event", "reason", reason, "error", err.Error())
|
||||
return
|
||||
}
|
||||
r.folded[key] = ev
|
||||
}
|
||||
@@ -0,0 +1,307 @@
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
|
||||
package fluxplunger
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
helmv2 "github.com/fluxcd/helm-controller/api/v2"
|
||||
"github.com/go-logr/logr"
|
||||
corev1 "k8s.io/api/core/v1"
|
||||
apierrors "k8s.io/apimachinery/pkg/api/errors"
|
||||
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client/fake"
|
||||
"sigs.k8s.io/controller-runtime/pkg/client/interceptor"
|
||||
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
|
||||
"sigs.k8s.io/controller-runtime/pkg/log"
|
||||
)
|
||||
|
||||
// logEntry is one line a captureSink saw; err is set for an error-level line.
|
||||
type logEntry struct {
|
||||
msg string
|
||||
err error
|
||||
kv []interface{}
|
||||
}
|
||||
|
||||
// captureSink records every line logged through it, so a test can tell what an
|
||||
// operator reading the pod's log would see, and at which level.
|
||||
type captureSink struct {
|
||||
mu *sync.Mutex
|
||||
entries *[]logEntry
|
||||
kv []interface{}
|
||||
}
|
||||
|
||||
func newCaptureLogger() (logr.Logger, *[]logEntry) {
|
||||
entries := &[]logEntry{}
|
||||
return logr.New(captureSink{mu: &sync.Mutex{}, entries: entries}), entries
|
||||
}
|
||||
|
||||
func (s captureSink) Init(logr.RuntimeInfo) {}
|
||||
func (s captureSink) Enabled(level int) bool { return true }
|
||||
func (s captureSink) Info(_ int, msg string, kv ...interface{}) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
*s.entries = append(*s.entries, logEntry{msg: msg, kv: append(append([]interface{}{}, s.kv...), kv...)})
|
||||
}
|
||||
func (s captureSink) Error(err error, msg string, kv ...interface{}) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
*s.entries = append(*s.entries, logEntry{msg: msg, err: err, kv: append(append([]interface{}{}, s.kv...), kv...)})
|
||||
}
|
||||
func (s captureSink) WithValues(kv ...interface{}) logr.LogSink {
|
||||
s.kv = append(append([]interface{}{}, s.kv...), kv...)
|
||||
return s
|
||||
}
|
||||
func (s captureSink) WithName(string) logr.LogSink { return s }
|
||||
|
||||
// Fences left behind reach the operator as an error-level line that names the
|
||||
// releases, not an Info line an alert on errors never sees.
|
||||
func TestRunThenDrain_FencesLeftBehindAreAnErrorNamingThem(t *testing.T) {
|
||||
logger, entries := newCaptureLogger()
|
||||
ctx := log.IntoContext(context.Background(), logger)
|
||||
RunThenDrain(ctx,
|
||||
func(context.Context) error { return nil },
|
||||
func(context.Context) (int, error) {
|
||||
return 1, errors.Join(context.DeadlineExceeded, errors.New("still fenced: tenant-root/vm-disk-0"))
|
||||
},
|
||||
time.Minute)
|
||||
for _, e := range *entries {
|
||||
if e.err != nil && strings.Contains(e.err.Error(), "tenant-root/vm-disk-0") {
|
||||
return
|
||||
}
|
||||
}
|
||||
t.Fatalf("no error-level line names the release still fenced: %+v", *entries)
|
||||
}
|
||||
|
||||
// When no listing ever succeeded the drain does not know how many fences are
|
||||
// held; the line must say so instead of reporting a count of -1.
|
||||
func TestRunThenDrain_AnUnansweredListingIsNotACountOfMinusOne(t *testing.T) {
|
||||
logger, entries := newCaptureLogger()
|
||||
ctx := log.IntoContext(context.Background(), logger)
|
||||
RunThenDrain(ctx,
|
||||
func(context.Context) error { return nil },
|
||||
func(context.Context) (int, error) { return -1, context.DeadlineExceeded },
|
||||
time.Minute)
|
||||
found := false
|
||||
for _, e := range *entries {
|
||||
for i := 0; i+1 < len(e.kv); i += 2 {
|
||||
if v, ok := e.kv[i+1].(int); ok && v < 0 {
|
||||
t.Fatalf("a line reports %v=%d: %+v", e.kv[i], v, e)
|
||||
}
|
||||
}
|
||||
if e.err != nil && strings.Contains(e.msg, "no listing") {
|
||||
found = true
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
t.Fatalf("no error-level line says no listing succeeded: %+v", *entries)
|
||||
}
|
||||
}
|
||||
|
||||
// A drain that released everything says so once.
|
||||
func TestRunThenDrain_ACompleteDrainSaysSo(t *testing.T) {
|
||||
logger, entries := newCaptureLogger()
|
||||
ctx := log.IntoContext(context.Background(), logger)
|
||||
RunThenDrain(ctx,
|
||||
func(context.Context) error { return nil },
|
||||
func(context.Context) (int, error) { return 0, nil },
|
||||
time.Minute)
|
||||
for _, e := range *entries {
|
||||
if e.err == nil && strings.Contains(e.msg, "Released every uninstall fence") {
|
||||
return
|
||||
}
|
||||
}
|
||||
t.Fatalf("a complete drain did not say so: %+v", *entries)
|
||||
}
|
||||
|
||||
// A flux-plunger that never led drained nothing, so it must not claim it
|
||||
// released every fence.
|
||||
func TestRunManager_ANeverLedProcessDoesNotClaimItReleasedTheFences(t *testing.T) {
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(fencedLiveRelease()).
|
||||
WithInterceptorFuncs(interceptor.Funcs{Patch: ssaModelApply}).Build()
|
||||
logger, entries := newCaptureLogger()
|
||||
ctx := log.IntoContext(context.Background(), logger)
|
||||
if err := RunManager(ctx, stubManager{client: c, reader: c, elected: make(chan struct{})}, 5*time.Second); err != nil {
|
||||
t.Fatalf("RunManager: %v", err)
|
||||
}
|
||||
for _, e := range *entries {
|
||||
if strings.Contains(e.msg, "Released every uninstall fence") {
|
||||
t.Fatalf("a process that never led claims it released the fences: %+v", *entries)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func fencedLiveReleaseNamed(name string) *helmv2.HelmRelease {
|
||||
return &helmv2.HelmRelease{ObjectMeta: metav1.ObjectMeta{
|
||||
Namespace: testNS, Name: name, Finalizers: []string{fenceFinalizer},
|
||||
}}
|
||||
}
|
||||
|
||||
// patchFailingFor fails every patch of the named release and applies the rest.
|
||||
func patchFailingFor(name string) func(context.Context, client.WithWatch, client.Object, client.Patch, ...client.PatchOption) error {
|
||||
return func(ctx context.Context, cl client.WithWatch, obj client.Object, patch client.Patch, opts ...client.PatchOption) error {
|
||||
if obj.GetName() == name {
|
||||
return errors.New("the object has been modified; please apply your changes to the latest version")
|
||||
}
|
||||
return ssaModelApply(ctx, cl, obj, patch, opts...)
|
||||
}
|
||||
}
|
||||
|
||||
// One release whose fence cannot be given up must not cost the others theirs.
|
||||
func TestDrainFences_AFailingReleaseDoesNotStopTheOthers(t *testing.T) {
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).
|
||||
WithObjects(fencedLiveReleaseNamed("a-fails"), fencedLiveReleaseNamed("b-ok")).
|
||||
WithInterceptorFuncs(interceptor.Funcs{Patch: patchFailingFor("a-fails")}).Build()
|
||||
r := &FluxPlunger{Client: c, APIReader: c}
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 2500*time.Millisecond)
|
||||
defer cancel()
|
||||
left, err := r.DrainFences(ctx)
|
||||
if err == nil || left != 1 {
|
||||
t.Fatalf("want the failing release left fenced and reported, left=%d err=%v", left, err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), testNS+"/a-fails") {
|
||||
t.Errorf("the error does not name the release left fenced: %v", err)
|
||||
}
|
||||
hr := &helmv2.HelmRelease{}
|
||||
if err := c.Get(context.Background(), client.ObjectKey{Namespace: testNS, Name: "b-ok"}, hr); err != nil {
|
||||
t.Fatalf("get: %v", err)
|
||||
}
|
||||
if controllerutil.ContainsFinalizer(hr, fenceFinalizer) {
|
||||
t.Fatal("a release after a failing one kept its fence")
|
||||
}
|
||||
}
|
||||
|
||||
// A drain cut short in the pass that released a fence must not name that
|
||||
// release as still fenced: an operator would go looking at the wrong one.
|
||||
func TestDrainFences_NamesOnlyTheReleasesStillFenced(t *testing.T) {
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).
|
||||
WithObjects(fencedLiveReleaseNamed("a-fails"), fencedLiveReleaseNamed("b-ok")).
|
||||
WithInterceptorFuncs(interceptor.Funcs{Patch: patchFailingFor("a-fails")}).Build()
|
||||
r := &FluxPlunger{Client: c, APIReader: c}
|
||||
// Shorter than the pause between passes: the drain ends in its first pass.
|
||||
ctx, cancel := context.WithTimeout(context.Background(), drainPollInterval/2)
|
||||
defer cancel()
|
||||
left, err := r.DrainFences(ctx)
|
||||
if err == nil {
|
||||
t.Fatal("want the drain cut short")
|
||||
}
|
||||
if strings.Contains(err.Error(), "b-ok") {
|
||||
t.Errorf("the error names a release whose fence this pass released: %v", err)
|
||||
}
|
||||
if !strings.Contains(err.Error(), testNS+"/a-fails") || left != 1 {
|
||||
t.Errorf("want only a-fails named and counted, left=%d err=%v", left, err)
|
||||
}
|
||||
}
|
||||
|
||||
// The drain polls; without a pause between passes it would list every
|
||||
// HelmRelease in the cluster as fast as the apiserver answers.
|
||||
func TestDrainFences_PausesBetweenPasses(t *testing.T) {
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(fencedLiveReleaseNamed("a-fails")).
|
||||
WithInterceptorFuncs(interceptor.Funcs{Patch: patchFailingFor("a-fails")}).Build()
|
||||
reader := &countingReader{Reader: c}
|
||||
r := &FluxPlunger{Client: c, APIReader: reader}
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 2500*time.Millisecond)
|
||||
defer cancel()
|
||||
_, _ = r.DrainFences(ctx)
|
||||
if reader.lists > 5 {
|
||||
t.Fatalf("%d listings in 2.5s; the drain must pause %v between passes", reader.lists, drainPollInterval)
|
||||
}
|
||||
}
|
||||
|
||||
// Repeats of one Event on one object fold into a single counted Event, as the
|
||||
// manager's recorder would fold them, instead of one object per drain pass.
|
||||
func TestAPIEventRecorder_FoldsRepeats(t *testing.T) {
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).Build()
|
||||
rec := newAPIEventRecorder(context.Background(), c)
|
||||
hr := fencedLiveRelease()
|
||||
for i := 0; i < 3; i++ {
|
||||
rec.Event(hr, corev1.EventTypeWarning, "TerminatingWhileSuspended", "same message")
|
||||
}
|
||||
rec.Event(hr, corev1.EventTypeNormal, "SuspensionNotOwned", "another message")
|
||||
events := &corev1.EventList{}
|
||||
if err := c.List(context.Background(), events); err != nil {
|
||||
t.Fatalf("list events: %v", err)
|
||||
}
|
||||
if len(events.Items) != 2 {
|
||||
t.Fatalf("want 2 Events (one folded, one distinct), got %d: %+v", len(events.Items), events.Items)
|
||||
}
|
||||
for _, e := range events.Items {
|
||||
want := int32(1)
|
||||
if e.Reason == "TerminatingWhileSuspended" {
|
||||
want = 3
|
||||
}
|
||||
if e.Count != want {
|
||||
t.Errorf("%s: count %d, want %d", e.Reason, e.Count, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The chart's ClusterRole grants events create and patch, not update; a fold
|
||||
// that updates the Event is refused in a real cluster and the count stays 1.
|
||||
func TestAPIEventRecorder_FoldsWithTheVerbsTheChartGrants(t *testing.T) {
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).WithInterceptorFuncs(interceptor.Funcs{
|
||||
Update: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.UpdateOption) error {
|
||||
if _, ok := obj.(*corev1.Event); ok {
|
||||
return apierrors.NewForbidden(corev1.Resource("events"), obj.GetName(), errors.New("update is not granted"))
|
||||
}
|
||||
return cl.Update(ctx, obj, opts...)
|
||||
},
|
||||
}).Build()
|
||||
rec := newAPIEventRecorder(context.Background(), c)
|
||||
for i := 0; i < 3; i++ {
|
||||
rec.Event(fencedLiveRelease(), corev1.EventTypeWarning, "TerminatingWhileSuspended", "same message")
|
||||
}
|
||||
events := &corev1.EventList{}
|
||||
if err := c.List(context.Background(), events); err != nil {
|
||||
t.Fatalf("list events: %v", err)
|
||||
}
|
||||
if len(events.Items) != 1 || events.Items[0].Count != 3 {
|
||||
t.Fatalf("want one Event counted 3 without the update verb, got %+v", events.Items)
|
||||
}
|
||||
}
|
||||
|
||||
// Each write is capped well inside the shortest drain the chart allows (15s),
|
||||
// so one slow write cannot take the rest of the drain with it. The bound is
|
||||
// spelled out here rather than read from eventWriteTimeout, so raising the cap
|
||||
// fails this test instead of moving it.
|
||||
func TestAPIEventRecorder_CapsEachWrite(t *testing.T) {
|
||||
var got time.Time
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).WithInterceptorFuncs(interceptor.Funcs{
|
||||
Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
|
||||
got, _ = ctx.Deadline()
|
||||
return cl.Create(ctx, obj, opts...)
|
||||
},
|
||||
}).Build()
|
||||
parent, cancel := context.WithTimeout(context.Background(), time.Hour)
|
||||
defer cancel()
|
||||
start := time.Now()
|
||||
newAPIEventRecorder(parent, c).Event(fencedLiveRelease(), corev1.EventTypeWarning, "TerminatingWhileSuspended", "m")
|
||||
if got.IsZero() || got.Sub(start) > 10*time.Second {
|
||||
t.Fatalf("event write deadline %v after start, want at most 10s", got.Sub(start))
|
||||
}
|
||||
}
|
||||
|
||||
// An Event write must not outlive the drain: the pod is killed when the grace
|
||||
// period ends, whatever is in flight.
|
||||
func TestAPIEventRecorder_WritesInsideTheDrainsDeadline(t *testing.T) {
|
||||
var got time.Time
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).WithInterceptorFuncs(interceptor.Funcs{
|
||||
Create: func(ctx context.Context, cl client.WithWatch, obj client.Object, opts ...client.CreateOption) error {
|
||||
got, _ = ctx.Deadline()
|
||||
return cl.Create(ctx, obj, opts...)
|
||||
},
|
||||
}).Build()
|
||||
parent, cancel := context.WithTimeout(context.Background(), time.Second)
|
||||
defer cancel()
|
||||
want, _ := parent.Deadline()
|
||||
newAPIEventRecorder(parent, c).Event(fencedLiveRelease(), corev1.EventTypeWarning, "TerminatingWhileSuspended", "m")
|
||||
if got.IsZero() || got.After(want) {
|
||||
t.Fatalf("event write deadline %v is not inside the drain's %v", got, want)
|
||||
}
|
||||
}
|
||||
@@ -180,17 +180,24 @@ func (r *FluxPlunger) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Res
|
||||
// the holder of the leader Lease reconciles, a stopping instance keeps that
|
||||
// Lease through its drain (ApplyShutdownTimings), and controller-runtime
|
||||
// serialises reconciles per key within the one process, so there is no
|
||||
// concurrent owner to race. Releasing it un-sticks the release and lets
|
||||
// error-recovery start over. A suspension flux-plunger does not own belongs to
|
||||
// an operator or another controller and must be left untouched.
|
||||
// concurrent owner to race. The one exception is a stop that overran the
|
||||
// manager's grace period: Start then returns without joining its workers,
|
||||
// and a reconcile still in flight can set a fence after the drain's last
|
||||
// listing, which the next leader or the pre-delete hook then releases.
|
||||
// Releasing it un-sticks the release and lets error-recovery start over. A
|
||||
// suspension flux-plunger does not own belongs to an operator or another
|
||||
// controller and must be left untouched.
|
||||
if !r.ownsSuspension(hr) {
|
||||
logger.Info("HelmRelease is suspended by external process, skipping")
|
||||
// On a release flux-plunger recovered before, the suspension is most
|
||||
// likely one an earlier flux-plunger left mid-recovery under the shared
|
||||
// flux-client-side-apply manager, which no one will lift unless told.
|
||||
if _, recovered := hr.Annotations[annotationLastProcessedVersion]; recovered {
|
||||
// A release frozen in the failure recovery starts from is most likely
|
||||
// one an earlier flux-plunger left suspended mid-recovery under the
|
||||
// shared flux-client-side-apply manager, which no one will lift unless
|
||||
// told: helm-controller rewrites no condition of a suspended release.
|
||||
// The processed-version annotation is no sign of it, since it stays on
|
||||
// every release flux-plunger ever recovered.
|
||||
if hasNoDeployedReleasesError(hr) {
|
||||
r.recordEvent(hr, corev1.EventTypeWarning, "SuspensionNotOwned",
|
||||
"Left a suspension flux-plunger does not own untouched on a release it has recovered before; if an earlier flux-plunger left it, resume the release (see the flux-plunger chart README)")
|
||||
"Left a suspension flux-plunger does not own untouched on a release stuck with no deployed releases; if an earlier flux-plunger left it mid-recovery, resume the release (see the flux-plunger chart README)")
|
||||
} else {
|
||||
r.recordEvent(hr, corev1.EventTypeNormal, "SuspensionNotOwned",
|
||||
"Left an externally-owned suspension untouched; flux-plunger only clears suspensions it set")
|
||||
@@ -406,13 +413,14 @@ func (r *FluxPlunger) reconcileTerminating(ctx context.Context, hr *helmv2.HelmR
|
||||
// Another manager's suspension: helm-controller skips the uninstall for it
|
||||
// and leaves the release's resources in place, which a migration that
|
||||
// suspends then deletes relies on. That is not flux-plunger's decision to
|
||||
// override, and not a release its fence should hold. A release that
|
||||
// carries flux-plunger's processed-version annotation was recovered before,
|
||||
// so the suspension that skips its uninstall was not a migration's choice (an
|
||||
// operator's hand, or one an earlier flux-plunger release left behind): that
|
||||
// is the orphaning this controller exists to prevent, and a Warning.
|
||||
// override, and not a release its fence should hold. A release frozen in
|
||||
// the failure recovery starts from is most likely suspended by an earlier
|
||||
// flux-plunger that stopped mid-recovery, not by a migration's choice: that
|
||||
// is the orphaning this controller exists to prevent, and a Warning. The
|
||||
// processed-version annotation is no sign of it, since it stays on every
|
||||
// release flux-plunger ever recovered.
|
||||
logger.Info("HelmRelease is being deleted while suspended by another manager; helm-controller skips the uninstall")
|
||||
if _, recovered := hr.Annotations[annotationLastProcessedVersion]; recovered {
|
||||
if hasNoDeployedReleasesError(hr) {
|
||||
r.recordEvent(hr, corev1.EventTypeWarning, "TerminatingWhileSuspended",
|
||||
"HelmRelease is being deleted while suspended by another manager; helm-controller skips the Helm uninstall for a suspended release, so its resources are left behind")
|
||||
} else {
|
||||
|
||||
@@ -1193,18 +1193,26 @@ func TestReconcile_EmitsEventOnExternalSuspension(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A suspension flux-plunger does not own on a release flux-plunger has recovered
|
||||
// before is most likely one an earlier flux-plunger left mid-recovery under the
|
||||
// shared field manager: it stays suspended for good unless someone acts, so it
|
||||
// is a Warning, while one on a release flux-plunger never touched stays Normal.
|
||||
func TestReconcile_ExternalSuspensionOnARecoveredReleaseWarns(t *testing.T) {
|
||||
// A suspension flux-plunger does not own is most likely one an earlier
|
||||
// flux-plunger left mid-recovery, under the shared field manager, when the
|
||||
// release is frozen in the failure recovery starts from: helm-controller
|
||||
// rewrites no condition of a suspended release, so a release suspended
|
||||
// mid-recovery keeps "has no deployed releases" for good unless someone acts,
|
||||
// and that is a Warning. The processed-version annotation is no sign of it: it
|
||||
// stays on every release flux-plunger ever recovered, so a later restore,
|
||||
// migration or operator suspension of a healthy release would be told to resume
|
||||
// it, and a first recovery left mid-flight had not written it yet.
|
||||
func TestReconcile_ExternalSuspensionOnAStuckReleaseWarns(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
recovered bool
|
||||
stuck bool
|
||||
want string
|
||||
}{
|
||||
{"recovered before", true, corev1.EventTypeWarning},
|
||||
{"never recovered", false, corev1.EventTypeNormal},
|
||||
{"stuck, recovered before", true, true, corev1.EventTypeWarning},
|
||||
{"stuck, first recovery", false, true, corev1.EventTypeWarning},
|
||||
{"healthy, recovered before", true, false, corev1.EventTypeNormal},
|
||||
{"healthy, never recovered", false, false, corev1.EventTypeNormal},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
hr := noDeployedReleasesHR()
|
||||
@@ -1212,6 +1220,9 @@ func TestReconcile_ExternalSuspensionOnARecoveredReleaseWarns(t *testing.T) {
|
||||
if tc.recovered {
|
||||
hr.Annotations = map[string]string{annotationLastProcessedVersion: "3"}
|
||||
}
|
||||
if !tc.stuck {
|
||||
hr.Status.Conditions = []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue, Reason: "InstallSucceeded", Message: "Helm install succeeded"}}
|
||||
}
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(hr).
|
||||
WithInterceptorFuncs(interceptor.Funcs{Patch: ssaModelApply}).Build()
|
||||
rec := record.NewFakeRecorder(8)
|
||||
@@ -1750,6 +1761,7 @@ func TestReconcile_LiftsFenceOnlyOnceUninstallReached(t *testing.T) {
|
||||
func TestReconcile_TerminatingForeignSuspensionReleasesFence(t *testing.T) {
|
||||
hr := terminatingFixture(fencedFixture(noDeployedReleasesHR()))
|
||||
hr.Spec.Suspend = true // not flux-plunger's
|
||||
hr.Status.Conditions = []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue, Reason: "UpgradeSucceeded", Message: "Helm upgrade succeeded"}}
|
||||
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(hr).
|
||||
WithInterceptorFuncs(interceptor.Funcs{Patch: ssaModelApply}).Build()
|
||||
@@ -1778,6 +1790,52 @@ func TestReconcile_TerminatingForeignSuspensionReleasesFence(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A release deleted under a suspension flux-plunger does not own loses its
|
||||
// resources either way; that is an accident, and a Warning, when the suspension
|
||||
// is most likely one an earlier flux-plunger left mid-recovery, which freezes
|
||||
// the release in the failure recovery starts from. A migration that suspends
|
||||
// then deletes a healthy release meant it, whatever annotation an earlier
|
||||
// recovery left on it.
|
||||
func TestReconcile_TerminatingForeignSuspensionWarnsOnlyOnAStuckRelease(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
recovered bool
|
||||
stuck bool
|
||||
want string
|
||||
}{
|
||||
{"stuck, recovered before", true, true, corev1.EventTypeWarning},
|
||||
{"stuck, first recovery", false, true, corev1.EventTypeWarning},
|
||||
{"healthy, recovered before", true, false, corev1.EventTypeNormal},
|
||||
{"healthy, never recovered", false, false, corev1.EventTypeNormal},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
hr := terminatingFixture(fencedFixture(noDeployedReleasesHR()))
|
||||
hr.Spec.Suspend = true
|
||||
if tc.recovered {
|
||||
hr.Annotations = map[string]string{annotationLastProcessedVersion: "3"}
|
||||
}
|
||||
if !tc.stuck {
|
||||
hr.Status.Conditions = []metav1.Condition{{Type: "Ready", Status: metav1.ConditionTrue, Reason: "UpgradeSucceeded", Message: "Helm upgrade succeeded"}}
|
||||
}
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(hr).
|
||||
WithInterceptorFuncs(interceptor.Funcs{Patch: ssaModelApply}).Build()
|
||||
rec := record.NewFakeRecorder(8)
|
||||
r := &FluxPlunger{Client: c, owns: testOwns, Recorder: rec}
|
||||
|
||||
reconcile(t, r)
|
||||
|
||||
select {
|
||||
case e := <-rec.Events:
|
||||
if !strings.HasPrefix(e, tc.want+" TerminatingWhileSuspended ") {
|
||||
t.Fatalf("want a %s TerminatingWhileSuspended event, got %q", tc.want, e)
|
||||
}
|
||||
default:
|
||||
t.Fatal("no event")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A fenced, terminating, unsuspended release that helm-controller has not handled
|
||||
// yet is held by flux-plunger's fence. That hold must not be silent: the reconcile
|
||||
// requeues itself so it is re-checked without waiting for an event, and once the
|
||||
@@ -1835,48 +1893,6 @@ func TestReconcile_HeldFenceRequeuesAndWarnsPastTheDeadline(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// The same terminating-under-a-foreign-suspension state is two different things.
|
||||
// A migration that suspends then deletes chose to keep the resources, so it is
|
||||
// Normal. A release flux-plunger has recovered before (it carries the
|
||||
// processed-version annotation: an operator suspended it by hand and deleted it,
|
||||
// or an earlier flux-plunger release left it suspended) is losing its uninstall by
|
||||
// accident, the failure this controller exists to prevent, so it is a Warning.
|
||||
func TestReconcile_TerminatingForeignSuspensionWarnsOnlyForAPreviouslyRecoveredRelease(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
annotated bool
|
||||
wantType string
|
||||
}{
|
||||
{"never recovered by flux-plunger (migration)", false, corev1.EventTypeNormal},
|
||||
{"recovered by flux-plunger before", true, corev1.EventTypeWarning},
|
||||
}
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
hr := terminatingFixture(noDeployedReleasesHR())
|
||||
hr.Spec.Suspend = true // not flux-plunger's
|
||||
if tc.annotated {
|
||||
hr.Annotations = map[string]string{annotationLastProcessedVersion: "3"}
|
||||
}
|
||||
|
||||
c := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(hr).
|
||||
WithInterceptorFuncs(interceptor.Funcs{Patch: ssaModelApply}).Build()
|
||||
rec := record.NewFakeRecorder(8)
|
||||
r := &FluxPlunger{Client: c, owns: testOwns, Recorder: rec}
|
||||
|
||||
reconcile(t, r)
|
||||
|
||||
select {
|
||||
case e := <-rec.Events:
|
||||
if !strings.HasPrefix(e, tc.wantType+" TerminatingWhileSuspended ") {
|
||||
t.Fatalf("expected a %s TerminatingWhileSuspended event, got %q", tc.wantType, e)
|
||||
}
|
||||
default:
|
||||
t.Fatalf("expected a TerminatingWhileSuspended event, got none")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// A fence left without a suspension on a live release (another writer set
|
||||
// spec.suspend=false and took the field), or a fence whose suspension another
|
||||
// manager took over, guards nothing and must be lifted, without starting a recovery.
|
||||
|
||||
@@ -2,11 +2,14 @@ package fluxplunger
|
||||
|
||||
import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
helmv2 "github.com/fluxcd/helm-controller/api/v2"
|
||||
)
|
||||
|
||||
// The chart's pre-delete hook restates two of this package's names in shell and
|
||||
@@ -68,6 +71,80 @@ func TestDeploymentTemplate_SubtractsTheStopAndTheMarginTheBinaryUses(t *testing
|
||||
}
|
||||
}
|
||||
|
||||
// The binary must refuse a drain deadline it could not drain under, whatever
|
||||
// the chart or a hand-written Deployment passes, and before it reaches for a
|
||||
// cluster: the run has no kubeconfig, so a binary that skipped the check fails
|
||||
// on that instead, with another message.
|
||||
func TestMain_RefusesADrainTimeoutThatLeavesNoTime(t *testing.T) {
|
||||
bin := t.TempDir() + "/flux-plunger"
|
||||
if out, err := exec.Command("go", "build", "-o", bin, "../../../cmd/flux-plunger").CombinedOutput(); err != nil {
|
||||
t.Fatalf("build flux-plunger: %v\n%s", err, out)
|
||||
}
|
||||
cmd := exec.Command(bin, "--"+ShutdownDrainTimeoutFlag+"=0s")
|
||||
cmd.Env = append(os.Environ(), "KUBECONFIG=/dev/null", "KUBERNETES_SERVICE_HOST=")
|
||||
out, err := cmd.CombinedOutput()
|
||||
if err == nil {
|
||||
t.Fatalf("flux-plunger started with a drain deadline of 0s:\n%s", out)
|
||||
}
|
||||
if want := "--" + ShutdownDrainTimeoutFlag + " must be positive"; !strings.Contains(string(out), want) {
|
||||
t.Fatalf("flux-plunger did not refuse the deadline itself, want %q in:\n%s", want, out)
|
||||
}
|
||||
}
|
||||
|
||||
// The chart's lowest accepted grace period has to leave the drain time after the
|
||||
// manager's stop and the margin before SIGKILL, or raising either constant makes
|
||||
// the chart render a drain deadline of zero or less at its own minimum.
|
||||
func TestDeploymentTemplate_TheLowestGracePeriodLeavesTheDrainTime(t *testing.T) {
|
||||
raw, err := os.ReadFile(deploymentTemplate)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", deploymentTemplate, err)
|
||||
}
|
||||
m := regexp.MustCompile(`\(lt \$grace (\d+)\)`).FindStringSubmatch(string(raw))
|
||||
if m == nil {
|
||||
t.Fatal("deployment has no lower bound on shutdownGracePeriodSeconds")
|
||||
}
|
||||
floor, err := strconv.Atoi(m[1])
|
||||
if err != nil {
|
||||
t.Fatalf("lower bound %q: %v", m[1], err)
|
||||
}
|
||||
if drain := time.Duration(floor)*time.Second - managerStopTimeout - killMargin; drain <= 0 {
|
||||
t.Fatalf("at the lowest grace period %ds the drain gets %v (manager stop %v, margin %v)", floor, drain, managerStopTimeout, killMargin)
|
||||
}
|
||||
if want := strconv.Itoa(int(managerStopTimeout/time.Second)) + "s stop"; !strings.Contains(string(raw), want) {
|
||||
t.Errorf("the range message does not name the manager's %q", want)
|
||||
}
|
||||
}
|
||||
|
||||
// The README and the values comment restate the drain's timings for operators;
|
||||
// they must name the numbers the binary and the chart use, or a change to either
|
||||
// leaves the prose stating the old ones.
|
||||
func TestDocs_StateTheTimingsTheBinaryAndTheChartUse(t *testing.T) {
|
||||
raw, err := os.ReadFile(deploymentTemplate)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", deploymentTemplate, err)
|
||||
}
|
||||
m := regexp.MustCompile(`\(lt \$grace (\d+)\) \(gt \$grace (\d+)\)`).FindStringSubmatch(string(raw))
|
||||
if m == nil {
|
||||
t.Fatal("deployment has no range on shutdownGracePeriodSeconds")
|
||||
}
|
||||
want := []string{
|
||||
"between " + m[1] + " and " + m[2],
|
||||
strconv.Itoa(int(managerStopTimeout/time.Second)) + " seconds",
|
||||
strconv.Itoa(int((managerStopTimeout+killMargin)/time.Second)) + " seconds",
|
||||
}
|
||||
for _, doc := range []string{"../../../packages/system/flux-plunger/README.md", "../../../packages/system/flux-plunger/values.yaml"} {
|
||||
text, err := os.ReadFile(doc)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", doc, err)
|
||||
}
|
||||
for _, w := range want {
|
||||
if !strings.Contains(string(text), w) {
|
||||
t.Errorf("%s does not state %q", doc, w)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The chart passes the drain deadline as a flag; a name the binary does not
|
||||
// define makes flux-plunger exit on start.
|
||||
func TestDeploymentTemplate_PassesTheDrainFlagTheBinaryDefines(t *testing.T) {
|
||||
@@ -109,3 +186,28 @@ func TestHookTemplate_UsesTheControllersNames(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The hook tells a release helm-controller has stopped holding by
|
||||
// helm-controller's own finalizer. A rename on the flux side that the hook did
|
||||
// not follow would read every release being deleted as handled, and the hook
|
||||
// would unfence it without waiting for the uninstall.
|
||||
func TestHookTemplate_UsesHelmControllersFinalizer(t *testing.T) {
|
||||
raw, err := os.ReadFile(hookTemplate)
|
||||
if err != nil {
|
||||
t.Fatalf("read %s: %v", hookTemplate, err)
|
||||
}
|
||||
hook := string(raw)
|
||||
if want := `"` + helmv2.HelmReleaseFinalizer + `"`; !strings.Contains(hook, want) {
|
||||
t.Fatalf("hook does not name helm-controller's finalizer %s", want)
|
||||
}
|
||||
// Every other fluxcd.io literal in quotes, the RBAC API group aside, is that
|
||||
// finalizer, whole.
|
||||
for _, l := range regexp.MustCompile(`"[^"\s]*fluxcd\.io[^"\s]*"`).FindAllString(hook, -1) {
|
||||
if l == `"`+helmv2.GroupVersion.Group+`"` {
|
||||
continue
|
||||
}
|
||||
if l != `"`+helmv2.HelmReleaseFinalizer+`"` {
|
||||
t.Errorf("hook template names %s, helm-controller's finalizer is %q", l, helmv2.HelmReleaseFinalizer)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -177,6 +177,21 @@ func TestApplyShutdownTimings_TheLeaseOutlastsTheStopAndTheDrain(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A drain deadline that is not positive is already spent when the drain starts,
|
||||
// so every fence would be left behind without one listing.
|
||||
func TestValidateShutdownDrainTimeout_RefusesADeadlineThatLeavesNoTime(t *testing.T) {
|
||||
for _, d := range []time.Duration{0, -5 * time.Second} {
|
||||
if err := ValidateShutdownDrainTimeout(d); err == nil {
|
||||
t.Errorf("drain timeout %v accepted", d)
|
||||
}
|
||||
}
|
||||
for _, d := range []time.Duration{time.Second, 65 * time.Second} {
|
||||
if err := ValidateShutdownDrainTimeout(d); err != nil {
|
||||
t.Errorf("drain timeout %v refused: %v", d, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// controller-runtime reports a stop that overran GracefulShutdownTimeout as an
|
||||
// error from Start. A reconcile slow to notice its cancelled context must not
|
||||
// cost the drain the fence exists for; the exit stays non-zero.
|
||||
|
||||
@@ -1,12 +1,12 @@
|
||||
# flux-plunger
|
||||
|
||||
Recovers a HelmRelease stuck with `has no deployed releases`: it suspends the release, deletes the stale Helm storage Secret, records the processed revision in the `flux-plunger.cozystack.io/last-processed-version` annotation, and unsuspends.
|
||||
Recovers a HelmRelease stuck with `has no deployed releases`: it suspends the release, records the revision it is about to reclaim in the `flux-plunger.cozystack.io/last-processed-version` annotation, deletes that stale Helm storage Secret, and unsuspends. The annotation goes first so that a write which fails never lets a later pass delete the next revision; a release whose annotation names a revision whose Secret is still there is one whose delete failed and will be retried.
|
||||
|
||||
## The uninstall fence
|
||||
|
||||
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. The hook stops, waits for and lists again only for the flux-plunger of this release, found by its release's labels in the release's namespace. Another one, a second installation under another release name, a pod whose labels were changed, or a binary run outside the cluster, is not stopped by the hook: it keeps the fences it sets and gives them up itself when it stops, and a fence it sets after the hook's listing is its own to release. 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:
|
||||
|
||||
@@ -20,12 +20,16 @@ Fenced releases can be listed with:
|
||||
kubectl get helmreleases.helm.toolkit.fluxcd.io --all-namespaces --output go-template='{{range .items}}{{$ns := .metadata.namespace}}{{$name := .metadata.name}}{{range .metadata.finalizers}}{{if eq . "flux-plunger.cozystack.io/uninstall-fence"}}{{$ns}}/{{$name}}{{"\n"}}{{end}}{{end}}{{end}}'
|
||||
```
|
||||
|
||||
## A restore during a recovery
|
||||
|
||||
A backup restore that starts while flux-plunger holds its own suspension of the release does not take `spec.suspend` over: the etcd, CNPG and Velero restore steps write the field only when its value differs, and it is already `true`. flux-plunger still owns the suspension, so when its recovery ends it sets `spec.suspend` back to `false` under the running restore, and helm-controller reconciles the release mid-restore. The window is the few API calls between flux-plunger acquiring the suspension and relinquishing it. If a restore misbehaves right after a recovery, suspend the release again by hand for the rest of the restore.
|
||||
|
||||
## Suspensions left by an earlier flux-plunger
|
||||
|
||||
Earlier flux-plunger releases set `spec.suspend` through the shared `flux-client-side-apply` manager, so a suspension one of them left behind cannot be told apart from a migration's or an operator's and is treated as external: it is not cleared automatically. Such a leftover exists only if the old pod stopped in the middle of a recovery, and flux-plunger raises a `SuspensionNotOwned` Warning on it. To list candidates, suspended releases carrying the flux-plunger annotation whose `spec.suspend` is not owned by flux-plunger's apply:
|
||||
Earlier flux-plunger releases set `spec.suspend` through the shared `flux-client-side-apply` manager, so a suspension one of them left behind cannot be told apart from a migration's or an operator's and is treated as external: it is not cleared automatically. Such a leftover exists only if the old pod stopped in the middle of a recovery, and since helm-controller rewrites no condition of a suspended release, it stays frozen in the failure recovery started from: `Ready=False` with `has no deployed releases`. flux-plunger raises a `SuspensionNotOwned` Warning on a release in that state, and a Normal event on any other suspension it does not own; the processed-version annotation is no sign of a leftover, since it stays on every release flux-plunger ever recovered. To list candidates, suspended releases in that state whose `spec.suspend` is not owned by flux-plunger's apply:
|
||||
|
||||
```bash
|
||||
kubectl get helmreleases.helm.toolkit.fluxcd.io --all-namespaces --output json | jq --raw-output '.items[] | select(.spec.suspend == true) | select(.metadata.annotations["flux-plunger.cozystack.io/last-processed-version"] != null) | select([.metadata.managedFields[]? | select(.manager == "flux-plunger" and .operation == "Apply" and .fieldsV1["f:spec"]["f:suspend"] != null)] | length == 0) | "\(.metadata.namespace)/\(.metadata.name)"'
|
||||
kubectl get helmreleases.helm.toolkit.fluxcd.io --all-namespaces --output json | jq --raw-output '.items[] | select(.spec.suspend == true) | select([.status.conditions[]? | select(.type == "Ready" and .status == "False" and (.message // "" | contains("has no deployed releases")))] | length > 0) | select([.metadata.managedFields[]? | select(.manager == "flux-plunger" and .operation == "Apply" and .fieldsV1["f:spec"]["f:suspend"] != null)] | length == 0) | "\(.metadata.namespace)/\(.metadata.name)"'
|
||||
```
|
||||
|
||||
Clearing one is an operator action: confirm nothing else (a migration, a restore) is holding it, then resume it with `flux resume helmrelease NAME --namespace NAMESPACE`.
|
||||
@@ -22,12 +22,12 @@ spec:
|
||||
rejected on upgrade.
|
||||
*/}}
|
||||
{{- $grace := int .Values.shutdownGracePeriodSeconds }}
|
||||
{{- if ne (toString .Values.shutdownGracePeriodSeconds) (toString $grace) }}
|
||||
{{- fail "shutdownGracePeriodSeconds must be a whole number of seconds: terminationGracePeriodSeconds takes an integer" }}
|
||||
{{- end }}
|
||||
{{- if or (lt $grace 40) (gt $grace 140) }}
|
||||
{{- fail "shutdownGracePeriodSeconds must be between 40 and 140: below, the drain gets no time after the manager's 10s stop; above, the pre-delete hook cannot wait the pod out inside its deadline" }}
|
||||
{{- end }}
|
||||
{{- if ne (toString .Values.shutdownGracePeriodSeconds) (toString $grace) }}
|
||||
{{- fail "shutdownGracePeriodSeconds must be a whole number of seconds: terminationGracePeriodSeconds takes an integer" }}
|
||||
{{- end }}
|
||||
replicas: 1
|
||||
strategy:
|
||||
type: RollingUpdate
|
||||
|
||||
@@ -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
|
||||
@@ -183,11 +185,23 @@ spec:
|
||||
scope="${HR_SCOPE:---all-namespaces}"
|
||||
# The pod is given its grace period, and a little more, to drain.
|
||||
pod_wait={{ $podWait }}
|
||||
# Retries stop here, a minute short of activeDeadlineSeconds, which
|
||||
# counts from the Job's start, so the report is still written when
|
||||
# scheduling and the image pull took part of it. The pod wait is cut
|
||||
# to what is left of this window too.
|
||||
# Retries stop here, a minute short of activeDeadlineSeconds. That
|
||||
# deadline counts from the Job's start and this window from the
|
||||
# script's, so scheduling and the image pull come out of the minute:
|
||||
# one that takes longer gets the Job killed before its report, which
|
||||
# fails the uninstall for helm-controller to retry. The pod wait is
|
||||
# cut to what is left of this window too.
|
||||
retry_until=$(( $(date +%s) + {{ $retryWindow }} ))
|
||||
# Retries stop at $stop_at: the whole window for the steps that cover
|
||||
# every release, and a share of it for each release, so one that
|
||||
# can never be read or unfenced does not leave the releases listed
|
||||
# after it a single attempt.
|
||||
stop_at=$retry_until
|
||||
per_release=30
|
||||
begin_release() {
|
||||
stop_at=$(( $(date +%s) + per_release ))
|
||||
[ "$stop_at" -le "$retry_until" ] || stop_at=$retry_until
|
||||
}
|
||||
# Each fenced release once, however many copies of the fence it
|
||||
# carries: the apiserver accepts a finalizer listed twice.
|
||||
listing='{{`{{range .items}}{{$fenced := false}}{{range .metadata.finalizers}}{{if eq . "flux-plunger.cozystack.io/uninstall-fence"}}{{$fenced = true}}{{end}}{{end}}{{if $fenced}}{{.metadata.namespace}}/{{.metadata.name}}{{"\n"}}{{end}}{{end}}`}}'
|
||||
@@ -207,7 +221,7 @@ spec:
|
||||
printf '%s\n' "$out"
|
||||
return 0
|
||||
fi
|
||||
[ "$(date +%s)" -lt "$retry_until" ] || return 1
|
||||
[ "$(date +%s)" -lt "$stop_at" ] || return 1
|
||||
sleep 2
|
||||
done
|
||||
}
|
||||
@@ -243,7 +257,7 @@ spec:
|
||||
--patch "[{\"op\":\"test\",\"path\":\"/metadata/resourceVersion\",\"value\":\"$ls_rv\"},{\"op\":\"test\",\"path\":\"/metadata/uid\",\"value\":\"$ls_uid\"},{\"op\":\"replace\",\"path\":\"/spec/suspend\",\"value\":false}]"; then
|
||||
return 0
|
||||
fi
|
||||
[ "$(date +%s)" -lt "$retry_until" ] || return 1
|
||||
[ "$(date +%s)" -lt "$stop_at" ] || return 1
|
||||
sleep 2
|
||||
done
|
||||
}
|
||||
@@ -272,7 +286,7 @@ spec:
|
||||
fi ;;
|
||||
esac
|
||||
fi
|
||||
[ "$(date +%s)" -lt "$retry_until" ] || return 1
|
||||
[ "$(date +%s)" -lt "$stop_at" ] || return 1
|
||||
sleep 2
|
||||
done
|
||||
}
|
||||
@@ -290,6 +304,10 @@ spec:
|
||||
fi
|
||||
if [ -z "$present" ]; then
|
||||
echo "no $deploy Deployment in $NAMESPACE; nothing to stop"
|
||||
# kubectl scale retries a conflict for up to five minutes on its own,
|
||||
# but without --current-replicas or --resource-version it sends a
|
||||
# merge patch of the scale subresource, which carries no
|
||||
# resourceVersion to conflict on, so that retry never runs here.
|
||||
elif ! retry kubectl --namespace "$NAMESPACE" scale deployment "$deploy" --replicas=0; then
|
||||
echo "could not scale $deploy down; no fence was touched" >&2
|
||||
exit 1
|
||||
@@ -327,20 +345,59 @@ 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 not flux-plunger'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. Fails
|
||||
# when the release cannot be read.
|
||||
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}}`}}'
|
||||
}
|
||||
|
||||
# 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: it is still suspended, not by flux-plunger" ;;
|
||||
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%%/*}
|
||||
name=${ref#*/}
|
||||
begin_release
|
||||
lift_suspension "$hns" "$name"
|
||||
case $? in
|
||||
2)
|
||||
@@ -352,22 +409,41 @@ 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
|
||||
continue
|
||||
fi
|
||||
released=$((released + 1))
|
||||
# A release whose state cannot be read waits like one helm-controller
|
||||
# has not handled: unfencing it on a failed read could delete it
|
||||
# before its uninstall.
|
||||
state=$(retry read_state "$hns" "$name") || state=unknown
|
||||
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" ;;
|
||||
unhandled|unknown)
|
||||
pending="$pending $ref"
|
||||
continue ;;
|
||||
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#*/}") || state=unknown
|
||||
case "$state" in
|
||||
unhandled|unknown)
|
||||
still="$still $ref" ;;
|
||||
*)
|
||||
begin_release
|
||||
report_release "$ref" "$state" ;;
|
||||
esac
|
||||
done
|
||||
pending=$still
|
||||
done
|
||||
for ref in $pending; do
|
||||
begin_release
|
||||
state=$(read_state "${ref%%/*}" "${ref#*/}") || state=unknown
|
||||
report_release "$ref" "$state"
|
||||
done
|
||||
stop_at=$retry_until
|
||||
|
||||
echo "released $released of $total fenced HelmReleases, $unhandled without an uninstall"
|
||||
# A flux-plunger still running could have fenced a release after the
|
||||
|
||||
@@ -56,3 +56,12 @@ tests:
|
||||
asserts:
|
||||
- failedTemplate:
|
||||
errorPattern: shutdownGracePeriodSeconds must be between 40 and 140
|
||||
|
||||
# A whole number of a million or more prints in exponent form, which the
|
||||
# whole-number check would misread as a fraction.
|
||||
- it: names the range, not the fraction, for a whole number far above it
|
||||
set:
|
||||
shutdownGracePeriodSeconds: 1000000
|
||||
asserts:
|
||||
- failedTemplate:
|
||||
errorPattern: shutdownGracePeriodSeconds must be between 40 and 140
|
||||
Reference in new issue
Block a user