fix: fsync snap directory when saving a received snapshot db

[gyuho]

SaveDBFrom synced a received snapshot database before renaming it into place, but did not sync the containing directory. Linux fsync(2) documents that syncing a file does not necessarily persist its directory entry; the directory must also be synced.

The snapshot receiver calls SaveDBFrom before processing the Raft message, which can later sync the WAL snapshot record. Sync the snapshot directory after rename and on the existing-file retry path, and return sync errors so snapshot handling stops before Raft processing.

Add direct unit coverage that both SaveDBFrom paths invoke directory sync and propagate failures. Keep gofail E2E coverage scoped to failure/retry and crash-before-return control flow; remove tests and claims that did not establish directory-entry durability.

[kubernetes-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: gyuho
Once this PR has been reviewed and has the lgtm label, please assign jmhbnz 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

[kubernetes-prow]

Hi @gyuho. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[gyuho]

To clarify the sequence, 1. etcd receives a snapshot, 2. writes it to a temp file, 3. fsyncs the file, 4. renames it into place as snap/db -- but never fsynced the snap/ directory. A power-off crash in that window could lose the rename, so the freshly received snapshot silently "disappears" on restart.

[gyuho]

Was also able to reproduce this using something like GOFAIL_FAILPOINTS=applyBeforeOpenSnapshot=sleep("60s") + echo b > /proc/sysrq-trigger. Here are another sequence that I tested:

• failpoint on member-3: pause 60 seconds right before it opens a new snap.db.
• Stop member-3's etcd. Write keys to the cluster and compact the log. Start member-3, to trigger the leader to send it a full snapshot.
• Member-3 saves snap.db to disk and pauses, as instructed. File exists; folder never fsynced.
• SIGKILL member-3's etcd process (and stop the systemd unit, so it can't restart mid-surgery).
• Then, still on member-3 over SSM, run rm -f /var/lib/etcd/member/snap/*.snap.db. The machine never crashed -- deleting the file by hand creates the exact disk state a power cut would leave. The kill has to come first so no running process is holding or rewriting that file.
• Start member-3. It finds the saved-snapshot record, looks for snap.db, finds nothing, panics

[fuweid]

it could happen. I run into such kind of issues in production several times for containerd snapshot files. It's easy to reproduce it with https://github.com/etcd-io/bbolt/tree/main/tests/dmflakey for power-off.

[codecov]

Codecov Report

❌ Patch coverage is 88.88889% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.71%. Comparing base (f744d45) to head (21b7d75).
⚠️ Report is 22 commits behind head on main.

Files with missing lines	Patch %	Lines
server/etcdserver/api/snap/db.go	86.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
server/etcdserver/api/snap/snapshotter.go	68.49% <100.00%> (+1.59%)	⬆️
server/etcdserver/api/snap/db.go	70.96% <86.66%> (+9.26%)	⬆️

... and 32 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #22314      +/-   ##
==========================================
+ Coverage   69.66%   69.71%   +0.04%     
==========================================
  Files         449      448       -1     
  Lines       38193    38175      -18     
==========================================
+ Hits        26608    26614       +6     
+ Misses      10162    10131      -31     
- Partials     1423     1430       +7     

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f744d45...21b7d75. Read the comment docs.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[kubernetes-prow]

@gyuho: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-etcd-e2e-amd64	21b7d75	link	true	/test pull-etcd-e2e-amd64

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[gyuho]

/retest