aanogueira opened a new pull request, #13896:
URL: https://github.com/apache/apisix/pull/13896
## Problem
A freshly created health-check target defaults to `internal_health =
healthy` with zero probes (`add_target`'s hardcoded `is_healthy=true`), so a
pod that restarts while its backend is already unhealthy briefly routes real
traffic to it until the first active probe corrects the target's state.
Separately, checker creation itself is entirely lazy -- seeded only by
`fetch_checker()` on the live request path -- so an idle-but-critical upstream
with no prior traffic would never get a checker built at all ahead of a
readiness check.
There is currently no way to tell "healthy" (a real check passed) apart from
"healthy" (the zero-probe default) from outside the checker, so nothing can
gate readiness on "has this actually been checked yet."
## Changes
- `hack/patches/lua-resty-healthcheck-probed-gate.patch` (companion
library-side change, api7/lua-resty-healthcheck -- see note below): adds a
per-target probe-attempt **counter** in shm, incremented each time an active
check is actually dispatched for a target (success, failure, or timeout all
count -- attempted, not "healthy"), and a module function
`all_targets_probed(name, shm_name, min_attempts)` so any worker can ask "has
every target had at least `min_attempts` real checks yet?" A single attempt is
not always enough: with e.g. `unhealthy.http_failures = 2` configured,
`internal_health` only converges after two consecutive attempts, so a boolean
"was it ever probed" flag is not sufficient on its own -- confirmed via a live
kind end-to-end trial, where a boolean-based version still leaked real traffic
for 6 seconds after the pod was already marked Ready.
- `apisix/healthcheck_manager.lua`: adds two accessors for a readiness
plugin to use --
- `ensure_checker(resource_path)`: proactively seeds a checker for a
resource even with zero prior traffic, reusing the existing
`timer_create_checker` construction path. Also resolves domain-name upstream
nodes via `parse_domain_in_up` up front -- that resolution otherwise only
happens on the live request path, so a checker built ahead of traffic would
start probing under an unresolved identity and get silently rebuilt (wiping its
probe count) the moment real traffic first resolved the domain.
- `is_resource_probed(resource_path)`: delegates to the library's
`all_targets_probed`, computing the required attempt threshold from the
checker's own config (`max(unhealthy.http_failures, .tcp_failures, .timeouts,
healthy.successes)`).
- `t/node/healthcheck-fresh-node-default-healthy.t`: new tests covering lazy
checker creation (`fetch_checker` returns `false` until the next timer tick),
`ensure_checker` building a checker with zero prior traffic,
`all_targets_probed` flipping only after a real probe, and the multi-attempt
threshold behavior specifically (TEST 5: stays `false` after 1 attempt when
`min_attempts=2`, flips `true` only after the 2nd).
- `hack/kind-repro/`: a local, Docker/kind-based validation harness (not
part of the fix itself) used to reproduce and validate this end to end --
pod-restart-while-unhealthy now shows zero leaked requests before or after the
readiness transition, down from 64 leaked responses with an earlier
boolean-based version of this same gate.
## Dependency note
The probe-counter shm mechanism lives in the vendored
`lua-resty-healthcheck-api7` library, not this repo. `hack/patches/` contains
the patch and a full reference copy of the patched file for this repo's own
local validation harness; the library-side change itself will be proposed as a
standalone PR against api7/lua-resty-healthcheck (separate from
api7/lua-resty-healthcheck#59, an unrelated, already-open fix for
apache/apisix#13888 that this patch happens to stack on top of in this repo's
local patch files only -- the two are independent fixes for independent issues).
## Testing
- `t/node/healthcheck-fresh-node-default-healthy.t` (this repo)
- Local kind end-to-end trial: pod restart while backend already unhealthy,
comparing leaked `/pay` requests and the `/health_check_internal/ready`
transition, stock vs. patched (see
`hack/kind-repro/coldstart-gate-kind-trial.sh`)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
