Skip to content

enable replication support slot management via hiera - #1694

Open
ikonia wants to merge 13 commits into
puppetlabs:mainfrom
ikonia:be-pg12-standby
Open

enable replication support slot management via hiera#1694
ikonia wants to merge 13 commits into
puppetlabs:mainfrom
ikonia:be-pg12-standby

Conversation

@ikonia

@ikonia ikonia commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

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

jst-cyr commented Aug 14, 2026

Copy link
Copy Markdown
Member

@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

ikonia commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

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

ikonia commented Aug 15, 2026

Copy link
Copy Markdown
Contributor Author

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 ?

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

jst-cyr commented Aug 17, 2026

Copy link
Copy Markdown
Member

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.

ikonia added 13 commits August 20, 2026 13:11
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.
The unless clause's SQL query didn't quote the role name, causing
server-side errors when the role name contains a character like a
hyphen (e.g. "monitoring-user").

Carried forward from be (70edc1b, originally from
antaflos/fix_default_privileges_quoting) during resync onto
upstream/main. Not yet present in 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants