diff --git a/.github/scripts/check-pg-healthchecks.sh b/.github/scripts/check-pg-healthchecks.sh new file mode 100755 index 00000000..8ec08ebc --- /dev/null +++ b/.github/scripts/check-pg-healthchecks.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# Every pg_isready must name a host, so the healthcheck probes TCP. +# +# Without -h, pg_isready uses the Unix socket, and the official postgres image +# runs initdb against a temporary server started with `listen_addresses=''` -- +# socket up, TCP refused. The healthcheck therefore passes *during* init, and a +# dependent with `condition: service_healthy` starts into a window where the +# port it actually connects to does not exist yet. +# +# This is not theoretical. From e2e run 35247949580, plc-postgres's own log +# against plc's crash: +# +# 16:55:27.593 temp server: listening on Unix socket ONLY +# 16:55:27.633 temp server: ready to accept connections <- healthcheck goes green +# 16:55:29.441 temp server: shut down +# 16:55:29.887 real server: listening on IPv4 0.0.0.0:5432 +# 16:55:30.531 plc exits: ECONNREFUSED 172.18.0.6:5432 +# +# The healthcheck was green 2.25 seconds before TCP existed. That is why +# TestUploadAndRetrieve/filesystem failed 2 of 40 e2e runs naming a *different* +# container each time -- whichever dependent lost the race that run. +# +# The rule has no list in it. Every pg_isready is either given a host or it is +# a bug, so nothing here needs updating when a service is added. It covers Go +# as well as YAML because smelt/pkg/generate builds one of these strings. +# +# It matches an *invocation*, not a mention: the name must be preceded by a +# quote, so it is the start of a quoted command string. The first version of +# this check did not, and its own step name in ci.yml -- "every pg_isready +# healthcheck probes TCP" -- failed it. A guard that cannot tell running a +# thing from naming it is not guarding the thing. +# +# The gap that leaves: an unquoted invocation, e.g. `--health-cmd pg_isready` +# in an Actions services: block. Nothing in this repository writes one, and +# the alternative -- matching every mention -- is what just misfired. +set -euo pipefail + +cd "$(dirname "$0")/../.." + +status=0 +checked=0 +while IFS= read -r hit; do + file="${hit%%:*}" + rest="${hit#*:}" + line="${rest%%:*}" + checked=$((checked + 1)) + case "$rest" in + *pg_isready*-h[[:space:]]*|*pg_isready*--host*) echo "ok $file:$line" ;; + *) echo "FAIL $file:$line pg_isready with no -h probes the Unix socket"; status=1 ;; + esac +done < <(grep -rn '["'"'"']pg_isready' --include='*.yml' --include='*.yaml' --include='*.go' . | grep -v '^\./\.git/') + +if [ "$checked" -eq 0 ]; then + echo "No pg_isready healthchecks found -- this check has nothing to guard." + echo "That is suspicious rather than fine: it used to find several." + exit 1 +fi + +if [ "$status" -eq 0 ]; then + echo "All $checked pg_isready probes name a host." +else + echo + echo "Add -h 127.0.0.1. Without it the probe uses the Unix socket, which is" + echo "up during initdb while TCP is not, so dependents start too early." +fi +exit "$status" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 10d2bff4..b1b9c573 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,6 +56,8 @@ jobs: run: .github/scripts/check-base-images.sh - name: every pulled compose image is pinned by digest run: .github/scripts/check-stack-images.sh + - name: every pg_isready healthcheck probes TCP + run: .github/scripts/check-pg-healthchecks.sh - name: gofmt run: | unformatted=$(gofmt -s -l .) diff --git a/smelt/pkg/generate/compose.go b/smelt/pkg/generate/compose.go index ceed4140..e34771fa 100644 --- a/smelt/pkg/generate/compose.go +++ b/smelt/pkg/generate/compose.go @@ -150,7 +150,7 @@ func buildPostgresService() ComposeService { "piri-postgres-data:/var/lib/postgresql/data", }, Healthcheck: &Healthcheck{ - Test: []string{"CMD-SHELL", "pg_isready -U piri -d postgres"}, + Test: []string{"CMD-SHELL", "pg_isready -U piri -d postgres -h 127.0.0.1"}, StartInterval: "1s", Interval: "5s", Timeout: "3s", diff --git a/smelt/systems/hilt/compose.yml b/smelt/systems/hilt/compose.yml index cfdadecf..9ce179e8 100644 --- a/smelt/systems/hilt/compose.yml +++ b/smelt/systems/hilt/compose.yml @@ -136,7 +136,7 @@ services: volumes: - hilt-postgres-data:/var/lib/postgresql/data healthcheck: - test: ["CMD-SHELL", "pg_isready -U hilt -d hilt"] + test: ["CMD-SHELL", "pg_isready -U hilt -d hilt -h 127.0.0.1"] start_interval: 1s interval: 5s timeout: 3s diff --git a/smelt/systems/ingot/compose.yml b/smelt/systems/ingot/compose.yml index ebbad34a..c8f476ee 100644 --- a/smelt/systems/ingot/compose.yml +++ b/smelt/systems/ingot/compose.yml @@ -135,7 +135,7 @@ services: volumes: - ingot-postgres-data:/var/lib/postgresql/data healthcheck: - test: ["CMD-SHELL", "pg_isready -U ingot -d ingot"] + test: ["CMD-SHELL", "pg_isready -U ingot -d ingot -h 127.0.0.1"] start_interval: 1s interval: 5s timeout: 3s diff --git a/smelt/systems/plc/compose.yml b/smelt/systems/plc/compose.yml index e91ab0b4..b6dea7b5 100644 --- a/smelt/systems/plc/compose.yml +++ b/smelt/systems/plc/compose.yml @@ -48,7 +48,7 @@ services: volumes: - plc-postgres-data:/var/lib/postgresql/data healthcheck: - test: ["CMD-SHELL", "pg_isready -U plc -d plc"] + test: ["CMD-SHELL", "pg_isready -U plc -d plc -h 127.0.0.1"] start_interval: 1s interval: 5s timeout: 3s diff --git a/smelt/systems/swarf/compose.yml b/smelt/systems/swarf/compose.yml index 1a1e99a1..80ffe422 100644 --- a/smelt/systems/swarf/compose.yml +++ b/smelt/systems/swarf/compose.yml @@ -57,7 +57,7 @@ services: volumes: - swarf-postgres-data:/var/lib/postgresql/data healthcheck: - test: ["CMD-SHELL", "pg_isready -U swarf -d swarf"] + test: ["CMD-SHELL", "pg_isready -U swarf -d swarf -h 127.0.0.1"] start_interval: 1s interval: 5s timeout: 3s diff --git a/smelt/systems/upload/compose.yml b/smelt/systems/upload/compose.yml index 2df53600..2707d1f7 100644 --- a/smelt/systems/upload/compose.yml +++ b/smelt/systems/upload/compose.yml @@ -81,7 +81,7 @@ services: volumes: - upload-postgres-data:/var/lib/postgresql/data healthcheck: - test: ["CMD-SHELL", "pg_isready -U sprue -d sprue"] + test: ["CMD-SHELL", "pg_isready -U sprue -d sprue -h 127.0.0.1"] start_interval: 1s interval: 5s timeout: 3s