Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update etcdutl restore to create v2 snapshot from v3 #16376

Merged
merged 1 commit into from
Aug 14, 2023
Merged

Conversation

geetasg
Copy link

@geetasg geetasg commented Aug 4, 2023

for i := range epc.Procs[0].Config().Args {
if epc.Procs[0].Config().Args[i] == "--data-dir" {
epc.Procs[0].Config().Args[i+1] = newDataDir
}
}

//Verify that initial snapshot is created by the restore operation
verifySnapshot(t, epc)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a verification that the membership data read from the v2 snapshot is expected?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated.

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a nit.

thx

tests/e2e/cluster_downgrade_test.go Outdated Show resolved Hide resolved
Copy link
Member

@jmhbnz jmhbnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall lgtm, nice work @geetasg 👍🏻

tests/e2e/ctl_v3_snapshot_test.go Outdated Show resolved Hide resolved
Signed-off-by: Geeta Gharpure <geetagh@amazon.com>
for i := range epc.Procs[0].Config().Args {
if epc.Procs[0].Config().Args[i] == "--data-dir" {
epc.Procs[0].Config().Args[i+1] = newDataDir
}
}

// Verify that initial snapshot is created by the restore operation
verifySnapshotMembers(t, epc, membersBefore)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cluster in this test case has only one member, please consider to add a case verify that the members read from the snapshot are also expected in multiple members cluster in a separate PR.

@ahrtr ahrtr merged commit 731048d into etcd-io:main Aug 14, 2023
27 checks passed
@geetasg geetasg mentioned this pull request Sep 10, 2023
21 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants