fix(drbd): peer hosts render upstream's placeholder, not local disk

The .res-file renderer copied the local node's `Volume.Disk` into
every `on <node> { … }` block. That worked when both sides happened
to use the same backing path (LVM with the same VG name, etc.) but
broke the moment paths differed — e.g. FILE_THIN, where each
satellite's `losetup --find` lands on a different /dev/loopN.

Mirror upstream LINSTOR's ConfFileBuilder:

  - local diskful host          → real backing-disk path
  - local OR peer diskless host → `disk none`
  - peer diskful host           → `disk /dev/drbd/this/is/not/used`

drbd never reads the peer-side `disk` field, but the parser
requires a stable non-empty / non-`none` token; using the peer's
real path would clash when the peer is mid-conversion from
diskless. The constant placeholder is what upstream emits and
what drbd users have indexed off for years.

To carry the per-peer diskless signal end-to-end:

  - dispatcher.addPeerEntries now stamps
    `peer.<name>.diskless=true` on the drbd_options bag when the
    peer Resource carries the DISKLESS flag.
  - satellite reconciler's host-list builder reads that key and
    sets the new drbd.Host{IsLocal, Diskless} fields.
  - drbd.writeOnBlock consumes the flags to pick the right `disk`
    via a small `diskField` helper.

New TestBuildPeerDisks pins the precedence so a regression here
is a unit-test failure rather than a multi-node DRBD setup that
silently refuses to come up.

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

Author: kvaps

pkg/dispatcher/dispatcher.go

@@ -41,6 +41,12 @@ import (
 // it renders the file (drbd doesn't accept literal 0.0.0.0).
 const drbdAddrAny = "0.0.0.0"
 
+// boolPropTrue is the canonical string-form `true` value blockstor
+// stamps on drbd_options bag keys that are flag-like (auto-primary,
+// peer.<n>.diskless, …). Pinning the literal avoids consumer-side
+// drift between `"true"`/`"True"`/`"1"`.
+const boolPropTrue = "true"
+
 // BuildDesired translates a Resource + its same-RD peers into the
 // satellite-facing DesiredResource. Port/minor/node-id assignment is
 // deterministic from the RD name + sorted peer list — good enough for
@@ -106,7 +112,7 @@ func BuildDesired(target *blockstoriov1alpha1.Resource, peers []blockstoriov1alp
 	// primary --force only on firstActivation.
 	if !slices.Contains(target.Spec.Flags, "DISKLESS") &&
 		idOf[target.Spec.NodeName] == lowestDiskfulID(target, peers) {
-		drbdOpts["auto-primary"] = "true"
+		drbdOpts["auto-primary"] = boolPropTrue
 	}
 
 	addPeerEntries(drbdOpts, dropped, peers, nodes, port, idOf)
@@ -247,6 +253,13 @@ func addPeerEntries(drbdOpts map[string]string, dropped []string, peers []blocks
 		drbdOpts["peer."+peer+".port"] = strconv.Itoa(peerPort)
 		drbdOpts["peer."+peer+".node-id"] = strconv.Itoa(int(idOf[peer]))
 		drbdOpts["peer."+peer+".address"] = peerAddress(peer, nodes)
+
+		// Surface the peer's DISKLESS flag so the satellite's .res
+		// renderer can emit `disk none;` instead of the diskful
+		// placeholder for diskless peers.
+		if p, ok := peerByName[peer]; ok && slices.Contains(p.Spec.Flags, "DISKLESS") {
+			drbdOpts["peer."+peer+".diskless"] = boolPropTrue
+		}
 	}
 }
 

pkg/drbd/conffile.go

@@ -86,6 +86,21 @@ type Host struct {
 	// NodeID is the drbd-9 node identifier (0..31). Must be unique
 	// within the resource's host list.
 	NodeID int
+
+	// IsLocal marks this host as the satellite rendering the .res
+	// file (only one host per resource has this true). Peer hosts
+	// get a placeholder `disk` value upstream LINSTOR uses
+	// (`/dev/drbd/this/is/not/used`) — drbd never reads the peer
+	// host's `disk` field, but the parser refuses an empty one and
+	// treats `none` as DISKLESS, so a constant placeholder is what
+	// upstream emits.
+	IsLocal bool
+
+	// Diskless marks the host as a DRBD diskless replica — `disk`
+	// must be rendered as the literal `none`. Same value on local
+	// vs. peer hosts: a peer that's diskless still gets `none`,
+	// not the placeholder.
+	Diskless bool
 }
 
 // Volume is one data volume on the resource.
@@ -178,23 +193,48 @@ func writeOptions(b *strings.Builder, opts map[string]string) {
 }
 
 // writeOnBlock emits one `on <node> { … }` block including every volume
-// definition for this peer.
-func writeOnBlock(b *strings.Builder, h *Host, vols []Volume) {
-	fmt.Fprintf(b, "  on %s {\n", h.NodeName)
-	fmt.Fprintf(b, "    address %s:%d;\n", h.Address, h.Port)
-	fmt.Fprintf(b, "    node-id %d;\n", h.NodeID)
-
-	for _, v := range vols {
-		fmt.Fprintf(b, "    volume %d {\n", v.Number)
-		fmt.Fprintf(b, "      device %s minor %d;\n", v.Device, v.Minor)
-		fmt.Fprintf(b, "      disk %s;\n", v.Disk)
+// definition for this peer. The `disk` value follows upstream LINSTOR's
+// ConfFileBuilder:
+//   - DISKLESS host (local or peer) → `none`
+//   - local diskful host → the real backing path (Volume.Disk)
+//   - peer diskful host → the literal placeholder `/dev/drbd/this/is/not/used`
+//
+// The peer placeholder exists because drbd never reads the peer's
+// `disk` field but the parser rejects empty / requires a stable
+// non-`none` token; using each peer's actual backing path would
+// also clash when a peer is mid-conversion from diskless and its
+// path is reported as `none`.
+const peerDiskPlaceholder = "/dev/drbd/this/is/not/used"
+
+func writeOnBlock(b *strings.Builder, host *Host, vols []Volume) {
+	fmt.Fprintf(b, "  on %s {\n", host.NodeName)
+	fmt.Fprintf(b, "    address %s:%d;\n", host.Address, host.Port)
+	fmt.Fprintf(b, "    node-id %d;\n", host.NodeID)
+
+	for _, vol := range vols {
+		fmt.Fprintf(b, "    volume %d {\n", vol.Number)
+		fmt.Fprintf(b, "      device %s minor %d;\n", vol.Device, vol.Minor)
+		fmt.Fprintf(b, "      disk %s;\n", diskField(host, vol))
 		b.WriteString("      meta-disk internal;\n")
 		b.WriteString("    }\n")
 	}
 
 	b.WriteString("  }\n")
 }
 
+// diskField picks the `disk` value to render for one (host, volume)
+// pair. See writeOnBlock for the precedence.
+func diskField(host *Host, vol Volume) string {
+	switch {
+	case host.Diskless:
+		return "none"
+	case host.IsLocal:
+		return vol.Disk
+	default:
+		return peerDiskPlaceholder
+	}
+}
+
 // writeConnectionMesh emits one `connection { … }` block per (i, j)
 // host pair with i < j. drbd-9 requires the mesh to be explicit; with
 // N>2 peers an implicit "everyone talks to everyone" doesn't exist.

pkg/drbd/conffile_test.go

@@ -47,7 +47,7 @@ func TestBuildSinglePeerSingleVolume(t *testing.T) {
 		Name: "pvc-1",
 		Net:  drbd.Net{ProtocolC: true},
 		Hosts: []drbd.Host{
-			{NodeName: "n1", Address: "10.0.0.1", Port: 7000, NodeID: 0},
+			{NodeName: "n1", Address: "10.0.0.1", Port: 7000, NodeID: 0, IsLocal: true},
 			{NodeName: "n2", Address: "10.0.0.2", Port: 7000, NodeID: 1},
 		},
 		Volumes: []drbd.Volume{
@@ -66,9 +66,14 @@ func TestBuildSinglePeerSingleVolume(t *testing.T) {
 		"    node-id 0;",
 		"    volume 0 {",
 		"      device /dev/drbd1000 minor 1000;",
+		// Local node uses the real backing disk path.
 		"      disk /dev/vg/pvc-1_00000;",
 		"      meta-disk internal;",
 		"  on n2 {",
+		// Peer node uses upstream's placeholder — drbd never reads
+		// the peer-side `disk`, but the parser requires a stable
+		// non-empty / non-`none` token.
+		"      disk /dev/drbd/this/is/not/used;",
 	} {
 		if !strings.Contains(got, want) {
 			t.Errorf("missing %q in output:\n%s", want, got)
@@ -166,6 +171,43 @@ func TestBuildEmitsResourceOptions(t *testing.T) {
 	}
 }
 
+// TestBuildPeerDisks pins the `disk` precedence upstream LINSTOR
+// uses: local diskful → real path; local diskless → `none`;
+// peer diskful → `/dev/drbd/this/is/not/used`; peer diskless →
+// `none` (not the placeholder).
+func TestBuildPeerDisks(t *testing.T) {
+	got, err := drbd.Build(drbd.Resource{
+		Name: "pvc-1",
+		Net:  drbd.Net{ProtocolC: true},
+		Hosts: []drbd.Host{
+			{NodeName: "n1", Address: "10.0.0.1", Port: 7000, NodeID: 0, IsLocal: true},
+			{NodeName: "n2", Address: "10.0.0.2", Port: 7000, NodeID: 1},
+			{NodeName: "n3", Address: "10.0.0.3", Port: 7000, NodeID: 2, Diskless: true},
+		},
+		Volumes: []drbd.Volume{
+			{Number: 0, Device: "/dev/drbd1000", Disk: "/dev/loop42", Minor: 1000},
+		},
+	})
+	if err != nil {
+		t.Fatalf("Build: %v", err)
+	}
+
+	cases := []struct {
+		needle string
+		why    string
+	}{
+		{"  on n1 {\n    address 10.0.0.1:7000;\n    node-id 0;\n    volume 0 {\n      device /dev/drbd1000 minor 1000;\n      disk /dev/loop42;\n", "local diskful → real path"},
+		{"  on n2 {\n    address 10.0.0.2:7000;\n    node-id 1;\n    volume 0 {\n      device /dev/drbd1000 minor 1000;\n      disk /dev/drbd/this/is/not/used;\n", "peer diskful → placeholder"},
+		{"  on n3 {\n    address 10.0.0.3:7000;\n    node-id 2;\n    volume 0 {\n      device /dev/drbd1000 minor 1000;\n      disk none;\n", "peer diskless → none"},
+	}
+
+	for _, c := range cases {
+		if !strings.Contains(got, c.needle) {
+			t.Errorf("%s: missing block\n  want substring %q\n  in:\n%s", c.why, c.needle, got)
+		}
+	}
+}
+
 // TestBuildDeterministic: same input → same output, twice in a row. Map
 // iteration order would otherwise leak into the .res file and trigger
 // spurious drbdadm reloads on every reconcile.

pkg/satellite/reconciler.go

@@ -587,7 +587,7 @@ func (r *Reconciler) applyDRBD(ctx context.Context, dr *satellitepb.DesiredResou
 	// "Inconsistent" into "UpToDate" without a human running
 	// drbdadm. Subsequent reconciles see firstActivation=false and
 	// skip the seed.
-	if firstActivation && !diskless && dr.GetDrbdOptions()["auto-primary"] == "true" {
+	if firstActivation && !diskless && dr.GetDrbdOptions()["auto-primary"] == drbdBoolPropTrue {
 		err = r.cfg.Adm.PrimaryForce(ctx, dr.GetName())
 		if err != nil {
 			return errors.Wrapf(err, "auto-primary %s", dr.GetName())
@@ -688,6 +688,8 @@ func buildResFile(dr *satellitepb.DesiredResource, localNode, localAddr string,
 		Address:  resolveAddr(opts["address"], localAddr),
 		Port:     port,
 		NodeID:   nodeID,
+		IsLocal:  true,
+		Diskless: isDiskless(dr.GetFlags()),
 	})
 
 	for _, peer := range dr.GetPeers() {
@@ -699,6 +701,7 @@ func buildResFile(dr *satellitepb.DesiredResource, localNode, localAddr string,
 			Address:  resolveAddr(opts["peer."+peer+".address"], ""),
 			Port:     peerPort,
 			NodeID:   peerNodeID,
+			Diskless: opts["peer."+peer+".diskless"] == drbdBoolPropTrue,
 		})
 	}
 
@@ -789,6 +792,12 @@ func splitDRBDOptions(opts map[string]string) (map[string]string, map[string]str
 // substitutes the satellite's own IP whenever it sees this value.
 const drbdAddrPlaceholder = "0.0.0.0"
 
+// drbdBoolPropTrue mirrors dispatcher.boolPropTrue — the literal
+// `true` the dispatcher stamps on flag-like drbd_options keys. We
+// inline rather than re-export to keep `pkg/satellite` from
+// importing `pkg/dispatcher` just for one constant.
+const drbdBoolPropTrue = "true"
+
 // resolveAddr substitutes the satellite's own IP whenever the
 // controller-supplied address is the placeholder (which it is until
 // the controller starts learning each satellite's pod IP and passing