fix(uptime-monitor): fail only on the ESO reasons that never retry (REN-106) #999
@@ -71,12 +71,85 @@ spec:
|
||||
# Kustomization would report a CEL evaluation error rather than a clean
|
||||
# in-progress.
|
||||
#
|
||||
# No `inProgress` expression: Flux treats "neither failed nor current" as
|
||||
# in-progress, which is the behaviour wanted here.
|
||||
# `failed` NAMES THE TWO REASONS ESO STOPS RETRYING ON, AND DELIBERATELY LEAVES
|
||||
# OUT `SecretSyncedError` (REN-106). The first cut of this block fired on any
|
||||
# `Ready=False` at all, and that path is undampened: the `flux-failures` Alert
|
||||
# (infrastructure/flux-notifications/alerts.yaml) forwards error events from
|
||||
# every Kustomization to Alertmanager, and a Flux health check has no `for:`
|
||||
# window, so a single bad poll would have mailed the owner.
|
||||
#
|
||||
# Read off the ESO the cluster actually runs, v2.11.0 (the chart range
|
||||
# `>=2.7.0 <3.0.0` in infrastructure/external-secrets/helmrelease.yaml resolves
|
||||
# there today). On the Secret target path ESO sets `Ready=False` with exactly
|
||||
# three reasons, and what separates them is whether it intends to try again
|
||||
# (pkg/controllers/externalsecret/externalsecret_controller.go):
|
||||
# SecretSyncedError — provider read, template or update failed. Almost every
|
||||
# site returns the error, so controller-runtime requeues
|
||||
# with backoff. This is the reason an OpenBao blip, a
|
||||
# token renewal AND a not-yet-populated key all produce —
|
||||
# the transient case and the REN-100 case share one
|
||||
# string, so the string cannot tell them apart.
|
||||
# SecretOwnedByOther — the target Secret is owned by something else.
|
||||
# SecretImmutable — the target Secret is immutable and needs an update.
|
||||
# The last two return `nil`, commented in ESO's own source as "cant be fixed by
|
||||
# retrying": ESO has given up, nothing is in flight, so failing at once is
|
||||
# correct and cannot be a flap. Narrowing to `reason == 'SecretSyncedError'`
|
||||
# instead — REN-106's first suggestion — would have inverted that, keeping the
|
||||
# one flap-prone reason and dropping the two that cannot flap.
|
||||
#
|
||||
# Two details that make `failed` an allowlist of reasons rather than the
|
||||
# complement of `SecretSyncedError`: one `SecretSyncedError` site
|
||||
# (`ErrSecretSetCtrlRef`) also returns `nil`, so that string is not a reliable
|
||||
# "still trying" signal either way; and the generic-target path (`spec.target`
|
||||
# pointing at something other than a Secret) has its own reason set,
|
||||
# `ResourceSyncedError`, which is not used here — this ExternalSecret targets a
|
||||
# Secret — and is left to the in-progress path if it ever is.
|
||||
#
|
||||
# A `SecretSyncedError` is dampened, not dropped: it matches neither
|
||||
# expression, so Flux reports in-progress and keeps polling until `timeout: 5m`
|
||||
# above expires and the Kustomization fails. That timeout is the `for:` window
|
||||
# this path otherwise has none of — a blip that clears inside it never reaches
|
||||
# the owner, a sync that is genuinely stuck still does, about five minutes
|
||||
# later.
|
||||
#
|
||||
# EVERY SHAPE AN ExternalSecret STATUS CAN TAKE GETS A VERDICT FROM THE TWO
|
||||
# EXPRESSIONS BELOW. There are three, not two, and the third one — no `Ready`
|
||||
# condition at all — is the one a reader is most likely to miss:
|
||||
#
|
||||
# 1. no conditions yet (applied, first reconcile unfinished): the `has()`
|
||||
# guard makes both expressions false -> in-progress, bounded by
|
||||
# `timeout: 5m`.
|
||||
# 2. `Ready=True`: `current` -> Current. Note this includes reasons
|
||||
# `SecretDeleted` and `SecretMissing`, which ESO reports as Ready=True
|
||||
# (markAsDone) even though no target Secret exists. Reaching them needs
|
||||
# `deletionPolicy: Delete`/`Merge` or `creationPolicy: Merge`; this
|
||||
# ExternalSecret takes the defaults (Retain/Owner), where a vanished
|
||||
# provider key is a `SecretSyncedError` instead. Anyone who later sets
|
||||
# either policy here reopens REN-100 through this expression.
|
||||
# 3. `Ready=False`: `failed` for the two non-retrying reasons above,
|
||||
# in-progress for the rest.
|
||||
#
|
||||
# No `inProgress` expression: only `apiVersion` and `current` are required by
|
||||
# the CRD (flux-system/gotk-components.yaml, kustomize-controller v1.9.5), and
|
||||
# Flux evaluates inProgress -> failed -> current and returns in-progress when
|
||||
# nothing matches (fluxcd/pkg runtime/cel, StatusEvaluator.Evaluate). An
|
||||
# `inProgress` naming shapes 1 and 3 would therefore be a no-op on all three.
|
||||
#
|
||||
# "conditions present but no `Ready` condition" is NOT a fourth shape (REN-106).
|
||||
# On a live object ESO only ever writes conditions of type `Ready`; the
|
||||
# `Deleted` type is never persisted — it is built on a throwaway object in the
|
||||
# `IsNotFound` branch of Reconcile purely to retire that object's metric series
|
||||
# ("this does not actually set the condition on the ExternalSecret, because it
|
||||
# does not exist"). So `externalsecret_status_condition{condition="Deleted"}`
|
||||
# outlives the object it names, until the ESO pod restarts.
|
||||
# `ai/wiki-index-basic-auth` is that: retired from git in 419cbed3, target
|
||||
# Secret gone from the namespace, sync counter frozen, still a series in an ESO
|
||||
# pod older than the prune. Any alert built on this metric (REN-103) has to
|
||||
# exclude that shape; this health check never sees it.
|
||||
healthCheckExprs:
|
||||
- apiVersion: external-secrets.io/v1
|
||||
kind: ExternalSecret
|
||||
failed: "has(status.conditions) && status.conditions.exists(c, c.type == 'Ready' && c.status == 'False')"
|
||||
failed: "has(status.conditions) && status.conditions.exists(c, c.type == 'Ready' && c.status == 'False' && (c.reason == 'SecretOwnedByOther' || c.reason == 'SecretImmutable'))"
|
||||
current: "has(status.conditions) && status.conditions.exists(c, c.type == 'Ready' && c.status == 'True')"
|
||||
dependsOn:
|
||||
# ESO itself, and the `uptime-monitor` namespace (declared centrally in
|
||||
|
||||
Reference in New Issue
Block a user