K8SPG-992: remove keep-job finalizer if cluster is deleted - #1724
K8SPG-992: remove keep-job finalizer if cluster is deleted#1724pooknull wants to merge 7 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the PGBackup controller finalization flow to ensure backup Jobs don’t remain stuck with the keep-job finalizer when the owning cluster is gone (or being deleted), allowing those Jobs to be cleaned up properly.
Changes:
- Remove the
keep-jobfinalizer from the backup Job during backup finalization when the cluster is unavailable (pg == nil). - Extend unit test coverage to create a Job with
keep-joband assert the finalizer gets removed when the cluster is unavailable.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
percona/controller/pgbackup/controller.go |
Adds logic to remove the Job’s keep-job finalizer when the cluster is unavailable during backup finalization. |
percona/controller/pgbackup/controller_test.go |
Updates tests to include a Job object and verifies the keep-job finalizer is removed in the cluster-unavailable scenario. |
| // If the cluster was previously deleted, we cannot call finishBackup to remove | ||
| // annotations from the PGCluster. In that case, we no longer need the job to | ||
| // exist and we can remove the keep-job finalizer. |
There was a problem hiding this comment.
I don't understand why the removal of keep-job is tied to the execution of delete-backup finalizer. Doesn't this mean we will still have this issue if the delete-backup finalizer is not present?
Shouldn't we instead set a watch on the Job and Cluster, requeue events for corresponding PGBackup and at the top of Reconcile remove the finalizer on the Job when pgCluster == nil (or if backup is in a terminal state)?
There was a problem hiding this comment.
The delete-backup finalizer is added before the backup job is created. So a backup job with keep-job cannot exist without the corresponding pgbackup having the delete-backup finalizer.
The reason for tying keep-job removal to the delete-backup finalizer is the idea that delete-backup owns the backup cleanup sequence: it removes the backup labels and annotations from the cluster and then releases keep-job in the required order. If the cluster no longer exists, releasing keep-job is still the remaining part of the same cleanup operation, so it belongs in this finalizer.
Moving this logic to the top of Reconcile would not change the behavior unless someone manually removed the delete-backup finalizer, which users should not touch.
commit: 9924e38 |
https://perconadev.atlassian.net/browse/K8SPG-992
DESCRIPTION
Problem:
When cluster is deleted while a backup is running, the backup job can keep the
keep-jobfinalizer, preventing the job from being deleted.Solution:
delete-backupfinalizer should find the backup job if the cluster is unavailable and remove thekeep-jobfinalizer.CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
Config/Logging/Testability