diff --git a/Dockerfile b/Dockerfile new file mode 100644 index 0000000000..d4439790e6 --- /dev/null +++ b/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"] 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..b6e9ff2005 --- /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 0000000000..4449d50891 --- /dev/null +++ b/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