This repository has been archived by the owner on Apr 28, 2022. It is now read-only.
Fix max backup deletion #26
Closed
Conversation
This file contains 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
added 2 commits
August 28, 2019 15:26
…n of only the oldest backups when rotating When rotating backups based on a maximum number allowed, newer backups were being deleted when the number of digits in an etcd store revision increased. E.g., when "v99" became "v100", the newer "v100" backup was being deleted, because the "1" in "100" was sorted to come before the first "9" in "99". This new sorting method relies on the timestamp in each backup path, rather than the revision number. The reason the timestamp is given precedence over the revision when sorting is because the revision could potentially go backwards when an older backup is restored. See my comment in pkg/backup/util/util.go's SortableBackupPaths type for more details. Fixes coreos#2113
|
@mdkrajewski could you maybe recreate this PR yourself? |
@Marlinc I'd be happy to do that if you like. Just now I tried, but GitHub saw that it was the same as this PR, and didn't let me create a duplicate. Is there some reason why it would be better for me to recreate the PR in this repo instead of you? |
|
Hey @mdkrajewski, Thanks for checking, if you open the PR then it will be attributed to you instead of me. I'll close the PR on this side. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Copy of coreos#2116 by @mdkrajewski
etcd-backup-operator: fixed sorting of etcd backups to ensure deletion of only the oldest backups when rotating
Issue: When rotating backups based on a maximum number allowed, newer backups were being deleted when the number of digits in an etcd store revision increased.
E.g., when "v99" became "v100", the newer "v100" backup was being deleted, because the "1" in "100" was sorted to come before the first "9" in "99".
Fix: This new sorting method relies on the timestamp in each backup path, rather than the revision number.
The reason the timestamp is given precedence over the revision when sorting is because the revision could potentially go backwards when an older backup is restored.
See my comment in pkg/backup/util/util.go's SortableBackupPaths type for more details.
Fixes coreos#2113