Skip to content

feat: add pgaftest binary, tutorial specs, and CI workflow - #1137

Merged
dimitri merged 1 commit into
mainfrom
pgaftest/binary-and-docs
Jul 9, 2026
Merged

feat: add pgaftest binary, tutorial specs, and CI workflow#1137
dimitri merged 1 commit into
mainfrom
pgaftest/binary-and-docs

Conversation

@dimitri

@dimitri dimitri commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Introduces pgaftest, a purpose-built test runner that reads .pgaf spec files, generates docker-compose.yml from the cluster{} block, drives the cluster lifecycle, and emits TAP output for CI.

Depends on #1136 (src/bin/common/ shared library) — already merged.


src/bin/pgaftest/

New binary linked against libpgaf_common.a and the pg_autoctl/ business-logic sources (compiled as shared-*.o):

File Purpose
main.c entry point
cli_root.c sub-commands: run / setup / step / down / demo / indent
cli_demo.c/h demo app (moved from pg_autoctl do demo)
cli_indent.c/h pgaftest indent — re-formats .pgaf files
compose_gen.c/h generate docker-compose.yml from cluster{} block
test_runner.c/h headless CI runner + interactive setup + step/down
test_spec.h AST node structs
test_spec_parse.y Bison grammar
test_spec_scan.l Flex lexer
test_spec_parse.c/h, test_spec_scan.c pre-generated parser — regenerate with make generate

Interactive tmux mode

pgaftest setup spec.pgaf --tmux replaces pg_autoctl do tmux compose session: generates compose from the spec's cluster{} block and starts a tmux session with per-node panes.

src/bin/Makefile

all: common pg_autoctl pgaftest

Dockerfile

  • Bison touch guard — touches pre-generated parser .c/.h so make does not re-run bison; guarded with -d for older release tarballs
  • New pgaftest target stage — Bookworm-slim with docker-ce-cli, docker-compose-plugin, libpq5, and the pgaftest binary

docs/

  • docs/ref/pgaftest.rst — manual page covering all sub-commands with --tmux alternative
  • docs/tutorial/interactive_tutorial.pgaf — basic two-node tutorial as a .pgaf spec
  • docs/tutorial/citus_tutorial.pgaf — Citus coordinator + worker variant
  • docs/ref/manual.rstpgaftest added to Manual Pages toctree

.github/workflows/run-pgaftest.yml

Builds the pgaftest Docker image once, fans out one job per .pgaf spec in parallel.

Testing

  • Builds cleanly locally
  • Style check passes
  • test_basic_operation — 32 passed (pg17, Docker)
  • test_multi_async — passed (pg17, Docker)

@dimitri dimitri self-assigned this Jul 9, 2026
@dimitri dimitri added enhancement New feature or request Developer productivity Enhancements to ability to ship quality code Packaging and CI Enhancements to our CI integration labels Jul 9, 2026
pgaftest is a purpose-built test runner that reads .pgaf spec files,
generates a docker-compose.yml from the cluster{} block, drives the
cluster lifecycle, and emits TAP output for CI.

New binary linked against libpgaf_common.a (src/bin/common/) and the
pg_autoctl/ business-logic sources (compiled as shared-*.o):

  main.c            — entry point
  cli_root.c        — sub-command table: run / setup / step / down / demo
  cli_demo.c/h      — demo app (moved from pg_autoctl do demo)
  cli_indent.c/h    — pgaftest indent sub-command (re-formats .pgaf files)
  compose_gen.c/h   — generate docker-compose.yml from cluster{} block
  test_runner.c/h   — headless CI runner + interactive setup + step/down
  test_spec.h       — AST node structs
  test_spec_parse.y — Bison grammar
  test_spec_scan.l  — Flex lexer
  test_spec_parse.c/h, test_spec_scan.c — pre-generated parser (avoids
    bison/flex version skew in Docker); regenerate with: make generate

Updated to build common → pg_autoctl → pgaftest in order.

  - Bison touch guard: touches pre-generated parser .c/.h files so make
    does not re-run bison (system bison version may differ from the one
    used to commit the files); guarded by -d for compatibility with older
    release tarballs that do not include src/bin/pgaftest/.
  - New pgaftest target stage: Bookworm-slim image with docker-ce-cli,
    docker-compose-plugin, libpq5, and the pgaftest binary — no Postgres
    server, no test Python packages, no leakage of build tooling.

  docs/ref/pgaftest.rst         — manual page: all sub-commands with
    usage examples; includes --tmux alternative to pg_autoctl do tmux
  docs/tutorial/interactive_tutorial.pgaf — basic two-node tutorial
    expressed as a .pgaf spec; run with: pgaftest run or pgaftest setup
  docs/tutorial/citus_tutorial.pgaf — Citus coordinator + worker variant
  docs/ref/manual.rst           — pgaftest.rst added to Manual Pages toctree

CI workflow that builds the pgaftest Docker image once, then fans out
one job per .pgaf spec in the tests/specs/ directory in parallel.
@dimitri
dimitri force-pushed the pgaftest/binary-and-docs branch from cb4dfa1 to 3a88347 Compare July 9, 2026 15:43
@dimitri
dimitri merged commit a4fce35 into main Jul 9, 2026
26 checks passed
@dimitri
dimitri deleted the pgaftest/binary-and-docs branch July 9, 2026 16:45
dimitri added a commit that referenced this pull request Jul 9, 2026
Introduces the full pgaftest test suite under tests/tap/ and the GitHub
Actions workflow that runs it.  Depends on the pgaftest binary (#1137).

## tests/tap/specs/

32 .pgaf specs covering the complete test matrix previously expressed
as Python/nosetests files.  Each spec encodes topology (cluster{}),
setup/teardown, and an ordered step sequence.

Single-node / basic:
  basic_operation, basic_operation_listen_flag, create_run (via ensure),
  create_standby_with_pgdata, config_get_set, skip_pg_hba, auth,
  enable_ssl, ssl_cert, ssl_self_signed, ensure, maintenance_and_drop

Monitor:
  monitor_disabled, replace_monitor, extension_update, installcheck

Multi-standby:
  multi_standbys, multi_alternate, multi_async, multi_maintenance,
  multi_ifdown

Citus:
  basic_citus_operation, nonha_citus_operation, citus_multi_standbys,
  citus_cluster_name, citus_force_failover, citus_skip_pg_hba

Platform / upgrade:
  debian_clusters, tablespaces, upgrade

## tests/tap/schedule

Lists all specs in the run order used by the CI matrix.

## tests/tap/README.md

Documents the .pgaf DSL, how to run specs locally with pgaftest, and
how to add new specs.

## .github/workflows/run-pgaftest.yml

Builds the pgaftest Docker image once (FROM Dockerfile pgaftest target),
then fans out one job per entry in tests/tap/schedule in parallel.
Each job mounts the spec file, runs pgaftest run, and reports TAP.
dimitri added a commit that referenced this pull request Jul 10, 2026
* tests: add pgaftest .pgaf spec suite and CI workflow

Introduces the full pgaftest test suite under tests/tap/ and the GitHub
Actions workflow that runs it.  Depends on the pgaftest binary (#1137).

## tests/tap/specs/

32 .pgaf specs covering the complete test matrix previously expressed
as Python/nosetests files.  Each spec encodes topology (cluster{}),
setup/teardown, and an ordered step sequence.

Single-node / basic:
  basic_operation, basic_operation_listen_flag, create_run (via ensure),
  create_standby_with_pgdata, config_get_set, skip_pg_hba, auth,
  enable_ssl, ssl_cert, ssl_self_signed, ensure, maintenance_and_drop

Monitor:
  monitor_disabled, replace_monitor, extension_update, installcheck

Multi-standby:
  multi_standbys, multi_alternate, multi_async, multi_maintenance,
  multi_ifdown

Citus:
  basic_citus_operation, nonha_citus_operation, citus_multi_standbys,
  citus_cluster_name, citus_force_failover, citus_skip_pg_hba

Platform / upgrade:
  debian_clusters, tablespaces, upgrade

## tests/tap/schedule

Lists all specs in the run order used by the CI matrix.

## tests/tap/README.md

Documents the .pgaf DSL, how to run specs locally with pgaftest, and
how to add new specs.

## .github/workflows/run-pgaftest.yml

Builds the pgaftest Docker image once (FROM Dockerfile pgaftest target),
then fans out one job per entry in tests/tap/schedule in parallel.
Each job mounts the spec file, runs pgaftest run, and reports TAP.

* fix: pgsetup wait PGDATA env, upgrade Makefile with pgaf-next/current targets

Two fixes found by analyzing CI failures on PR #1138:

1. pg_autoctl inspect pgsetup wait: picks up PGDATA from environment

   The previous keeper_cli_pgsetup_wait_getopts had its own minimal option
   parser that never called cli_common_get_set_pgdata_or_exit, so
   keeperOptions.pgSetup.pgdata was empty when --pgdata was not given on
   the command line. cli_common_pgsetup_init then failed immediately with
   "BUG: keeper_config_set_pathnames_from_pgdata: empty pgdata".

   Fix: delegate to keeper_cli_keeper_setup_getopts (after stripping
   --timeout and --read-write) so that PGDATA is picked up from the
   environment. Also port the richer implementation from pgaftest-infra:
   config-file appearance wait, shared deadline, and --read-write phase.

2. tests/upgrade/Makefile: add pgaf-next / pgaf-current targets

   The CI workflow runs make -C tests/upgrade pgaf-next / pgaf-current
   but the old Makefile had no such targets (only build / up / down etc).
   Replace with the new Makefile that auto-detects PREV_TAG from git tags
   and defaults.h, and add the companion files it needs:
   Dockerfile.current, pg_autoctl_shim.sh, install-extension.sh.

* Fix CI failures: drop --no-wait early exit, formation support, version-agnostic paths

pg_autoctl drop node --no-wait (exit 35072):
- Add early exit in cli_drop_local_node() after cli_drop_node_from_monitor()
  when config->listen_notifications_timeout == 0 (i.e. --no-wait).  Without
  this the code falls through into the supervisor-wait loop where the process
  gets SIGKILLed (exit 137 = raw waitpid 35072) before the supervisor exits.

pg_autoctl create monitor --formation <name>:
- Restore MonitorConfig.formationCount / formationNames / formationKinds
  fields and MONITOR_MAX_FORMATIONS removed in a previous refactor.
- Restore --formation and --autoctl-node-password options in
  cli_create_monitor_getopts() so nonha_citus_operation and similar specs
  that declare named formations actually create them.
- Restore monitor_install() autoctl_node_password parameter and the
  ALTER ROLE ... PASSWORD call after CREATE EXTENSION.
- Restore the post-init formation-creation loop in service_monitor_init_start()
  that calls monitor_create_formation() for each --formation argument.

Spec fixes (version-agnostic paths, transient-state races):
- debian_clusters.pgaf: use ${PGDATA}/postgresql.conf instead of hardcoded
  /var/lib/postgresql/17/main/postgresql.conf so PG14-16 CI jobs pass.
- config_get_set.pgaf: expect /bin/pg_ctl (substring) instead of full
  /usr/lib/postgresql/17/bin/pg_ctl path; also sync ssl.sslmode test from
  reference branch.
- ensure.pgaf, multi_alternate.pgaf: skip waiting for the transient 'demoted'
  state (sub-second, races on loaded CI runners); wait for the stable
  secondary+primary end state with a 300s timeout instead.
- multi_ifdown.pgaf: replace the unreliable test_014_secondary_reports_lsn
  (compose stop + network connect + blocking perform failover) with the
  reference design: stop node3+node1 cleanly, reconnect node2, non-blocking
  failover, wait for node2 at report_lsn.  Add test_015_start_node3_node1
  step so node2 can pg_rewind missing WAL from the survivors.

* Sync with pgaftest-infra-combined: restore all reference-branch content

Full sync of pgaftest/test-suite to match pgaftest-infra-combined exactly.
Restores all spec, C-source, and build-file differences that had drifted
since the branch was split:

Spec fixes (transient-state races, version-agnostic paths, test redesigns):
- basic_operation.pgaf: wait for stable end state around demote_timeout;
  use 'assert stays primary while { stop postgres monitor }' pattern;
  longer CI-safe timeouts
- auth.pgaf, enable_ssl.pgaf, ssl_cert.pgaf, upgrade.pgaf: minor alignment
- citus_skip_pg_hba.pgaf: coord0a no longer deferred; use
  'pg_autoctl override pgsetup hba-lan'; rewritten test sequence
- monitor_disabled.pgaf: inline JSON via shell printf instead of
  /etc/pgaf/specs/*.json bind-mount; remove companion JSON files
- multi_async.pgaf: reorder node4 disconnect around LSN election
- multi_maintenance.pgaf, multi_standbys.pgaf: remove redundant intermediate
  waits; adjust timeouts to match reference

C source (functional):
- cli_common.c: --monitor-password mapped to no-op 'w'; --replication-password
  to 'e'; matches option letters expected by pg_autoctl node run
- cli_create_node.c: remove --monitor-password / --replication-password from
  cli_create_postgres_getopts (handled via node.ini, not CLI)
- cli_do_misc.c, cli_do_root.c/.h: restore reference implementation of
  'pg_autoctl inspect/override pgsetup' sub-commands
- cli_drop_node.c: restore dropNoWait bool (reference style)
- cli_root.c, config.c, coordinator.c, demoapp.c, fsm.c, keeper.c,
  keeper_config.c/.h, keeper_pg_init.c, monitor.c, monitor_config.c,
  nodespec.c/.h, primary_standby.c, state.c, watch.c: variable renames,
  comment updates, and minor refactors aligned to reference
- ipaddr.c: minor fix from reference
- formation_metadata.c, node_metadata.c: minor monitor-side tweaks

Build / misc:
- Makefile, Dockerfile: realigned to reference versions
- Makefile.azure: removed (deleted in reference)
- .gitignore: restored reference entries
- docs/: restore citus-quickstart.rst, operations.rst, tutorial.rst
- tests/network.py: minor update
- tests/upgrade/install-extension.sh, pg_autoctl_shim.sh: file-mode sync

* Dockerfile: COPY Makefile.azure* so upgrade build from v2.1 archive succeeds

v2.1's Makefile includes Makefile.azure (line 37).  When the upgrade test
extracts the v2.1 git archive into a tmpdir and substitutes the current
Dockerfile, the build stage only COPYd Makefile and Makefile.citus — so
make failed with 'Makefile.azure: No such file or directory'.

Add a glob COPY so Makefile.azure is picked up when present in the build
context (v2.1 archive) and silently skipped when absent (current tree).

* test_runner: zero-init ms[] to silence -Wmaybe-uninitialized

GCC cannot prove the fill loop runs at least once, so it warns that ms
may be uninitialized when passed to runner_drain_notify.  Zero-init the
array to make the intent explicit and suppress the warning cleanly.

* test_runner: open LISTEN channel on Docker Desktop for Mac

On Docker Desktop for Mac, connections through published ports appear
to the container as 192.168.65.1 (the Docker Desktop VM gateway), which
is outside the Docker bridge CIDR that pg_autoctl adds to pg_hba when
--pg-hba-lan is set.  This caused the direct libpq LISTEN connection to
fail immediately, falling through to subprocess polling with the message
'LISTEN not available' — meaning no real-time state notifications and no
'* [notify]' convergence markers in the output.

When the subprocess readiness check confirms the monitor is up but LISTEN
has not connected yet, append 'host all all 0.0.0.0/0 trust' to pg_hba
and reload.  This is safe for local test containers that already run with
--auth trust.  Retry the LISTEN connection once after the reload; the
'LISTEN not available' fallback remains for environments where even that
fails.

* test_runner: fix '*' convergence markers and suppress exec output

Two related fixes:

1. Inter-command drain must not run before wait commands.

   runner_exec_step drained the libpq notify buffer before EVERY command
   using NULL mark arrays.  When a wait command (CMD_WAIT_STATE,
   CMD_WAIT_STATES, CMD_WAIT_MULTI) followed an exec that triggered state
   transitions, the convergence notifications were consumed without '*'
   marks, then the wait's own initial drain found nothing left — the '*'
   prefix was silently dropped.

   Skip the inter-command drain when the upcoming command is a wait.
   Each wait command already opens with its own drain that passes the
   correct mark arrays, so the '*' prefix is applied where it belongs.

2. Suppress pg_autoctl's own log output from stop/start postgres.

   CMD_STOP_POSTGRES and CMD_START_POSTGRES called 'docker compose exec'
   without -T and without output redirection, so pg_autoctl's internal
   log lines (with their own timestamp/PID) appeared on stdout interleaved
   with pgaftest's output.  Add -T and redirect to /dev/null.

* test_runner: capture stop/start postgres output, show only on error

Replace the /dev/null redirect with run_cmd_capture_both(), which
captures stdout and stderr in memory and shows the combined output only
when the command fails.  Normal successful runs stay quiet; error output
is available exactly where it's needed.

run_cmd_capture_both() appends '2>&1' to the shell command before
calling popen(cmd, "r").  The shell redirects fd 2 onto fd 1 before
exec'ing the child, so both streams arrive on the single pipe end that
popen returns.  We read until EOF, trim trailing whitespace, and drain
any overflow past the buffer so pclose() doesn't leave an unread pipe
(which would send SIGPIPE to the child and make docker report exit 137).

* test_runner: reliable '*' markers in CMD_WAIT_MULTI

Two-phase drain strategy for CMD_WAIT_MULTI:

1. Drain with marks immediately after each subprocess check, so
   notifications that accumulated during the ~200ms docker exec
   are consumed and marked before we test allMet.

2. When allMet (or allListenMet) turns true, loop wait_socket(200ms)
   until the socket goes quiet, draining with marks on each pass.
   This catches notifications that were sent by the monitor after the
   database commit (which the subprocess reads) but haven't yet
   traversed the Docker Desktop VM network layer to our libpq socket.

The race that caused missing '*' markers:
  - Monitor commits state change → sends NOTIFY → responds to node-active
  - Subprocess queries the database and sees the committed state
  - NOTIFY is still in transit through the VM network (~5-50ms on Mac)
  - Without the post-success drain loop, NOTIFY arrived after return
    and was consumed unmarked by the next command's inter-drain

The 200ms wait_socket loop terminates as soon as the socket is idle,
so in the common case (CI Linux where NOTIFY arrives before subprocess
returns) there is no extra latency; on Docker Desktop for Mac it waits
just long enough for the in-flight NOTIFY to arrive.

* test_runner: fix '*' markers for all nodes in CMD_WAIT_MULTI

Introduce notify_flush_until_satisfied() for the post-convergence
drain in CMD_WAIT_MULTI instead of a fixed-window 200ms wait loop.

The previous code looped while wait_socket(200ms) returned true,
so it exited on the first 200ms window of silence.  The race:

  - drain (after subprocess): consumes wait_primary→primary (no *)
  - post-success loop: wait_socket(200ms) returns false (socket quiet)
  - return true
  - primary→primary arrives 50ms later → consumed by [03-03]
    inter-drain without marks

The new helper loops until either ALL listenSatisfied[i] flags are
set (every expected convergence NOTIFY has arrived and been marked)
or 1 second has elapsed.  This means the flush continues as long as
there are still outstanding NOTIFYs to collect, and only exits early
when the socket is quiet AND all conditions are already marked.

The 1-second cap is a generous safety valve; in practice the helper
returns as soon as the last in-flight NOTIFY lands (sub-second on
local Docker networks).

* ensure.pgaf: fix test_004_demoted reliability

Two changes to test_004_demoted:

1. Wait for node1=primary before stopping it.
   After step 003 stops and restarts postgres on node2, the cluster
   may go through a second oscillation (node2→catchingup, node1→
   wait_primary) before settling again.  If compose stop node1 fires
   while node1 is in wait_primary the monitor does not trigger a
   failover, and the step hangs until timeout.

2. Wait for node2=wait_primary (not primary) before restarting node1.
   node2 reaches 'wait_primary' (point of no return) once the monitor
   has committed to promoting it, but it cannot reach 'primary' while
   node1 is stopped — it needs a synchronous standby to acknowledge WAL.
   When node1 restarts it connects as a sync standby and node2 then
   transitions wait_primary → primary.  Waiting for 'primary' here
   would time out because node2 can never reach that state without node1.

* tests/tap/specs/ensure.pgaf: converge test_004 and test_005 with Python test

Both tests now cover the same scenarios as their Python predecessor
(tests/test_ensure.py):

test_004_demoted:
- Stop postgres first while pg_autoctl is still running so it can
  report the outage to the monitor before being killed (matches Python's
  node1.stop_postgres() followed by node1.stop_pg_autoctl())
- Add explicit wait for the 'demoted' state after node1 comes back,
  matching Python's assert node1.wait_until_state('demoted')

test_005_inject_error_in_node2:
- After injecting the bad config and stopping postgres (pgctl off),
  re-enable auto-start with 'pgctl on' (ignoring its exit code, since
  postgres cannot come up with the broken config) so pg_autoctl keeps
  retrying — this matches Python's node2.restart_postgres() intent
  where postgres repeatedly fails to start, and the monitor eventually
  triggers a failover to node1

* tests/tap/specs/ensure.pgaf: drop racy 'wait for demoted' in test_004

The 'demoted' state is sub-second transient: node1's pg_autoctl may
execute the demoted transition and report 'catchingup' before the
convergence NOTIFY (demoted ➜ demoted) is ever emitted on the LISTEN
channel, causing 'wait until node1 state is demoted' to time out on
loaded CI runners.

The Python test handles this differently: it starts node1 without
waiting for postgres to come up and polls the database directly.  The
pgaf LISTEN-based approach cannot reliably catch the window.

Keep only the stable end state check (node2=primary, node1=secondary),
which is what the test ultimately cares about.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Developer productivity Enhancements to ability to ship quality code enhancement New feature or request Packaging and CI Enhancements to our CI integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant