Take `Full-Snapshot` via `EtcdOpsTask` while hibernating and performing etcd version upgrade. by Shreyas-s14 · Pull Request #1300 · gardener/etcd-druid

Shreyas-s14 · 2026-03-02T05:36:26Z

How to categorize this PR?

/area disaster-recovery
/area control-plane
/area compliance
/kind enhancement

What this PR does / why we need it:
This PR does the below 2 things:

Performs a Full Snapshot Operation via EtcdOpsTask upon hibernation of the etcd resource (i.e spec.Replicas=0) with a backoff of 3.
Performs a Full Snapshot Operation via EtcdOpsTask upon etcd upgrade with a backoff of 3. This Etcd upgrade is carried out by:
- Introducing a feature gate in etcd-druid to carry out this procedure.
- Maintaining multiple images for etcd-wrapper and etcd-backup-restore and selecting the right one based on whether the feature-gate is set.

Which issue(s) this PR fixes:
Fixes Partially #445

Checklist:

Update documentation in the /docs folder (if applicable)
- If new files added to docs, or docs structure modified:
  - Update mkdocs.yml. Follow Updating Documentation guide to test the changes locally.
  - Update Table of Contents for the docs.
Add tests that cover your changes (if applicable)
- Unit tests
- Integration tests
- E2E tests

Special notes for your reviewer:
As of now, the PR for etcd-upgrade in etcd-wrapper and etcd-backup-restore have not been merged. Hence custom images have been used. Once they are merged, the images will be replaced with the official built ones.

Release note:

Added feature-gate for etcd upgrade to 3.5.26. Take Full Snapshot via EtcdOpsTask on hibernation of etcd and etcd upgrade via feature-gate.

anveshreddy18

Thanks for the PR. I have the following comments for now, PTAL. I'll be adding more as I'm reviewing other files.

api/config/v1alpha1/features_test.go

internal/component/configmap/etcdconfig.go

internal/component/statefulset/statefulset.go

charts/templates/operatorconfigmap.yaml

…rnation and etcd upgrade

internal/component/statefulset/statefulset.go

internal/component/statefulset/statefulset_test.go

anveshreddy18

Thanks for addressing all my review comments @Shreyas-s14.
/lgtm

gardener-prow · 2026-03-06T07:19:14Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: anveshreddy18
Once this PR has been reviewed and has the lgtm label, please assign unmarshall for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

CaptainIRS

@Shreyas-s14 thanks for the PR! I have a few comments, PTAL, thanks.

CaptainIRS · 2026-03-06T06:17:05Z

internal/component/statefulset/statefulset_test.go

+
+			if tc.expectedNil {
+				g.Expect(syncErr).ToNot(HaveOccurred())
+			} else if tc.expectedRequeue {


tc.expectedRequeue isn't being set anywhere, so this would always be false. Also I think the tc.expectedNil check should have an else component since if you change the expectedNil values in the current tests, the tests still pass.

CaptainIRS · 2026-03-06T07:19:22Z

internal/component/statefulset/statefulset.go

+			Config: druidv1alpha1.EtcdOpsTaskConfig{
+				OnDemandSnapshot: &druidv1alpha1.OnDemandSnapshotConfig{Type: druidv1alpha1.OnDemandSnapshotTypeFull},
+			},
+			TTLSecondsAfterFinished: ptr.To(int32(36)), // 48 hours


36 seems too low, wondering if this change is intentional?

Oh thanks for the catch
It was supposed to be 2 days, but for testing purposes, i made it to 36 secs to see deletion flows. Will change

CaptainIRS · 2026-03-06T07:26:00Z

internal/component/statefulset/statefulset.go

+
+	// Pre-sync snapshot task constants
+	preSyncTaskPrefixHibernation = "presync-snapshot-hibernation-"
+	preSyncTaskPrefixUpgrade     = "presync-snapshot-upgrade-"


Should we rename this to something generic since the pre-sync task also executes for any version change (both upgrade and downgrade)?

as of now, we do not take snapshots on downgrade.

Ah, I missed the return nil in the if !druidconfigv1alpha1.DefaultFeatureGates.IsEnabled(druidconfigv1alpha1.UpgradeEtcdVersion) { block, sorry.
Though, I wonder if we should be taking backups on downgrade (i.e if the feature flag is set to false and reconciliation is triggered) since the etcd doc suggests so in https://etcd.io/docs/v3.5/downgrades/downgrade_3_5/#server-downgrade-checklists

CaptainIRS · 2026-03-06T07:45:34Z

internal/common/constants.go

+	// ImageKeyEtcdV3_5 is the key for the etcd v3.5 image in the image vector.
+	ImageKeyEtcdWrapperV3_5 = "etcd-wrapper-3_5"
+	// ImageKeyEtcdBackupRestoreV3_5 is the key for the etcd-backup-restore v3.5 image in the image vector.
+	ImageKeyEtcdBackupRestoreV3_5 = "etcd-backup-restore-3_5"


Just a thought: could we instead name it generic like ImageKeyEtcdWrapperNext so that in the future if we with to perform upgrades, we could just use the same mechanism to roll out upgrades in landscapes?

CaptainIRS · 2026-03-06T07:46:42Z

internal/component/statefulset/statefulset_test.go

+			stsReplicas:        3,
+			etcdReplicas:       0,
+			etcdWrapperImage:   currentImage,
+			existingTasks:      []*druidv1alpha1.EtcdOpsTask{buildPreSyncTask(preSyncTaskPrefixHibernation, 2, ptr.To(druidv1alpha1.TaskStateFailed))},


Could we have this reference the constant instead like maxPreSyncRetries - 1? (same for other entries below as well)

CaptainIRS · 2026-03-06T07:50:35Z

test/it/controller/etcd/assertions.go

+		}
+		return nil
+	}
+	g.Eventually(checkFn).Within(timeout).WithPolling(pollInterval).WithContext(ctx).Should(BeNil())


Suggested change

g.Eventually(checkFn).Within(timeout).WithPolling(pollInterval).WithContext(ctx).Should(BeNil())

g.Eventually(checkFn).Within(timeout).WithPolling(pollInterval).WithContext(ctx).Should(Succeed())

CaptainIRS · 2026-03-06T07:53:35Z

api/config/v1alpha1/features.go

 	// This is now GA. Any attempt to disable this feature will be an error.
 	UseEtcdWrapper = "UseEtcdWrapper"
+
+	// UpgradeEtcdVersionTo is the name of the feature which enables upgrade of etcd version to v3.5.


Suggested change

// UpgradeEtcdVersionTo is the name of the feature which enables upgrade of etcd version to v3.5.

// UpgradeEtcdVersion is the name of the feature which enables upgrade of etcd version to v3.5.

ishan16696 · 2026-03-05T18:46:27Z

api/config/v1alpha1/features.go

 	UseEtcdWrapper = "UseEtcdWrapper"
+
+	// UpgradeEtcdVersionTo is the name of the feature which enables upgrade of etcd version to v3.5.
+	UpgradeEtcdVersion = "UpgradeEtcdVersion"


can you also update the doc here https://github.com/gardener/etcd-druid/blob/master/docs/deployment/feature-gates.md#feature-gates-for-alpha-or-beta-features

ishan16696 · 2026-03-06T07:56:44Z

internal/images/images.yaml

+  sourceRepository: github.com/gardener/etcd-wrapper
+  repository: shreyas14/wrapper


adding this comment as reminder to update the image here.

ishan16696 · 2026-03-06T07:57:12Z

internal/images/images.yaml

+#TODO: @Shreyas-s14 Change the image to etcd-backup-restore image once PR is merged
+- name: etcd-backup-restore-3_5
+  resourceId:
+    name: 'etcdbrctl-3_5'
+  sourceRepository: github.com/gardener/etcd-backup-restore
+  repository: shreyas14/backup-restore


same as above

ishan16696 · 2026-03-06T11:52:53Z

/hold until etcd-wrapper and etcd-backup-restore release as image tag need to be updated in this PR.

ishan16696 · 2026-03-06T12:17:56Z

wording of release note doesn't seems to be correct:

Take Full Snapshot via EtcdOpsTask on hibernation of etcd and etcd upgrade via feature-gate.

I think you're taking full snapshot before hibernation via EtcdOpsTask irrespective of feature gate. I think it better to mention this as separate release note.

ishan16696 · 2026-03-06T12:11:41Z

internal/component/statefulset/statefulset.go

+	if etcd.Spec.Replicas == 0 {
+		return r.ensurePreSyncSnapshot(ctx, etcd, preSyncTaskPrefixHibernation)
+	}


Suggested change

if etcd.Spec.Replicas == 0 {

return r.ensurePreSyncSnapshot(ctx, etcd, preSyncTaskPrefixHibernation)

}

if etcd.Spec.Replicas == 0 && *existingSts.Spec.Replicas > 0{

return r.ensurePreSyncSnapshot(ctx, etcd, preSyncTaskPrefixHibernation)

}

if we are taking a full snapshot before hibernation, isn't it make sense to also check that sts spec.replicas is not 0 yet, wdyt ?

Shreyas-s14 requested a review from a team as a code owner March 2, 2026 05:36

federated-github-access bot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 2, 2026

github-actions bot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. and removed ok-to-test Indicates a non-member PR verified by an org member that is safe to test. labels Mar 2, 2026

anveshreddy18 self-assigned this Mar 2, 2026

anveshreddy18 added this to the v0.36.0 milestone Mar 2, 2026

anveshreddy18 requested changes Mar 4, 2026

View reviewed changes

anveshreddy18 reviewed Mar 4, 2026

View reviewed changes

internal/component/statefulset/statefulset.go Outdated Show resolved Hide resolved

charts/templates/operatorconfigmap.yaml Show resolved Hide resolved

Shreyas-s14 added 2 commits March 5, 2026 09:10

feature to take full snapshot via etcdopstask during presync for hibe…

e268897

…rnation and etcd upgrade

rebase with master + addressed review comments by @anveshreddy18

36248bc

Shreyas-s14 force-pushed the etcd-upgr-snapshot branch from ddafda0 to 36248bc Compare March 5, 2026 09:06

anveshreddy18 requested changes Mar 5, 2026

View reviewed changes

Shreyas-s14 added 2 commits March 6, 2026 10:07

addressed review comments from @anveshreddy18

a5e739f

fixed flaky api unit tests due to shared test env

de8a6f0

anveshreddy18 approved these changes Mar 6, 2026

View reviewed changes

gardener-prow bot added the lgtm Indicates that a PR is ready to be merged. label Mar 6, 2026

CaptainIRS requested changes Mar 6, 2026

View reviewed changes

gardener-prow bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 6, 2026

gardener-prow bot requested a review from anveshreddy18 March 6, 2026 08:00

CaptainIRS self-assigned this Mar 6, 2026

ishan16696 reviewed Mar 6, 2026

View reviewed changes

gardener-prow bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 6, 2026

ishan16696 reviewed Mar 6, 2026

View reviewed changes

	g.Eventually(checkFn).Within(timeout).WithPolling(pollInterval).WithContext(ctx).Should(BeNil())
	g.Eventually(checkFn).Within(timeout).WithPolling(pollInterval).WithContext(ctx).Should(Succeed())

	// UpgradeEtcdVersionTo is the name of the feature which enables upgrade of etcd version to v3.5.
	// UpgradeEtcdVersion is the name of the feature which enables upgrade of etcd version to v3.5.

		sourceRepository: github.com/gardener/etcd-wrapper
		repository: shreyas14/wrapper

Conversation

Shreyas-s14 commented Mar 2, 2026 • edited by anveshreddy18 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anveshreddy18 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

anveshreddy18 left a comment

Choose a reason for hiding this comment

Uh oh!

gardener-prow bot commented Mar 6, 2026

Uh oh!

CaptainIRS left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ishan16696 commented Mar 6, 2026

Uh oh!

ishan16696 commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Shreyas-s14 commented Mar 2, 2026 •

edited by anveshreddy18

Loading

ishan16696 commented Mar 6, 2026 •

edited

Loading