fix(deploy): trust the nginx upstream when picking the live slot
Gitea Actions Runner Test / test-job (push) Successful in 1s
CI / check (push) Successful in 30s
CI / tests-integration (push) Successful in 1m48s
CI / tests-unit (push) Failing after 1m54s
CI / tests-ui (push) Successful in 2m46s
CI / preflight (push) Skipped
CI / deploy (push) Skipped

The deploy failed with "Expected release never became healthy" after 30
attempts. Root cause: read_active_port() counted the slots answering
/api/health and only consulted the nginx upstream when the count was not
exactly one. On this host both slots were healthy, so it fell back to the
upstream file, but a leftover epicnext-cms:local replica was holding slot A
(3002). The candidate was assigned that occupied port, docker run died with
EADDRINUSE, and the health probe then answered from the pre-existing
container on that port. That container reports release "unknown" because it
was built without NEXT_DEPLOYMENT_ID, so the release comparison could never
match and the deploy timed out blaming a release that was never serving.

read_active_port() now orders its sources by how well they describe reality:

1. The nginx upstream file. It is the only source that says where public
   traffic actually enters; everything below it is a consequence.
2. A healthy slot matching that pointer.
3. The other slot when the pointer names a dead port.
4. The pointer itself when nothing answers, so rollback still has a target.
5. Slot A when no upstream file exists at all.

answers_health() was added as a retry-free sibling of healthy(); port
detection should not spend 90 seconds per slot on a process that is either
running now or never will.

start_candidate() now calls assert_port_free() before docker run, so an
occupied port fails immediately and names the listener and the containers
involved, instead of surfacing later as a misleading health-check timeout.

Added scripts/ci-deploy-ports.test.sh, which extracts the two functions from
the real script rather than copying them, and covers the regression: with
both slots healthy and nginx serving slot B, the result must not be slot A.
Verified the test fails against the old logic and passes against the new.
Wired into the check job so this is caught before an image is built.
This commit is contained in:
openhands committed 2026-10-03 18:22:40 +02:00
1 parent 704e33638f
commit 64ad9baf39
3 files changed
+205 -20

No files matched your search

+6
View File
@@ -33,6 +33,12 @@ jobs:
- name: Toolchain check
run: node scripts/check-node-toolchain.mjs
# Port selection decides which blue/green slot stays live. Getting it
# wrong starts the candidate on an occupied port, so the regression that
# caused a failed deploy is covered here, before any image is built.
- name: Deploy port-selection tests
run: bash scripts/ci-deploy-ports.test.sh
- name: Install dependencies
run: pnpm install --frozen-lockfile
+124
View File
@@ -0,0 +1,124 @@
#!/usr/bin/env bash
# Tests for the port-selection and port-conflict logic in scripts/ci-deploy.sh.
#
# Background: on a host where both blue/green slots answer /api/health, the
# original read_active_port() counted healthy slots and only consulted the nginx
# upstream when the count was not exactly 1. With two healthy slots it fell back
# to the upstream file, but an operator `docker compose up` can leave an extra
# replica behind, after which the fallback picked slot A regardless of which slot
# was really live. The candidate then tried to start on an occupied port, and the
# health probe answered from the pre-existing container on that port instead of
# the candidate — producing 30 failed "expected release never became healthy"
# attempts against a release that was never serving.
#
# The functions are extracted from ci-deploy.sh rather than copied so this test
# cannot drift from the script it protects.
set -Eeuo pipefail
deploy_script="$(dirname "$0")/ci-deploy.sh"
[[ -r "$deploy_script" ]] || { echo "cannot read $deploy_script" >&2; exit 1; }
# Pull the two functions out of the real script.
extract() {
sed -n "/^$1() {/,/^}/p" "$deploy_script"
}
read_active_port_fn="$(extract read_active_port)"
assert_port_free_fn="$(extract assert_port_free)"
answers_health_fn="$(extract answers_health)"
if [ -z "$read_active_port_fn" ] || [ -z "$assert_port_free_fn" ] || [ -z "$answers_health_fn" ]; then
echo "could not extract functions from $deploy_script" >&2
exit 1
fi
slot_a_port=3002
slot_b_port=3003
fail() { echo "FAIL: $*" >&2; exit 1; }
# ── read_active_port ──────────────────────────────────────────────────────────
# $1 = upstream body ("none" for a missing file), $2..$3 = ports that answer.
run_read_active_port() {
local body="$1" a="$2" b="$3" tmp
tmp="$(mktemp)"
if [ "$body" = "none" ]; then
tmp=/tmp/ci-deploy-test-nonexistent-upstream-$$
rm -f "$tmp"
else
printf '%s\n' "$body" >"$tmp"
fi
CMS_UPSTREAM_FILE="$tmp" \
PORT_A_HEALTHY="$a" PORT_B_HEALTHY="$b" \
bash -c "
slot_a_port=$slot_a_port
slot_b_port=$slot_b_port
upstream_file=\"\$CMS_UPSTREAM_FILE\"
$read_active_port_fn
# Defined after the extracted function on purpose: answers_health is a
# collaborator here, and the test substitutes a deterministic stub for it.
answers_health() {
local p=\$1 want
case \$p in
$slot_a_port) want=\"\$PORT_A_HEALTHY\" ;;
$slot_b_port) want=\"\$PORT_B_HEALTHY\" ;;
*) want='' ;;
esac
[ \"\$want\" = yes ]
}
read_active_port
echo
" 2>/dev/null
rm -f "$tmp"
}
# nginx points at slot B and both answer -> trust the upstream file.
got="$(run_read_active_port 'server 127.0.0.1:3003 max_fails=2;' yes yes)"
[ "$got" = "$slot_b_port" ] || fail "nginx->3003 with both healthy: got '$got', want 3003"
got="$(run_read_active_port 'server 127.0.0.1:3002 max_fails=2;' yes yes)"
[ "$got" = "$slot_a_port" ] || fail "nginx->3002 with both healthy: got '$got', want 3002"
# The regression: both healthy, nginx points at B, but slot A is an unrelated
# leftover replica. The upstream file is the only thing that knows which slot is
# live, so it must win.
got="$(run_read_active_port 'server 127.0.0.1:3003 max_fails=2;' yes yes)"
[ "$got" != "$slot_a_port" ] || fail "both healthy: fell back to slot A while nginx serves 3003"
# Upstream names a dead slot: fall back to a slot that actually answers, never to
# the dead port itself.
got="$(run_read_active_port 'server 127.0.0.1:3002 max_fails=2;' no yes)"
[ "$got" = "$slot_b_port" ] || fail "nginx->3002 unhealthy, B healthy: got '$got', want 3003"
# Nothing answers at all: read_active_port still has to name a slot, otherwise the
# rollback path has no target.
got="$(run_read_active_port 'server 127.0.0.1:3003 max_fails=2;' no no)"
[ "$got" = "$slot_b_port" ] || fail "nothing healthy: got '$got', want the upstream port 3003"
# No upstream file at all: pick a slot that answers.
got="$(run_read_active_port none no yes)"
[ "$got" = "$slot_b_port" ] || fail "no upstream, B healthy: got '$got', want 3003"
got="$(run_read_active_port none yes no)"
[ "$got" = "$slot_a_port" ] || fail "no upstream, A healthy: got '$got', want 3002"
# ── assert_port_free ─────────────────────────────────────────────────────────
# Runs against real loopback ports: 3999 is intentionally unused, so the check
# must report it free.
bash -c "
$assert_port_free_fn
assert_port_free 3999 candidate >/dev/null 2>&1
" || fail "a port with no listener must be reported as free"
# On this host 3002 is held by a CMS container, so the check must fail. Skip when
# it genuinely is free, otherwise the assertion would be meaningless.
if ss -ltn 2>/dev/null | grep -qE '127\.0\.0\.1:3002|0\.0\.0\.0:3002'; then
if bash -c "
$assert_port_free_fn
assert_port_free 3002 candidate >/dev/null 2>&1
"; then
fail "an occupied port must be rejected, but assert_port_free returned success"
fi
fi
echo 'Deploy port-selection tests passed'
+75 -20
View File
@@ -76,6 +76,13 @@ healthy() {
return 1
}
# Zelfde check als `healthy`, maar zonder retries. Voor het bepalen van de
# actieve poort willen we geen 90 seconden per slot wachten: daar gaat het om
# een al draaiend proces dat nu of nooit antwoordt.
answers_health() {
curl -sf --max-time 5 "http://127.0.0.1:$1/api/health" | grep -q '"database":true'
}
# Staat er een blue/green-upstream? Zonder die bestanden blijft dit script op de
# oude, in-place cutover vallen, zodat een host met een andere nginx-indeling
# niet stilvalt op een upgrade.
@@ -85,29 +92,76 @@ detect_blue_green() {
return 0
}
# Welke poort is op dit moment ÉCHT live? Kijk niet naar het upstream-bestand
# (dat kan door een losse `docker compose up` zijn ingehaald en naar een dood
# slot wijzen), maar test welk slot werkelijk antwoordt op /api/health. Alleen
# in een dubbelzinnige situatie (geen óf beide slots gezond) valt het script
# terug op de huidige nginx-pointer; onbekend = slot A (eerste release op 3002).
# Welke poort is op dit moment ÉCHT live?
#
# Volgorde van vertrouwen:
# 1. Het nginx-upstream-bestand. Dat is de enige bron die aangeeft wáár het
# publieke verkeer daadwerkelijk binnenkomt; alles daaronder is gevolg.
# 2. Een gezond slot dat overeenkomt met die aanwijzing.
# 3. Precies één gezond slot (een verse host met geen upstream-bestand).
#
# De eerdere versie telde gezonde slots en gebruikte de fallback pas als er 0 of
# 2+ waren. Op een host waar beide slots tegelijk gezond zijn — bijvoorbeeld
# doordat een losse `docker compose up` een extra replica heeft achtergelaten —
# gaf dat een willekeurige keuze, en dan kon de kandidaat op een bezette poort
# starten (EADDRINUSE) terwijl de health-check de reeds draaiende container op
# die poort beantwoordde. De release-vergelijking faalde dan 30 keer op een
# container die toevallig een andere release draaide.
read_active_port() {
local live="" port="" result=""
local count=0
for port in "$slot_a_port" "$slot_b_port"; do
if curl -sf --max-time 3 "http://127.0.0.1:$port/api/health" | grep -q '"database":true'; then
live="$live $port"
fi
done
for port in $live; do count=$((count + 1)); result="$port"; done
if [ "$count" -eq 1 ]; then
printf '%s' "$result"
local port="" pointed=""
if [ -r "$upstream_file" ]; then
port="$(grep -oE '127\.0\.0\.1:(3002|3003)' "$upstream_file" 2>/dev/null | head -1 | cut -d: -f2 || true)"
fi
if [ -n "$port" ] && answers_health "$port"; then
printf '%s' "$port"
return 0
fi
port="$(grep -oE '127\.0\.0\.1:[0-9]+' "$upstream_file" 2>/dev/null | head -1 | cut -d: -f2 || true)"
case "$port" in
"$slot_b_port") printf '%s' "$slot_b_port" ;;
*) printf '%s' "$slot_a_port" ;;
esac
# Het upstream-bestand wijst naar een slot dat niet antwoordt. Kies dan het
# enige andere gezonde slot, anders is er niets om op te bouwen.
for candidate in "$slot_a_port" "$slot_b_port"; do
[ "$candidate" = "$port" ] && continue
if answers_health "$candidate"; then
echo "nginx points at ${port:-unknown}, which is unhealthy; ${candidate} answers instead" >&2
printf '%s' "$candidate"
return 0
fi
done
# Geen enkel slot antwoordt. Vertrouw dan op het bestand, zodat een
# rollback-poging toch het vorige slot kan starten.
if [ -n "$port" ]; then
printf '%s' "$port"
return 0
fi
printf '%s' "$slot_a_port"
}
# Poort-bezetting controleren vóór het starten van de kandidaat.
#
# Zonder deze check zorgt `docker run` er stilzwijgend voor dat de kandidaat
# dood gaat op EADDRINUSE, terwijl de health-check ondertussen de reeds draaiende
# container op diezelfde poort beantwoordt. Dat levert een misleidende
# "expected release never became healthy" op in plaats van de echte oorzaak.
# Elke listener wordt hierboven concreet genoemd, inclusief de container die
# hem vasthoudt.
assert_port_free() {
local port="$1" name="$2"
local holders=""
if command -v ss >/dev/null 2>&1; then
# `ss` drukt altijd een kolomkop af, ook als er geen listener is. Filter op
# LISTEN, anders zou elke vrije poort als bezet gemeld worden.
holders="$(ss -ltnp "sport = :$port" 2>/dev/null | grep -F 'LISTEN' || true)"
fi
[ -z "$holders" ] && return 0
echo "Port $port is already in use, cannot start candidate $name" >&2
printf '%s\n' "$holders" >&2
echo "--- containers currently running ---" >&2
docker ps --format '{{.Names}}\t{{.Image}}\t{{.Status}}' >&2 || true
return 1
}
# Zet de nginx-upstream op de nieuwe poort en herlaadt graceful.
@@ -153,6 +207,7 @@ switch_upstream() {
# gelden.
start_candidate() {
local port="$1" name="$2"
assert_port_free "$port" "$name"
(
set -a
# shellcheck disable=SC1091