Add binary pg_upgrade CI job (existing-mode suite + dependency guard) - #20
Draft
jnasbyupgrade wants to merge 1 commit into
Draft
Add binary pg_upgrade CI job (existing-mode suite + dependency guard)#20jnasbyupgrade wants to merge 1 commit into
jnasbyupgrade wants to merge 1 commit into
Conversation
…-mode suite Adds a pg-upgrade-test job (3 legs: 12->13, 16->17, 12->17) that installs extension_drop on an old cluster, plants a dependency guard, performs a real binary pg_upgrade, and runs the suite in TEST_LOAD_SOURCE=existing mode against the migrated database, with a dynamic version assertion and a docs-only cost gate for the new heavy job. Modeled on Postgres-Extensions/cat_tools's bin/test_existing and .github/scripts/pg_upgrade_cluster, substantially simplified: extension_drop has only ever shipped one real version, so no bridge leg or update-scenario machinery is needed. Verified end to end locally (PG12 -> PG17) before committing, which also surfaced a real conflict between a persistent dependency guard and test/sql/schema.sql's unconditional non-cascade drop, now handled by excluding schema.sql/zzz_build.sql from the existing-mode REGRESS list (both are already proven by the fresh-install `test` job). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Aug 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Migrated from fork-internal PR jnasbyupgrade#6 to enable a native same-repo stacked PR (bases off
test-install-foundation, which now exists directly onPostgres-Extensions/extension_tools). Original PR: jnasbyupgrade#6Stack position
Third link in a stack, all in
jnasbyupgrade/extension_tools:fix-cat-tools-install(base, upstream draft PR #10)→
test-install-foundation(fork-internal draft PR #4)→
pg-upgrade-ci(this PR, based ontest-install-foundation).None of these branches exist yet on
Postgres-Extensions/extension_tools, so this PR's own CI may not be meaningfully triggerable/mergeable until the earlier links land somewhere real — flagging this per the task instructions rather than guessing at why a run behaves oddly.What this builds
Following
~/advanced-extension-testing.md(reference implementation:Postgres-Extensions/cat_tools's currentmaster, specifically itspg-upgrade-testjob inci.ymlandbin/test_existing) and~/test-fixes.mditems 1/2/3/5/6:bin/test_existing— a committed, parameterized script (plant-guard,prepare,run-suitesubcommands) factoring the install→guard→pg_upgrade→assert→run-suite flow out of inline YAML..github/scripts/pg_upgrade_cluster— the binary-pg_upgrade cluster mechanics (recreate-old,upgrade), generic CI plumbing with nothing extension-specific in it.pg-upgrade-testjob inci.yml— 3 legs:12→13,16→17(adjacent-major legs near the floor/ceiling) and12→17(full span). Each leg installs extension_drop fresh on the old cluster, plants + proves a dependency guard, binarypg_upgrades straight to the new major, then runs the suite inTEST_LOAD_SOURCE=existingmode against the real migrated database.assert_version()derives expected version frommake -s print-PGXNVERSION, with empty-value guards on both sides.changesjob — cheap docs-only gate (fail-safe-firstGITHUB_OUTPUTwrite), gating only the new heavypg-upgrade-testjob (the pre-existingtestjob is already cheap, so it stays ungated).pg-upgrade-testneeds[changes, test]; workflow-levelconcurrencygroup withcancel-in-progress: true.all-checks-passedneeds list updated to[changes, test, pg-upgrade-test].What I took verbatim / adapted / skipped from cat_tools
.github/scripts/pg_upgrade_cluster: taken essentially verbatim — it's pure cluster mechanics (pg_ctlcluster/pg_createcluster/pg_upgrade against the pgxn-tools "test" cluster convention), nothing cat_tools-specific in it.bin/test_existing: adapted and substantially smaller. extension_drop has only ever shipped one real version (1.0.0 — seeHISTORY.asc/RELEASE.md; the only PGXN listing, 0.1.x from 2017, predates the current SQL entirely), so:pg_upgrade-unsafe old version exists to bridge from (per the doc's own guidance not to build this preemptively).TEST_LOAD_SOURCE=updatemode is wired up but unexercised for the same reason).old_pg/new_pgpairs viastrategy.matrix.include) and the dependency-guard technique itself (plant a view with a hardpg_dependedge on a stable extension member, prove a non-CASCADE drop is blocked) are the same pattern, unchanged.Something NOT in cat_tools's model — found by actually running this locally
cat_tools's control file pins
schema = 'cat_tools'(non-relocatable), so it has no test that freely retargets the install schema. extension_drop'stest/sql/schema.sql(added by the foundation PR) does — it proves the schema-targeting/quoting pipeline by unconditionally doingDROP EXTENSION extension_drop;(non-CASCADE) partway through. That collides for real with a persistent dependency guard: the very first statement inschema.sqlfailed with the guard's own2BP01error when I ran the full suite against a real pg_upgraded database locally.test/sql/zzz_build.sqlsurvives (it does a CASCADE drop) but adds adrop cascades to view ...NOTICE not present in the checked-in expected output. Fix:bin/test_existing'srun_suitecomputes an existing-modeREGRESSlist that excludesschemaandzzz_build(both already proven by the regular freshtestjob on every PostgreSQL major) — derived fromtest/sql/*.sql's actual contents, not a hardcoded list, so a future new test file is automatically included unless deliberately excluded. Flagging this loudly per the "ultimate goal" section: this is a genuinely new pattern (schema-flexible extension + persistent dependency guard) that cat_tools's own model doesn't need to solve.Deliberately skipped
testjob's PG list (9.3–17, extension_drop's own claimed floor) andpg-upgrade-test's floor (12) are genuinely different lists for a real reason, not accidental duplication:make installunconditionally buildscat_toolsfromPostgres-Extensions/cat_tools'smaster(PGXN's published cat_tools is a stale 2017 release extension_drop can't use — see the Makefile's owncat_toolstarget comment), and that current cat_tools requires PostgreSQL ≥ 12 for a fresh install (its ownMETA.json). Unifying two lists that are supposed to differ would be actively wrong here, so I left them separate and documented why inci.yml's top-of-file comment.pg-upgrade-stepwise(§6c-bis, every-major climb) — not built. extension_drop has no catalog-touching views/functions (per §7's risk profile), so the incremental value over the 3 existing legs looked low relative to the cost (installing every PG major, N sequentialpg_upgrades). Can be added later if that risk profile changes.all-checks-passedneeds-list self-check (cat_tools has a Python step verifying itsneeds:list matches the actual job set) — skipped to keep this PR's diff focused; I manually confirmed[changes, test, pg-upgrade-test]matches the actual 3 non-aggregator jobs.Local verification
This container has PostgreSQL 12 (port 5412) and 17 (port 5417) running — exactly the floor/ceiling this job uses. I did not use the persistent
mainclusters; instead created throwawaytest-named clusters (matching the pgxn-toolspg-startconventionpg_upgrade_clusterexpects) on a separate port, ran the actual committed scripts against them, and tore them down afterward:make install PG_CONFIG=.../12/bin/pg_config(installs extension_drop + git-built cat_tools 0.3.0 on PG12)bin/test_existing prepare extdrop_upg3— creates the DB,CREATE EXTENSION ... CASCADE, plants + proves the dependency guard (confirmed non-CASCADE drop blocked with2BP01).make install PG_CONFIG=.../17/bin/pg_config(extension_drop/cat_tools are pure SQL, so this just needs to be present in the new cluster's sharedir).INITDB_OPTS="--data-checksums --auth trust" .github/scripts/pg_upgrade_cluster upgrade 12 17— real binarypg_upgrade, completed clean.bin/test_existing run-suite extdrop_upg3— asserted version (1.0.0both sides), re-proved the guard survived pg_upgrade, ranmake test/make verify-resultsin existing mode (dependency_guard+simple,schema/zzz_buildexcluded per above) — all passed, exit code 0. Re-confirmed the guard was still present afterward.This process caught a real bug before it ever reached CI: my first draft passed
REGRESS=$regressthrough an unquoted flat string, which word-split the multi-valueREGRESS="dependency_guard simple"into a bogussimplemake target (No rule to make target 'simple'). Fixed by switching to a bash array (existing_args=(...),make test "${existing_args[@]}") — see the comment left at that fix site inbin/test_existing.No environment quirks unrelated to this change were encountered (the
12 → 13/16 → 17legs weren't locally re-verified since only PG12/17 binaries are installed in this dev container, but they exercise the identical script paths already proven end-to-end on12 → 17).CI status
Pushed to
jnasby/pg-upgrade-ci; watching the Actions run now and will report/fix any failures.