fix(docker): apply Postgres schema on first boot

[ckbaker10]

Problem it solves

on a fresh startup the healthcheck stays on bad gateway

Approach taken

Added ensure_db_schema() to build/scripts/standalone/entrypoint.sh, invoked once after service nginx start. It:

• No-ops if ${EO_ROOT}/server/schema/postgresql/createdb.sql isn't present.
• Polls Postgres via psql -tAc 'SELECT 1' (1s interval, 60s cap) before doing anything.
• Checks to_regclass('public.task_result') to detect an already-applied schema and skip re-running.
• Otherwise applies createdb.sql via psql -f.

Reuses only vars already present in the script (EO_ROOT, DB_HOST, DB_PORT, DB_USER, DB_NAME, DB_PWD) — no new env vars, no other files touched.

Testing

tested on 2 live instances

Branch: fix/ensure-db-schema-on-boot

[chrip]

Summary

Adds an idempotent ensure_db_schema() to build/scripts/standalone/entrypoint.sh that waits
for Postgres and applies createdb.sql on first boot, fixing the 502-on-/healthcheck that a
fresh DB volume produces. I verified every claim in the description against the code and the
image build — they all hold, and the implementation is careful (right probe table, right var
names, correct placement after service postgresql start, no data-destructive step).

Two things worth an answer before merge: the interaction with set -e changes the failure mode
from "boots degraded" to "container exits", and the build already contains a commented-out
version of exactly this step that this PR now supersedes and should clean up.

Verification

Claim / Item	Reality	Status
"The stock image has no init step for the Postgres schema"	Confirmed for the entrypoint — no psql, createdb.sql or task_result reference anywhere in entrypoint.sh before this PR.	✓
…but is it applied elsewhere?	build/.docker/standalone.bake.Dockerfile:59 has this exact command, commented out: #RUN sudo -u postgres bash -c "PGPASSWORD=eurooffice psql … -f ${EO_ROOT}/server/schema/postgresql/createdb.sql". The .deb postinst does apply it (document-server-package/deb/template/postinst.m4:166) but is installed with DS_DOCKER_INSTALLATION=true (Dockerfile:57), and postinst gates DB work on that flag. So nothing applies it at runtime. Root cause confirmed.	✓
psql is available in the image	Yes — postgresql postgresql-client installed at standalone.bake.Dockerfile:34.	✓
"Reuses only vars already present (EO_ROOT, DB_HOST, DB_PORT, DB_USER, DB_NAME, DB_PWD)"	All pre-existing: EO_ROOT:7, DB_HOST:30, DB_PORT:31, DB_NAME:32, DB_USER:33. DB_PWD is not defaulted but is a first-class var (deprecated_var DB_PASSWORD DB_PWD at :83, used at :200 and :270). No new env vars.	✓
Schema path is correct	server/schema/postgresql/createdb.sql exists and matches ${EO_ROOT}/server/schema/postgresql/createdb.sql.	✓
task_result is a valid probe table	createdb.sql:26 — CREATE TABLE IF NOT EXISTS "task_result".	✓
Placement — is Postgres running by then?	Yes. service postgresql start is line 424; the new block is inserted after line 428. For external DBs, line 161 already blocks unbounded on nc -z "$DB_HOST" "$DB_PORT". Ordering is right.	✓
Unconditional psql is safe (no MySQL/MSSQL path)	Yes — entrypoint.sh:62-65 hard-exit 1s unless DB_TYPE=postgres. The standalone image is postgres-only by design.	✓
Re-applying createdb.sql is safe	Fully idempotent: 2 × CREATE TABLE IF NOT EXISTS, 1 × CREATE OR REPLACE FUNCTION, zero bare CREATE TABLE / CREATE INDEX / CREATE TYPE. So ON_ERROR_STOP=1 will not trip on a re-run.	✓
Hardcoded public. in the probe ignores DB_SCHEMA	Not a gap — DB_SCHEMA/search_path/PGOPTIONS are not supported anywhere in the standalone entrypoint (only in the package postinst). Consistent with the image's capability.	✓
removetbl.sql is not run	Correct and deliberate-looking. postinst runs removetbl.sql before createdb.sql in non-cluster mode (postinst.m4:162-165); that drops tables. Omitting it is the safe choice for a boot-time path.	✓
DCO	Green. Signed-off-by: ckbkr <…> present on the single commit.	✓

Issues & Suggestions

⚠️ Major

• set -e turns a timeout into a container that won't start — and the message misattributes
  the cause. entrypoint.sh:1-2 is #!/bin/sh + set -e, and ensure_db_schema is invoked
  as a bare command, so return 1 aborts the entrypoint and the container exits.

  Failing fast is arguably better than 502-forever, but this is an unannounced behaviour change
  and the diagnostic is wrong in the most likely case. db_psql -tAc 'SELECT 1' fails for
  authentication errors just as it does for unreachability — so a wrong DB_PWD now yields:

  ERROR: Postgres not reachable at localhost:5432 after 60s
  

  …and the container dies, when previously it booted and docservice logged the actual auth
  error. That is a worse debugging experience for the single most common misconfiguration.

  Suggestion: capture stderr from the probe and surface it on the final attempt, e.g.

  if [ "$tries" -gt 60 ]; then
    echo "ERROR: could not connect to Postgres at ${DB_HOST}:${DB_PORT} as ${DB_USER}/${DB_NAME} after 60s" >&2
    db_psql -tAc 'SELECT 1' >&2 || true   # let psql explain (auth vs. unreachable)
    return 1
  fi

  Also worth stating in the PR description that the container now exits rather than booting
  degraded — that affects anyone relying on the old behaviour.

• standalone.bake.Dockerfile:59 should be removed as part of this PR. That commented-out
  line is the build-time version of what this PR now does at runtime. Leaving it invites someone
  to re-enable it, which would be redundant at best. Deleting it (and noting why the runtime
  approach is correct — build-time schema application does not survive a fresh DB volume, which
  is precisely this bug) makes the fix self-documenting. One extra line of diff, and it closes
  the loop on the root cause.

ℹ️ Minor / 💡 Suggestions

• ℹ️ The call sits after service nginx start (line 428), so nginx serves 502 during schema
  application. Since the reported symptom is the 502, moving ensure_db_schema to just
  after service postgresql start (line 424) would shrink that window to zero. Small win, and
  it reads more logically — DB readiness before the web tier.
• ℹ️ The to_regclass probe is redundant given createdb.sql is fully idempotent, and it
  carries a subtle edge: to_regclass('public.task_result')::text renders as
  public.task_result (not task_result) when public is not in search_path, so
  grep -qx 'task_result' misses and the schema is re-applied every boot. Harmless here
  (idempotent), but … IS NOT NULL returning t/f would be robust, or drop the probe and
  just apply unconditionally.
• ℹ️ The skip path is silent (return 0). One echo "Postgres schema already present, skipping."
  makes boot logs self-explanatory when someone is debugging a different 502.
• ℹ️ No tests, which is expected for a shell entrypoint — there's no harness for it in this
  repo, and "tested on 2 live instances" is reasonable evidence. Noting it only because the
  Nextcloud checklist asks. Manually confirming the second boot (schema already present →
  skip path) and the fresh-volume boot are the two cases that matter.
• 💡 The [ -f "$schema_file" ] || return 0 guard is the right instinct — it keeps this a no-op
  in any image variant that ships without the schema files.

Verdict

Comment — a well-targeted, correctly implemented fix; I verified the root cause, the probe
table, the variable reuse, the service ordering and the idempotency of createdb.sql, and all
of it checks out. Not blocking, but two things deserve a response first: the set -e interaction
now kills the container on a 60s timeout (with a message that misreads auth failures as
unreachability), and the superseded commented-out step at standalone.bake.Dockerfile:59 should
go with it.

Assisted-by: ClaudeCode:claude-opus-5

[chrip]

Summary

Five of the six points from my earlier review are cleanly
addressed — the probe is now a proper boolean, the call moved ahead of nginx, the skip
path logs, psql's real error is surfaced on the final attempt, and the superseded
commented-out line in the Dockerfile is gone. Good, precise follow-up.

Going back over the root cause more carefully, though, I have to correct my own earlier
review: I confirmed the premise "the stock image has no init step for the Postgres
schema", and that is not right. The .deb postinst applies createdb.sql at image
build time and is not gated on DS_DOCKER_INSTALLATION, so the schema is baked into
the image's bundled cluster. The e2e suite proves it empirically.

That reframing exposes a blocking regression: in the image's default configuration
DB_PWD is unset, so ensure_db_schema authenticates with an empty password, can never
connect, burns the full 60s, and — now that the timeout is fatal under set -e — takes
the container down. The fix remains genuinely correct and valuable for the external /
fresh-volume DB case; it just must not be able to brick the default boot.

Prior feedback

From	Point	Status
@chrip (review, 7f653d8)	set -e turns the timeout into a container exit, and the message misattributes auth failures as unreachability	⚠️ partial — diagnostic fixed (entrypoint.sh:451 re-runs the probe unsuppressed to stderr). The behaviour change itself is still unannounced: the PR description doesn't mention that the container now exits instead of booting degraded. See 🔴-1 for why this now matters more, not less
@chrip (review)	standalone.bake.Dockerfile:59 — remove the superseded commented-out schema-apply line	✓ addressed — replaced with a two-line explanatory comment. Note the reason given is inaccurate; see ⚠️-2
@chrip (review)	Move the call before service nginx start to shrink the 502 window	✓ addressed — now entrypoint.sh:466, nginx starts at :471
@chrip (review)	to_regclass(...)::text + grep -qx breaks under a non-default search_path	✓ addressed — SELECT to_regclass('public.task_result') IS NOT NULL compared to t (:458). Correct: to_regclass returns NULL rather than erroring for a missing relation
@chrip (review)	The skip path is silent	✓ addressed — :459 logs "Postgres schema already present, skipping."
@chrip (review)	No tests	ℹ️ still none, still fine — no shell harness exists in this repo. But see 🔴-1: CI does have a test that would have caught this one

Verification

Claim / Item	Reality	Status
"The stock standalone image has no init step for the Postgres schema"	❌ Incorrect — this corrects my earlier review. postinst.m4:339 calls install_db unconditionally inside case configure); only save_wopi_params (:346), font/cache generation (:379) and the service restarts (:409) are gated on DS_DOCKER_INSTALLATION. install_db → install_postges (:138-167) runs removetbl.sql then createdb.sql. standalone.bake.Dockerfile:47 starts Postgres and :57 installs the .deb in the same RUN, so the schema is applied at build time and baked into the image's cluster	❌
…corroborated independently?	Yes. e2e/global-setup.ts:42 runs the standalone image with only -e EXAMPLE_ENABLED=true (no DB_PWD) and waits for /healthcheck to return true (:60-65). /healthcheck reflects docservice readiness, and docservice is exactly the process that dies on a missing task_result. The e2e job passed on run 30342589387 (2026-07-28). A stock image without the schema could not go green there	✓
So is the fix pointless?	No. It is right for the two cases that do hit the bug: an external DB_HOST, and a volume mounted over the Postgres datadir. Neither is covered by build-time application. The fix is well-aimed — only the stated premise and the new Dockerfile comment are wrong	✓
DB_PWD has a usable default	❌ No. entrypoint.sh:29-33 defaults DB_TYPE/HOST/PORT/NAME/USER but never DB_PWD; grepping the whole file, it appears only at :83 (deprecated_var DB_PASSWORD DB_PWD), :200, :270 and :441, always as ${DB_PWD:-}. Contrast build/scripts/orchestrated/docker-entrypoint.sh:98, which does default it (${DB_PWD:-onlyoffice}). It is not persisted under $PRIVATE_DIR either (:95-124 covers only JWT and secure-link secrets)	❌
Password auth is required over TCP in the image	Yes, by every available signal. Nothing in the repo touches pg_hba.conf (zero matches repo-wide), so Debian's PG16 default applies — host all all 127.0.0.1/32 scram-sha-256. Dockerfile:49 creates the role with a password, :55 feeds ds/db-pwd password eurooffice to debconf, and the now-deleted line :59 itself used PGPASSWORD=eurooffice. docservice connects over the same TCP path using the dbPass postinst baked into local.json — which only works if a password is actually demanded	⚠️ see 🔴-1
psql never blocks on a password prompt	❌ db_psql (:441-444) omits -w. postinst.m4:142 and postrm.m4:35 both use -w deliberately	❌
Placement — Postgres up, nginx not yet	✓ service postgresql start :424, ensure_db_schema :466, service nginx start :471. External hosts still block earlier on nc -z (:161)	✓
createdb.sql is idempotent (safe re-run under ON_ERROR_STOP=1)	✓ Unchanged from my last pass: 2 × CREATE TABLE IF NOT EXISTS, 1 × CREATE OR REPLACE FUNCTION, no bare CREATE	✓
removetbl.sql deliberately not run	✓ Still correct — postinst runs it (:162-165) but dropping tables at boot would be destructive	✓
Unconditional psql is safe (no MySQL path)	✓ :62-65 hard-exit 1s unless DB_TYPE=postgres	✓
Dockerfile edit is inert	✓ :58 (rm -rf …) carries no trailing \, so both the old comment and the new one sit outside the RUN. No layer change	✓
No global-name collisions from the new function	✓ schema_file, tries, db_psql are unique in the file — nothing else uses those names. See ℹ️ for the leak itself	✓
Scope	✓ Exactly 2 files, both standalone-boot. The file-size work was correctly split out into #305	✓
DCO	✓ Both commits signed off; DCO check green	✓
Conventional Commits	✓ fix(docker): … on both	✓
AI disclosure	✓ Assisted-by: ClaudeCode:claude-sonnet-5 on both commits (not in the description, but the trailer is the convention that matters)	✓
CI actually exercised this change	❌ No. statusCheckRollup contains only the DCO check. The build workflow run for 3222241 is action_required — awaiting maintainer approval, so build and e2e never ran	❌

Issues & Suggestions

🔴 Blocking

• The default configuration can no longer boot: DB_PWD is unset, so the probe
  authenticates with an empty password, times out, and set -e kills the container.
  entrypoint.sh:441 uses PGPASSWORD="${DB_PWD:-}", and DB_PWD has no default
  anywhere in the standalone entrypoint. In the stock image the role eurooffice was
  created with password eurooffice (Dockerfile:49), nothing relaxes pg_hba.conf,
  and the connection is TCP (-h localhost) — so an empty password cannot authenticate.
  The until loop therefore never succeeds, runs the full 60 s, hits return 1 at
  :452, and because ensure_db_schema is invoked as a bare command under set -e
  (:1-2, :466) the entrypoint aborts before exec /usr/bin/supervisord (:494).

  The irony is that in this configuration there is nothing to do: the schema is already
  present (see the Verification table), so the container used to boot and work — and now
  fails to start at all, with a message blaming the DB for being unreachable.

  The repo has a test that demonstrates exactly this. e2e/global-setup.ts:42 starts the
  image with no DB_PWD and waits on /healthcheck; that job is green on main
  (run 30342589387) and would time out after this change. Because CI here is
  action_required and never ran, nothing caught it.

  Cheapest correct fix — the effective password is already sitting in local.json, which
  the entrypoint has finished writing by this point, and jq is already a dependency:

  db_pwd="${DB_PWD:-$(jq -r '.services.CoAuthoring.sql.dbPass // empty' "$CONFIG_FILE" 2>/dev/null || true)}"
  db_psql() {
    PGPASSWORD="$db_pwd" psql -w -v ON_ERROR_STOP=1 \
      -h "$DB_HOST" -p "$DB_PORT" -U "$DB_USER" -d "$DB_NAME" "$@"
  }

  This is also self-consistent with :200, which deliberately leaves postinst's baked
  dbPass in place when DB_PWD is empty — the value the entrypoint declines to
  overwrite is precisely the one it needs here.

  Worth doing as well, belt-and-braces: don't let a schema-bootstrap failure be fatal
  to boot. ensure_db_schema || echo "WARNING: schema bootstrap failed; docservice may 502" >&2
  keeps the diagnostic while preserving today's "boots degraded" contract. If you'd
  rather keep fail-fast, that's defensible — but then it belongs in the PR description as
  an explicit behaviour change (my earlier point, still open).

  Please verify before merge — one command settles the pg_hba question:
  docker run --rm --entrypoint sh <image> -c 'grep -vE "^#|^$" /etc/postgresql/16/main/pg_hba.conf'.
  If it says trust for 127.0.0.1/32 the empty password is harmless and this drops to a
  nit; every signal in the repo says it will say scram-sha-256. Approving the pending
  workflow run so e2e executes is the other, stronger check.

• db_psql is missing -w, so a password prompt can hang past the 60 s cap.
  entrypoint.sh:441-444. Without -w, psql tries to prompt on a password request; it
  opens /dev/tty if available and otherwise falls back to stdin. Under docker run -d
  stdin is /dev/null and you get EOF, but with an interactive or attached stdin the
  probe can block indefinitely — inside until … >/dev/null 2>&1, invisibly, with the
  timeout never reached. postinst.m4:142 and postrm.m4:35 both pass -w for this
  reason. One flag; please add it regardless of how 🔴-1 is resolved.

⚠️ Major

• The new Dockerfile comment enshrines the wrong explanation.
  standalone.bake.Dockerfile:59-60 now reads "Postgres schema is applied at container
  boot (entrypoint.sh's ensure_db_schema), not at build time: build-time application
  doesn't survive a fresh DB volume." The second clause is true in isolation, but the
  first is misleading: the schema is applied at build time, by the postinst
  (postinst.m4:339), and that is why the deleted line was commented out in the first
  place — it was redundant, not superseded. Someone reading this later will conclude the
  image ships without a schema, which is false.

  Suggested rewording:

  # The .deb postinst applies server/schema/postgresql/createdb.sql at build time
  # (postinst.m4: install_db is not gated on DS_DOCKER_INSTALLATION), which is why the
  # explicit psql call that used to live here was redundant. It does not help when the
  # Postgres datadir is a fresh volume or DB_HOST points at an external server, so
  # entrypoint.sh re-applies it idempotently at boot (ensure_db_schema).

  Same correction applies to the PR description and the first commit message ("The stock
  standalone image has no init step for the Postgres schema"). Worth stating instead
  which configuration reproduced the 502 on your two live instances — external Postgres
  or a mounted datadir? That's the fact that justifies the change, and right now it's the
  one thing the PR doesn't say.

ℹ️ Minor / 💡 Suggestions

• ℹ️ The PR description is stale. It still says the function is "invoked once after
  service nginx start" (it's now before) and describes the old to_regclass(...) probe.
  Worth refreshing along with ⚠️-2 so the description matches 3222241.
• ℹ️ schema_file, tries and db_psql leak into global scope — POSIX sh has no
  local, and a nested function definition persists after the outer function returns.
  No collisions today (verified), so this is a hygiene nit; prefixing (_eds_tries) or
  unset -f db_psql on the way out would tidy it.
• ℹ️ db_psql -tAc 'SELECT 1' >&2 || true (:451) works, but not for the reason it
  looks like. psql already writes diagnostics to stderr; the >&2 only moves the
  (empty) stdout. Harmless — a short comment saying "re-run unsuppressed so psql explains
  auth vs. unreachable" would stop a future reader from 'fixing' it.
• ℹ️ Two commits, the second titled "address review feedback" — worth squashing before
  merge unless the repo squash-merges anyway, so the history reads as one fix.
• 💡 #305 (feat/upload-size-limits) touches the same entrypoint.sh. Whichever lands
  second will want a rebase; no logical conflict.
• 💡 Once 🔴-1 is fixed, the case genuinely worth a manual check is the one that motivated
  the PR: point DB_HOST at an empty external Postgres and confirm (a) first boot applies
  the schema, (b) second boot logs "already present, skipping", (c) a wrong DB_PWD
  produces psql's real auth error rather than a bare "could not connect".

Code quality & conventions

Commits are Conventional and both carry Signed-off-by: (DCO green) plus
Assisted-by: ClaudeCode:claude-sonnet-5 — disclosure convention satisfied. Scope is
tight: 2 files, both standalone-boot, with the file-size work correctly split into #305.
No new files, so no SPDX obligation. No tests, which is right for a shell entrypoint —
but note the repo's e2e suite already covers the affected path and has not been allowed
to run on this PR.

Verdict

Request changes — the follow-up commit resolved five of my six earlier points well,
and the underlying fix is the right idea for external and volume-mounted databases. Two
things need to change first. The blocker is that DB_PWD has no default, so in the
stock image the probe authenticates with an empty password against a scram-sha-256
pg_hba, exhausts the 60 s budget, and — now that the timeout is fatal under set -e —
prevents the container from starting at all, in the one configuration where the schema
was already present and everything worked. Reading dbPass out of local.json as a
fallback and adding -w fixes it in three lines. Separately, I got the root cause wrong
in my earlier review and the PR has inherited it: postinst applies createdb.sql at
build time and is not gated on DS_DOCKER_INSTALLATION, so the description, the first
commit message, and the new Dockerfile comment all need correcting to say what actually
reproduces the 502.

Assisted-by: ClaudeCode:claude-opus-5

[chrip]

Hey @ckbaker10, Sorry for the back-and-forth. I have the reviews generated automatically, and I always take a look at them myself before posting. The review before the last one looked plausible to me. Apparently, the AI has now corrected itself. I hope you can make the change as well. It's a little weird that the reviews are more complicated than the code change. Sorry again. Thanks!

[ckbaker10]

I'd appreciate it if the AI just created the modifications for it's feedback if it's already spending tokens it might as well just fix it.

[chrip]

Everything from my last review is addressed, and addressed properly: the password falls back
to the dbPass postinst bakes into local.json, -w is on the psql call, both misleading
comments are corrected, db_psql no longer leaks, and a bootstrap failure is a warning
instead of a container exit.

One new defect came in with those changes, and it silently disables the safety net this same
commit added. unset -f db_psql at entrypoint.sh:483 is the last command in the
function, so ensure_db_schema returns 0 unconditionally on the apply path — a failing
db_psql -f "$schema_file" at :482 is swallowed and the || echo "WARNING: …" at :485
never fires. (Verified in sh, dash and bash.) That's the path where the real failures
live: ON_ERROR_STOP=1 tripping, permission denied for schema public, a full datadir.
Two lines to fix:

  echo "Applying Postgres schema from ${schema_file}..."
  db_psql -f "$schema_file"
  rc=$?
  unset -f db_psql
  return "$rc"
}

Non-blocking: the description still says "invoked once after service nginx start" (it runs
before) and predates the local.json fallback — worth a refresh, but don't hold merge for it.

With that patch I'm happy to approve. Note CI has never actually run on this PR — all four
pushes are stuck at action_required, so the e2e job that boots the image with no DB_PWD
hasn't exercised any of this. That's on our side to release, not yours.

Assisted-by: ClaudeCode:claude-opus-5

[chrip]

af20645d is exactly the fix — rc=$? captured before the unset, returned explicitly.

I drove the shipped function through all four paths with a stubbed psql (extracted
entrypoint.sh:443-487 verbatim, ran it under /bin/sh with set -e):

• schema already present → "already present, skipping", no warning ✓
• apply succeeds → silent success ✓
• apply fails → WARNING fires (this was the swallowed case) ✓
• can't connect → generic ERROR, then psql's own "password authentication failed", then the
  WARNING, and the entrypoint still reaches supervisord rather than being killed by set -e ✓

That closes everything from all three rounds: the local.json password fallback, -w, the
corrected root-cause comments, the non-fatal boot, and now a bootstrap failure that actually
reports itself. Nice commit message on this one too — it states the mechanism rather than just
"fix review comment".

Approving.

Assisted-by: ClaudeCode:claude-opus-5