fix(api): bound all subprocess calls + run 2 workers to prevent API stall
HAProxy Manager Build and Push / Build-and-Push (push) Successful in 1m9s
HAProxy Manager Build and Push / Build-and-Push (push) Successful in 1m9s
The management API wedged on whp01 2026-07-07: every panel call to the manager (config regenerate during a WHP site update, SSL, even /health) timed out at 30s while customer sites stayed up. Root cause: all four gunicorn gthread worker threads were permanently blocked in socket reads inside untimed subprocess.run() calls (certbot ACME / socat reloads). A stalled external command holds its worker thread forever; gunicorn --timeout can't rescue it (gthread only kills a worker whose main thread stops heart-beating, and ours kept polling). Stalled calls accumulated until the 4-thread pool was exhausted and the whole API went dark. - Wrap subprocess.run with a default timeout (HAPROXY_MGR_SUBPROCESS_TIMEOUT, 180s) so every external command is bounded and releases its thread on expiry via the existing per-endpoint try/except. Bounding by default covers all ~30 call sites and any future one. - certbot renew keeps an explicit 900s timeout (walks every lineage). - API_WORKERS default 1 -> 2: a single worker made a thread-pool wedge a total outage; a second worker keeps the API answering while one recycles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+36
-2
@@ -18,6 +18,38 @@ import time
|
|||||||
import re
|
import re
|
||||||
import fcntl
|
import fcntl
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Bounded subprocess execution (incident 2026-07-07)
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Every external command this manager runs — certbot ACME issuance/renewal,
|
||||||
|
# `socat` reloads over the haproxy admin socket, `haproxy -c` validation — is a
|
||||||
|
# potential hang. The management API runs under gunicorn gthread workers, and a
|
||||||
|
# subprocess.run() with NO timeout blocks its worker thread forever if the
|
||||||
|
# command stalls (e.g. an ACME/upstream that stops responding mid-read).
|
||||||
|
# gunicorn's --timeout does not rescue this: for gthread it only kills a worker
|
||||||
|
# whose *main* thread stops heart-beating, but the main thread keeps polling
|
||||||
|
# while pool threads are wedged. Enough stalled calls exhaust the 4-thread pool
|
||||||
|
# and the whole API stops responding — "healthy" health-check, every request
|
||||||
|
# 30s-timeouts — which is exactly what stalled WHP site updates on 2026-07-07.
|
||||||
|
#
|
||||||
|
# Fix: give EVERY subprocess.run() a default timeout unless the caller passes
|
||||||
|
# one explicitly. On expiry Python kills the child and raises
|
||||||
|
# subprocess.TimeoutExpired (a subclass of Exception); the existing per-endpoint
|
||||||
|
# try/except turns that into a clean error AND releases the worker thread.
|
||||||
|
# Bounding by default (instead of editing ~30 call sites) means no site can be
|
||||||
|
# missed and any future call is protected automatically.
|
||||||
|
DEFAULT_SUBPROCESS_TIMEOUT = int(os.environ.get('HAPROXY_MGR_SUBPROCESS_TIMEOUT', '180'))
|
||||||
|
_unbounded_subprocess_run = subprocess.run
|
||||||
|
|
||||||
|
|
||||||
|
def _bounded_subprocess_run(*args, **kwargs):
|
||||||
|
if kwargs.get('timeout') is None:
|
||||||
|
kwargs['timeout'] = DEFAULT_SUBPROCESS_TIMEOUT
|
||||||
|
return _unbounded_subprocess_run(*args, **kwargs)
|
||||||
|
|
||||||
|
|
||||||
|
subprocess.run = _bounded_subprocess_run
|
||||||
|
|
||||||
app = Flask(__name__)
|
app = Flask(__name__)
|
||||||
|
|
||||||
# Default page server (port 8080) — served to HAProxy clients whose request hit
|
# Default page server (port 8080) — served to HAProxy clients whose request hit
|
||||||
@@ -811,10 +843,12 @@ def renew_certificates():
|
|||||||
# Defensive: clear any stale lock left by a SIGKILLed prior run.
|
# Defensive: clear any stale lock left by a SIGKILLed prior run.
|
||||||
clear_stale_certbot_locks()
|
clear_stale_certbot_locks()
|
||||||
|
|
||||||
# Run certbot renew
|
# Run certbot renew. Explicit long timeout (overrides the module
|
||||||
|
# default): `renew` walks every lineage and can legitimately make many
|
||||||
|
# ACME round-trips when several certs are actually due.
|
||||||
result = subprocess.run([
|
result = subprocess.run([
|
||||||
'certbot', 'renew', '--quiet'
|
'certbot', 'renew', '--quiet'
|
||||||
], capture_output=True, text=True)
|
], capture_output=True, text=True, timeout=900)
|
||||||
|
|
||||||
if result.returncode == 0:
|
if result.returncode == 0:
|
||||||
# Check if any certificates were renewed
|
# Check if any certificates were renewed
|
||||||
|
|||||||
+7
-1
@@ -47,7 +47,13 @@ HAPROXY_SUPERVISOR_INTERVAL="${HAPROXY_SUPERVISOR_INTERVAL:-15}"
|
|||||||
# Tunable via env: HAPROXY_MGR_API_WORKERS (default 1), HAPROXY_MGR_API_TIMEOUT
|
# Tunable via env: HAPROXY_MGR_API_WORKERS (default 1), HAPROXY_MGR_API_TIMEOUT
|
||||||
# (default 120 — API can do slow ACME calls), HAPROXY_MGR_MAX_REQUESTS (default
|
# (default 120 — API can do slow ACME calls), HAPROXY_MGR_MAX_REQUESTS (default
|
||||||
# 1000 — worker recycle frequency).
|
# 1000 — worker recycle frequency).
|
||||||
API_WORKERS="${HAPROXY_MGR_API_WORKERS:-1}"
|
#
|
||||||
|
# API_WORKERS default is 2 (was 1). A single worker is a single point of
|
||||||
|
# failure: if its gthread pool ever wedges (see the 2026-07-07 subprocess-hang
|
||||||
|
# incident — now bounded by DEFAULT_SUBPROCESS_TIMEOUT in haproxy_manager.py),
|
||||||
|
# the entire management API goes dark. A second worker keeps the API answering
|
||||||
|
# (config regenerate, health, SSL) while the other recycles via --max-requests.
|
||||||
|
API_WORKERS="${HAPROXY_MGR_API_WORKERS:-2}"
|
||||||
API_TIMEOUT="${HAPROXY_MGR_API_TIMEOUT:-120}"
|
API_TIMEOUT="${HAPROXY_MGR_API_TIMEOUT:-120}"
|
||||||
MAX_REQ="${HAPROXY_MGR_MAX_REQUESTS:-1000}"
|
MAX_REQ="${HAPROXY_MGR_MAX_REQUESTS:-1000}"
|
||||||
MAX_REQ_JITTER="${HAPROXY_MGR_MAX_REQUESTS_JITTER:-100}"
|
MAX_REQ_JITTER="${HAPROXY_MGR_MAX_REQUESTS_JITTER:-100}"
|
||||||
|
|||||||
Reference in New Issue
Block a user