diff --git a/modules/postgres/wait_strategies.go b/modules/postgres/wait_strategies.go index 92dc3f6ec6..461fc5f695 100644 --- a/modules/postgres/wait_strategies.go +++ b/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! diff --git a/modules/postgres/wait_strategies_test.go b/modules/postgres/wait_strategies_test.go new file mode 100644 index 0000000000..f4a34c940e --- /dev/null +++ b/modules/postgres/wait_strategies_test.go @@ -0,0 +1,102 @@ +package postgres_test + +import ( + "context" + "database/sql" + "fmt" + "io" + "strings" + "testing" + "time" + + "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 returns 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 has to run 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 the stop signal with an immediate + // SIGKILL. The session opened 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) + } +}