diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1d42aa4..0c7c8d2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -29,7 +29,8 @@ # - by far the most expensive job here, # installing two full PostgreSQL majors and # running the real pg_upgrade binary per leg). -# pg-tle-test -- pg_tle DEPLOYMENT: fresh install registered +# pg-tle-test -- pg_tle DEPLOYMENT: fresh install AND the +# 0.9.6 -> current update path, both registered # through AWS pg_tle's database-backed catalog # instead of a filesystem .control file. # @@ -376,15 +377,18 @@ jobs: bin/compare_fresh_vs_update "" 0.9.6 count_nulls_upgrade_none bin/compare_fresh_vs_update Quoted 0.9.6 count_nulls_upgrade_quoted - # Fresh-install smoke test only, deliberately - NOT extended to the - # update path. pgxntool 2.3.0's fix for installcheck's ordering bug + # Covers both a fresh install AND the 0.9.6 -> current update path, both + # purely via pg_tle. pgxntool 2.3.0's fix for installcheck's ordering bug # (Postgres-Extensions/pgxntool#83) made `installcheck` (and so `make # test`) unconditionally depend on `install`, which writes a real # .control file to disk - defeating the entire point of proving a pg_tle - # deployment never touches the filesystem. There's currently no way to - # invoke the real pgTAP suite without that happening first; filed as - # Postgres-Extensions/pgxntool#90. Revisit extending this job to the - # update path once that's resolved. + # deployment never touches the filesystem. There's still no upstream fix + # for that (Postgres-Extensions/pgxntool#90, open) that would let + # bin/test_existing's real pgTAP suite run without it, so the update-path + # steps below use TEST_EXISTING_DEPLOY=pgtle (see bin/test_existing), + # which instead sandboxes `make test`'s install step behind a scratch + # DESTDIR - harmless here since a pg_tle-deployed database never needs + # those files. pg-tle-test: needs: [changes] if: needs.changes.outputs.docs_only != 'true' @@ -506,6 +510,44 @@ jobs: fi - name: Verify no stray extension control files after the fresh-install smoke test run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + - name: Install count_nulls at 0.9.6, purely via pg_tle (update-path prep) + # A SECOND, separate scratch database, created after the template1 + # registration above so it inherits both registrations too (same + # reasoning as count_nulls_smoke). prepare-old creates the database, + # CREATE EXTENSIONs at 0.9.6 (pure SQL - resolves through pg_tle's + # catalog, no `make install` call, which would defeat the whole + # point), then plants + proves the dependency guard so a stray + # CASCADE drop anywhere below can't silently turn the eventual + # existing-mode run into a fresh install instead. + run: | + test ! -e /usr/share/postgresql/${{ matrix.pg }}/extension/count_nulls.control + bin/test_existing prepare-old count_nulls_pgtle_update "" 0.9.6 + - name: Verify no stray extension control files after installing 0.9.6 via pg_tle + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + - name: Update 0.9.6 -> current, purely via pg_tle + # Pure SQL (ALTER EXTENSION ... UPDATE), no filesystem write either. + run: bin/test_existing update count_nulls_pgtle_update + - name: Verify no stray extension control files after the pg_tle update + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control + - name: Run the real pgTAP suite against the pg_tle-updated database (existing mode) + # run-suite re-proves the dependency guard, drops it, then runs the + # FULL suite via --use-existing against the real pg_tle-deployed + + # updated database - the same suite/expected-output as every other + # leg (see test/README.md). TEST_EXISTING_DEPLOY=pgtle makes + # run-suite sandbox `make test`'s otherwise-unavoidable `install` + # step behind a scratch DESTDIR instead of writing to the real + # extension directory (see bin/test_existing's TEST_EXISTING_DEPLOY + # comment), and makes test/install/load.sql's existing-mode + # assertion cross-check pgtle.available_extensions() instead of + # pg_available_extensions (which never sees pg_tle registrations - + # see the Makefile's TEST_EXISTING_DEPLOY comment). + run: TEST_EXISTING_DEPLOY=pgtle bin/test_existing run-suite count_nulls_pgtle_update "" + - name: Verify no stray extension control files after the pgTAP suite + # THE step that actually proves the DESTDIR sandboxing worked: the + # real extension directory must still be clean after `make test` + # ran (with its otherwise-unavoidable `install` step) sandboxed + # behind a scratch DESTDIR. + run: bin/assert_fs_clean verify ${{ matrix.pg }} /tmp/control_baseline.txt pg_tle.control # A single stable check name for use as a required status check in branch # protection rules. Matrix jobs produce check names like diff --git a/Makefile b/Makefile index 170fb87..639a228 100644 --- a/Makefile +++ b/Makefile @@ -83,6 +83,43 @@ $(error TEST_LOAD_SOURCE must be 'fresh', 'update' or 'existing', got '$(TEST_LO endif export PGOPTIONS := $(PGOPTIONS) -c count_nulls.test_load_mode=$(TEST_LOAD_SOURCE) +# TEST_EXISTING_DEPLOY: in 'existing' mode (TEST_LOAD_SOURCE=existing), how +# was the extension actually deployed onto the cluster before this run +# started? Unlike TEST_LOAD_SOURCE/TEST_SCHEMA above, this does not select or +# change any install behavior - it only tells test/install/load.sql's +# existing-mode assertion where to cross-check "what does the cluster +# consider count_nulls's current version" against the actually-installed +# extversion: +# - filesystem (default): a real .control file is on disk (a real `make +# install`, or a pg_upgrade'd cluster carrying one over) - cross-check +# against pg_available_extensions.default_version, which reads .control +# files directly off disk. +# - pgtle: count_nulls was registered purely through pg_tle's +# database-backed catalog (see the pg-tle-test CI job), never touching +# the filesystem. pg_available_extensions does NOT see pg_tle +# registrations at all - it only ever reads .control files off disk - so +# its default_version comes back NULL for a pg_tle-only extension even +# though a version-less CREATE EXTENSION resolves correctly through +# pg_tle. pg_tle ships its own separate, non-integrated analog instead: +# pgtle.available_extensions(), a C function whose own doc comment in +# pg_tle's tleextension.c says "The system view pg_available_extensions +# provides a user interface to this SRF" - i.e. pg_tle's SRF is modeled +# on pg_available_extensions, but pg_tle never hooks or populates the +# real view itself. In this mode, cross-check against pg_tle's SRF +# instead. +# +# Deliberately the SAME env var name bin/test_existing already reads (to +# decide whether to sandbox `make test`'s install step via a scratch +# DESTDIR) - it's already exported to any `make` invocation bin/test_existing +# spawns as a child process (and explicitly passed through on run_suite's +# `make test` command line too), so no extra plumbing is needed to get it +# here. +TEST_EXISTING_DEPLOY ?= filesystem +ifeq ($(filter $(TEST_EXISTING_DEPLOY),filesystem pgtle),) +$(error TEST_EXISTING_DEPLOY must be 'filesystem' or 'pgtle', got '$(TEST_EXISTING_DEPLOY)') +endif +export PGOPTIONS := $(PGOPTIONS) -c count_nulls.test_existing_deploy=$(TEST_EXISTING_DEPLOY) + # Convenience wrapper: `make test-update` == `make test TEST_LOAD_SOURCE=update`. # Must recurse (a fresh $(MAKE)) rather than depend on `test`, so the # parse-time TEST_LOAD_SOURCE conditional above re-evaluates with update set. diff --git a/bin/test_existing b/bin/test_existing index f0b12f5..d47ca8f 100755 --- a/bin/test_existing +++ b/bin/test_existing @@ -22,6 +22,14 @@ # code creates, so pg_upgrade needs to run against already-current objects, # not ones still frozen at the old INSTALL_VERSION. # +# The pg-tle-test CI job's update-path leg reuses the exact same three +# subcommands unmodified, in the same prepare-old -> update -> run-suite +# order (a pg_tle-registered database needs no real pg_upgrade or +# filesystem install between prepare-old and update - both are pure SQL, +# so there's no binary-upgrade step to reorder around there) - see +# TEST_EXISTING_DEPLOY below for the one piece of plumbing that job +# needed. +# # Not CI-only: a developer can run any subcommand locally against a scratch # database. Modeled on Postgres-Extensions/cat_tools's bin/test_existing. # Two differences from that script: count_nulls ships no @@ -53,6 +61,37 @@ # actively PROVE that (see bin/test_existing.sql/assert_guard.sql): if the # drop unexpectedly succeeds, this script fails CI rather than silently # passing. +# +# TEST_EXISTING_DEPLOY (optional environment variable, read by run_suite): +# unset/filesystem (default): count_nulls was deployed the normal way (a +# real `make install`). run_suite calls `make test` exactly as before. +# pgtle: count_nulls was deployed purely through pg_tle's database-backed +# catalog (see the pg-tle-test CI job), never `make install`ed onto the +# filesystem. pgxntool's `test` target unconditionally depends on +# `install` (base.mk: `TEST_DEPS += install installcheck`, and +# `installcheck: install` - the pgxntool#83/2.3.0 ordering fix), so +# naively calling `make test` here would silently write a real +# count_nulls.control to disk - which would then shadow the pg_tle +# registration on any FUTURE version-less CREATE EXTENSION in the same +# cluster, defeating the entire point of proving a pg_tle-only +# deployment, without ever raising an error. +# +# In this mode run_suite instead passes DESTDIR= to `make +# test`: PGXS's own `install` target prefixes every path with +# $(DESTDIR), so it still "runs" (satisfying Make's dependency graph) +# but writes harmlessly to a throwaway directory instead of the real +# extension directory - fine here specifically because the pg_tle- +# deployed database under test never needs those files (it's already +# live via pg_tle's own catalog). One more prerequisite needs help: +# `installcheck: pgtap` has a DESTDIR-aware prerequisite check +# ($(DESTDIR)$(datadir)/extension/pgtap.control) but a DESTDIR-blind +# recipe (`pgxn install pgtap --sudo`) - against an empty scratch +# DESTDIR, Make would consider pgtap "missing" and re-run that recipe +# for real (network + sudo), even though pgtap is already genuinely +# installed from an earlier CI step. seed_pgtap_stub() pre-seeds a +# zero-byte stub at that exact scratch-prefixed path so Make considers +# the prerequisite already satisfied and never invokes the real recipe. +# See Postgres-Extensions/pgxntool#90 (open, no upstream fix yet). set -euo pipefail # Run from the repository root (where `make` works and test paths resolve), @@ -155,6 +194,30 @@ create_extension_in_schema() { psql_do "$db" -c "${sql}CREATE EXTENSION count_nulls VERSION '$version'" } +# --------------------------------------------------------------------------- +# pg_tle DESTDIR sandbox helpers (see TEST_EXISTING_DEPLOY in the file header) +# --------------------------------------------------------------------------- + +# Seed a stub pgtap.control at the exact DESTDIR-prefixed path pgxntool's +# `pgtap` target's prerequisite check looks for +# ($(DESTDIR)$(datadir)/extension/pgtap.control), so Make considers that +# prerequisite already satisfied and never invokes the real recipe (`pgxn +# install pgtap --sudo`) against a scratch DESTDIR that can never contain a +# real install. Existence is all Make checks here - pgtap has no other +# prerequisites to compare timestamps against - so a zero-byte file is +# enough. +seed_pgtap_stub() { + local destdir=$1 datadir control_path + datadir=$(make -s print-datadir 2>/dev/null | sed -n 's/.*set to "\(.*\)"$/\1/p') + if [ -z "$datadir" ]; then + echo "FAIL: could not resolve datadir (make -s print-datadir)" >&2 + exit 1 + fi + control_path="${destdir}${datadir}/extension/pgtap.control" + mkdir -p "$(dirname "$control_path")" + : > "$control_path" +} + # --------------------------------------------------------------------------- # Subcommand implementations # --------------------------------------------------------------------------- @@ -179,16 +242,44 @@ prepare_old() { # then runs the suite via --use-existing so pg_regress does NOT drop/recreate # the database. run_suite() { - local db=$1 schema=$2 + local db=$1 schema=$2 deploy=${TEST_EXISTING_DEPLOY:-filesystem} + case "$deploy" in + filesystem|pgtle) ;; + *) + echo "FAIL: TEST_EXISTING_DEPLOY must be 'filesystem' or 'pgtle', got '$deploy'" >&2 + exit 1 + ;; + esac + assert_version "$db" current assert_drop_blocked "$db" drop_guard "$db" + + # See TEST_EXISTING_DEPLOY in the file header. Only 'pgtle' changes + # anything here; a scratch DESTDIR is created and baked into a trap so it + # is cleaned up regardless of how this function/script exits (a RETURN + # trap would NOT fire under `set -e` if `make test` below reports a + # regression - EXIT does, for any exit reason). + local destdir_opt=() + if [ "$deploy" = pgtle ]; then + local scratch + scratch=$(mktemp -d) + trap "rm -rf '$scratch'" EXIT + seed_pgtap_stub "$scratch" + destdir_opt=(DESTDIR="$scratch") + fi + # In existing mode pg_regress runs against $db via --use-existing and must # NOT create/drop its own database. `make test` (not just `make # verify-results`) is a real gate as of pgxntool 2.3.0 - it now exits # non-zero on regression failures instead of always exiting 0 regardless # of pg_regress's result (see this repo's pgxntool 2.3.0 bump). - make test TEST_LOAD_SOURCE=existing TEST_SCHEMA="$schema" CONTRIB_TESTDB="$db" EXTRA_REGRESS_OPTS=--use-existing + # TEST_EXISTING_DEPLOY is passed through explicitly (not left to + # environment inheritance) so test/install/load.sql's existing-mode + # assertion (via the count_nulls.test_existing_deploy GUC - see the + # Makefile) picks the right source regardless of how this function is + # invoked. + make test TEST_LOAD_SOURCE=existing TEST_SCHEMA="$schema" CONTRIB_TESTDB="$db" EXTRA_REGRESS_OPTS=--use-existing TEST_EXISTING_DEPLOY="$deploy" "${destdir_opt[@]}" } usage() { diff --git a/test/install/load.sql b/test/install/load.sql index 5d6d1bf..0234000 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -67,20 +67,52 @@ SELECT :'count_nulls_test_load_mode' = 'existing' AS count_nulls_existing_mode \if :count_nulls_existing_mode /* * Already installed by something external to this pg_regress invocation - * (a real pg_upgrade run - see the pg-upgrade-test CI job). Only assert - * it's present and at the current version; do NOT drop/create/update it - - * the whole point of this mode is testing the REAL migrated objects. + * (a real pg_upgrade run, or a pg_tle registration - see the + * pg-upgrade-test / pg-tle-test CI jobs). Only assert it's present and at + * the current version; do NOT drop/create/update it - the whole point of + * this mode is testing the REAL migrated/deployed objects. + * + * The "current version" half of that assertion (v_default below) has two + * sources depending on count_nulls.test_existing_deploy (see the + * TEST_EXISTING_DEPLOY comment in the Makefile): + * - filesystem (default): pg_available_extensions.default_version, read + * straight from a real .control file on disk. + * - pgtle: count_nulls was registered purely through pg_tle's + * database-backed catalog, never touching the filesystem. + * pg_available_extensions does NOT see pg_tle registrations at all - it + * only ever reads .control files off disk - so it comes back NULL here + * even though CREATE EXTENSION correctly resolves the default version + * through pg_tle. pg_tle ships its own separate, non-integrated analog + * for this: pgtle.available_extensions() (see pg_tle's tleextension.c, + * which documents pg_available_extensions as merely modeled on this + * SRF, not backed by it). Use that instead when running under pg_tle, + * rather than weakening the check for the filesystem case. */ DO $$ DECLARE v_installed text := (SELECT extversion FROM pg_extension WHERE extname = 'count_nulls'); - v_default text := (SELECT default_version FROM pg_available_extensions WHERE name = 'count_nulls'); + v_deploy text := current_setting('count_nulls.test_existing_deploy'); + v_default text; BEGIN IF v_installed IS NULL THEN RAISE EXCEPTION 'count_nulls.test_load_mode=existing but count_nulls is not installed'; END IF; + + IF v_deploy = 'pgtle' THEN + SELECT default_version INTO v_default + FROM pgtle.available_extensions() WHERE name = 'count_nulls'; + ELSIF v_deploy = 'filesystem' THEN + SELECT default_version INTO v_default + FROM pg_available_extensions WHERE name = 'count_nulls'; + ELSE + RAISE EXCEPTION + 'count_nulls.test_existing_deploy must be ''filesystem'' or ''pgtle'', got ''%''' + , v_deploy + ; + END IF; + IF v_installed IS DISTINCT FROM v_default THEN - RAISE EXCEPTION 'count_nulls installed at % but default_version is %', v_installed, v_default; + RAISE EXCEPTION 'count_nulls installed at % but default_version (deploy=%) is %', v_installed, v_deploy, v_default; END IF; END $$;