K8SPSMDB-1202 add logs for scheduled backup cleanup#2381
Merged
Conversation
nmarukovich
requested review from
egegunes,
gkech,
hors,
mayankshah1607,
oksana-grishchenko and
pooknull
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds additional structured logging around scheduled backup retention cleanup in the Percona Server for MongoDB operator, improving observability when old scheduled backups are deleted.
Changes:
- Log when retention-count cleanup is about to remove scheduled backups.
- Log each backup deletion and include the backup name in deletion error logs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+167
to
+170
| if len(oldjobs) > 0 { | ||
| log.Info("cleaning up old backups based on retention count", | ||
| "job", item.Name, "keep", ret.Count, "toDelete", len(oldjobs)) | ||
| } |
Comment on lines
172
to
+176
| for _, todel := range oldjobs { | ||
| log.Info("deleting outdated backup", "job", item.Name, "backup", todel.Name) | ||
| err = r.client.Delete(ctx, &todel) | ||
| if err != nil { | ||
| log.Error(err, "failed to delete backup object") | ||
| log.Error(err, "failed to delete backup object", "backup", todel.Name) |
egegunes
changed the title
egegunes
previously approved these changes
Jun 5, 2026
hors
previously approved these changes
Jun 5, 2026
Comment on lines
+167
to
+170
| if len(oldjobs) > 0 { | ||
| log.Info("cleaning up old backups based on retention count", | ||
| "job", item.Name, "keep", ret.Count, "toDelete", len(oldjobs)) | ||
| } |
gkech
reviewed
Jun 5, 2026
Comment on lines
+167
to
+170
| if len(oldjobs) > 0 { | ||
| log.Info("cleaning up old backups based on retention count", | ||
| "job", item.Name, "keep", ret.Count, "toDelete", len(oldjobs)) | ||
| } |
Contributor
There was a problem hiding this comment.
Why do we have this log when, at the same time we are iterating oldjobs and we generate one log for each old job we have?
gkech
reviewed
Jun 5, 2026
| } | ||
|
|
||
| for _, todel := range oldjobs { | ||
| log.Info("deleting outdated backup", "job", item.Name, "backup", todel.Name) |
Contributor
There was a problem hiding this comment.
I would't say that these old jobs are outdated, they are not. They are just exceeding retention. Maybe we can say exceeding retention
Contributor
There was a problem hiding this comment.
suggestion: deleting backup exceeding retention
pooknull
approved these changes
Jun 9, 2026
egegunes
approved these changes
Jun 10, 2026
Comment on lines
+168
to
172
| log.Info("deleting backup exceeding retention", "job", item.Name, "backup", todel.Name) | ||
| err = r.client.Delete(ctx, &todel) | ||
| if err != nil { | ||
| log.Error(err, "failed to delete backup object") | ||
| log.Error(err, "failed to delete backup object", "backup", todel.Name) | ||
| return true |
gkech
approved these changes
Jun 10, 2026
Comment on lines
+168
to
172
| log.Info("deleting backup exceeding retention", "job", item.Name, "backup", todel.Name) | ||
| err = r.client.Delete(ctx, &todel) | ||
| if err != nil { | ||
| log.Error(err, "failed to delete backup object") | ||
| log.Error(err, "failed to delete backup object", "backup", todel.Name) | ||
| return true |
Collaborator
commit: c451228 |
Comment on lines
+168
to
172
| log.Info("deleting backup exceeding retention", "job", item.Name, "backup", todel.Name) | ||
| err = r.client.Delete(ctx, &todel) | ||
| if err != nil { | ||
| log.Error(err, "failed to delete backup object") | ||
| log.Error(err, "failed to delete backup object", "backup", todel.Name) | ||
| return true |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CHANGE DESCRIPTION
Problem:
Short explanation of the problem.
Logs about scheduled clean up were added.
Cause:
Short explanation of the root cause of the issue if applicable.
Solution:
Short explanation of the solution we are providing with this PR.
CHECKLIST
Jira
Needs Doc) and QA (Needs QA)?Tests
compare/*-oc.yml)?Config/Logging/Testability