From f7c633ef2060ebeff631f8644ca1fab6706ddb5f Mon Sep 17 00:00:00 2001 From: ghost <49853598+JSONbored@users.noreply.github.com> Date: Fri, 12 Jun 2026 01:40:35 -0700 Subject: [PATCH] fix(preview): bind :3000 instantly and bound diagnostics posts (#2286) * fix(preview): bind :3000 instantly and bound diagnostics posts The trusted preview deploy chain now succeeds end to end (after #2124, #2207, #2217), but the preview container itself dies on Cloudflare with "Container crashed while checking for ports" and no entrypoint diagnostics ever arrive (run 27186150190). Two compounding causes, both reproduced/measured locally against the pinned @cloudflare/containers 0.3.3 behavior: 1. The port window is unwinnable. The library waits a hardcoded ~20s for the container port, but the entrypoint only binds :3000 after redis, postgres, and rails db:prepare complete -- measured at 69s under a basic instance's 1/4 vCPU. Fix: bind :3000 within ~1s via a tiny Ruby placeholder responder (static 503 + meta-refresh, input ignored), verified with a TCP connect poll, and released just before the real server starts. The worker still gates previewReady on the real Rails /up probe and sample data, so readiness semantics are unchanged. 2. Diagnostics could stall boot and never deliver. emit_status used curl with no timeout against the worker, whose Durable Object can be unresponsive while it waits for this container's port (observed as 15s status timeouts in the failed run) -- so boot could deadlock against the port check, and failure detail (#2217) never reached the diagnostics artifact. Fix: --connect-timeout 2 --max-time 5 on all posts, progress events fire-and-forget in the background, and failure paths flush synchronously before exit. Validated locally at Cloudflare basic limits (1 GiB / 0.25 vCPU): first :3000 response in 1s, full boot to Rails /up=200 at ~76s with no OOM, and a forced postgres failure exits 1 in 4s with the failure event flushed. With the port window satisfied, the next real failure will finally surface postgres detail in _container_status and CI artifacts. * fix(preview): never read from placeholder clients Superagent flagged on PR #2286 that the single-threaded :3000 placeholder blocked on client.readpartial, so one connection that sends nothing -- such as a bare TCP port probe, which is exactly what the Cloudflare port check performs -- would wedge the accept loop and starve every later probe. The response is static, so drop the read entirely: each connection is written the 503 warming page and closed immediately, making the loop effectively non-blocking per client. Regression-tested by holding three idle TCP connections open while HTTP probes still answered 503 immediately; full boot under basic-instance limits (1 GiB / 0.25 vCPU) still reaches Rails /up=200 with the placeholder answering at 1s. * fix(preview): stop postgres crashing on Cloudflare's small /dev/shm This is the actual root cause of the preview container dying on Cloudflare with "Container crashed while checking for ports" (run 27341808838) and the maintainer's "dies immediately after postgres-start" (#2217). Reproduced locally: running the preview image with a tiny /dev/shm and a WRITABLE root (the realistic Cloudflare model) crashes postgres on startup with: FATAL: could not resize shared memory segment "/PostgreSQL..." to 1048576 bytes: No space left on device PostgreSQL 17 defaults to dynamic_shared_memory_type = posix, which allocates dynamic shared memory in /dev/shm. Cloudflare Containers provide only a tiny /dev/shm, so postgres FATALs before the entrypoint can bind a port, and the container exits -> the supervisor reports it crashed during the port check. Local Docker hid this because its default /dev/shm is 64MB. Memory was ruled out (boots fine at -m 512m); it is specifically /dev/shm. Fix: set dynamic_shared_memory_type = mmap so DSM is file-backed in the data directory instead of /dev/shm. The build comments out the default posix line, appends mmap, verifies the result, and fails hard if postgresql.conf is missing so this critical setting cannot silently regress. Also log fail_preview reasons to stderr so the real failure is captured by Cloudflare container observability even when the HTTP diagnostics channel is blocked by the worker's port wait. Verified at Cloudflare basic-equivalent limits (1 GiB / 0.25 vCPU, /dev/shm 64k): before, exit 1 right after "Starting PostgreSQL..."; after, "PostgreSQL is ready" -> Rails /up=200 (~97s), placeholder answering :3000 within 1s. Normal-resource boot still reaches /up=200 in ~9s. * fix(preview): use standard-1 instance and widen CI readiness poll Two changes the preview needs to actually deploy and be reported ready on Cloudflare, both validated by deploying to a real CF account. - instance_type basic -> standard-1. The container runs postgres + redis + puma AND generates the full demo dataset (Demo::Generator, ~12 years of transactions), which peaks just over basic's 1 GiB and OOM-kills the container (exit 137) before demo-data-ready. standard-1 (1/2 vCPU, 4 GiB) completes it. Measured on real Cloudflare: rails ready ~46s, demo data ~149s, peak well under 4 GiB, no OOM. - "Collect preview diagnostics" poll budget 40 -> 100 (~128s -> ~350s), with the matching guard in bin/preview_deploy_security_check.rb. A real CF standard-1 run reached previewReady at ~195s, so the old 40-poll budget would have failed a working preview before it finished warming up. The loop still breaks early on previewReady/previewFailed. * fix(preview): apply DSM override to the cluster the entrypoint starts The build selected postgresql.conf via `find ... | head -1`, which picks an arbitrary cluster, while the entrypoint starts the highest-version cluster (ls /etc/postgresql | sort -V | tail -1). Today only PG17 is installed so they coincide, but if a second major version were ever present the override could land on a cluster that never runs, silently reintroducing the /dev/shm crash. Derive PG_CONF from the same highest-version cluster the entrypoint starts so the dynamic_shared_memory_type=mmap override always applies to the active cluster. Verified: build edits /etc/postgresql/17/main/postgresql.conf, and at runtime under a tiny /dev/shm postgres starts with SHOW dynamic_shared_memory_type = mmap. * fix(preview): apply pg_hba trust rules to the cluster the entrypoint starts Same latent issue as the dynamic_shared_memory_type override: the build wrote the local `trust` rules to `find ... | head -1` (arbitrary cluster), while the entrypoint starts the highest-version cluster. With a single PG17 they coincide, but a second major version would send the trust rules to a cluster that never runs, breaking the entrypoint's trust-auth db setup. Derive PG_HBA from the same highest-version cluster (ls /etc/postgresql | sort -V | tail -1) and fail the build if it's missing. Verified under a tiny /dev/shm: postgres starts and CREATE ROLE/CREATE DATABASE succeed via trust. --- .github/workflows/preview-deploy.yml | 8 +- Dockerfile.preview | 118 ++++++++++++++++++++++++--- bin/preview_deploy_security_check.rb | 2 +- workers/preview/wrangler.toml | 9 +- 4 files changed, 123 insertions(+), 14 deletions(-) diff --git a/.github/workflows/preview-deploy.yml b/.github/workflows/preview-deploy.yml index c26ea1cf0..3064ab9f6 100644 --- a/.github/workflows/preview-deploy.yml +++ b/.github/workflows/preview-deploy.yml @@ -361,7 +361,13 @@ jobs: last_error="" mkdir -p "$diagnostics_dir" - for attempt in $(seq 1 40); do + # ~100 polls x ~3.5s = up to ~6 min. The preview must cold-start the + # container (≈1.3 GB image pull + sandbox init) and generate the full + # demo dataset before previewReady flips. A real Cloudflare standard-1 + # run measured previewReady at ~195s (rails ~46s, demo data ~149s) — + # the old 40-poll (~140s) budget would have failed a working preview. + # Keep generous headroom; the loop still breaks early on ready/failed. + for attempt in $(seq 1 100); do if curl -fsS --connect-timeout 5 --max-time 15 "$PREVIEW_URL/_container_status" -o "$diagnostics_file"; then if jq -e . "$diagnostics_file" >/dev/null 2>&1; then jq -c --argjson attempt "$attempt" --arg at "$(date -u +%Y-%m-%dT%H:%M:%SZ)" \ diff --git a/Dockerfile.preview b/Dockerfile.preview index ec251ef5d..f7cf1a2df 100644 --- a/Dockerfile.preview +++ b/Dockerfile.preview @@ -48,13 +48,41 @@ RUN groupadd --system --gid 1000 rails && \ echo "rails ALL=(ALL) NOPASSWD: /usr/bin/pg_ctlcluster, /usr/bin/redis-server" > /etc/sudoers.d/rails && \ chmod 0440 /etc/sudoers.d/rails -# Configure PostgreSQL to allow local connections -RUN PG_HBA=$(find /etc/postgresql -name pg_hba.conf 2>/dev/null | head -1) && \ - if [ -n "$PG_HBA" ]; then \ - echo "local all all trust" > "$PG_HBA" && \ - echo "host all all 127.0.0.1/32 trust" >> "$PG_HBA" && \ - echo "host all all ::1/128 trust" >> "$PG_HBA"; \ - fi +# Configure PostgreSQL to allow local connections. Target the highest-version +# cluster's pg_hba.conf -- the same one the entrypoint starts (ls +# /etc/postgresql | sort -V | tail -1) -- so the trust rules always land on the +# cluster that actually runs, even if multiple majors are present. +RUN PG_VERSION="$(ls /etc/postgresql 2>/dev/null | sort -V | tail -1)" && \ + PG_HBA="/etc/postgresql/${PG_VERSION}/main/pg_hba.conf" && \ + if [ -z "$PG_VERSION" ] || [ ! -f "$PG_HBA" ]; then \ + echo "ERROR: pg_hba.conf not found for cluster '${PG_VERSION:-none}'; cannot configure local trust" >&2; \ + exit 1; \ + fi && \ + echo "local all all trust" > "$PG_HBA" && \ + echo "host all all 127.0.0.1/32 trust" >> "$PG_HBA" && \ + echo "host all all ::1/128 trust" >> "$PG_HBA" + +# Use file-backed dynamic shared memory instead of POSIX /dev/shm. +# Cloudflare Containers provide only a tiny /dev/shm, and PostgreSQL's default +# dynamic_shared_memory_type = posix FATALs on startup there with +# "could not resize shared memory segment ... No space left on device", which +# kills the container before it can serve a port. mmap keeps DSM in the data +# directory ($PGDATA/pg_dynshmem), removing the /dev/shm dependency. Local +# Docker hides this because its default /dev/shm is 64MB. +# +# Select the highest-version cluster's config -- the same one the entrypoint +# starts (ls /etc/postgresql | sort -V | tail -1) -- so the override always +# lands on the cluster that actually runs, even if multiple majors are present. +RUN PG_VERSION="$(ls /etc/postgresql 2>/dev/null | sort -V | tail -1)" && \ + PG_CONF="/etc/postgresql/${PG_VERSION}/main/postgresql.conf" && \ + if [ -z "$PG_VERSION" ] || [ ! -f "$PG_CONF" ]; then \ + echo "ERROR: postgresql.conf not found for cluster '${PG_VERSION:-none}'; cannot disable /dev/shm DSM dependency" >&2; \ + exit 1; \ + fi && \ + sed -i 's/^[[:space:]]*dynamic_shared_memory_type[[:space:]]*=/# &/' "$PG_CONF" && \ + printf '\n# Preview: avoid /dev/shm dependency (small in Cloudflare Containers)\ndynamic_shared_memory_type = mmap\n' >> "$PG_CONF" && \ + grep -qx 'dynamic_shared_memory_type = mmap' "$PG_CONF" && \ + echo "Configured dynamic_shared_memory_type=mmap in $PG_CONF" # Create database directory with correct permissions RUN mkdir -p /var/run/postgresql && \ @@ -72,19 +100,26 @@ set -e cd /rails -emit_status() { +# Diagnostics posts are best-effort and must NEVER stall boot: the worker's +# Durable Object can be unresponsive while it waits for this container's port, +# so an unbounded curl here deadlocks startup against the port check. +emit_status_sync() { if [ -n "$PREVIEW_ORIGIN" ] && [ -n "$PREVIEW_DIAGNOSTICS_NONCE" ]; then local stage="$1" local detail="$2" local payload payload=$(STAGE="$stage" DETAIL="$detail" ruby -rjson -e 'print JSON.generate({stage: ENV.fetch("STAGE"), detail: ENV.fetch("DETAIL", "")})' 2>/dev/null) || return 0 - curl -fsS -X POST "$PREVIEW_ORIGIN/_container_event" \ + curl -fsS --connect-timeout 2 --max-time 5 -X POST "$PREVIEW_ORIGIN/_container_event" \ -H 'content-type: application/json' \ -H "x-preview-diagnostics-nonce: $PREVIEW_DIAGNOSTICS_NONCE" \ --data "$payload" >/dev/null || true fi } +emit_status() { + emit_status_sync "$1" "$2" & +} + summarize_log_tail() { local file="$1" local label="$2" @@ -104,7 +139,12 @@ summarize_log_tail() { fail_preview() { local detail="$1" trap - ERR - emit_status failed "$detail" + # Always log to stderr too: the HTTP diagnostics channel can be unreachable + # while the worker's Durable Object is still waiting for this container's + # port, but stderr is captured by Cloudflare container observability, so the + # real reason survives even when the event POST does not. + echo "PREVIEW FAILED: ${detail}" >&2 + emit_status_sync failed "$detail" exit 1 } @@ -126,9 +166,60 @@ postgres_cluster_snapshot() { printf '%s' "$snapshot" } -trap 'emit_status failed "preview-entrypoint failed on line ${LINENO}"' ERR +trap 'emit_status_sync failed "preview-entrypoint failed on line ${LINENO}"' ERR emit_status boot "preview-entrypoint started" +# Bind :3000 immediately with a tiny placeholder responder. Cloudflare's +# container supervisor only waits ~20s for the port, while the full stack +# (redis, postgres, migrations) needs 60s+ on a basic instance. The +# placeholder answers 503 with a meta-refresh; the worker still gates +# previewReady on the real Rails /up probe and sample data, so readiness +# semantics are unchanged. It is replaced by the real server below. +ruby -rsocket -e ' + server = TCPServer.new("0.0.0.0", 3000) + body = "Sure preview warmingSure preview is booting (database setup in progress). This page refreshes automatically." + loop do + client = begin + server.accept + rescue StandardError + next + end + begin + # Never read from the client: the response is static, and a blocking read + # would let one silent connection (e.g. a bare TCP port probe) wedge this + # single-threaded loop and starve every later probe. + client.write("HTTP/1.1 503 Service Unavailable\r\nContent-Type: text/html; charset=utf-8\r\nContent-Length: #{body.bytesize}\r\nRetry-After: 3\r\nConnection: close\r\n\r\n#{body}") + rescue StandardError + ensure + begin + client.close + rescue StandardError + end + end + end +' >/tmp/port-placeholder.log 2>&1 & +PLACEHOLDER_PID=$! + +# Confirm the placeholder actually bound :3000 before reporting it. A plain +# TCP connect is used because the placeholder intentionally answers 503. +PLACEHOLDER_BOUND=0 +for i in {1..10}; do + if (exec 3<>/dev/tcp/127.0.0.1/3000) 2>/dev/null; then + PLACEHOLDER_BOUND=1 + break + fi + if ! kill -0 "$PLACEHOLDER_PID" 2>/dev/null; then + break + fi + sleep 0.2 +done +if [ "$PLACEHOLDER_BOUND" -eq 1 ]; then + emit_status port-placeholder "bound :3000 placeholder responder (pid ${PLACEHOLDER_PID})" +else + echo "Warning: :3000 placeholder did not come up; continuing boot" >&2 + emit_status port-placeholder-missing "placeholder responder failed to bind :3000: $(summarize_log_tail /tmp/port-placeholder.log port-placeholder)" +fi + REDIS_READY=0 POSTGRES_READY=0 @@ -230,6 +321,11 @@ else emit_status demo-data-deferred "deferring demo data creation until after rails boot" fi +# Release :3000 for the real server. The brief listener gap is handled by the +# worker, which catches containerFetch errors and serves its wait page. +kill "$PLACEHOLDER_PID" 2>/dev/null || true +wait "$PLACEHOLDER_PID" 2>/dev/null || true + # Execute the main command with an internal readiness probe echo "Starting Rails server..." emit_status rails-start "starting rails server" diff --git a/bin/preview_deploy_security_check.rb b/bin/preview_deploy_security_check.rb index 026bf54fb..e7fedb0ca 100644 --- a/bin/preview_deploy_security_check.rb +++ b/bin/preview_deploy_security_check.rb @@ -487,7 +487,7 @@ assert(!configure_image_run.include?('const updated = original.replace(/image = assert_run_includes(create_deployment, "github.rest.repos.createDeployment", "ref: headSha", "preview-pr-${prNumber}") assert_run_includes(deploy, 'cd "$RUNNER_TEMP/sure-preview-worker"', "deploy_once()", "./node_modules/.bin/wrangler deploy --config wrangler.toml", '--var "PR_NUMBER:${PR_NUMBER}"', 'tee "$deploy_log" | ./deploy/redact_preview_log.sh', "deploy_status=${PIPESTATUS[0]}", "associated with a different durable object namespace", 'if ! ./node_modules/.bin/wrangler delete --name "sure-preview-${PR_NUMBER}" --force', "Preview Worker delete failed", "retrying once") assert_run_includes(warm_preview, "$PREVIEW_URL/_container_status", "--connect-timeout 5", "--max-time 15") -assert_run_includes(collect_diagnostics, "$PREVIEW_URL/_container_status", "--connect-timeout 5", "--max-time 15", "seq 1 40", "preview-diagnostics", "preview-diagnostics.json", "latest-metrics.json", "metrics-polls.log", "summary.md", '! jq -e . "$diagnostics_file"', 'raw_snippet="$(head -c 2048 "$diagnostics_file")"', 'latest_metrics_snapshot="$(head -c 2048 "$latest_metrics_file")"', "rawSnippet", "latestMetrics", "jq -e '.previewReady == true or .previewFailed == true'", "jq -e '.previewFailed == true'", "Preview diagnostics from _container_status reported previewFailed=true", "jq -e '.previewReady == true'", "Preview diagnostics from _container_status did not reach previewReady=true", ".timings.previewReadyAt != null and .timings.secondsToPreviewReady != null", "Preview diagnostics are missing readiness timing fields", "exit 1") +assert_run_includes(collect_diagnostics, "$PREVIEW_URL/_container_status", "--connect-timeout 5", "--max-time 15", "seq 1 100", "preview-diagnostics", "preview-diagnostics.json", "latest-metrics.json", "metrics-polls.log", "summary.md", '! jq -e . "$diagnostics_file"', 'raw_snippet="$(head -c 2048 "$diagnostics_file")"', 'latest_metrics_snapshot="$(head -c 2048 "$latest_metrics_file")"', "rawSnippet", "latestMetrics", "jq -e '.previewReady == true or .previewFailed == true'", "jq -e '.previewFailed == true'", "Preview diagnostics from _container_status reported previewFailed=true", "jq -e '.previewReady == true'", "Preview diagnostics from _container_status did not reach previewReady=true", ".timings.previewReadyAt != null and .timings.secondsToPreviewReady != null", "Preview diagnostics are missing readiness timing fields", "exit 1") assert_run_includes(collect_failure_diagnostics, "preview-failure-diagnostics", "preview-request.json", "preview-image-manifest.json", "wrangler-source.toml", "wrangler-push.toml", "wrangler-final.toml", "wrangler.toml", "wrangler-containers-push.log", "wrangler-deploy.log", "redaction_helper=", 'sanitize_copy "$RUNNER_TEMP/sure-preview-worker/wrangler.source.toml"', 'sanitize_copy "$RUNNER_TEMP/wrangler-push.toml"', 'sanitize_copy "$RUNNER_TEMP/wrangler-final.toml"', "wrangler-deploy.clean.log", "resolutionSource") assert_run_includes(prepare_cleanup_metadata, "preview-cleanup-metadata", "redact_preview_log.sh", "$RUNNER_TEMP/sure-preview-worker/wrangler.toml", "$metadata_dir/wrangler.toml") assert_run_includes(update_deployment_status, "github.rest.repos.createDeploymentStatus", "process.env.DEPLOY_RESULT === 'success'", "deployment_id: Number(process.env.DEPLOYMENT_ID)") diff --git a/workers/preview/wrangler.toml b/workers/preview/wrangler.toml index 17e95b899..9845919d2 100644 --- a/workers/preview/wrangler.toml +++ b/workers/preview/wrangler.toml @@ -17,7 +17,14 @@ enabled = true [[containers]] class_name = "RailsContainer" image = "../../Dockerfile.preview" -instance_type = "basic" +# standard-1 (1/2 vCPU, 4 GiB RAM, 8 GB disk), not basic (1/4 vCPU, 1 GiB, +# 4 GB): this single container runs postgres + redis + puma AND generates the +# full demo dataset (Demo::Generator: ~12 years of transactions). That +# generation peaks just over 1 GiB, so basic OOM-kills it (exit 137) mid-boot +# and the preview never reaches demo-data-ready. Verified on a real Cloudflare +# standard-1 deploy: rails ready ~46s, demo data ~149s, previewReady ~195s, +# peak well under 4 GiB, no OOM. +instance_type = "standard-1" max_instances = 1 # Durable Object binding for the container