diff --git a/scripts/images/rds-postgres/rds-init b/scripts/images/rds-postgres/rds-init index c52bce80e..5b2502875 100644 --- a/scripts/images/rds-postgres/rds-init +++ b/scripts/images/rds-postgres/rds-init @@ -36,6 +36,11 @@ PG_USER="${RDS_PG_USER:-postgres}" DATA_MOUNT="${RDS_DATA_MOUNT:-/var/lib/postgresql}" PGDATA="${RDS_PGDATA:-${DATA_MOUNT}/${PG_VERSION}/data}" +# Written between initdb and a working master role. It lives on the data volume +# because no trap survives a crash or a hard kill, and it is what stops the next +# boot attaching to a datadir whose master role was never created. +BOOTSTRAP_SENTINEL="${PGDATA}/rds-bootstrap-incomplete" + HANDOFF_DIR="${RDS_HANDOFF_DIR:-/run/spinifex-rds}" ENV_FILE="${HANDOFF_DIR}/bootstrap.env" SOCKET_DIR="${RDS_SOCKET_DIR:-/run/postgresql}" @@ -60,9 +65,9 @@ fail() { echo "[rds-init] ERROR: $*" >&2; exit 1; } _created_datadir=0 _bootstrapped=0 -# Runs from an EXIT trap rather than a statement, so `set -e` on any unguarded -# step between initdb and a working master role cannot skip it — that would leave -# a healthy-looking engine whose master role has no password, unrepairable. +# Runs from a trap rather than a statement, so neither `set -e` on an unguarded +# step between initdb and a working master role nor a signal can skip it — either +# would leave a healthy-looking engine with no usable master role, unrepairable. clear_unfinished_datadir() { [ "${_created_datadir}" = "1" ] || return 0 [ "${_bootstrapped}" = "1" ] && return 0 @@ -163,6 +168,20 @@ initialise_cluster() { # Only a datadir initdb built over an empty directory may be swept later. _created_datadir="${_was_empty}" trap clear_unfinished_datadir EXIT + # An EXIT trap does not run on a signal in a POSIX shell, so the same sweep + # is armed for the signals a shutdown, an ACPI powerdown or a host-side + # force-stop delivers. EXIT is disarmed first so the sweep runs once. + trap 'trap - EXIT; clear_unfinished_datadir; exit 1' INT TERM HUP + + # No trap survives a crash or a SIGKILL, so the incomplete state is also + # recorded on the volume itself. Cleared once the master role exists. + printf '%s\n' \ + 'rds-init created this datadir and did not finish bootstrapping the master role.' \ + 'While this file exists rds-init refuses to start the engine: the one-shot master' \ + 'password is spent, so the cluster cannot be repaired in place. Recover out of band.' \ + > "${BOOTSTRAP_SENTINEL}" + chown "${PG_USER}:${PG_USER}" "${BOOTSTRAP_SENTINEL}" + chmod 0600 "${BOOTSTRAP_SENTINEL}" cat >> "${PGDATA}/pg_hba.conf" <<'EOF' @@ -268,9 +287,10 @@ start_bootstrap_server() { start" } +# The trust-authenticated pg_hba is removed by the caller, not here: a caller +# reading this function's status must be reading pg_ctl's. stop_bootstrap_server() { as_pg "'${PG_BIN}/pg_ctl' --pgdata='${PGDATA}' --wait --timeout=60 --mode=fast stop" - rm -f "${BOOTSTRAP_DIR}/pg_hba.conf" } # Identifiers and the password ride the environment and are re-quoted by psql, @@ -296,15 +316,37 @@ bootstrap_master() { create_database || _rc=$? fi - # A failed stop must not abort before the check below, or a failed bootstrap - # would be reported as a pg_ctl error and skip the cleanup. - stop_bootstrap_server || log "warning: the bootstrap server did not stop cleanly" + # `set -e` is suspended for a function called from an AND-OR list, so the + # stop status is captured rather than propagated. A failed stop must also not + # abort before the bootstrap check below, which names the real cause. + _stop_rc=0 + stop_bootstrap_server || _stop_rc=$? + rm -f "${BOOTSTRAP_DIR}/pg_hba.conf" - # The datadir is swept by the EXIT trap, which also covers the steps here - # that abort under `set -e` before this check is ever reached. + # The datadir is swept by the traps, which also cover the steps here that + # abort under `set -e` before this check is ever reached. if [ "${_rc}" -ne 0 ]; then fail "bootstrapping the master role/database failed (rc=${_rc})" fi + + # The master role exists from here, so the datadir is complete and nothing + # may clear it — including on the stop failure reported below. + mark_bootstrap_complete + + # A postmaster still holding the datadir blocks the postgresql service for + # the life of the boot, so this is a failure and not a warning. + if [ "${_stop_rc}" -ne 0 ]; then + fail "the bootstrap server did not stop (rc=${_stop_rc}) and still holds ${PGDATA}; the postgresql service cannot start until it is gone" + fi +} + +# Retires the guards armed at initdb, in an order where no signal in between can +# clear a datadir that now carries a usable master role. +mark_bootstrap_complete() { + _bootstrapped=1 + rm -f "${BOOTSTRAP_SENTINEL}" || + fail "the master role exists but ${BOOTSTRAP_SENTINEL} could not be removed; the next boot will refuse to start until it is deleted" + trap - EXIT INT TERM HUP } # The master role is administrative but deliberately not a PostgreSQL SUPERUSER, @@ -363,14 +405,18 @@ load_handoff require_data_volume if [ -s "${PGDATA}/PG_VERSION" ]; then + # The engine would start cleanly on such a datadir and pass every health + # probe with no master role to log in as. The sentinel says the bootstrap did + # not finish, not that the volume is empty, so nothing here clears it. + if [ -e "${BOOTSTRAP_SENTINEL}" ]; then + fail "datadir ${PGDATA} carries ${BOOTSTRAP_SENTINEL}: an earlier bootstrap was interrupted before the master role existed, and the one-shot master password is spent; recover the data volume out of band rather than starting the engine" + fi log "datadir ${PGDATA} is already initialised — attaching" install_runtime_config else initialise_cluster install_runtime_config bootstrap_master - _bootstrapped=1 - trap - EXIT fi consume_secrets diff --git a/scripts/images/rds-postgres/rds-init_test.sh b/scripts/images/rds-postgres/rds-init_test.sh index 6992555d7..7b0626335 100644 --- a/scripts/images/rds-postgres/rds-init_test.sh +++ b/scripts/images/rds-postgres/rds-init_test.sh @@ -96,12 +96,19 @@ EOF # pg_ctl stub: records start/stop of the bootstrap server. PGCTL_START_FAIL # simulates a cluster that will not come up, e.g. a parameter the guest cannot -# allocate. +# allocate; PGCTL_STOP_FAIL a postmaster that will not let go of the datadir. +# PGCTL_KILL_PARENT signals rds-init from the start call, which is the window +# between initdb and the master role. cat > "${STUBBIN}/pg_ctl" <<'EOF' #!/bin/sh echo "pg_ctl $*" >> "${PGCTL_CALLS}" for a in "$@"; do [ "$a" = "start" ] && [ "${PGCTL_START_FAIL:-0}" = "1" ] && exit 1 + [ "$a" = "stop" ] && [ "${PGCTL_STOP_FAIL:-0}" = "1" ] && exit 1 + if [ "$a" = "start" ] && [ "${PGCTL_KILL_PARENT:-0}" = "1" ]; then + kill -TERM "$(cat "${KILL_PID_FILE}")" + exit 0 + fi done exit 0 EOF @@ -130,8 +137,11 @@ pass() { echo "ok: $*"; } DATA_MOUNT="${WORK}/data" PGDATA="${DATA_MOUNT}/18/data" +SENTINEL="${PGDATA}/rds-bootstrap-incomplete" HANDOFF="${WORK}/run/spinifex-rds" MOUNTS="${WORK}/mounts" +KILL_PID_FILE="${WORK}/rds-init.pid" +export KILL_PID_FILE INITDB_CALLS="${WORK}/initdb.calls" PGCTL_CALLS="${WORK}/pg_ctl.calls" @@ -175,8 +185,8 @@ reset_state() { unset INITDB_FAIL PG_HBA_AS_DIR PSQL_FAIL LS_FAIL RDS_ALLOW_LOCAL_DATADIR MASTER_USER || true } -# run: invoke rds-init with every path knob pointed into the temp dir. -run() { +# run_env: invoke a command with every path knob pointed into the temp dir. +run_env() { env RDS_PG_BIN="${STUBBIN}" \ RDS_DATA_MOUNT="${DATA_MOUNT}" \ RDS_HANDOFF_DIR="${HANDOFF}" \ @@ -189,12 +199,28 @@ run() { PG_HBA_AS_DIR="${PG_HBA_AS_DIR:-0}" \ PSQL_FAIL="${PSQL_FAIL:-0}" \ INITDB_CALLS="${INITDB_CALLS}" PGCTL_CALLS="${PGCTL_CALLS}" PSQL_CALLS="${PSQL_CALLS}" \ - sh "${SCRIPT}" "${WORK}/out" 2>&1 || { fail "$1: non-zero exit: $(cat "${WORK}/out")"; return 1; }; } run_fails() { run > "${WORK}/out" 2>&1 && fail "$1: expected a non-zero exit" || pass "$1: refused"; } +# run_signalled: run rds-init in the background with its PID published for the +# stub that signals it. The wrapper writes the PID before exec'ing the script, +# so the stub cannot read the file before it exists. +run_signalled() { + run_env sh -c 'echo $$ > "$1"; shift; exec sh "$@"' _ "${KILL_PID_FILE}" "${SCRIPT}" \ + > "${WORK}/out" 2>&1 & + _bg=$! + wait "${_bg}" +} + +run_signalled_fails() { + run_signalled && fail "$1: expected a non-zero exit" || pass "$1: refused" +} + # --- Case 1: initialize on a fresh data volume --- reset_state write_handoff initialize 's3cr3t' appdb @@ -243,6 +269,12 @@ if run_ok "initialize"; then && pass "initialize: bootstrap server is socket-only" || fail "initialize: bootstrap server listened on TCP" grep -q 'stop' "${PGCTL_CALLS}" \ && pass "initialize: bootstrap server stopped" || fail "initialize: bootstrap server left running" + [ -e "${SENTINEL}" ] \ + && fail "initialize: incomplete-bootstrap sentinel left behind" \ + || pass "initialize: sentinel cleared once the master role exists" + [ -e "${WORK}/run/rds-init/pg_hba.conf" ] \ + && fail "initialize: the trust-auth pg_hba outlived the bootstrap window" \ + || pass "initialize: trust-auth pg_hba removed" fi # --- Case 2: the master password does not outlive its use --- @@ -403,6 +435,58 @@ run_fails "preexisting-nosweep" || fail "preexisting-nosweep: swept a datadir it did not create" unset INITDB_FAIL +# --- Case 6g: a SIGTERM between initdb and the master role clears the datadir --- +# An EXIT trap does not run on a signal in a POSIX shell, so a shutdown, an ACPI +# powerdown from a stop arriving while the instance is still creating, or a +# host-side force-stop would otherwise leave a datadir with no master role. +reset_state +write_handoff initialize 's3cr3t' appdb +PGCTL_KILL_PARENT=1 +export PGCTL_KILL_PARENT +run_signalled_fails "sigterm-midbootstrap" +[ -e "${PGDATA}/PG_VERSION" ] \ + && fail "sigterm-midbootstrap: datadir kept after a signal mid-bootstrap" \ + || pass "sigterm-midbootstrap: datadir cleared" +[ -s "${PSQL_CALLS}" ] \ + && fail "sigterm-midbootstrap: the master role work ran after the signal" \ + || pass "sigterm-midbootstrap: stopped before the master role" +unset PGCTL_KILL_PARENT + +# --- Case 6h: a datadir carrying the sentinel is refused, not started --- +# No trap survives a crash or a SIGKILL, so the sentinel is the only record that +# the bootstrap did not finish. It says the master role is missing, not that the +# volume is empty, so the refusal must not clear anything. +reset_state +write_handoff initialize 's3cr3t' appdb +if run_ok "sentinel-setup"; then + echo 'customer table data' > "${PGDATA}/base-relation" + : > "${SENTINEL}" + write_handoff attach '' appdb + run_fails "sentinel-refused" + [ -f "${PGDATA}/base-relation" ] \ + && pass "sentinel-refused: datadir preserved" || fail "sentinel-refused: datadir cleared" + grep -q 'recover the data volume out of band' "${WORK}/out" \ + && pass "sentinel-refused: refusal points at out-of-band recovery" \ + || fail "sentinel-refused: no refusal message" +fi + +# --- Case 6i: a bootstrap server that will not stop is a failure --- +# It holds the datadir, so the postgresql service cannot start for the life of +# the boot. The master role exists by then, so the datadir itself must survive. +reset_state +write_handoff initialize 's3cr3t' appdb +PGCTL_STOP_FAIL=1 +export PGCTL_STOP_FAIL +run_fails "stop-fail" +grep -q 'did not stop' "${WORK}/out" \ + && pass "stop-fail: reported rather than swallowed" || fail "stop-fail: no failure message" +[ -e "${PGDATA}/PG_VERSION" ] \ + && pass "stop-fail: bootstrapped datadir kept" || fail "stop-fail: cleared a datadir with a master role" +[ -e "${SENTINEL}" ] \ + && fail "stop-fail: sentinel kept although the master role exists" \ + || pass "stop-fail: sentinel cleared" +unset PGCTL_STOP_FAIL + # --- Case 7: no serving cert -> TLS off rather than a failed start --- reset_state write_handoff initialize 's3cr3t' ''