fix(postgres): make BasicWaitStrategies safe with container reuse

Dockerfile

@@ -0,0 +1,21 @@
+FROM public.ecr.aws/d3j8x8q7/olympus-base-go:latest
+
+WORKDIR /app
+
+# Copy dependency manifests first to cache the download layer separately from source.
+COPY go.mod go.sum ./
+COPY modules/postgres/go.mod modules/postgres/go.sum modules/postgres/
+
+# Download all dependencies while network is available.
+# modules/postgres/go.mod has a replace directive pointing to the root (../../..),
+# so root dependencies must be downloaded first.
+RUN go mod download && \
+    cd modules/postgres && go mod download
+
+# Copy full source and pre-compile to populate the build cache.
+COPY . .
+
+RUN go build ./... && \
+    cd modules/postgres && go build ./...
+
+CMD ["/bin/bash"]

modules/postgres/wait_strategies.go

@@ -6,9 +6,15 @@ import (
 )
 
 // BasicWaitStrategies is a simple but reliable way to wait for postgres to start.
-// It returns a two-step wait strategy:
+// It returns a three-step wait strategy:
 //
 //   - It will wait for the container to log `database system is ready to accept connections` twice, because it will restart itself after the first startup.
+//   - It will then probe the live server state with `pg_isready` until postgres accepts connections.
+//     Container logs survive restarts, so on a reused container the log message of a previous run
+//     could otherwise report readiness before the current process accepts connections
+//     (https://github.com/testcontainers/testcontainers-go/issues/3671). The probe prefers the
+//     unix socket, falls back to TCP on loopback honoring PGPORT, needs no valid credentials,
+//     and is skipped on images that do not ship pg_isready, preserving the previous behavior.
 //   - It will then wait for docker to actually serve the port on localhost.
 //     For non-linux OSes like Mac and Windows, Docker or Rancher Desktop will have to start a separate proxy.
 //     Without this, the tests will be flaky on those OSes!
@@ -18,6 +24,10 @@ func BasicWaitStrategies() testcontainers.CustomizeRequestOption {
 		// First, we wait for the container to log readiness twice.
 		// This is because it will restart itself after the first startup.
 		wait.ForLog("database system is ready to accept connections").WithOccurrence(2),
+		// Then, we probe the live server state until it accepts connections, because
+		// the log message of a previous run of a reused container cannot prove that
+		// the current process is ready. Skipped on images without pg_isready.
+		wait.ForExec([]string{"sh", "-c", `command -v pg_isready >/dev/null 2>&1 || exit 0; pg_isready || pg_isready -h 127.0.0.1 -p "${PGPORT:-5432}"`}),
 		// Then, we wait for docker to actually serve the port on localhost.
 		// For non-linux OSes like Mac and Windows, Docker or Rancher Desktop will have to start a separate proxy.
 		// Without this, the tests will be flaky on those OSes!

modules/postgres/wait_strategies_test.go

@@ -0,0 +1,107 @@
+package postgres_test
+
+import (
+	"context"
+	"database/sql"
+	"fmt"
+	"io"
+	"strings"
+	"testing"
+	"time"
+
+	_ "github.com/jackc/pgx/v5/stdlib"
+	"github.com/stretchr/testify/require"
+
+	"github.com/testcontainers/testcontainers-go"
+	"github.com/testcontainers/testcontainers-go/modules/postgres"
+)
+
+// TestBasicWaitStrategies_reusedContainer reproduces the false-positive ready
+// signal reported in https://github.com/testcontainers/testcontainers-go/issues/3671:
+// container logs survive restarts, so on a reused container a log-based wait
+// strategy can be satisfied by the output of a previous run and unblock before
+// the current postgres process accepts connections.
+func TestBasicWaitStrategies_reusedContainer(t *testing.T) {
+	ctx := context.Background()
+
+	reuseName := fmt.Sprintf("postgres-reused-wait-%d", time.Now().UnixNano())
+
+	run := func() *postgres.PostgresContainer {
+		t.Helper()
+		ctr, err := postgres.Run(ctx, "postgres:16-alpine",
+			postgres.WithDatabase(dbname),
+			postgres.WithUsername(user),
+			postgres.WithPassword(password),
+			postgres.BasicWaitStrategies(),
+			testcontainers.WithReuseByName(reuseName),
+		)
+		testcontainers.CleanupContainer(t, ctr)
+		require.NoError(t, err)
+		return ctr
+	}
+
+	connect := func(c *postgres.PostgresContainer) *sql.DB {
+		t.Helper()
+		connStr, err := c.ConnectionString(ctx, "sslmode=disable")
+		require.NoError(t, err)
+		db, err := sql.Open("pgx", connStr)
+		require.NoError(t, err)
+		t.Cleanup(func() { db.Close() })
+		return db
+	}
+
+	// recoveries counts how many crash recoveries the container has logged.
+	// Logs accumulate across restarts of the same container, which is the very
+	// property that makes log-based waits unsafe with reuse.
+	recoveries := func(c *postgres.PostgresContainer) int {
+		t.Helper()
+		rc, err := c.Logs(ctx)
+		require.NoError(t, err)
+		defer rc.Close()
+		logs, err := io.ReadAll(rc)
+		require.NoError(t, err)
+		return strings.Count(string(logs), "database system was not properly shut down")
+	}
+
+	const rowCount = 3_000_000
+
+	ctr := run()
+
+	db := connect(ctr)
+	_, err := db.ExecContext(ctx, "CREATE TABLE reuse_wait (v int)")
+	require.NoError(t, err)
+
+	for i := 0; i < 2; i++ {
+		// Generate WAL so that the unclean restart below forces crash recovery,
+		// during which postgres does not accept connections yet.
+		_, err = db.ExecContext(ctx, "TRUNCATE reuse_wait")
+		require.NoError(t, err)
+		_, err = db.ExecContext(ctx, fmt.Sprintf("INSERT INTO reuse_wait SELECT generate_series(1, %d)", rowCount))
+		require.NoError(t, err)
+
+		// Stop with a zero timeout follows SIGTERM with an immediate SIGKILL.
+		// The session above is kept open on purpose: postgres waits for it on
+		// SIGTERM, so the SIGKILL always interrupts an unclean shutdown and the
+		// next start is guaranteed to run crash recovery.
+		noGrace := time.Duration(0)
+		require.NoError(t, ctr.Stop(ctx, &noGrace))
+
+		ctr = run()
+
+		// The wait strategy must not unblock before postgres accepts connections:
+		// a single attempt with no retries must succeed.
+		db = connect(ctr)
+		var rows int
+		require.NoErrorf(t,
+			db.QueryRowContext(ctx, "SELECT count(*) FROM reuse_wait").Scan(&rows),
+			"reuse cycle %d: container reported ready before postgres accepted connections", i,
+		)
+		require.Equal(t, rowCount, rows)
+
+		// Guard the reproducer itself: every cycle must have gone through crash
+		// recovery, otherwise the scenario above degraded silently.
+		require.Equalf(t, i+1, recoveries(ctr),
+			"reuse cycle %d: expected the restart to run crash recovery", i,
+		)
+	}
+}

test.sh

@@ -0,0 +1,172 @@
+diff --git a/modules/postgres/wait_strategies_test.go b/modules/postgres/wait_strategies_test.go
+new file mode 100644
+index 0000000..b6e9ff2
+--- /dev/null
++++ b/modules/postgres/wait_strategies_test.go
+@@ -0,0 +1,107 @@
++package postgres_test
++
++import (
++	"context"
++	"database/sql"
++	"fmt"
++	"io"
++	"strings"
++	"testing"
++	"time"
++
++	_ "github.com/jackc/pgx/v5/stdlib"
++	"github.com/stretchr/testify/require"
++
++	"github.com/testcontainers/testcontainers-go"
++	"github.com/testcontainers/testcontainers-go/modules/postgres"
++)
++
++// TestBasicWaitStrategies_reusedContainer reproduces the false-positive ready
++// signal reported in https://github.com/testcontainers/testcontainers-go/issues/3671:
++// container logs survive restarts, so on a reused container a log-based wait
++// strategy can be satisfied by the output of a previous run and unblock before
++// the current postgres process accepts connections.
++func TestBasicWaitStrategies_reusedContainer(t *testing.T) {
++	ctx := context.Background()
++
++	reuseName := fmt.Sprintf("postgres-reused-wait-%d", time.Now().UnixNano())
++
++	run := func() *postgres.PostgresContainer {
++		t.Helper()
++		ctr, err := postgres.Run(ctx, "postgres:16-alpine",
++			postgres.WithDatabase(dbname),
++			postgres.WithUsername(user),
++			postgres.WithPassword(password),
++			postgres.BasicWaitStrategies(),
++			testcontainers.WithReuseByName(reuseName),
++		)
++		testcontainers.CleanupContainer(t, ctr)
++		require.NoError(t, err)
++		return ctr
++	}
++
++	connect := func(c *postgres.PostgresContainer) *sql.DB {
++		t.Helper()
++		connStr, err := c.ConnectionString(ctx, "sslmode=disable")
++		require.NoError(t, err)
++		db, err := sql.Open("pgx", connStr)
++		require.NoError(t, err)
++		t.Cleanup(func() { db.Close() })
++		return db
++	}
++
++	// recoveries counts how many crash recoveries the container has logged.
++	// Logs accumulate across restarts of the same container, which is the very
++	// property that makes log-based waits unsafe with reuse.
++	recoveries := func(c *postgres.PostgresContainer) int {
++		t.Helper()
++		rc, err := c.Logs(ctx)
++		require.NoError(t, err)
++		defer rc.Close()
++		logs, err := io.ReadAll(rc)
++		require.NoError(t, err)
++		return strings.Count(string(logs), "database system was not properly shut down")
++	}
++
++	const rowCount = 3_000_000
++
++	ctr := run()
++
++	db := connect(ctr)
++	_, err := db.ExecContext(ctx, "CREATE TABLE reuse_wait (v int)")
++	require.NoError(t, err)
++
++	for i := 0; i < 2; i++ {
++		// Generate WAL so that the unclean restart below forces crash recovery,
++		// during which postgres does not accept connections yet.
++		_, err = db.ExecContext(ctx, "TRUNCATE reuse_wait")
++		require.NoError(t, err)
++		_, err = db.ExecContext(ctx, fmt.Sprintf("INSERT INTO reuse_wait SELECT generate_series(1, %d)", rowCount))
++		require.NoError(t, err)
++
++		// Stop with a zero timeout follows SIGTERM with an immediate SIGKILL.
++		// The session above is kept open on purpose: postgres waits for it on
++		// SIGTERM, so the SIGKILL always interrupts an unclean shutdown and the
++		// next start is guaranteed to run crash recovery.
++		noGrace := time.Duration(0)
++		require.NoError(t, ctr.Stop(ctx, &noGrace))
++
++		ctr = run()
++
++		// The wait strategy must not unblock before postgres accepts connections:
++		// a single attempt with no retries must succeed.
++		db = connect(ctr)
++		var rows int
++		require.NoErrorf(t,
++			db.QueryRowContext(ctx, "SELECT count(*) FROM reuse_wait").Scan(&rows),
++			"reuse cycle %d: container reported ready before postgres accepted connections", i,
++		)
++		require.Equal(t, rowCount, rows)
++
++		// Guard the reproducer itself: every cycle must have gone through crash
++		// recovery, otherwise the scenario above degraded silently.
++		require.Equalf(t, i+1, recoveries(ctr),
++			"reuse cycle %d: expected the restart to run crash recovery", i,
++		)
++	}
++}
+diff --git a/test.sh b/test.sh
+new file mode 100755
+index 0000000..2fb53a5
+--- /dev/null
++++ b/test.sh
+@@ -0,0 +1,53 @@
++#!/usr/bin/env bash
++set -uo pipefail
++
++OUTPUT_PATH=""
++MODE=""
++
++while [[ $# -gt 0 ]]; do
++    case "$1" in
++        --output_path)
++            OUTPUT_PATH="$2"
++            shift 2
++            ;;
++        base|new)
++            MODE="$1"
++            shift
++            ;;
++        *)
++            shift
++            ;;
++    esac
++done
++
++if [[ -z "$OUTPUT_PATH" ]]; then
++    echo "Error: --output_path is required" >&2
++    exit 1
++fi
++
++if [[ -z "$MODE" ]]; then
++    echo "Error: mode (base or new) is required" >&2
++    exit 1
++fi
++
++REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
++cd "$REPO_ROOT/modules/postgres"
++
++case "$MODE" in
++    base)
++        go test -v -count=1 -timeout 10m \
++            -run "^(TestContainerWithWaitForSQL|TestWithConfigFile|TestWithInitScript|TestWithOrderedInitScript)$" \
++            ./... 2>&1 \
++            | go-junit-report -set-exit-code > "$OUTPUT_PATH"
++        ;;
++    new)
++        go test -v -count=1 -timeout 10m \
++            -run "^TestBasicWaitStrategies_reusedContainer$" \
++            ./... 2>&1 \
++            | go-junit-report -set-exit-code > "$OUTPUT_PATH"
++        ;;
++    *)
++        echo "Error: mode must be 'base' or 'new'" >&2
++        exit 1
++        ;;
++esac