fix(satellite): fire late-add metadata/self-heal only for unattached desired volumes (BUG-048 deadlock)

pkg/drbd/attached_volumes_test.go

@@ -0,0 +1,114 @@
+// SPDX-License-Identifier: Apache-2.0
+
+/*
+Copyright 2026 Cozystack contributors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package drbd_test
+
+import (
+	"testing"
+
+	"github.com/cozystack/blockstor/pkg/drbd"
+	"github.com/cozystack/blockstor/pkg/storage"
+)
+
+// AttachedVolumes is the kernel-truth backing for the BUG-048 resize-deadlock
+// gate: it returns the LOCAL volumes that are present-and-attached (have a
+// valid DRBD-9 metadata block — disk-state UpToDate / Inconsistent /
+// Consistent / Outdated). A volume absent from the device list, or Diskless /
+// mid-transition, is excluded so the reconciler still runs the metadata pass
+// for a genuine late-add but never for a converged/resizing RD.
+
+func attachedVolumesAdm(t *testing.T, json string) *drbd.Adm {
+	t.Helper()
+
+	fx := storage.NewFakeExec()
+	fx.Responses["drbdsetup status pvc-av --json"] = storage.FakeResponse{Stdout: []byte(json)}
+
+	return drbd.NewAdm(fx)
+}
+
+func assertAttachedSet(t *testing.T, got map[int32]struct{}, want ...int32) {
+	t.Helper()
+
+	if len(got) != len(want) {
+		t.Fatalf("attached set size = %d, want %d (got %v, want %v)", len(got), len(want), got, want)
+	}
+
+	for _, v := range want {
+		if _, ok := got[v]; !ok {
+			t.Errorf("expected volume %d in attached set %v", v, got)
+		}
+	}
+}
+
+// Every metadata-bearing disk-state counts as attached. Diskless and
+// transient states do NOT — those are volumes that still need the metadata
+// pass (a genuine late-add or a re-attach).
+func TestAttachedVolumes_OnlyMetadataBearingStatesCount(t *testing.T) {
+	adm := attachedVolumesAdm(t, `[{
+	  "name":"pvc-av","node-id":0,"role":"Secondary",
+	  "devices":[
+	    {"volume":0,"disk-state":"UpToDate"},
+	    {"volume":1,"disk-state":"Inconsistent"},
+	    {"volume":2,"disk-state":"Consistent"},
+	    {"volume":3,"disk-state":"Outdated"},
+	    {"volume":4,"disk-state":"Diskless"},
+	    {"volume":5,"disk-state":"Negotiating"},
+	    {"volume":6,"disk-state":"DUnknown"}
+	  ]
+	}]`)
+
+	// 0..3 are attached (metadata-bearing); 4 (Diskless), 5 (Negotiating),
+	// 6 (DUnknown) are not.
+	assertAttachedSet(t, adm.AttachedVolumes(t.Context(), "pvc-av"), 0, 1, 2, 3)
+}
+
+// A volume entirely absent from the kernel device list (the genuine late-add
+// window — `vd c` added it to the desired set but the kernel has not brought
+// it up yet) is simply not in the set, so the reconciler sees it as
+// unattached and runs the metadata pass for it.
+func TestAttachedVolumes_AbsentVolumeNotInSet(t *testing.T) {
+	adm := attachedVolumesAdm(t, `[{
+	  "name":"pvc-av","node-id":0,"role":"Secondary",
+	  "devices":[
+	    {"volume":0,"disk-state":"UpToDate"}
+	  ]
+	}]`)
+
+	got := adm.AttachedVolumes(t.Context(), "pvc-av")
+	assertAttachedSet(t, got, 0)
+
+	if _, ok := got[1]; ok {
+		t.Errorf("volume 1 is absent from the device list and must NOT be reported attached; got %v", got)
+	}
+}
+
+// Conservative toward FIRING (fail-toward-correctness): on a probe failure,
+// empty output, or an unloaded slot the set is empty, so the reconciler treats
+// every desired volume as possibly-unattached and runs the idempotent metadata
+// pass — the pre-#164 behaviour. The deadlock-relevant skip only happens once
+// the kernel positively reports volumes attached.
+func TestAttachedVolumes_ConservativeEmptyOnNoKernelSlot(t *testing.T) {
+	fx := storage.NewFakeExec()
+	// No registered response → FakeExec returns empty stdout, nil error
+	// (the "kernel slot present but status empty" / blank-output shape).
+	adm := drbd.NewAdm(fx)
+
+	if got := adm.AttachedVolumes(t.Context(), "pvc-missing"); len(got) != 0 {
+		t.Errorf("expected empty attached set on no kernel data; got %v", got)
+	}
+}

pkg/drbd/drbdsetup_show.go

@@ -324,3 +324,72 @@ func parseShowJSON(out []byte) map[string]KernelSlot {
 
 	return slots
 }
+
+// AttachedVolumes probes `drbdsetup status <res> --json` and returns the
+// set of LOCAL volume numbers that are present-and-attached in the kernel —
+// i.e. the kernel has a lower disk attached and a valid DRBD-9 metadata
+// block for them (disk-state UpToDate / Inconsistent / Consistent /
+// Outdated). A volume that is absent from the device list entirely, or
+// present but Diskless / DUnknown / mid-transition (Attaching / Detaching /
+// Failed / Negotiating), is NOT in the returned set.
+//
+// DRBD invariant exploited by the BUG-048 resize-deadlock fix: the kernel
+// CANNOT attach a lower disk without a valid metadata block, so an attached
+// volume ALREADY has metadata — there is nothing for ensurePerVolumeMetadata
+// to create-md and a `drbdadm dump-md` probe on it is pointless. That
+// dump-md is an md_buffer consumer; firing it on every post-activation
+// reconcile contends with an in-flight `vd s` resize (DRBD holds md_buffer
+// in change_cluster_wide_device_size / drbd_determine_dev_size), which is
+// the reboot-proof cluster-wide deadlock BUG-048's #164 gate widening
+// introduced. The reconciler gates the per-volume metadata pass on "some
+// DESIRED volume is NOT in this set" so the probe only fires for a genuine
+// late-add (a volume that really still needs metadata) and never on a
+// converged or resizing RD whose volumes are all attached.
+//
+// Conservative on any probe/parse failure (kernel slot absent, status
+// non-zero, malformed JSON, missing node-id) → empty set. An empty set means
+// "nothing is known-attached", so the caller treats every desired volume as
+// possibly needing metadata and runs the (idempotent) per-volume pass — the
+// fail-toward-correctness direction (the pre-BUG-048 behaviour).
+func (a *Adm) AttachedVolumes(ctx context.Context, resource string) map[int32]struct{} {
+	out, err := a.exec.Run(ctx, "drbdsetup", "status", resource, "--json")
+	if err != nil {
+		return nil
+	}
+
+	var status drbdsetupStatusRoot
+
+	err = json.Unmarshal(out, &status)
+	if err != nil || len(status) == 0 {
+		return nil
+	}
+
+	return attachedVolumeSet(status[0].Devices)
+}
+
+// attachedVolumeSet returns the volume numbers from `devices` whose
+// disk-state proves the lower disk is attached with a valid metadata block.
+// Split out of AttachedVolumes so unit tests can drive it from fixtures
+// without the FakeExec round-trip.
+func attachedVolumeSet(devices []drbdsetupStatusDevice) map[int32]struct{} {
+	out := map[int32]struct{}{}
+
+	for _, dev := range devices {
+		switch DiskState(dev.DiskState) {
+		case DiskStateUpToDate, DiskStateInconsistent,
+			DiskStateConsistent, DiskStateOutdated:
+			// Lower disk attached with metadata — DRBD cannot reach any of
+			// these states without a valid DRBD-9 superblock. No create-md /
+			// dump-md needed for this volume.
+			out[dev.VolumeNumber] = struct{}{}
+		case DiskStateDiskless, DiskStateAttaching, DiskStateDetaching,
+			DiskStateFailed, DiskStateNegotiating, DiskStateDUnknown:
+			// Diskless (no lower disk), or a transient / failed state where
+			// the disk is not cleanly attached: treat as still-needing the
+			// metadata pass so a genuine late-add or a re-attach is not
+			// skipped.
+		}
+	}
+
+	return out
+}