feat: default output_plugin_libraries for Spock on patched PG minors - #64
Conversation
… minors PostgreSQL 16.15/17.11/18.5+ gate logical decoding output plugins behind a new output_plugin_libraries allow-list that excludes spock_output by default, breaking replication once a node picks up a patched minor via the chart's mutable image tag. Set it to include spock_output by default, and document overriding it to null at the chart-wide level for clusters pinned to an older minor — a per-node override does not work, since it round-trips through the Kubernetes API as an empty string rather than an omitted key, which pre-patch Postgres still rejects.
|
Warning Review limit reached
Next review available in: 18 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe chart now sets PostgreSQL ChangesPostgreSQL plugin configuration
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 9 |
| Duplication | 4 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
|
|
||
| ### output_plugin_libraries and older PostgreSQL minor versions | ||
|
|
||
| PostgreSQL 16.15, 17.11, 18.5, and newer minor versions add a new `output_plugin_libraries` allow-list that gates which logical decoding output plugins a server accepts. Its built-in default is `pgoutput, test_decoding`, which does not include `spock_output`. Without it on the list, Spock cannot create its replication slot and replication stops on the provider node. |
There was a problem hiding this comment.
18.5 was recalled, so you can list 18.6 here.
18.5 was recalled and replaced by 18.6, so it never shipped as a patched minor. Point at the version that actually did.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/configuration.md (1)
244-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the older-version fallback explicit.
State that a cluster remaining on an older minor must use a consistent image version across all nodes and set the chart-wide
output_plugin_libraries: nulloverride. Do not rely on the phrase “pin every node's image consistently” alone.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/configuration.md` around lines 244 - 246, Update the warning in the configuration documentation to explicitly require clusters remaining on an older minor to use the same image version on every node and set the chart-wide pgEdge.clusterSpec output_plugin_libraries override to null. Replace the ambiguous “pin every node's image consistently” guidance while preserving the explanation that single-node null overrides do not omit the parameter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/configuration.md`:
- Around line 244-246: Update the warning in the configuration documentation to
explicitly require clusters remaining on an older minor to use the same image
version on every node and set the chart-wide pgEdge.clusterSpec
output_plugin_libraries override to null. Replace the ambiguous “pin every
node's image consistently” guidance while preserving the explanation that
single-node null overrides do not omit the parameter.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8aa10964-eb02-4b4f-af0b-9861bb2aafba
📒 Files selected for processing (3)
docs/configuration.mddocs/configuration.md.gotmplvalues.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- values.yaml
- docs/configuration.md.gotmpl
The chart's default Postgres image (18-spock5-standard) still resolves to a minor version that predates output_plugin_libraries, so chart-testing and integration-tests fail against it. Point both at the same tag in pgEdge's internal image repo instead, which already has a patched version from postgres-images#28's approved build — one env var, no changes to any test values file, easy to remove once the public tag catches up. Also adds the changie entry for this feature.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test.yaml:
- Line 20: Update the TEST_POSTGRES_IMAGE setup in the workflow so both test
jobs authenticate to GHCR before Kind pulls the image. Configure narrowly scoped
registry credentials from the available GitHub token, create the corresponding
Kubernetes imagePullSecret in the Kind cluster, and ensure both jobs use that
secret when pulling the PostgreSQL image.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a725f933-0747-42be-bb8c-722b3c046962
📒 Files selected for processing (2)
.github/workflows/test.yamlchanges/unreleased/Added-20260812-212552.yaml
mmols
changed the title
…t too TestMultiClusterInstall builds its own install options independently of the shared helper the other integration tests use, and never had the pgEdge.clusterSpec.imageName override wired in at all — only initSpockImageName. Confirmed live: this is why integration-tests kept failing after pointing CI at the internal repo's patched image; every other test already picked it up through the shared helper. Also trims the output_plugin_libraries comment in values.yaml per review: the full explanation lives in the docs already.
2 participants
Description
Newer PostgreSQL versions (16.15, 17.11, 18.5+) block Spock's replication plugin by default, which breaks replication once a node updates to one of these versions. This sets the new output_plugin_libraries setting to allow it, so replication keeps working.
One thing to know: if you need to stay on an older PG version, you can disable this by setting it to null — but only at the top-level chart config. Setting it to null per-node doesn't actually work (it gets sent as an empty value instead of removed, which still breaks). I found this while testing and documented it clearly so no one hits it by surprise.
Tested by:
Reproducing the actual crash on a live cluster running the affected PG version
Confirming the top-level null override boots cleanly and replication works
Confirming the per-node override fails, and checking exactly why
Adding unit tests for all three cases; full test suite passes