diff --git a/atlas-lib/README.md b/atlas-lib/README.md index 63966b3b3..12f3f9dee 100644 --- a/atlas-lib/README.md +++ b/atlas-lib/README.md @@ -37,6 +37,10 @@ atlas/ │ ├── multipath.go ConnectPaths (ordered per-path connect) + PathResult │ └── wait.go ConnectDevice / WaitForDevice: attach -> nvme.Device ├── nqn/ Build & parse simplyblock lvol NQNs +├── blockfs/ Does a block device already hold data, and can that be answered +│ ├── doc.go Why blkid's answer is unsafe, and how a stalled device is read +│ ├── probe.go State, Result, Prober iface; NewDeviceProber (local impl) +│ └── signature.go The on-disk signature table + match ├── lvm/ Linux LVM commands + content-based identity │ ├── doc.go Why identity is read from content, and how scoping is decided │ ├── lvm.go Manager, Run (the escape hatch) @@ -430,6 +434,43 @@ _Today:_ the CSI node service still connects, repairs, and ANA-reconciles paths with nvme-cli in `csi-driver/pkg/util/initiator.go`. `FabricsConnector` is the kernel-direct replacement (it needs no nvme-cli binary in the node image). +#### Decide whether a device may be formatted + +Staging a filesystem volume ends in a question with one catastrophic wrong +answer: does this device already hold data? The conventional way of asking is +unsafe. `blkid` exits 2 both for a device carrying no signature and for one it +could not read, and `k8s.io/mount-utils` maps that single exit code to +"unformatted" and runs `mkfs`, so a volume behind a degraded path is wiped +rather than staged. `blockfs` reads the device and keeps the two apart: + +```go +switch probe := blockfs.NewDeviceProber().Probe(ctx, devicePath); probe.State { +case blockfs.StateFormatted: + // Mount it as it is. ext4 and xfs each replay their own journal. + mount(devicePath, probe.Signature) +case blockfs.StateBlank: + // All zeros, read successfully: the only state that permits a format. + formatAndMount(devicePath) +case blockfs.StateForeign: + // LVM2, LUKS, swap, or a partition table: not mountable, still data. + handleError(fmt.Errorf("%s holds a %s signature", devicePath, probe.Signature)) +case blockfs.StateUnreadable: + // Nothing can be concluded, so nothing may be assumed. Fail the stage and + // let the caller retry: an outage is recoverable and a wiped volume is not. + handleError(probe.Err) +} +``` + +The probe is bounded (20s by default, under `nvme_core.io_timeout`) so a stalled +path resolves as unreadable instead of holding a NodeStage open, and a signature +found in a partially read device still counts as formatted — the data is there +whatever became of the rest. + +_Today:_ `csi-driver/pkg/spdk/nodeserver.go`'s `formatAndMount` is the live call +site, and the reason it no longer delegates the decision to +`SafeFormatAndMount`. See `operator/docs/tests/test-plan-node-stage-format.md` +for the reproduction. + #### Detach without collateral damage Disconnecting a subsystem tears down every namespace on it, so a namespaced diff --git a/atlas-lib/blockfs/doc.go b/atlas-lib/blockfs/doc.go new file mode 100644 index 000000000..f48eac767 --- /dev/null +++ b/atlas-lib/blockfs/doc.go @@ -0,0 +1,39 @@ +// Package blockfs answers one question about a block device — does it already +// hold somebody's data — and, crucially, keeps that answer apart from "the +// device could not be read." +// +// The distinction is the whole point of the package, because the conventional +// way of asking loses it. `blkid` exits 2 both when a device carries no +// filesystem signature and when its probes came back empty because the reads +// underneath them failed, and k8s.io/mount-utils maps that single exit code to +// "unformatted" and runs mkfs. A volume behind a degraded NVMe-oF path is +// therefore indistinguishable from a blank one: a read that exceeds +// nvme_core.io_timeout, or a controller that has passed its ctrl_loss_tmo, +// fails the probe, and staging the volume then destroys the data it holds. +// +// That is not a hypothetical. It is a production data-loss incident, and it +// reproduces on any host in a few commands: put ext4 on a device, stack a +// dm-flakey table with error_reads over it so reads fail while writes still +// land, and `blkid -p -s TYPE -s PTTYPE -o export` returns exit 2 with empty +// output on a filesystem that is plainly there. Upstream tracks the same defect +// reached through a corrupted primary superblock as +// kubernetes/kubernetes#140376, still open, and neither fix proposed there +// covers a device that will not answer a read at all. +// +// So this package reads the device itself and reports what it found. Callers +// deciding whether to format must treat only StateBlank as an empty device: +// every other state either says the device holds data or says nothing could be +// concluded, and both of those are reasons to stop rather than to guess. The +// asymmetry is deliberate — refusing to stage a volume costs an outage, while +// formatting one costs the data. +// +// # Reading a device that may not answer +// +// A read issued to a block device whose paths are gone cannot be interrupted: +// no timeout, cancellation, or close returns those bytes any sooner, and the +// I/O ends only when the kernel gives up on it. Probe therefore reads on its +// own goroutine, which owns the file and the buffer outright, and reports +// StateUnreadable when the deadline passes. The goroutine may outlive the call; +// it holds one descriptor and 128 KiB until the kernel completes or fails the +// I/O, which nvme_core.io_timeout bounds. +package blockfs diff --git a/atlas-lib/blockfs/probe.go b/atlas-lib/blockfs/probe.go new file mode 100644 index 000000000..6745ec379 --- /dev/null +++ b/atlas-lib/blockfs/probe.go @@ -0,0 +1,186 @@ +// The probe itself: the states it can conclude, the Prober seam consumers +// depend on, and the local implementation that reads a real device. Kept apart +// from signature.go so the decision procedure reads in one piece, without the +// magic-number table in the way. +package blockfs + +import ( + "context" + "errors" + "fmt" + "io" + "os" + "time" + + "github.com/simplyblock/atlas/errs/deferrers" +) + +// State is what a probe concluded about a device. +type State string + +const ( + // StateFormatted: the device holds a filesystem that can be mounted as it + // is. Formatting it would destroy data. + StateFormatted State = "Formatted" + + // StateForeign: the device holds a recognized signature that is not a + // mountable filesystem — an encrypted container, an LVM physical volume, a + // swap area, or a partition table. It is somebody's data even though it + // cannot be mounted, so formatting it would still destroy something. + StateForeign State = "Foreign" + + // StateBlank: the device answered the read, and every byte of it is zero. + // This is the only state from which formatting is safe. + StateBlank State = "Blank" + + // StateUnknown: the device answered the read and carries no signature this + // package knows, but it is not all zeros either. Nothing identifies the + // content, and nothing rules out that it matters. + StateUnknown State = "Unknown" + + // StateUnreadable: the device did not answer the read, so nothing at all + // can be concluded about what it holds. Never treat this as blank — it is + // what a volume behind a lost or timed-out NVMe-oF path looks like. + StateUnreadable State = "Unreadable" +) + +// Result is a probe's conclusion. +type Result struct { + State State + // Signature names what was matched, for StateFormatted and StateForeign. + // It is a filesystem family ("ext" covers ext2, ext3, and ext4), not an + // exact revision, and is empty in every other state. + Signature string + // Err is why the device could not be read. Set only for StateUnreadable. + Err error +} + +// Prober reads a block device and reports whether it already holds data. +// Consumers depend on the interface so their tests need no block device. +type Prober interface { + // Probe never returns an error beside its Result: a device that cannot be + // read is a conclusion the caller has to act on (StateUnreadable), not a + // failure of the call, and collapsing the two is the mistake this package + // exists to prevent. + Probe(ctx context.Context, device string) Result +} + +// ReaderAtCloser is the part of a device a probe uses. +type ReaderAtCloser interface { + io.ReaderAt + io.Closer +} + +// Opener opens a device for probing. It is a seam: a test supplies one that +// fails the way a dead path does, which no temporary file can imitate. +type Opener func(device string) (ReaderAtCloser, error) + +// defaultProbeTimeout bounds a single probe. It sits under the 30 seconds +// nvme_core.io_timeout gives an NVMe command by default, so a probe against a +// stalled path resolves as unreadable rather than holding a NodeStageVolume +// open until the kernel gives up. +const defaultProbeTimeout = 20 * time.Second + +// DeviceProber probes a real block device. +type DeviceProber struct { + open Opener + timeout time.Duration +} + +// Option configures a DeviceProber. +type Option func(*DeviceProber) + +// WithOpener replaces how a device is opened. +func WithOpener(open Opener) Option { + return func(p *DeviceProber) { p.open = open } +} + +// WithTimeout replaces how long a single probe may take. +func WithTimeout(timeout time.Duration) Option { + return func(p *DeviceProber) { p.timeout = timeout } +} + +// NewDeviceProber returns a Prober reading local block devices. +func NewDeviceProber(opts ...Option) *DeviceProber { + p := &DeviceProber{ + open: func(device string) (ReaderAtCloser, error) { return os.Open(device) }, + timeout: defaultProbeTimeout, + } + for _, opt := range opts { + opt(p) + } + return p +} + +var _ Prober = (*DeviceProber)(nil) + +// Probe reads the start of device and classifies what it found. +func (p *DeviceProber) Probe(ctx context.Context, device string) Result { + ctx, cancel := context.WithTimeout(ctx, p.timeout) + defer cancel() + + f, err := p.open(device) + if err != nil { + return Result{State: StateUnreadable, Err: fmt.Errorf("open %s: %w", device, err)} + } + + type readResult struct { + data []byte + err error + } + done := make(chan readResult, 1) + + // The goroutine owns the file and the buffer outright, and nothing here + // touches either afterward. A read against a device whose paths are gone + // returns no sooner for being abandoned, so on a timeout this call leaves + // the read running and the goroutine ends on its own once the kernel + // completes or fails the I/O. + go func() { + defer deferrers.Close(f) + + buf := make([]byte, probeLength) + n, readErr := f.ReadAt(buf, 0) + if n < 0 { + n = 0 + } + done <- readResult{data: buf[:n], err: readErr} + }() + + select { + case <-ctx.Done(): + return Result{State: StateUnreadable, Err: fmt.Errorf("read %s: %w", device, ctx.Err())} + case r := <-done: + return classify(r.data, r.err) + } +} + +// classify turns the bytes a probe managed to read into a conclusion. +// +// A read that failed can still be conclusive. A signature found in the part +// that did arrive proves the device holds data whatever became of the rest, +// and resolving a partial read that way errs toward keeping the data. Absent a +// signature, a failed read concludes nothing: neither "blank" nor "unknown" +// may be inferred from bytes that never came. +func classify(data []byte, readErr error) Result { + if sig, ok := match(data); ok { + if sig.mountable { + return Result{State: StateFormatted, Signature: sig.name} + } + return Result{State: StateForeign, Signature: sig.name} + } + + // io.EOF is how ReadAt reports a device shorter than the probe window, + // which is a small device rather than a failure. Every signature above + // still falls within the bytes that arrived, or beyond the device's end. + if readErr != nil && !errors.Is(readErr, io.EOF) { + return Result{State: StateUnreadable, Err: readErr} + } + if len(data) == 0 { + return Result{State: StateUnreadable, Err: errors.New("device returned no bytes")} + } + + if isZero(data) { + return Result{State: StateBlank} + } + return Result{State: StateUnknown} +} diff --git a/atlas-lib/blockfs/probe_test.go b/atlas-lib/blockfs/probe_test.go new file mode 100644 index 000000000..019bf5af5 --- /dev/null +++ b/atlas-lib/blockfs/probe_test.go @@ -0,0 +1,289 @@ +// Tests for the probe's decision procedure: the state it reaches for each kind +// of device, and above all that a device which will not answer a read is never +// reported as blank. The failing reads come from an injected opener, because a +// temporary file cannot be made to return EIO. +package blockfs + +import ( + "context" + "encoding/binary" + "errors" + "io" + "os" + "path/filepath" + "syscall" + "testing" + "time" +) + +// deviceWith returns a probe result for a device whose leading bytes are the +// given content. +func deviceWith(t *testing.T, content []byte) Result { + t.Helper() + + device := filepath.Join(t.TempDir(), "device") + if err := os.WriteFile(device, content, 0o600); err != nil { + t.Fatalf("write device: %v", err) + } + return NewDeviceProber().Probe(context.Background(), device) +} + +// withMagic returns probeLength bytes carrying magic at offset. +func withMagic(offset int, magic []byte) []byte { + content := make([]byte, probeLength) + copy(content[offset:], magic) + return content +} + +// ext4Device returns the bytes of a device holding an ext4 filesystem, as far +// as any probe is concerned: s_magic at 0x38 into a superblock at 1024. +func ext4Device() []byte { + content := make([]byte, probeLength) + binary.LittleEndian.PutUint16(content[1024+0x38:], 0xEF53) + return content +} + +func TestProbeClassifiesDeviceContent(t *testing.T) { + tests := []struct { + name string + content []byte + wantState State + wantSignature string + }{ + { + name: "ext4", + content: ext4Device(), + wantState: StateFormatted, + wantSignature: "ext", + }, + { + name: "xfs", + content: withMagic(0, []byte("XFSB")), + wantState: StateFormatted, + wantSignature: "xfs", + }, + { + name: "btrfs", + content: withMagic(0x10040, []byte("_BHRfS_M")), + wantState: StateFormatted, + wantSignature: "btrfs", + }, + { + name: "an LUKS container is data even though it cannot be mounted", + content: withMagic(0, []byte{'L', 'U', 'K', 'S', 0xBA, 0xBE}), + wantState: StateForeign, + wantSignature: "LUKS", + }, + { + name: "an LVM physical volume label in the second sector", + content: withMagic(512, []byte("LABELONE")), + wantState: StateForeign, + wantSignature: "LVM2", + }, + { + name: "a partition table", + content: withMagic(512, []byte("EFI PART")), + wantState: StateForeign, + wantSignature: "GPT", + }, + { + name: "a freshly provisioned volume reads as zeros", + content: make([]byte, probeLength), + wantState: StateBlank, + }, + { + name: "content with no signature is not blank either", + content: withMagic(8192, []byte("some tenant's bytes")), + wantState: StateUnknown, + }, + { + name: "a device shorter than the probe window is still classified", + content: ext4Device()[:4096], + wantState: StateFormatted, + }, + { + name: "a short blank device is blank", + content: make([]byte, 4096), + wantState: StateBlank, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := deviceWith(t, tc.content) + if got.State != tc.wantState { + t.Errorf("state = %q, want %q (err %v)", got.State, tc.wantState, got.Err) + } + if tc.wantSignature != "" && got.Signature != tc.wantSignature { + t.Errorf("signature = %q, want %q", got.Signature, tc.wantSignature) + } + }) + } +} + +// erroringDevice fails every read with err, the way a block device does once +// its NVMe-oF paths are gone. Close is signaled over a channel because it runs +// on the probe's read goroutine, not the caller's. +type erroringDevice struct { + err error + closed chan struct{} +} + +func newErroringDevice(err error) *erroringDevice { + return &erroringDevice{err: err, closed: make(chan struct{})} +} + +func (d *erroringDevice) ReadAt([]byte, int64) (int, error) { return 0, d.err } + +func (d *erroringDevice) Close() error { + close(d.closed) + return nil +} + +// TestProbeReportsUnreadableRatherThanBlank is the property the package exists +// for: a device whose reads fail must never come back as blank, because a +// caller reading "blank" formats it. +func TestProbeReportsUnreadableRatherThanBlank(t *testing.T) { + device := newErroringDevice(syscall.EIO) + prober := NewDeviceProber(WithOpener(func(string) (ReaderAtCloser, error) { + return device, nil + })) + + got := prober.Probe(context.Background(), "/dev/nvme0n1") + if got.State != StateUnreadable { + t.Fatalf("state = %q, want %q", got.State, StateUnreadable) + } + if !errors.Is(got.Err, syscall.EIO) { + t.Errorf("err = %v, want it to wrap EIO", got.Err) + } +} + +// TestProbeReportsUnreadableWhenTheDeviceCannotBeOpened covers the device that +// vanished between the connect and the stage: total path loss removes the node. +func TestProbeReportsUnreadableWhenTheDeviceCannotBeOpened(t *testing.T) { + missing := filepath.Join(t.TempDir(), "nvme0n1") + + got := NewDeviceProber().Probe(context.Background(), missing) + if got.State != StateUnreadable { + t.Fatalf("state = %q, want %q", got.State, StateUnreadable) + } + if !errors.Is(got.Err, os.ErrNotExist) { + t.Errorf("err = %v, want it to wrap ErrNotExist", got.Err) + } +} + +// TestProbeReportsUnreadableWhenTheDeviceReturnsNothing covers a namespace the +// kernel published with no size behind it, which reads as an immediate EOF. +func TestProbeReportsUnreadableWhenTheDeviceReturnsNothing(t *testing.T) { + got := deviceWith(t, nil) + if got.State != StateUnreadable { + t.Fatalf("state = %q, want %q", got.State, StateUnreadable) + } +} + +// blockingDevice never answers, the way a read against a stalled path does. +type blockingDevice struct{ release chan struct{} } + +func (d *blockingDevice) ReadAt([]byte, int64) (int, error) { + <-d.release + return 0, syscall.EIO +} +func (d *blockingDevice) Close() error { return nil } + +// TestProbeTimesOutRatherThanBlocking pins the bound on a probe: a read that +// never returns must not hold a NodeStageVolume open indefinitely, and the +// result is still unreadable rather than blank. +func TestProbeTimesOutRatherThanBlocking(t *testing.T) { + device := &blockingDevice{release: make(chan struct{})} + // Released at the end so the abandoned read goroutine finishes with the + // test rather than outliving it. + defer close(device.release) + + prober := NewDeviceProber( + WithOpener(func(string) (ReaderAtCloser, error) { return device, nil }), + WithTimeout(50*time.Millisecond), + ) + + start := time.Now() + got := prober.Probe(context.Background(), "/dev/nvme0n1") + if elapsed := time.Since(start); elapsed > 5*time.Second { + t.Errorf("probe took %s; it should give up after its timeout", elapsed) + } + if got.State != StateUnreadable { + t.Fatalf("state = %q, want %q", got.State, StateUnreadable) + } + if !errors.Is(got.Err, context.DeadlineExceeded) { + t.Errorf("err = %v, want it to wrap DeadlineExceeded", got.Err) + } +} + +// TestProbeHonorsCallerCancellation keeps the probe from outliving the RPC that +// asked for it. +func TestProbeHonorsCallerCancellation(t *testing.T) { + device := &blockingDevice{release: make(chan struct{})} + defer close(device.release) + + prober := NewDeviceProber(WithOpener(func(string) (ReaderAtCloser, error) { return device, nil })) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + got := prober.Probe(ctx, "/dev/nvme0n1") + if got.State != StateUnreadable { + t.Fatalf("state = %q, want %q", got.State, StateUnreadable) + } +} + +// partialDevice returns some bytes and then fails, the way a device does when +// the path dies mid-probe. +type partialDevice struct{ data []byte } + +func (d *partialDevice) ReadAt(p []byte, _ int64) (int, error) { + n := copy(p, d.data) + return n, syscall.EIO +} +func (d *partialDevice) Close() error { return nil } + +// TestProbeTrustsASignatureFoundInAPartialRead resolves a half-answered read +// toward keeping the data: a filesystem seen in the bytes that did arrive is +// there whatever happened to the rest. +func TestProbeTrustsASignatureFoundInAPartialRead(t *testing.T) { + prober := NewDeviceProber(WithOpener(func(string) (ReaderAtCloser, error) { + return &partialDevice{data: ext4Device()[:2048]}, nil + })) + + got := prober.Probe(context.Background(), "/dev/nvme0n1") + if got.State != StateFormatted { + t.Fatalf("state = %q, want %q", got.State, StateFormatted) + } +} + +// TestProbeDoesNotConcludeBlankFromAPartialRead is the same situation without +// the signature: zeros that arrived before the read failed say nothing about +// the bytes that did not. +func TestProbeDoesNotConcludeBlankFromAPartialRead(t *testing.T) { + prober := NewDeviceProber(WithOpener(func(string) (ReaderAtCloser, error) { + return &partialDevice{data: make([]byte, 2048)}, nil + })) + + got := prober.Probe(context.Background(), "/dev/nvme0n1") + if got.State != StateUnreadable { + t.Fatalf("state = %q, want %q", got.State, StateUnreadable) + } +} + +// TestProbeClosesTheDevice keeps a probe per stage from leaking a descriptor. +func TestProbeClosesTheDevice(t *testing.T) { + device := newErroringDevice(io.EOF) + prober := NewDeviceProber(WithOpener(func(string) (ReaderAtCloser, error) { return device, nil })) + + prober.Probe(context.Background(), "/dev/nvme0n1") + + // The read runs on its own goroutine, so the close it defers may land just + // after Probe returns. + select { + case <-device.closed: + case <-time.After(10 * time.Second): + t.Error("probe did not close the device") + } +} diff --git a/atlas-lib/blockfs/signature.go b/atlas-lib/blockfs/signature.go new file mode 100644 index 000000000..2b12aa6f3 --- /dev/null +++ b/atlas-lib/blockfs/signature.go @@ -0,0 +1,76 @@ +// The on-disk signatures this package recognizes, and the match against a +// device's leading bytes. They live here, apart from the probe itself, because +// the table is the part that grows: every entry is a magic number at a fixed +// offset, and adding one is data rather than logic. +package blockfs + +import "bytes" + +// probeLength is how much of a device's start is read. It is set by the +// furthest signature below — Btrfs, whose superblock sits at 64 KiB — rounded +// up to a power of two, so every offset in the table falls inside one read. +const probeLength = 128 << 10 + +// signature is a magic number at a fixed offset from the start of a device. +type signature struct { + // name is what gets reported and logged: a filesystem family, or the name + // of whatever else claimed the device. + name string + // offset is where magic begins, in bytes from the start of the device. + offset int + magic []byte + // mountable distinguishes a filesystem, which a consumer can mount and use + // as it is, from a signature that merely proves the device is somebody's: + // an encrypted container, an LVM physical volume, a swap area, or a + // partition table. + mountable bool +} + +// signatures are matched in order, so the entries that identify a device +// beyond doubt come before the weaker ones. A GPT disk carries a protective +// MBR, so GPT precedes it, and the two-byte MBR boot signature — the one entry +// short enough to turn up in unrelated data — comes last. +// +// The ext entry covers ext2, ext3, and ext4 alike: they share s_magic, and +// telling them apart means reading feature flags that no caller here needs, +// since the only decision at stake is whether the device may be formatted. +var signatures = []signature{ + {name: "xfs", offset: 0, magic: []byte("XFSB"), mountable: true}, + {name: "ext", offset: 1024 + 0x38, magic: []byte{0x53, 0xEF}, mountable: true}, + {name: "btrfs", offset: 0x10040, magic: []byte("_BHRfS_M"), mountable: true}, + {name: "LUKS", offset: 0, magic: []byte{'L', 'U', 'K', 'S', 0xBA, 0xBE}}, + // An LVM physical volume's label may sit in any of the first four sectors. + {name: "LVM2", offset: 0, magic: []byte("LABELONE")}, + {name: "LVM2", offset: 512, magic: []byte("LABELONE")}, + {name: "LVM2", offset: 1024, magic: []byte("LABELONE")}, + {name: "LVM2", offset: 1536, magic: []byte("LABELONE")}, + {name: "swap", offset: 4096 - 10, magic: []byte("SWAPSPACE2")}, + {name: "GPT", offset: 512, magic: []byte("EFI PART")}, + {name: "MBR", offset: 510, magic: []byte{0x55, 0xAA}}, +} + +// match returns the first signature present in data. A signature whose offset +// lies beyond the bytes that were read is skipped rather than treated as +// absent: a device smaller than one signature's offset simply cannot carry it. +func match(data []byte) (signature, bool) { + for _, s := range signatures { + end := s.offset + len(s.magic) + if end > len(data) { + continue + } + if bytes.Equal(data[s.offset:end], s.magic) { + return s, true + } + } + return signature{}, false +} + +// isZero reports whether every byte in data is zero. +func isZero(data []byte) bool { + for _, b := range data { + if b != 0 { + return false + } + } + return true +} diff --git a/csi-driver/charts/spdk-csi/latest/spdk-csi/templates/node-rbac.yaml b/csi-driver/charts/spdk-csi/latest/spdk-csi/templates/node-rbac.yaml index d8ada817f..656900079 100644 --- a/csi-driver/charts/spdk-csi/latest/spdk-csi/templates/node-rbac.yaml +++ b/csi-driver/charts/spdk-csi/latest/spdk-csi/templates/node-rbac.yaml @@ -24,9 +24,16 @@ rules: - apiGroups: [""] resources: ["pods"] verbs: ["get","list","watch","delete"] +# rbac-justified: the node plugin reads a claim's on-disk-filesystem annotation +# before deciding whether a volume may be formatted, and records the filesystem +# there after staging one, so a later stage knows what the volume holds without +# reading the device. patch rather than update: the write is a merge patch of +# that one key, so a concurrent writer of any other annotation is left alone. +# Cluster-wide because claims live in whichever namespace a workload chose, and +# unnamed because their names are not known ahead of time. - apiGroups: [""] resources: ["persistentvolumeclaims"] - verbs: ["get", "list", "watch"] + verbs: ["get", "list", "watch", "patch"] - apiGroups: [""] resources: ["persistentvolumes"] verbs: ["get", "list", "watch"] diff --git a/csi-driver/pkg/spdk/nodeserver.go b/csi-driver/pkg/spdk/nodeserver.go index c30f5aa0e..86b4d5847 100644 --- a/csi-driver/pkg/spdk/nodeserver.go +++ b/csi-driver/pkg/spdk/nodeserver.go @@ -33,6 +33,7 @@ import ( "golang.org/x/sys/unix" "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/simplyblock/atlas/blockfs" "github.com/simplyblock/atlas/errs/deferrers" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -54,6 +55,26 @@ type nodeServer struct { volumeLocks *util.VolumeLocks kubeClient kubernetes.Interface guardian *util.Guardian + // exec runs the node's filesystem tools (blkid, mkfs). It is a field rather + // than exec.New() at the call site so a test can drive the format decision + // without a kernel, a blkid, or an mkfs on the host. + exec exec.Interface + // fsProber decides whether a device already holds data. Left nil outside + // tests: deviceProber falls back to a real probe, so a node server built + // without one still refuses to guess rather than silently formatting. + fsProber blockfs.Prober +} + +// defaultDeviceProber reads real block devices. One instance serves every +// stage: it holds no per-device state. +var defaultDeviceProber = blockfs.NewDeviceProber() + +// deviceProber returns the probe behind the format decision. +func (ns *nodeServer) deviceProber() blockfs.Prober { + if ns.fsProber != nil { + return ns.fsProber + } + return defaultDeviceProber } //nolint:unparam // error return kept for constructor symmetry / future use @@ -63,6 +84,7 @@ func newNodeServer(d *csicommon.CSIDriver, kubeClient kubernetes.Interface) (*no mounter: mount.New(""), volumeLocks: util.NewVolumeLocks(), kubeClient: kubeClient, + exec: exec.New(), } // Build one Kubernetes cache manager and share it across the node plugin: @@ -380,7 +402,7 @@ func (ns *nodeServer) NodeStageVolume( initiator.Disconnect(ctx) //nolint:errcheck // ignore error } }() - if err = ns.stageVolume(devicePath, stagingTargetPath, req, vc); err != nil { // idempotent + if err = ns.stageVolume(ctx, devicePath, stagingTargetPath, req, vc); err != nil { // idempotent klog.Errorf("failed to stage volume, volumeID: %s devicePath:%s err: %v", volumeID, devicePath, err) return nil, status.Error(codes.Internal, err.Error()) } @@ -396,6 +418,114 @@ func (ns *nodeServer) NodeStageVolume( return &csi.NodeStageVolumeResponse{}, nil } +// formatAndMount mounts devicePath at stagingPath, formatting it first only +// when the device is provably empty. +// +// The decision is made here rather than left to mount-utils, whose +// SafeFormatAndMount probes with blkid and cannot tell a device that carries no +// filesystem from one whose reads failed: blkid exits 2 for both, mount-utils +// resolves that to "unformatted," and mkfs runs. A volume behind a degraded +// NVMe-oF path — reads timing out under nvme_core.io_timeout, or a controller +// past its ctrl_loss_tmo — is therefore reformatted rather than staged, which +// is how a production cluster lost a volume's data. Upstream tracks the same +// defect as kubernetes/kubernetes#140376. +// +// So only a device that answered a read and proved to be all zeros may be +// formatted. Every other state is either somebody's data or an unanswered +// question, and staging fails rather than guessing: an outage is recoverable +// and a wiped volume is not. The repair paths never reach here at all, because +// they mount a device they know is formatted. +func (ns *nodeServer) formatAndMount( + ctx context.Context, + devicePath, stagingPath, fsType string, + mntFlags, formatOptions []string, +) error { + probe := ns.deviceProber().Probe(ctx, devicePath) + + switch probe.State { + case blockfs.StateFormatted: + if family := fsFamily(fsType); family != "" && family != probe.Signature { + klog.Warningf( + "formatAndMount: %s holds a %s filesystem but the volume asks for %s; mounting as %s rather than reformatting", + devicePath, probe.Signature, fsType, fsType, + ) + } else { + klog.Infof("formatAndMount: %s already holds a %s filesystem; mounting it unchanged", devicePath, probe.Signature) + } + // Plain Mount, never FormatAndMount: the device already carries a + // filesystem, and ext4 and XFS each replay their own journal as they + // mount. This is what the restage path does with a device it did not + // create, and a cold stage owes the volume the same treatment. + if err := ns.mounter.Mount(devicePath, stagingPath, fsType, mntFlags); err != nil { + return fmt.Errorf("mount %s at %s: %w", devicePath, stagingPath, err) + } + return nil + + case blockfs.StateForeign: + return fmt.Errorf( + "refusing to stage %s: it carries a %s signature, and formatting it would destroy data"+ + " this volume did not put there", + devicePath, probe.Signature, + ) + + case blockfs.StateUnreadable: + return fmt.Errorf( + "refusing to stage %s: it could not be read (%w), and a device that will not answer"+ + " a read must not be assumed empty", + devicePath, probe.Err, + ) + + case blockfs.StateUnknown: + // Formatting here keeps a volume whose first blocks hold unrecognizable + // bytes stageable, which is what the driver did before it probed at all. + // The warning is what makes the case visible if it ever turns out to be + // a signature worth recognizing. + klog.Warningf( + "formatAndMount: %s carries no filesystem signature but is not blank either; formatting it as %s", + devicePath, fsType, + ) + + case blockfs.StateBlank: + + default: + return fmt.Errorf("refusing to stage %s: unrecognized probe state %q", devicePath, probe.State) + } + + mounter := mount.SafeFormatAndMount{Interface: ns.mounter, Exec: ns.exec} + return mounter.FormatAndMountSensitiveWithFormatOptions( + devicePath, + stagingPath, + fsType, + mntFlags, + nil, + formatOptions, + ) +} + +// The filesystem types this driver knows by name. The ext family shares one +// on-disk signature, so a probe reports the family rather than the revision. +const ( + fsTypeExt4 = "ext4" + fsTypeXFS = "xfs" + fsFamilyExt = "ext" + fsFamilyBtrfs = "btrfs" +) + +// fsFamily maps a requested filesystem type onto the family a probe reports, so +// that a volume asking for ext4 is not warned about the "ext" a probe names. It +// returns "" for a type no probe can recognize, which suppresses the comparison +// rather than warning about every such volume. +func fsFamily(fsType string) string { + switch fsType { + case "ext2", "ext3", fsTypeExt4: + return fsFamilyExt + case fsTypeXFS, fsFamilyBtrfs: + return fsType + default: + return "" + } +} + func (ns *nodeServer) NodeUnstageVolume( ctx context.Context, req *csi.NodeUnstageVolumeRequest, @@ -448,7 +578,7 @@ func (ns *nodeServer) NodePublishVolume( // If the backing NVMe-oF device was lost (total path loss), repair it before // bind-mounting into the pod — otherwise the pod inherits the dead mount/ // missing device. kubelet skips NodeStage when the volume is still referenced - // on this node (e.g. a same-node pod replacement), so NodePublish is the + // on this node (e.g., a same-node pod replacement), so NodePublish is the // reliable place to heal. if err := ns.healVolumeBeforePublish(ctx, req); err != nil { klog.Errorf("failed to heal volume %s before publish: %v", volumeID, err) @@ -627,7 +757,7 @@ func xfsStripeOptions(volumeContext map[string]string) []string { } // xfsFormatConfigPath is the mkfs.xfs config file that pins which on-disk features -// new xfs volumes are created with. xfsprogs ships it; the container image only has +// new XFS volumes are created with. xfsprogs ships it; the container image only has // to contain a matching xfsprogs. Kept in sync with the assertion in // deploy/image/Dockerfile_base. const xfsFormatConfigPath = "/usr/share/xfsprogs/mkfs/lts_5.15.conf" @@ -645,7 +775,7 @@ const xfsFormatConfigPath = "/usr/share/xfsprogs/mkfs/lts_5.15.conf" // XFS (nvme0n1): Filesystem cannot be safely mounted by this kernel. // XFS (nvme0n1): SB validate failed with error -22. // -// Unlike the ext4 equivalent there is no repair path: xfs features can only be added, +// Unlike the ext4 equivalent there is no repair path: XFS features can only be added, // never removed, and such a filesystem cannot be mounted even read-only. Prevention // is the only option. // @@ -694,6 +824,7 @@ func checkXFSFormatConfig() error { // //nolint:cyclop // many cases in switch increases complexity func (ns *nodeServer) stageVolume( + ctx context.Context, devicePath, stagingPath string, req *csi.NodeStageVolumeRequest, volumeContext map[string]string, @@ -717,27 +848,18 @@ func (ns *nodeServer) stageVolume( mntFlags := stagingMountFlags(req.GetVolumeCapability()) formatOptions := []string{} - if fsType == "xfs" { + if fsType == fsTypeXFS { formatOptions = append(formatOptions, xfsFeatureOptions()...) formatOptions = append(formatOptions, xfsStripeOptions(volumeContext)...) } klog.Infof("mount %s to %s, fstype: %s, flags: %v", devicePath, stagingPath, fsType, mntFlags) klog.Infof("formatOptions %v", formatOptions) - mounter := mount.SafeFormatAndMount{Interface: ns.mounter, Exec: exec.New()} - err = mounter.FormatAndMountSensitiveWithFormatOptions( - devicePath, - stagingPath, - fsType, - mntFlags, - nil, - formatOptions, - ) - if err != nil { + if err = ns.formatAndMount(ctx, devicePath, stagingPath, fsType, mntFlags, formatOptions); err != nil { return err } - if fsType == "ext4" { + if fsType == fsTypeExt4 { reserved := volumeContext["tune2fs_reserved_blocks"] if reserved != "" { cmd := osexec.Command("tune2fs", "-m", reserved, devicePath) @@ -766,7 +888,7 @@ func fsTypeOrDefault(volCap *csi.VolumeCapability) string { if fsType := volCap.GetMount().GetFsType(); fsType != "" { return fsType } - return "ext4" + return fsTypeExt4 } // stagingMountFlags builds the mount flags used when mounting a volume at its @@ -774,8 +896,8 @@ func fsTypeOrDefault(volCap *csi.VolumeCapability) string { func stagingMountFlags(volCap *csi.VolumeCapability) []string { flags := append([]string{}, volCap.GetMount().GetMountFlags()...) - if volCap.GetMount().GetFsType() == "xfs" { - // xfs refuses to mount two filesystems with the same uuid; nouuid lets a + if volCap.GetMount().GetFsType() == fsTypeXFS { + // XFS refuses to mount two filesystems with the same UUID; nouuid lets a // volume and its clone/restored snapshot mount on the same node. flags = append(flags, "nouuid") } @@ -809,7 +931,7 @@ func (ns *nodeServer) stagingMountDead(stagingPath string) bool { return mount.IsCorruptedMnt(err) } // Some filesystems (notably ext4) do NOT shut down when their backing block - // device is removed on total NVMe-oF path loss — unlike xfs, which goes EIO + // device is removed on total NVMe-oF path loss — unlike XFS, which goes EIO // and is caught above. IsMountPoint and stat then both succeed from cache, so // the dead mount looks healthy and never gets restaged. Detect it by checking // that the block device backing the mount still exists: the mountpoint's diff --git a/csi-driver/pkg/spdk/nodeserver_format_test.go b/csi-driver/pkg/spdk/nodeserver_format_test.go new file mode 100644 index 000000000..6551f71e9 --- /dev/null +++ b/csi-driver/pkg/spdk/nodeserver_format_test.go @@ -0,0 +1,286 @@ +// Tests for the one decision NodeStageVolume can never get wrong: whether the +// device it just connected may be handed to mkfs. They live apart from the +// other node-server tests because they are about the format decision alone, +// and they drive stageVolume through the mounter and exec seams so no kernel, +// blkid, or mkfs on the host takes part. +package spdk + +import ( + "context" + "encoding/binary" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/container-storage-interface/spec/lib/go/csi" + mount "k8s.io/mount-utils" + "k8s.io/utils/exec" + fakeexec "k8s.io/utils/exec/testing" +) + +// ext4SuperblockOffset and ext4MagicOffset locate the ext2/3/4 magic: the +// superblock starts 1024 bytes into the device and carries s_magic 0x38 bytes +// into itself. +const ( + ext4SuperblockOffset = 1024 + ext4MagicOffset = 0x38 + ext4Magic = 0xEF53 +) + +// writeExt4Device creates a file standing in for a block device that holds an +// ext4 filesystem. Only the magic matters: every probe that decides whether a +// device is formatted reads it from this offset. +func writeExt4Device(t *testing.T) string { + t.Helper() + + device := filepath.Join(t.TempDir(), "nvme0n1") + content := make([]byte, 256<<10) + binary.LittleEndian.PutUint16(content[ext4SuperblockOffset+ext4MagicOffset:], ext4Magic) + if err := os.WriteFile(device, content, 0o600); err != nil { + t.Fatalf("write fake device: %v", err) + } + return device +} + +// newBlindBlkidExec returns an exec whose blkid exits 2 having printed nothing, +// which is what util-linux reports both for a device with no filesystem and for +// a device it could not read. Every command it runs is appended to ran, so a +// test can assert on what was and was not executed. +func newBlindBlkidExec(ran *[][]string) *fakeexec.FakeExec { + action := func(cmd string, args ...string) exec.Cmd { + *ran = append(*ran, append([]string{cmd}, args...)) + + output := func() ([]byte, []byte, error) { return nil, nil, nil } + if cmd == "blkid" { + output = func() ([]byte, []byte, error) { return nil, nil, fakeexec.FakeExitError{Status: 2} } + } + + fake := &fakeexec.FakeCmd{CombinedOutputScript: []fakeexec.FakeAction{output}} + return fakeexec.InitFakeCmd(fake, cmd, args...) + } + + scripts := make([]fakeexec.FakeCommandAction, 8) + for i := range scripts { + scripts[i] = action + } + return &fakeexec.FakeExec{CommandScript: scripts} +} + +// ext4StageRequest builds the filesystem stage request the tests below drive +// through stageVolume. ext4 throughout: it is what the production incident ran, +// and what a device holding xfs is deliberately mismatched against. +func ext4StageRequest() *csi.NodeStageVolumeRequest { + return &csi.NodeStageVolumeRequest{ + VolumeId: "test-cluster:test-pool:test-lvol", + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Mount{ + Mount: &csi.VolumeCapability_MountVolume{FsType: "ext4"}, + }, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + } +} + +// formatCommands returns the mkfs invocations found in ran. +func formatCommands(ran [][]string) [][]string { + var formats [][]string + for _, argv := range ran { + if strings.HasPrefix(argv[0], "mkfs") { + formats = append(formats, argv) + } + } + return formats +} + +// Regression: 2026-09-03-csi-blkid-reformat — a volume that already held a +// filesystem was reformatted whenever blkid could not read it. blkid exits 2 +// both for "no signature found" and for "could not read the device," and +// mount-utils maps that exit code straight to "unformatted" and runs +// mkfs.ext4 -F, so a single degraded NVMe-oF path during a cold NodeStageVolume +// destroyed the volume's data. Verified on a live host: reads failing under a +// dm-flakey error_reads table make blkid exit 2 with empty output on a device +// that unquestionably holds ext4. +func TestStageVolumeNeverFormatsADeviceHoldingAFilesystem(t *testing.T) { + device := writeExt4Device(t) + staging := t.TempDir() + + var ran [][]string + mounter := mount.NewFakeMounter(nil) + ns := &nodeServer{mounter: mounter, exec: newBlindBlkidExec(&ran)} + + err := ns.stageVolume(context.Background(), device, staging, ext4StageRequest(), map[string]string{}) + if err != nil { + t.Fatalf("stageVolume: %v", err) + } + + if formats := formatCommands(ran); len(formats) > 0 { + t.Errorf("formatted a device that holds a filesystem: ran %v", formats) + } + + // The fake mounter records the resolved staging path, and on some hosts the + // temporary directory sits behind a symlink. + resolved, err := filepath.EvalSymlinks(staging) + if err != nil { + t.Fatalf("resolve staging path: %v", err) + } + var staged bool + for _, mp := range mounter.MountPoints { + if mp.Device == device && (mp.Path == staging || mp.Path == resolved) { + staged = true + } + } + if !staged { + t.Errorf("device was not mounted at the staging path; mount points: %+v", mounter.MountPoints) + } +} + +// Regression: 2026-09-03-csi-blkid-reformat — a device the plugin cannot read at +// all was treated as an empty one and formatted. Nothing can be concluded about +// a device that will not answer a read, so staging has to fail and let kubelet +// retry rather than decide the volume is blank. +func TestStageVolumeRefusesADeviceItCannotRead(t *testing.T) { + // Never created: standing in for a device whose reads fail, which is what a + // volume behind a lost or timed-out NVMe-oF path does. + device := filepath.Join(t.TempDir(), "nvme0n1") + staging := t.TempDir() + + var ran [][]string + ns := &nodeServer{mounter: mount.NewFakeMounter(nil), exec: newBlindBlkidExec(&ran)} + + err := ns.stageVolume(context.Background(), device, staging, ext4StageRequest(), map[string]string{}) + if err == nil { + t.Error("stageVolume succeeded on a device it could not read; expected it to refuse") + } + + if formats := formatCommands(ran); len(formats) > 0 { + t.Errorf("formatted a device it could not read: ran %v", formats) + } +} + +// deviceWithSignature writes a device whose leading bytes carry magic at offset. +func deviceWithSignature(t *testing.T, offset int, magic []byte) string { + t.Helper() + + device := filepath.Join(t.TempDir(), "nvme0n1") + content := make([]byte, 256<<10) + copy(content[offset:], magic) + if err := os.WriteFile(device, content, 0o600); err != nil { + t.Fatalf("write fake device: %v", err) + } + return device +} + +// TestStageVolumeFormatsABlankDevice is the availability half of the format +// decision: refusing to format is only safe as long as a volume that genuinely +// needs a filesystem still gets one, or no PVC would ever come up. +func TestStageVolumeFormatsABlankDevice(t *testing.T) { + // All zeros, which is what a freshly provisioned lvol reads as. + device := deviceWithSignature(t, 0, nil) + staging := t.TempDir() + + var ran [][]string + mounter := mount.NewFakeMounter(nil) + ns := &nodeServer{mounter: mounter, exec: newBlindBlkidExec(&ran)} + + if err := ns.stageVolume( + context.Background(), device, staging, ext4StageRequest(), map[string]string{}, + ); err != nil { + t.Fatalf("stageVolume refused a blank device: %v", err) + } + + if len(mounter.MountPoints) == 0 { + t.Error("blank device was not mounted") + } + + // mount-utils implements the format decision only on Linux; elsewhere it + // mounts without probing, so there is no mkfs to find. + if runtime.GOOS != "linux" { + return + } + formats := formatCommands(ran) + if len(formats) != 1 || formats[0][0] != "mkfs.ext4" { + t.Errorf("expected one mkfs.ext4 for a blank device, got %v", formats) + } +} + +// TestStageVolumeRefusesADeviceHoldingForeignData covers content that is +// somebody's data without being a filesystem this driver can mount. Formatting +// it would destroy it just as surely. +func TestStageVolumeRefusesADeviceHoldingForeignData(t *testing.T) { + // An LVM2 physical-volume label in the second sector. + device := deviceWithSignature(t, 512, []byte("LABELONE")) + + var ran [][]string + ns := &nodeServer{mounter: mount.NewFakeMounter(nil), exec: newBlindBlkidExec(&ran)} + + err := ns.stageVolume( + context.Background(), device, t.TempDir(), ext4StageRequest(), map[string]string{}, + ) + if err == nil { + t.Error("stageVolume accepted a device holding an LVM2 physical volume") + } + + if formats := formatCommands(ran); len(formats) > 0 { + t.Errorf("formatted a device holding foreign data: ran %v", formats) + } +} + +// TestStageVolumeMountsAMismatchedFilesystemRatherThanReformatting pins the +// direction a disagreement resolves in. A volume whose StorageClass says ext4 +// over a device holding XFS is a misconfiguration, and the mount will fail and +// say so — but the data is not this driver's to overwrite on the way there. +func TestStageVolumeMountsAMismatchedFilesystemRatherThanReformatting(t *testing.T) { + device := deviceWithSignature(t, 0, []byte("XFSB")) + staging := t.TempDir() + + var ran [][]string + mounter := mount.NewFakeMounter(nil) + ns := &nodeServer{mounter: mounter, exec: newBlindBlkidExec(&ran)} + + if err := ns.stageVolume( + context.Background(), device, staging, ext4StageRequest(), map[string]string{}, + ); err != nil { + t.Fatalf("stageVolume: %v", err) + } + + if formats := formatCommands(ran); len(formats) > 0 { + t.Errorf("reformatted a device whose filesystem did not match the requested type: ran %v", formats) + } + if len(mounter.MountPoints) == 0 { + t.Error("device was not mounted") + } +} + +// TestStageVolumeSkipsRawBlockVolumes keeps the decision out of the path of a +// volume that has no filesystem by design. +func TestStageVolumeSkipsRawBlockVolumes(t *testing.T) { + device := writeExt4Device(t) + + var ran [][]string + mounter := mount.NewFakeMounter(nil) + ns := &nodeServer{mounter: mounter, exec: newBlindBlkidExec(&ran)} + + req := &csi.NodeStageVolumeRequest{ + VolumeId: "test-cluster:test-pool:test-lvol", + VolumeCapability: &csi.VolumeCapability{ + AccessType: &csi.VolumeCapability_Block{Block: &csi.VolumeCapability_BlockVolume{}}, + AccessMode: &csi.VolumeCapability_AccessMode{ + Mode: csi.VolumeCapability_AccessMode_SINGLE_NODE_WRITER, + }, + }, + } + if err := ns.stageVolume(context.Background(), device, t.TempDir(), req, map[string]string{}); err != nil { + t.Fatalf("stageVolume: %v", err) + } + + if len(ran) > 0 { + t.Errorf("ran commands for a raw block volume: %v", ran) + } + if len(mounter.MountPoints) > 0 { + t.Errorf("mounted a raw block volume: %+v", mounter.MountPoints) + } +} diff --git a/csi-driver/pkg/spdk/nodeserver_live_test.go b/csi-driver/pkg/spdk/nodeserver_live_test.go new file mode 100644 index 000000000..f5a8e1d71 --- /dev/null +++ b/csi-driver/pkg/spdk/nodeserver_live_test.go @@ -0,0 +1,129 @@ +// A live check of the format decision against a real NVMe-oF device, run by +// hand on a node that has one. It is skipped unless SBTEST_DEVICE names the +// device, so it costs an ordinary test run nothing. It exists because the unit +// tests stand in a temporary file for the block device, and the decision this +// package makes is only worth as much as it is worth against the real thing. +package spdk + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/simplyblock/atlas/blockfs" + mount "k8s.io/mount-utils" +) + +// TestLiveStageVolumeDoesNotReformatARealVolume stages a real device that +// already holds a filesystem, with a blkid that reports nothing — which is what +// a degraded NVMe-oF path produces, as `blkid -p` under a dm-flakey error_reads +// table demonstrates. The volume's data has to survive, and mkfs must never be +// issued. +// +// Run it on a node with a staged simplyblock volume: +// +// SBTEST_DEVICE=/dev/nvme0n1 SBTEST_EXPECT_FILE=precious.txt ./spdk.test \ +// -test.run TestLiveStageVolume -test.v +func TestLiveStageVolumeDoesNotReformatARealVolume(t *testing.T) { + device := os.Getenv("SBTEST_DEVICE") + if device == "" { + t.Skip("SBTEST_DEVICE is not set; this test needs a real block device") + } + expectFile := os.Getenv("SBTEST_EXPECT_FILE") + + staging := t.TempDir() + var ran [][]string + ns := &nodeServer{mounter: mount.New(""), exec: newBlindBlkidExec(&ran)} + + err := ns.stageVolume(context.Background(), device, staging, ext4StageRequest(), map[string]string{}) + if err != nil { + t.Fatalf("stageVolume on %s: %v", device, err) + } + defer func() { + if umountErr := mount.New("").Unmount(staging); umountErr != nil { + t.Logf("unmount %s: %v", staging, umountErr) + } + }() + + if formats := formatCommands(ran); len(formats) > 0 { + t.Fatalf("issued mkfs against a real volume that holds data: %v", formats) + } + t.Logf("commands issued while staging: %v", ran) + + entries, err := os.ReadDir(staging) + if err != nil { + t.Fatalf("read staged filesystem: %v", err) + } + names := make([]string, 0, len(entries)) + for _, e := range entries { + names = append(names, e.Name()) + } + t.Logf("staged filesystem contains: %v", names) + + if expectFile == "" { + return + } + content, err := os.ReadFile(filepath.Join(staging, expectFile)) + if err != nil { + t.Fatalf("the volume's data did not survive staging: %v", err) + } + t.Logf("%s survived: %s", expectFile, content) +} + +// TestLiveUpstreamFormatDecisionOnARealVolume is the negative control for the +// test above, and the reason this driver no longer lets mount-utils decide. +// It asks SafeFormatAndMount what it would do with the very same device — the +// call the driver used to make — while blkid reports nothing, and records the +// command it chooses. Nothing is executed: the exec is a fake, and the mounter +// is a fake, so this observes the decision without acting on it. +// +// If this ever stops finding an mkfs, upstream has fixed +// kubernetes/kubernetes#140376 and the driver's own probe could be revisited. +func TestLiveUpstreamFormatDecisionOnARealVolume(t *testing.T) { + device := os.Getenv("SBTEST_DEVICE") + if device == "" { + t.Skip("SBTEST_DEVICE is not set; this test needs a real block device") + } + + var ran [][]string + mounter := mount.SafeFormatAndMount{ + Interface: mount.NewFakeMounter(nil), + Exec: newBlindBlkidExec(&ran), + } + + err := mounter.FormatAndMountSensitiveWithFormatOptions( + device, t.TempDir(), "ext4", nil, nil, nil, + ) + if err != nil { + t.Logf("FormatAndMount returned: %v", err) + } + + formats := formatCommands(ran) + t.Logf("commands mount-utils chose for a volume holding data: %v", ran) + if len(formats) == 0 { + t.Log("mount-utils did not choose to format; upstream may have been fixed") + return + } + t.Logf("mount-utils would have destroyed the volume with: %v", formats) +} + +// TestLiveProbeSeesAFreshVolumeAsBlank is the availability half of the +// contract: a freshly provisioned lvol has to come back blank, or the driver +// would refuse to format volumes it is supposed to format. Point +// SBTEST_BLANK_DEVICE at the device of a raw-block volume, which the driver +// never formats. +func TestLiveProbeSeesAFreshVolumeAsBlank(t *testing.T) { + device := os.Getenv("SBTEST_BLANK_DEVICE") + if device == "" { + t.Skip("SBTEST_BLANK_DEVICE is not set; this test needs a fresh raw block device") + } + + result := blockfs.NewDeviceProber().Probe(context.Background(), device) + t.Logf("probe of %s: state=%s signature=%q err=%v", device, result.State, result.Signature, result.Err) + + if result.State != blockfs.StateBlank { + t.Fatalf("state = %q, want %q: a fresh volume that does not read as blank would never be formatted", + result.State, blockfs.StateBlank) + } +} diff --git a/csi-driver/scripts/backfill-on-disk-filesystem.sh b/csi-driver/scripts/backfill-on-disk-filesystem.sh new file mode 100755 index 000000000..587652ecb --- /dev/null +++ b/csi-driver/scripts/backfill-on-disk-filesystem.sh @@ -0,0 +1,184 @@ +#!/bin/bash +# +# Back-fills the storage.simplyblock.io/on-disk-filesystem annotation onto +# simplyblock claims that already carry a filesystem, so the node plugin knows +# what a volume holds without having to read the device. +# +# It exists for the window an upgrade opens. The plugin records that annotation +# itself, but only from a volume's first successful stage onward, so a claim +# provisioned by an older driver has nothing recorded until then, and a stage +# that cannot read its device has nothing to fall back on. Running this once +# after the upgrade annotates the whole fleet in a single pass instead of one +# volume at a time. +# +# Only simplyblock volumes are touched, and a volume is identified as one by its +# PersistentVolume naming this driver in spec.csi.driver. That is the per-volume +# fact and it does not go stale: a StorageClass can be renamed, deleted, or have +# its parameters edited after the volumes it provisioned were created, so reading +# the class would both miss simplyblock volumes and risk recording a filesystem +# that was never on disk. For the same reason the filesystem recorded is the +# volume's own spec.csi.fsType, and the class parameter is consulted only when +# the volume records none. +# +# A claim is annotated only when a running pod is using it, which is the +# evidence that matters: a volume mounted into a running pod has been formatted, +# whereas a claim that is merely bound may never have been staged at all. +# +# Dry run by default; pass --apply to write. +# +# Usage: +# backfill-on-disk-filesystem.sh [--apply] [-n NAMESPACE | -A] [CLAIM] + +set -euo pipefail + +ANNOTATION="storage.simplyblock.io/on-disk-filesystem" +DRIVER="csi.simplyblock.io" + +# The filesystems the node plugin formats and mounts. A StorageClass asking for +# anything else is left alone: the plugin ignores an annotation outside this set, +# so writing one would only mislead whoever reads it next. +SUPPORTED_FS="ext4 xfs" + +# The plugin's own default when a StorageClass names no filesystem, from +# fsTypeOrDefault in csi-driver/pkg/spdk/nodeserver.go. Keep the two in step. +DEFAULT_FS="ext4" + +usage() { + sed -n '3,23p' "$0" | sed 's/^#\{1,\} \{0,1\}//' +} + +apply=false +ns_args=() +claim_filter="" + +while [ $# -gt 0 ]; do + case "$1" in + --apply) apply=true; shift ;; + -A|--all-namespaces) ns_args=(--all-namespaces); shift ;; + -n|--namespace) ns_args=(--namespace "$2"); shift 2 ;; + -h|--help) usage; exit 0 ;; + -*) echo "unknown option: $1" >&2; usage >&2; exit 2 ;; + *) claim_filter="$1"; shift ;; + esac +done + +command -v kubectl >/dev/null || { echo "kubectl is not on PATH" >&2; exit 1; } + +# Every claim bound to a simplyblock volume, as "namespace/namefsType", +# read from the PersistentVolumes because that is where the driver name and the +# provisioned filesystem are recorded per volume. +echo "reading volumes..." >&2 +simplyblock_claims=$( + kubectl get pv -o go-template=' +{{- range .items -}} + {{- if .spec.csi -}} + {{- if eq .spec.csi.driver "'"$DRIVER"'" -}} + {{- if .spec.claimRef -}} + {{- .spec.claimRef.namespace }}/{{ .spec.claimRef.name }}{{ "\t" }} + {{- with .spec.csi.fsType }}{{ . }}{{ end }}{{ "\n" -}} + {{- end -}} + {{- end -}} + {{- end -}} +{{- end -}}' 2>/dev/null +) + +# Every claim a running pod mounts, as namespace/name. One pass over the pods +# rather than a describe per claim: `describe` prints only the first consumer on +# its "Used By:" line, and a cluster with a few thousand claims would otherwise +# spend minutes in round trips. A pending pod does not count — it has not +# mounted its volume yet, so it is no evidence the volume was ever formatted. +echo "reading pods..." >&2 +# shellcheck disable=SC2016 # $ns is a go-template variable, not a shell one. +claims_in_use=$( + kubectl get pods "${ns_args[@]}" --field-selector=status.phase=Running -o go-template=' +{{- range .items -}} + {{- $ns := .metadata.namespace -}} + {{- range .spec.volumes -}} + {{- if .persistentVolumeClaim -}} + {{- $ns }}/{{ .persistentVolumeClaim.claimName }}{{ "\n" -}} + {{- end -}} + {{- end -}} +{{- end -}}' 2>/dev/null | sort -u +) + +# StorageClass -> filesystem, the fallback for a volume recording none. +class_filesystems=$( + kubectl get storageclass -o go-template=' +{{- range .items -}} + {{- if eq .provisioner "'"$DRIVER"'" -}} + {{- .metadata.name }}{{ "\t" }} + {{- with .parameters }}{{ with index . "csi.storage.k8s.io/fstype" }}{{ . }}{{ end }}{{ end }}{{ "\n" -}} + {{- end -}} +{{- end -}}' 2>/dev/null +) + +printf '%-22s %-38s %-8s %s\n' NAMESPACE CLAIM FS ACTION + +annotated=0 +skipped=0 + +while IFS=$'\t' read -r ns name class phase mode existing; do + [ -n "${name:-}" ] || continue + [ -z "$claim_filter" ] || [ "$name" = "$claim_filter" ] || continue + + fstype="" + reason="" + + # A raw block volume carries no filesystem at all, and recording one would be + # a claim the plugin might later act on. + if [ "${mode:-}" = "Block" ]; then + reason="skip: raw block volume" + elif [ "${phase:-}" != "Bound" ]; then + reason="skip: claim is ${phase:-unknown}" + elif [ -n "${existing:-}" ]; then + reason="skip: already records $existing" + elif ! printf '%s\n' "$simplyblock_claims" | grep -q "^${ns}/${name}"$'\t'; then + reason="skip: not a simplyblock volume" + elif ! printf '%s\n' "$claims_in_use" | grep -qxF "$ns/$name"; then + reason="skip: no running pod mounts it" + else + fstype=$(printf '%s\n' "$simplyblock_claims" | grep "^${ns}/${name}"$'\t' | cut -f2) + if [ -z "$fstype" ] && [ -n "${class:-}" ]; then + fstype=$(printf '%s\n' "$class_filesystems" | grep "^${class}"$'\t' | cut -f2) + fi + [ -n "$fstype" ] || fstype="$DEFAULT_FS" + case " $SUPPORTED_FS " in + *" $fstype "*) ;; + *) reason="skip: unsupported filesystem $fstype" ;; + esac + fi + + if [ -n "$reason" ]; then + printf '%-22s %-38s %-8s %s\n' "$ns" "$name" "${fstype:--}" "$reason" + skipped=$((skipped + 1)) + continue + fi + + if [ "$apply" = true ]; then + if kubectl -n "$ns" annotate pvc "$name" "$ANNOTATION=$fstype" >/dev/null 2>&1; then + printf '%-22s %-38s %-8s %s\n' "$ns" "$name" "$fstype" "annotated" + annotated=$((annotated + 1)) + else + printf '%-22s %-38s %-8s %s\n' "$ns" "$name" "$fstype" "FAILED" + fi + else + printf '%-22s %-38s %-8s %s\n' "$ns" "$name" "$fstype" "would annotate" + annotated=$((annotated + 1)) + fi +done < <( + kubectl get pvc "${ns_args[@]}" -o go-template=' +{{- range .items -}} + {{- .metadata.namespace }}{{ "\t" }}{{ .metadata.name }}{{ "\t" }} + {{- with .spec.storageClassName }}{{ . }}{{ end }}{{ "\t" }} + {{- .status.phase }}{{ "\t" }} + {{- with .spec.volumeMode }}{{ . }}{{ end }}{{ "\t" }} + {{- with .metadata.annotations }}{{ with index . "storage.simplyblock.io/on-disk-filesystem" }}{{ . }}{{ end }}{{ end }}{{ "\n" -}} +{{- end -}}' 2>/dev/null +) + +echo +if [ "$apply" = true ]; then + echo "annotated $annotated claim(s), skipped $skipped" +else + echo "would annotate $annotated claim(s), skipped $skipped — rerun with --apply to write" +fi diff --git a/helm-charts/charts/simplyblock-operator/templates/node-rbac.yaml b/helm-charts/charts/simplyblock-operator/templates/node-rbac.yaml index ef175c624..b7c9b4f33 100644 --- a/helm-charts/charts/simplyblock-operator/templates/node-rbac.yaml +++ b/helm-charts/charts/simplyblock-operator/templates/node-rbac.yaml @@ -22,9 +22,16 @@ rules: - apiGroups: [""] resources: ["pods"] verbs: ["get","list","watch","delete"] +# rbac-justified: the node plugin reads a claim's on-disk-filesystem annotation +# before deciding whether a volume may be formatted, and records the filesystem +# there after staging one, so a later stage knows what the volume holds without +# reading the device. patch rather than update: the write is a merge patch of +# that one key, so a concurrent writer of any other annotation is left alone. +# Cluster-wide because claims live in whichever namespace a workload chose, and +# unnamed because their names are not known ahead of time. - apiGroups: [""] resources: ["persistentvolumeclaims"] - verbs: ["get", "list", "watch"] + verbs: ["get", "list", "watch", "patch"] - apiGroups: [""] resources: ["persistentvolumes"] verbs: ["get", "list", "watch"] diff --git a/operator/docs/tests/test-plan-node-stage-format.md b/operator/docs/tests/test-plan-node-stage-format.md new file mode 100644 index 000000000..645135dbb --- /dev/null +++ b/operator/docs/tests/test-plan-node-stage-format.md @@ -0,0 +1,204 @@ +# Test Plan: The NodeStageVolume Format Decision + +Covers the one decision the CSI node plugin can never get wrong: whether the +device it has just connected may be handed to `mkfs`. The behavior under test +lives in `csi-driver/pkg/spdk/nodeserver.go` (`formatAndMount`) and +`atlas-lib/blockfs` (the probe it decides on). + +## Why this plan exists + +The driver used to leave the decision to `k8s.io/mount-utils`, whose +`SafeFormatAndMount` probes with `blkid` and cannot tell a device carrying no +filesystem from one whose reads failed: `blkid` reports exit status 2 for both, +`mount-utils` resolves that to "unformatted," and `mkfs.ext4 -F -m0` runs. A +volume behind a degraded NVMe-oF path — reads timing out under +`nvme_core.io_timeout`, or a controller past its `ctrl_loss_tmo` — was therefore +reformatted rather than staged, and a production cluster lost a volume's data +that way. Upstream tracks the same defect, reached through a corrupted primary +superblock rather than an unreadable device, as +[kubernetes/kubernetes#140376](https://github.com/kubernetes/kubernetes/issues/140376), +open and untriaged. + +The rule the plan pins: only a device that answered a read and proved to be all +zeros may be formatted. Every other outcome is either somebody's data or an +unanswered question, and staging fails rather than guessing. + +## Coverage map + +| Prefix | Level | What it needs | +|--------|-------------------|------------------------------------------------------------------------| +| `PB-` | Unit, probe | nothing; a temporary file or an injected opener stands in for a device | +| `FD-` | Unit, decision | a fake mounter and a fake exec; **Linux**, see below | +| `LV-` | Live | a node with a staged simplyblock volume, run by hand | +| `FI-` | Failure injection | a host with `dm-flakey`, run by hand | + +Types are `Positive`, `Negative`, `Boundary`, and `Regression`. The `Test` +column names the implementing function, or `—` when nothing covers it yet. + +**The `FD-` rows only exercise anything on Linux.** `mount-utils` implements the +format decision in `mount_linux.go`; on every other platform +`FormatAndMountSensitive` mounts without probing, so these cases pass vacuously +on macOS. Run them on a Linux host, or in CI, before trusting them: + +```bash +CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go test -c -o /tmp/spdk.test ./pkg/spdk/ +# then, on a Linux host +/tmp/spdk.test -test.run TestStageVolume -test.v +``` + +--- + +## 1. The probe (`atlas-lib/blockfs`) + +| ID | Scenario | Type | Test | +|-------|------------------------------------------------------------------------------------|------------|---------------------------------------------------------| +| PB-01 | An ext2/3/4 superblock is reported as a formatted device | Positive | `TestProbeClassifiesDeviceContent` | +| PB-02 | An XFS superblock is reported as a formatted device | Positive | `TestProbeClassifiesDeviceContent` | +| PB-03 | A Btrfs superblock, at 64 KiB, is inside the probe window | Boundary | `TestProbeClassifiesDeviceContent` | +| PB-04 | LUKS, LVM2, swap, and partition-table signatures are foreign data, not filesystems | Negative | `TestProbeClassifiesDeviceContent` | +| PB-05 | An LVM2 label in any of the first four sectors is found | Boundary | `TestProbeClassifiesDeviceContent` | +| PB-06 | An all-zero device is blank, the only state that permits a format | Positive | `TestProbeClassifiesDeviceContent` | +| PB-07 | Readable, non-zero, unrecognized content is `Unknown`, not blank | Negative | `TestProbeClassifiesDeviceContent` | +| PB-08 | A device shorter than the probe window is still classified | Boundary | `TestProbeClassifiesDeviceContent` | +| PB-09 | A device whose reads fail with EIO is `Unreadable`, never blank | Regression | `TestProbeReportsUnreadableRatherThanBlank` | +| PB-10 | A device that cannot be opened at all is `Unreadable` | Regression | `TestProbeReportsUnreadableWhenTheDeviceCannotBeOpened` | +| PB-11 | A device answering zero bytes is `Unreadable`, not blank | Boundary | `TestProbeReportsUnreadableWhenTheDeviceReturnsNothing` | +| PB-12 | A read that never returns gives up at the timeout, below `nvme_core.io_timeout` | Negative | `TestProbeTimesOutRatherThanBlocking` | +| PB-13 | Caller cancellation ends the probe | Negative | `TestProbeHonorsCallerCancellation` | +| PB-14 | A signature in a partially read device is trusted: the data is there regardless | Boundary | `TestProbeTrustsASignatureFoundInAPartialRead` | +| PB-15 | Zeros from a partial read do **not** conclude blank | Regression | `TestProbeDoesNotConcludeBlankFromAPartialRead` | +| PB-16 | Every probe closes its device, so one stage per volume leaks no descriptor | Positive | `TestProbeClosesTheDevice` | + +## 2. The decision (`csi-driver/pkg/spdk`) + +| ID | Scenario | Type | Test | +|-------|----------------------------------------------------------------------------------------------------|------------|--------------------------------------------------------------------| +| FD-01 | A device holding a filesystem is mounted, never formatted, even when `blkid` reports nothing | Regression | `TestStageVolumeNeverFormatsADeviceHoldingAFilesystem` | +| FD-02 | A device that cannot be read fails the stage instead of being formatted | Regression | `TestStageVolumeRefusesADeviceItCannotRead` | +| FD-03 | A blank device is still formatted, so first-time provisioning does not regress | Positive | `TestStageVolumeFormatsABlankDevice` | +| FD-04 | A device carrying a foreign signature fails the stage | Negative | `TestStageVolumeRefusesADeviceHoldingForeignData` | +| FD-05 | A filesystem that does not match the requested `fsType` is mounted with a warning, not reformatted | Boundary | `TestStageVolumeMountsAMismatchedFilesystemRatherThanReformatting` | +| FD-06 | A raw block volume skips the decision entirely | Positive | `TestStageVolumeSkipsRawBlockVolumes` | + +## 3. Live, run by hand + +Needs a node with a staged simplyblock volume. `SBTEST_DEVICE` names the device +of a volume that holds data, `SBTEST_BLANK_DEVICE` that of a raw-block volume +the driver has never formatted: + +```bash +SBTEST_DEVICE=/dev/nvme0n1 SBTEST_EXPECT_FILE=precious.txt \ + SBTEST_BLANK_DEVICE=/dev/nvme2n1 \ + ./spdk.test -test.run TestLive -test.v +``` + +| ID | Scenario | Type | Test | +|-------|----------------------------------------------------------------------------------------------------|------------|-------------------------------------------------| +| LV-01 | A real volume holding data is staged with its data intact while `blkid` reports nothing | Regression | `TestLiveStageVolumeDoesNotReformatARealVolume` | +| LV-02 | `mount-utils` still chooses `mkfs` for that same device, which is why the driver does not delegate | Regression | `TestLiveUpstreamFormatDecisionOnARealVolume` | +| LV-03 | A freshly provisioned lvol reads as blank, so it will still be formatted | Positive | `TestLiveProbeSeesAFreshVolumeAsBlank` | + +Verified on a four-node K3s cluster (Rocky 9.5, kernel 5.14) on 2026-09-03. +LV-01 issued no commands at all, and the volume's marker file and its 64 MiB +payload both survived. LV-02 chose `mkfs.ext4 -F -m0 /dev/nvme0n1` against that +same device. LV-03 reported `Blank`. + +## 4. Failure injection, run by hand + +Not automated: no Go test can make a device fail reads on demand. Two recipes +follow. The loop device is the quicker one and needs no cluster; the NVMe-oF one +runs the whole failure against a real simplyblock volume and is what establishes +that this is reachable in production rather than only in a model. + +A degraded NVMe-oF path presents in three different ways, and only one of them +destroys anything, so a reproduction has to say which it is reaching: + +| The paths are | The device node | I/O | `blkid` | Consequence | +|---------------------------------------|-----------------|-----------|---------|---------------------------------------------------| +| down, within `ctrl_loss_tmo` | present | queues | blocks | staging hangs, kubelet retries; nothing formatted | +| down, past `ctrl_loss_tmo` | **gone** | n/a | exit 2 | mkfs hits a missing path and fails | +| down, with `fast_io_fail_tmo` reached | **present** | **fails** | exit 2 | **mkfs runs against the volume** | + +The third is the production case. Reaching it needs `fast_io_fail_tmo`, which +tells the driver to fail I/O quickly while the controller keeps reconnecting, so +the namespace stays present and unreadable rather than queueing or vanishing. +`CONFIG_FAULT_INJECTION` is not required and is absent from the kernels this +product runs on. + +| ID | Scenario | Type | Test | +|-------|------------------------------------------------------------------------------------------|------------|------| +| FI-01 | `blkid` exits 2 with empty output on an ext4 device whose reads fail | Regression | — | +| FI-02 | `mkfs` on such a device discards its blocks, then fails, leaving no valid filesystem | Regression | — | +| FI-03 | With reads recovered between probe and `mkfs`, the volume is silently reformatted empty | Regression | — | +| FI-04 | A volume whose reads fail during a cold stage is refused rather than formatted | Regression | — | +| FI-05 | On a real NVMe-oF volume, `fast_io_fail_tmo` yields exit 2 with the device still present | Regression | — | +| FI-06 | That volume is then reformatted once the path recovers, and its data is gone | Regression | — | + +Reproduction for FI-01 through FI-03, on any host with `dm-flakey`. It runs +against a loop device, never a real volume: + +```bash +dd if=/dev/zero of=disk.img bs=1M count=256 +LOOP=$(losetup --find --show disk.img) +mkfs.ext4 -q -F "$LOOP" +mount "$LOOP" mnt && echo data > mnt/precious.txt && sync && umount mnt + +modprobe dm-flakey +dmsetup create sbrepro --table "0 $(blockdev --getsz "$LOOP") linear $LOOP 0" +blkid -p -s TYPE -s PTTYPE -o export /dev/mapper/sbrepro # TYPE=ext4, exit 0 + +# Reads fail, writes still land: what a degraded NVMe-oF path does. +dmsetup suspend sbrepro +dmsetup load sbrepro --table "0 $(blockdev --getsz "$LOOP") flakey $LOOP 0 0 60 1 error_reads" +dmsetup resume sbrepro +blkid -p -s TYPE -s PTTYPE -o export /dev/mapper/sbrepro # empty, exit 2 +``` + +### FI-05 and FI-06, on a real simplyblock volume + +Needs a node with a simplyblock volume attached, and destroys that volume's +data, so run it against one provisioned for the purpose. `` and `` are the +storage nodes the volume's paths point at, from +`cat /sys/class/nvme/nvme*/address`. + +```bash +DEV=/dev/nvme0n1 +mkfs.ext4 -q -F $DEV # give the volume a filesystem +mount $DEV /mnt && echo data > /mnt/precious.txt && sync && umount /mnt +blkid -p -s UUID -o value $DEV # note the UUID + +# Fail I/O quickly while the controller keeps reconnecting, so the namespace +# stays present rather than queueing I/O or being torn down. +for c in /sys/class/nvme/nvme0 /sys/class/nvme/nvme1; do + echo 3600 > $c/ctrl_loss_tmo + echo 5 > $c/fast_io_fail_tmo +done + +# Sever every path. +iptables -I OUTPUT -d -p tcp --dport 4428 -j DROP +iptables -I OUTPUT -d -p tcp --dport 4428 -j DROP +sleep 25 +echo 3 > /proc/sys/vm/drop_caches # or the probe is answered from cache + +blkid -p -s TYPE -s PTTYPE -o export $DEV # FI-05: exit 2, no output, device present +``` + +Then let the paths recover and run what the driver would run next: + +```bash +iptables -D OUTPUT -d -p tcp --dport 4428 -j DROP +iptables -D OUTPUT -d -p tcp --dport 4428 -j DROP +mkfs.ext4 -F -m0 $DEV # FI-06: exits 0 +blkid -p -s UUID -o value $DEV # a different UUID: the volume is gone +``` + +Verified on 2026-09-03 against a four-node cluster, kernel 5.14, on a 2 GiB +volume with two live HA paths. The probe returned exit 2 with empty output while +`/dev/nvme0n1` was present and its ext4 intact, `mkfs` then exited 0, and the +volume came back with a new UUID holding nothing but `lost+found`. Dropping the +page cache matters: without it the probe is answered from cache and returns exit +0 even with every path severed. + +Verified on 2026-09-03 with util-linux 2.40.2 and dm-flakey v1.5.0. The probe +returned exit 2 with empty output, and `mkfs.ext4 -F -m0` then destroyed the +filesystem in both the still-failing and the recovered case.