enable replication support slot management via hiera by ikonia · Pull Request #1694 · puppetlabs/puppetlabs-postgresql

ikonia · 2026-08-13T12:44:01Z

Summary

he branch is rebased from master as of 12/08/2026 and includes a few outstanding PR's to get the functionality needed.
enables replication support for Postgres

Adds primary-side named replication slot management to postgresql::server closing the last gap in this branch's physical streaming replication support: a standby configured via postgresql::server::recovery

postgresql::server::recovery primary_slot_name had nothing on the primary side to actually create the slot it connects to.
Previously this required hand-declaring the native postgresql_replication_slot resource directly in a manifest; it's now Hiera-drivable the same way databases/roles/grant_roles/extensions already are.

Validated end-to-end against a real PostgreSQL 18 master/replica pair

postgresql::server::replication_slots (new hash parameter)

Mirrors the existing grant_roles pattern: a single-level hash, splatted straight onto the native postgresql_replication_slot resource.
Example: postgresql::server::replication_slots: { standby1_slot: { ensure: present } }
Known, deliberate limitation: the underlying native type has no port/psql_path/connect_settings parameters, unlike every sibling resource in this module. This only matters for multi-instance-per-host deployments, this can be expanded on if desired in later MR's but is out of scope for this initial enhancement

Additional Context

Testing

Full base-backup + streaming replication cycle between a real PG18 master and replica, using a named slot created via this new parameter.
Validated under real load, not just synthetic test data: a ~170-table Foreman database, restored via pg_dump/pg_restore into the master, replicated correctly to the replica — confirmed via LSN comparison and slot health (wal_status, active) on both sides.
Spec coverage added/fixed at both the wrapper level (spec/classes/server_spec.rb) and the type level (spec/unit/puppet/type/postgresql_replication_slot_spec.rb).

Related Issues (if any)

this branch and merge contains open PR's already merged in
#1661
#1674

Checklist

[ x] 🟢 Spec tests.
[ x] 🟢 Acceptance tests.
[ x] Manually verified. (For example puppet apply)

jst-cyr · 2026-08-14T20:56:20Z

@ikonia : This looks like a lot of good work, thank you! The CI spec test seems to have flagged a few syntax issues: https://github.com/puppetlabs/puppetlabs-postgresql/actions/runs/31800747767/job/94767953894?pr=1694

ikonia · 2026-08-15T07:44:14Z

syntax wise it's 'working' I put a chunk of effort into manually testing the changes, so I'm confident there is nothing syntax broken, I saw those errors on the test suite, but I wasn't confident the test suite was to be trusted, it gives no information as to where it's finding these errors, and coupled with the fact that linting passed and manual and automated testing passed with no syntax errors, I didn't have a high degree of confidence that these warnings where genuine (I've had other PR's fail due to outdated tests). If I can understand how to get more information out of the test to see what it's actually complaining about (I understand the error, just not where it's picking it up from) I'll be more than happy to update it to make it compliant

ikonia · 2026-08-15T08:03:43Z

fixed the test failure @jst-cyr it's erroring now, but that looks like a metadata problem because the systemd 10 change is not merged in ?

jst-cyr · 2026-08-17T12:33:22Z

Thank you @ikonia ! Yes, we are now blocked by needing to get #1691 reviewed and merged up so we can pull that down into all the pending PRs.

jst-cyr · 2026-09-01T16:56:09Z

Running CI for this PR here: #1702

jst-cyr · 2026-09-01T17:06:26Z

@ikonia : I tried running acceptance tests, but it looks like there are some spec test failures that are blocking it getting to the acceptance tests. I'm not seeing those issues on #1661, so it must be with the changes introduced in this PR that are potentially not in #1661?

Failed run: https://github.com/puppetlabs/puppetlabs-postgresql/actions/runs/32396244926/job/99946185064?pr=1694

Errors:
Error: postgresql::server::extension with mandatory arguments only is expected to contain Postgresql_psql[template_postgis: CREATE EXTENSION "postgis"] that requires Postgresql::Server::Database[template_postgis]

Error: postgresql::server::extension when schema is specified is expected to contain Postgresql_psql[template_postgis: ALTER EXTENSION "postgis" SET SCHEMA "pg_catalog"]

Error: postgresql::server::extension when setting package name is expected to contain Package[postgis] that comes before Postgresql_psql[template_postgis: CREATE EXTENSION "postgis"]

Error: postgresql::server::extension when ensuring absence is expected to contain Postgresql_psql[template_postgis: DROP EXTENSION "postgis"] that requires Postgresql::Server::Database[template_postgis]

Error: postgresql::server::extension when ensuring absence is expected to contain Package[postgis] with ensure => "absent" and name => "postgis"

Error: postgresql::server::extension when ensuring absence when keeping package installed is expected to contain Postgresql_psql[template_postgis: DROP EXTENSION "postgis"] that requires Postgresql::Server::Database[template_postgis]

Error: postgresql::server::extension when ensuring absence when keeping package installed is expected to contain Package[postgis] that requires Postgresql_psql[template_postgis: DROP EXTENSION "postgis"]

Error: postgresql::server::extension when extension version is specified is expected to contain Postgresql_psql[template_postgis: ALTER EXTENSION "postgis" UPDATE TO '99.99.99'] that requires Postgresql::Server::Database[template_postgis]

Error: postgresql::server::extension when extension version is latest is expected to contain Postgresql_psql[template_postgis: ALTER EXTENSION "postgis" UPDATE] that requires Postgresql::Server::Database[template_postgis]

Error: postgresql::server::grant_role with mandatory arguments only is expected to contain Postgresql_psql[grant_role:test] that requires Class[postgresql::server]

Error: postgresql::server::grant_role with db arguments is expected to contain Postgresql_psql[grant_role:test] that requires Class[postgresql::server]

Error: postgresql::server::grant_role with ensure => absent is expected to contain Postgresql_psql[grant_role:test] that requires Class[postgresql::server]

Error: Puppet::Type::Postgresql_replication_slot is ensurable

ikonia · 2026-09-10T08:08:10Z

happy to look and review this, I had a problem in another module where my local execution of the test suite didn't align to the PR triggered, this could be similar

ikonia · 2026-09-10T08:18:43Z

I think I know what this is, the contrib package is not being installed by default in the tests (don't know why) and that's stopping the extensions being created, I think this is a test problem (why is the contrib package not being installed) rather than a module problem

jst-cyr · 2026-09-10T21:58:08Z

I'm going to upfront admit that I needed some Claude digging and going back and forth with it to find some clues, but given what I'm seeing I think it might have to do with the new 'split' on the extension.

There's this error multiple times in the tests: parameter 'extension' expects a String value, got Undef (line: 6)

If the tests aren't putting a ":" into the extension, then this split won't work. So that is likely contributing to some of the errors.

The analysis I ran also pointed at two other bugs, but I haven't dug into them deeper to check on them. This was the output I was given, it might help?

🤖 Claude analysis

Bug 2 — `manifests/server/grant_role.pp`: dropped ordering edge

Symptom (3 failures, same root cause):

postgresql::server::grant_role with mandatory arguments only is expected to contain
Postgresql_psql[grant_role:test] that requires Class[postgresql::server]

expected that the catalogue would contain Postgresql_psql[grant_role:test]
with that requires Class[postgresql::server]

Cause: this PR removes the ordering relationship to Class['postgresql::server']:

   connect_settings => $connect_settings,
   }

-  if empty($connect_settings) {
-    Class['postgresql::server'] -> Postgresql_psql["grant_role:${name}"]
-  }
   if defined(Postgresql::Server::Role[$role]) {

spec/defines/server/grant_role_spec.rb asserts that edge exists (when connect_settings is empty, i.e. connecting to the local instance), so removing it fails the test. This looks like an accidental deletion rather than an intentional change — nothing in the PR description mentions changing grant_role ordering.

Fix: restore the removed block.

Bug 3 — `spec/unit/puppet/type/postgresql_replication_slot_spec.rb`: wrong assertion for "is ensurable"

Symptom (1 failure):

Puppet::Type::Postgresql_replication_slot is ensurable
Failure/Error: expect(subject.property(:ensure)).not_to be_nil
expected: not nil
     got: nil

Cause: the type itself is fine — lib/puppet/type/postgresql_replication_slot.rb declares ensurable and is unchanged by this PR (confirmed identical on main and on this PR's branch head). The new spec test itself is what's wrong:

subject do
  described_class.new(name: 'standby1_slot')
end

it 'is ensurable' do
  expect(subject.property(:ensure)).not_to be_nil
end

Puppet::Type#property(:ensure) only returns a property object once it's been explicitly instantiated on that resource instance (e.g. by passing ensure: to .new, or after retrieve/resource[:ensure] forces the default to materialize). Since subject is built without an ensure: param, property(:ensure) is nil even though the type is correctly ensurable — this is testing the wrong thing, not exposing a real defect.

Fix: assert ensurability at the class level instead of on an unconfigured instance, e.g.:

it 'is ensurable' do
  expect(described_class.attrtype(:ensure)).to eq(:property)
end

(The other new test right below it, 'accepts ensure => present and ensure => absent', already covers the instance-level behavior correctly by passing ensure: explicitly.)

ikonia · 2026-09-12T10:08:52Z

I think this is good now, test failure that's outstanding appears to be Ci related, functional test validation all passing

jst-cyr · 2026-09-14T17:00:50Z

@ikonia : I rebased and will kick off the acceptance testing!

jst-cyr · 2026-09-14T17:05:32Z

@ikonia : The rebase triggered a new run of the CI Spec tests and it looks like there are some 2 errors showing up.

CI failure

Error: postgresql::server::default_privileges valid object_type supported privilege on PostgreSQL >= 17 is expected to contain Postgresql_psql[default_privileges:test] with unless => "SELECT 1 WHERE EXISTS (SELECT * FROM pg_default_acl AS da LEFT JOIN pg_namespace AS n ON da.defaclnamespace = n.oid WHERE 'test=arwdDxtm' = ANY (defaclacl) AND nspname = 'public' and defaclobjtype = 'r')"

Error: postgresql::server::default_privileges valid object_type supported privilege on a legacy dotless 9.x version is expected to contain Postgresql_psql[default_privileges:test] with unless => "SELECT 1 WHERE EXISTS (SELECT * FROM pg_default_acl AS da LEFT JOIN pg_namespace AS n ON da.defaclnamespace = n.oid WHERE 'test=arwdDxt' = ANY (defaclacl) AND nspname = 'public' and defaclobjtype = 'r')"

ikonia · 2026-09-14T17:46:20Z

looking at the failures, this was tested specially against v18, so surprising

ikonia · 2026-09-14T18:12:56Z

introduced by the rebase commit #2aabad5 test cases added (along with a fix to default privilege) that where not on this branch before. Investigating

ikonia · 2026-09-14T18:54:57Z

the cherry picked fix in the rebase highlighted a flaw in the logic I'd used to quote hyphenated names, and the updates to the test suite to properly test it brought in alerted it was a bad fix. aligned the fix to the updated test suites and this seems much better now. finished testing it still works manually against Postgres 18 to make sure my logic was good, and it does work against this functionality (didn't re-test the whole module again)

Test failures are again back to CI related rather then functional

jst-cyr · 2026-09-14T20:40:50Z

Thanks @ikonia ! I'll get that acceptance testing matrix working in a separate PR so we can see the results there.

jst-cyr · 2026-09-15T13:41:37Z

Acceptance Test results: https://github.com/puppetlabs/puppetlabs-postgresql/actions/runs/34894654788?pr=1707

There were some infra provisioning issues, so I needed to do some rerunning today on targeted systems. Results:

The Debian 11, AlmaLinux-8, and AlmaLinux-9 failures are an exact match to the failures in the latest nightly run against main. This PR does not seem to introduce any new issues.
Debian 13 failure is new. Have rerun multiple times and it seems to be a repeatable issue that is not seen in main.

Debian-13 nightly passes, but this PR fails 6 examples in spec/acceptance/server/default_privileges_spec.rb with idempotency/apply-exit-code mismatches around postgresql::server::default_privileges (grant/revoke default privileges, target-role variants, schema unset). Worth checking whether the replication slot-management change in this branch is affecting default-privileges idempotency on Debian-13, or if it's an environment quirk specific to that image.

ikonia · 2026-09-16T10:58:55Z

I'll check this today, the Debian 13 tests I did passed, but they where basic function tests rather than a complete test suite

ikonia · 2026-09-24T15:51:17Z

ok I think this is good again now, I needed a little claude help to track it down, but it appears when rebased it reverted a change I'd made to fix something else, I've now rebased and re-applied my fix on top of that and it looks good

jst-cyr · 2026-09-24T20:59:34Z

Running CI tests for this PR here: #1709

ikonia · 2026-09-25T07:06:10Z

tests look good to me, the failures are present on main already

jst-cyr · 2026-09-25T20:04:31Z

Acceptance Test Results: https://github.com/puppetlabs/puppetlabs-postgresql/actions/runs/36058538426?pr=1709

A lot of green, but failures on Alma Linux 8, Alma Linux 9, and Debian 11. All of these failures match to the existing failures in nightly tests against main, so not introduced by this PR.

I think we're good for engineering review now!

ikonia · 2026-09-25T20:36:56Z

thank you for the persistence and effort @jst-cyr

jst-cyr · 2026-09-28T20:08:26Z

And thanks to you as well, @ikonia ! Hopefully nothing too much found in the review.

Generate postgresql::server::database resources from a $databases hash, mirroring the existing $roles/$grants pattern. Carried forward from be (3940f30/a30df35) during resync onto upstream/main.

Generate postgresql::server::grant_role resources from a $grant_roles hash. Declaring grant_role from inside class postgresql::server means its own Class['postgresql::server'] -> Postgresql_psql[...] ordering edge (used when connect_settings is empty) now cycles back on itself, so drop that edge here; the Postgresql::Server::Role ordering below is unaffected and still applies when the role is known. Carried forward from be (payload of 3a24f48 + 9527c31) during resync onto upstream/main.

Generate postgresql::server::extension resources from an $extensions hash keyed by database name, auto-creating the database first if it isn't already declared. Resources are titled "${database}:${extension}" to keep titles unique across databases, so extension.pp's $extension default now parses the extension name back out of that title instead of assuming $name is the bare extension name. Carried forward from be (payload of c55d2fc + 9bd9595 + 1b16edc) during resync onto upstream/main.

PostgreSQL has ignored recovery.conf since v12, but this module has never accounted for that: postgresql::server::recovery always wrote a recovery.conf-style file regardless of PostgreSQL version. Gate the define on versioncmp($postgresql::server::_version, '12'): - < 12 keeps the existing concat + recovery.conf.epp behavior, byte-for-byte unchanged, so existing hand-declared usages on older hosts are unaffected. - >= 12 instead creates an empty standby.signal marker file and writes primary_conninfo/primary_slot_name/restore_command/etc. as ordinary postgresql.conf GUCs via postgresql::server::config_entry, ordered to land before the instance's first service start (these are startup-only settings a reload cannot apply). Adds promote_trigger_file and recovery_target_action (their PG12+ replacements for trigger_file and pause_at_recovery_target); the legacy-only params are still accepted with a warning for a softer migration path. Also adds a new $standby hash parameter on postgresql::server (with a Postgresql::Standby type alias) so standby/recovery configuration is finally reachable from a single Hiera key, the same way roles, pg_hba_rules and config_entries already are -- previously postgresql::server::recovery had no hash-of-resources wrapper at all and could only be hand-declared in a manifest. Replication slot creation on the primary (postgresql_replication_slot) is a separate, still-unreachable-from-Hiera gap and is intentionally out of scope here.

postgresql::server::recovery still notified the pre-multi-instance Class['postgresql::server::reload'] token. Every sibling resource (pg_hba, pg_ident, config_entry) was migrated to the instance-scoped Postgresql::Server::Instance::Reload['main'] during the 2023 multi-instance refactor; this define was missed. Unrelated to the PG12+ standby-config work in the preceding commit.

The describe block named postgresql_replication_slot, but its subject instantiated Puppet::Type.type(:postgresql_psql) instead, so it never actually exercised this type. Point subject at the right type and add coverage for name validation and ensurable.

Generates postgresql_replication_slot resources from a $replication_slots hash, so a primary can Hiera-drive the named physical replication slots that a standby's primary_slot_name (postgresql::server::recovery/$standby) connects through. Mirrors the existing $grant_roles pattern: a single-level hash splatted straight onto the resource, since the native type has nothing beyond name/ensure to wrap. Deliberately not extending the native postgresql_replication_slot type or provider here: it has no port/psql_path/connect_settings, unlike every sibling resource, so this only supports the default instance. That only matters for multi-instance-per-host deployments; fixing it also requires redesigning self.instances/prefetch, which currently lists slots via a single connection-oblivious psql call. Left as a documented limitation rather than scope for this change.

jst-cyr · 2026-10-16T02:35:57Z

Running CI for this PR here: #1719

jst-cyr · 2026-10-16T03:19:44Z

Acceptance Test Results: https://github.com/puppetlabs/puppetlabs-postgresql/actions/runs/38841248493?pr=1719

Rebased to get latest from main. Full green on tests, no more provisioning issues, we are still good for engineering review!

ikonia requested review from a team, SimonHoenscheid, alexjfisher, bastelfreak, deric, ekohl and smortex as code owners August 13, 2026 12:44

ikonia force-pushed the be-pg12-standby branch from c67ee23 to a22499e Compare August 14, 2026 12:32

ikonia changed the title ~~enable replication support slot management via hiera - #1693~~ enable replication support slot management via hiera Aug 15, 2026

ikonia mentioned this pull request Aug 18, 2026

support database creation from hiera parameters #1661

Open

jst-cyr force-pushed the be-pg12-standby branch from 7e65172 to 2069d23 Compare August 20, 2026 17:11

jst-cyr mentioned this pull request Sep 1, 2026

enable replication support slot management via hiera #1702

Closed

jst-cyr force-pushed the be-pg12-standby branch from bf41bb2 to 5bba729 Compare September 14, 2026 16:58

jst-cyr mentioned this pull request Sep 14, 2026

(TESTS-1694) enable replication support slot management via hiera #1707

Closed

ikonia force-pushed the be-pg12-standby branch from 00aac88 to 257c250 Compare September 24, 2026 15:47

jst-cyr mentioned this pull request Sep 24, 2026

(TESTS-1694) enable replication support slot management via hiera #1709

Closed

ikonia added 16 commits October 15, 2026 22:31

Add databases hash parameter to postgresql::server

61bc003

Generate postgresql::server::database resources from a $databases hash, mirroring the existing $roles/$grants pattern. Carried forward from be (3940f30/a30df35) during resync onto upstream/main.

linting mistake

a3ecec2

correctly commited linting fix

34d0254

check: strict_indent fix flagged in earlier PR

6bdaedf

additional fix to strict_indent

2c04320

update missing trailing comma failure in spec

75fb4d9

updates to fix test failures

2e3de94

revert 1 spec change and fix forward

89c0b57

fix grant_roles spec

b1b019d

ensurable test not valid for replications slot

70f7455

jst-cyr force-pushed the be-pg12-standby branch from 257c250 to 70f7455 Compare October 16, 2026 02:31

jst-cyr mentioned this pull request Oct 16, 2026

(TESTS-1694) enable replication support slot management via hiera #1719

Closed

Conversation

ikonia commented Aug 13, 2026

Summary

Additional Context

Related Issues (if any)

Checklist

Uh oh!

jst-cyr commented Aug 14, 2026

Uh oh!

ikonia commented Aug 15, 2026

Uh oh!

ikonia commented Aug 15, 2026

Uh oh!

jst-cyr commented Aug 17, 2026

Uh oh!

jst-cyr commented Sep 1, 2026

Uh oh!

jst-cyr commented Sep 1, 2026

Uh oh!

ikonia commented Sep 10, 2026

Uh oh!

ikonia commented Sep 10, 2026

Uh oh!

jst-cyr commented Sep 10, 2026

🤖 Claude analysis

Bug 2 — manifests/server/grant_role.pp: dropped ordering edge

Bug 3 — spec/unit/puppet/type/postgresql_replication_slot_spec.rb: wrong assertion for "is ensurable"

Uh oh!

ikonia commented Sep 12, 2026

Uh oh!

jst-cyr commented Sep 14, 2026

Uh oh!

jst-cyr commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ikonia commented Sep 14, 2026

Uh oh!

ikonia commented Sep 14, 2026

Uh oh!

ikonia commented Sep 14, 2026

Uh oh!

jst-cyr commented Sep 14, 2026

Uh oh!

jst-cyr commented Sep 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ikonia commented Sep 16, 2026

Uh oh!

ikonia commented Sep 24, 2026

Uh oh!

jst-cyr commented Sep 24, 2026

Uh oh!

ikonia commented Sep 25, 2026

Uh oh!

jst-cyr commented Sep 25, 2026

Uh oh!

ikonia commented Sep 25, 2026

Uh oh!

jst-cyr commented Sep 28, 2026

Uh oh!

jst-cyr commented Oct 16, 2026

Uh oh!

jst-cyr commented Oct 16, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Bug 2 — `manifests/server/grant_role.pp`: dropped ordering edge

Bug 3 — `spec/unit/puppet/type/postgresql_replication_slot_spec.rb`: wrong assertion for "is ensurable"

jst-cyr commented Sep 14, 2026 •

edited

Loading

jst-cyr commented Sep 15, 2026 •

edited

Loading