Extend Etcd CRD and etcd-druid to Support Configurable Delta Snapshot Retention
#651
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
gardener-robot
added
area/backup
gardener-robot-ci-1
added
reviewed/ok-to-test
seshachalam-yv
force-pushed
the
feature/add-delta-snapshot-retention-period
branch
from
524e4e9 to
00b2df5
Compare
gardener-robot-ci-1
added
reviewed/ok-to-test
shreyas-s-rao
requested changes
Jul 31, 2023
There was a problem hiding this comment.
@seshachalam-yv thanks for the PR. While the PR adds the new field to the Etcd CR, druid does not use the value anywhere. Please ensure that druid creates the etcd statefulset with this field's value correctly passed to the etcdbrctl command.
|
/retest |
|
/test pull-etcd-druid-e2e-kind-alpha-features |
|
@aaronfern You have pull request review open invite, please check |
seshachalam-yv
force-pushed
the
feature/add-delta-snapshot-retention-period
branch
from
00b2df5 to
da9d680
Compare
gardener-robot
added
size/m
gardener-robot-ci-3
added
reviewed/ok-to-test
Updated etcd-druid to propagate --delta-snapshot-retention-period flag from the Etcd CRD to etcd-backup-restore.
seshachalam-yv
force-pushed
the
feature/add-delta-snapshot-retention-period
branch
from
da9d680 to
072cea6
Compare
gardener-robot-ci-3
added
reviewed/ok-to-test
seshachalam-yv
changed the title
DeltaSnapshotRetentionPeriod Field to Etcd CRDEtcd CRD and etcd-druid to Support Configurable Delta Snapshot Retention
shreyas-s-rao
requested changes
Aug 4, 2023
There was a problem hiding this comment.
@seshachalam-yv thanks for your PR. I have a few comments. PTAL.
Please also change the release note to something like:
Introduce `Spec.Backup.DeltaSnapshotRetentionPeriod` in the `Etcd` resource to allow configuring retention period for delta snapshots.
(Release note does not need to have implementation details of the API, but just the general usage)
gardener-robot-ci-1
added
reviewed/ok-to-test
gardener-robot-ci-3
added
the
reviewed/ok-to-test
gardener-robot-ci-2
removed
the
reviewed/ok-to-test
|
@aaronfern You have pull request review open invite, please check |
gardener-robot
added
reviewed/lgtm
gardener-robot-ci-3
added
reviewed/ok-to-test
gardener-robot
added
the
status/closed
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.
How to categorize this PR?
/area backup
/kind api-change
What this PR does / why we need it:
This PR extends the Etcd CRD to include a
DeltaSnapshotRetentionPeriodfield. This new field allows users to define the duration for retaining old delta snapshots, providing more flexibility in managing backup retention. Additionally, this PR modifies etcd-druid to propagate the --delta-snapshot-retention-period flag from the Etcd CRD to the etcd-backup-restore component.This change allows the etcd-backup-restore to have access to the
DeltaSnapshotRetentionPeriodconfiguration which was not previously propagated. As such, it provides greater flexibility in the configuration of backup parameters.Which issue(s) this PR fixes:
Fixes #649
Special notes for your reviewer:
Release note: