Commit ebaf5fa
authored
ci: overhaul CI infrastructure (#1148)
* tests: move Citus DROP TABLE out of teardown into an explicit test step
teardown_module() is not a safe place to run DDL that depends on the
cluster being in a clean state. If the final test perturbs the cluster
(e.g. test_011_start_worker2b_again reattaches a failed node), Citus
metadata sync may still be in progress when teardown fires immediately
afterwards. The DROP TABLE then fails with:
ObjectNotInPrerequisiteState: <node> is a metadata node, but is out of sync
Because the exception is raised before cluster.destroy(), all background
pg_autoctl and postgres processes stay alive inside the test container,
keeping the docker run from exiting. The job then burns its remaining
CI budget until the 15-minute hard timeout fires — consistently across
PG14, 15, 16, and 17.
Fix: move wait_until_metadata_sync() + DROP TABLE into a numbered test
step that runs while the cluster is in a known-stable state, and reduce
teardown_module() to cluster.destroy() only.
- test_basic_citus_operation: test_015_drop_table (after coordinator failover)
- test_citus_multi_standbys: test_012_drop_table (after worker2b rejoins as secondary)
- test_citus_force_failover: test_004b_drop_table (before node-drop tests)
- test_nonha_citus_operation: test_007b_drop_table (before primary failures)
* tests: fix five categories of spurious CI failures
1. basic_operation.pgaf test_022_detect_network_partition: sleep 3s → 30s
The monitor assigns demote_timeout (and node3 reaches wait_primary) on
the monitor/node3 side. Node2's local keeper detects connection loss
independently via network_partition_timeout (default 20s, defaults.h).
Those two clocks are decoupled: 3s was not enough for node2's keeper to
fire its own partition check and call fsm_stop_postgres. 30s covers
the worst case (20s timeout + keeper loop interval + Postgres shutdown).
2. wait_until_assigned_state: add or_state= keyword argument
The FSM can transition draining → demote_timeout in the same wall-clock
second (monitor event log confirms same timestamp). A 100ms poll loop
can miss draining entirely. or_state= lets callers accept either state.
Used in test_003_002_stop_primary.
3. has_needed_replication_slots: 5s retry on OperationalError
A newly-joined async or sync standby node can transiently refuse
connections right after its FSM state converges to secondary.
wait_until_state checks pg_auto_failover state, not psycopg2
connectivity. Retry the pgmajor() call for up to 5s (with cache
invalidation) before surfacing the exception.
4. DataNode.run_sql_query_retry: new helper for transient connectivity
Like run_sql_query but retries on OperationalError and
CannotConnectNow (ERRCODE 57P03, 'database system is starting up')
for up to a caller-specified timeout with 0.5s back-off.
5. test_nonha_citus_operation test_004: use run_sql_query_retry (30s)
create_distributed_table internally connects to each worker node.
Even after worker pg_autoctl state reaches 'single', Postgres can
still report 'system is starting up' at the Citus inter-node level.
Retry for 30s instead of failing immediately.
* tests: fix Citus DROP TABLE race against metadata sync
wait_until_metadata_sync() reflects the coordinator's view of sync
completion, but a recently-rejoined worker (e.g. after re-attach as
secondary) may still be applying Citus metadata updates when the
coordinator considers sync done. DROP TABLE then fails with:
ObjectNotInPrerequisiteState: <node> is a metadata node, but is out of sync
Two related fixes:
1. Add DataNode.citus_run_ddl_after_sync(query, timeout=60): a helper
that calls wait_until_metadata_sync() then the DDL, and retries the
whole pair with 1s back-off if ObjectNotInPrerequisiteState is raised,
for up to timeout seconds. Use it everywhere DROP TABLE t1 appears
after a metadata-sync wait.
2. Remove test_004b_drop_table from test_citus_force_failover: this test
intentionally leaves workers in disrupted states (worker1a dead, worker1b
in wait_primary with no secondary). There is no clean stable point to
run Citus DDL here; DROP TABLE coverage is provided by the other citus
test files.
Verified: 77 tests pass cleanly across two consecutive local runs on PG16.
* tests: extend has_needed_replication_slots retry to cover list_replication_slot_names
The 5s retry loop introduced in the previous commit only guarded pgmajor().
After that call returned, list_replication_slot_names() made a fresh
connection outside the retry scope — which could still get 'Connection
refused' if Postgres dropped the socket between the two calls.
Reproduce: test_009_add_sync_standby in test_multi_async.py on PG14 in CI;
node4 reaches FSM 'secondary' so wait_until_state passes, but Postgres
transiently refuses the subsequent slot query.
Fix: fold all database access inside the try/except block so any
OperationalError from any call is retried uniformly for up to retry_timeout
seconds. Also removes the unused 'hostname' local variable.
* tests: pgaftest summary, drop-node coverage, CI image caching, pg_autoctl node init
Summary table (pgaftest)
After compose down completes, pgaftest prints a pg_regress-style per-step
table to stderr showing timing and result for every step:
test test_003_create_t1 ... ok ( 432 ms)
test test_022_detect_network_partition ... FAILED ( 31204 ms)
Timing is measured with clock_gettime(CLOCK_MONOTONIC) around each
runner_exec_step() call. The table appears after teardown{} (which
includes compose down) so it does not interleave with live logging.
On fail-fast runs only the completed steps appear.
pg_autoctl drop node --destroy / --force coverage
New spec tests/tap/specs/drop_node_destroy.pgaf exercises:
- pg_autoctl drop node --no-wait (PGDATA kept)
- pg_autoctl drop node --destroy (PGDATA removed)
- pg_autoctl drop node --destroy --force (no monitor; PGDATA removed)
Added to multi-misc.sch schedule.
pg_autoctl node init
New subcommand in cli_node.c that runs pg_autoctl create <kind> without
--run, initialising a node's PGDATA without starting the supervisor.
Useful in Dockerfile build stages to pre-bake initdb into image layers.
Idempotent: exits 0 if PGDATA is already initialised.
CI image caching (run-pgaftest.yml)
Switch docker build to docker buildx build with --cache-from/--cache-to
type=gha per version so that the apt-heavy base/citus layers are reused
across CI runs when the Dockerfile hasn't changed (saves 3-6 min per
PG version per build job).
Prebuilt monitor image (Dockerfile + compose_gen.c)
New Dockerfile target 'prebuilt' extends 'run' by running pg_autoctl
node init for the monitor, baking initdb into a named layer. The
compose generator picks PGAF_PREBUILT_IMAGE for the monitor service so
pg_autoctl node run skips initdb and starts immediately. Uploaded as
a separate artifact and passed as PGAF_PREBUILT_IMAGE in test jobs.
* cleanup: remove accidentally staged run-test.sh and pycache files
* tests: simplify Docker command to pg_autoctl node run, unify init/run paths
- All containers now use command: ["pg_autoctl", "node", "run",
"/etc/pgaf/node.ini"] — no shell wrapper, no PID-cleanup or SSL-copy
baked into the compose command string.
- cli_node_run() now handles everything internally:
· deletes stale PID file before exec
· copies SSL certs (verify-ca/verify-full) via node_copy_ssl_certs()
· runs pg_createcluster for debian_cluster nodes (cold start)
· cold start: fork+exec pg_autoctl create <kind>, waitpid, then exec
pg_autoctl run — both paths now end with the same exec-run sequence
· warm start: nodespec_apply() then exec pg_autoctl run (unchanged)
- cli_node_init() shares the same init logic via node_do_init(); used
standalone for Dockerfile RUN layers and interactive pre-init.
- NodeSpec gains debian_cluster field; parsed from [options] section of
the node.ini file; no change to compose_gen node.ini generation.
- Remove Dockerfile prebuilt, debian, testrun stages — all nodes use the
single run image. pg_createcluster now runs inside pg_autoctl node run
for debian-cluster nodes, removing the need for a separate image.
- Remove CI workflow steps for prebuilt/debian image build, upload,
download, and load. Remove separate installcheck job (make installcheck
now gates the build stage itself).
- pgaftest summary now matches pg_regress TAP format:
ok N - step_name NNN ms
1..N
# All N tests passed.
* fix: restore \n in compose_gen command/healthcheck stanzas (\\→\n)
* pgaftest: two-gate deferred (createDeferred + launchDeferred), TAP fix, new spec
Implement create-deferred as a second gate alongside the existing
launch-deferred (run-deferred) flag. Each axis is independent:
launch deferred → create immediate, run deferred
create deferred → create deferred, run immediate
create and launch deferred → both gates held
node.ini [launch] section now writes two keys:
create = deferred|immediate
mode = deferred|immediate
Grammar additions (test_spec_scan.l, test_spec_parse.y):
- T_CREATE token added to CLUSTER_BODY scanner state
- T_AND added to CLUSTER_BODY (was STEP_BODY only)
- bare "deferred" now sets both createDeferred + launchDeferred
- "launch deferred" = run-deferred only (unchanged semantics)
- "create deferred" = create-deferred only
- "create and launch deferred" = both gates
cli_node_run() ordering fixed:
1. PID cleanup
2. SSL cert copy (always immediate)
3. createDeferred poll loop
4. pg_createcluster + node_do_init (cold start)
5. launchDeferred poll loop
6. setenv + execv pg_autoctl run
cli_node_start() now clears both createDeferred and launchDeferred.
Rename 6 specs from "launch deferred" to "create and launch deferred"
since those nodes were always intended to hold both gates.
Add launch_deferred_set_metadata.pgaf: ports test_basic_operation.py
pattern — create-immediate/run-deferred node registers with monitor,
test sets node metadata while waiting at run gate, pg_autoctl node
start releases it, cluster reaches primary+secondary.
TAP output: remove pre-teardown tap_plan() that duplicated the summary
printed by runner_print_summary() after docker compose down.
compose_gen.c: volume ro/rw mode and healthcheck depends_on conditions
updated to check createDeferred || launchDeferred.
* pgaftest: fix debian cluster volume mount and pg_createcluster invocation
Two bugs prevented debian_clusters.pgaf from working:
1. compose_gen.c: debian-cluster nodes had their named volume mounted at
/var/lib/postgres (the standard pg_autoctl data home), but the Debian
cluster PGDATA lives at /var/lib/postgresql/<ver>/<name>. The cluster
data was being written to the ephemeral container layer rather than the
named volume, so it was never persisted. Fix: mount the volume at
/var/lib/postgresql for nodes that have debianCluster set.
2. cli_node.c: pg_createcluster was invoked with --user docker --group
postgres, but those flags change cluster ownership without actually
granting the docker user access (pg_createcluster still creates the
PGDATA directory with mode 700, owned by postgres:postgres). Fix:
drop the --user/--group flags and let pg_createcluster use its
defaults (postgres:postgres ownership, which is what a real Debian
install produces), then follow up with sudo chmod -R g+rwX on PGDATA
so the docker user (a member of the postgres group) can run
pg_autoctl create postgres against it.
* nodespec: rename [launch] mode key to run
The ini key name now matches the command it gates: pg_autoctl node run.
Create and run are now symmetric:
[launch]
create = deferred # hold before pg_autoctl create
run = deferred # hold before pg_autoctl run
No behaviour change — only the ini key string changed from 'mode' to 'run'.
* pgaftest: TAP spec header, drop_node_destroy sequence, debian backup dir fix
runner_print_summary: emit '# <specfile.pgaf>' as the first line of the
TAP output block so the reader immediately knows which spec produced it.
drop_node_destroy.pgaf: add the missing sequence block listing all 9
steps in order. Without it the runner auto-sequences from the linked
list of step definitions, which produced only the first 2 in one run.
cli_node.c (debian cluster): extend the post-pg_createcluster chmod to
the version directory (/var/lib/postgresql/17/) instead of just the
cluster PGDATA (/var/lib/postgresql/17/main). pg_autoctl creates a
sibling backup directory (/var/lib/postgresql/17/backup/node_N/) during
registration; with chmod scoped only to PGDATA that directory was owned
by postgres:postgres with no group write, causing 'Permission denied'
and a registration retry loop that never converged.
* cli_node: use initdb --allow-group-access for Debian clusters
Pass --allow-group-access through pg_createcluster to initdb instead of
running a separate sudo chmod -R g+rwX after the fact.
initdb sets PGDATA to mode 750 and data files to 640/750 atomically at
cluster creation time. The docker user (member of the postgres group via
adduser docker postgres in the Dockerfile) can then read and write the
cluster without any post-creation chmod step.
The sibling backup directory that pg_autoctl creates during registration
(/var/lib/postgresql/<ver>/backup/) lives under the version directory
which pg_createcluster already creates with mode 0755, so group members
can write there without special handling.
Removes one fork/waitpid pair and the pgverdir path-stripping logic.
* monitor: post-init formation service; fix createDeferred buf; nonha_citus_operation
Three related changes:
1. Refactor post-init formation creation (service_monitor.c, cli_node.c)
The original approach forked the post-init child in cli_node_run (before
execv) and hand-off its PID via PG_AUTOCTL_POST_INIT_PID for the
supervisor to adopt. The cleaner approach: start_monitor() reads the node
spec from PG_AUTOCTL_NODESPEC (already set before execv) and directly adds
a RP_TEMPORARY 'post-init' service whose startFunction forks the child.
The supervisor owns it from the start, no env var PID hand-off needed.
2. Fix off-by-one in createDeferredStr buffer (nodespec.c)
char createDeferredStr[8] was one byte too small for 'deferred' (8 chars +
null = 9 bytes), causing the last character to be cut off so
strcmp(createDeferredStr, 'deferred') always returned non-zero and the
createDeferred flag was never set. All deferred nodes were proceeding
immediately to pg_autoctl create, racing with each other.
Fix: widen the buffer to 16.
3. nonha_citus_operation.pgaf: secondary false + DSL changes
- Add 'secondary false' keyword to the formation DSL (test_spec_scan.l,
test_spec_parse.y, and regenerated test_spec_parse.c/h test_spec_scan.c)
- compose_gen: auto-derive kind=citus from coordinator/worker node types;
emit secondary=false when disableSecondary is set
- nodespec.c: parse secondary=false in [formation name] sections
- nodespec.h, monitor_config.h: add formationDisableSecondary arrays
- monitor.c: treat 42P17 (formation not found) as a retryable registration
error so keepers retry until the post-init service creates the formation
- nonha_citus_operation.pgaf: use 'formation non-ha secondary false { }'
Tested: pgaftest run tests/tap/specs/nonha_citus_operation.pgaf — all 11 tests pass.
* docker: add shared base image with all PG versions and Citus builds
Introduce Dockerfile.base that builds a single long-lived image containing:
- All supported Postgres major versions (14–19) from apt.postgresql.org
- One Citus build per Postgres version (CITUSTAG_<N> ARGs, default tags
baked in, overridable for new releases)
- All build tools needed to compile pg_auto_failover and pgaftest
Per-PG pg_auto_failover builds (Dockerfile) now use FROM ${BASE} so the
heavy apt/Citus work is in ghcr.io rather than rebuilt per PR via GHA cache.
New GitHub workflows:
base-image.yml:
Builds and pushes the base image to ghcr.io. Triggered by:
- push on main when Dockerfile.base changes
- workflow_dispatch (manual, with optional force_rebuild)
- workflow_call from run-pgaftest.yml (pull-or-build on cache miss)
check-versions.yml:
Weekly cron (Monday 06:00 UTC) that:
- Inspects org.pgaf.citustag.<N> labels on the current base image
- Queries GitHub Releases API for latest tag in each Citus series
- Runs apt-cache inside the base image to detect newer PGDG packages
- Triggers a forced base image rebuild if any version has changed
run-pgaftest.yml changes:
- Adds ensure-base job (calls base-image.yml) between style_checker
and build-images; fast path is a manifest check (~5 s)
- build-images and build-pgaftest pass BASE=ghcr.io/.../pgaf-base:bookworm
and no longer pass CITUSTAG (that lives in the base image)
- GHA layer cache now only covers the pg_auto_failover compilation layer
* node post-init: proper supervisor service via pg_autoctl node post-init
Replace the inline fork+loop+waitpid implementation in service_post_init_start()
with a simple fork+execv into the new 'pg_autoctl node post-init' command.
pg_autoctl node post-init:
- Reads the node spec from PG_AUTOCTL_NODESPEC (set before execv by node run)
or falls back to deriving the spec path from --pgdata
- Reads the MonitorConfig from <pgdata>/pg_autoctl.cfg to get a fully
populated PostgresSetup (port, socket dir, etc.)
- Calls pg_setup_wait_until_is_ready() with a 120 s timeout before touching
the monitor, so it never races against Postgres startup
- Creates each [formation <name>] section via run_program() calls to
'pg_autoctl create formation', consistent with the rest of the codebase
- Exits 0 on success; the RP_TEMPORARY supervisor policy reaps it cleanly
service_post_init_start():
- Reduced to 10 lines: fork + execv + setsid; all logic moved to the command
- Removed sys/wait.h (no longer calls waitpid)
nodespec_apply(): formation support for monitor nodes
- When the spec file watcher detects a change on a monitor node (kind ==
NODE_KIND_UNKNOWN), apply formation-level changes live:
* New [formation <name>] sections → pg_autoctl create formation
* Changed secondary= setting → pg_autoctl enable/disable secondary
- Formation kind changes are not applied live (immutable after creation)
* ci: merge into single workflow; fix BASE org and BINDIR
Org name fix (citusdata → hapostgres):
- Dockerfile: ghcr.io/hapostgres/pg_auto_failover/pgaf-base:bookworm
- base-image.yml: same
- check-versions.yml: same
Dockerfile: BINDIR=/usr/local/bin on make install
pg_config --bindir returns /usr/lib/postgresql/<N>/bin which is the
wrong install prefix for pg_autoctl and pgaftest; override BINDIR so
both binaries land in /usr/local/bin as the COPY instructions expect.
Also update the pgaftest COPY to source from /usr/local/bin (not the
old pg_config --bindir path).
Makefile: adapt build targets to new Dockerfile
- Drop CITUSTAG vars (now baked into the base image)
- Add BASE variable (default ghcr.io/hapostgres/…/pgaf-base:bookworm)
- All BUILD_ARGS_pg* now pass --build-arg BASE=$(BASE)
- build-pg* targets add --target run (was the implicit default before)
- Remove stale build-image / build-demo targets
- Remove CITUSTAG ?= (it lives in Dockerfile.base now)
GitHub workflows: merge run-tests.yml + run-pgaftest.yml → ci.yml
Single workflow with this dependency graph:
style_checker
└── ensure_base (base-image.yml reusable workflow)
├── build_run_images (PG 14-19 → pgaf:run-pgN)
│ └── test_pgaftest (21 spec jobs)
├── build_pgaftest (pgaf:pgaftest)
│ └── test_pgaftest + upgrade
└── build_test_images (PG 14-19 → pg_auto_failover_test:pgN)
└── test_python (PG × {multi,single,monitor,ssl,citus})
* build: refactor Makefile and Docker infrastructure
- Split Makefile into Makefile (core) + Makefile.docker (shared Docker
targets, included by both top-level and tests/Makefile) +
Makefile.installcheck (runs inside base image via pg_virtualenv)
- Add tests/Makefile with all pytest targets; remove pytest targets from
top-level Makefile
- Drop PG18/19 everywhere; default to PG17 (pg_auto_failover not yet
supported on those versions)
- Remove Makefile.citus (TESTS_CITUS folded into tests/Makefile)
- Remove tmux/cluster/compose/valgrind Make targets (replaced by pgaftest)
- Rename build-test-pg* -> build-pytest-pg* (--target test is pytest only)
- Add build-pgaftest target in Makefile.docker
- Add installcheck to Dockerfile build stage via pg_virtualenv; drop
separate installcheck CI job (it now runs inside every build_run_images job)
- Remove debian Dockerfile stage: pg_autoctl node run already calls
pg_createcluster when debian_cluster is set in node.ini
- compose_gen.c: remove PGAF_DEBIAN_IMAGE / debian image special case;
forward PGVERSION as build-arg in inline build stanza so docker uses
the right PG version when PGAF_IMAGE is not set
- CI: drop debian image build/save/upload/download steps; drop docker tag
alias (pgaf:run -> pgaf:run-pgN); pass PGAF_IMAGE=pgaf:run-pg$N directly
- Dockerfile.base: add libcurl4-gnutls-dev (Citus build dep), silence pip
root warning, restrict PGDG source to PG14-17
* style: exclude bison/flex generated files from citus-style; replace fprintf with fformat in test_runner.c
* fix: scope Citus build cd in subshell to avoid cwd-deleted error on rm -rf
* fix: three CI failures from logs analysis
- monitor.destroy() was re-raising on 'drop monitor' failure, poisoning
the last test when monitor died before teardown_module() ran; drop raise
- tests/tablespaces/Makefile still called build-test-pgN (old name);
rename to build-pytest-pgN
- upgrade job had no docker/login-action so 'make pgaf-next' failed with
401 Unauthorized pulling ghcr.io/…/pgaf-base:bookworm
* build: move version target to Makefile.docker; fix run-test target name
* ci: run style_checker and ensure_base in parallel; fix test_spec_parse.h style exclusion
* Fix CI failures: debian_clusters, SSL certs, teardown, grammar, deferred launch
debian_clusters: restore --user docker --group postgres to pg_createcluster
pg_createcluster without --user docker creates PGDATA owned by postgres:postgres
with mode 750. The container user (docker) is in group postgres so has group
r-x but NOT write access, causing pg_autoctl create postgres to fail when
creating files inside PGDATA. Restore --user docker so docker owns PGDATA.
compose_gen: emit shell-wrapped command for SSL specs (SSL_COPY_CERTS_CMD)
SSL_COPY_CERTS_CMD was defined but never used. Certs were bind-mounted
read-only at /etc/pgaf/ssl/ but postgres needs them in the writable
/var/lib/postgres/ volume. Add write_node_command() helper that emits
a /bin/sh -c wrapper for any spec that uses ssl_needs_certs.
test_runner: always run teardown{} block, even when setup fails
The runner returned false immediately after setup failure without calling
the teardown block, leaving docker stacks running. Call teardown even
on setup failure so containers are always cleaned up.
test_spec: add T_TRUE token; accept T_REPLICATION_QUORUM T_TRUE | T_FALSE
'replication-quorum false' was a parse error because 'false' is lexed as
T_FALSE in CLUSTER_BODY state but the grammar only accepted T_IDENT.
Add T_TRUE token and update the grammar to accept both boolean tokens.
Regenerate bison/flex files using the base image toolchain (Bison 3.8.2).
Dockerfile: remove Makefile.installcheck from upgrade build COPY line
The upgrade build uses 'git archive v2.1' which predates that file; the
COPY line failed with 'not found'.
tests/Makefile: fix run-test and tablespaces prebuilt targets
launch_deferred_set_metadata spec: fix race in test_000 and test_001
node2 starts after node1 is healthy (depends_on: service_healthy) but
pg_autoctl create postgres takes several seconds. The old test_000
waited for node1=single (already satisfied) and test_001 immediately
queried the monitor before node2 had registered. Fix: test_000 now
waits for node1=wait_primary, which the monitor transitions to only
after node2 registers -- a reliable signal that node2 is in the node
table.
test_runner: use nodehost in psql fallback for monitor node-state lookup
The psql fallback queried pgautofailover.node WHERE nodename='<service>'.
After 'pg_autoctl set node metadata --name node_b', nodename changes but
nodehost (the container TCP hostname) stays as 'node2'. Use nodehost so
wait_until works correctly after a node rename.
* Fix debian_clusters setup, SSL cert truncation, and upgrade Makefile
compose_gen.c: write debian_cluster under [options] section in generated
node.ini files. The nodespec parser reads it from [options] (line 130 of
nodespec.c), but the generated ini was writing it under [node]. As a
result spec.debianCluster was always empty, pg_createcluster was never
called, and node1 never initialized — causing the 60 s setup timeout in
the debian_clusters test.
cli_node.c: guard copy_file() against src==dst. node_copy_ssl_certs()
copies spec->ssl_cert_file to $HOME/server.crt. When ssl_cert_file is
already at /var/lib/postgres/server.crt and HOME is /var/lib/postgres, the
source and destination paths are identical; the old code opened the
destination with O_TRUNC first, zeroing the file before reading from it.
tests/upgrade/Makefile: copy Makefile.docker to the temp dir before
building pgaf:current. Makefile.docker was added after the v2.1 tag, so
git archive v2.1 does not include it, but the current Dockerfile's
COPY Makefile Makefile.docker ./ requires it.
* debian_clusters: chown version dir after pg_createcluster
pg_createcluster creates /var/lib/postgresql/<ver>/ owned by the postgres
system user with mode 755. pg_autoctl (running as the docker user, which
is in group postgres) subsequently tries to create a backup directory at
/var/lib/postgresql/<ver>/backup/node_N; that fails with EACCES because
the group does not have write permission on the version directory.
Fix: after pg_createcluster exits successfully, run
sudo chown docker /var/lib/postgresql/<ver>
so that the docker user owns the version directory and can create
subdirectories inside it.
* refactor: move pg_createcluster test setup into debian.c
Extract the Debian-specific pg_createcluster + chown block from
cli_node.c into pg_createcluster_for_test() in src/bin/common/debian.c,
where the rest of the Debian-specific code already lives.
cli_node.c now just calls pg_createcluster_for_test() with a comment
that this is a pgaftest testing facility, not intended for production.
* fix: four pre-existing test failures
cli_drop_node.c: --no-wait ignored on monitor-side drop
When 'pg_autoctl drop node --name <n> --no-wait' is run from the
monitor, it went through cli_drop_node_from_monitor_and_wait() which
never checked dropNoWait and always blocked for up to 60s waiting for
the row to disappear. Add the same early-return guard that the local
path already had.
test_monitor_disabled: race on node2 config file not yet written
test_004 starts node2 in the background then returns immediately.
test_005 calls pg_autoctl inspect fsm state on node2 before it has
finished writing its config file, causing 'missing [pg_autoctl]
section'. Add wait_until_pg_autoctl_is_running() to PGNode (polls
for config_file_path()) and call it at the end of test_004.
test_multi_maintenance: cluster not settled before set_candidate_priority
test_004 returned immediately after DML without confirming all three
nodes were healthy. A monitor health-check gap during the CHECKPOINT
could trigger a failover that left node1 non-primary when test_005's
set_candidate_priority (which blocks for 60s) ran. Add the three
wait_until_state assertions at the end of test_004.
test_multi_standbys test_015_003: apply_settings cycle not waited for
After set_formation_number_sync_standbys via SQL, the test used
wait_until_assigned_state(primary) which returned instantly because
the assigned state was already primary. The apply_settings → primary
cycle that actually rewrites synchronous_standby_names had not run
yet. Switch to wait_until_state(apply_settings) + wait_until_state(
primary) which checks reportedstate and therefore blocks until the
cycle completes.
* fix: drop_node_destroy test suite and test_015_003 regression
pgaftest: add 'run' command (compose run --rm) for stopped containers
docker compose exec requires a running container; after 'pg_autoctl stop'
the container exits. Add CMD_RUN / T_RUN / 'run <svc> <args>' which
uses 'docker compose run --rm' instead, starting a fresh container with
the same volumes. Used in drop_node_destroy for the --destroy steps
where the supervisor must be stopped before calling drop node --destroy.
drop_node_destroy.pgaf: restructure all 9 steps to work correctly
- test_003/004: drop while running (keeper self-exits on next heartbeat),
verify PGDATA while container is still up, then wait for stop
- test_005/006: stop supervisor first (container exits), then 'run --rm'
for the drop --destroy command and the PGDATA-absent check
- test_007/008/009: same pattern for --destroy --force
test_multi_standbys test_015_003: correct wait after set_number_sync_standbys
The earlier fix waited for apply_settings AND primary. But with both
standbys failed, fsm_enable_sync_rep() writes synchronous_standby_names
to postgresql.conf (via fsm_apply_settings) then returns false because
no standby is in quorum sync state. Node1 is stuck in apply_settings
until test_015_004 restarts the standbys. Only wait for apply_settings,
which confirms the SSN was written; primary comes in test_015_004.
* Fix debian_clusters: /usr/bin/sudo path + single [options] INI section
initialize_program() sets prog->program = args[0] verbatim — no PATH
lookup — so access("sudo", ...) fails with ENOENT. Use the absolute
path /usr/bin/sudo in both pg_createcluster and chown calls.
Also fix compose_gen.c: the debian_cluster ini entry was written in a
separate [options] block before the ssl/auth/pg_hba_lan block, producing
two [options] sections. Move debian_cluster = %s into the existing
combined [options] block so the generated node.ini has a single section.
* fix: upgrade build, login-action Node 24, Dockerfile Makefile* glob
- Dockerfile: COPY Makefile* ./ instead of explicit Makefile Makefile.docker
so that old source archives (e.g. v2.1 with Makefile.azure) work with the
current Dockerfile without modification
- tests/upgrade/Makefile: drop the cp Makefile.docker override — the old
build system from the PREV_TAG archive is used as-is, which is correct
since build dependencies have not changed across releases
- ci.yml: pin docker/login-action to v3.4.0 (first release built against
Node.js 24, eliminating the Node.js 20 deprecation warning)
* Fix CI warnings, citus_skip_pg_hba failure, and add log capture
Actions warnings:
- docker/login-action@v3 → v3.4.0 (Node.js 24)
- docker/setup-buildx-action@v3 → v3.10.0 (Node.js 24)
- Remove deprecated 'install: true' from setup-buildx-action stanzas
pytest monitor PG17 flaky race (test_003_002_stop_primary):
- The keeper uses LISTEN/NOTIFY (monitor_wait_for_state_change) when the
monitor is enabled, so state changes from the monitor wake the keeper
immediately — no polling delay.
- The real fix is accepting draining or demote_timeout: the monitor moves
node2 from draining to demote_timeout so fast that a 100ms Python poll
can miss draining entirely.
citus_skip_pg_hba.pgaf / test_002d_run_worker:
- Root cause: pg_autoctl create citus-worker --auth skip fails when the
coordinator cannot call master_add_node (pg_hba blocks), which caused
cli_node_run to exit the container even though pg_autoctl.cfg was written
- Fix: use XDG config path (not PGDATA/pg_autoctl.cfg) for warm-start
detection; when node_do_init fails but config was written, fall through
to exec pg_autoctl run so the keeper can retry activation after HBA fix
- The spec's pgsetup wait + hba-lan + wait-for-single flow is now correct
pgaftest log capture on failure:
- Emit docker compose logs for all containers when a step or setup block
fails, before teardown destroys the evidence
* Fix CI warnings and tablespaces prebuilt path
Actions Node.js 20 deprecation warnings:
- docker/login-action@v3/v3.4.0 → v4.4.0 (Node.js 24) in ci.yml and base-image.yml
- docker/setup-buildx-action@v3/v3.10.0 → v4.2.0 (Node.js 24) in ci.yml and base-image.yml
- Remove deprecated 'install: true' from setup-buildx-action in base-image.yml
pytest / tablespaces (PG14) — 401 Unauthorized on ghcr.io:
- run-test-prebuilt for TEST=tablespaces was calling 'make test teardown' in
tablespaces/Makefile, whose 'test' target depends on 'build', which calls
'build-pytest-pg14' — rebuilding the image from scratch via a docker build
that tries to pull the ghcr.io base image without a login step
- Fix: add 'run-test-prebuilt' target to tablespaces/Makefile that only runs
'docker compose build' (FROMs the already-loaded local image) plus the test
steps plus teardown, skipping the build-pytest-pgN rebuild entirely
- Update tests/Makefile to call 'make -C tablespaces run-test-prebuilt'
* Fix config write race: use temp-file + rename for atomic writes
keeper_config_write_file and monitor_config_write_file both opened the
config file with fopen(..., "w"), which truncates the file to 0 bytes
immediately and then writes the new content. Any concurrent reader
(e.g. pg_autoctl inspect pgsetup wait) that ran between the truncation
and fclose would see an empty file, fail to find the [pg_autoctl] section,
and exit non-zero.
This caused the pytest/ssl test_006_enable_ssl_primary failure:
stop_pg_autoctl() -- process exits
enable_ssl() -- pg_autoctl enable ssl rewrites the config
run() -- starts keeper in the background; keeper writes
the config early in its init (truncates it)
wait_until_pg_is_running() -- pgsetup wait reads the config and sees
an empty file during the truncation window
Fix: write to <path>.tmp first, then rename() to the final path.
rename() is atomic on POSIX -- readers always see either the old complete
file or the new complete file, never an empty file.
* Fix Monitor.failover() race: wait for stable primary before perform_failover
perform_failover's GetNodeToFailoverFromInGroup requires both reportedstate
and goalstate to equal an eligible primary state (primary/single/join_primary).
wait_until_state("primary") only polls reportedstate — there is a window where
the keeper just reported primary but the monitor's goalstate column has not yet
been updated to match.
Fix: before calling perform_failover, poll pgautofailover.node until at least
one node satisfies reportedstate = goalstate IN ('primary','single','join_primary'),
mirroring the exact C predicate in GetNodeToFailoverFromInGroup.
* Fix test_003_002_stop_primary race: check assigned state not reported state
When node3 is the sole remaining standby after node2 fails, the monitor
assigns report_lsn then immediately assigns prepare_promotion in the same
node_active() transaction (both events share the same millisecond timestamp).
node3's keeper never reports reportedstate=report_lsn because it transitions
to prepare_promotion before the next poll round.
wait_until_state() polls reportedstate — it never observes report_lsn.
Fix: use wait_until_assigned_state(target_state='report_lsn',
or_state='prepare_promotion') to match the goalstate column instead,
mirroring the draining/demote_timeout fix already applied for node2 in 89ff563.1 parent e975832 commit ebaf5fa
56 files changed
Lines changed: 8244 additions & 7943 deletions
File tree
- .github/workflows
- src/bin
- common
- pg_autoctl
- pgaftest
- tests
- tablespaces
- tap
- schedules
- specs
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
0 commit comments