Cleanup the legacy v2 snapshot files on bootstrap

Project: etcd-io/etcd | Language: Go | Concepts: refactoring, testing-strategy, error-handling | Difficulty: mid
Link: https://github.com/etcd-io/etcd/pull/22336

The problem

etcd previously supported v2 snapshot files, but support for these files has been removed. However, old v2 snapshot files (.snap) could still remain in the data directory, which are now unused and should be cleaned up automatically when etcd starts.

Why it's non-trivial

A naive fix might just delete files with a .snap extension, but this risks removing files that are still in use or not related to legacy v2 snapshots. The cleanup should only target obsolete v2 snapshot files, ensure the operation is safe, and handle errors properly. Additionally, the change needs to be tested to confirm that the cleanup happens as expected without affecting other functionality.

The approach

1. Cleanup logic added to bootstrap

The PR adds code to remove legacy v2 .snap files from the snapshot directory during server bootstrap:

+	// TODO: we can remove this code in the next release etcd v3.9
+	if err := fileutil.RemoveMatchFile(cfg.Logger, cfg.SnapDir(), func(fileName string) bool {
+		return strings.HasSuffix(strings.ToLower(fileName), ".snap")
+	}); err != nil {
+		cfg.Logger.Error(
+			"failed to remove v2 snapshot file(s) in snapshot directory",
+			zap.String("path", cfg.SnapDir()),
+			zap.Error(err),
+		)
+	} else {
+		cfg.Logger.Info("cleaned up all legacy v2 snapshot files")
+	}

• This uses fileutil.RemoveMatchFile to delete files with the .snap extension (case-insensitive) in the snapshot directory.
• If an error occurs, it logs an error with the path and error details.
• On success, it logs that the cleanup was performed.
• The comment indicates this code is temporary and can be removed in a future release (v3.9 or later).

2. Test added to verify cleanup

A new test ensures that legacy v2 snapshot files are deleted on bootstrap:

+// TestCleanupV2SnapshotOnBootstrap verifies that etcd cleans up the legacy
+// v2 snapshot files on bootstrap.
+// TODO: we can remove this test in the next etcd release v3.9
+func TestCleanupV2SnapshotOnBootstrap(t *testing.T) {
+	e2e.BeforeTest(t)
+
+	t.Log("Create a new single member etcd cluster")
+	cfg := e2e.ConfigStandalone(*e2e.NewConfig(
+		e2e.WithKeepDataDir(true),
+	))
+	epc, err := e2e.NewEtcdProcessCluster(t.Context(), t, e2e.WithConfig(cfg))
+	require.NoError(t, err)
+	defer func() {
+		assert.NoError(t, epc.Close())
+	}()
+
+	t.Log("Stop the etcd member, and create a legacy v2 snapshot file")
+	require.NoError(t, epc.Procs[0].Stop())
+
+	snapshotDir := datadir.ToSnapDir(epc.Procs[0].Config().DataDirPath)
+	require.NoError(t, os.WriteFile(filepath.Join(snapshotDir, "10.snap"), []byte{}, 0o644))
+
+	names, rerr := fileutil.ReadDir(snapshotDir, fileutil.WithExt(".snap"))
+	require.NoError(t, rerr)
+	require.Len(t, names, 1)
+
+	t.Log("Start the etcd member again, and expect it removes the legacy v2 snapshot file automatically")
+	require.NoError(t, epc.Procs[0].Start(t.Context()))
+
+	names, rerr = fileutil.ReadDir(snapshotDir, fileutil.WithExt(".snap"))
+	require.NoError(t, rerr)
+	require.Empty(t, names)
+}

• The test creates a cluster, stops etcd, manually adds a .snap file, and verifies its presence.
• After restarting etcd, it checks that the .snap file has been deleted, confirming the cleanup logic works.

3. Minor improvements from review feedback

• The test uses os.WriteFile to create the .snap file, ensuring the file descriptor is not leaked.
• Comments and TODOs were updated to clarify when the cleanup code and test can be removed.

From the review thread

fuweid: Maybe we should use os.WriteFile here? GC can cleanup the FD but it looks leaky. :)

serathius: Why v4.0? Can we just suggest next release after v3.8 like v3.9

ahrtr: Good catch, updated to use os.WriteFile & thx

ahrtr: Updated the comment to remove in v3.9. It should be OK to remove in any release >= 3.9

Takeaways

• When removing obsolete files, use targeted matching (e.g., file extension and directory) to avoid deleting unrelated or active files.
• Always add automated tests to verify cleanup or migration logic, especially for transitional code that will be removed in the future.
• Handle file operations carefully: use safe APIs and close resources to avoid leaks, even in test code.
• Clearly document temporary code and tests with TODOs and version targets for future removal.