diff --git a/services/db/docker-entrypoint.sh b/services/db/docker-entrypoint.sh index 8a725c432d..49757343a1 100644 --- a/services/db/docker-entrypoint.sh +++ b/services/db/docker-entrypoint.sh @@ -206,6 +206,12 @@ apply_migrations_for_role() { # Init + migrations are then retried as a unit, and /tmp/.db_ready is touched ONLY # after both genuinely succeed, so dependents (gated on `.db_ready`) never start # against a half-initialized cluster. +# +# Clear the marker first: /tmp outlives a stop whenever the runtime keeps the +# container's filesystem, as `docker restart` does. A marker left by the +# previous start would report ready as soon as the server answers, before this +# start's init scripts and migrations ran. +rm -f /tmp/.db_ready ( trap 'exit 0' SIGTERM SIGINT until pg_isready -h 127.0.0.1 -p 5432 -U "$POSTGRES_USER" >/dev/null 2>&1; do diff --git a/services/db/docker-entrypoint.test.ts b/services/db/docker-entrypoint.test.ts new file mode 100644 index 0000000000..430db8d11e --- /dev/null +++ b/services/db/docker-entrypoint.test.ts @@ -0,0 +1,122 @@ +// The entrypoint publishes /tmp/.db_ready once this start's init scripts and +// migrations finished, and the healthcheck reads it. /tmp outlives a stop +// (`docker restart` keeps the container's filesystem), so a marker the +// previous start left must be cleared before the server answers, or a restart +// reports ready before its migrations ran. +import { afterAll, describe, expect, test } from 'bun:test'; +import { spawnSync } from 'node:child_process'; +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +const MARKER = '/tmp/.db_ready'; +const source = readFileSync( + join(import.meta.dir, 'docker-entrypoint.sh'), + 'utf8', +); + +describe('docker-entrypoint.sh ready marker', () => { + test('the marker is cleared before the background init block that touches it', () => { + const lines = source.split('\n'); + const code = (line: string) => !line.trimStart().startsWith('#'); + const indexes = (match: (line: string) => boolean) => + lines.flatMap((line, i) => (code(line) && match(line) ? [i] : [])); + + const clears = indexes((line) => line === `rm -f ${MARKER}`); + const touches = indexes((line) => line.includes(`touch ${MARKER}`)); + const blockStart = lines.indexOf('('); + const blockEnd = lines.indexOf(') &', blockStart); + + // One clear, at the top level: not inside a function or the block itself. + expect(clears).toHaveLength(1); + expect(blockStart).toBeGreaterThan(-1); + expect(blockEnd).toBeGreaterThan(blockStart); + expect(touches).toHaveLength(1); + const [clear = -1] = clears; + const [touch = -1] = touches; + expect(clear).toBeLessThan(blockStart); + expect(touch).toBeGreaterThan(blockStart); + expect(touch).toBeLessThan(blockEnd); + }); + + // Run the real script with the server and its probe replaced by doubles on + // PATH. The fake server records whether the marker exists when it starts, + // then answers the probe and waits for the background block to publish it. + const root = mkdtempSync(join(tmpdir(), 'tale-db-entrypoint-')); + afterAll(() => rmSync(root, { recursive: true, force: true })); + + test( + 'a restart reports ready only after its own init finished', + () => { + const bin = join(root, 'bin'); + const state = join(root, 'state'); + const marker = join(root, 'db_ready'); + mkdirSync(bin); + mkdirSync(state); + writeFileSync( + join(bin, 'pg_isready'), + '#!/bin/sh\n[ -e "$TALE_DB_TEST_STATE/up" ]\n', + { mode: 0o755 }, + ); + writeFileSync( + join(bin, 'postgres-entrypoint.sh'), + [ + '#!/bin/sh', + 'if [ -e "$TALE_DB_TEST_MARKER" ]; then seen=present; else seen=absent; fi', + 'echo "$seen" > "$TALE_DB_TEST_STATE/marker-at-start"', + 'touch "$TALE_DB_TEST_STATE/up"', + 'i=0', + 'while [ ! -e "$TALE_DB_TEST_MARKER" ] && [ "$i" -lt 200 ]; do', + ' sleep 0.1', + ' i=$((i + 1))', + 'done', + '', + ].join('\n'), + { mode: 0o755 }, + ); + // Redirect the marker and the baked-in config tree into the sandbox. + const script = join(root, 'docker-entrypoint.sh'); + const rewritten = source + .replaceAll(MARKER, marker) + .replaceAll('/etc/postgresql', join(root, 'etc-postgresql')); + expect(rewritten).not.toContain(MARKER); + writeFileSync(script, rewritten, { mode: 0o755 }); + + const boot = () => { + rmSync(join(state, 'up'), { force: true }); + rmSync(join(state, 'marker-at-start'), { force: true }); + const run = spawnSync('bash', [script, 'postgres'], { + encoding: 'utf8', + timeout: 30_000, + env: { + PATH: `${bin}:${process.env.PATH ?? '/usr/bin:/bin'}`, + DB_PASSWORD: 'not-a-secret', + TALE_DB_ROLE: 'platform', + TALE_DB_TEST_STATE: state, + TALE_DB_TEST_MARKER: marker, + }, + }); + expect(run.stderr).toBe(''); + expect(run.status).toBe(0); + expect(run.stdout).toContain('Database ready.'); + return readFileSync(join(state, 'marker-at-start'), 'utf8').trim(); + }; + + // First start: no marker yet, and init publishes it. + expect(boot()).toBe('absent'); + expect(existsSync(marker)).toBe(true); + // Restart: the marker of the first start is still on disk, and must be + // gone before the server answers. + expect(boot()).toBe('absent'); + expect(existsSync(marker)).toBe(true); + }, + { timeout: 60_000 }, + ); +}); diff --git a/services/db/package.json b/services/db/package.json index 58f7c7e9a8..09bf9b4e03 100644 --- a/services/db/package.json +++ b/services/db/package.json @@ -6,6 +6,7 @@ "setup": "echo 'Docker service - no local setup'", "serve": "docker compose up db -d && docker compose logs -f db", "build": "echo 'No build step'", + "test": "bun test", "docker:build": "docker compose build db", "logs": "docker compose logs -f db", "shell": "docker exec -it tale-db psql -U tale -d tale"