Skip to content

Commit b35fbe1

Browse files
kvapsclaude
andcommitted
feat(satellite): on-disk signature cross-check helpers (Phase 10.7)
Pure-function detectors the PhysicalDevice discovery loop will compose with lsblk's TYPE/FSTYPE filter to publish only truly free devices: HasLVMSignature — pvs --noheadings -o pv_name | match HasZFSSignature — zpool list -PHv | match (missing tool ⇒ false) HasDRBDSignature — drbdmeta dump-md exit-zero HasOtherSignature — wipefs -n non-empty stdout `IsDeviceFree(ctx, exec, lsblkDevice)` composes everything: the lsblk row's IsFreeBlockDevice() short-circuits the chain (saves exec calls on busy hosts with thousands of partitions); the four signature detectors run in order, first-positive aborts. Each detector's "tool not installed" path swallows the exec error (LVM-only host can't have ZFS signatures, FILE-only host can't have DRBD ones, etc). Pinned via 6 unit tests covering happy/empty/missing-tool paths plus the two short-circuit invariants — the "stop after first positive" + "skip everything when lsblk already disqualified" optimisations matter on real hardware where these detectors are the hot path of a 60-second discovery tick. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 229e277 commit b35fbe1

2 files changed

Lines changed: 331 additions & 0 deletions

File tree

pkg/satellite/signatures.go

Lines changed: 159 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,159 @@
1+
/*
2+
Copyright 2026 Cozystack contributors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package satellite
18+
19+
import (
20+
"context"
21+
"strings"
22+
23+
"github.com/cockroachdb/errors"
24+
25+
"github.com/cozystack/blockstor/pkg/storage"
26+
)
27+
28+
// HasLVMSignature reports whether the named device is already a
29+
// member of an LVM physical volume. The discovery loop runs this
30+
// (alongside HasZFSSignature, HasDRBDSignature, HasOtherSignature)
31+
// before publishing a device as a `PhysicalDevice` CRD —
32+
// `lsblk` alone misses already-attached LVM PVs that haven't been
33+
// fully formatted with a kernel-recognised signature.
34+
//
35+
// Method: `pvs --noheadings -o pv_name` lists every PV the host
36+
// knows about. We string-match the device path. Empty stdout
37+
// means no PVs at all on this host (fresh install) — return
38+
// false. Phase 10.7.
39+
func HasLVMSignature(ctx context.Context, exec storage.Exec, devicePath string) (bool, error) {
40+
out, err := exec.Run(ctx, "pvs", "--noheadings", "-o", "pv_name")
41+
if err != nil {
42+
return false, errors.Wrap(err, "pvs")
43+
}
44+
45+
for line := range strings.SplitSeq(string(out), "\n") {
46+
if strings.TrimSpace(line) == devicePath {
47+
return true, nil
48+
}
49+
}
50+
51+
return false, nil
52+
}
53+
54+
// HasZFSSignature reports whether the named device is part of any
55+
// imported ZFS pool. `zpool list -PHv` walks every pool's vdev
56+
// tree; we string-match the device path against any line.
57+
//
58+
// `-P` forces full device paths (so we can compare against
59+
// /dev/disk/by-id symlinks); `-H` strips the header; `-v`
60+
// expands vdevs. Phase 10.7.
61+
func HasZFSSignature(ctx context.Context, exec storage.Exec, devicePath string) (bool, error) {
62+
out, err := exec.Run(ctx, "zpool", "list", "-PHv")
63+
if err != nil {
64+
// `zpool` may not be installed (FILE / LVM-only host); a
65+
// missing binary surfaces as exec error, which we treat
66+
// as "no ZFS in play, so no signature". The caller is
67+
// responsible for distinguishing "missing tool" from
68+
// "real failure" via the error type if needed.
69+
return false, nil
70+
}
71+
72+
for line := range strings.SplitSeq(string(out), "\n") {
73+
// zpool list -v rows are tab/space separated; the first
74+
// column is either the pool name (no leading whitespace)
75+
// or a vdev / device (leading whitespace). We just look
76+
// for the substring; false-positives on pool names that
77+
// happen to contain `/dev/...` are pathological.
78+
if strings.Contains(line, devicePath) {
79+
return true, nil
80+
}
81+
}
82+
83+
return false, nil
84+
}
85+
86+
// HasDRBDSignature reports whether the named device carries a
87+
// DRBD-9 metadata block. `drbdmeta dump-md` reads the metadata
88+
// block — exit-zero means it parsed something, exit-non-zero
89+
// means no recognisable metadata. We don't care about the parsed
90+
// content here, only the success/failure signal.
91+
//
92+
// We pass `0 v09 <device> internal` (minor 0, version 9, internal
93+
// metadata) — drbdmeta accepts a placeholder minor since we're
94+
// only reading. Phase 10.7.
95+
func HasDRBDSignature(ctx context.Context, exec storage.Exec, devicePath string) (bool, error) {
96+
_, err := exec.Run(ctx, "drbdmeta", "0", "v09", devicePath, "internal", "dump-md")
97+
if err != nil {
98+
// Any failure (no metadata / drbdmeta missing / read error)
99+
// → treat as no signature. The caller's `wipefs -n` pass
100+
// catches anything else.
101+
return false, nil //nolint:nilerr // any drbdmeta failure ⇒ no DRBD signature
102+
}
103+
104+
return true, nil
105+
}
106+
107+
// HasOtherSignature is the catch-all that uses `wipefs -n` to
108+
// detect filesystem / RAID / partition-table signatures lsblk's
109+
// FSTYPE column may have missed. `-n` is dry-run: lists matches
110+
// without modifying the device. Non-empty stdout = at least one
111+
// signature found.
112+
func HasOtherSignature(ctx context.Context, exec storage.Exec, devicePath string) (bool, error) {
113+
out, err := exec.Run(ctx, "wipefs", "-n", devicePath)
114+
if err != nil {
115+
return false, errors.Wrap(err, "wipefs")
116+
}
117+
118+
return strings.TrimSpace(string(out)) != "", nil
119+
}
120+
121+
// IsDeviceFree composes the four cross-checks: a device is free
122+
// iff lsblk-derived `IsFreeBlockDevice` returns true AND none of
123+
// the four signature detectors fired. Returns the first error
124+
// encountered; pre-empts subsequent checks (so a ZFS-host that
125+
// has `zpool` installed but pvs missing fails cleanly on the
126+
// LVM check rather than silently treating "exec error" as
127+
// "free").
128+
//
129+
// The lsblk row stays the single source of truth for the
130+
// device path — we feed `LsblkDevice.KName` (e.g. `/dev/sda`)
131+
// into the signature checks since that's what pvs/zpool/drbdmeta
132+
// all expect.
133+
func IsDeviceFree(ctx context.Context, exec storage.Exec, dev *LsblkDevice) (bool, error) {
134+
if !dev.IsFreeBlockDevice() {
135+
return false, nil
136+
}
137+
138+
devicePath := "/dev/" + dev.KName
139+
140+
checks := []func(context.Context, storage.Exec, string) (bool, error){
141+
HasLVMSignature,
142+
HasZFSSignature,
143+
HasDRBDSignature,
144+
HasOtherSignature,
145+
}
146+
147+
for _, check := range checks {
148+
has, err := check(ctx, exec, devicePath)
149+
if err != nil {
150+
return false, err
151+
}
152+
153+
if has {
154+
return false, nil
155+
}
156+
}
157+
158+
return true, nil
159+
}

pkg/satellite/signatures_test.go

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,172 @@
1+
/*
2+
Copyright 2026 Cozystack contributors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package satellite_test
18+
19+
import (
20+
"errors"
21+
"testing"
22+
23+
"github.com/cozystack/blockstor/pkg/satellite"
24+
"github.com/cozystack/blockstor/pkg/storage"
25+
)
26+
27+
// errZpoolNotFound stands in for the exec.Command-not-found error
28+
// FakeExec surfaces when the host doesn't have ZFS installed. The
29+
// LVM-only-host path is the one we most care about pinning.
30+
var errZpoolNotFound = errors.New("zpool: command not found")
31+
32+
// TestHasLVMSignaturePvsMatch pins the LVM signature detection: a
33+
// device path appearing in `pvs --noheadings -o pv_name` output
34+
// surfaces as has=true. Each line is trimmed before comparison so
35+
// pvs's typical leading-spaces formatting doesn't break the match.
36+
func TestHasLVMSignaturePvsMatch(t *testing.T) {
37+
t.Parallel()
38+
39+
fx := storage.NewFakeExec()
40+
fx.Expect("pvs --noheadings -o pv_name",
41+
storage.FakeResponse{Stdout: []byte(" /dev/sda\n /dev/nvme0n1\n")})
42+
43+
has, err := satellite.HasLVMSignature(t.Context(), fx, "/dev/sda")
44+
if err != nil {
45+
t.Fatalf("HasLVMSignature: %v", err)
46+
}
47+
48+
if !has {
49+
t.Errorf("got has=false, want true (sda is in pvs output)")
50+
}
51+
}
52+
53+
// TestHasLVMSignatureNoMatch pins the empty-output / missing-device
54+
// case: `pvs` reports no PVs (empty stdout) → has=false.
55+
func TestHasLVMSignatureNoMatch(t *testing.T) {
56+
t.Parallel()
57+
58+
fx := storage.NewFakeExec()
59+
fx.Expect("pvs --noheadings -o pv_name",
60+
storage.FakeResponse{Stdout: []byte("")})
61+
62+
has, err := satellite.HasLVMSignature(t.Context(), fx, "/dev/sda")
63+
if err != nil {
64+
t.Fatalf("HasLVMSignature: %v", err)
65+
}
66+
67+
if has {
68+
t.Errorf("got has=true on empty pvs output, want false")
69+
}
70+
}
71+
72+
// TestHasZFSSignatureMissingTool pins the LVM-only-host path:
73+
// `zpool` not installed surfaces as exec error from FakeExec. The
74+
// detector swallows it (has=false) — the host can't have ZFS
75+
// signatures if there's no ZFS at all.
76+
func TestHasZFSSignatureMissingTool(t *testing.T) {
77+
t.Parallel()
78+
79+
fx := storage.NewFakeExec()
80+
fx.Expect("zpool list -PHv",
81+
storage.FakeResponse{Err: errZpoolNotFound})
82+
83+
has, err := satellite.HasZFSSignature(t.Context(), fx, "/dev/sda")
84+
if err != nil {
85+
t.Errorf("HasZFSSignature: missing tool must NOT propagate error, got %v", err)
86+
}
87+
88+
if has {
89+
t.Errorf("got has=true on missing zpool, want false")
90+
}
91+
}
92+
93+
// TestHasOtherSignatureWipefsNonEmpty: wipefs -n with non-empty
94+
// output (any signature found) → has=true. This is the catch-all
95+
// for filesystem / RAID / partition-table signatures lsblk's FSTYPE
96+
// column may have missed.
97+
func TestHasOtherSignatureWipefsNonEmpty(t *testing.T) {
98+
t.Parallel()
99+
100+
fx := storage.NewFakeExec()
101+
fx.Expect("wipefs -n /dev/sda",
102+
storage.FakeResponse{Stdout: []byte("/dev/sda: 8 bytes were erased at offset 0x00000218 (md_raid_member): fc 4e 2b a9\n")})
103+
104+
has, err := satellite.HasOtherSignature(t.Context(), fx, "/dev/sda")
105+
if err != nil {
106+
t.Fatalf("HasOtherSignature: %v", err)
107+
}
108+
109+
if !has {
110+
t.Errorf("got has=false on non-empty wipefs output, want true")
111+
}
112+
}
113+
114+
// TestIsDeviceFreeAllChecksPassButLVM pins the short-circuit
115+
// behaviour of IsDeviceFree: as soon as one of the four signature
116+
// detectors fires, the rest are skipped. This is load-bearing —
117+
// running drbdmeta on every device on a busy host is expensive.
118+
func TestIsDeviceFreeAllChecksPassButLVM(t *testing.T) {
119+
t.Parallel()
120+
121+
fx := storage.NewFakeExec()
122+
fx.Expect("pvs --noheadings -o pv_name",
123+
storage.FakeResponse{Stdout: []byte("/dev/sda\n")})
124+
125+
dev := &satellite.LsblkDevice{
126+
Name: "sda",
127+
KName: "sda",
128+
Type: "disk",
129+
// FSType, Mountpoint empty → passes IsFreeBlockDevice
130+
}
131+
132+
free, err := satellite.IsDeviceFree(t.Context(), fx, dev)
133+
if err != nil {
134+
t.Fatalf("IsDeviceFree: %v", err)
135+
}
136+
137+
if free {
138+
t.Errorf("got free=true, want false (sda has LVM signature)")
139+
}
140+
141+
// Pin the short-circuit: zpool / drbdmeta / wipefs MUST NOT
142+
// have been called once LVM said "no".
143+
for _, line := range fx.CommandLines() {
144+
if line != "pvs --noheadings -o pv_name" {
145+
t.Errorf("unexpected exec call after LVM positive: %q", line)
146+
}
147+
}
148+
}
149+
150+
// TestIsDeviceFreeShortCircuitsOnLsblkRejection: when the lsblk
151+
// row already disqualifies the device (TYPE=part), no signature
152+
// detectors run at all — saves time on busy hosts with thousands
153+
// of partitions.
154+
func TestIsDeviceFreeShortCircuitsOnLsblkRejection(t *testing.T) {
155+
t.Parallel()
156+
157+
fx := storage.NewFakeExec()
158+
dev := &satellite.LsblkDevice{Name: "sda1", KName: "sda1", Type: "part"}
159+
160+
free, err := satellite.IsDeviceFree(t.Context(), fx, dev)
161+
if err != nil {
162+
t.Fatalf("IsDeviceFree: %v", err)
163+
}
164+
165+
if free {
166+
t.Errorf("got free=true on TYPE=part, want false")
167+
}
168+
169+
if len(fx.CommandLines()) != 0 {
170+
t.Errorf("expected no exec calls on lsblk-rejected device; got %v", fx.CommandLines())
171+
}
172+
}

0 commit comments

Comments
 (0)