fix(slots): cleanup replication slots on replicas when features disabled by mnencia · Pull Request #9381 · cloudnative-pg/cloudnative-pg

mnencia · 2025-12-07T10:53:18Z

When both replicationSlots.highAvailability.enabled and replicationSlots.synchronizeReplicas.enabled are disabled, the Replicator now performs cleanup by calling reconcile() before continuing the loop, ensuring user-defined replication slots are properly removed from replica pods.

This change also adds webhook validation to prevent users from removing
the synchronizeReplicas configuration section while it is still enabled.
This ensures that cleanup logic can properly execute before the
configuration is removed, preventing orphaned replication slots on
replicas.

Closes #9380

TODO

document the known limitations: when excludedPatterns is modified the operator isn't able to compare the difference with the previous this my cause some lingering rep slots

github-actions · 2025-12-07T10:53:29Z

❗ By default, the pull request is configured to backport to all release branches.

To stop backporting this pr, remove the label: backport-requested ◀️ or add the label 'do not backport'
To stop backporting this pr to a certain release branch, remove the specific branch label: release-x.y

mnencia · 2025-12-07T10:59:06Z

/test

github-actions · 2025-12-07T10:59:17Z

@mnencia, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/20003199261

armru · 2025-12-10T10:36:47Z

/test limit=local

github-actions · 2025-12-10T10:36:57Z

@armru, here's the link to the E2E on CNPG workflow run: https://github.com/cloudnative-pg/cloudnative-pg/actions/runs/20095657321

Copilot

Pull request overview

This PR fixes cleanup of user-defined replication slots on replica pods when replication slot features are disabled. It adds webhook validation to prevent configuration removal while features are enabled and ensures cleanup reconciliation runs before the replicator continues its loop.

Key Changes:

Modified replication slot runner to execute cleanup reconciliation when features are disabled
Added webhook validation preventing removal of synchronizeReplicas configuration while enabled
Enhanced test coverage for new synchronizeReplicas validation rules

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
internal/management/controller/slots/runner/runner.go	Added cleanup reconciliation call when replication features are disabled to properly remove user-defined slots from replicas
internal/webhook/v1/cluster_webhook.go	Added validation to prevent removing synchronizeReplicas configuration section while the feature is enabled, ensuring cleanup can execute properly
internal/webhook/v1/cluster_webhook_test.go	Added comprehensive test cases for new synchronizeReplicas validation rules, including scenarios for removing configuration sections and disabling features

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

internal/webhook/v1/cluster_webhook.go

internal/webhook/v1/cluster_webhook_test.go

When both replicationSlots.highAvailability.enabled and replicationSlots.synchronizeReplicas.enabled are disabled, the Replicator now performs cleanup by calling reconcile() before continuing the loop, ensuring user-defined replication slots are properly removed from replica pods. Closes #9380 Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>

This change also adds webhook validation to prevent users from removing the synchronizeReplicas configuration section while it is still enabled. This ensures that cleanup logic can properly execute before the configuration is removed, preventing orphaned replication slots on replicas. Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>

leonardoce · 2025-12-15T15:14:14Z

After a chat with @armru we found that the improved webhook doesn't really check prevent the mistake from happening because it will run after the defaulting webhook, and the defaulting webhook will just set the cluster.spec.replicationSlots.highAvailability and cluster.spec.replicationSlots.synchronizeReplicas if they are not set.

The condition in https://github.com/cloudnative-pg/cloudnative-pg/pull/9381/files#diff-30bf0aa461c01460908ae409672d7d5be2a1d9065620671ea75ef81a5e9cec03R2311 won't ever be met because of that.

To cover this, we need to keep the old version of the cluster.spec.replicationSlots.highAvailability stanza in the cluster status to clean up only the replication slots that are managed by the operator. This will be addressed in a subsequent commit.

…led (#9381) When both replicationSlots.highAvailability.enabled and replicationSlots.synchronizeReplicas.enabled are disabled, the Replicator now performs cleanup by calling reconcile() before continuing the loop, ensuring user-defined replication slots are properly removed from replica pods. This change also adds webhook validation to prevent users from removing the synchronizeReplicas configuration section while it is still enabled. This ensures that cleanup logic can properly execute before the configuration is removed, preventing orphaned replication slots on replicas. Closes #9380 Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com> Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com> Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com> (cherry picked from commit b7c9690)

mnencia requested a review from a team as a code owner December 7, 2025 10:53

dosubot bot added the size:XS This PR changes 0-9 lines, ignoring generated files. label Dec 7, 2025

cnpg-bot added backport-requested ◀️ This pull request should be backported to all supported releases release-1.25 release-1.26 release-1.27 labels Dec 7, 2025

dosubot bot added bug 🐛 Something isn't working ok to merge 👌 This PR can be merged labels Dec 7, 2025

mnencia removed the ok to merge 👌 This PR can be merged label Dec 7, 2025

cnpg-bot added the ok to merge 👌 This PR can be merged label Dec 7, 2025

dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Dec 10, 2025

armru approved these changes Dec 10, 2025

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 10, 2025

Copilot AI review requested due to automatic review settings December 11, 2025 09:36

Copilot started reviewing on behalf of mnencia December 11, 2025 09:36 View session

mnencia force-pushed the dev/9380 branch from a46c2db to 0837a0d Compare December 11, 2025 09:40

Copilot AI reviewed Dec 11, 2025

View reviewed changes

internal/webhook/v1/cluster_webhook.go Outdated Show resolved Hide resolved

internal/webhook/v1/cluster_webhook_test.go Show resolved Hide resolved

mnencia force-pushed the dev/9380 branch 2 times, most recently from 8fe975a to a49b60a Compare December 11, 2025 10:04

mnencia added release-1.28 and removed release-1.26 labels Dec 11, 2025

mnencia and others added 3 commits December 15, 2025 13:18

tests: improve coverage

3198161

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>

chore: spelling and grammar

5d86138

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>

leonardoce force-pushed the dev/9380 branch from a49b60a to 5d86138 Compare December 15, 2025 12:18

leonardoce approved these changes Dec 15, 2025

View reviewed changes

leonardoce merged commit b7c9690 into main Dec 15, 2025
31 of 34 checks passed

leonardoce deleted the dev/9380 branch December 15, 2025 15:15

github-actions bot mentioned this pull request Dec 15, 2025

Backport failure for pull request 9381 #9438

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(slots): cleanup replication slots on replicas when features disabled#9381

fix(slots): cleanup replication slots on replicas when features disabled#9381
leonardoce merged 4 commits intomainfrom
dev/9380

mnencia commented Dec 7, 2025 •

edited by armru

Loading

Uh oh!

github-actions bot commented Dec 7, 2025

Uh oh!

mnencia commented Dec 7, 2025

Uh oh!

github-actions bot commented Dec 7, 2025

Uh oh!

armru commented Dec 10, 2025

Uh oh!

github-actions bot commented Dec 10, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

leonardoce commented Dec 15, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

mnencia commented Dec 7, 2025 • edited by armru Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO

Uh oh!

github-actions bot commented Dec 7, 2025

Uh oh!

mnencia commented Dec 7, 2025

Uh oh!

github-actions bot commented Dec 7, 2025

Uh oh!

armru commented Dec 10, 2025

Uh oh!

github-actions bot commented Dec 10, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

leonardoce commented Dec 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mnencia commented Dec 7, 2025 •

edited by armru

Loading

leonardoce commented Dec 15, 2025 •

edited

Loading