Skip to content

fix(csi): refuse to reformat a restore whose device reads blank - #373

Merged
onedr0p merged 1 commit into
mainfrom
fix/blank-restore-reformat
Jul 29, 2026
Merged

fix(csi): refuse to reformat a restore whose device reads blank#373
onedr0p merged 1 commit into
mainfrom
fix/blank-restore-reformat

Conversation

@onedr0p

@onedr0p onedr0p commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

A restored volume whose device comes up blank is silently mkfs'd instead of refused, so the consumer mounts an empty filesystem and the restore reads as successful. This closes the silent half; the underlying "why was the device blank" is still open (see below).

What CI showed

Both merge-queue runs of #364 failed Go E2E (replicated) on restore data-consistency specs. kubelet derives the staging directory from sha256(volume handle), which lets the ginkgo timeline be correlated to the agent logs. For volumegroupsnapshottable-2633 in run 30474605146:

PVC volume staged
restored-…-0 pvc-9d27148c… (sha256 ca643272…) mkfs ext4 on /dev/drbd1008 @ 17:38:51
restored-…-1 pvc-a141f9e3… (sha256 a0eac492…) mkfs ext4 on /dev/drbd1006 @ 17:38:51
restored-…-2 pvc-a2b31493… no mkfs, data intact

The two the test flagged were reformatted at NodeStage. Their sources were mkfs'd at 17:38:15/17:38:19 and the group snapshot was cut at 17:38:31, so MiroirSnapshot.Status.SourceFormatted should have been true and CreateVolume should have stamped the clones Formatted — which would have tripped the existing codes.DataLoss refusal in stage.EnsureFilesystem instead of formatting.

Why the refusal did not fire

The refusal reads vol.Status.Formatted from whatever stage.Device loaded, and the node service is built on the manager's cached client (csi.NewNode(mgr.GetClient(), …)). The controller stamps a clone's inherited flag through the APIReader immediately after Create (controller.go:1900), and the restore is staged seconds later — comfortably inside the window where the node's informer cache can still carry the volume as never-formatted. The guard then reads false and mkfs proceeds.

The change

stage.Deps gains an uncached Reader (same convention as SnapshotReconciler.Reader and Controller.APIReader, falling back to the cached client when unset). When a device reads blank on a volume with Spec.Source != nil, the "never formatted" answer is confirmed against the API server before mkfs; a confirmation that cannot be read is Unavailable, so kubelet retries instead of formatting on a guess.

  • Fresh volumes are untouched: no content source, no extra read, first-stage mkfs as before.
  • A restore of a genuinely unformatted source still formats — the live flag says so.
  • The gateway needs no wiring; its client is already uncached.

What this does not fix

Why the leg was blank while DRBD reported it UpToDate. Ruled out by reading: birthInitPending excludes every Spec.Source != nil volume, the group cut does be.Sync and gates on local + peer UpToDate, publishMembers does set SourceFormatted, snapshot LV names are not truncated so members cannot collide, and a missing snapshot LV fails loudly in CreateFromSnapshot. Pinning it needs the volume CR, drbdsetup status and agent logs from the moment of staging; diagnostics.sh runs at suite end, after the namespace is gone, and the agents log nothing at info level on the restore path. Worth a follow-up that dumps per-spec state on failure.

After this change that failure mode surfaces as a DataLoss stage refusal — a stuck pod with a named cause — instead of a pod that comes up holding an empty filesystem.

Verified

go build ./..., go vet ./..., the unit suite, test-integration (27 specs, envtest) and golangci-lint run all pass. New unit tests cover the stale-cache refusal, the unformatted-source restore that must still format, the fresh-volume path taking no extra read, and the read failure surfacing Unavailable.

A clone inherits its source volume's Formatted flag from the controller
between the Create and the first stage, but the node service stages
through the cached client: the volume can still read unformatted there
while the blank-device refusal is exactly what is needed. A restore
whose device comes up blank is then mkfs'd, which turns a lost clone
into an empty filesystem the consumer mounts without complaint.

Confirm the cached "never formatted" against the API server before
accepting a blank device on a volume with a content source, and treat an
unreadable confirmation as Unavailable rather than a licence to format.
Fresh volumes keep answering from the cache, so first-stage mkfs costs no
extra read.

Signed-off-by: Devin Buhl <devin@buhl.casa>
@onedr0p
onedr0p merged commit b7ab14b into main Jul 29, 2026
16 checks passed
@onedr0p
onedr0p deleted the fix/blank-restore-reformat branch July 29, 2026 20:03
gabrielcosi pushed a commit to gabrielcosi/home-ops that referenced this pull request Jul 29, 2026
….11.12 ➔ 0.11.13) (#185)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [ghcr.io/home-operations/charts/miroir](https://github.com/home-operations/miroir) | patch | `0.11.12` → `0.11.13` |

---

### Release Notes

<details>
<summary>home-operations/miroir (ghcr.io/home-operations/charts/miroir)</summary>

### [`v0.11.13`](https://github.com/home-operations/miroir/blob/HEAD/CHANGELOG.md#01113-2026-07-29)

[Compare Source](home-operations/miroir@0.11.12...0.11.13)

##### Bug Fixes

- **csi:** refuse to reformat a restore whose device reads blank ([#&#8203;373](home-operations/miroir#373)) ([b7ab14b](home-operations/miroir@b7ab14b))

##### Continuous Integration

- split the conformance leg and build the e2e images once ([#&#8203;371](home-operations/miroir#371)) ([3d58054](home-operations/miroir@3d58054))

</details>

---

### Configuration

📅 **Schedule**: (in timezone Europe/Berlin)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4wLjAiLCJ1cGRhdGVkSW5WZXIiOiI0NC4wLjAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbInJlbm92YXRlL2NvbnRhaW5lciIsInR5cGUvcGF0Y2giXX0=-->

Reviewed-on: https://git.xcd.dev/gabrielcosi/home-ops/pulls/185
doonga pushed a commit to greyrock-labs/home-ops that referenced this pull request Jul 30, 2026
….11.12 ➔ 0.11.13) (#211)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [ghcr.io/home-operations/charts/miroir](https://github.com/home-operations/miroir) | patch | `0.11.12` → `0.11.13` |

---

### Release Notes

<details>
<summary>home-operations/miroir (ghcr.io/home-operations/charts/miroir)</summary>

### [`v0.11.13`](https://github.com/home-operations/miroir/blob/HEAD/CHANGELOG.md#01113-2026-07-29)

[Compare Source](home-operations/miroir@0.11.12...0.11.13)

##### Bug Fixes

- **csi:** refuse to reformat a restore whose device reads blank ([#&#8203;373](home-operations/miroir#373)) ([b7ab14b](home-operations/miroir@b7ab14b))

##### Continuous Integration

- split the conformance leg and build the e2e images once ([#&#8203;371](home-operations/miroir#371)) ([3d58054](home-operations/miroir@3d58054))

</details>

---

### Configuration

📅 **Schedule**: (in timezone America/New_York)

- Branch creation
  - At any time (no schedule defined)
- Automerge
  - At any time (no schedule defined)

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Mend Renovate](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4wLjAiLCJ1cGRhdGVkSW5WZXIiOiI0NC4wLjAiLCJ0YXJnZXRCcmFuY2giOiJtYWluIiwibGFiZWxzIjpbInJlbm92YXRlL2NvbnRhaW5lciIsInR5cGUvcGF0Y2giXX0=-->

Reviewed-on: https://git.greyrock.io/greyrock-labs/home-ops/pulls/211
eleboucher pushed a commit to eleboucher/homelab that referenced this pull request Jul 31, 2026
….11.12 ➔ 0.11.15) (#1764)

This PR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [ghcr.io/home-operations/charts/miroir](https://github.com/home-operations/miroir) | patch | `0.11.12` → `0.11.15` |

---

### Release Notes

<details>
<summary>home-operations/miroir (ghcr.io/home-operations/charts/miroir)</summary>

### [`v0.11.15`](https://github.com/home-operations/miroir/blob/HEAD/CHANGELOG.md#01115-2026-07-31)

[Compare Source](home-operations/miroir@0.11.14...0.11.15)

##### Bug Fixes

- **csi:** log retryable status codes below error level ([#&#8203;380](home-operations/miroir#380)) ([4774d21](home-operations/miroir@4774d21))
- **snapshot:** lift possibly-set barrier on failed suspend-io and sweep mid-runtime ([#&#8203;382](home-operations/miroir#382)) ([09e3aa7](home-operations/miroir@09e3aa7))

### [`v0.11.14`](https://github.com/home-operations/miroir/blob/HEAD/CHANGELOG.md#01114-2026-07-30)

[Compare Source](home-operations/miroir@0.11.13...0.11.14)

##### Features

- **go:** update module google.golang.org/grpc (v1.82.1 → v1.83.0) ([#&#8203;375](home-operations/miroir#375)) ([572e629](home-operations/miroir@572e629))

##### Bug Fixes

- **ci:** key the lint cache on the toolchain and drop the prefix fallback ([#&#8203;374](home-operations/miroir#374)) ([789e990](home-operations/miroir@789e990))

### [`v0.11.13`](https://github.com/home-operations/miroir/blob/HEAD/CHANGELOG.md#01113-2026-07-29)

[Compare Source](home-operations/miroir@0.11.12...0.11.13)

##### Bug Fixes

- **csi:** refuse to reformat a restore whose device reads blank ([#&#8203;373](home-operations/miroir#373)) ([b7ab14b](home-operations/miroir@b7ab14b))

##### Continuous Integration

- split the conformance leg and build the e2e images once ([#&#8203;371](home-operations/miroir#371)) ([3d58054](home-operations/miroir@3d58054))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0My4xMDEuMSIsInVwZGF0ZWRJblZlciI6IjQzLjEwMS4xIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJyZW5vdmF0ZS9jb250YWluZXIiLCJ0eXBlL3BhdGNoIl19-->

Reviewed-on: https://git.erwanleboucher.dev/eleboucher/homelab/pulls/1764
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant