fix(storage): drop invalid lvcreate --kernel flag breaking LVM-thin clone/restore (BUG-043) + n-rst harness (BUG-044) (#158)

* fix(storage): drop invalid lvcreate --kernel flag from LVM-thin clone/restore (BUG-043)

Thin.RestoreVolumeFromSnapshot built an `lvcreate --snapshot ...`
command that included a `--kernel` token. That is not a valid lvcreate
option (`lvcreate: unrecognized option '--kernel'`, exit 3) on
LVM >= 2.03, so the destination backing LV was never created. As a
result DRBD never came up and clone / snapshot-restore onto LVM-thin
pools never converged to UpToDate. This is the shared seed path for
both clone and snapshot-restore on LVM-thin (LUKS cells default to
POOL=lvm-thin and hit it too); ZFS pools use a separate `zfs clone`
path and were unaffected.

Remove the `--kernel` token. The remaining args form a correct thin
snapshot/clone command — `lvcreate --snapshot --setactivationskip n
--activate y --name <tgt> <vg>/<src-snap>` — matching the normal
CreateSnapshot shape plus the activation flags needed to make the
cloned LV usable. The stale doc-comment is corrected accordingly.

Add a unit regression pinning the generated lvcreate argv for the
thin restore/clone path: it asserts the expected thin-snapshot form
and that no lvcreate invocation carries `--kernel`.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>

* test(harness): read wire flags via .flags not .rsc_flags in cli-matrix (BUG-044)

n-rst-recreates-tiebreaker and several sibling cli-matrix cells read
the resource-level flags from the `linstor resource list
--machine-readable` output via a `.rsc_flags` jq key that does not
exist on the blockstor wire — the field is `.flags`
(pkg/api/v1/resource.go: `Flags []string json:"flags"`). `.rsc_flags //
[]` therefore always evaluated to an empty array, so every flag check
silently counted zero matches: n-rst saw 0 TIE_BREAKER even though the
witness is correctly recreated, and the tiebreaker-collapse / reap /
diskless-selection assertions in the sibling cells degenerated to
no-ops or wrong-row selections.

Switch every `.rsc_flags` reference to `.flags`, mirroring the working
sibling cells (r-c-over-tiebreaker-skip-sync.sh, r-l-conns-shapes.sh)
that already read `.flags` from the same machine-readable resource
list. The product is correct; this is a harness-only fix.

Co-Authored-By: Claude <noreply@anthropic.com>
Signed-off-by: Andrei Kvapil <kvapss@gmail.com>

---------

Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>

Author: kvaps

pkg/storage/lvm/lvm_thin.go

@@ -256,7 +256,7 @@ func (t *Thin) DeleteSnapshot(ctx context.Context, snap storage.Snapshot) error
 //
 // Upstream LINSTOR equivalent:
 //
-//	lvcreate -s --kernel --activate y --name <tgt> <vg>/<src-snap>
+//	lvcreate -s --setactivationskip n --activate y --name <tgt> <vg>/<src-snap>
 //
 // Idempotent: target LV present → resumed reconcile, no-op.
 func (t *Thin) RestoreVolumeFromSnapshot(ctx context.Context, target storage.Volume, src storage.Snapshot) error {
@@ -272,7 +272,6 @@ func (t *Thin) RestoreVolumeFromSnapshot(ctx context.Context, target storage.Vol
 
 	_, err := t.exec.Run(ctx, "lvcreate",
 		Args("--snapshot",
-			"--kernel",
 			"--setactivationskip", "n",
 			"--activate", "y",
 			"--name", tgtName,

pkg/storage/lvm/lvm_thin_test.go

@@ -346,6 +346,52 @@ func TestThinCreateSnapshot(t *testing.T) {
 	}
 }
 
+// TestThinRestoreVolumeFromSnapshotIssuesThinSnapshotLvcreate is the
+// BUG-043 regression pin. The clone / snapshot-restore seed path on
+// LVM-thin must drive a plain thin-snapshot `lvcreate --snapshot
+// --setactivationskip n --activate y --name <tgt> <vg>/<src-snap>` —
+// the original code emitted a `--kernel` token, which is NOT a valid
+// lvcreate option (`lvcreate: unrecognized option '--kernel'`, exit 3)
+// on LVM >= 2.03. With that flag the destination backing LV was never
+// created, so DRBD never came up and clone/restore onto LVM-thin pools
+// never converged to UpToDate. The negative assertion (no `--kernel`)
+// is load-bearing; the positive assertion pins the activation
+// semantics so the cloned LV is actually usable (skip-activation
+// cleared, activated).
+func TestThinRestoreVolumeFromSnapshotIssuesThinSnapshotLvcreate(t *testing.T) {
+	fx := storage.NewFakeExec()
+	// Target LV not yet present → restore proceeds past the
+	// idempotency short-circuit.
+	fx.Expect("lvs --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --noheadings -o lv_name vg/pvc-2_00000",
+		storage.FakeResponse{Stdout: []byte("")})
+	// Source snapshot LV exists → clone source is found.
+	fx.Expect("lvs --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --noheadings -o lv_name vg/pvc-1_snap-1_00000",
+		storage.FakeResponse{Stdout: []byte("  pvc-1_snap-1_00000")})
+
+	p := lvm.NewThin(lvm.ThinConfig{VolumeGroup: "vg", ThinPool: "thinpool"}, fx)
+
+	err := p.RestoreVolumeFromSnapshot(t.Context(),
+		storage.Volume{ResourceName: "pvc-2", VolumeNumber: 0},
+		storage.Snapshot{ResourceName: "pvc-1", SnapshotName: "snap-1"})
+	if err != nil {
+		t.Fatalf("RestoreVolumeFromSnapshot: %v", err)
+	}
+
+	want := "lvcreate --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --snapshot --setactivationskip n --activate y --name pvc-2_00000 vg/pvc-1_snap-1_00000"
+	if !slices.Contains(fx.CommandLines(), want) {
+		t.Errorf("expected %q in calls; got %v", want, fx.CommandLines())
+	}
+
+	// BUG-043: no lvcreate invocation may carry the invalid `--kernel`
+	// flag — it makes LVM >= 2.03 reject the command with exit 3.
+	for _, line := range fx.CommandLines() {
+		if strings.HasPrefix(line, "lvcreate ") && strings.Contains(line, "--kernel") {
+			t.Errorf("BUG-043: thin restore lvcreate must NOT pass the invalid "+
+				"--kernel flag; got %q", line)
+		}
+	}
+}
+
 // TestThinExecError surfaces the wrapped exec error verbatim.
 func TestThinExecError(t *testing.T) {
 	fx := storage.NewFakeExec()

tests/e2e/cli-matrix/multi-volume-late-vd-create.sh

@@ -126,7 +126,7 @@ if [[ "$late_up" != "true" ]]; then
     # Surface the smoking gun: a Diskless line for a non-DISKLESS spec.
     echo "----- linstor r l --resources $RD (with flags) -----" >&2
     "${LCTL[@]}" --machine-readable resource list --resources "$RD" 2>/dev/null \
-        | jq -r '.[][]? | "\(.node_name) vol=\(.vlms[]?.vlm_nr) state=\(.vlms[]?.state.disk_state) flags=\(.rsc_flags//[])"' >&2 || true
+        | jq -r '.[][]? | "\(.node_name) vol=\(.vlms[]?.vlm_nr) state=\(.vlms[]?.state.disk_state) flags=\(.flags//[])"' >&2 || true
     exit 1
 fi
 
@@ -137,7 +137,7 @@ fi
 # Bug 332 (Unintentional Diskless) from spec-pinned diskless replicas.
 echo ">> [Bug 332] kernel-truth: drbdadm status on a diskful node"
 satellite_node=$("${LCTL[@]}" --machine-readable resource list --resources "$RD" 2>/dev/null \
-    | jq -r '.[][]? | select((.rsc_flags//[]) | (map(. == "DISKLESS") | any | not)) | .node_name' \
+    | jq -r '.[][]? | select((.flags//[]) | (map(. == "DISKLESS") | any | not)) | .node_name' \
     2>/dev/null | head -1)
 
 if [[ -z "$satellite_node" ]]; then

tests/e2e/cli-matrix/n-rst-recreates-tiebreaker.sh

@@ -186,7 +186,7 @@ n_tb=0
 while (( $(date +%s) < deadline )); do
     wire=$(linstor_r_l_json "$RD")
     n_tb=$(printf '%s' "$wire" \
-        | jq -r '.[][] | select((.rsc_flags // []) | index("TIE_BREAKER")) | .name' 2>/dev/null \
+        | jq -r '.[][] | select((.flags // []) | index("TIE_BREAKER")) | .name' 2>/dev/null \
         | wc -l | tr -d ' ' || echo 0)
     if [[ "$n_tb" == "1" ]]; then
         break
@@ -195,7 +195,7 @@ while (( $(date +%s) < deadline )); do
 done
 if [[ "$n_tb" != "1" ]]; then
     echo "FAIL (Bug 386): linstor r l shows $n_tb TIE_BREAKER rows for $RD, want 1" >&2
-    printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .rsc_flags}' 2>/dev/null >&2 || true
+    printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .flags}' 2>/dev/null >&2 || true
     exit 1
 fi
 

tests/e2e/cli-matrix/r-d-collapses-tiebreaker.sh

@@ -165,11 +165,11 @@ wire=$(linstor_r_l_json "$RD")
 n_wire=$(printf '%s' "$wire" | jq -r '.[][] | .name' 2>/dev/null | wc -l | tr -d ' ' || echo 0)
 if [[ "$n_wire" != "1" ]]; then
     echo "FAIL (Bug 338): linstor r l shows $n_wire rows for $RD, want 1" >&2
-    printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .rsc_flags}' 2>/dev/null >&2 || true
+    printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .flags}' 2>/dev/null >&2 || true
     exit 1
 fi
 
-wire_flags=$(printf '%s' "$wire" | jq -r '.[][] | .rsc_flags // [] | join(",")' 2>/dev/null || echo "")
+wire_flags=$(printf '%s' "$wire" | jq -r '.[][] | .flags // [] | join(",")' 2>/dev/null || echo "")
 if [[ "$wire_flags" == *"TIE_BREAKER"* ]] || [[ "$wire_flags" == *"DISKLESS"* ]]; then
     echo "FAIL (Bug 338): surviving row carries unexpected flags=$wire_flags" >&2
     exit 1

tests/e2e/cli-matrix/r-td-diskless-reaps-tiebreaker.sh

@@ -171,11 +171,11 @@ wire=$(linstor_r_l_json "$RD")
 n_wire=$(printf '%s' "$wire" | jq -r '.[][] | .name' 2>/dev/null | wc -l | tr -d ' ' || echo 0)
 if [[ "$n_wire" != "2" ]]; then
     echo "FAIL: linstor r l shows $n_wire rows for $RD, want 2" >&2
-    printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .rsc_flags}' 2>/dev/null >&2 || true
+    printf '%s\n' "$wire" | jq '.[][]| {name, node_name, flags: .flags}' 2>/dev/null >&2 || true
     exit 1
 fi
 
-wire_flags=$(printf '%s' "$wire" | jq -r '.[][] | .rsc_flags // [] | join(",")' 2>/dev/null || echo "")
+wire_flags=$(printf '%s' "$wire" | jq -r '.[][] | .flags // [] | join(",")' 2>/dev/null || echo "")
 if [[ "$wire_flags" == *"TIE_BREAKER"* ]]; then
     echo "FAIL: a surviving row still carries TIE_BREAKER (flags=$wire_flags)" >&2
     exit 1

tests/e2e/cli-matrix/rd-d-deleting-surface.sh

@@ -135,13 +135,13 @@ while (( $(date +%s) < deadline )); do
     # Wire-level assertion: the Resource view carries the DELETE flag for
     # the blocked replica. golinstor's `r l -o json` is a doubly-nested
     # array of per-replica rows; the resource-level flags live under
-    # `rsc_flags` (see multi-volume-late-vd-create.sh / n-rst-recreates-
+    # `flags` (see multi-volume-late-vd-create.sh / n-rst-recreates-
     # tiebreaker.sh for the same shape).
     rl_json=$("${LCTL[@]}" resource list --resources "$RD" -o json 2>/dev/null || echo '[]')
     has_delete=$(jq -r --arg n "$N2" '
         [ .[]?[]?
           | select(.node_name == $n)
-          | (.rsc_flags // []) | index("DELETE") ] | any' <<<"$rl_json" 2>/dev/null || echo false)
+          | (.flags // []) | index("DELETE") ] | any' <<<"$rl_json" 2>/dev/null || echo false)
     # Fallback: the human-readable State column rendered by the CLI.
     rl_txt=$("${LCTL[@]}" resource list --resources "$RD" 2>/dev/null || true)
     if [[ "$has_delete" == "true" ]] || grep -qiE 'DELETING' <<<"$rl_txt"; then

tests/e2e/cli-matrix/vd-d-prune-resource-volumes-no-flap.sh

@@ -119,7 +119,7 @@ both_up=false
 while (( $(date +%s) < deadline )); do
     # 2 diskful replicas × 2 volumes = 4 disk_state strings, all UpToDate.
     states=$("${LCTL[@]}" --machine-readable resource list --resources "$RD" 2>/dev/null \
-        | jq -r '[.[][]? | select((.rsc_flags//[]) | (map(. == "DISKLESS" or . == "TIE_BREAKER") | any | not)) | .vlms[]? | .state.disk_state // "Unknown"] | join(",")' \
+        | jq -r '[.[][]? | select((.flags//[]) | (map(. == "DISKLESS" or . == "TIE_BREAKER") | any | not)) | .vlms[]? | .state.disk_state // "Unknown"] | join(",")' \
         2>/dev/null || echo "")
     count_uptodate=$(awk -F, '{ for (i=1;i<=NF;i++) if ($i=="UpToDate") n++ } END { print n+0 }' <<<"$states")
     if (( count_uptodate == 4 )); then