From 12fa7f292ece78154e46e3c2b30c9de620b46017 Mon Sep 17 00:00:00 2001 From: Andre Nogueira Date: Thu, 27 Aug 2026 21:20:38 +0100 Subject: [PATCH 1/4] feat(healthcheck): add a per-target probed flag for a cold-start readiness gate (PS-12691) 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. hack/patches/lua-resty-healthcheck-probed-gate.patch (stacks on the existing #13888 reconcile patch) adds a per-target "probed" shm flag, written the first time an active check is actually dispatched for that target (success, failure, or timeout all count -- attempted, not "healthy"), and a module function resty.healthcheck.all_targets_probed(name, shm_name) to query it from any worker. apisix/healthcheck_manager.lua adds two accessors for a readiness plugin to use: ensure_checker(resource_path), which proactively seeds a checker for a resource even with zero prior traffic (checker creation is otherwise entirely lazy, seeded only by fetch_checker() from the live request path -- an idle-but-critical upstream would never get a checker built without this), and is_resource_probed(resource_path), which delegates to the new library function. Extends t/node/healthcheck-fresh-node-default-healthy.t (TEST 3, TEST 4) to cover both: all_targets_probed() flips false->true only after a real probe fires, and ensure_checker() builds a checker with zero prior traffic. Companion plugin-side change (mollie-health-check.lua, edge-app) wires these into the readiness endpoint. Signed-off-by: Andre Nogueira --- apisix/healthcheck_manager.lua | 68 + .../healthcheck-probed-gate-patched-full.lua | 2098 +++++++++++++++++ .../lua-resty-healthcheck-probed-gate.patch | 126 + .../healthcheck-fresh-node-default-healthy.t | 274 +++ 4 files changed, 2566 insertions(+) create mode 100644 hack/patches/healthcheck-probed-gate-patched-full.lua create mode 100644 hack/patches/lua-resty-healthcheck-probed-gate.patch create mode 100644 t/node/healthcheck-fresh-node-default-healthy.t diff --git a/apisix/healthcheck_manager.lua b/apisix/healthcheck_manager.lua index 1a69cf53245c..8b8601af193e 100644 --- a/apisix/healthcheck_manager.lua +++ b/apisix/healthcheck_manager.lua @@ -273,6 +273,74 @@ function _M.fetch_node_status(checker, ip, port, hostname) end +-- Cold-start readiness gate support (PS-12691): checker creation is +-- otherwise entirely lazy -- waiting_pool is only ever seeded by +-- fetch_checker(), which is only called from the live request path. An +-- upstream that receives zero traffic before a readiness probe fires would +-- never get a checker built at all, so a "has this been probed" gate would +-- hang forever for an idle-but-critical upstream on a fresh pod. This lets a +-- caller (the readiness plugin) force that seeding proactively. +-- +-- Scoped to plain, non-plugin-constructed resource paths (e.g. +-- "/upstreams/", "/routes/") -- unlike timer_create_checker, this +-- does not resolve the get_plugin_name()/construct_upstream branch used by +-- plugins like ai-proxy-multi. Critical upstreams for readiness gating are +-- configured as standalone apisixUpstreams entries, not inline plugin +-- config, so this scope is sufficient. +function _M.ensure_checker(resource_path) + if working_pool[resource_path] then + -- already created, by traffic or a previous ensure_checker call + return true + end + + local res_conf = resource.fetch_latest_conf(resource_path) + if not res_conf then + core.log.error("ensure_checker: resource not found: ", resource_path) + return false, "resource not found" + end + + local upstream = res_conf.value.upstream or res_conf.value + if not upstream.checks then + core.log.warn("ensure_checker: resource has no checks configured: ", resource_path) + return false, "no checks configured" + end + if not upstream.nodes or #upstream.nodes == 0 then + return false, "no nodes" + end + + local new_version = upstream_utils.version(res_conf.modifiedIndex, upstream._nodes_ver) + if waiting_pool[resource_path] ~= new_version then + core.log.info("ensure_checker: seeding waiting pool for ", resource_path, + " with version: ", new_version) + waiting_pool[resource_path] = new_version + end + return true +end + + +-- Cold-start readiness gate support (PS-12691): true only once every target +-- of the resource's checker has had at least one real active-check attempt +-- (see resty.healthcheck's all_targets_probed -- "probed" means attempted, +-- not "healthy"). false if there is no live checker yet (ensure_checker not +-- called, or timer_create_checker hasn't run its next tick yet). +function _M.is_resource_probed(resource_path) + local item = working_pool[resource_path] + if not item or not item.checker or item.checker.dead then + return false + end + + if not healthcheck then + healthcheck = require("resty.healthcheck") + end + local ok, err = healthcheck.all_targets_probed(item.checker.name, healthcheck_shdict_name) + if ok == nil then + core.log.error("is_resource_probed: ", err) + return false + end + return ok +end + + local function add_working_pool(resource_path, resource_ver, checker, checks) working_pool[resource_path] = { version = resource_ver, diff --git a/hack/patches/healthcheck-probed-gate-patched-full.lua b/hack/patches/healthcheck-probed-gate-patched-full.lua new file mode 100644 index 000000000000..910365a1354c --- /dev/null +++ b/hack/patches/healthcheck-probed-gate-patched-full.lua @@ -0,0 +1,2098 @@ +-------------------------------------------------------------------------- +-- Healthcheck library for OpenResty. +-- +-- Some notes on the usage of this library: +-- +-- - Each target will have 4 counters, 1 success counter and 3 failure +-- counters ('http', 'tcp', and 'timeout'). Any failure will _only_ reset the +-- success counter, but a success will reset _all three_ failure counters. +-- +-- - All targets are uniquely identified by their IP address and port number +-- combination, most functions take those as arguments. +-- +-- - All keys in the SHM will be namespaced by the healthchecker name as +-- provided to the `new` function. Hence no collissions will occur on shm-keys +-- as long as the `name` is unique. +-- +-- - Active healthchecks will be synchronized across workers, such that only +-- a single active healthcheck runs. +-- +-- - Events will be raised in every worker, see [lua-resty-worker-events](https://github.com/Kong/lua-resty-worker-events) +-- for details. +-- +-- @copyright 2017-2023 Kong Inc. +-- @author Hisham Muhammad, Thijs Schreijer +-- @license Apache 2.0 + +local ERR = ngx.ERR +local WARN = ngx.WARN +local DEBUG = ngx.DEBUG +local ngx_log = ngx.log +local tostring = tostring +local ipairs = ipairs +local table_insert = table.insert +local table_remove = table.remove +local string_format = string.format +local ssl = require("ngx.ssl") +local resty_timer = require "resty.timer" +local bit = require("bit") +local re_find = ngx.re.find +local ngx_now = ngx.now +local ngx_worker_id = ngx.worker.id +local ngx_worker_pid = ngx.worker.pid +local pcall = pcall +local get_phase = ngx.get_phase +local type = type +local assert = assert + + +local RESTY_EVENTS_VER = [[^0\.1\.\d+$]] +local RESTY_WORKER_EVENTS_VER = "0.3.3" + + +local new_tab +local nkeys +local is_array +local codec + + +local TESTING = _G.__TESTING_HEALTHCHECKER or false + +do + local ok + + ok, new_tab = pcall(require, "table.new") + if not ok then + new_tab = function () return {} end + end + + -- OpenResty branch of LuaJIT New API + ok, nkeys = pcall(require, "table.nkeys") + if not ok then + nkeys = function (tab) + local count = 0 + for _, v in pairs(tab) do + if v ~= nil then + count = count + 1 + end + end + return count + end + end + + ok, is_array = pcall(require, "table.isarray") + if not ok then + is_array = function(t) + for k in pairs(t) do + if type(k) ~= "number" or math.floor(k) ~= k then + return false + end + end + return true + end + end + + ok, codec = pcall(require, "string.buffer") + if not ok then + codec = require("cjson.safe").new() + end +end + + +local worker_events +--- This function loads the worker events module received as arg. It will throw +-- error() if it is not possible to load the module. +local function load_events_module(self) + if self.events_module == "resty.worker.events" then + worker_events = require("resty.worker.events") + assert(worker_events, "could not load lua-resty-worker-events") + assert(worker_events._VERSION == RESTY_WORKER_EVENTS_VER, + "unsupported lua-resty-worker-events version") + + elseif self.events_module == "resty.events" then + worker_events = require("resty.events.compat") + local version_match = ngx.re.match(worker_events._VERSION, RESTY_EVENTS_VER, "o") + assert(version_match, "unsupported lua-resty-events version") + + else + error("unknown events module") + end + + assert(worker_events.configured(), "please configure the '" .. + self.events_module .. "' module before using 'lua-resty-healthcheck'") +end + + +-- constants +local EVENT_SOURCE_PREFIX = "lua-resty-healthcheck" +local LOG_PREFIX = "[healthcheck] " +local SHM_PREFIX = "lua-resty-healthcheck:" +local EMPTY = setmetatable({},{ + __newindex = function() + error("the EMPTY table is read only, check your code!", 2) + end + }) + +--- timer constants +-- evaluate active checks every 0.1s +local CHECK_INTERVAL = 0.1 +-- use a 10% jitter to start each worker timer +local CHECK_JITTER = CHECK_INTERVAL * 0.1 +-- lock valid period: the worker which acquires the lock owns it for 15 times +-- the check interval. If it does not update the shm during this period, we +-- consider that it is not able to continue checking (the worker probably was killed) +local LOCK_PERIOD = CHECK_INTERVAL * 15 + +-- Only the periodic-lock holder ever runs active probes (see active_check_timer +-- below), so every other worker's target.internal_health is updated *solely* by +-- the worker_events broadcast raised in incr_counter. That broadcast has no +-- delivery guarantee and, once a target is already at the reported health, is +-- never raised again for the same state -- so a single missed event permanently +-- strands a worker's local view (apache/apisix#13888). RECONCILE_INTERVAL bounds +-- how long that divergence can last: every worker re-derives internal_health +-- from the authoritative shm state on this cadence, independent of events. +local RECONCILE_INTERVAL = 1 +-- interval between stale targets cleanup +local CLEANUP_INTERVAL = CHECK_INTERVAL * 25 + +-- Counters: a 32-bit shm integer can hold up to four 8-bit counters. +local CTR_SUCCESS = 0x00000001 +local CTR_HTTP = 0x00000100 +local CTR_TCP = 0x00010000 +local CTR_TIMEOUT = 0x01000000 + +local MASK_FAILURE = 0xffffff00 +local MASK_SUCCESS = 0x000000ff + +local COUNTER_NAMES = { + [CTR_SUCCESS] = "SUCCESS", + [CTR_HTTP] = "HTTP", + [CTR_TCP] = "TCP", + [CTR_TIMEOUT] = "TIMEOUT", +} + +--- The list of potential events generated. +-- The `checker.EVENT_SOURCE` field can be used to subscribe to the events, see the +-- example below. Each of the events will get a table passed containing +-- the target details `ip`, `port`, and `hostname`. +-- See [lua-resty-worker-events](https://github.com/Kong/lua-resty-worker-events). +-- @field remove Event raised when a target is removed from the checker. +-- @field healthy This event is raised when the target status changed to +-- healthy (and when a target is added as `healthy`). +-- @field unhealthy This event is raised when the target status changed to +-- unhealthy (and when a target is added as `unhealthy`). +-- @field mostly_healthy This event is raised when the target status is +-- still healthy but it started to receive "unhealthy" updates via active or +-- passive checks. +-- @field mostly_unhealthy This event is raised when the target status is +-- still unhealthy but it started to receive "healthy" updates via active or +-- passive checks. +-- @table checker.events +-- @usage -- Register for all events from `my_checker` +-- local event_callback = function(target, event, source, source_PID) +-- local t = target.ip .. ":" .. target.port .." by name '" .. +-- target.hostname .. "' ") +-- +-- if event == my_checker.events.remove then +-- print(t .. "has been removed") +-- elseif event == my_checker.events.healthy then +-- print(t .. "is now healthy") +-- elseif event == my_checker.events.unhealthy then +-- print(t .. "is now unhealthy") +-- end +-- end +-- +-- worker_events.register(event_callback, my_checker.EVENT_SOURCE) +local EVENTS = setmetatable({}, { + __index = function(self, key) + error(("'%s' is not a valid event name"):format(tostring(key))) + end +}) +for _, event in ipairs({ + "remove", + "healthy", + "unhealthy", + "mostly_healthy", + "mostly_unhealthy", + "clear", +}) do + EVENTS[event] = event +end + +local INTERNAL_STATES = {} +for i, key in ipairs({ + "healthy", + "unhealthy", + "mostly_healthy", + "mostly_unhealthy", +}) do + INTERNAL_STATES[i] = key + INTERNAL_STATES[key] = i +end + +-- Some color for demo purposes +local use_color = false +local id = function(x) return x end +local worker_color = use_color and function(str) return ("\027["..tostring(31 + ngx_worker_pid() % 5).."m"..str.."\027[0m") end or id + +-- Debug function +local function dump(...) print(require("pl.pretty").write({...})) end -- luacheck: ignore 211 + +local _M = {} + +-- checker objects (weak) table +local hcs = setmetatable({}, { + __mode = "v", +}) + +local active_check_timer + +-- last time (ngx.now()) the shm-vs-local-cache reconciliation sweep ran; shared +-- by all checkers on this worker since the sweep itself iterates `hcs` +local last_reconcile_time = 0 + +-- serialize a table to a string +local serialize = codec.encode + + +-- deserialize a string to a table +local deserialize = codec.decode + + +local function key_for(key_prefix, ip, port, hostname) + return string_format("%s:%s:%s%s", key_prefix, ip, port, hostname and ":" .. hostname or "") +end + + +-- resty.lock timeout when yieldable +local LOCK_TIMEOUT = 5 + +local run_locked +do + -- resty_lock is restricted to this scope in order to keep sensitive + -- lock-handling code separate separate from all other business logic + -- + -- If you need to use resty_lock in a way that is not covered by the + -- `run_locked` helper function defined below, it's strongly-advised to + -- define it fully within this scope unless you have a very good reason + -- + -- (see https://github.com/Kong/lua-resty-healthcheck/pull/112) + local resty_lock = require "resty.lock" + + local yieldable = { + rewrite = true, + access = true, + content = true, + timer = true, + } + + local function run_in_timer(premature, self, key, fn, ...) + if premature then + return + end + + local ok, err = run_locked(self, key, fn, ...) + if not ok then + self:log(ERR, "locked function for key '", key, "' failed in timer: ", err) + end + end + + local function schedule(self, key, fn, ...) + local ok, err = ngx.timer.at(0, run_in_timer, self, key, fn, ...) + if not ok then + return nil, "failed scheduling locked function for key '" .. key .. + "', " .. err + end + + return "scheduled" + end + + -- resty.lock consumes these options immediately, so this table can be reused + local opts = { + exptime = 10, -- timeout after which lock is released anyway + timeout = LOCK_TIMEOUT, -- max wait time to acquire lock + } + + --- + -- Acquire a lock and run a function + -- + -- The function call itself is wrapped with `pcall` to protect against + -- exception. + -- + -- This function exhibits some special behavior when called during a + -- non-yieldable phase such as `init_worker` or `log`: + -- + -- 1. The lock timeout is set to 0 to ensure that `resty.lock` does not + -- attempt to sleep/yield + -- 2. If acquiring the lock fails due to a timeout, `run_locked` + -- (this function) is re-scheduled to run in a timer. In this case, + -- the function returns `"scheduled"` + -- + -- @param self The checker object + -- @param key the key/identifier to acquire a lock for + -- @param fn The function to execute + -- @param ... arguments that will be passed to fn + -- @return The results of the function; or nil and an error message + -- in case it fails locking. + function run_locked(self, key, fn, ...) + -- we're extra extra extra defensive in this code path + local typ = type(key) + -- XXX is a number key ever expected? + assert(typ == "string" or typ == "number", + "unexpected lock key type: " .. typ) + key = tostring(key) + + -- first aqcuire a lock or conditionally re-schedule ourselves in a timer + local lock + do + local yield = yieldable[get_phase()] + + if yield then + opts.timeout = LOCK_TIMEOUT + else + -- if yielding is not possible in the current phase, use a zero timeout + -- so that resty.lock will return `nil, "timeout"` immediately instead of + -- calling ngx.sleep() + opts.timeout = 0 + end + + local err + lock, err = resty_lock:new(self.shm_name, opts) + if not lock then + return nil, "failed creating lock for '" .. key .. "', " .. err + end + + local elapsed + elapsed, err = lock:lock(key) + + if not elapsed and err == "timeout" and not yield then + -- yielding is not possible in the current phase, so retry in a timer + return schedule(self, key, fn, ...) + + elseif not elapsed then + return nil, "failed acquiring lock for '" .. key .. "', " .. err + end + end + + local pok, perr, res = pcall(fn, ...) + + local ok, err = lock:unlock() + if not ok then + self:log(ERR, "failed unlocking '", key, "', ", err) + end + + if not pok then + return nil, "locked function threw an exception: " .. tostring(perr) + end + + return perr, res + end +end + + +local deepcopy +do + local function _deepcopy(orig, copied) + -- prevent infinite loop when a field refers its parent + copied[orig] = true + -- If the array-like table contains nil in the middle, + -- the len might be smaller than the expected. + -- But it doesn't affect the correctness. + local len = #orig + local copy = table.new(len, table.nkeys(orig) - len) + for orig_key, orig_value in pairs(orig) do + if type(orig_value) == "table" and not copied[orig_value] then + copy[orig_key] = _deepcopy(orig_value, copied) + else + copy[orig_key] = orig_value + end + end + + local mt = getmetatable(orig) + if mt ~= nil then + setmetatable(copy, mt) + end + + return copy + end + + + local copied_recorder = {} + + function deepcopy(orig) + local orig_type = type(orig) + if orig_type ~= 'table' then + return orig + end + + local res = _deepcopy(orig, copied_recorder) + table.clear(copied_recorder) + return res + end +end + + +local checker = {} + + +------------------------------------------------------------------------------ +-- Node management. +-- @section node-management +------------------------------------------------------------------------------ + + +-- @return the target list from the shm, an empty table if not found, or +-- `nil + error` upon a failure +local function fetch_target_list(self) + local target_list, err = self.shm:get(self.TARGET_LIST) + if err then + return nil, "failed to fetch target_list from shm: " .. err + end + + return target_list and deserialize(target_list) or {} +end + + +local function with_target_list(self, fn) + local targets, err = fetch_target_list(self) + if not targets then + return nil, err + end + + -- this is only ever called in the context of `run_locked`, + -- so no pcall needed + return fn(targets) +end + + +--- Run the given function holding a lock on the target list. +-- @param self The checker object +-- @param fn The function to execute +-- @return The results of the function; or nil and an error message +-- in case it fails locking. +local function locking_target_list(self, fn) + local ok, err = run_locked(self, self.TARGET_LIST_LOCK, with_target_list, self, fn) + + if ok == "scheduled" then + self:log(DEBUG, "target_list function re-scheduled in timer") + end + + return ok, err +end + + +--- Get a target +local function get_target(self, ip, port, hostname) + hostname = hostname or ip + return ((self.targets[ip] or EMPTY)[port] or EMPTY)[hostname] +end + +--- Add a target to the healthchecker. +-- When the ip + port + hostname combination already exists, it will simply +-- return success (without updating `is_healthy` status). +-- @param ip IP address of the target to check. +-- @param port the port to check against. +-- @param hostname (optional) hostname to set as the host header in the HTTP +-- probe request +-- @param is_healthy (optional) a boolean value indicating the initial state, +-- default is `true`. +-- @param hostheader (optional) a value to use for the Host header on +-- active healthchecks. +-- @param tbl_meta (optional) a lua table with custom info of business stuff +-- @return `true` on success, or `nil + error` on failure. +function checker:add_target(ip, port, hostname, is_healthy, hostheader, tbl_meta) + ip = tostring(assert(ip, "no ip address provided")) + port = assert(tonumber(port), "no port number provided") + hostname = hostname or ip + if is_healthy == nil then + is_healthy = true + end + + local internal_health = is_healthy and "healthy" or "unhealthy" + + local ok, err = locking_target_list(self, function(target_list) + local found = false + + -- check whether we already have this target + for _, target in ipairs(target_list) do + if target.ip == ip and target.port == port and target.hostname == (hostname) then + if target.purge_time == nil then + self:log(DEBUG, "adding an existing target: ", hostname or "", " ", ip, + ":", port, " (ignoring)") + return false + end + target.purge_time = nil + found = true + internal_health = self:get_target_status(ip, port, hostname) and + "healthy" or "unhealthy" + break + end + end + + -- we first add the internal health, and only then the updated list. + -- this prevents a state where a target is in the list, but does not + -- have a key in the shm. + local ok, err = self.shm:set(key_for(self.TARGET_STATE, ip, port, hostname), + INTERNAL_STATES[internal_health]) + if not ok then + self:log(ERR, "failed to set initial health status in shm: ", err) + end + + -- target does not exist, go add it + if not found then + target_list[#target_list + 1] = { + ip = ip, + port = port, + hostname = hostname, + hostheader = hostheader, + meta = tbl_meta, + } + end + target_list = serialize(target_list) + + ok, err = self.shm:set(self.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + -- raise event for our newly added target + if not found then + self:raise_event(self.events[internal_health], ip, port, hostname) + end + + return true + end) + + if ok == false then + -- the target already existed, no event, but still success + return true + end + + return ok, err + +end + + +-- Remove health status entries from an individual target from shm +-- @param self The checker object +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname hostname of the target being checked. +local function clear_target_data_from_shm(self, ip, port, hostname) + local ok, err = self.shm:set(key_for(self.TARGET_STATE, ip, port, hostname), nil) + if not ok then + self:log(ERR, "failed to remove health status from shm: ", err) + end + ok, err = self.shm:set(key_for(self.TARGET_COUNTER, ip, port, hostname), nil) + if not ok then + self:log(ERR, "failed to clear health counter from shm: ", err) + end + ok, err = self.shm:set(key_for(self.TARGET_PROBED, ip, port, hostname), nil) + if not ok then + self:log(ERR, "failed to clear probed flag from shm: ", err) + end +end + + +--- Remove a target from the healthchecker. +-- The target not existing is not considered an error. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @return `true` on success, or `nil + error` on failure. +function checker:remove_target(ip, port, hostname) + ip = tostring(assert(ip, "no ip address provided")) + port = assert(tonumber(port), "no port number provided") + + return locking_target_list(self, function(target_list) + + -- find the target + local target_found + for i, target in ipairs(target_list) do + if target.ip == ip and target.port == port and target.hostname == hostname then + target_found = target + table_remove(target_list, i) + break + end + end + + if not target_found then + return true + end + + -- go update the shm + target_list = serialize(target_list) + + -- we first write the updated list, and only then remove the health + -- status; this prevents race conditions when a healthchecker gets the + -- initial state from the shm + local ok, err = self.shm:set(self.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + clear_target_data_from_shm(self, ip, port, hostname) + + -- raise event for our removed target + self:raise_event(self.events.remove, ip, port, hostname) + + return true + end) +end + + +--- Clear all healthcheck data. +-- @return `true` on success, or `nil + error` on failure. +function checker:clear() + + return locking_target_list(self, function(target_list) + + local old_target_list = target_list + + -- go update the shm + target_list = serialize({}) + + local ok, err = self.shm:set(self.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + -- remove all individual statuses + for _, target in ipairs(old_target_list) do + local ip, port, hostname = target.ip, target.port, target.hostname + clear_target_data_from_shm(self, ip, port, hostname) + end + + self.targets = {} + + -- raise event for our removed target + self:raise_event(self.events.clear) + + return true + end) +end + + +--- Clear all healthcheck data after a period of time. +-- Useful for keeping target status between configuration reloads. +-- @param delay delay in seconds before purging target state. +-- @return `true` on success, or `nil + error` on failure. +function checker:delayed_clear(delay) + assert(tonumber(delay), "no delay provided") + + return locking_target_list(self, function(target_list) + local purge_time = ngx_now() + delay + + -- add purge time to all targets + for _, target in ipairs(target_list) do + target.purge_time = purge_time + end + + target_list = serialize(target_list) + local ok, err = self.shm:set(self.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + return true + end) +end + + +--- Get the current status of the target. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname the hostname of the target being checked. +-- @return `true` if healthy, `false` if unhealthy, or `nil + error` on failure. +function checker:get_target_status(ip, port, hostname) + + local target = get_target(self, ip, port, hostname) + if not target then + return nil, "target not found" + end + return target.internal_health == "healthy" + or target.internal_health == "mostly_healthy" + +end + + +------------------------------------------------------------------------------ +-- Health management. +-- Functions that allow reporting of failures/successes for passive checks. +-- @section health-management +------------------------------------------------------------------------------ + + +-- Run the given function holding a lock on the target. +-- @param self The checker object +-- @param ip Target IP +-- @param port Target port +-- @param hostname Target hostname +-- @param fn The function to execute +-- @return The results of the function; or true in case it fails locking and +-- will retry asynchronously; or nil+err in case it fails to retry. +local function locking_target(self, ip, port, hostname, fn) + local key = key_for(self.TARGET_LOCK, ip, port, hostname) + + local ok, err = run_locked(self, key, fn) + + if ok == "scheduled" then + self:log(DEBUG, "target function for ", key, " was re-scheduled") + end + + return ok, err +end + + +-- Extract the value of the counter at `idx` from multi-counter `multictr`. +-- @param multictr A 32-bit multi-counter holding 4 values. +-- @param idx The shift index specifying which counter to get. +-- @return The 8-bit value extracted from the 32-bit multi-counter. +local function ctr_get(multictr, idx) + return bit.band(multictr / idx, 0xff) +end + + +-- Increment the healthy or unhealthy counter. If the threshold of occurrences +-- is reached, it changes the status of the target in the shm and posts an +-- event. +-- @param self The checker object +-- @param health_report "healthy" for the success counter that drives a target +-- towards the healthy state; "unhealthy" for the failure counter. +-- @param ip Target IP +-- @param port Target port +-- @param hostname Target hostname +-- @param limit the limit after which target status is changed +-- @param ctr_type the counter to increment, see CTR_xxx constants +-- @return True if succeeded, or nil and an error message. +local function incr_counter(self, health_report, ip, port, hostname, limit, ctr_type) + + -- fail fast on counters that are disabled by configuration + if limit == 0 then + return true + end + + hostname = hostname or ip + port = tonumber(port) + local target = get_target(self, ip, port, hostname) + if not target then + -- sync issue: warn, but return success + self:log(WARN, "trying to increment a target that is not in the list: ", + hostname and "(" .. hostname .. ") " or "", ip, ":", port) + return true + end + + local current_health = target.internal_health + if health_report == current_health then + -- No need to count successes when internal health is fully "healthy" + -- or failures when internal health is fully "unhealthy" + return true + end + + return locking_target(self, ip, port, hostname, function() + local counter_key = key_for(self.TARGET_COUNTER, ip, port, hostname) + local multictr, err = self.shm:incr(counter_key, ctr_type, 0) + if err then + return nil, err + end + + local ctr = ctr_get(multictr, ctr_type) + + self:log(WARN, health_report, " ", COUNTER_NAMES[ctr_type], + " increment (", ctr, "/", limit, ") for '", hostname or "", + "(", ip, ":", port, ")'") + + local new_multictr + if ctr_type == CTR_SUCCESS then + new_multictr = bit.band(multictr, MASK_SUCCESS) + else + new_multictr = bit.band(multictr, MASK_FAILURE) + end + + if new_multictr ~= multictr then + self.shm:set(counter_key, new_multictr) + end + + local new_health + if ctr >= limit then + new_health = health_report + elseif current_health == "healthy" and bit.band(new_multictr, MASK_FAILURE) > 0 then + new_health = "mostly_healthy" + elseif current_health == "unhealthy" and bit.band(new_multictr, MASK_SUCCESS) > 0 then + new_health = "mostly_unhealthy" + end + + if new_health and new_health ~= current_health then + local state_key = key_for(self.TARGET_STATE, ip, port, hostname) + self.shm:set(state_key, INTERNAL_STATES[new_health]) + self:raise_event(self.events[new_health], ip, port, hostname) + end + + return true + + end) + +end + + +--- Report a health failure. +-- Reports a health failure which will count against the number of occurrences +-- required to make a target "fall". The type of healthchecker, +-- "tcp" or "http" (see `new`) determines against which counter the occurence goes. +-- If `unhealthy.tcp_failures` (for TCP failures) or `unhealthy.http_failures` +-- is set to zero in the configuration, this function is a no-op +-- and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, or `nil + error` on failure. +function checker:report_failure(ip, port, hostname, check) + + local checks = self.checks[check or "passive"] + local limit, ctr_type + if self.checks[check or "passive"].type == "tcp" then + limit = checks.unhealthy.tcp_failures + ctr_type = CTR_TCP + else + limit = checks.unhealthy.http_failures + ctr_type = CTR_HTTP + end + + return incr_counter(self, "unhealthy", ip, port, hostname, limit, ctr_type) + +end + + +--- Report a health success. +-- Reports a health success which will count against the number of occurrences +-- required to make a target "rise". +-- If `healthy.successes` is set to zero in the configuration, +-- this function is a no-op and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, or `nil + error` on failure. +function checker:report_success(ip, port, hostname, check) + + local limit = self.checks[check or "passive"].healthy.successes + + return incr_counter(self, "healthy", ip, port, hostname, limit, CTR_SUCCESS) + +end + + +--- Report a http response code. +-- How the code is interpreted is based on the configuration for healthy and +-- unhealthy statuses. If it is in neither strategy, it will be ignored. +-- If `healthy.successes` (for healthy HTTP status codes) +-- or `unhealthy.http_failures` (fur unhealthy HTTP status codes) +-- is set to zero in the configuration, this function is a no-op +-- and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @param http_status the http statuscode, or nil to report an invalid http response. +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, `nil` if the status was ignored (not in active or +-- passive health check lists) or `nil + error` on failure. +function checker:report_http_status(ip, port, hostname, http_status, check) + http_status = tonumber(http_status) or 0 + + local checks = self.checks[check or "passive"] + + local status_type, limit, ctr + if checks.healthy.http_statuses[http_status] then + status_type = "healthy" + limit = checks.healthy.successes + ctr = CTR_SUCCESS + elseif checks.unhealthy.http_statuses[http_status] + or http_status == 0 then + status_type = "unhealthy" + limit = checks.unhealthy.http_failures + ctr = CTR_HTTP + else + return + end + + return incr_counter(self, status_type, ip, port, hostname, limit, ctr) + +end + +--- Report a failure on TCP level. +-- If `unhealthy.tcp_failures` is set to zero in the configuration, +-- this function is a no-op and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname hostname of the target being checked. +-- @param operation The socket operation that failed: +-- "connect", "send" or "receive". +-- TODO check what kind of information we get from the OpenResty layer +-- in order to tell these error conditions apart +-- https://github.com/openresty/lua-resty-core/blob/master/lib/ngx/balancer.md#get_last_failure +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, or `nil + error` on failure. +function checker:report_tcp_failure(ip, port, hostname, operation, check) + + local limit = self.checks[check or "passive"].unhealthy.tcp_failures + + -- TODO what do we do with the `operation` information + return incr_counter(self, "unhealthy", ip, port, hostname, limit, CTR_TCP) + +end + + +--- Report a timeout failure. +-- If `unhealthy.timeouts` is set to zero in the configuration, +-- this function is a no-op and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, or `nil + error` on failure. +function checker:report_timeout(ip, port, hostname, check) + + local limit = self.checks[check or "passive"].unhealthy.timeouts + + return incr_counter(self, "unhealthy", ip, port, hostname, limit, CTR_TIMEOUT) + +end + + +--- Sets the current status of all targets with the given hostname and port. +-- @param hostname hostname being checked. +-- @param port the port being checked against +-- @param is_healthy boolean: `true` for healthy, `false` for unhealthy +-- @return `true` on success, or `nil + error` on failure. +function checker:set_all_target_statuses_for_hostname(hostname, port, is_healthy) + assert(type(hostname) == "string", "no hostname provided") + port = assert(tonumber(port), "no port number provided") + assert(type(is_healthy) == "boolean") + + local all_ok = true + local errs = {} + for _, target in ipairs(self.targets) do + if target.port == port and target.hostname == hostname then + local ok, err = self:set_target_status(target.ip, port, hostname, is_healthy) + if not ok then + all_ok = nil + table.insert(errs, err) + end + end + end + + return all_ok, #errs > 0 and table.concat(errs, "; ") or nil +end + + +--- Sets the current status of the target. +-- This will immediately set the status and clear its counters. +-- @param ip IP address of the target being checked +-- @param port the port being checked against +-- @param hostname (optional) hostname of the target being checked. +-- @param is_healthy boolean: `true` for healthy, `false` for unhealthy +-- @return `true` on success, or `nil + error` on failure +function checker:set_target_status(ip, port, hostname, is_healthy) + ip = tostring(assert(ip, "no ip address provided")) + port = assert(tonumber(port), "no port number provided") + assert(type(is_healthy) == "boolean") + hostname = hostname or ip + + local health_report = is_healthy and "healthy" or "unhealthy" + + local target = get_target(self, ip, port, hostname) + if not target then + -- sync issue: warn, but return success + self:log(WARN, "trying to set status for a target that is not in the list: ", ip, ":", port) + return true + end + + local counter_key = key_for(self.TARGET_COUNTER, ip, port, hostname) + local state_key = key_for(self.TARGET_STATE, ip, port, hostname) + + local ok, err = locking_target(self, ip, port, hostname, function() + + local _, err = self.shm:set(counter_key, 0) + if err then + return nil, err + end + + self.shm:set(state_key, INTERNAL_STATES[health_report]) + if err then + return nil, err + end + + self:raise_event(self.events[health_report], ip, port, hostname) + + return true + + end) + + if ok then + self:log(WARN, health_report, " forced for ", hostname, " ", ip, ":", port) + end + return ok, err +end + + +-- Introspection function for testing +local function test_get_counter(self, ip, port, hostname) + return locking_target(self, ip, port, hostname, function() + local counter = self.shm:get(key_for(self.TARGET_COUNTER, ip, port, hostname)) + local internal_health = (get_target(self, ip, port, hostname) or EMPTY).internal_health + return counter, internal_health + end) +end + + +--============================================================================ +-- Healthcheck runner +--============================================================================ + + +-- Runs a single healthcheck probe +function checker:run_single_check(ip, port, hostname, hostheader) + + -- Mark the target as probed *before* the attempt, not after: "probed" means + -- an active check was actually dispatched for this target at least once + -- (success, failure, or timeout all count), which is what a cold-start + -- readiness gate needs to know -- not whether the check succeeded. Written + -- to shm (not a local field) since a readiness probe can land on any + -- worker, not just whichever one holds the periodic-probe lock and runs + -- this function. + local probed_ok, probed_err = self.shm:set( + key_for(self.TARGET_PROBED, ip, port, hostname), true) + if not probed_ok then + self:log(ERR, "failed to mark target as probed in shm: ", probed_err) + end + + local sock, err = ngx.socket.tcp() + if not sock then + self:log(ERR, "failed to create stream socket: ", err) + return + end + + sock:settimeout(self.checks.active.timeout * 1000) + + local ok + ok, err = sock:connect(ip, port) + if not ok then + if err == "timeout" then + sock:close() -- timeout errors do not close the socket. + return self:report_timeout(ip, port, hostname, "active") + end + return self:report_tcp_failure(ip, port, hostname, "connect", "active") + end + + if self.checks.active.type == "tcp" then + sock:close() + return self:report_success(ip, port, hostname, "active") + end + + if self.checks.active.type == "https" then + local https_sni, session, err + https_sni = self.checks.active.https_sni or hostheader or hostname + if self.ssl_cert and self.ssl_key then + ok, err = sock:setclientcert(self.ssl_cert, self.ssl_key) + + if not ok then + self:log(ERR, "failed to set client certificate: ", err) + end + end + + session, err = sock:sslhandshake(nil, https_sni, + self.checks.active.https_verify_certificate) + + if not session then + sock:close() + self:log(ERR, "failed SSL handshake with '", hostname or "", " (", ip, ":", port, ")', using server name (sni) '", https_sni, "': ", err) + return self:report_tcp_failure(ip, port, hostname, "connect", "active") + end + + end + + local req_headers = self.checks.active.req_headers + local headers + if self.checks.active._headers_str then + headers = self.checks.active._headers_str + else + local headers_length = nkeys(req_headers) + if headers_length > 0 then + if is_array(req_headers) then + self:log(WARN, "array headers is deprecated") + headers = table.concat(req_headers, "\r\n") + else + headers = new_tab(0, headers_length) + local idx = 0 + for key, values in pairs(req_headers) do + if type(values) == "table" then + for _, value in ipairs(values) do + idx = idx + 1 + headers[idx] = key .. ": " .. tostring(value) + end + else + idx = idx + 1 + headers[idx] = key .. ": " .. tostring(values) + end + end + headers = table.concat(headers, "\r\n") + end + if #headers > 0 then + headers = headers .. "\r\n" + end + end + self.checks.active._headers_str = headers or "" + end + + local method = self.checks.active.http_method + local path = self.checks.active.http_path + local body = self.checks.active.http_req_body + local final_hostheader = hostheader or hostname or ip + local request + if body and #body > 0 then + request = ("%s %s HTTP/1.1\r\nConnection: close\r\n%sHost: %s\r\nContent-Length: %d\r\n\r\n%s") + :format(method, path, headers, final_hostheader, #body, body) + else + request = ("%s %s HTTP/1.1\r\nConnection: close\r\n%sHost: %s\r\n\r\n") + :format(method, path, headers, final_hostheader) + end + self:log(DEBUG, "request: ", request) + + local bytes + bytes, err = sock:send(request) + if not bytes then + self:log(ERR, "failed to send http request to '", hostname, " (", ip, ":", port, ")': ", err) + if err == "timeout" then + sock:close() -- timeout errors do not close the socket. + return self:report_timeout(ip, port, hostname, "active") + end + return self:report_tcp_failure(ip, port, hostname, "send", "active") + end + + local status_line + status_line, err = sock:receive() + if not status_line then + self:log(ERR, "failed to receive status line from '", hostname, " (",ip, ":", port, ")': ", err) + if err == "timeout" then + sock:close() -- timeout errors do not close the socket. + return self:report_timeout(ip, port, hostname, "active") + end + return self:report_tcp_failure(ip, port, hostname, "receive", "active") + end + + local from, to = re_find(status_line, + [[^HTTP/\d+\.\d+\s+(\d+)]], + "joi", nil, 1) + local status + if from then + status = tonumber(status_line:sub(from, to)) + else + self:log(ERR, "bad status line from '", hostname, " (", ip, ":", port, ")': ", status_line) + -- note: 'status' will be reported as 'nil' + end + + sock:close() + + self:log(DEBUG, "Reporting '", hostname, " (", ip, ":", port, ")' (got HTTP ", status, ")") + + return self:report_http_status(ip, port, hostname, status, "active") +end + +-- executes a work package (a list of checks) sequentially +function checker:run_work_package(work_package) + for _, work_item in ipairs(work_package) do + self:log(DEBUG, "Checking ", work_item.hostname, " ", + work_item.hostheader and "(host header: ".. work_item.hostheader .. ")" + or "", work_item.ip, ":", work_item.port, + " (currently ", work_item.debug_health, ")") + local hostheader = work_item.hostheader or work_item.hostname + self:run_single_check(work_item.ip, work_item.port, work_item.hostname, hostheader) + end +end + +-- runs the active healthchecks concurrently, in multiple work packages. +-- @param list the list of targets to check +function checker:active_check_targets(list) + local idx = 1 + local work_packages = {} + + for _, work_item in ipairs(list) do + local package = work_packages[idx] + if not package then + package = {} + work_packages[idx] = package + end + package[#package + 1] = work_item + idx = idx + 1 + if idx > self.checks.active.concurrency then idx = 1 end + end + + -- hand out work-packages to the threads, note the "-1" because this timer + -- thread will handle the last package itself. + local threads = {} + for i = 1, #work_packages - 1 do + threads[i] = ngx.thread.spawn(self.run_work_package, self, work_packages[i]) + end + -- run last package myself + self:run_work_package(work_packages[#work_packages]) + + -- wait for everybody to finish + for _, thread in ipairs(threads) do + ngx.thread.wait(thread) + end +end + +--============================================================================ +-- Internal callbacks, timers and events +--============================================================================ +-- The timer callbacks are responsible for checking the status, upon success/ +-- failure they will call the health-management functions to deal with the +-- results of the checks. + + +-- @return `true` on success, or false if the lock was not acquired, or `nil + error` +-- in case of errors +local function get_periodic_lock(shm, key) + local my_pid = ngx_worker_pid() + local checker_pid = shm:get(key) + + if checker_pid == nil then + -- no worker is checking, try to acquire the lock + local ok, err = shm:add(key, my_pid, LOCK_PERIOD) + if not ok then + if err == "exists" then + -- another worker got the lock before + return false + end + ngx_log(ERR, "failed to add key '", key, "': ", err) + return nil, err + end + elseif checker_pid ~= my_pid then + -- another worker is checking + return false + end + + return true +end + + +-- touch the shm to refresh the valid period +local function renew_periodic_lock(shm, key) + local my_pid = ngx_worker_pid() + + local _, err = shm:set(key, my_pid, LOCK_PERIOD) + if err then + ngx_log(ERR, "failed to update key '", key, "': ", err) + end +end + + +--- Active health check callback function. +-- @param self the checker object this timer runs on +-- @param health_mode either "healthy" or "unhealthy" to indicate what check +local function checker_callback(self, health_mode) + if self.checker_callback_count then + self.checker_callback_count = self.checker_callback_count + 1 + end + + local list_to_check = {} + local targets, err = fetch_target_list(self) + if not targets then + self:log(ERR, "checker_callback: ", err) + return + end + + for _, target in ipairs(targets) do + local tgt = get_target(self, target.ip, target.port, target.hostname) + local internal_health = tgt and tgt.internal_health or nil + if (health_mode == "healthy" and (internal_health == "healthy" or + internal_health == "mostly_healthy")) + or (health_mode == "unhealthy" and (internal_health == "unhealthy" or + internal_health == "mostly_unhealthy")) + then + list_to_check[#list_to_check + 1] = { + ip = target.ip, + port = target.port, + hostname = target.hostname, + hostheader = target.hostheader, + meta = target.meta, + debug_health = internal_health, + } + end + end + + if not list_to_check[1] then + self:log(DEBUG, "checking ", health_mode, " targets: nothing to do") + else + local timer = resty_timer({ + interval = 0, + recurring = false, + immediate = false, + detached = true, + expire = function() + self:log(DEBUG, "checking ", health_mode, " targets: #", #list_to_check) + self:active_check_targets(list_to_check) + end, + }) + if timer == nil then + self:log(ERR, "failed to create timer to check ", health_mode) + end + end +end + +-- Event handler callback +function checker:event_handler(event_name, ip, port, hostname) + + local target_found = get_target(self, ip, port, hostname) + + if event_name == self.events.remove then + if target_found then + -- remove hash part + self.targets[target_found.ip][target_found.port][target_found.hostname] = nil + if not next(self.targets[target_found.ip][target_found.port]) then + -- no more hostnames on this port, so delete it + self.targets[target_found.ip][target_found.port] = nil + end + if not next(self.targets[target_found.ip]) then + -- no more ports on this ip, so delete it + self.targets[target_found.ip] = nil + end + -- remove from list part + for i, target in ipairs(self.targets) do + if target.ip == ip and target.port == port and + target.hostname == hostname then + table_remove(self.targets, i) + break + end + end + self:log(DEBUG, "event: target '", hostname or "", " (", ip, ":", port, + ")' removed") + + else + self:log(WARN, "event: trying to remove an unknown target '", + hostname or "", "(", ip, ":", port, ")'") + end + + elseif event_name == self.events.healthy or + event_name == self.events.mostly_healthy or + event_name == self.events.unhealthy or + event_name == self.events.mostly_unhealthy + then + if not target_found then + -- it is a new target, must add it first + target_found = { ip = ip, port = port, hostname = hostname or ip } + self.targets[target_found.ip] = self.targets[target_found.ip] or {} + self.targets[target_found.ip][target_found.port] = self.targets[target_found.ip][target_found.port] or {} + self.targets[target_found.ip][target_found.port][target_found.hostname] = target_found + self.targets[#self.targets + 1] = target_found + self:log(DEBUG, "event: target added '", hostname or "", "(", ip, ":", port, ")'") + end + do + local from_status = target_found.internal_health + local to_status = event_name + local from = from_status == "healthy" or from_status == "mostly_healthy" + local to = to_status == "healthy" or to_status == "mostly_healthy" + + if from ~= to then + self.status_ver = self.status_ver + 1 + end + + self:log(DEBUG, "event: target status '", hostname or "", "(", ip, ":", + port, ")' from '", from, "' to '", to, "', ver: ", self.status_ver) + end + target_found.internal_health = event_name + + elseif event_name == self.events.clear then + -- clear local cache + self.targets = {} + self:log(DEBUG, "event: local cache cleared") + + else + self:log(WARN, "event: unknown event received '", event_name, "'") + end +end + + +-- Re-derive a checker's local internal_health for every target the shm target +-- list says exists, correcting any target whose cached value has drifted from +-- a worker_events broadcast this worker never received (apache/apisix#13888), +-- AND backfilling any target this worker's self.targets never even contains +-- an entry for at all. +-- +-- The second case is not hypothetical: add_target()'s "already exists in shm" +-- branch (see its own comment) returns early without ever calling raise_event, +-- so a worker whose *own* add_target call loses that race gets no event either +-- -- and if this worker's initial checker.new() read of the target list also +-- raced ahead of the writer (observed directly: "Got initial target list (0 +-- targets)" immediately followed by "adding an existing target ... (ignoring)" +-- for every target), self.targets ends up with no entry for that target at +-- all: not stale, structurally absent. Since checker_callback() only ever +-- looks a target up via get_target(self, ...) against this same self.targets, +-- such a target is permanently invisible to this worker's active-check cycle +-- -- if this worker also holds the periodic probe lock (which never +-- voluntarily rotates), NO worker ever probes that target again for the life +-- of the process. Sourcing this sweep from fetch_target_list() (shm, the +-- authoritative source used across the file, e.g. add_target/checker_callback +-- itself) instead of iterating self.targets directly is what lets a missing +-- entry be detected in the first place. +local function reconcile_target_health(checker_obj) + local targets, err = fetch_target_list(checker_obj) + if not targets then + checker_obj:log(ERR, "reconcile: failed to fetch target list from shm: ", err) + return + end + + for _, target in ipairs(targets) do + local state_key = key_for(checker_obj.TARGET_STATE, target.ip, target.port, + target.hostname) + local raw_state = checker_obj.shm:get(state_key) + -- add_target() always writes TARGET_STATE before a target is considered + -- live (see its comment), so nil here means this read raced a concurrent + -- add/remove, not a genuine absence of state -- skip it for this sweep, + -- it will be consistent again on the next one. + if raw_state ~= nil then + local shm_health = INTERNAL_STATES[raw_state] + if shm_health then + local target_found = get_target(checker_obj, target.ip, target.port, target.hostname) + if not target_found then + -- lazily insert, mirroring event_handler's own "it is a new target, + -- must add it first" branch -- keeps both the ip/port/hostname + -- lookup table and the array part (used elsewhere, e.g. remove) + -- consistent with how every other insertion path populates them. + target_found = { ip = target.ip, port = target.port, + hostname = target.hostname or target.ip } + checker_obj.targets[target_found.ip] = checker_obj.targets[target_found.ip] or {} + checker_obj.targets[target_found.ip][target_found.port] = + checker_obj.targets[target_found.ip][target_found.port] or {} + checker_obj.targets[target_found.ip][target_found.port][target_found.hostname] = + target_found + checker_obj.targets[#checker_obj.targets + 1] = target_found + checker_obj:log(WARN, "reconciled missing target from shm (never seen locally) '", + target_found.hostname or "", "(", target_found.ip, ":", + target_found.port, ")' as '", shm_health, "'") + elseif shm_health ~= target_found.internal_health then + local from = target_found.internal_health == "healthy" or + target_found.internal_health == "mostly_healthy" + local to = shm_health == "healthy" or shm_health == "mostly_healthy" + if from ~= to then + checker_obj.status_ver = checker_obj.status_ver + 1 + end + checker_obj:log(WARN, "reconciled target status from shm (missed event) '", + target_found.hostname or "", "(", target_found.ip, ":", + target_found.port, ")' from '", target_found.internal_health, + "' to '", shm_health, "'") + end + target_found.internal_health = shm_health + end + end + end +end + + +------------------------------------------------------------------------------ +-- Initializing. +-- @section initializing +------------------------------------------------------------------------------ + +-- Log a message specific to this checker +-- @param level standard ngx log level constant +function checker:log(level, ...) + ngx_log(level, worker_color(self.LOG_PREFIX), ...) +end + + +-- Raises an event for a target status change. +function checker:raise_event(event_name, ip, port, hostname) + local target = { ip = ip, port = port, hostname = hostname } + worker_events.post(self.EVENT_SOURCE, event_name, target) +end + + +--- Stop the background health checks. +-- The timers will be flagged to exit, but will not exit immediately. Only +-- after the current timers have expired they will be marked as stopped. +-- @return `true` +function checker:stop() + self.checks.active.healthy.active = false + self.checks.active.unhealthy.active = false + worker_events.unregister(self.ev_callback, self.EVENT_SOURCE) + self:log(DEBUG, "healthchecker stopped") + return true +end + + +--- Start the background health checks. +-- @return `true`, or `nil + error`. +function checker:start() + if self.checks.active.healthy.interval > 0 then + self.checks.active.healthy.active = true + -- the first active check happens only after `interval` + self.checks.active.healthy.last_run = ngx_now() + end + + if self.checks.active.unhealthy.interval > 0 then + self.checks.active.unhealthy.active = true + self.checks.active.unhealthy.last_run = ngx_now() + end + + worker_events.unregister(self.ev_callback, self.EVENT_SOURCE) -- ensure we never double subscribe + worker_events.register_weak(self.ev_callback, self.EVENT_SOURCE) + + self:log(DEBUG, "active check flagged as active") + return true +end + + +--============================================================================ +-- Create health-checkers +--============================================================================ + + +local NO_DEFAULT = {} +local MAXNUM = 2^31 - 1 + + +local function fail(ctx, k, msg) + ctx[#ctx + 1] = k + error(table.concat(ctx, ".") .. ": " .. msg, #ctx + 1) +end + + +local function fill_in_settings(opts, defaults, ctx) + ctx = ctx or {} + local obj = {} + for k, default in pairs(defaults) do + local v = opts[k] + + -- basic type-check of configuration + if default ~= NO_DEFAULT + and v ~= nil + and type(v) ~= type(default) then + fail(ctx, k, "invalid value") + end + + if v ~= nil then + if type(v) == "table" then + if default[1] then -- do not recurse on arrays + obj[k] = v + else + ctx[#ctx + 1] = k + obj[k] = fill_in_settings(v, default, ctx) + ctx[#ctx + 1] = nil + end + else + if type(v) == "number" and (v < 0 or v > MAXNUM) then + fail(ctx, k, "must be between 0 and " .. MAXNUM) + end + obj[k] = v + end + elseif default ~= NO_DEFAULT then + obj[k] = deepcopy(default) + end + + end + return obj +end + + +local defaults = { + name = NO_DEFAULT, + shm_name = NO_DEFAULT, + type = NO_DEFAULT, + status_ver = 0, + events_module = "resty.worker.events", + checks = { + active = { + type = "http", + timeout = 1, + concurrency = 10, + http_method = "GET", + http_path = "/", + http_req_body = "", + https_sni = NO_DEFAULT, + https_verify_certificate = true, + headers = {""}, + healthy = { + interval = 0, -- 0 = disabled by default + http_statuses = { 200, 302 }, + successes = 2, + }, + unhealthy = { + interval = 0, -- 0 = disabled by default + http_statuses = { 429, 404, + 500, 501, 502, 503, 504, 505 }, + tcp_failures = 2, + timeouts = 3, + http_failures = 5, + }, + req_headers = {""}, + }, + passive = { + type = "http", + healthy = { + http_statuses = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 226, + 300, 301, 302, 303, 304, 305, 306, 307, 308 }, + successes = 5, + }, + unhealthy = { + http_statuses = { 429, 500, 503 }, + tcp_failures = 2, + timeouts = 7, + http_failures = 5, + }, + }, + }, +} + + +local function to_set(tbl, key) + local set = {} + for _, item in ipairs(tbl[key]) do + set[item] = true + end + tbl[key] = set +end + + +local check_valid_type +do + local valid_types = { + http = true, + tcp = true, + https = true, + } + check_valid_type = function(var, val) + assert(valid_types[val], + var .. " can only be 'http', 'https' or 'tcp', got '" .. + tostring(val) .. "'") + end +end + +--- Creates a new health-checker instance. +-- It will be started upon creation. +-- +-- *NOTE*: the returned `checker` object must be anchored, if not it will be +-- removed by Lua's garbage collector and the healthchecks will cease to run. +-- @param opts table with checker options. Options are: +-- +-- * `name`: name of the health checker +-- * `shm_name`: the name of the `lua_shared_dict` specified in the Nginx configuration to use +-- * `ssl_cert`: certificate for mTLS connections (string or parsed object) +-- * `ssl_key`: key for mTLS connections (string or parsed object) +-- * `checks.active.type`: "http", "https" or "tcp" (default is "http") +-- * `checks.active.timeout`: socket timeout for active checks (in seconds) +-- * `checks.active.concurrency`: number of targets to check concurrently +-- * `checks.active.http_method`: method to use in the HTTP request to run on active checks (default is `GET`) +-- * `checks.active.http_path`: path to use in the HTTP request to run on active checks +-- * `checks.active.http_req_body`: body to send in the HTTP request to run on active checks (a non-empty body adds a `Content-Length` header) +-- * `checks.active.https_sni`: SNI server name incase of HTTPS +-- * `checks.active.https_verify_certificate`: boolean indicating whether to verify the HTTPS certificate +-- * `checks.active.headers`: one or more lists of values indexed by header name +-- * `checks.active.healthy.interval`: interval between checks for healthy targets (in seconds) +-- * `checks.active.healthy.http_statuses`: which HTTP statuses to consider a success +-- * `checks.active.healthy.successes`: number of successes to consider a target healthy +-- * `checks.active.unhealthy.interval`: interval between checks for unhealthy targets (in seconds) +-- * `checks.active.unhealthy.http_statuses`: which HTTP statuses to consider a failure +-- * `checks.active.unhealthy.tcp_failures`: number of TCP failures to consider a target unhealthy +-- * `checks.active.unhealthy.timeouts`: number of timeouts to consider a target unhealthy +-- * `checks.active.unhealthy.http_failures`: number of HTTP failures to consider a target unhealthy +-- * `checks.passive.type`: "http", "https" or "tcp" (default is "http"; for passive checks, "http" and "https" are equivalent) +-- * `checks.passive.healthy.http_statuses`: which HTTP statuses to consider a failure +-- * `checks.passive.healthy.successes`: number of successes to consider a target healthy +-- * `checks.passive.unhealthy.http_statuses`: which HTTP statuses to consider a success +-- * `checks.passive.unhealthy.tcp_failures`: number of TCP failures to consider a target unhealthy +-- * `checks.passive.unhealthy.timeouts`: number of timeouts to consider a target unhealthy +-- * `checks.passive.unhealthy.http_failures`: number of HTTP failures to consider a target unhealthy +-- +-- If any of the health counters above (e.g. `checks.passive.unhealthy.timeouts`) +-- is set to zero, the according category of checks is not taken to account. +-- This way active or passive health checks can be disabled selectively. +-- +-- @return checker object, or `nil + error` +function _M.new(opts) + + opts = opts or {} + local active_type = (((opts or EMPTY).checks or EMPTY).active or EMPTY).type + local passive_type = (((opts or EMPTY).checks or EMPTY).passive or EMPTY).type + + local self = fill_in_settings(opts, defaults) + + load_events_module(self) + + -- If using deprecated self.type, that takes precedence over + -- a default value. TODO: remove this in a future version + if self.type then + self.checks.active.type = active_type or self.type + self.checks.passive.type = passive_type or self.type + check_valid_type("type", self.type) + end + + assert(self.checks.active.healthy.successes < 255, "checks.active.healthy.successes must be at most 254") + assert(self.checks.active.unhealthy.tcp_failures < 255, "checks.active.unhealthy.tcp_failures must be at most 254") + assert(self.checks.active.unhealthy.http_failures < 255, "checks.active.unhealthy.http_failures must be at most 254") + assert(self.checks.active.unhealthy.timeouts < 255, "checks.active.unhealthy.timeouts must be at most 254") + assert(self.checks.passive.healthy.successes < 255, "checks.passive.healthy.successes must be at most 254") + assert(self.checks.passive.unhealthy.tcp_failures < 255, "checks.passive.unhealthy.tcp_failures must be at most 254") + assert(self.checks.passive.unhealthy.http_failures < 255, "checks.passive.unhealthy.http_failures must be at most 254") + assert(self.checks.passive.unhealthy.timeouts < 255, "checks.passive.unhealthy.timeouts must be at most 254") + + if opts.test then + self.test_get_counter = test_get_counter + self.checker_callback_count = 0 + end + + assert(self.name, "required option 'name' is missing") + assert(self.shm_name, "required option 'shm_name' is missing") + + check_valid_type("checks.active.type", self.checks.active.type) + check_valid_type("checks.passive.type", self.checks.passive.type) + + self.shm = ngx.shared[tostring(opts.shm_name)] + assert(self.shm, ("no shm found by name '%s'"):format(opts.shm_name)) + + -- load certificate and key + if opts.ssl_cert and opts.ssl_key then + if type(opts.ssl_cert) == "cdata" then + self.ssl_cert = opts.ssl_cert + else + self.ssl_cert = assert(ssl.parse_pem_cert(opts.ssl_cert)) + end + + if type(opts.ssl_key) == "cdata" then + self.ssl_key = opts.ssl_key + else + self.ssl_key = assert(ssl.parse_pem_priv_key(opts.ssl_key)) + end + + end + + -- other properties + self.targets = nil -- list of targets, initially loaded, maintained by events + self.events = nil -- hash table with supported events (prevent magic strings) + self.ev_callback = nil -- callback closure per checker instance + + -- Convert status lists to sets + to_set(self.checks.active.unhealthy, "http_statuses") + to_set(self.checks.active.healthy, "http_statuses") + to_set(self.checks.passive.unhealthy, "http_statuses") + to_set(self.checks.passive.healthy, "http_statuses") + + -- decorate with methods and constants + self.events = EVENTS + for k,v in pairs(checker) do + self[k] = v + end + + -- prepare shm keys + self.TARGET_STATE = SHM_PREFIX .. self.name .. ":state" + self.TARGET_COUNTER = SHM_PREFIX .. self.name .. ":counter" + self.TARGET_LIST = SHM_PREFIX .. self.name .. ":target_list" + self.TARGET_LIST_LOCK = SHM_PREFIX .. self.name .. ":target_list_lock" + self.TARGET_LOCK = SHM_PREFIX .. self.name .. ":target_lock" + self.TARGET_PROBED = SHM_PREFIX .. self.name .. ":probed" + self.PERIODIC_LOCK = SHM_PREFIX .. ":period_lock:" + -- prepare constants + self.EVENT_SOURCE = EVENT_SOURCE_PREFIX .. " [" .. self.name .. "]" + self.LOG_PREFIX = LOG_PREFIX .. "(" .. self.name .. ") " + + -- register for events, and directly after load initial target list + -- order is important! + do + -- Lock the list, in case it is being cleared by another worker + local ok, err = locking_target_list(self, function(target_list) + + self.targets = target_list + self:log(DEBUG, "Got initial target list (", #self.targets, " targets)") + + -- load individual statuses + for _, target in ipairs(self.targets) do + local state_key = key_for(self.TARGET_STATE, target.ip, target.port, target.hostname) + target.internal_health = INTERNAL_STATES[self.shm:get(state_key)] + self:log(DEBUG, "Got initial status ", target.internal_health, " ", + target.hostname, " ", target.ip, ":", target.port) + -- fill-in the hash part for easy lookup + self.targets[target.ip] = self.targets[target.ip] or {} + self.targets[target.ip][target.port] = self.targets[target.ip][target.port] or {} + self.targets[target.ip][target.port][target.hostname or target.ip] = target + end + + return true + end) + if not ok then + -- locking failed, we don't protect `targets` of being nil in other places + -- so consider this as not recoverable + return nil, "Error loading initial target list: " .. err + end + + self.ev_callback = function(data, event) + -- just a wrapper to be able to access `self` as a closure + return self:event_handler(event, data.ip, data.port, data.hostname) + end + + -- handle events to sync up in case there was a change by another worker + worker_events:poll() + end + + -- turn on active health check + local ok, err = self:start() + if not ok then + self:stop() + return nil, err + end + + -- if active checker is not running, start it + if active_check_timer == nil then + + self:log(DEBUG, "worker ", ngx_worker_id(), " (pid: ", ngx_worker_pid(), ") ", + "starting active check timer") + local shm, key = self.shm, self.PERIODIC_LOCK + local cleanup_key = key .. ":cleanup" + active_check_timer, err = resty_timer({ + recurring = true, + interval = CHECK_INTERVAL, + jitter = CHECK_JITTER, + detached = false, + expire = function() + + local cur_time = ngx_now() + + -- Self-heal from a missed worker_events broadcast (apache/apisix#13888). + -- Unlike the probing/cleanup elections below, this runs on EVERY worker + -- EVERY tick it's due, regardless of who owns the periodic/cleanup + -- locks -- a worker with no active checker of its own can still be + -- routing traffic off a stale cached status for a checker it merely + -- reads (get_target_status), so it must reconcile too. + if cur_time - last_reconcile_time >= RECONCILE_INTERVAL then + last_reconcile_time = cur_time + for _, checker_obj in pairs(hcs) do + reconcile_target_health(checker_obj) + end + end + + -- Stale-target cleanup is decoupled from active probing and the + -- periodic lock. A passive-only deployment (checks.passive but no + -- active interval) has no active checker on any worker, yet still marks + -- targets via delayed_clear() and must purge them; gating cleanup on + -- the periodic lock (which only an active worker ever holds) would leak + -- those targets forever (apache/apisix#13385). + -- + -- A single worker per window is elected via an atomic shm:add on + -- cleanup_key (TTL = CLEANUP_INTERVAL): any worker can win, including a + -- passive-only one, so the leak fix holds, while the other workers skip + -- the pass and do not all contend on each checker's TARGET_LIST_LOCK. + -- The elected worker purges every checker in its own `hcs` in one pass. + -- Purging writes to the shared shm target_list, so cleaning a checker on + -- any one worker is globally effective; if checkers are distributed + -- asymmetrically across workers, a given upstream is purged in whichever + -- window a worker that owns its checker wins the election -- eventually + -- consistent rather than every-worker-every-window. + local won, add_err = shm:add(cleanup_key, ngx_worker_pid(), CLEANUP_INTERVAL) + if won then + for _, checker_obj in pairs(hcs) do + -- clear targets marked for delayed removal + locking_target_list(checker_obj, function(target_list) + local removed_targets = {} + local index = 1 + while index <= #target_list do + local target = target_list[index] + if target.purge_time and target.purge_time <= cur_time then + table_insert(removed_targets, target) + table_remove(target_list, index) + else + index = index + 1 + end + end + + if #removed_targets > 0 then + target_list = serialize(target_list) + + local ok, err = shm:set(checker_obj.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + for _, target in ipairs(removed_targets) do + clear_target_data_from_shm(checker_obj, target.ip, target.port, target.hostname) + checker_obj:raise_event(checker_obj.events.remove, target.ip, target.port, target.hostname) + end + end + end) + end + elseif add_err ~= "exists" then + ngx_log(ERR, "failed to elect cleanup worker for '", cleanup_key, "': ", add_err) + end + + -- The periodic lock distributes ACTIVE probing to a single worker. A + -- worker with no active checker must release the lock (if it holds it) + -- and back off, so a worker that still owns active checkers can take + -- over; otherwise active health checks stay stuck after a disable -> + -- re-enable cycle (apache/apisix#13235). This is gated on + -- has_active_checker rather than on `hcs` being non-empty, because a + -- disabled checker lingers in the weak `hcs` table with active=false + -- until it is garbage collected. + local has_active_checker = false + for _, checker_obj in pairs(hcs) do + if checker_obj.checks.active.healthy.active or + checker_obj.checks.active.unhealthy.active then + has_active_checker = true + break + end + end + + if not has_active_checker then + -- release the lock only if we are still the holder, so a worker that + -- still owns active checkers can take over + if shm:get(key) == ngx_worker_pid() then + shm:delete(key) + end + return + end + + if get_periodic_lock(shm, key) then + renew_periodic_lock(shm, key) + else + return + end + + -- active probing: only the periodic-lock holder runs the probes + for _, checker_obj in pairs(hcs) do + if checker_obj.checks.active.healthy.active and + (checker_obj.checks.active.healthy.last_run + + checker_obj.checks.active.healthy.interval <= cur_time) + then + checker_obj.checks.active.healthy.last_run = cur_time + checker_callback(checker_obj, "healthy") + end + + if checker_obj.checks.active.unhealthy.active and + (checker_obj.checks.active.unhealthy.last_run + + checker_obj.checks.active.unhealthy.interval <= cur_time) + then + checker_obj.checks.active.unhealthy.last_run = cur_time + checker_callback(checker_obj, "unhealthy") + end + end + end, + }) + if not active_check_timer then + self:log(ERR, "Could not start active check timer: ", err) + end + end + + table.insert(hcs, self) + + -- TODO: push entire config in debug level logs + self:log(DEBUG, "Healthchecker started!") + return self +end + + +function _M.get_target_list(name, shm_name) + local self = { + name = name, + shm_name = shm_name, + log = checker.log, + } + self.shm = ngx.shared[tostring(shm_name)] + assert(self.shm, ("no shm found by name '%s'"):format(shm_name)) + self.TARGET_STATE = SHM_PREFIX .. self.name .. ":state" + self.TARGET_COUNTER = SHM_PREFIX .. self.name .. ":counter" + self.TARGET_LIST = SHM_PREFIX .. self.name .. ":target_list" + self.TARGET_LIST_LOCK = SHM_PREFIX .. self.name .. ":target_list_lock" + self.TARGET_PROBED = SHM_PREFIX .. self.name .. ":probed" + self.LOG_PREFIX = LOG_PREFIX .. "(" .. self.name .. ") " + + local ok, err = locking_target_list(self, function(target_list) + self.targets = target_list + for _, target in ipairs(self.targets) do + local state_key = key_for(self.TARGET_STATE, target.ip, target.port, target.hostname) + target.status = INTERNAL_STATES[self.shm:get(state_key)] + local probed_key = key_for(self.TARGET_PROBED, target.ip, target.port, target.hostname) + target.probed = self.shm:get(probed_key) == true + if not target.hostheader then + target.hostheader = nil + end + end + + return true + end) + + for _, target in ipairs(self.targets) do + local key = key_for(self.TARGET_LOCK, target.ip, target.port, target.hostname) + local ok = run_locked(self, key, function() + local counter = self.shm:get(key_for(self.TARGET_COUNTER, + target.ip, target.port, target.hostname)) + target.counter = { + success = ctr_get(counter, CTR_SUCCESS), + http_failure = ctr_get(counter, CTR_HTTP), + tcp_failure = ctr_get(counter, CTR_TCP), + timeout_failure = ctr_get(counter, CTR_TIMEOUT), + } + return true + end) + + if not ok then + target.counter = { + success = 0, + http_failure = 0, + tcp_failure = 0, + timeout_failure = 0, + } + end + end + + if not ok then + return nil, "Error loading target list: " .. err + end + + return self.targets +end + + +-- Returns true only if the checker has at least one target AND every one of +-- them has had at least one active-check attempt ("probed" means attempted, +-- not "healthy" -- a failed or timed-out check still counts). false if there +-- are no targets yet (nothing to probe) or any target is unprobed. nil+err on +-- a shm read failure, mirroring get_target_list's own error contract. +-- +-- Deliberately a module function operating on shm (like get_target_list), +-- not a `checker:` instance method: the probed bit must be readable from any +-- worker evaluating a readiness gate, not just whichever one created the +-- checker locally or holds the periodic-probe lock. +function _M.all_targets_probed(name, shm_name) + local targets, err = _M.get_target_list(name, shm_name) + if not targets then + return nil, err + end + if #targets == 0 then + return false + end + for _, target in ipairs(targets) do + if not target.probed then + return false + end + end + return true +end + + +if TESTING then + -- test-only hook: shorten the stale-target cleanup window so the periodic + -- cleanup path can be exercised deterministically without waiting for the + -- default CLEANUP_INTERVAL (CHECK_INTERVAL * 25). + function _M._set_cleanup_interval(interval) + CLEANUP_INTERVAL = interval + end + + -- test-only hook: shorten the shm-vs-local-cache reconciliation cadence so + -- the self-heal path (apache/apisix#13888) can be exercised deterministically + -- without waiting for the default RECONCILE_INTERVAL. + function _M._set_reconcile_interval(interval) + RECONCILE_INTERVAL = interval + last_reconcile_time = 0 + end +end + + +return _M diff --git a/hack/patches/lua-resty-healthcheck-probed-gate.patch b/hack/patches/lua-resty-healthcheck-probed-gate.patch new file mode 100644 index 000000000000..24a447184a96 --- /dev/null +++ b/hack/patches/lua-resty-healthcheck-probed-gate.patch @@ -0,0 +1,126 @@ +# Local, pre-upstream fix for PS-12691's cold-start leak: a freshly created +# target defaults to internal_health = "healthy" (add_target's hardcoded +# is_healthy=true, see apisix/healthcheck_manager.lua's create_checker), so a +# pod that restarts while its backend is already unhealthy routes real +# traffic to it for several seconds -- until the first active probe corrects +# the target's state. 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." +# +# This adds a per-target "probed" flag, written to shm (not a local field, +# since only the periodic-lock-holding worker ever runs active probes, but a +# readiness-gate request can land on any worker) the first time an active +# check is actually dispatched for that target -- success, failure, or +# timeout all count; "probed" means attempted, not "healthy". A new module +# function, _M.all_targets_probed(name, shm_name), lets any worker ask +# "has every target of this checker had at least one real check yet?" +# +# Stacks on top of hack/patches/lua-resty-healthcheck-13888.patch -- apply +# that one first. Targets the same lua-resty-healthcheck-api7 3.2.3 base. +# `deps/` is gitignored and regenerated by `make deps`, so this patch is not +# applied automatically -- re-apply both patches, in order, after every +# `make deps`: +# +# patch -p3 -d deps/share/lua/5.1/resty < hack/patches/lua-resty-healthcheck-13888.patch +# patch -p3 -d deps/share/lua/5.1/resty < hack/patches/lua-resty-healthcheck-probed-gate.patch +# +# This is a local testing aid only, scoped to the cold-start readiness gate +# (apisix/healthcheck_manager.lua's ensure_checker/is_resource_probed, and +# edge-app's mollie-health-check.lua critical_upstreams_probed()). Not +# intended for an upstream PR on its own -- the reconcile patch above is the +# one being upstreamed (api7/lua-resty-healthcheck#59); this one only matters +# to how Mollie's readiness plugin uses the library, not to the library's +# own correctness. +--- a/lib/resty/healthcheck.lua ++++ b/lib/resty/healthcheck.lua +@@ -587,6 +587,10 @@ + if not ok then + self:log(ERR, "failed to clear health counter from shm: ", err) + end ++ ok, err = self.shm:set(key_for(self.TARGET_PROBED, ip, port, hostname), nil) ++ if not ok then ++ self:log(ERR, "failed to clear probed flag from shm: ", err) ++ end + end + + +@@ -1050,6 +1054,19 @@ + -- Runs a single healthcheck probe + function checker:run_single_check(ip, port, hostname, hostheader) + ++ -- Mark the target as probed *before* the attempt, not after: "probed" means ++ -- an active check was actually dispatched for this target at least once ++ -- (success, failure, or timeout all count), which is what a cold-start ++ -- readiness gate needs to know -- not whether the check succeeded. Written ++ -- to shm (not a local field) since a readiness probe can land on any ++ -- worker, not just whichever one holds the periodic-probe lock and runs ++ -- this function. ++ local probed_ok, probed_err = self.shm:set( ++ key_for(self.TARGET_PROBED, ip, port, hostname), true) ++ if not probed_ok then ++ self:log(ERR, "failed to mark target as probed in shm: ", probed_err) ++ end ++ + local sock, err = ngx.socket.tcp() + if not sock then + self:log(ERR, "failed to create stream socket: ", err) +@@ -1773,6 +1790,7 @@ + self.TARGET_LIST = SHM_PREFIX .. self.name .. ":target_list" + self.TARGET_LIST_LOCK = SHM_PREFIX .. self.name .. ":target_list_lock" + self.TARGET_LOCK = SHM_PREFIX .. self.name .. ":target_lock" ++ self.TARGET_PROBED = SHM_PREFIX .. self.name .. ":probed" + self.PERIODIC_LOCK = SHM_PREFIX .. ":period_lock:" + -- prepare constants + self.EVENT_SOURCE = EVENT_SOURCE_PREFIX .. " [" .. self.name .. "]" +@@ -1982,6 +2000,7 @@ + self.TARGET_COUNTER = SHM_PREFIX .. self.name .. ":counter" + self.TARGET_LIST = SHM_PREFIX .. self.name .. ":target_list" + self.TARGET_LIST_LOCK = SHM_PREFIX .. self.name .. ":target_list_lock" ++ self.TARGET_PROBED = SHM_PREFIX .. self.name .. ":probed" + self.LOG_PREFIX = LOG_PREFIX .. "(" .. self.name .. ") " + + local ok, err = locking_target_list(self, function(target_list) +@@ -1989,6 +2008,8 @@ + for _, target in ipairs(self.targets) do + local state_key = key_for(self.TARGET_STATE, target.ip, target.port, target.hostname) + target.status = INTERNAL_STATES[self.shm:get(state_key)] ++ local probed_key = key_for(self.TARGET_PROBED, target.ip, target.port, target.hostname) ++ target.probed = self.shm:get(probed_key) == true + if not target.hostheader then + target.hostheader = nil + end +@@ -2029,6 +2050,33 @@ + end + + ++-- Returns true only if the checker has at least one target AND every one of ++-- them has had at least one active-check attempt ("probed" means attempted, ++-- not "healthy" -- a failed or timed-out check still counts). false if there ++-- are no targets yet (nothing to probe) or any target is unprobed. nil+err on ++-- a shm read failure, mirroring get_target_list's own error contract. ++-- ++-- Deliberately a module function operating on shm (like get_target_list), ++-- not a `checker:` instance method: the probed bit must be readable from any ++-- worker evaluating a readiness gate, not just whichever one created the ++-- checker locally or holds the periodic-probe lock. ++function _M.all_targets_probed(name, shm_name) ++ local targets, err = _M.get_target_list(name, shm_name) ++ if not targets then ++ return nil, err ++ end ++ if #targets == 0 then ++ return false ++ end ++ for _, target in ipairs(targets) do ++ if not target.probed then ++ return false ++ end ++ end ++ return true ++end ++ ++ + if TESTING then + -- test-only hook: shorten the stale-target cleanup window so the periodic + -- cleanup path can be exercised deterministically without waiting for the diff --git a/t/node/healthcheck-fresh-node-default-healthy.t b/t/node/healthcheck-fresh-node-default-healthy.t new file mode 100644 index 000000000000..166ec23296fa --- /dev/null +++ b/t/node/healthcheck-fresh-node-default-healthy.t @@ -0,0 +1,274 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX 'no_plan'; + +repeat_each(1); +log_level('info'); +no_root_location(); +no_shuffle(); + +run_tests(); + +__DATA__ + +=== TEST 1: a brand-new upstream's checker is not created until the async timer runs +# Reproduces PS-12691 / the IMP-3253 incident mechanism: apisix.healthcheck_manager +# .fetch_checker() (apisix/healthcheck_manager.lua) enqueues a never-before-seen +# resource into waiting_pool and returns nil for the request that triggered it -- +# the actual checker is only built later by the timer_every(1, ...) background +# timer (timer_create_checker). Neither #13627 nor #13629 (both in 3.18.0) touch +# this path: this test must still pass (i.e. still reproduce) on current HEAD. +# +# Wrap fetch_checker() itself (same monkey-patch style as +# healthcheck-incremental-update.t) so each call logs whether it got a live +# checker back, independent of internal resource_path/version bookkeeping. +--- extra_init_worker_by_lua + local healthcheck_manager = require("apisix.healthcheck_manager") + local orig_fetch_checker = healthcheck_manager.fetch_checker + local call_count = 0 + healthcheck_manager.fetch_checker = function(...) + call_count = call_count + 1 + local checker = orig_fetch_checker(...) + ngx.log(ngx.WARN, "fetch_checker call #", call_count, + " returned a live checker: ", tostring(checker ~= nil)) + return checker + end +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local checks = [[{ + "active":{ + "http_path":"/hello", + "timeout":1, + "type":"http", + "healthy":{ "interval":1, "successes":1 }, + "unhealthy":{ "interval":1, "http_failures":1 } + } + }]] + assert(t('/apisix/admin/routes/1', ngx.HTTP_PUT, [[{ + "upstream": { + "nodes": {"127.0.0.1:1980": 1}, + "type": "roundrobin", + "checks": ]] .. checks .. [[ + }, + "uri": "/hello" + }]]) < 300) + + -- first request: fetch_checker() has never seen this resource_path, + -- so it enqueues it into waiting_pool and returns nil this call -- + -- health filtering is bypassed for this request regardless of node health + t('/hello', ngx.HTTP_GET) + + ngx.sleep(1.5) -- let timer_create_checker build the checker + + -- second request: the checker now exists in the working pool + t('/hello', ngx.HTTP_GET) + + ngx.say("done") + } + } +--- request +GET /t +--- response_body +done +--- grep_error_log eval +qr/fetch_checker call #\d returned a live checker: (?:true|false)/ +--- grep_error_log_out +fetch_checker call #1 returned a live checker: false +fetch_checker call #2 returned a live checker: true +--- timeout: 5 + + + +=== TEST 2: a freshly added target defaults to healthy with zero probes +# create_checker()/sync_checker_targets() in apisix/healthcheck_manager.lua call +# checker:add_target(host, port, check_host, true, host_hdr) -- is_healthy is a +# hardcoded literal `true`. The vendored resty.healthcheck add_target() defaults +# a target to "healthy" the instant it is registered, before any probe has run. +# With healthy/unhealthy probe intervals set to 0 (disabled), no probe can ever +# fire, so any "healthy" reading here is provably the zero-probe default, not the +# result of a successful check. +--- config + location /t { + content_by_lua_block { + local healthcheck = require("resty.healthcheck") + + local checker = healthcheck.new({ + name = "test-ps12691-default-healthy", + shm_name = "upstream-healthcheck", + checks = { + active = { + healthy = { interval = 0 }, + unhealthy = { interval = 0 }, + }, + }, + events_module = "resty.events", + }) + if not checker then + ngx.say("failed to create checker") + return + end + + -- mirror healthcheck_manager.create_checker()'s call exactly: + -- add_target(host, port, check_host, true, host_hdr) + local ok, err = checker:add_target("127.0.0.1", 19791, nil, true, nil) + if not ok then + ngx.say("failed to add target: ", err) + return + end + ngx.sleep(0.2) -- let add_target's own event settle locally; no probe possible (interval=0) + + local status = checker:get_target_status("127.0.0.1", 19791) + ngx.say("fresh, never-probed target status: ", tostring(status)) + + checker:stop() + } + } +--- request +GET /t +--- response_body +fresh, never-probed target status: true +--- no_error_log +[error] +--- timeout: 5 + + + +=== TEST 3: all_targets_probed() is false until the first real active check fires +# Closes the gap TEST 2 exposes: a cold-start readiness gate needs a way to +# tell "healthy by default, never checked" apart from "healthy, actually +# checked" from outside the checker. resty.healthcheck.all_targets_probed() +# (hack/patches/lua-resty-healthcheck-probed-gate.patch) reads a per-target +# shm flag written the first time run_single_check actually dispatches a +# probe -- attempted, not "passed". Uses a real active interval (1s) against +# 127.0.0.1:1980, the standard t::APISIX mock backend, which answers /hello. +--- config + location /t { + content_by_lua_block { + local healthcheck = require("resty.healthcheck") + + local checker = healthcheck.new({ + name = "test-ps12691-all-targets-probed", + shm_name = "upstream-healthcheck", + checks = { + active = { + type = "http", + http_path = "/hello", + timeout = 1, + healthy = { interval = 1, successes = 1 }, + unhealthy = { interval = 1, http_failures = 1 }, + }, + }, + events_module = "resty.events", + }) + if not checker then + ngx.say("failed to create checker") + return + end + + local ok, err = checker:add_target("127.0.0.1", 1980, nil, true, nil) + if not ok then + ngx.say("failed to add target: ", err) + return + end + ngx.sleep(0.2) -- let add_target's own event settle locally + + local before = healthcheck.all_targets_probed( + "test-ps12691-all-targets-probed", "upstream-healthcheck") + ngx.say("before any active check has run: ", tostring(before)) + + ngx.sleep(1.5) -- past the 1s active.healthy.interval: one probe must have fired + + local after = healthcheck.all_targets_probed( + "test-ps12691-all-targets-probed", "upstream-healthcheck") + ngx.say("after the first active check: ", tostring(after)) + + checker:stop() + } + } +--- request +GET /t +--- response_body +before any active check has run: false +after the first active check: true +--- no_error_log +[error] +--- timeout: 5 + + + +=== TEST 4: ensure_checker() builds a checker with zero prior traffic +# Closes TEST 1's gap directly: healthcheck_manager.ensure_checker() +# (apisix/healthcheck_manager.lua) seeds waiting_pool proactively, without +# needing any request to fetch_checker() first -- unlike TEST 1, no t('/hello', +# ...) request is made at all before the checker is expected to exist. +--- extra_init_worker_by_lua + local healthcheck_manager = require("apisix.healthcheck_manager") + local orig_fetch_checker = healthcheck_manager.fetch_checker + local call_count = 0 + healthcheck_manager.fetch_checker = function(...) + call_count = call_count + 1 + return orig_fetch_checker(...) + end +--- config + location /t { + content_by_lua_block { + local t = require("lib.test_admin").test + local healthcheck_manager = require("apisix.healthcheck_manager") + local checks = [[{ + "active":{ + "http_path":"/hello", + "timeout":1, + "type":"http", + "healthy":{ "interval":1, "successes":1 }, + "unhealthy":{ "interval":1, "http_failures":1 } + } + }]] + -- a standalone /upstreams/ object, matching how a real + -- "critical upstream" is configured (apisixUpstreams entries), + -- not an inline route upstream -- an inline upstream's + -- resource_key is the owning route's key ("/routes/"), not + -- "/upstreams/", which is the identifier space + -- ensure_checker/is_resource_probed are scoped to. + assert(t('/apisix/admin/upstreams/ensure-checker-test', ngx.HTTP_PUT, [[{ + "nodes": {"127.0.0.1:1980": 1}, + "type": "roundrobin", + "checks": ]] .. checks .. [[ + }]]) < 300) + assert(t('/apisix/admin/routes/ensure-checker-test', ngx.HTTP_PUT, [[{ + "upstream_id": "ensure-checker-test", + "uri": "/ensure-checker-test" + }]]) < 300) + + -- no request to /ensure-checker-test at all -- fetch_checker() has + -- never been called for this resource by real traffic + local ok, err = healthcheck_manager.ensure_checker("/upstreams/ensure-checker-test") + ngx.say("ensure_checker: ", tostring(ok), " ", tostring(err)) + + ngx.sleep(1.5) -- let timer_create_checker build the checker on its next tick + + ngx.say("resource probed after ensure_checker with zero traffic: ", + tostring(healthcheck_manager.is_resource_probed("/upstreams/ensure-checker-test"))) + } + } +--- request +GET /t +--- response_body_like +ensure_checker: true nil +resource probed after ensure_checker with zero traffic: true +--- timeout: 5 From 6df04e776f917542aec3adc6baee3bf86ff1cb12 Mon Sep 17 00:00:00 2001 From: Andre Nogueira Date: Fri, 28 Aug 2026 00:47:16 +0100 Subject: [PATCH 2/4] test(healthcheck): add local patch and regression tests for apache/apisix#13888 The lua-resty-healthcheck-probed-gate.patch committed in 12fa7f29 stacks on this reconcile patch (its own header already documents that), but the patch file itself was left out of that commit -- add it now, along with the two regression tests that reproduce the bug it fixes: a worker that misses the worker_events broadcast for a target's health-state change never re-converges with shm, since the broadcast has no delivery guarantee and is never raised again once a target is already at the reported state. Already raised upstream as api7/lua-resty-healthcheck#59; this local copy is for this repo's own kind-repro validation harness. Signed-off-by: Andre Nogueira --- .../healthcheck-13888-patched-full.lua | 2050 +++++++++++++++++ .../patches/lua-resty-healthcheck-13888.patch | 179 ++ ...hcheck-missed-event-reconcile-stockcheck.t | 103 + t/node/healthcheck-missed-event-reconcile.t | 117 + 4 files changed, 2449 insertions(+) create mode 100644 hack/patches/healthcheck-13888-patched-full.lua create mode 100644 hack/patches/lua-resty-healthcheck-13888.patch create mode 100644 t/node/healthcheck-missed-event-reconcile-stockcheck.t create mode 100644 t/node/healthcheck-missed-event-reconcile.t diff --git a/hack/patches/healthcheck-13888-patched-full.lua b/hack/patches/healthcheck-13888-patched-full.lua new file mode 100644 index 000000000000..147c9e51560f --- /dev/null +++ b/hack/patches/healthcheck-13888-patched-full.lua @@ -0,0 +1,2050 @@ +-------------------------------------------------------------------------- +-- Healthcheck library for OpenResty. +-- +-- Some notes on the usage of this library: +-- +-- - Each target will have 4 counters, 1 success counter and 3 failure +-- counters ('http', 'tcp', and 'timeout'). Any failure will _only_ reset the +-- success counter, but a success will reset _all three_ failure counters. +-- +-- - All targets are uniquely identified by their IP address and port number +-- combination, most functions take those as arguments. +-- +-- - All keys in the SHM will be namespaced by the healthchecker name as +-- provided to the `new` function. Hence no collissions will occur on shm-keys +-- as long as the `name` is unique. +-- +-- - Active healthchecks will be synchronized across workers, such that only +-- a single active healthcheck runs. +-- +-- - Events will be raised in every worker, see [lua-resty-worker-events](https://github.com/Kong/lua-resty-worker-events) +-- for details. +-- +-- @copyright 2017-2023 Kong Inc. +-- @author Hisham Muhammad, Thijs Schreijer +-- @license Apache 2.0 + +local ERR = ngx.ERR +local WARN = ngx.WARN +local DEBUG = ngx.DEBUG +local ngx_log = ngx.log +local tostring = tostring +local ipairs = ipairs +local table_insert = table.insert +local table_remove = table.remove +local string_format = string.format +local ssl = require("ngx.ssl") +local resty_timer = require "resty.timer" +local bit = require("bit") +local re_find = ngx.re.find +local ngx_now = ngx.now +local ngx_worker_id = ngx.worker.id +local ngx_worker_pid = ngx.worker.pid +local pcall = pcall +local get_phase = ngx.get_phase +local type = type +local assert = assert + + +local RESTY_EVENTS_VER = [[^0\.1\.\d+$]] +local RESTY_WORKER_EVENTS_VER = "0.3.3" + + +local new_tab +local nkeys +local is_array +local codec + + +local TESTING = _G.__TESTING_HEALTHCHECKER or false + +do + local ok + + ok, new_tab = pcall(require, "table.new") + if not ok then + new_tab = function () return {} end + end + + -- OpenResty branch of LuaJIT New API + ok, nkeys = pcall(require, "table.nkeys") + if not ok then + nkeys = function (tab) + local count = 0 + for _, v in pairs(tab) do + if v ~= nil then + count = count + 1 + end + end + return count + end + end + + ok, is_array = pcall(require, "table.isarray") + if not ok then + is_array = function(t) + for k in pairs(t) do + if type(k) ~= "number" or math.floor(k) ~= k then + return false + end + end + return true + end + end + + ok, codec = pcall(require, "string.buffer") + if not ok then + codec = require("cjson.safe").new() + end +end + + +local worker_events +--- This function loads the worker events module received as arg. It will throw +-- error() if it is not possible to load the module. +local function load_events_module(self) + if self.events_module == "resty.worker.events" then + worker_events = require("resty.worker.events") + assert(worker_events, "could not load lua-resty-worker-events") + assert(worker_events._VERSION == RESTY_WORKER_EVENTS_VER, + "unsupported lua-resty-worker-events version") + + elseif self.events_module == "resty.events" then + worker_events = require("resty.events.compat") + local version_match = ngx.re.match(worker_events._VERSION, RESTY_EVENTS_VER, "o") + assert(version_match, "unsupported lua-resty-events version") + + else + error("unknown events module") + end + + assert(worker_events.configured(), "please configure the '" .. + self.events_module .. "' module before using 'lua-resty-healthcheck'") +end + + +-- constants +local EVENT_SOURCE_PREFIX = "lua-resty-healthcheck" +local LOG_PREFIX = "[healthcheck] " +local SHM_PREFIX = "lua-resty-healthcheck:" +local EMPTY = setmetatable({},{ + __newindex = function() + error("the EMPTY table is read only, check your code!", 2) + end + }) + +--- timer constants +-- evaluate active checks every 0.1s +local CHECK_INTERVAL = 0.1 +-- use a 10% jitter to start each worker timer +local CHECK_JITTER = CHECK_INTERVAL * 0.1 +-- lock valid period: the worker which acquires the lock owns it for 15 times +-- the check interval. If it does not update the shm during this period, we +-- consider that it is not able to continue checking (the worker probably was killed) +local LOCK_PERIOD = CHECK_INTERVAL * 15 + +-- Only the periodic-lock holder ever runs active probes (see active_check_timer +-- below), so every other worker's target.internal_health is updated *solely* by +-- the worker_events broadcast raised in incr_counter. That broadcast has no +-- delivery guarantee and, once a target is already at the reported health, is +-- never raised again for the same state -- so a single missed event permanently +-- strands a worker's local view (apache/apisix#13888). RECONCILE_INTERVAL bounds +-- how long that divergence can last: every worker re-derives internal_health +-- from the authoritative shm state on this cadence, independent of events. +local RECONCILE_INTERVAL = 1 +-- interval between stale targets cleanup +local CLEANUP_INTERVAL = CHECK_INTERVAL * 25 + +-- Counters: a 32-bit shm integer can hold up to four 8-bit counters. +local CTR_SUCCESS = 0x00000001 +local CTR_HTTP = 0x00000100 +local CTR_TCP = 0x00010000 +local CTR_TIMEOUT = 0x01000000 + +local MASK_FAILURE = 0xffffff00 +local MASK_SUCCESS = 0x000000ff + +local COUNTER_NAMES = { + [CTR_SUCCESS] = "SUCCESS", + [CTR_HTTP] = "HTTP", + [CTR_TCP] = "TCP", + [CTR_TIMEOUT] = "TIMEOUT", +} + +--- The list of potential events generated. +-- The `checker.EVENT_SOURCE` field can be used to subscribe to the events, see the +-- example below. Each of the events will get a table passed containing +-- the target details `ip`, `port`, and `hostname`. +-- See [lua-resty-worker-events](https://github.com/Kong/lua-resty-worker-events). +-- @field remove Event raised when a target is removed from the checker. +-- @field healthy This event is raised when the target status changed to +-- healthy (and when a target is added as `healthy`). +-- @field unhealthy This event is raised when the target status changed to +-- unhealthy (and when a target is added as `unhealthy`). +-- @field mostly_healthy This event is raised when the target status is +-- still healthy but it started to receive "unhealthy" updates via active or +-- passive checks. +-- @field mostly_unhealthy This event is raised when the target status is +-- still unhealthy but it started to receive "healthy" updates via active or +-- passive checks. +-- @table checker.events +-- @usage -- Register for all events from `my_checker` +-- local event_callback = function(target, event, source, source_PID) +-- local t = target.ip .. ":" .. target.port .." by name '" .. +-- target.hostname .. "' ") +-- +-- if event == my_checker.events.remove then +-- print(t .. "has been removed") +-- elseif event == my_checker.events.healthy then +-- print(t .. "is now healthy") +-- elseif event == my_checker.events.unhealthy then +-- print(t .. "is now unhealthy") +-- end +-- end +-- +-- worker_events.register(event_callback, my_checker.EVENT_SOURCE) +local EVENTS = setmetatable({}, { + __index = function(self, key) + error(("'%s' is not a valid event name"):format(tostring(key))) + end +}) +for _, event in ipairs({ + "remove", + "healthy", + "unhealthy", + "mostly_healthy", + "mostly_unhealthy", + "clear", +}) do + EVENTS[event] = event +end + +local INTERNAL_STATES = {} +for i, key in ipairs({ + "healthy", + "unhealthy", + "mostly_healthy", + "mostly_unhealthy", +}) do + INTERNAL_STATES[i] = key + INTERNAL_STATES[key] = i +end + +-- Some color for demo purposes +local use_color = false +local id = function(x) return x end +local worker_color = use_color and function(str) return ("\027["..tostring(31 + ngx_worker_pid() % 5).."m"..str.."\027[0m") end or id + +-- Debug function +local function dump(...) print(require("pl.pretty").write({...})) end -- luacheck: ignore 211 + +local _M = {} + +-- checker objects (weak) table +local hcs = setmetatable({}, { + __mode = "v", +}) + +local active_check_timer + +-- last time (ngx.now()) the shm-vs-local-cache reconciliation sweep ran; shared +-- by all checkers on this worker since the sweep itself iterates `hcs` +local last_reconcile_time = 0 + +-- serialize a table to a string +local serialize = codec.encode + + +-- deserialize a string to a table +local deserialize = codec.decode + + +local function key_for(key_prefix, ip, port, hostname) + return string_format("%s:%s:%s%s", key_prefix, ip, port, hostname and ":" .. hostname or "") +end + + +-- resty.lock timeout when yieldable +local LOCK_TIMEOUT = 5 + +local run_locked +do + -- resty_lock is restricted to this scope in order to keep sensitive + -- lock-handling code separate separate from all other business logic + -- + -- If you need to use resty_lock in a way that is not covered by the + -- `run_locked` helper function defined below, it's strongly-advised to + -- define it fully within this scope unless you have a very good reason + -- + -- (see https://github.com/Kong/lua-resty-healthcheck/pull/112) + local resty_lock = require "resty.lock" + + local yieldable = { + rewrite = true, + access = true, + content = true, + timer = true, + } + + local function run_in_timer(premature, self, key, fn, ...) + if premature then + return + end + + local ok, err = run_locked(self, key, fn, ...) + if not ok then + self:log(ERR, "locked function for key '", key, "' failed in timer: ", err) + end + end + + local function schedule(self, key, fn, ...) + local ok, err = ngx.timer.at(0, run_in_timer, self, key, fn, ...) + if not ok then + return nil, "failed scheduling locked function for key '" .. key .. + "', " .. err + end + + return "scheduled" + end + + -- resty.lock consumes these options immediately, so this table can be reused + local opts = { + exptime = 10, -- timeout after which lock is released anyway + timeout = LOCK_TIMEOUT, -- max wait time to acquire lock + } + + --- + -- Acquire a lock and run a function + -- + -- The function call itself is wrapped with `pcall` to protect against + -- exception. + -- + -- This function exhibits some special behavior when called during a + -- non-yieldable phase such as `init_worker` or `log`: + -- + -- 1. The lock timeout is set to 0 to ensure that `resty.lock` does not + -- attempt to sleep/yield + -- 2. If acquiring the lock fails due to a timeout, `run_locked` + -- (this function) is re-scheduled to run in a timer. In this case, + -- the function returns `"scheduled"` + -- + -- @param self The checker object + -- @param key the key/identifier to acquire a lock for + -- @param fn The function to execute + -- @param ... arguments that will be passed to fn + -- @return The results of the function; or nil and an error message + -- in case it fails locking. + function run_locked(self, key, fn, ...) + -- we're extra extra extra defensive in this code path + local typ = type(key) + -- XXX is a number key ever expected? + assert(typ == "string" or typ == "number", + "unexpected lock key type: " .. typ) + key = tostring(key) + + -- first aqcuire a lock or conditionally re-schedule ourselves in a timer + local lock + do + local yield = yieldable[get_phase()] + + if yield then + opts.timeout = LOCK_TIMEOUT + else + -- if yielding is not possible in the current phase, use a zero timeout + -- so that resty.lock will return `nil, "timeout"` immediately instead of + -- calling ngx.sleep() + opts.timeout = 0 + end + + local err + lock, err = resty_lock:new(self.shm_name, opts) + if not lock then + return nil, "failed creating lock for '" .. key .. "', " .. err + end + + local elapsed + elapsed, err = lock:lock(key) + + if not elapsed and err == "timeout" and not yield then + -- yielding is not possible in the current phase, so retry in a timer + return schedule(self, key, fn, ...) + + elseif not elapsed then + return nil, "failed acquiring lock for '" .. key .. "', " .. err + end + end + + local pok, perr, res = pcall(fn, ...) + + local ok, err = lock:unlock() + if not ok then + self:log(ERR, "failed unlocking '", key, "', ", err) + end + + if not pok then + return nil, "locked function threw an exception: " .. tostring(perr) + end + + return perr, res + end +end + + +local deepcopy +do + local function _deepcopy(orig, copied) + -- prevent infinite loop when a field refers its parent + copied[orig] = true + -- If the array-like table contains nil in the middle, + -- the len might be smaller than the expected. + -- But it doesn't affect the correctness. + local len = #orig + local copy = table.new(len, table.nkeys(orig) - len) + for orig_key, orig_value in pairs(orig) do + if type(orig_value) == "table" and not copied[orig_value] then + copy[orig_key] = _deepcopy(orig_value, copied) + else + copy[orig_key] = orig_value + end + end + + local mt = getmetatable(orig) + if mt ~= nil then + setmetatable(copy, mt) + end + + return copy + end + + + local copied_recorder = {} + + function deepcopy(orig) + local orig_type = type(orig) + if orig_type ~= 'table' then + return orig + end + + local res = _deepcopy(orig, copied_recorder) + table.clear(copied_recorder) + return res + end +end + + +local checker = {} + + +------------------------------------------------------------------------------ +-- Node management. +-- @section node-management +------------------------------------------------------------------------------ + + +-- @return the target list from the shm, an empty table if not found, or +-- `nil + error` upon a failure +local function fetch_target_list(self) + local target_list, err = self.shm:get(self.TARGET_LIST) + if err then + return nil, "failed to fetch target_list from shm: " .. err + end + + return target_list and deserialize(target_list) or {} +end + + +local function with_target_list(self, fn) + local targets, err = fetch_target_list(self) + if not targets then + return nil, err + end + + -- this is only ever called in the context of `run_locked`, + -- so no pcall needed + return fn(targets) +end + + +--- Run the given function holding a lock on the target list. +-- @param self The checker object +-- @param fn The function to execute +-- @return The results of the function; or nil and an error message +-- in case it fails locking. +local function locking_target_list(self, fn) + local ok, err = run_locked(self, self.TARGET_LIST_LOCK, with_target_list, self, fn) + + if ok == "scheduled" then + self:log(DEBUG, "target_list function re-scheduled in timer") + end + + return ok, err +end + + +--- Get a target +local function get_target(self, ip, port, hostname) + hostname = hostname or ip + return ((self.targets[ip] or EMPTY)[port] or EMPTY)[hostname] +end + +--- Add a target to the healthchecker. +-- When the ip + port + hostname combination already exists, it will simply +-- return success (without updating `is_healthy` status). +-- @param ip IP address of the target to check. +-- @param port the port to check against. +-- @param hostname (optional) hostname to set as the host header in the HTTP +-- probe request +-- @param is_healthy (optional) a boolean value indicating the initial state, +-- default is `true`. +-- @param hostheader (optional) a value to use for the Host header on +-- active healthchecks. +-- @param tbl_meta (optional) a lua table with custom info of business stuff +-- @return `true` on success, or `nil + error` on failure. +function checker:add_target(ip, port, hostname, is_healthy, hostheader, tbl_meta) + ip = tostring(assert(ip, "no ip address provided")) + port = assert(tonumber(port), "no port number provided") + hostname = hostname or ip + if is_healthy == nil then + is_healthy = true + end + + local internal_health = is_healthy and "healthy" or "unhealthy" + + local ok, err = locking_target_list(self, function(target_list) + local found = false + + -- check whether we already have this target + for _, target in ipairs(target_list) do + if target.ip == ip and target.port == port and target.hostname == (hostname) then + if target.purge_time == nil then + self:log(DEBUG, "adding an existing target: ", hostname or "", " ", ip, + ":", port, " (ignoring)") + return false + end + target.purge_time = nil + found = true + internal_health = self:get_target_status(ip, port, hostname) and + "healthy" or "unhealthy" + break + end + end + + -- we first add the internal health, and only then the updated list. + -- this prevents a state where a target is in the list, but does not + -- have a key in the shm. + local ok, err = self.shm:set(key_for(self.TARGET_STATE, ip, port, hostname), + INTERNAL_STATES[internal_health]) + if not ok then + self:log(ERR, "failed to set initial health status in shm: ", err) + end + + -- target does not exist, go add it + if not found then + target_list[#target_list + 1] = { + ip = ip, + port = port, + hostname = hostname, + hostheader = hostheader, + meta = tbl_meta, + } + end + target_list = serialize(target_list) + + ok, err = self.shm:set(self.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + -- raise event for our newly added target + if not found then + self:raise_event(self.events[internal_health], ip, port, hostname) + end + + return true + end) + + if ok == false then + -- the target already existed, no event, but still success + return true + end + + return ok, err + +end + + +-- Remove health status entries from an individual target from shm +-- @param self The checker object +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname hostname of the target being checked. +local function clear_target_data_from_shm(self, ip, port, hostname) + local ok, err = self.shm:set(key_for(self.TARGET_STATE, ip, port, hostname), nil) + if not ok then + self:log(ERR, "failed to remove health status from shm: ", err) + end + ok, err = self.shm:set(key_for(self.TARGET_COUNTER, ip, port, hostname), nil) + if not ok then + self:log(ERR, "failed to clear health counter from shm: ", err) + end +end + + +--- Remove a target from the healthchecker. +-- The target not existing is not considered an error. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @return `true` on success, or `nil + error` on failure. +function checker:remove_target(ip, port, hostname) + ip = tostring(assert(ip, "no ip address provided")) + port = assert(tonumber(port), "no port number provided") + + return locking_target_list(self, function(target_list) + + -- find the target + local target_found + for i, target in ipairs(target_list) do + if target.ip == ip and target.port == port and target.hostname == hostname then + target_found = target + table_remove(target_list, i) + break + end + end + + if not target_found then + return true + end + + -- go update the shm + target_list = serialize(target_list) + + -- we first write the updated list, and only then remove the health + -- status; this prevents race conditions when a healthchecker gets the + -- initial state from the shm + local ok, err = self.shm:set(self.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + clear_target_data_from_shm(self, ip, port, hostname) + + -- raise event for our removed target + self:raise_event(self.events.remove, ip, port, hostname) + + return true + end) +end + + +--- Clear all healthcheck data. +-- @return `true` on success, or `nil + error` on failure. +function checker:clear() + + return locking_target_list(self, function(target_list) + + local old_target_list = target_list + + -- go update the shm + target_list = serialize({}) + + local ok, err = self.shm:set(self.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + -- remove all individual statuses + for _, target in ipairs(old_target_list) do + local ip, port, hostname = target.ip, target.port, target.hostname + clear_target_data_from_shm(self, ip, port, hostname) + end + + self.targets = {} + + -- raise event for our removed target + self:raise_event(self.events.clear) + + return true + end) +end + + +--- Clear all healthcheck data after a period of time. +-- Useful for keeping target status between configuration reloads. +-- @param delay delay in seconds before purging target state. +-- @return `true` on success, or `nil + error` on failure. +function checker:delayed_clear(delay) + assert(tonumber(delay), "no delay provided") + + return locking_target_list(self, function(target_list) + local purge_time = ngx_now() + delay + + -- add purge time to all targets + for _, target in ipairs(target_list) do + target.purge_time = purge_time + end + + target_list = serialize(target_list) + local ok, err = self.shm:set(self.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + return true + end) +end + + +--- Get the current status of the target. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname the hostname of the target being checked. +-- @return `true` if healthy, `false` if unhealthy, or `nil + error` on failure. +function checker:get_target_status(ip, port, hostname) + + local target = get_target(self, ip, port, hostname) + if not target then + return nil, "target not found" + end + return target.internal_health == "healthy" + or target.internal_health == "mostly_healthy" + +end + + +------------------------------------------------------------------------------ +-- Health management. +-- Functions that allow reporting of failures/successes for passive checks. +-- @section health-management +------------------------------------------------------------------------------ + + +-- Run the given function holding a lock on the target. +-- @param self The checker object +-- @param ip Target IP +-- @param port Target port +-- @param hostname Target hostname +-- @param fn The function to execute +-- @return The results of the function; or true in case it fails locking and +-- will retry asynchronously; or nil+err in case it fails to retry. +local function locking_target(self, ip, port, hostname, fn) + local key = key_for(self.TARGET_LOCK, ip, port, hostname) + + local ok, err = run_locked(self, key, fn) + + if ok == "scheduled" then + self:log(DEBUG, "target function for ", key, " was re-scheduled") + end + + return ok, err +end + + +-- Extract the value of the counter at `idx` from multi-counter `multictr`. +-- @param multictr A 32-bit multi-counter holding 4 values. +-- @param idx The shift index specifying which counter to get. +-- @return The 8-bit value extracted from the 32-bit multi-counter. +local function ctr_get(multictr, idx) + return bit.band(multictr / idx, 0xff) +end + + +-- Increment the healthy or unhealthy counter. If the threshold of occurrences +-- is reached, it changes the status of the target in the shm and posts an +-- event. +-- @param self The checker object +-- @param health_report "healthy" for the success counter that drives a target +-- towards the healthy state; "unhealthy" for the failure counter. +-- @param ip Target IP +-- @param port Target port +-- @param hostname Target hostname +-- @param limit the limit after which target status is changed +-- @param ctr_type the counter to increment, see CTR_xxx constants +-- @return True if succeeded, or nil and an error message. +local function incr_counter(self, health_report, ip, port, hostname, limit, ctr_type) + + -- fail fast on counters that are disabled by configuration + if limit == 0 then + return true + end + + hostname = hostname or ip + port = tonumber(port) + local target = get_target(self, ip, port, hostname) + if not target then + -- sync issue: warn, but return success + self:log(WARN, "trying to increment a target that is not in the list: ", + hostname and "(" .. hostname .. ") " or "", ip, ":", port) + return true + end + + local current_health = target.internal_health + if health_report == current_health then + -- No need to count successes when internal health is fully "healthy" + -- or failures when internal health is fully "unhealthy" + return true + end + + return locking_target(self, ip, port, hostname, function() + local counter_key = key_for(self.TARGET_COUNTER, ip, port, hostname) + local multictr, err = self.shm:incr(counter_key, ctr_type, 0) + if err then + return nil, err + end + + local ctr = ctr_get(multictr, ctr_type) + + self:log(WARN, health_report, " ", COUNTER_NAMES[ctr_type], + " increment (", ctr, "/", limit, ") for '", hostname or "", + "(", ip, ":", port, ")'") + + local new_multictr + if ctr_type == CTR_SUCCESS then + new_multictr = bit.band(multictr, MASK_SUCCESS) + else + new_multictr = bit.band(multictr, MASK_FAILURE) + end + + if new_multictr ~= multictr then + self.shm:set(counter_key, new_multictr) + end + + local new_health + if ctr >= limit then + new_health = health_report + elseif current_health == "healthy" and bit.band(new_multictr, MASK_FAILURE) > 0 then + new_health = "mostly_healthy" + elseif current_health == "unhealthy" and bit.band(new_multictr, MASK_SUCCESS) > 0 then + new_health = "mostly_unhealthy" + end + + if new_health and new_health ~= current_health then + local state_key = key_for(self.TARGET_STATE, ip, port, hostname) + self.shm:set(state_key, INTERNAL_STATES[new_health]) + self:raise_event(self.events[new_health], ip, port, hostname) + end + + return true + + end) + +end + + +--- Report a health failure. +-- Reports a health failure which will count against the number of occurrences +-- required to make a target "fall". The type of healthchecker, +-- "tcp" or "http" (see `new`) determines against which counter the occurence goes. +-- If `unhealthy.tcp_failures` (for TCP failures) or `unhealthy.http_failures` +-- is set to zero in the configuration, this function is a no-op +-- and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, or `nil + error` on failure. +function checker:report_failure(ip, port, hostname, check) + + local checks = self.checks[check or "passive"] + local limit, ctr_type + if self.checks[check or "passive"].type == "tcp" then + limit = checks.unhealthy.tcp_failures + ctr_type = CTR_TCP + else + limit = checks.unhealthy.http_failures + ctr_type = CTR_HTTP + end + + return incr_counter(self, "unhealthy", ip, port, hostname, limit, ctr_type) + +end + + +--- Report a health success. +-- Reports a health success which will count against the number of occurrences +-- required to make a target "rise". +-- If `healthy.successes` is set to zero in the configuration, +-- this function is a no-op and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, or `nil + error` on failure. +function checker:report_success(ip, port, hostname, check) + + local limit = self.checks[check or "passive"].healthy.successes + + return incr_counter(self, "healthy", ip, port, hostname, limit, CTR_SUCCESS) + +end + + +--- Report a http response code. +-- How the code is interpreted is based on the configuration for healthy and +-- unhealthy statuses. If it is in neither strategy, it will be ignored. +-- If `healthy.successes` (for healthy HTTP status codes) +-- or `unhealthy.http_failures` (fur unhealthy HTTP status codes) +-- is set to zero in the configuration, this function is a no-op +-- and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @param http_status the http statuscode, or nil to report an invalid http response. +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, `nil` if the status was ignored (not in active or +-- passive health check lists) or `nil + error` on failure. +function checker:report_http_status(ip, port, hostname, http_status, check) + http_status = tonumber(http_status) or 0 + + local checks = self.checks[check or "passive"] + + local status_type, limit, ctr + if checks.healthy.http_statuses[http_status] then + status_type = "healthy" + limit = checks.healthy.successes + ctr = CTR_SUCCESS + elseif checks.unhealthy.http_statuses[http_status] + or http_status == 0 then + status_type = "unhealthy" + limit = checks.unhealthy.http_failures + ctr = CTR_HTTP + else + return + end + + return incr_counter(self, status_type, ip, port, hostname, limit, ctr) + +end + +--- Report a failure on TCP level. +-- If `unhealthy.tcp_failures` is set to zero in the configuration, +-- this function is a no-op and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname hostname of the target being checked. +-- @param operation The socket operation that failed: +-- "connect", "send" or "receive". +-- TODO check what kind of information we get from the OpenResty layer +-- in order to tell these error conditions apart +-- https://github.com/openresty/lua-resty-core/blob/master/lib/ngx/balancer.md#get_last_failure +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, or `nil + error` on failure. +function checker:report_tcp_failure(ip, port, hostname, operation, check) + + local limit = self.checks[check or "passive"].unhealthy.tcp_failures + + -- TODO what do we do with the `operation` information + return incr_counter(self, "unhealthy", ip, port, hostname, limit, CTR_TCP) + +end + + +--- Report a timeout failure. +-- If `unhealthy.timeouts` is set to zero in the configuration, +-- this function is a no-op and returns `true`. +-- @param ip IP address of the target being checked. +-- @param port the port being checked against. +-- @param hostname (optional) hostname of the target being checked. +-- @param check (optional) the type of check, either "passive" or "active", default "passive". +-- @return `true` on success, or `nil + error` on failure. +function checker:report_timeout(ip, port, hostname, check) + + local limit = self.checks[check or "passive"].unhealthy.timeouts + + return incr_counter(self, "unhealthy", ip, port, hostname, limit, CTR_TIMEOUT) + +end + + +--- Sets the current status of all targets with the given hostname and port. +-- @param hostname hostname being checked. +-- @param port the port being checked against +-- @param is_healthy boolean: `true` for healthy, `false` for unhealthy +-- @return `true` on success, or `nil + error` on failure. +function checker:set_all_target_statuses_for_hostname(hostname, port, is_healthy) + assert(type(hostname) == "string", "no hostname provided") + port = assert(tonumber(port), "no port number provided") + assert(type(is_healthy) == "boolean") + + local all_ok = true + local errs = {} + for _, target in ipairs(self.targets) do + if target.port == port and target.hostname == hostname then + local ok, err = self:set_target_status(target.ip, port, hostname, is_healthy) + if not ok then + all_ok = nil + table.insert(errs, err) + end + end + end + + return all_ok, #errs > 0 and table.concat(errs, "; ") or nil +end + + +--- Sets the current status of the target. +-- This will immediately set the status and clear its counters. +-- @param ip IP address of the target being checked +-- @param port the port being checked against +-- @param hostname (optional) hostname of the target being checked. +-- @param is_healthy boolean: `true` for healthy, `false` for unhealthy +-- @return `true` on success, or `nil + error` on failure +function checker:set_target_status(ip, port, hostname, is_healthy) + ip = tostring(assert(ip, "no ip address provided")) + port = assert(tonumber(port), "no port number provided") + assert(type(is_healthy) == "boolean") + hostname = hostname or ip + + local health_report = is_healthy and "healthy" or "unhealthy" + + local target = get_target(self, ip, port, hostname) + if not target then + -- sync issue: warn, but return success + self:log(WARN, "trying to set status for a target that is not in the list: ", ip, ":", port) + return true + end + + local counter_key = key_for(self.TARGET_COUNTER, ip, port, hostname) + local state_key = key_for(self.TARGET_STATE, ip, port, hostname) + + local ok, err = locking_target(self, ip, port, hostname, function() + + local _, err = self.shm:set(counter_key, 0) + if err then + return nil, err + end + + self.shm:set(state_key, INTERNAL_STATES[health_report]) + if err then + return nil, err + end + + self:raise_event(self.events[health_report], ip, port, hostname) + + return true + + end) + + if ok then + self:log(WARN, health_report, " forced for ", hostname, " ", ip, ":", port) + end + return ok, err +end + + +-- Introspection function for testing +local function test_get_counter(self, ip, port, hostname) + return locking_target(self, ip, port, hostname, function() + local counter = self.shm:get(key_for(self.TARGET_COUNTER, ip, port, hostname)) + local internal_health = (get_target(self, ip, port, hostname) or EMPTY).internal_health + return counter, internal_health + end) +end + + +--============================================================================ +-- Healthcheck runner +--============================================================================ + + +-- Runs a single healthcheck probe +function checker:run_single_check(ip, port, hostname, hostheader) + + local sock, err = ngx.socket.tcp() + if not sock then + self:log(ERR, "failed to create stream socket: ", err) + return + end + + sock:settimeout(self.checks.active.timeout * 1000) + + local ok + ok, err = sock:connect(ip, port) + if not ok then + if err == "timeout" then + sock:close() -- timeout errors do not close the socket. + return self:report_timeout(ip, port, hostname, "active") + end + return self:report_tcp_failure(ip, port, hostname, "connect", "active") + end + + if self.checks.active.type == "tcp" then + sock:close() + return self:report_success(ip, port, hostname, "active") + end + + if self.checks.active.type == "https" then + local https_sni, session, err + https_sni = self.checks.active.https_sni or hostheader or hostname + if self.ssl_cert and self.ssl_key then + ok, err = sock:setclientcert(self.ssl_cert, self.ssl_key) + + if not ok then + self:log(ERR, "failed to set client certificate: ", err) + end + end + + session, err = sock:sslhandshake(nil, https_sni, + self.checks.active.https_verify_certificate) + + if not session then + sock:close() + self:log(ERR, "failed SSL handshake with '", hostname or "", " (", ip, ":", port, ")', using server name (sni) '", https_sni, "': ", err) + return self:report_tcp_failure(ip, port, hostname, "connect", "active") + end + + end + + local req_headers = self.checks.active.req_headers + local headers + if self.checks.active._headers_str then + headers = self.checks.active._headers_str + else + local headers_length = nkeys(req_headers) + if headers_length > 0 then + if is_array(req_headers) then + self:log(WARN, "array headers is deprecated") + headers = table.concat(req_headers, "\r\n") + else + headers = new_tab(0, headers_length) + local idx = 0 + for key, values in pairs(req_headers) do + if type(values) == "table" then + for _, value in ipairs(values) do + idx = idx + 1 + headers[idx] = key .. ": " .. tostring(value) + end + else + idx = idx + 1 + headers[idx] = key .. ": " .. tostring(values) + end + end + headers = table.concat(headers, "\r\n") + end + if #headers > 0 then + headers = headers .. "\r\n" + end + end + self.checks.active._headers_str = headers or "" + end + + local method = self.checks.active.http_method + local path = self.checks.active.http_path + local body = self.checks.active.http_req_body + local final_hostheader = hostheader or hostname or ip + local request + if body and #body > 0 then + request = ("%s %s HTTP/1.1\r\nConnection: close\r\n%sHost: %s\r\nContent-Length: %d\r\n\r\n%s") + :format(method, path, headers, final_hostheader, #body, body) + else + request = ("%s %s HTTP/1.1\r\nConnection: close\r\n%sHost: %s\r\n\r\n") + :format(method, path, headers, final_hostheader) + end + self:log(DEBUG, "request: ", request) + + local bytes + bytes, err = sock:send(request) + if not bytes then + self:log(ERR, "failed to send http request to '", hostname, " (", ip, ":", port, ")': ", err) + if err == "timeout" then + sock:close() -- timeout errors do not close the socket. + return self:report_timeout(ip, port, hostname, "active") + end + return self:report_tcp_failure(ip, port, hostname, "send", "active") + end + + local status_line + status_line, err = sock:receive() + if not status_line then + self:log(ERR, "failed to receive status line from '", hostname, " (",ip, ":", port, ")': ", err) + if err == "timeout" then + sock:close() -- timeout errors do not close the socket. + return self:report_timeout(ip, port, hostname, "active") + end + return self:report_tcp_failure(ip, port, hostname, "receive", "active") + end + + local from, to = re_find(status_line, + [[^HTTP/\d+\.\d+\s+(\d+)]], + "joi", nil, 1) + local status + if from then + status = tonumber(status_line:sub(from, to)) + else + self:log(ERR, "bad status line from '", hostname, " (", ip, ":", port, ")': ", status_line) + -- note: 'status' will be reported as 'nil' + end + + sock:close() + + self:log(DEBUG, "Reporting '", hostname, " (", ip, ":", port, ")' (got HTTP ", status, ")") + + return self:report_http_status(ip, port, hostname, status, "active") +end + +-- executes a work package (a list of checks) sequentially +function checker:run_work_package(work_package) + for _, work_item in ipairs(work_package) do + self:log(DEBUG, "Checking ", work_item.hostname, " ", + work_item.hostheader and "(host header: ".. work_item.hostheader .. ")" + or "", work_item.ip, ":", work_item.port, + " (currently ", work_item.debug_health, ")") + local hostheader = work_item.hostheader or work_item.hostname + self:run_single_check(work_item.ip, work_item.port, work_item.hostname, hostheader) + end +end + +-- runs the active healthchecks concurrently, in multiple work packages. +-- @param list the list of targets to check +function checker:active_check_targets(list) + local idx = 1 + local work_packages = {} + + for _, work_item in ipairs(list) do + local package = work_packages[idx] + if not package then + package = {} + work_packages[idx] = package + end + package[#package + 1] = work_item + idx = idx + 1 + if idx > self.checks.active.concurrency then idx = 1 end + end + + -- hand out work-packages to the threads, note the "-1" because this timer + -- thread will handle the last package itself. + local threads = {} + for i = 1, #work_packages - 1 do + threads[i] = ngx.thread.spawn(self.run_work_package, self, work_packages[i]) + end + -- run last package myself + self:run_work_package(work_packages[#work_packages]) + + -- wait for everybody to finish + for _, thread in ipairs(threads) do + ngx.thread.wait(thread) + end +end + +--============================================================================ +-- Internal callbacks, timers and events +--============================================================================ +-- The timer callbacks are responsible for checking the status, upon success/ +-- failure they will call the health-management functions to deal with the +-- results of the checks. + + +-- @return `true` on success, or false if the lock was not acquired, or `nil + error` +-- in case of errors +local function get_periodic_lock(shm, key) + local my_pid = ngx_worker_pid() + local checker_pid = shm:get(key) + + if checker_pid == nil then + -- no worker is checking, try to acquire the lock + local ok, err = shm:add(key, my_pid, LOCK_PERIOD) + if not ok then + if err == "exists" then + -- another worker got the lock before + return false + end + ngx_log(ERR, "failed to add key '", key, "': ", err) + return nil, err + end + elseif checker_pid ~= my_pid then + -- another worker is checking + return false + end + + return true +end + + +-- touch the shm to refresh the valid period +local function renew_periodic_lock(shm, key) + local my_pid = ngx_worker_pid() + + local _, err = shm:set(key, my_pid, LOCK_PERIOD) + if err then + ngx_log(ERR, "failed to update key '", key, "': ", err) + end +end + + +--- Active health check callback function. +-- @param self the checker object this timer runs on +-- @param health_mode either "healthy" or "unhealthy" to indicate what check +local function checker_callback(self, health_mode) + if self.checker_callback_count then + self.checker_callback_count = self.checker_callback_count + 1 + end + + local list_to_check = {} + local targets, err = fetch_target_list(self) + if not targets then + self:log(ERR, "checker_callback: ", err) + return + end + + for _, target in ipairs(targets) do + local tgt = get_target(self, target.ip, target.port, target.hostname) + local internal_health = tgt and tgt.internal_health or nil + if (health_mode == "healthy" and (internal_health == "healthy" or + internal_health == "mostly_healthy")) + or (health_mode == "unhealthy" and (internal_health == "unhealthy" or + internal_health == "mostly_unhealthy")) + then + list_to_check[#list_to_check + 1] = { + ip = target.ip, + port = target.port, + hostname = target.hostname, + hostheader = target.hostheader, + meta = target.meta, + debug_health = internal_health, + } + end + end + + if not list_to_check[1] then + self:log(DEBUG, "checking ", health_mode, " targets: nothing to do") + else + local timer = resty_timer({ + interval = 0, + recurring = false, + immediate = false, + detached = true, + expire = function() + self:log(DEBUG, "checking ", health_mode, " targets: #", #list_to_check) + self:active_check_targets(list_to_check) + end, + }) + if timer == nil then + self:log(ERR, "failed to create timer to check ", health_mode) + end + end +end + +-- Event handler callback +function checker:event_handler(event_name, ip, port, hostname) + + local target_found = get_target(self, ip, port, hostname) + + if event_name == self.events.remove then + if target_found then + -- remove hash part + self.targets[target_found.ip][target_found.port][target_found.hostname] = nil + if not next(self.targets[target_found.ip][target_found.port]) then + -- no more hostnames on this port, so delete it + self.targets[target_found.ip][target_found.port] = nil + end + if not next(self.targets[target_found.ip]) then + -- no more ports on this ip, so delete it + self.targets[target_found.ip] = nil + end + -- remove from list part + for i, target in ipairs(self.targets) do + if target.ip == ip and target.port == port and + target.hostname == hostname then + table_remove(self.targets, i) + break + end + end + self:log(DEBUG, "event: target '", hostname or "", " (", ip, ":", port, + ")' removed") + + else + self:log(WARN, "event: trying to remove an unknown target '", + hostname or "", "(", ip, ":", port, ")'") + end + + elseif event_name == self.events.healthy or + event_name == self.events.mostly_healthy or + event_name == self.events.unhealthy or + event_name == self.events.mostly_unhealthy + then + if not target_found then + -- it is a new target, must add it first + target_found = { ip = ip, port = port, hostname = hostname or ip } + self.targets[target_found.ip] = self.targets[target_found.ip] or {} + self.targets[target_found.ip][target_found.port] = self.targets[target_found.ip][target_found.port] or {} + self.targets[target_found.ip][target_found.port][target_found.hostname] = target_found + self.targets[#self.targets + 1] = target_found + self:log(DEBUG, "event: target added '", hostname or "", "(", ip, ":", port, ")'") + end + do + local from_status = target_found.internal_health + local to_status = event_name + local from = from_status == "healthy" or from_status == "mostly_healthy" + local to = to_status == "healthy" or to_status == "mostly_healthy" + + if from ~= to then + self.status_ver = self.status_ver + 1 + end + + self:log(DEBUG, "event: target status '", hostname or "", "(", ip, ":", + port, ")' from '", from, "' to '", to, "', ver: ", self.status_ver) + end + target_found.internal_health = event_name + + elseif event_name == self.events.clear then + -- clear local cache + self.targets = {} + self:log(DEBUG, "event: local cache cleared") + + else + self:log(WARN, "event: unknown event received '", event_name, "'") + end +end + + +-- Re-derive a checker's local internal_health for every target the shm target +-- list says exists, correcting any target whose cached value has drifted from +-- a worker_events broadcast this worker never received (apache/apisix#13888), +-- AND backfilling any target this worker's self.targets never even contains +-- an entry for at all. +-- +-- The second case is not hypothetical: add_target()'s "already exists in shm" +-- branch (see its own comment) returns early without ever calling raise_event, +-- so a worker whose *own* add_target call loses that race gets no event either +-- -- and if this worker's initial checker.new() read of the target list also +-- raced ahead of the writer (observed directly: "Got initial target list (0 +-- targets)" immediately followed by "adding an existing target ... (ignoring)" +-- for every target), self.targets ends up with no entry for that target at +-- all: not stale, structurally absent. Since checker_callback() only ever +-- looks a target up via get_target(self, ...) against this same self.targets, +-- such a target is permanently invisible to this worker's active-check cycle +-- -- if this worker also holds the periodic probe lock (which never +-- voluntarily rotates), NO worker ever probes that target again for the life +-- of the process. Sourcing this sweep from fetch_target_list() (shm, the +-- authoritative source used across the file, e.g. add_target/checker_callback +-- itself) instead of iterating self.targets directly is what lets a missing +-- entry be detected in the first place. +local function reconcile_target_health(checker_obj) + local targets, err = fetch_target_list(checker_obj) + if not targets then + checker_obj:log(ERR, "reconcile: failed to fetch target list from shm: ", err) + return + end + + for _, target in ipairs(targets) do + local state_key = key_for(checker_obj.TARGET_STATE, target.ip, target.port, + target.hostname) + local raw_state = checker_obj.shm:get(state_key) + -- add_target() always writes TARGET_STATE before a target is considered + -- live (see its comment), so nil here means this read raced a concurrent + -- add/remove, not a genuine absence of state -- skip it for this sweep, + -- it will be consistent again on the next one. + if raw_state ~= nil then + local shm_health = INTERNAL_STATES[raw_state] + if shm_health then + local target_found = get_target(checker_obj, target.ip, target.port, target.hostname) + if not target_found then + -- lazily insert, mirroring event_handler's own "it is a new target, + -- must add it first" branch -- keeps both the ip/port/hostname + -- lookup table and the array part (used elsewhere, e.g. remove) + -- consistent with how every other insertion path populates them. + target_found = { ip = target.ip, port = target.port, + hostname = target.hostname or target.ip } + checker_obj.targets[target_found.ip] = checker_obj.targets[target_found.ip] or {} + checker_obj.targets[target_found.ip][target_found.port] = + checker_obj.targets[target_found.ip][target_found.port] or {} + checker_obj.targets[target_found.ip][target_found.port][target_found.hostname] = + target_found + checker_obj.targets[#checker_obj.targets + 1] = target_found + checker_obj:log(WARN, "reconciled missing target from shm (never seen locally) '", + target_found.hostname or "", "(", target_found.ip, ":", + target_found.port, ")' as '", shm_health, "'") + elseif shm_health ~= target_found.internal_health then + local from = target_found.internal_health == "healthy" or + target_found.internal_health == "mostly_healthy" + local to = shm_health == "healthy" or shm_health == "mostly_healthy" + if from ~= to then + checker_obj.status_ver = checker_obj.status_ver + 1 + end + checker_obj:log(WARN, "reconciled target status from shm (missed event) '", + target_found.hostname or "", "(", target_found.ip, ":", + target_found.port, ")' from '", target_found.internal_health, + "' to '", shm_health, "'") + end + target_found.internal_health = shm_health + end + end + end +end + + +------------------------------------------------------------------------------ +-- Initializing. +-- @section initializing +------------------------------------------------------------------------------ + +-- Log a message specific to this checker +-- @param level standard ngx log level constant +function checker:log(level, ...) + ngx_log(level, worker_color(self.LOG_PREFIX), ...) +end + + +-- Raises an event for a target status change. +function checker:raise_event(event_name, ip, port, hostname) + local target = { ip = ip, port = port, hostname = hostname } + worker_events.post(self.EVENT_SOURCE, event_name, target) +end + + +--- Stop the background health checks. +-- The timers will be flagged to exit, but will not exit immediately. Only +-- after the current timers have expired they will be marked as stopped. +-- @return `true` +function checker:stop() + self.checks.active.healthy.active = false + self.checks.active.unhealthy.active = false + worker_events.unregister(self.ev_callback, self.EVENT_SOURCE) + self:log(DEBUG, "healthchecker stopped") + return true +end + + +--- Start the background health checks. +-- @return `true`, or `nil + error`. +function checker:start() + if self.checks.active.healthy.interval > 0 then + self.checks.active.healthy.active = true + -- the first active check happens only after `interval` + self.checks.active.healthy.last_run = ngx_now() + end + + if self.checks.active.unhealthy.interval > 0 then + self.checks.active.unhealthy.active = true + self.checks.active.unhealthy.last_run = ngx_now() + end + + worker_events.unregister(self.ev_callback, self.EVENT_SOURCE) -- ensure we never double subscribe + worker_events.register_weak(self.ev_callback, self.EVENT_SOURCE) + + self:log(DEBUG, "active check flagged as active") + return true +end + + +--============================================================================ +-- Create health-checkers +--============================================================================ + + +local NO_DEFAULT = {} +local MAXNUM = 2^31 - 1 + + +local function fail(ctx, k, msg) + ctx[#ctx + 1] = k + error(table.concat(ctx, ".") .. ": " .. msg, #ctx + 1) +end + + +local function fill_in_settings(opts, defaults, ctx) + ctx = ctx or {} + local obj = {} + for k, default in pairs(defaults) do + local v = opts[k] + + -- basic type-check of configuration + if default ~= NO_DEFAULT + and v ~= nil + and type(v) ~= type(default) then + fail(ctx, k, "invalid value") + end + + if v ~= nil then + if type(v) == "table" then + if default[1] then -- do not recurse on arrays + obj[k] = v + else + ctx[#ctx + 1] = k + obj[k] = fill_in_settings(v, default, ctx) + ctx[#ctx + 1] = nil + end + else + if type(v) == "number" and (v < 0 or v > MAXNUM) then + fail(ctx, k, "must be between 0 and " .. MAXNUM) + end + obj[k] = v + end + elseif default ~= NO_DEFAULT then + obj[k] = deepcopy(default) + end + + end + return obj +end + + +local defaults = { + name = NO_DEFAULT, + shm_name = NO_DEFAULT, + type = NO_DEFAULT, + status_ver = 0, + events_module = "resty.worker.events", + checks = { + active = { + type = "http", + timeout = 1, + concurrency = 10, + http_method = "GET", + http_path = "/", + http_req_body = "", + https_sni = NO_DEFAULT, + https_verify_certificate = true, + headers = {""}, + healthy = { + interval = 0, -- 0 = disabled by default + http_statuses = { 200, 302 }, + successes = 2, + }, + unhealthy = { + interval = 0, -- 0 = disabled by default + http_statuses = { 429, 404, + 500, 501, 502, 503, 504, 505 }, + tcp_failures = 2, + timeouts = 3, + http_failures = 5, + }, + req_headers = {""}, + }, + passive = { + type = "http", + healthy = { + http_statuses = { 200, 201, 202, 203, 204, 205, 206, 207, 208, 226, + 300, 301, 302, 303, 304, 305, 306, 307, 308 }, + successes = 5, + }, + unhealthy = { + http_statuses = { 429, 500, 503 }, + tcp_failures = 2, + timeouts = 7, + http_failures = 5, + }, + }, + }, +} + + +local function to_set(tbl, key) + local set = {} + for _, item in ipairs(tbl[key]) do + set[item] = true + end + tbl[key] = set +end + + +local check_valid_type +do + local valid_types = { + http = true, + tcp = true, + https = true, + } + check_valid_type = function(var, val) + assert(valid_types[val], + var .. " can only be 'http', 'https' or 'tcp', got '" .. + tostring(val) .. "'") + end +end + +--- Creates a new health-checker instance. +-- It will be started upon creation. +-- +-- *NOTE*: the returned `checker` object must be anchored, if not it will be +-- removed by Lua's garbage collector and the healthchecks will cease to run. +-- @param opts table with checker options. Options are: +-- +-- * `name`: name of the health checker +-- * `shm_name`: the name of the `lua_shared_dict` specified in the Nginx configuration to use +-- * `ssl_cert`: certificate for mTLS connections (string or parsed object) +-- * `ssl_key`: key for mTLS connections (string or parsed object) +-- * `checks.active.type`: "http", "https" or "tcp" (default is "http") +-- * `checks.active.timeout`: socket timeout for active checks (in seconds) +-- * `checks.active.concurrency`: number of targets to check concurrently +-- * `checks.active.http_method`: method to use in the HTTP request to run on active checks (default is `GET`) +-- * `checks.active.http_path`: path to use in the HTTP request to run on active checks +-- * `checks.active.http_req_body`: body to send in the HTTP request to run on active checks (a non-empty body adds a `Content-Length` header) +-- * `checks.active.https_sni`: SNI server name incase of HTTPS +-- * `checks.active.https_verify_certificate`: boolean indicating whether to verify the HTTPS certificate +-- * `checks.active.headers`: one or more lists of values indexed by header name +-- * `checks.active.healthy.interval`: interval between checks for healthy targets (in seconds) +-- * `checks.active.healthy.http_statuses`: which HTTP statuses to consider a success +-- * `checks.active.healthy.successes`: number of successes to consider a target healthy +-- * `checks.active.unhealthy.interval`: interval between checks for unhealthy targets (in seconds) +-- * `checks.active.unhealthy.http_statuses`: which HTTP statuses to consider a failure +-- * `checks.active.unhealthy.tcp_failures`: number of TCP failures to consider a target unhealthy +-- * `checks.active.unhealthy.timeouts`: number of timeouts to consider a target unhealthy +-- * `checks.active.unhealthy.http_failures`: number of HTTP failures to consider a target unhealthy +-- * `checks.passive.type`: "http", "https" or "tcp" (default is "http"; for passive checks, "http" and "https" are equivalent) +-- * `checks.passive.healthy.http_statuses`: which HTTP statuses to consider a failure +-- * `checks.passive.healthy.successes`: number of successes to consider a target healthy +-- * `checks.passive.unhealthy.http_statuses`: which HTTP statuses to consider a success +-- * `checks.passive.unhealthy.tcp_failures`: number of TCP failures to consider a target unhealthy +-- * `checks.passive.unhealthy.timeouts`: number of timeouts to consider a target unhealthy +-- * `checks.passive.unhealthy.http_failures`: number of HTTP failures to consider a target unhealthy +-- +-- If any of the health counters above (e.g. `checks.passive.unhealthy.timeouts`) +-- is set to zero, the according category of checks is not taken to account. +-- This way active or passive health checks can be disabled selectively. +-- +-- @return checker object, or `nil + error` +function _M.new(opts) + + opts = opts or {} + local active_type = (((opts or EMPTY).checks or EMPTY).active or EMPTY).type + local passive_type = (((opts or EMPTY).checks or EMPTY).passive or EMPTY).type + + local self = fill_in_settings(opts, defaults) + + load_events_module(self) + + -- If using deprecated self.type, that takes precedence over + -- a default value. TODO: remove this in a future version + if self.type then + self.checks.active.type = active_type or self.type + self.checks.passive.type = passive_type or self.type + check_valid_type("type", self.type) + end + + assert(self.checks.active.healthy.successes < 255, "checks.active.healthy.successes must be at most 254") + assert(self.checks.active.unhealthy.tcp_failures < 255, "checks.active.unhealthy.tcp_failures must be at most 254") + assert(self.checks.active.unhealthy.http_failures < 255, "checks.active.unhealthy.http_failures must be at most 254") + assert(self.checks.active.unhealthy.timeouts < 255, "checks.active.unhealthy.timeouts must be at most 254") + assert(self.checks.passive.healthy.successes < 255, "checks.passive.healthy.successes must be at most 254") + assert(self.checks.passive.unhealthy.tcp_failures < 255, "checks.passive.unhealthy.tcp_failures must be at most 254") + assert(self.checks.passive.unhealthy.http_failures < 255, "checks.passive.unhealthy.http_failures must be at most 254") + assert(self.checks.passive.unhealthy.timeouts < 255, "checks.passive.unhealthy.timeouts must be at most 254") + + if opts.test then + self.test_get_counter = test_get_counter + self.checker_callback_count = 0 + end + + assert(self.name, "required option 'name' is missing") + assert(self.shm_name, "required option 'shm_name' is missing") + + check_valid_type("checks.active.type", self.checks.active.type) + check_valid_type("checks.passive.type", self.checks.passive.type) + + self.shm = ngx.shared[tostring(opts.shm_name)] + assert(self.shm, ("no shm found by name '%s'"):format(opts.shm_name)) + + -- load certificate and key + if opts.ssl_cert and opts.ssl_key then + if type(opts.ssl_cert) == "cdata" then + self.ssl_cert = opts.ssl_cert + else + self.ssl_cert = assert(ssl.parse_pem_cert(opts.ssl_cert)) + end + + if type(opts.ssl_key) == "cdata" then + self.ssl_key = opts.ssl_key + else + self.ssl_key = assert(ssl.parse_pem_priv_key(opts.ssl_key)) + end + + end + + -- other properties + self.targets = nil -- list of targets, initially loaded, maintained by events + self.events = nil -- hash table with supported events (prevent magic strings) + self.ev_callback = nil -- callback closure per checker instance + + -- Convert status lists to sets + to_set(self.checks.active.unhealthy, "http_statuses") + to_set(self.checks.active.healthy, "http_statuses") + to_set(self.checks.passive.unhealthy, "http_statuses") + to_set(self.checks.passive.healthy, "http_statuses") + + -- decorate with methods and constants + self.events = EVENTS + for k,v in pairs(checker) do + self[k] = v + end + + -- prepare shm keys + self.TARGET_STATE = SHM_PREFIX .. self.name .. ":state" + self.TARGET_COUNTER = SHM_PREFIX .. self.name .. ":counter" + self.TARGET_LIST = SHM_PREFIX .. self.name .. ":target_list" + self.TARGET_LIST_LOCK = SHM_PREFIX .. self.name .. ":target_list_lock" + self.TARGET_LOCK = SHM_PREFIX .. self.name .. ":target_lock" + self.PERIODIC_LOCK = SHM_PREFIX .. ":period_lock:" + -- prepare constants + self.EVENT_SOURCE = EVENT_SOURCE_PREFIX .. " [" .. self.name .. "]" + self.LOG_PREFIX = LOG_PREFIX .. "(" .. self.name .. ") " + + -- register for events, and directly after load initial target list + -- order is important! + do + -- Lock the list, in case it is being cleared by another worker + local ok, err = locking_target_list(self, function(target_list) + + self.targets = target_list + self:log(DEBUG, "Got initial target list (", #self.targets, " targets)") + + -- load individual statuses + for _, target in ipairs(self.targets) do + local state_key = key_for(self.TARGET_STATE, target.ip, target.port, target.hostname) + target.internal_health = INTERNAL_STATES[self.shm:get(state_key)] + self:log(DEBUG, "Got initial status ", target.internal_health, " ", + target.hostname, " ", target.ip, ":", target.port) + -- fill-in the hash part for easy lookup + self.targets[target.ip] = self.targets[target.ip] or {} + self.targets[target.ip][target.port] = self.targets[target.ip][target.port] or {} + self.targets[target.ip][target.port][target.hostname or target.ip] = target + end + + return true + end) + if not ok then + -- locking failed, we don't protect `targets` of being nil in other places + -- so consider this as not recoverable + return nil, "Error loading initial target list: " .. err + end + + self.ev_callback = function(data, event) + -- just a wrapper to be able to access `self` as a closure + return self:event_handler(event, data.ip, data.port, data.hostname) + end + + -- handle events to sync up in case there was a change by another worker + worker_events:poll() + end + + -- turn on active health check + local ok, err = self:start() + if not ok then + self:stop() + return nil, err + end + + -- if active checker is not running, start it + if active_check_timer == nil then + + self:log(DEBUG, "worker ", ngx_worker_id(), " (pid: ", ngx_worker_pid(), ") ", + "starting active check timer") + local shm, key = self.shm, self.PERIODIC_LOCK + local cleanup_key = key .. ":cleanup" + active_check_timer, err = resty_timer({ + recurring = true, + interval = CHECK_INTERVAL, + jitter = CHECK_JITTER, + detached = false, + expire = function() + + local cur_time = ngx_now() + + -- Self-heal from a missed worker_events broadcast (apache/apisix#13888). + -- Unlike the probing/cleanup elections below, this runs on EVERY worker + -- EVERY tick it's due, regardless of who owns the periodic/cleanup + -- locks -- a worker with no active checker of its own can still be + -- routing traffic off a stale cached status for a checker it merely + -- reads (get_target_status), so it must reconcile too. + if cur_time - last_reconcile_time >= RECONCILE_INTERVAL then + last_reconcile_time = cur_time + for _, checker_obj in pairs(hcs) do + reconcile_target_health(checker_obj) + end + end + + -- Stale-target cleanup is decoupled from active probing and the + -- periodic lock. A passive-only deployment (checks.passive but no + -- active interval) has no active checker on any worker, yet still marks + -- targets via delayed_clear() and must purge them; gating cleanup on + -- the periodic lock (which only an active worker ever holds) would leak + -- those targets forever (apache/apisix#13385). + -- + -- A single worker per window is elected via an atomic shm:add on + -- cleanup_key (TTL = CLEANUP_INTERVAL): any worker can win, including a + -- passive-only one, so the leak fix holds, while the other workers skip + -- the pass and do not all contend on each checker's TARGET_LIST_LOCK. + -- The elected worker purges every checker in its own `hcs` in one pass. + -- Purging writes to the shared shm target_list, so cleaning a checker on + -- any one worker is globally effective; if checkers are distributed + -- asymmetrically across workers, a given upstream is purged in whichever + -- window a worker that owns its checker wins the election -- eventually + -- consistent rather than every-worker-every-window. + local won, add_err = shm:add(cleanup_key, ngx_worker_pid(), CLEANUP_INTERVAL) + if won then + for _, checker_obj in pairs(hcs) do + -- clear targets marked for delayed removal + locking_target_list(checker_obj, function(target_list) + local removed_targets = {} + local index = 1 + while index <= #target_list do + local target = target_list[index] + if target.purge_time and target.purge_time <= cur_time then + table_insert(removed_targets, target) + table_remove(target_list, index) + else + index = index + 1 + end + end + + if #removed_targets > 0 then + target_list = serialize(target_list) + + local ok, err = shm:set(checker_obj.TARGET_LIST, target_list) + if not ok then + return nil, "failed to store target_list in shm: " .. err + end + + for _, target in ipairs(removed_targets) do + clear_target_data_from_shm(checker_obj, target.ip, target.port, target.hostname) + checker_obj:raise_event(checker_obj.events.remove, target.ip, target.port, target.hostname) + end + end + end) + end + elseif add_err ~= "exists" then + ngx_log(ERR, "failed to elect cleanup worker for '", cleanup_key, "': ", add_err) + end + + -- The periodic lock distributes ACTIVE probing to a single worker. A + -- worker with no active checker must release the lock (if it holds it) + -- and back off, so a worker that still owns active checkers can take + -- over; otherwise active health checks stay stuck after a disable -> + -- re-enable cycle (apache/apisix#13235). This is gated on + -- has_active_checker rather than on `hcs` being non-empty, because a + -- disabled checker lingers in the weak `hcs` table with active=false + -- until it is garbage collected. + local has_active_checker = false + for _, checker_obj in pairs(hcs) do + if checker_obj.checks.active.healthy.active or + checker_obj.checks.active.unhealthy.active then + has_active_checker = true + break + end + end + + if not has_active_checker then + -- release the lock only if we are still the holder, so a worker that + -- still owns active checkers can take over + if shm:get(key) == ngx_worker_pid() then + shm:delete(key) + end + return + end + + if get_periodic_lock(shm, key) then + renew_periodic_lock(shm, key) + else + return + end + + -- active probing: only the periodic-lock holder runs the probes + for _, checker_obj in pairs(hcs) do + if checker_obj.checks.active.healthy.active and + (checker_obj.checks.active.healthy.last_run + + checker_obj.checks.active.healthy.interval <= cur_time) + then + checker_obj.checks.active.healthy.last_run = cur_time + checker_callback(checker_obj, "healthy") + end + + if checker_obj.checks.active.unhealthy.active and + (checker_obj.checks.active.unhealthy.last_run + + checker_obj.checks.active.unhealthy.interval <= cur_time) + then + checker_obj.checks.active.unhealthy.last_run = cur_time + checker_callback(checker_obj, "unhealthy") + end + end + end, + }) + if not active_check_timer then + self:log(ERR, "Could not start active check timer: ", err) + end + end + + table.insert(hcs, self) + + -- TODO: push entire config in debug level logs + self:log(DEBUG, "Healthchecker started!") + return self +end + + +function _M.get_target_list(name, shm_name) + local self = { + name = name, + shm_name = shm_name, + log = checker.log, + } + self.shm = ngx.shared[tostring(shm_name)] + assert(self.shm, ("no shm found by name '%s'"):format(shm_name)) + self.TARGET_STATE = SHM_PREFIX .. self.name .. ":state" + self.TARGET_COUNTER = SHM_PREFIX .. self.name .. ":counter" + self.TARGET_LIST = SHM_PREFIX .. self.name .. ":target_list" + self.TARGET_LIST_LOCK = SHM_PREFIX .. self.name .. ":target_list_lock" + self.LOG_PREFIX = LOG_PREFIX .. "(" .. self.name .. ") " + + local ok, err = locking_target_list(self, function(target_list) + self.targets = target_list + for _, target in ipairs(self.targets) do + local state_key = key_for(self.TARGET_STATE, target.ip, target.port, target.hostname) + target.status = INTERNAL_STATES[self.shm:get(state_key)] + if not target.hostheader then + target.hostheader = nil + end + end + + return true + end) + + for _, target in ipairs(self.targets) do + local key = key_for(self.TARGET_LOCK, target.ip, target.port, target.hostname) + local ok = run_locked(self, key, function() + local counter = self.shm:get(key_for(self.TARGET_COUNTER, + target.ip, target.port, target.hostname)) + target.counter = { + success = ctr_get(counter, CTR_SUCCESS), + http_failure = ctr_get(counter, CTR_HTTP), + tcp_failure = ctr_get(counter, CTR_TCP), + timeout_failure = ctr_get(counter, CTR_TIMEOUT), + } + return true + end) + + if not ok then + target.counter = { + success = 0, + http_failure = 0, + tcp_failure = 0, + timeout_failure = 0, + } + end + end + + if not ok then + return nil, "Error loading target list: " .. err + end + + return self.targets +end + + +if TESTING then + -- test-only hook: shorten the stale-target cleanup window so the periodic + -- cleanup path can be exercised deterministically without waiting for the + -- default CLEANUP_INTERVAL (CHECK_INTERVAL * 25). + function _M._set_cleanup_interval(interval) + CLEANUP_INTERVAL = interval + end + + -- test-only hook: shorten the shm-vs-local-cache reconciliation cadence so + -- the self-heal path (apache/apisix#13888) can be exercised deterministically + -- without waiting for the default RECONCILE_INTERVAL. + function _M._set_reconcile_interval(interval) + RECONCILE_INTERVAL = interval + last_reconcile_time = 0 + end +end + + +return _M diff --git a/hack/patches/lua-resty-healthcheck-13888.patch b/hack/patches/lua-resty-healthcheck-13888.patch new file mode 100644 index 000000000000..b09d7e98b648 --- /dev/null +++ b/hack/patches/lua-resty-healthcheck-13888.patch @@ -0,0 +1,179 @@ +# Local, pre-upstream fix for apache/apisix#13888 (health check state diverges +# across nginx workers -- a worker that misses the worker_events broadcast for +# a target's health transition never re-converges, since the source worker only +# raises that event once per state change). +# +# The reconciliation sweep this patch adds must source the authoritative +# target list from shm (fetch_target_list()), not iterate the worker's local +# self.targets directly: a worker can lose a target from self.targets +# entirely (not just have it go stale) when its own add_target() call loses +# a startup race against another worker and hits the "already exists in shm" +# early-return path, which never raises the add-event that would otherwise +# seed it locally. Confirmed directly against a local Docker repro: the +# periodic-probe-lock-holding worker logged "Got initial target list (0 +# targets)" at checker construction, immediately followed by "adding an +# existing target ... (ignoring)" for every configured node -- leaving that +# worker's self.targets with no entry at all for any of them, so its active +# checker_callback() logged "checking healthy/unhealthy targets: nothing to +# do" every tick for the rest of the process's life and never issued a single +# probe. Since the periodic lock never voluntarily rotates, no worker ever +# probed those targets again. +# +# Targets the `lua-resty-healthcheck-api7` rock pinned in +# apisix-master-0.rockspec (currently 3.2.3). `deps/` is gitignored and +# regenerated by `make deps`, so this patch is not applied automatically -- +# re-apply it after every `make deps`: +# +# patch -p3 -d deps/share/lua/5.1/resty < hack/patches/lua-resty-healthcheck-13888.patch +# +# This is a local testing aid only. Once validated, the same diff is the basis +# for a PR against https://github.com/api7/lua-resty-healthcheck -- do not bump +# the rockspec pin until that lands upstream. +--- a/lib/resty/healthcheck.lua ++++ b/lib/resty/healthcheck.lua +@@ -142,6 +142,16 @@ + -- the check interval. If it does not update the shm during this period, we + -- consider that it is not able to continue checking (the worker probably was killed) + local LOCK_PERIOD = CHECK_INTERVAL * 15 ++ ++-- Only the periodic-lock holder ever runs active probes (see active_check_timer ++-- below), so every other worker's target.internal_health is updated *solely* by ++-- the worker_events broadcast raised in incr_counter. That broadcast has no ++-- delivery guarantee and, once a target is already at the reported health, is ++-- never raised again for the same state -- so a single missed event permanently ++-- strands a worker's local view (apache/apisix#13888). RECONCILE_INTERVAL bounds ++-- how long that divergence can last: every worker re-derives internal_health ++-- from the authoritative shm state on this cadence, independent of events. ++local RECONCILE_INTERVAL = 1 + -- interval between stale targets cleanup + local CLEANUP_INTERVAL = CHECK_INTERVAL * 25 + +@@ -237,6 +247,10 @@ + + local active_check_timer + ++-- last time (ngx.now()) the shm-vs-local-cache reconciliation sweep ran; shared ++-- by all checkers on this worker since the sweep itself iterates `hcs` ++local last_reconcile_time = 0 ++ + -- serialize a table to a string + local serialize = codec.encode + +@@ -1383,6 +1397,82 @@ + end + + ++-- Re-derive a checker's local internal_health for every target the shm target ++-- list says exists, correcting any target whose cached value has drifted from ++-- a worker_events broadcast this worker never received (apache/apisix#13888), ++-- AND backfilling any target this worker's self.targets never even contains ++-- an entry for at all. ++-- ++-- The second case is not hypothetical: add_target()'s "already exists in shm" ++-- branch (see its own comment) returns early without ever calling raise_event, ++-- so a worker whose *own* add_target call loses that race gets no event either ++-- -- and if this worker's initial checker.new() read of the target list also ++-- raced ahead of the writer (observed directly: "Got initial target list (0 ++-- targets)" immediately followed by "adding an existing target ... (ignoring)" ++-- for every target), self.targets ends up with no entry for that target at ++-- all: not stale, structurally absent. Since checker_callback() only ever ++-- looks a target up via get_target(self, ...) against this same self.targets, ++-- such a target is permanently invisible to this worker's active-check cycle ++-- -- if this worker also holds the periodic probe lock (which never ++-- voluntarily rotates), NO worker ever probes that target again for the life ++-- of the process. Sourcing this sweep from fetch_target_list() (shm, the ++-- authoritative source used across the file, e.g. add_target/checker_callback ++-- itself) instead of iterating self.targets directly is what lets a missing ++-- entry be detected in the first place. ++local function reconcile_target_health(checker_obj) ++ local targets, err = fetch_target_list(checker_obj) ++ if not targets then ++ checker_obj:log(ERR, "reconcile: failed to fetch target list from shm: ", err) ++ return ++ end ++ ++ for _, target in ipairs(targets) do ++ local state_key = key_for(checker_obj.TARGET_STATE, target.ip, target.port, ++ target.hostname) ++ local raw_state = checker_obj.shm:get(state_key) ++ -- add_target() always writes TARGET_STATE before a target is considered ++ -- live (see its comment), so nil here means this read raced a concurrent ++ -- add/remove, not a genuine absence of state -- skip it for this sweep, ++ -- it will be consistent again on the next one. ++ if raw_state ~= nil then ++ local shm_health = INTERNAL_STATES[raw_state] ++ if shm_health then ++ local target_found = get_target(checker_obj, target.ip, target.port, target.hostname) ++ if not target_found then ++ -- lazily insert, mirroring event_handler's own "it is a new target, ++ -- must add it first" branch -- keeps both the ip/port/hostname ++ -- lookup table and the array part (used elsewhere, e.g. remove) ++ -- consistent with how every other insertion path populates them. ++ target_found = { ip = target.ip, port = target.port, ++ hostname = target.hostname or target.ip } ++ checker_obj.targets[target_found.ip] = checker_obj.targets[target_found.ip] or {} ++ checker_obj.targets[target_found.ip][target_found.port] = ++ checker_obj.targets[target_found.ip][target_found.port] or {} ++ checker_obj.targets[target_found.ip][target_found.port][target_found.hostname] = ++ target_found ++ checker_obj.targets[#checker_obj.targets + 1] = target_found ++ checker_obj:log(WARN, "reconciled missing target from shm (never seen locally) '", ++ target_found.hostname or "", "(", target_found.ip, ":", ++ target_found.port, ")' as '", shm_health, "'") ++ elseif shm_health ~= target_found.internal_health then ++ local from = target_found.internal_health == "healthy" or ++ target_found.internal_health == "mostly_healthy" ++ local to = shm_health == "healthy" or shm_health == "mostly_healthy" ++ if from ~= to then ++ checker_obj.status_ver = checker_obj.status_ver + 1 ++ end ++ checker_obj:log(WARN, "reconciled target status from shm (missed event) '", ++ target_found.hostname or "", "(", target_found.ip, ":", ++ target_found.port, ")' from '", target_found.internal_health, ++ "' to '", shm_health, "'") ++ end ++ target_found.internal_health = shm_health ++ end ++ end ++ end ++end ++ ++ + ------------------------------------------------------------------------------ + -- Initializing. + -- @section initializing +@@ -1749,6 +1839,19 @@ + + local cur_time = ngx_now() + ++ -- Self-heal from a missed worker_events broadcast (apache/apisix#13888). ++ -- Unlike the probing/cleanup elections below, this runs on EVERY worker ++ -- EVERY tick it's due, regardless of who owns the periodic/cleanup ++ -- locks -- a worker with no active checker of its own can still be ++ -- routing traffic off a stale cached status for a checker it merely ++ -- reads (get_target_status), so it must reconcile too. ++ if cur_time - last_reconcile_time >= RECONCILE_INTERVAL then ++ last_reconcile_time = cur_time ++ for _, checker_obj in pairs(hcs) do ++ reconcile_target_health(checker_obj) ++ end ++ end ++ + -- Stale-target cleanup is decoupled from active probing and the + -- periodic lock. A passive-only deployment (checks.passive but no + -- active interval) has no active checker on any worker, yet still marks +@@ -1933,6 +2036,14 @@ + function _M._set_cleanup_interval(interval) + CLEANUP_INTERVAL = interval + end ++ ++ -- test-only hook: shorten the shm-vs-local-cache reconciliation cadence so ++ -- the self-heal path (apache/apisix#13888) can be exercised deterministically ++ -- without waiting for the default RECONCILE_INTERVAL. ++ function _M._set_reconcile_interval(interval) ++ RECONCILE_INTERVAL = interval ++ last_reconcile_time = 0 ++ end + end + + diff --git a/t/node/healthcheck-missed-event-reconcile-stockcheck.t b/t/node/healthcheck-missed-event-reconcile-stockcheck.t new file mode 100644 index 000000000000..351bb7f71038 --- /dev/null +++ b/t/node/healthcheck-missed-event-reconcile-stockcheck.t @@ -0,0 +1,103 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +# NOT part of the real test suite -- a throwaway "red" counterpart to +# healthcheck-missed-event-reconcile.t, written to run against STOCK (unpatched) +# resty.healthcheck deps to prove apache/apisix#13888 is real at the unit level. +# Unlike the real test, this never calls _set_reconcile_interval() (a patched-only +# API that doesn't exist on stock deps and would error the whole test file). +# See /Users/andre.nogueira/projects/apisix/hack/patches/lua-resty-healthcheck-13888.patch +use t::APISIX 'no_plan'; + +repeat_each(1); +log_level('info'); +no_root_location(); +no_shuffle(); +worker_connections(256); + +run_tests(); + +__DATA__ + +=== TEST 1: on stock code, a missed event leaves the local cache stale forever +# Same shm-poke as healthcheck-missed-event-reconcile.t (simulates a dropped +# worker_events broadcast), but never calls the patched-only reconcile hook. +# On stock resty.healthcheck, get_target_status() reads only the local cache +# and nothing re-derives it from shm on any cadence -- so the stale value must +# still be wrong even several seconds later, not just "immediately after". +--- config + location /t { + content_by_lua_block { + local healthcheck = require("resty.healthcheck") + + local checker = healthcheck.new({ + name = "test-13888-stockcheck", + shm_name = "upstream-healthcheck", + checks = { + active = { + healthy = { interval = 0 }, + unhealthy = { interval = 0 }, + }, + }, + events_module = "resty.events", + }) + if not checker then + ngx.say("failed to create checker") + return + end + + local ok, err = checker:add_target("127.0.0.1", 12346) + if not ok then + ngx.say("failed to add target: ", err) + return + end + ngx.sleep(0.2) -- let add_target's own event settle locally + + local before = checker:get_target_status("127.0.0.1", 12346) + ngx.say("before shm write: ", tostring(before)) + + -- Simulate a dropped worker_events broadcast: mutate the + -- authoritative shm state directly, without incr_counter/raise_event. + local shm = ngx.shared["upstream-healthcheck"] + local state_key = checker.TARGET_STATE .. ":127.0.0.1:12346:127.0.0.1" + local ok, err = shm:set(state_key, 2) -- INTERNAL_STATES[2] == "unhealthy" + if not ok then + ngx.say("failed to poke shm: ", err) + return + end + + local immediately_after = checker:get_target_status("127.0.0.1", 12346) + ngx.say("immediately after shm write: ", tostring(immediately_after)) + + -- generous wait -- several times longer than the patched default + -- RECONCILE_INTERVAL (1s) -- to prove this is not a timing fluke + ngx.sleep(3) + + local after_wait = checker:get_target_status("127.0.0.1", 12346) + ngx.say("after 3s wait, still stale on stock: ", tostring(after_wait)) + + checker:stop() + } + } +--- request +GET /t +--- response_body +before shm write: true +immediately after shm write: true +after 3s wait, still stale on stock: true +--- no_error_log +[error] +--- timeout: 8 diff --git a/t/node/healthcheck-missed-event-reconcile.t b/t/node/healthcheck-missed-event-reconcile.t new file mode 100644 index 000000000000..f697e0e7cd57 --- /dev/null +++ b/t/node/healthcheck-missed-event-reconcile.t @@ -0,0 +1,117 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one or more +# contributor license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright ownership. +# The ASF licenses this file to You under the Apache License, Version 2.0 +# (the "License"); you may not use this file except in compliance with +# the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +# +use t::APISIX 'no_plan'; + +repeat_each(1); +log_level('info'); +no_root_location(); +no_shuffle(); +worker_connections(256); + +# enable the resty.healthcheck TESTING seams (apache/apisix#13888 reconcile hook) +# before apisix (and therefore resty.healthcheck) is first required +add_block_preprocessor(sub { + my ($block) = @_; + my $extra_init_by_lua_start = $block->extra_init_by_lua_start // ''; + $extra_init_by_lua_start .= "\n_G.__TESTING_HEALTHCHECKER = true\n"; + $block->set_value("extra_init_by_lua_start", $extra_init_by_lua_start); +}); + +run_tests(); + +__DATA__ + +=== TEST 1: a target's local health cache self-heals from shm after a missed event +# Reproduces apache/apisix#13888: incr_counter() only raises a worker_events +# broadcast once per state transition. A worker whose local self.targets cache +# missed that one broadcast (e.g. a resty.events delivery drop) has no other +# way to learn the target went unhealthy -- get_target_status() reads only the +# local cache, never shm, so it stays wrong forever. +# +# This writes directly to the shared dict, bypassing incr_counter/raise_event +# entirely, to simulate exactly that: shm updated, no event delivered. Before +# the reconcile sweep runs, get_target_status() must still report the stale +# ("healthy") value. After one sweep interval, it must have converged to the +# shm value ("unhealthy") on its own, with no event ever posted for it. +--- config + location /t { + content_by_lua_block { + local healthcheck = require("resty.healthcheck") + healthcheck._set_reconcile_interval(0.3) + + local checker = healthcheck.new({ + name = "test-13888-reconcile", + shm_name = "upstream-healthcheck", + checks = { + active = { + healthy = { interval = 0 }, + unhealthy = { interval = 0 }, + }, + }, + events_module = "resty.events", + }) + if not checker then + ngx.say("failed to create checker") + return + end + + local ok, err = checker:add_target("127.0.0.1", 12345) + if not ok then + ngx.say("failed to add target: ", err) + return + end + ngx.sleep(0.2) -- let add_target's own event settle locally + + local before = checker:get_target_status("127.0.0.1", 12345) + ngx.say("before shm write: ", tostring(before)) + + -- Simulate a dropped worker_events broadcast: mutate the + -- authoritative shm state directly, without incr_counter/raise_event. + local shm = ngx.shared["upstream-healthcheck"] + local state_key = checker.TARGET_STATE .. ":127.0.0.1:12345:127.0.0.1" + local ok, err = shm:set(state_key, 2) -- INTERNAL_STATES[2] == "unhealthy" + if not ok then + ngx.say("failed to poke shm: ", err) + return + end + + -- immediately after the shm write: the local cache has NOT been + -- told anything, so it must still read the pre-existing value + local immediately_after = checker:get_target_status("127.0.0.1", 12345) + ngx.say("immediately after shm write: ", tostring(immediately_after)) + + ngx.sleep(0.6) -- past the 0.3s test reconcile interval + + local after_reconcile = checker:get_target_status("127.0.0.1", 12345) + ngx.say("after reconcile: ", tostring(after_reconcile)) + + checker:stop() + } + } +--- request +GET /t +--- response_body +before shm write: true +immediately after shm write: true +after reconcile: false +--- grep_error_log eval +qr/reconciled target status from shm/ +--- grep_error_log_out +reconciled target status from shm +--- no_error_log +[error] +--- timeout: 5 From ae12b440f8c5bbf831bd02eba76382ab834fa448 Mon Sep 17 00:00:00 2001 From: Andre Nogueira Date: Fri, 28 Aug 2026 00:47:35 +0100 Subject: [PATCH 3/4] fix(healthcheck): require probe-count convergence, not one attempt, for the readiness gate A single active-check attempt does not guarantee a target's real health state is known: with e.g. unhealthy.http_failures = 2 configured, internal_health only actually converges after two consecutive attempts. The boolean "probed" flag from 12fa7f29 flipped true after the first attempt regardless, so a readiness gate built on it could still open before a target's true state was reached -- confirmed live via a kind end-to-end trial, where real traffic kept leaking for 6 seconds after the pod was already marked Ready. Changes the shm value from a boolean to a per-target attempt counter (incremented in run_single_check, not just set), and all_targets_probed(name, shm_name, min_attempts) now compares against an explicit threshold instead of requiring only one attempt. apisix.healthcheck_manager.is_resource_probed computes that threshold from the checker's own config: max(unhealthy.http_failures, .tcp_failures, .timeouts, healthy.successes). Also fixes a second gap found in the same trial: ensure_checker seeded a checker using the resource's raw, potentially-unresolved node list. Domain-name upstream nodes are otherwise only resolved by apisix.upstream.get_by_id -> parse_domain_in_up, which runs exclusively on the live request path -- a checker built ahead of traffic would start probing under the unresolved domain-string identity, then get silently rebuilt (wiping its accumulated probe count) the moment real traffic first resolved the domain and bumped _nodes_ver. ensure_checker now resolves proactively via the same parse_domain_in_up path, using the has_domain flag already set on the shared config object by the /upstreams config watcher's filter callback. Adds TEST 5 to cover the threshold behavior directly: all_targets_probed must stay false after one attempt when min_attempts=2, and only flip true after the second. Validated end to end in a local kind cluster: pod-restart-while-unhealthy now shows zero leaked requests before or after the readiness transition, down from 64 leaked responses in the prior boolean-based version. Signed-off-by: Andre Nogueira --- apisix/healthcheck_manager.lua | 73 ++++++++++++++- .../healthcheck-probed-gate-patched-full.lua | 52 +++++++---- .../lua-resty-healthcheck-probed-gate.patch | 91 ++++++++++++------- .../healthcheck-fresh-node-default-healthy.t | 81 +++++++++++++++++ 4 files changed, 239 insertions(+), 58 deletions(-) diff --git a/apisix/healthcheck_manager.lua b/apisix/healthcheck_manager.lua index 8b8601af193e..ac77c200f056 100644 --- a/apisix/healthcheck_manager.lua +++ b/apisix/healthcheck_manager.lua @@ -304,6 +304,32 @@ function _M.ensure_checker(resource_path) core.log.warn("ensure_checker: resource has no checks configured: ", resource_path) return false, "no checks configured" end + + -- Domain-name nodes are otherwise only resolved by apisix.upstream's + -- get_by_id -> parse_domain_in_up, which runs exclusively on the live + -- request path (apisix/init.lua). Without this, a checker seeded here + -- would start probing under the unresolved domain-string identity, then + -- get rebuilt -- wiping its accumulated shm state, including the probe + -- count is_resource_probed relies on -- the moment real traffic first + -- resolves the domain and bumps _nodes_ver. res_conf is the same shared + -- config object get_by_id operates on (both come from + -- core.config.fetch_created_obj), so has_domain/dns_nodes are already + -- populated by the config watcher's filter callback regardless of + -- traffic; resolve here so the checker is built once, under its final + -- identity, from the start. + if res_conf.has_domain then + local resolved, err = upstream_utils.parse_domain_in_up(res_conf) + if not resolved then + core.log.error("ensure_checker: failed to resolve domain nodes for ", + resource_path, ": ", err) + -- fall through with the unresolved config -- a subsequent real + -- request or ensure_checker call will retry the resolution + else + res_conf = resolved + upstream = res_conf.value.upstream or res_conf.value + end + end + if not upstream.nodes or #upstream.nodes == 0 then return false, "no nodes" end @@ -318,21 +344,58 @@ function _M.ensure_checker(resource_path) end +-- A single active-check attempt does not guarantee a target's real state is +-- known: with e.g. unhealthy.http_failures = 2 configured, internal_health +-- only converges after two consecutive failed attempts. Returns the +-- worst-case number of consecutive attempts needed to guarantee the real +-- state has been reached in either direction (healthy or unhealthy), or nil +-- if the resource has no active check configured at all -- in which case +-- there is nothing for a readiness gate to wait on. +local function required_probe_attempts(checks) + local active = checks and checks.active + if not active then + return nil + end + local unhealthy = active.unhealthy or {} + local healthy = active.healthy or {} + local max_attempts = 1 + for _, threshold in ipairs({ + unhealthy.http_failures, + unhealthy.tcp_failures, + unhealthy.timeouts, + healthy.successes, + }) do + if threshold and threshold > max_attempts then + max_attempts = threshold + end + end + return max_attempts +end + + -- Cold-start readiness gate support (PS-12691): true only once every target --- of the resource's checker has had at least one real active-check attempt --- (see resty.healthcheck's all_targets_probed -- "probed" means attempted, --- not "healthy"). false if there is no live checker yet (ensure_checker not --- called, or timer_create_checker hasn't run its next tick yet). +-- of the resource's checker has had enough real active-check attempts for +-- its state to have actually converged (see resty.healthcheck's +-- all_targets_probed -- "probed" means attempted, not "healthy", and one +-- attempt is not always enough, see required_probe_attempts above). false +-- if there is no live checker yet (ensure_checker not called, or +-- timer_create_checker hasn't run its next tick yet). function _M.is_resource_probed(resource_path) local item = working_pool[resource_path] if not item or not item.checker or item.checker.dead then return false end + local min_attempts = required_probe_attempts(item.checks) + if min_attempts == nil then + -- no active check configured: nothing for this gate to wait on. + return true + end + if not healthcheck then healthcheck = require("resty.healthcheck") end - local ok, err = healthcheck.all_targets_probed(item.checker.name, healthcheck_shdict_name) + local ok, err = healthcheck.all_targets_probed(item.checker.name, healthcheck_shdict_name, min_attempts) if ok == nil then core.log.error("is_resource_probed: ", err) return false diff --git a/hack/patches/healthcheck-probed-gate-patched-full.lua b/hack/patches/healthcheck-probed-gate-patched-full.lua index 910365a1354c..34e80ad4f069 100644 --- a/hack/patches/healthcheck-probed-gate-patched-full.lua +++ b/hack/patches/healthcheck-probed-gate-patched-full.lua @@ -589,7 +589,7 @@ local function clear_target_data_from_shm(self, ip, port, hostname) end ok, err = self.shm:set(key_for(self.TARGET_PROBED, ip, port, hostname), nil) if not ok then - self:log(ERR, "failed to clear probed flag from shm: ", err) + self:log(ERR, "failed to clear probe count from shm: ", err) end end @@ -1054,17 +1054,20 @@ end -- Runs a single healthcheck probe function checker:run_single_check(ip, port, hostname, hostheader) - -- Mark the target as probed *before* the attempt, not after: "probed" means - -- an active check was actually dispatched for this target at least once - -- (success, failure, or timeout all count), which is what a cold-start - -- readiness gate needs to know -- not whether the check succeeded. Written - -- to shm (not a local field) since a readiness probe can land on any - -- worker, not just whichever one holds the periodic-probe lock and runs - -- this function. - local probed_ok, probed_err = self.shm:set( - key_for(self.TARGET_PROBED, ip, port, hostname), true) + -- Count the attempt *before* the check runs, not after: a readiness gate + -- needs to know how many consecutive active-check attempts have actually + -- been dispatched for this target (success, failure, or timeout all count), + -- not whether any one of them succeeded. A single attempt is not enough on + -- its own -- with e.g. unhealthy.http_failures = 2 configured, the target's + -- real internal_health only converges after two consecutive attempts, so + -- the gate needs the count, not just a boolean "was it ever probed". + -- Written to shm (not a local field) since a readiness probe can land on + -- any worker, not just whichever one holds the periodic-probe lock and + -- runs this function. + local probed_key = key_for(self.TARGET_PROBED, ip, port, hostname) + local probed_ok, probed_err = self.shm:incr(probed_key, 1, 0) if not probed_ok then - self:log(ERR, "failed to mark target as probed in shm: ", probed_err) + self:log(ERR, "failed to increment probe count in shm: ", probed_err) end local sock, err = ngx.socket.tcp() @@ -2009,7 +2012,7 @@ function _M.get_target_list(name, shm_name) local state_key = key_for(self.TARGET_STATE, target.ip, target.port, target.hostname) target.status = INTERNAL_STATES[self.shm:get(state_key)] local probed_key = key_for(self.TARGET_PROBED, target.ip, target.port, target.hostname) - target.probed = self.shm:get(probed_key) == true + target.probe_count = self.shm:get(probed_key) or 0 if not target.hostheader then target.hostheader = nil end @@ -2051,16 +2054,25 @@ end -- Returns true only if the checker has at least one target AND every one of --- them has had at least one active-check attempt ("probed" means attempted, --- not "healthy" -- a failed or timed-out check still counts). false if there --- are no targets yet (nothing to probe) or any target is unprobed. nil+err on --- a shm read failure, mirroring get_target_list's own error contract. +-- them has had at least `min_attempts` consecutive active-check attempts +-- ("probed" means attempted, not "healthy" -- a failed or timed-out check +-- still counts toward the count). false if there are no targets yet (nothing +-- to probe) or any target has not yet reached min_attempts. nil+err on a shm +-- read failure, mirroring get_target_list's own error contract. +-- +-- min_attempts defaults to 1 for backward compatibility, but a caller that +-- knows the checker's own unhealthy/healthy thresholds should pass the +-- worst-case one: with e.g. unhealthy.http_failures = 2 configured, a +-- target's real internal_health only converges after two consecutive +-- attempts, so requiring only 1 would let a readiness gate open before the +-- target's true state is actually known. -- -- Deliberately a module function operating on shm (like get_target_list), --- not a `checker:` instance method: the probed bit must be readable from any --- worker evaluating a readiness gate, not just whichever one created the +-- not a `checker:` instance method: the probe count must be readable from +-- any worker evaluating a readiness gate, not just whichever one created the -- checker locally or holds the periodic-probe lock. -function _M.all_targets_probed(name, shm_name) +function _M.all_targets_probed(name, shm_name, min_attempts) + min_attempts = min_attempts or 1 local targets, err = _M.get_target_list(name, shm_name) if not targets then return nil, err @@ -2069,7 +2081,7 @@ function _M.all_targets_probed(name, shm_name) return false end for _, target in ipairs(targets) do - if not target.probed then + if (target.probe_count or 0) < min_attempts then return false end end diff --git a/hack/patches/lua-resty-healthcheck-probed-gate.patch b/hack/patches/lua-resty-healthcheck-probed-gate.patch index 24a447184a96..a2d877847e8b 100644 --- a/hack/patches/lua-resty-healthcheck-probed-gate.patch +++ b/hack/patches/lua-resty-healthcheck-probed-gate.patch @@ -2,19 +2,32 @@ # target defaults to internal_health = "healthy" (add_target's hardcoded # is_healthy=true, see apisix/healthcheck_manager.lua's create_checker), so a # pod that restarts while its backend is already unhealthy routes real -# traffic to it for several seconds -- until the first active probe corrects +# traffic to it for several seconds -- until enough active probes correct # the target's state. 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." # -# This adds a per-target "probed" flag, written to shm (not a local field, -# since only the periodic-lock-holding worker ever runs active probes, but a -# readiness-gate request can land on any worker) the first time an active -# check is actually dispatched for that target -- success, failure, or -# timeout all count; "probed" means attempted, not "healthy". A new module -# function, _M.all_targets_probed(name, shm_name), lets any worker ask -# "has every target of this checker had at least one real check yet?" +# This adds a per-target probe *count* in shm (not a local field, since only +# the periodic-lock-holding worker ever runs active probes, but a +# readiness-gate request can land on any worker), incremented each time an +# active check is actually dispatched for that target -- success, failure, +# or timeout all count; "probed" means attempted, not "healthy". A new +# module function, _M.all_targets_probed(name, shm_name, min_attempts), lets +# any worker ask "has every target of this checker had at least +# min_attempts real checks yet?" +# +# A single attempt is not enough on its own: with e.g. +# unhealthy.http_failures = 2 configured, a target's real internal_health +# only converges after two consecutive failed attempts, so a gate open after +# just 1 attempt can still route to a target whose true state is not yet +# known. min_attempts defaults to 1 for backward compatibility; callers that +# know the checker's own thresholds (apisix/healthcheck_manager.lua's +# is_resource_probed) should pass +# max(active.unhealthy.http_failures, .tcp_failures, .timeouts, +# active.healthy.successes) +# so the gate waits for the worst-case number of attempts needed to +# guarantee the real state has been reached in either direction. # # Stacks on top of hack/patches/lua-resty-healthcheck-13888.patch -- apply # that one first. Targets the same lua-resty-healthcheck-api7 3.2.3 base. @@ -40,32 +53,35 @@ end + ok, err = self.shm:set(key_for(self.TARGET_PROBED, ip, port, hostname), nil) + if not ok then -+ self:log(ERR, "failed to clear probed flag from shm: ", err) ++ self:log(ERR, "failed to clear probe count from shm: ", err) + end end -@@ -1050,6 +1054,19 @@ +@@ -1050,6 +1054,22 @@ -- Runs a single healthcheck probe function checker:run_single_check(ip, port, hostname, hostheader) -+ -- Mark the target as probed *before* the attempt, not after: "probed" means -+ -- an active check was actually dispatched for this target at least once -+ -- (success, failure, or timeout all count), which is what a cold-start -+ -- readiness gate needs to know -- not whether the check succeeded. Written -+ -- to shm (not a local field) since a readiness probe can land on any -+ -- worker, not just whichever one holds the periodic-probe lock and runs -+ -- this function. -+ local probed_ok, probed_err = self.shm:set( -+ key_for(self.TARGET_PROBED, ip, port, hostname), true) ++ -- Count the attempt *before* the check runs, not after: a readiness gate ++ -- needs to know how many consecutive active-check attempts have actually ++ -- been dispatched for this target (success, failure, or timeout all count), ++ -- not whether any one of them succeeded. A single attempt is not enough on ++ -- its own -- with e.g. unhealthy.http_failures = 2 configured, the target's ++ -- real internal_health only converges after two consecutive attempts, so ++ -- the gate needs the count, not just a boolean "was it ever probed". ++ -- Written to shm (not a local field) since a readiness probe can land on ++ -- any worker, not just whichever one holds the periodic-probe lock and ++ -- runs this function. ++ local probed_key = key_for(self.TARGET_PROBED, ip, port, hostname) ++ local probed_ok, probed_err = self.shm:incr(probed_key, 1, 0) + if not probed_ok then -+ self:log(ERR, "failed to mark target as probed in shm: ", probed_err) ++ self:log(ERR, "failed to increment probe count in shm: ", probed_err) + end + local sock, err = ngx.socket.tcp() if not sock then self:log(ERR, "failed to create stream socket: ", err) -@@ -1773,6 +1790,7 @@ +@@ -1773,6 +1793,7 @@ self.TARGET_LIST = SHM_PREFIX .. self.name .. ":target_list" self.TARGET_LIST_LOCK = SHM_PREFIX .. self.name .. ":target_list_lock" self.TARGET_LOCK = SHM_PREFIX .. self.name .. ":target_lock" @@ -73,7 +89,7 @@ self.PERIODIC_LOCK = SHM_PREFIX .. ":period_lock:" -- prepare constants self.EVENT_SOURCE = EVENT_SOURCE_PREFIX .. " [" .. self.name .. "]" -@@ -1982,6 +2000,7 @@ +@@ -1982,6 +2003,7 @@ self.TARGET_COUNTER = SHM_PREFIX .. self.name .. ":counter" self.TARGET_LIST = SHM_PREFIX .. self.name .. ":target_list" self.TARGET_LIST_LOCK = SHM_PREFIX .. self.name .. ":target_list_lock" @@ -81,30 +97,39 @@ self.LOG_PREFIX = LOG_PREFIX .. "(" .. self.name .. ") " local ok, err = locking_target_list(self, function(target_list) -@@ -1989,6 +2008,8 @@ +@@ -1989,6 +2011,8 @@ for _, target in ipairs(self.targets) do local state_key = key_for(self.TARGET_STATE, target.ip, target.port, target.hostname) target.status = INTERNAL_STATES[self.shm:get(state_key)] + local probed_key = key_for(self.TARGET_PROBED, target.ip, target.port, target.hostname) -+ target.probed = self.shm:get(probed_key) == true ++ target.probe_count = self.shm:get(probed_key) or 0 if not target.hostheader then target.hostheader = nil end -@@ -2029,6 +2050,33 @@ +@@ -2029,6 +2053,42 @@ end +-- Returns true only if the checker has at least one target AND every one of -+-- them has had at least one active-check attempt ("probed" means attempted, -+-- not "healthy" -- a failed or timed-out check still counts). false if there -+-- are no targets yet (nothing to probe) or any target is unprobed. nil+err on -+-- a shm read failure, mirroring get_target_list's own error contract. ++-- them has had at least `min_attempts` consecutive active-check attempts ++-- ("probed" means attempted, not "healthy" -- a failed or timed-out check ++-- still counts toward the count). false if there are no targets yet (nothing ++-- to probe) or any target has not yet reached min_attempts. nil+err on a shm ++-- read failure, mirroring get_target_list's own error contract. ++-- ++-- min_attempts defaults to 1 for backward compatibility, but a caller that ++-- knows the checker's own unhealthy/healthy thresholds should pass the ++-- worst-case one: with e.g. unhealthy.http_failures = 2 configured, a ++-- target's real internal_health only converges after two consecutive ++-- attempts, so requiring only 1 would let a readiness gate open before the ++-- target's true state is actually known. +-- +-- Deliberately a module function operating on shm (like get_target_list), -+-- not a `checker:` instance method: the probed bit must be readable from any -+-- worker evaluating a readiness gate, not just whichever one created the ++-- not a `checker:` instance method: the probe count must be readable from ++-- any worker evaluating a readiness gate, not just whichever one created the +-- checker locally or holds the periodic-probe lock. -+function _M.all_targets_probed(name, shm_name) ++function _M.all_targets_probed(name, shm_name, min_attempts) ++ min_attempts = min_attempts or 1 + local targets, err = _M.get_target_list(name, shm_name) + if not targets then + return nil, err @@ -113,7 +138,7 @@ + return false + end + for _, target in ipairs(targets) do -+ if not target.probed then ++ if (target.probe_count or 0) < min_attempts then + return false + end + end diff --git a/t/node/healthcheck-fresh-node-default-healthy.t b/t/node/healthcheck-fresh-node-default-healthy.t index 166ec23296fa..cd10626f3a04 100644 --- a/t/node/healthcheck-fresh-node-default-healthy.t +++ b/t/node/healthcheck-fresh-node-default-healthy.t @@ -272,3 +272,84 @@ GET /t ensure_checker: true nil resource probed after ensure_checker with zero traffic: true --- timeout: 5 + + + +=== TEST 5: all_targets_probed() with min_attempts > 1 waits for real convergence +# Closes a gap found via live kind testing: a single active-check attempt is +# not enough to know a target's real state when unhealthy.http_failures (or +# .tcp_failures/.timeouts, or healthy.successes) is configured above 1 -- +# internal_health only actually converges after that many CONSECUTIVE +# attempts. A readiness gate that opens after just 1 attempt can still route +# real traffic to a target whose true state has not yet been reached. This +# is why the shm value is now a per-target probe *count* +# (hack/patches/lua-resty-healthcheck-probed-gate.patch), not a boolean, and +# why all_targets_probed() takes an explicit min_attempts argument that +# apisix.healthcheck_manager.is_resource_probed() computes from the +# checker's own thresholds (required_probe_attempts()). +--- config + location /t { + content_by_lua_block { + local healthcheck = require("resty.healthcheck") + + local checker = healthcheck.new({ + name = "test-ps12691-min-attempts", + shm_name = "upstream-healthcheck", + checks = { + active = { + type = "http", + -- a path the mock backend does not serve: 404 is in + -- the default active.unhealthy.http_statuses list, + -- so every attempt fails deterministically without + -- needing a dedicated always-500 mock endpoint + http_path = "/definitely-not-a-real-endpoint-ps12691", + timeout = 1, + healthy = { interval = 1, successes = 1 }, + unhealthy = { interval = 1, http_failures = 2 }, + }, + }, + events_module = "resty.events", + }) + if not checker then + ngx.say("failed to create checker") + return + end + + local ok, err = checker:add_target("127.0.0.1", 1980, nil, true, nil) + if not ok then + ngx.say("failed to add target: ", err) + return + end + ngx.sleep(0.2) -- let add_target's own event settle locally + + ngx.sleep(1.5) -- past one interval: exactly one attempt has fired + + local after_one = healthcheck.all_targets_probed( + "test-ps12691-min-attempts", "upstream-healthcheck", 2) + ngx.say("after 1 attempt, min_attempts=2: ", tostring(after_one)) + + -- min_attempts=1 must already be satisfied after just 1 attempt, + -- proving the gap is specifically about the threshold, not a + -- broken counter. + local after_one_threshold_one = healthcheck.all_targets_probed( + "test-ps12691-min-attempts", "upstream-healthcheck", 1) + ngx.say("after 1 attempt, min_attempts=1: ", tostring(after_one_threshold_one)) + + ngx.sleep(1.2) -- past a second interval: a second attempt has fired + + local after_two = healthcheck.all_targets_probed( + "test-ps12691-min-attempts", "upstream-healthcheck", 2) + ngx.say("after 2 attempts, min_attempts=2: ", tostring(after_two)) + + checker:stop() + } + } +--- request +GET /t +--- response_body +after 1 attempt, min_attempts=2: false +after 1 attempt, min_attempts=1: true +after 2 attempts, min_attempts=2: true +--- no_error_log +[error] +--- timeout: 8 From 38357919c497bf24a21a81c5ba55326645ad5c9d Mon Sep 17 00:00:00 2001 From: Andre Nogueira Date: Fri, 28 Aug 2026 00:48:35 +0100 Subject: [PATCH 4/4] chore(hack): add a kind-based end-to-end validation harness for both fixes Local, Docker/kind-based reproduction and validation setup used throughout this branch's work, kept for anyone re-verifying either fix or extending it: - Dockerfile: builds APISIX from this repo's source, optionally applying the #13888 reconcile patch and/or the probed-gate patch, with an optional events-queue-shrink knob for forcing the #13888 queue-overflow path deterministically. - run-repro.sh / coldstart-trial.sh / coldstart-gate-trial.sh / coldstart-gate-kind-trial.sh: plain-docker and kind end-to-end trials measuring real leaked traffic against a mock backend, stock vs patched. - kind-cluster.yaml, manifests/, mock/, plugin/: supporting cluster config, mock upstream backends, and a staged copy of edge-app's mollie-health-check plugin (needed since a Docker build context cannot reach outside this repo). Not part of either patch itself -- a local testing aid only. Signed-off-by: Andre Nogueira --- hack/kind-repro/Dockerfile | 101 ++++++++ hack/kind-repro/coldstart-apisix.yaml | 46 ++++ hack/kind-repro/coldstart-gate-apisix.yaml | 55 ++++ hack/kind-repro/coldstart-gate-config.yaml | 11 + hack/kind-repro/coldstart-gate-kind-trial.sh | 102 ++++++++ hack/kind-repro/coldstart-gate-trial.sh | 72 +++++ hack/kind-repro/coldstart-trial.sh | 33 +++ hack/kind-repro/kind-cluster.yaml | 14 + .../manifests/apisix-config-13888.yaml | 97 +++++++ .../apisix-config-coldstart-gate.yaml | 73 ++++++ .../manifests/apisix-deployment-13888.yaml | 49 ++++ .../apisix-deployment-coldstart-gate.yaml | 47 ++++ .../manifests/mock-payments-api.yaml | 36 +++ hack/kind-repro/manifests/mock-payproc.yaml | 28 ++ hack/kind-repro/mock/Dockerfile | 3 + hack/kind-repro/mock/payments-api.conf | 21 ++ hack/kind-repro/mock/payproc.conf | 14 + .../kind-repro/plugin/mollie-health-check.lua | 245 ++++++++++++++++++ hack/kind-repro/run-repro.sh | 127 +++++++++ hack/kind-repro/toggle.sh | 25 ++ 20 files changed, 1199 insertions(+) create mode 100644 hack/kind-repro/Dockerfile create mode 100644 hack/kind-repro/coldstart-apisix.yaml create mode 100644 hack/kind-repro/coldstart-gate-apisix.yaml create mode 100644 hack/kind-repro/coldstart-gate-config.yaml create mode 100755 hack/kind-repro/coldstart-gate-kind-trial.sh create mode 100755 hack/kind-repro/coldstart-gate-trial.sh create mode 100755 hack/kind-repro/coldstart-trial.sh create mode 100644 hack/kind-repro/kind-cluster.yaml create mode 100644 hack/kind-repro/manifests/apisix-config-13888.yaml create mode 100644 hack/kind-repro/manifests/apisix-config-coldstart-gate.yaml create mode 100644 hack/kind-repro/manifests/apisix-deployment-13888.yaml create mode 100644 hack/kind-repro/manifests/apisix-deployment-coldstart-gate.yaml create mode 100644 hack/kind-repro/manifests/mock-payments-api.yaml create mode 100644 hack/kind-repro/manifests/mock-payproc.yaml create mode 100644 hack/kind-repro/mock/Dockerfile create mode 100644 hack/kind-repro/mock/payments-api.conf create mode 100644 hack/kind-repro/mock/payproc.conf create mode 100644 hack/kind-repro/plugin/mollie-health-check.lua create mode 100755 hack/kind-repro/run-repro.sh create mode 100755 hack/kind-repro/toggle.sh diff --git a/hack/kind-repro/Dockerfile b/hack/kind-repro/Dockerfile new file mode 100644 index 000000000000..577639a0c9e5 --- /dev/null +++ b/hack/kind-repro/Dockerfile @@ -0,0 +1,101 @@ +# +# Local repro image: builds APISIX from this repo's source, optionally with +# the local apache/apisix#13888 patch (hack/patches/lua-resty-healthcheck-13888.patch) +# applied to the vendored lua-resty-healthcheck-api7 rock. +# +# Build stock: docker build -f hack/kind-repro/Dockerfile -t apisix-repro:stock . +# Build patched: docker build -f hack/kind-repro/Dockerfile --build-arg APPLY_PATCH=true -t apisix-repro:patched . +# Build cold-start gate (reconcile + probed-gate patches, mollie-health-check +# plugin included): docker build -f hack/kind-repro/Dockerfile +# --build-arg APPLY_PATCH=true --build-arg APPLY_PROBED_GATE_PATCH=true +# -t apisix-repro:coldstart-gate . +# +# Adapted from docker/debian-dev/Dockerfile (this repo's own source-build +# Dockerfile) -- not editing that file directly since it's a tracked, +# upstream-shared file and this is a throwaway local repro image. +FROM debian:bullseye-slim AS build + +ARG APPLY_PATCH=false +# Stacks on APPLY_PATCH -- the probed-gate patch's diff assumes the #13888 +# reconcile patch is already applied (see hack/patches/lua-resty-healthcheck- +# probed-gate.patch's own header). Applying this without APPLY_PATCH=true +# will fail to apply cleanly; that failure is intentional, not worth guarding. +ARG APPLY_PROBED_GATE_PATCH=false +# TEST-ONLY: force real apache/apisix#13888-style dropped worker_events +# broadcasts by shrinking the resty.events per-worker queue (default 10,240, +# not exposed as an apisix config knob) down to 2 -- a couple of rapid +# health-state transitions is then enough to hit queue.lua's real +# "queue overflow" push() failure, which broker.lua just logs and drops +# (no retry/redelivery). Never for production images. +ARG SHRINK_EVENTS_QUEUE=false + +ENV DEBIAN_FRONTEND=noninteractive +ENV ENV_INST_LUADIR=/usr/local/apisix + +COPY . /apisix +WORKDIR /apisix + +RUN set -x \ + && apt-get -y update --fix-missing \ + && apt-get install -y \ + make \ + git \ + sudo \ + ca-certificates \ + curl \ + gcc \ + g++ \ + cmake \ + libyaml-dev \ + libxml2-dev \ + libxslt1-dev \ + pkg-config \ + libssl-dev \ + zlib1g-dev \ + patch \ + && rm -rf deps \ + && make deps \ + && if [ "$APPLY_PATCH" = "true" ]; then \ + patch -p3 -d deps/share/lua/5.1/resty < hack/patches/lua-resty-healthcheck-13888.patch; \ + fi \ + && if [ "$APPLY_PROBED_GATE_PATCH" = "true" ]; then \ + patch -p3 -d deps/share/lua/5.1/resty < hack/patches/lua-resty-healthcheck-probed-gate.patch; \ + fi \ + && if [ "$SHRINK_EVENTS_QUEUE" = "true" ]; then \ + sed -i 's/listening = listening,.*/&\n max_queue_len = 2,/' apisix/events.lua && \ + grep -n "max_queue_len" apisix/events.lua; \ + fi \ + && mkdir -p ${ENV_INST_LUADIR} \ + && cp -r deps ${ENV_INST_LUADIR} \ + && make install + +FROM debian:bullseye-slim + +RUN apt-get -y update --fix-missing \ + && apt-get install -y libldap2-dev libyaml-0-2 libxml2 libxslt1.1 curl \ + && apt-get remove --purge --auto-remove -y + +COPY --from=build /usr/local/apisix /usr/local/apisix +COPY --from=build /usr/local/openresty /usr/local/openresty +COPY --from=build /usr/bin/apisix /usr/bin/apisix + +# Mollie's readiness plugin (edge-app), staged into this repo's build context +# at hack/kind-repro/plugin/ -- harmless if unused (only loaded if a config's +# `plugins:` list names it). +COPY hack/kind-repro/plugin/mollie-health-check.lua /usr/local/apisix/apisix/plugins/mollie-health-check.lua + +ENV PATH=$PATH:/usr/local/openresty/luajit/bin:/usr/local/openresty/nginx/sbin:/usr/local/openresty/bin + +WORKDIR /usr/local/apisix + +RUN ln -sf /dev/stdout /usr/local/apisix/logs/access.log \ + && ln -sf /dev/stderr /usr/local/apisix/logs/error.log + +EXPOSE 9080 9443 + +COPY docker/debian-dev/docker-entrypoint.sh /docker-entrypoint.sh +RUN chmod +x /docker-entrypoint.sh + +ENTRYPOINT ["/docker-entrypoint.sh"] +CMD ["docker-start"] +STOPSIGNAL SIGQUIT diff --git a/hack/kind-repro/coldstart-apisix.yaml b/hack/kind-repro/coldstart-apisix.yaml new file mode 100644 index 000000000000..8921a7bc5369 --- /dev/null +++ b/hack/kind-repro/coldstart-apisix.yaml @@ -0,0 +1,46 @@ +routes: + - + id: pay + uri: /pay + upstream_id: online-payment-write +upstreams: + - + id: online-payment-write + type: roundrobin + scheme: http + pass_host: node + checks: + active: + type: http + timeout: 2 + http_path: /apisix-health-v2-online-payment-write + healthy: + interval: 3 + http_statuses: [200, 404] + successes: 2 + unhealthy: + interval: 3 + http_statuses: [500, 502, 503, 504] + http_failures: 2 + tcp_failures: 2 + timeouts: 2 + passive: + type: http + healthy: + http_statuses: [200, 201] + successes: PASSIVE_SUCCESSES_PLACEHOLDER + unhealthy: + http_statuses: [500, 502, 503, 504] + http_failures: 2 + tcp_failures: 2 + timeouts: 2 + nodes: + - host: mock-payments-api + port: 80 + weight: 100 + priority: 0 + - host: mock-payproc + port: 80 + weight: 50 + priority: -1 +#END diff --git a/hack/kind-repro/coldstart-gate-apisix.yaml b/hack/kind-repro/coldstart-gate-apisix.yaml new file mode 100644 index 000000000000..34a890b0ecb4 --- /dev/null +++ b/hack/kind-repro/coldstart-gate-apisix.yaml @@ -0,0 +1,55 @@ +routes: + - + id: pay + uri: /pay + upstream_id: online-payment-write + - + id: health-check + uris: + - /health_check_internal/ready + - /health_check_internal/live + plugins: + mollie-health-check: + critical_upstreams: CRITICAL_UPSTREAMS_PLACEHOLDER + critical_upstreams_max_wait: 10 +upstreams: + - + id: online-payment-write + type: roundrobin + scheme: http + pass_host: node + checks: + active: + type: http + timeout: 2 + http_path: /apisix-health-v2-online-payment-write + healthy: + interval: 3 + http_statuses: [200, 404] + successes: 2 + unhealthy: + interval: 3 + http_statuses: [500, 502, 503, 504] + http_failures: 2 + tcp_failures: 2 + timeouts: 2 + passive: + type: http + healthy: + http_statuses: [200, 201] + successes: 0 + unhealthy: + http_statuses: [500, 502, 503, 504] + http_failures: 2 + tcp_failures: 2 + timeouts: 2 + nodes: + - host: mock-payments-api + port: 80 + weight: 100 + priority: 0 + - host: mock-payproc + port: 80 + weight: 50 + priority: -1 +#END diff --git a/hack/kind-repro/coldstart-gate-config.yaml b/hack/kind-repro/coldstart-gate-config.yaml new file mode 100644 index 000000000000..5c1eca725d37 --- /dev/null +++ b/hack/kind-repro/coldstart-gate-config.yaml @@ -0,0 +1,11 @@ +deployment: + role: data_plane + role_data_plane: + config_provider: yaml +nginx_config: + worker_processes: 2 + error_log_level: warn +apisix: + enable_control: true +plugins: + - mollie-health-check diff --git a/hack/kind-repro/coldstart-gate-kind-trial.sh b/hack/kind-repro/coldstart-gate-kind-trial.sh new file mode 100755 index 000000000000..0f05fe84705a --- /dev/null +++ b/hack/kind-repro/coldstart-gate-kind-trial.sh @@ -0,0 +1,102 @@ +#!/usr/bin/env bash +# End-to-end proof that a real Kubernetes Service + readinessProbe, pointed at +# the cold-start gate's /health_check_internal/ready, actually withholds +# traffic from a pod until its critical upstream has been probed -- the thing +# the plain-docker trial (coldstart-gate-trial.sh) cannot show, since a raw +# `docker run -p` has no readiness-probe concept at all. +# +# Requires: the kind cluster from kind-cluster.yaml already up (kind create +# cluster --config kind-cluster.yaml), mock-payments-api/mock-payproc already +# applied and Running, and apisix-repro:coldstart-gate already `kind load +# docker-image`d. Uses kubectl context kind-apisix-healthcheck-repro. +set -uo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")" +CTX=kind-apisix-healthcheck-repro +NS=default + +kubectl --context "$CTX" -n "$NS" delete deployment,service apisix-repro-coldstart --ignore-not-found >/dev/null 2>&1 +kubectl --context "$CTX" -n "$NS" delete pod loadgen --ignore-not-found --grace-period=0 --force >/dev/null 2>&1 +kubectl --context "$CTX" -n "$NS" apply -f manifests/apisix-config-coldstart-gate.yaml >/dev/null + +./toggle.sh payments-api unhealthy >/dev/null + +echo "=== starting loadgen pod (hammers the Service by DNS name before the apisix pod even exists) ===" +kubectl --context "$CTX" -n "$NS" run loadgen --image=curlimages/curl --restart=Never --command -- \ + sh -c 'i=0; while [ $i -lt 700 ]; do + rm -f /tmp/body + t=$(date +%s) + code=$(curl -s -o /tmp/body -w "%{http_code}" --max-time 1 http://apisix-repro-coldstart.default.svc.cluster.local:9080/pay 2>/dev/null) + body=$(cat /tmp/body 2>/dev/null | tail -c 100) + echo "$t code=$code body=$body" + i=$((i+1)) + sleep 0.05 + done' >/dev/null 2>&1 + +# wait for loadgen to actually be running before triggering the cold start +for i in $(seq 1 30); do + phase=$(kubectl --context "$CTX" -n "$NS" get pod loadgen -o jsonpath='{.status.phase}' 2>/dev/null) + [ "$phase" = "Running" ] && break + sleep 0.3 +done +echo "loadgen phase: $phase" + +start=$(date +%s.%N) +echo "=== t=0: applying apisix-repro-coldstart deployment (the cold start) ===" +kubectl --context "$CTX" -n "$NS" apply -f manifests/apisix-deployment-coldstart-gate.yaml >/dev/null + +# Poll readiness at high frequency, timestamped relative to $start. +ready_at=-1 +for i in $(seq 1 150); do + ready=$(kubectl --context "$CTX" -n "$NS" get pods -l app=apisix-repro-coldstart \ + -o jsonpath='{.items[0].status.containerStatuses[0].ready}' 2>/dev/null) + now=$(date +%s.%N) + elapsed=$(echo "$now - $start" | bc) + if [ "$ready" = "true" ] && [ "$ready_at" = "-1" ]; then + ready_at="$elapsed" + echo " t=+${elapsed}s: pod became Ready" + break + fi + sleep 0.1 +done +[ "$ready_at" = "-1" ] && echo " pod never became Ready within the poll window" + +sleep 15 # let the loadgen keep running well past the ready transition, for the "after" sample + +echo "=== waiting for loadgen to finish ===" +kubectl --context "$CTX" -n "$NS" wait --for=condition=Ready=false pod/loadgen --timeout=5s >/dev/null 2>&1 || true +kubectl --context "$CTX" -n "$NS" logs loadgen > /tmp/loadgen-coldstart.log 2>&1 + +python3 - "$start" "$ready_at" <<'PYEOF' +import sys +start = float(sys.argv[1]) +ready_at = float(sys.argv[2]) if sys.argv[2] != "-1" else None +lines = open("/tmp/loadgen-coldstart.log").read().splitlines() +before = {"total": 0, "leaked": 0, "correct": 0, "no_response": 0} +after = {"total": 0, "leaked": 0, "correct": 0, "no_response": 0} +for line in lines: + parts = line.split(" ", 1) + if not parts or not parts[0].replace(".", "", 1).isdigit(): + continue + t = float(parts[0]) - start + bucket = before if (ready_at is None or t < ready_at) else after + bucket["total"] += 1 + # Only a genuine 201 response counts as leaked/correct -- code=000 (no + # response) must never be classified by stale/absent body content. + if "code=201" in line and "payments-api" in line: + bucket["leaked"] += 1 + elif "code=201" in line and "payproc" in line: + bucket["correct"] += 1 + elif "code=000" in line: + bucket["no_response"] += 1 +print(f"ready_at = {ready_at}") +print(f"BEFORE ready: total={before['total']} leaked_to_payments_api={before['leaked']} correct_payproc={before['correct']} no_response={before['no_response']}") +print(f"AFTER ready: total={after['total']} leaked_to_payments_api={after['leaked']} correct_payproc={after['correct']} no_response={after['no_response']}") +PYEOF + +echo "=== apisix pod logs (health-check gate lines) ===" +POD=$(kubectl --context "$CTX" -n "$NS" get pods -l app=apisix-repro-coldstart -o jsonpath='{.items[0].metadata.name}') +kubectl --context "$CTX" -n "$NS" logs "$POD" 2>&1 | grep -E "critical upstream|health-check" | tail -20 + +kubectl --context "$CTX" -n "$NS" delete pod loadgen --ignore-not-found --grace-period=0 --force >/dev/null 2>&1 +./toggle.sh payments-api healthy >/dev/null +echo "=== done (full loadgen log: /tmp/loadgen-coldstart.log) ===" diff --git a/hack/kind-repro/coldstart-gate-trial.sh b/hack/kind-repro/coldstart-gate-trial.sh new file mode 100755 index 000000000000..090fc634f59d --- /dev/null +++ b/hack/kind-repro/coldstart-gate-trial.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +# Usage: ./coldstart-gate-trial.sh [image] +# +# Validates the cold-start readiness gate (mollie-health-check.lua's +# critical_upstreams) against a real running APISIX + real plugin, using the +# same already-unhealthy-mock-backend setup as coldstart-trial.sh. Unlike +# that script, this measures /health_check_internal/ready's own transition +# timing, not leaked /pay requests -- a plain `docker run -p` has no +# readiness-probe concept, so nothing here is actually gated on /ready (see +# the plan's own note on this: proving the leak itself closes needs a real +# k8s Service + readinessProbe, a separate kind-based test). What this DOES +# prove: the gate mechanism itself -- /ready stays 503 exactly as long as the +# critical upstream is genuinely unprobed, and flips to 200 only once a real +# active check has actually fired against the (already-unhealthy) backend. +set -uo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")" +MODE="${1:?enabled or disabled}" +IMAGE="${2:-apisix-repro:coldstart-gate}" + +case "$MODE" in + enabled) CRITICAL='["online-payment-write"]' ;; + disabled) CRITICAL='[]' ;; + *) echo "usage: $0 [image]" >&2; exit 1 ;; +esac + +sed "s/CRITICAL_UPSTREAMS_PLACEHOLDER/${CRITICAL}/" coldstart-gate-apisix.yaml > coldstart-gate-apisix-rendered.yaml + +docker rm -f coldstart-gate-apisix >/dev/null 2>&1 || true +./toggle.sh payments-api unhealthy >/dev/null + +start=$SECONDS +echo "=== mode=$MODE (critical_upstreams=$CRITICAL), image=$IMAGE ===" + +docker run -d --name coldstart-gate-apisix --network apisix-race-net -p 19085:9080 \ + -e APISIX_STAND_ALONE=true \ + -v "$(pwd)/coldstart-gate-config.yaml:/usr/local/apisix/conf/config.yaml:ro" \ + -v "$(pwd)/coldstart-gate-apisix-rendered.yaml:/usr/local/apisix/conf/apisix.yaml:ro" \ + "$IMAGE" >/dev/null + +# Poll /ready every 200ms for up to 15s, recording the transition. +ready_at=-1 +for i in $(seq 1 75); do + code=$(curl -s --max-time 1 -o /dev/null -w '%{http_code}' http://127.0.0.1:19085/health_check_internal/ready 2>/dev/null) + elapsed=$((SECONDS - start)) + if [ "$code" = "200" ] && [ "$ready_at" = "-1" ]; then + ready_at=$elapsed + echo " t=+${elapsed}s: /ready -> 200 (first time)" + break + fi + sleep 0.2 +done + +if [ "$ready_at" = "-1" ]; then + echo " /ready never returned 200 within the poll window" +fi + +# Cross-check against the /pay leak count, same methodology as coldstart-trial.sh, +# to confirm the backend really was still unhealthy for the relevant window +# (not gating traffic -- see header note -- just corroborating the setup). +leaked=0; total=0 +for i in $(seq 1 30); do + b=$(curl -s --max-time 1 -o /dev/null -w '%header{X-Mock-Backend}' http://127.0.0.1:19085/pay 2>/dev/null) + total=$((total+1)) + [ "$b" = "payments-api" ] && leaked=$((leaked+1)) + sleep 0.2 +done +echo "=== mode=$MODE: /ready first-200 at t=+${ready_at}s, /pay leaked ${leaked}/${total} over the same post-start window ===" + +docker logs coldstart-gate-apisix 2>&1 | grep -E "health-check|critical upstream|failing open" | tail -20 + +docker rm -f coldstart-gate-apisix >/dev/null 2>&1 +./toggle.sh payments-api healthy >/dev/null diff --git a/hack/kind-repro/coldstart-trial.sh b/hack/kind-repro/coldstart-trial.sh new file mode 100755 index 000000000000..ec6b9a0bdce3 --- /dev/null +++ b/hack/kind-repro/coldstart-trial.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +set -uo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")" +PASSIVE_SUCCESSES="${1:?2 or 0}" +IMAGE="${2:-apisix-repro:3.15.0}" + +sed "s/PASSIVE_SUCCESSES_PLACEHOLDER/${PASSIVE_SUCCESSES}/" coldstart-apisix.yaml > coldstart-apisix-rendered.yaml + +docker rm -f coldstart-apisix >/dev/null 2>&1 || true +./toggle.sh payments-api unhealthy >/dev/null +docker run -d --name coldstart-apisix --network apisix-race-net -p 19084:9080 \ + -e APISIX_STAND_ALONE=true \ + -v "$(pwd)/nilwindow-config.yaml:/usr/local/apisix/conf/config.yaml:ro" \ + -v "$(pwd)/coldstart-apisix-rendered.yaml:/usr/local/apisix/conf/apisix.yaml:ro" \ + "$IMAGE" >/dev/null +start=$SECONDS +echo "=== passive.successes=$PASSIVE_SUCCESSES, image=$IMAGE ===" +leaked=0; total=0; first_correct=-1 +for i in $(seq 1 60); do + b=$(curl -s --max-time 1 -o /dev/null -w '%header{X-Mock-Backend}' http://127.0.0.1:19084/pay 2>/dev/null) + elapsed=$((SECONDS - start)) + total=$((total+1)) + if [ "$b" = "payments-api" ]; then + leaked=$((leaked+1)) + echo " t=+${elapsed}s req $i: $b <-- leaked" + else + [ "$first_correct" = "-1" ] && [ -n "$b" ] && first_correct=$elapsed + fi + sleep 0.2 +done +echo "=== passive.successes=$PASSIVE_SUCCESSES: $leaked/$total leaked, first correct at t=+${first_correct}s ===" +docker rm -f coldstart-apisix >/dev/null 2>&1 +./toggle.sh payments-api healthy >/dev/null diff --git a/hack/kind-repro/kind-cluster.yaml b/hack/kind-repro/kind-cluster.yaml new file mode 100644 index 000000000000..3ccafd7fadbb --- /dev/null +++ b/hack/kind-repro/kind-cluster.yaml @@ -0,0 +1,14 @@ +kind: Cluster +apiVersion: kind.x-k8s.io/v1alpha4 +name: apisix-healthcheck-repro +nodes: + - role: control-plane + extraPortMappings: + - containerPort: 30980 + hostPort: 30980 + protocol: TCP + extraMounts: + - hostPath: ./state/payments-api + containerPath: /mnt/state/payments-api + - hostPath: ./state/payproc + containerPath: /mnt/state/payproc diff --git a/hack/kind-repro/manifests/apisix-config-13888.yaml b/hack/kind-repro/manifests/apisix-config-13888.yaml new file mode 100644 index 000000000000..2fa53f8f771b --- /dev/null +++ b/hack/kind-repro/manifests/apisix-config-13888.yaml @@ -0,0 +1,97 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: apisix-repro-config +data: + config.yaml: | + deployment: + role: data_plane + role_data_plane: + config_provider: yaml + nginx_config: + # Deliberately higher than the IMP-3253 test's worker_processes=2 -- + # more workers means more contention for the broker's per-worker event + # queues (now capped at 2, see hack/kind-repro/Dockerfile + # SHRINK_EVENTS_QUEUE), which is what we want for THIS test. + worker_processes: 6 + error_log_level: warn + apisix.yaml: | + routes: + - + id: healthz + uri: /healthz + plugins: + serverless-pre-function: + _meta: + disable: false + phase: rewrite + functions: + - "return function(conf, ctx) ngx.header['X-Repro-Probe'] = 'process-only'; ngx.say('ok'); ngx.exit(200) end" + - + id: stall + uri: /stall + plugins: + serverless-pre-function: + _meta: + disable: false + phase: rewrite + functions: + - "return function(conf, ctx) local dur = tonumber(ngx.var.arg_s) or 3; local start = ngx.now(); local n = 0; while true do ngx.update_time(); if ngx.now() - start >= dur then break end; n = n + 1 end; ngx.say('stalled worker ', ngx.worker.pid(), ' for ', dur, 's (', n, ' spins)'); ngx.exit(200) end" + - + id: pay + uri: /pay + upstream_id: online-payment-write + plugins: + serverless-pre-function: + _meta: + disable: false + phase: header_filter + functions: + - "return function(conf, ctx) ngx.header['X-Worker-Pid'] = ngx.worker.pid() end" + upstreams: + - + id: online-payment-write + type: roundrobin + scheme: http + pass_host: node + retries: 2 + retry_timeout: 0 + timeout: + connect: 2 + send: 2 + read: 2 + checks: + active: + type: http + timeout: 2 + http_path: /apisix-health-v2-online-payment-write + healthy: + interval: 1 + http_statuses: [200, 404] + successes: 2 + unhealthy: + interval: 1 + http_statuses: [500, 502, 503, 504] + http_failures: 2 + tcp_failures: 2 + timeouts: 2 + passive: + type: http + healthy: + http_statuses: [200, 201] + successes: 2 + unhealthy: + http_statuses: [500, 502, 503, 504] + http_failures: 2 + tcp_failures: 2 + timeouts: 2 + nodes: + - host: mock-payments-api + port: 80 + weight: 100 + priority: 0 + - host: mock-payproc + port: 80 + weight: 50 + priority: -1 + #END diff --git a/hack/kind-repro/manifests/apisix-config-coldstart-gate.yaml b/hack/kind-repro/manifests/apisix-config-coldstart-gate.yaml new file mode 100644 index 000000000000..47724c7d8406 --- /dev/null +++ b/hack/kind-repro/manifests/apisix-config-coldstart-gate.yaml @@ -0,0 +1,73 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: apisix-repro-coldstart-config +data: + config.yaml: | + deployment: + role: data_plane + role_data_plane: + config_provider: yaml + nginx_config: + worker_processes: 2 + error_log_level: warn + apisix: + enable_control: true + plugins: + - mollie-health-check + apisix.yaml: | + routes: + - + id: pay + uri: /pay + upstream_id: online-payment-write + - + id: health-check + uris: + - /health_check_internal/ready + - /health_check_internal/live + plugins: + mollie-health-check: + critical_upstreams: ["online-payment-write"] + critical_upstreams_max_wait: 10 + upstreams: + - + id: online-payment-write + type: roundrobin + scheme: http + pass_host: node + checks: + active: + type: http + timeout: 2 + http_path: /apisix-health-v2-online-payment-write + healthy: + interval: 3 + http_statuses: [200, 404] + successes: 2 + unhealthy: + interval: 3 + http_statuses: [500, 502, 503, 504] + http_failures: 2 + tcp_failures: 2 + timeouts: 2 + passive: + type: http + healthy: + http_statuses: [200, 201] + successes: 0 + unhealthy: + http_statuses: [500, 502, 503, 504] + http_failures: 2 + tcp_failures: 2 + timeouts: 2 + nodes: + - host: mock-payments-api + port: 80 + weight: 100 + priority: 0 + - host: mock-payproc + port: 80 + weight: 50 + priority: -1 + #END diff --git a/hack/kind-repro/manifests/apisix-deployment-13888.yaml b/hack/kind-repro/manifests/apisix-deployment-13888.yaml new file mode 100644 index 000000000000..4c54933f49d8 --- /dev/null +++ b/hack/kind-repro/manifests/apisix-deployment-13888.yaml @@ -0,0 +1,49 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: apisix-repro +spec: + replicas: 1 + selector: + matchLabels: {app: apisix-repro} + template: + metadata: + labels: {app: apisix-repro} + spec: + containers: + - name: apisix + image: __APISIX_IMAGE__ + imagePullPolicy: IfNotPresent + env: + - name: APISIX_STAND_ALONE + value: "true" + ports: + - containerPort: 9080 + readinessProbe: + httpGet: {path: /healthz, port: 9080} + initialDelaySeconds: 0 + periodSeconds: 2 + failureThreshold: 1 + volumeMounts: + - name: config + mountPath: /usr/local/apisix/conf/config.yaml + subPath: config.yaml + - name: config + mountPath: /usr/local/apisix/conf/apisix.yaml + subPath: apisix.yaml + volumes: + - name: config + configMap: + name: apisix-repro-config +--- +apiVersion: v1 +kind: Service +metadata: + name: apisix-repro +spec: + type: NodePort + selector: {app: apisix-repro} + ports: + - port: 9080 + targetPort: 9080 + nodePort: 30980 diff --git a/hack/kind-repro/manifests/apisix-deployment-coldstart-gate.yaml b/hack/kind-repro/manifests/apisix-deployment-coldstart-gate.yaml new file mode 100644 index 000000000000..abb711c28c6f --- /dev/null +++ b/hack/kind-repro/manifests/apisix-deployment-coldstart-gate.yaml @@ -0,0 +1,47 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: apisix-repro-coldstart +spec: + replicas: 1 + selector: + matchLabels: {app: apisix-repro-coldstart} + template: + metadata: + labels: {app: apisix-repro-coldstart} + spec: + containers: + - name: apisix + image: apisix-repro:coldstart-gate + imagePullPolicy: IfNotPresent + env: + - name: APISIX_STAND_ALONE + value: "true" + ports: + - containerPort: 9080 + readinessProbe: + httpGet: {path: /health_check_internal/ready, port: 9080} + initialDelaySeconds: 0 + periodSeconds: 1 + failureThreshold: 1 + volumeMounts: + - name: config + mountPath: /usr/local/apisix/conf/config.yaml + subPath: config.yaml + - name: config + mountPath: /usr/local/apisix/conf/apisix.yaml + subPath: apisix.yaml + volumes: + - name: config + configMap: + name: apisix-repro-coldstart-config +--- +apiVersion: v1 +kind: Service +metadata: + name: apisix-repro-coldstart +spec: + selector: {app: apisix-repro-coldstart} + ports: + - port: 9080 + targetPort: 9080 diff --git a/hack/kind-repro/manifests/mock-payments-api.yaml b/hack/kind-repro/manifests/mock-payments-api.yaml new file mode 100644 index 000000000000..588a1322c36f --- /dev/null +++ b/hack/kind-repro/manifests/mock-payments-api.yaml @@ -0,0 +1,36 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: mock-payments-api +spec: + replicas: 1 + selector: + matchLabels: {app: mock-payments-api} + template: + metadata: + labels: {app: mock-payments-api} + spec: + containers: + - name: nginx + image: apisix-repro/mock-payments-api:local + imagePullPolicy: IfNotPresent + ports: + - containerPort: 80 + volumeMounts: + - name: state + mountPath: /state + volumes: + - name: state + hostPath: + path: /mnt/state/payments-api + type: DirectoryOrCreate +--- +apiVersion: v1 +kind: Service +metadata: + name: mock-payments-api +spec: + selector: {app: mock-payments-api} + ports: + - port: 80 + targetPort: 80 diff --git a/hack/kind-repro/manifests/mock-payproc.yaml b/hack/kind-repro/manifests/mock-payproc.yaml new file mode 100644 index 000000000000..b1e75c84797f --- /dev/null +++ b/hack/kind-repro/manifests/mock-payproc.yaml @@ -0,0 +1,28 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: mock-payproc +spec: + replicas: 1 + selector: + matchLabels: {app: mock-payproc} + template: + metadata: + labels: {app: mock-payproc} + spec: + containers: + - name: nginx + image: apisix-repro/mock-payproc:local + imagePullPolicy: IfNotPresent + ports: + - containerPort: 80 +--- +apiVersion: v1 +kind: Service +metadata: + name: mock-payproc +spec: + selector: {app: mock-payproc} + ports: + - port: 80 + targetPort: 80 diff --git a/hack/kind-repro/mock/Dockerfile b/hack/kind-repro/mock/Dockerfile new file mode 100644 index 000000000000..94c35d1dd39e --- /dev/null +++ b/hack/kind-repro/mock/Dockerfile @@ -0,0 +1,3 @@ +FROM nginx:alpine +ARG BACKEND_CONF=payments-api.conf +COPY ${BACKEND_CONF} /etc/nginx/conf.d/default.conf diff --git a/hack/kind-repro/mock/payments-api.conf b/hack/kind-repro/mock/payments-api.conf new file mode 100644 index 000000000000..0856657f4087 --- /dev/null +++ b/hack/kind-repro/mock/payments-api.conf @@ -0,0 +1,21 @@ +server { + listen 80; + + # Dedicated healthcheck endpoint -- respects the toggle sentinel. + # Path matches the real upstream's checks.active.http_path. + location /apisix-health-v2-online-payment-write { + if (-f /state/unhealthy) { + return 503; + } + return 200 'OK'; + } + + # Real traffic endpoint -- always succeeds, regardless of the toggle. + # Matches confirmed prod behavior: the "unhealthy" flag only affects the + # dedicated healthcheck path, not real payment requests. + location /pay { + default_type application/json; + add_header X-Mock-Backend "payments-api" always; + return 201 '{"backend":"payments-api"}'; + } +} diff --git a/hack/kind-repro/mock/payproc.conf b/hack/kind-repro/mock/payproc.conf new file mode 100644 index 000000000000..6b6735112b2c --- /dev/null +++ b/hack/kind-repro/mock/payproc.conf @@ -0,0 +1,14 @@ +server { + listen 80; + + # Fallback backend -- always healthy, never toggled. + location /apisix-health-v2-online-payment-write { + return 200 'OK'; + } + + location /pay { + default_type application/json; + add_header X-Mock-Backend "payproc" always; + return 201 '{"backend":"payproc"}'; + } +} diff --git a/hack/kind-repro/plugin/mollie-health-check.lua b/hack/kind-repro/plugin/mollie-health-check.lua new file mode 100644 index 000000000000..7a6300ca308f --- /dev/null +++ b/hack/kind-repro/plugin/mollie-health-check.lua @@ -0,0 +1,245 @@ +local core = require('apisix.core') +local http = require('resty.http') +local ngx = ngx + +local plugin_name = 'mollie-health-check' + +-- Gates the readiness probe on plugin-load health so a stale image missing a plugin +-- fails readiness instead of silently serving degraded traffic. Design and operational +-- details: docs/guides-and-runbooks/readiness-plugin-load-gating.md (PS-11555). + +-- APISIX Control API endpoint that lists the plugins APISIX actually registered. +-- It is backed by apisix.plugin.get_all(), so a plugin that failed to load is +-- absent here. Localhost-only; requires `enable_control: true` in config.yaml. +local control_schema_url = "http://127.0.0.1:9090/v1/schema" + +-- Worker-level cache of the readiness verdict: +-- false = not yet confirmed (re-checked on each readiness probe) +-- true = every configured plugin is loaded (latched: the plugin set is fixed +-- for the pod's lifetime in standalone mode, so we never re-query after +-- a success and steady-state probes do zero HTTP). +-- A failure is deliberately NOT latched, so a transient Control API error during +-- early startup self-heals on the next probe. +local plugins_verified = false + +-- Cold-start readiness gate (PS-12691): a freshly created health-check target +-- defaults to "healthy" with zero probes (resty.healthcheck's add_target), 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. Blocks readiness until every configured critical upstream has had +-- at least one real active-check attempt. Worker-scoped: `ensured_once` so +-- ensure_checker is only called once per worker (its underlying waiting_pool +-- is per-worker state), `gate_started_at` to bound how long an unresolvable +-- critical upstream (typo'd id, checker never created) can block readiness -- +-- a plain in-Lua timestamp, not the drain-marker gate's procfs-based +-- container-start helper, since this only needs "how long has this worker +-- been evaluating the gate," not a cross-container file-vs-start comparison, +-- and its safe-fallback direction is the opposite of the drain marker's +-- (fail toward not-ready, not toward ready, until the bound is hit). +local ensured_once = false +local gate_started_at = ngx.time() + +local schema = { + type = "object", + properties = { + maintenance_file = {type = "string", default = "/tmp/maintenance_mode_enabled"}, + normal_status = {type = "integer", default = ngx.HTTP_OK, minimum = 100, maximum = 599}, + normal_response_message = {type = "string", default = "ok"}, + maintenance_status = {type = "integer", default = ngx.HTTP_SERVICE_UNAVAILABLE, minimum = 100, maximum = 599}, + maintenance_response_message = {type = "string", default = "not_ok"}, + ready_uri = {type = "string", default = "/health_check_internal/ready"}, + live_uri = {type = "string", default = "/health_check_internal/live"}, + critical_upstreams = { + type = "array", + items = {type = "string"}, + default = {}, + description = "Upstream ids (as configured under apisixUpstreams) that must have " + .. "completed at least one active health-check probe before this pod reports ready.", + }, + critical_upstreams_max_wait = { + type = "integer", + default = 15, + minimum = 0, + description = "Seconds after which an unprobed critical upstream stops blocking " + .. "readiness (fail-open, logged loudly) -- guards a typo'd id or a checker " + .. "that never gets created.", + }, + }, +} + +local _M = { + version = 0.1, + priority = 1000, + name = plugin_name, + schema = schema, +} + +function _M.check_schema(conf, _schema_type) + return core.schema.check(schema, conf) +end + +-- Returns true only if every plugin listed in config.yaml `plugins:` is present +-- in APISIX's loaded set (per the Control API). Fail-closed: any error or any +-- missing plugin returns false so the readiness probe fails and the pod is held +-- out of rotation instead of silently serving 404s for the dropped routes. +local function all_plugins_loaded() + if plugins_verified then + return true + end + + local ok_conf, config_local = pcall(require, "apisix.core.config_local") + if not ok_conf then + core.log.error("health-check: cannot load apisix.core.config_local: ", config_local) + return false + end + + local local_conf = config_local.local_conf() + -- `plugins` must be a non-empty array. A missing/empty/non-array value is + -- anomalous: the config wasn't loaded as expected. It can never legitimately + -- be empty here — this very health-check plugin runs from that same list, so + -- at minimum it must be present. Fail closed rather than treating it as + -- "nothing to verify" (the type guard also avoids `#` erroring on a non-table). + if not local_conf or type(local_conf.plugins) ~= "table" or #local_conf.plugins == 0 then + core.log.error("health-check: no plugins in local config; failing closed") + return false + end + + local configured = local_conf.plugins + + local httpc = http.new() + -- Keep the worst case (connect+send+read) comfortably under the kubelet probe + -- timeout (timeoutSeconds: 1): 3x200ms = 600ms leaves ~400ms of margin for GC + -- pauses/load. The Control API is localhost so 200ms per phase is ample. + httpc:set_timeouts(200, 200, 200) + local res, err = httpc:request_uri(control_schema_url, {method = "GET"}) + if not res then + -- A connection refused here usually means the Control API is not enabled + -- (enable_control) — readiness fails closed until it is reachable. + core.log.error("health-check: Control API request failed (is enable_control set?): ", err) + return false + end + if res.status ~= 200 then + core.log.error("health-check: Control API returned status ", res.status) + return false + end + + local body, decode_err = core.json.decode(res.body) + if not body then + core.log.error("health-check: failed to decode Control API /v1/schema: ", decode_err) + return false + end + if type(body.plugins) ~= "table" then + core.log.error("health-check: unexpected Control API /v1/schema response (no plugins map)") + return false + end + local loaded = body.plugins + + local missing = {} + for _, name in ipairs(configured) do + if loaded[name] == nil then + missing[#missing + 1] = name + end + end + + if #missing > 0 then + core.log.error("health-check: configured plugins not loaded: ", table.concat(missing, ", ")) + return false + end + + plugins_verified = true + return true +end + +-- Returns true once every id in conf.critical_upstreams has had at least one +-- real active-check attempt (resty.healthcheck's all_targets_probed -- +-- attempted, not "healthy"), or once conf.critical_upstreams_max_wait has +-- elapsed since this worker started evaluating the gate, whichever comes +-- first. No configured critical upstreams is trivially true (opt-in gate). +-- +-- ensure_checker is called at most once per worker: checker creation is +-- otherwise entirely lazy (only ever triggered by live request traffic), so +-- a critical-but-currently-idle upstream on a fresh pod would never get a +-- checker built at all without this -- this forces that seeding regardless +-- of whether the upstream has served any traffic yet. +local function critical_upstreams_probed(conf) + local ids = conf.critical_upstreams + if not ids or #ids == 0 then + return true + end + + local ok_hcm, healthcheck_manager = pcall(require, "apisix.healthcheck_manager") + if not ok_hcm then + core.log.error("health-check: cannot load apisix.healthcheck_manager: ", healthcheck_manager) + return false + end + + if not ensured_once then + for _, id in ipairs(ids) do + local ok, err = healthcheck_manager.ensure_checker("/upstreams/" .. id) + if not ok then + core.log.warn("health-check: ensure_checker failed for critical upstream '", + id, "': ", err) + end + end + ensured_once = true + end + + local unprobed = {} + for _, id in ipairs(ids) do + if not healthcheck_manager.is_resource_probed("/upstreams/" .. id) then + unprobed[#unprobed + 1] = id + end + end + + if #unprobed == 0 then + return true + end + + local elapsed = ngx.time() - gate_started_at + if elapsed >= (conf.critical_upstreams_max_wait or 15) then + core.log.error("health-check: failing open after ", elapsed, + "s -- critical upstream(s) never probed: ", + table.concat(unprobed, ", "), + " (misconfigured id, or checker never created -- investigate)") + return true + end + + core.log.warn("health-check: not ready, critical upstream(s) unprobed: ", + table.concat(unprobed, ", "), " (", elapsed, "s elapsed)") + return false +end + +function _M.rewrite(conf, _ctx) + local uri = ngx.var.uri or "" + + -- Liveness must reflect "process is alive" only, never plugin-load state, so + -- the kubelet can distinguish liveness from readiness. Keep it independent. + if uri == conf.live_uri then + return core.response.exit(conf.normal_status, conf.normal_response_message) + end + + if uri == conf.ready_uri then + if not all_plugins_loaded() then + ngx.header["X-APISIX-Route"] = "health-check-plugin-failure" + return core.response.exit(ngx.HTTP_SERVICE_UNAVAILABLE, + {status = 503, detail = "Plugin load failure - pod not ready"}) + end + + if not critical_upstreams_probed(conf) then + ngx.header["X-APISIX-Route"] = "health-check-upstream-unprobed" + return core.response.exit(ngx.HTTP_SERVICE_UNAVAILABLE, + {status = 503, detail = "Critical upstream health check not yet probed - pod not ready"}) + end + + local f = io.open(conf.maintenance_file, "r") + if f ~= nil then + f:close() + return core.response.exit(conf.maintenance_status, conf.maintenance_response_message) + end + return core.response.exit(conf.normal_status, conf.normal_response_message) + end + + return core.response.exit(ngx.HTTP_NOT_FOUND, "Not found") +end + +return _M diff --git a/hack/kind-repro/run-repro.sh b/hack/kind-repro/run-repro.sh new file mode 100755 index 000000000000..3d91fdc01ae0 --- /dev/null +++ b/hack/kind-repro/run-repro.sh @@ -0,0 +1,127 @@ +#!/usr/bin/env bash +# Local kind e2e repro: does the apache/apisix#13888 patch change leak +# count/duration during a real k8s rolling pod replacement? +# +# See /Users/andre.nogueira/.claude/plans/enchanted-chasing-sky.md for the +# full design. Usage: +# +# ./run-repro.sh setup # create cluster, build+load all images +# ./run-repro.sh variant stock # run one variant against a live cluster +# ./run-repro.sh variant patched +# ./run-repro.sh teardown # delete kind cluster +# +set -euo pipefail +cd "$(dirname "${BASH_SOURCE[0]}")" +REPO_ROOT="$(cd ../.. && pwd)" +CLUSTER=apisix-healthcheck-repro +RESULTS_DIR=./results +mkdir -p "$RESULTS_DIR" + +log() { echo "[run-repro] $*" >&2; } + +cmd_setup() { + log "building mock images" + docker build -t apisix-repro/mock-payments-api:local --build-arg BACKEND_CONF=payments-api.conf ./mock + docker build -t apisix-repro/mock-payproc:local --build-arg BACKEND_CONF=payproc.conf ./mock + + log "building apisix images (stock + patched) from $REPO_ROOT -- this compiles openresty from source, several minutes each" + docker build -f ./Dockerfile -t apisix-repro:stock "$REPO_ROOT" + docker build -f ./Dockerfile --build-arg APPLY_PATCH=true -t apisix-repro:patched "$REPO_ROOT" + + log "creating kind cluster" + kind create cluster --config ./kind-cluster.yaml --name "$CLUSTER" + + log "loading images into kind" + for img in apisix-repro/mock-payments-api:local apisix-repro/mock-payproc:local apisix-repro:stock apisix-repro:patched; do + kind load docker-image "$img" --name "$CLUSTER" + done + + log "applying mocks + config" + kubectl apply -f manifests/mock-payments-api.yaml -f manifests/mock-payproc.yaml -f manifests/apisix-config.yaml + kubectl wait --for=condition=available --timeout=60s deployment/mock-payments-api deployment/mock-payproc + + log "setup done" +} + +cmd_teardown() { + kind delete cluster --name "$CLUSTER" || true + docker rmi apisix-repro:stock apisix-repro:patched apisix-repro/mock-payments-api:local apisix-repro/mock-payproc:local 2>/dev/null || true +} + +# Render apisix-deployment.yaml with the given image + probe timing, apply it, +# wait for it to be ready. +apply_apisix_deployment() { + local image="$1" min_ready="$2" readiness_delay="$3" + sed -e "s|__APISIX_IMAGE__|$image|" \ + -e "s|__MIN_READY_SECONDS__|$min_ready|" \ + -e "s|__READINESS_INITIAL_DELAY__|$readiness_delay|" \ + manifests/apisix-deployment.yaml | kubectl apply -f - + kubectl rollout status deployment/apisix-repro --timeout=120s +} + +# Hammer the NodePort service, one line per response to $1, tagging pod + +# backend + timestamp. Runs for $2 seconds. +hammer() { + local outfile="$1" duration="$2" + local end=$((SECONDS + duration)) + : > "$outfile" + while [ $SECONDS -lt $end ]; do + resp=$(curl -s --max-time 2 -o /dev/null -w '%{http_code} %header{X-Mock-Backend}\n' \ + -H 'Host: localhost' "http://127.0.0.1:30980/pay" 2>/dev/null || echo "ERR") + echo "$(date +%s.%N) $resp" >> "$outfile" + done +} + +cmd_variant() { + local variant="$1" # stock | patched + local probe_mode="${2:-fast}" # fast (no mitigation) | mitigated (prod IMP-3220 values) + local image="apisix-repro:$variant" + local min_ready=0 readiness_delay=0 + if [ "$probe_mode" = "mitigated" ]; then + min_ready=30 + readiness_delay=30 + fi + + log "=== variant=$variant probe_mode=$probe_mode ===" + ./toggle.sh payments-api healthy + + log "deploying $image (minReadySeconds=$min_ready readinessInitialDelay=$readiness_delay)" + apply_apisix_deployment "$image" "$min_ready" "$readiness_delay" + + log "baseline check" + sleep 2 + curl -s -o /dev/null -w 'baseline: %{http_code} backend=%header{X-Mock-Backend}\n' \ + "http://127.0.0.1:30980/pay" + + log "toggling payments-api unhealthy, waiting for steady-state convergence to payproc (measured ~45s locally, single pod)" + ./toggle.sh payments-api unhealthy + for i in $(seq 1 120); do + sleep 1 + backend=$(curl -s -o /dev/null -w '%header{X-Mock-Backend}' "http://127.0.0.1:30980/pay") + [ $((i % 5)) -eq 0 ] && log " convergence check $i: backend=$backend" + [ "$backend" = "payproc" ] && { log " converged after ${i}s"; break; } + done + + local outfile="$RESULTS_DIR/${variant}-${probe_mode}.log" + log "triggering rollout restart, hammering for 90s -> $outfile" + kubectl rollout restart deployment/apisix-repro + hammer "$outfile" 90 & + local hammer_pid=$! + kubectl rollout status deployment/apisix-repro --timeout=120s || true + wait "$hammer_pid" + + local total leaked + total=$(wc -l < "$outfile") + leaked=$(grep -c 'payments-api' "$outfile" || true) + log "variant=$variant probe_mode=$probe_mode: total=$total leaked_to_payments_api=$leaked" + + ./toggle.sh payments-api healthy + kubectl delete deployment apisix-repro --ignore-not-found +} + +case "${1:-}" in + setup) cmd_setup ;; + teardown) cmd_teardown ;; + variant) shift; cmd_variant "$@" ;; + *) echo "usage: $0 {setup|variant [fast|mitigated]|teardown}" >&2; exit 1 ;; +esac diff --git a/hack/kind-repro/toggle.sh b/hack/kind-repro/toggle.sh new file mode 100755 index 000000000000..fadfaaec7258 --- /dev/null +++ b/hack/kind-repro/toggle.sh @@ -0,0 +1,25 @@ +#!/usr/bin/env bash +# Usage: ./toggle.sh +# +# Flips a sentinel file under state//unhealthy. This directory is +# bind-mounted into the kind node via kind-cluster.yaml's extraMounts, and +# from there into the mock backend pod via a hostPath volume -- same +# mechanism as the docker-compose POC at +# ~/Mollie/edge-app/docs/poc/0003-apisix-healthcheck-worker-repro/toggle.sh, +# just relayed through one extra hop (host -> kind node -> pod) since kind +# pods can't bind-mount the host directly. +set -euo pipefail + +backend="${1:?backend required: payments-api or payproc}" +state="${2:?state required: healthy or unhealthy}" + +dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/state/$backend" +mkdir -p "$dir" + +if [ "$state" = "unhealthy" ]; then + touch "$dir/unhealthy" +else + rm -f "$dir/unhealthy" +fi + +echo "$backend -> $state"