diff --git a/.gitea/workflows/ci.yaml b/.gitea/workflows/ci.yaml index 68b88841..bb6a19f2 100644 --- a/.gitea/workflows/ci.yaml +++ b/.gitea/workflows/ci.yaml @@ -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 diff --git a/scripts/ci-deploy-ports.test.sh b/scripts/ci-deploy-ports.test.sh new file mode 100755 index 00000000..5afd7ad5 --- /dev/null +++ b/scripts/ci-deploy-ports.test.sh @@ -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' diff --git a/scripts/ci-deploy.sh b/scripts/ci-deploy.sh index d2d20956..6eccb73d 100644 --- a/scripts/ci-deploy.sh +++ b/scripts/ci-deploy.sh @@ -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