From eb46931eb42f7f8c67713254b26b659b5f86f720 Mon Sep 17 00:00:00 2001 From: Andrei Kvapil Date: Thu, 14 May 2026 17:39:33 +0200 Subject: [PATCH] test(wave2): DRBD ping-timeout net-option propagation (5.W03) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pin scenario 5.W03 (wave2-05-drbd-state-recovery.md): the generic `DrbdOptions/Net/ping-timeout` propagation chain through the satellite's `splitDRBDOptions` lands the value verbatim inside the rendered `net { … }` block as `ping-timeout 500;` (DRBD time encoding is 1/10s; 500 = 50.0s, the UG9 example). The chain is already wired by 5.W01 (RD scope, `--protocol C`): - effectiveprops.Resolve folds controller / RG / RD / Resource scopes into one flat prop bag via maps.Copy — no allowlist. - dispatcher.BuildDesired routes every `DrbdOptions/
/*` key into DesiredResource.DrbdOptions verbatim. - satellite.splitDRBDOptions partitions on the `
` token; `Net/*` keys go to drbd.Net.Options, the rest to top-level `options { }`. - drbd.Build emits them sorted into the rendered .res. 5.W03's contribution is operator-scope — `linstor node-connection drbd-peer-options --ping-timeout` writes the same prop at a different point in the hierarchy. By the time it reaches the satellite the bag is flat, so the renderer contract is scope- agnostic and one test covers controller / RD / node-connection / resource-connection paths uniformly. `TestApplyRendersPingTimeoutIntoNetBlock`: - 2-replica RD on n1↔n2 (single-replica .res has no connection{} mesh and would let a regression that only fires on multi-peer paths slip past). - Asserts the `net {` framing exists before grepping the substring (catches a renderer regression that suppresses the block entirely with a clearer error). - Asserts the unstripped `DrbdOptions/Net/ping-timeout` key does NOT leak — drbdadm rejects that form on parse and the .res would refuse to load even though `grep ping-timeout` would spuriously pass. Co-Authored-By: Claude Signed-off-by: Andrei Kvapil --- pkg/satellite/reconciler_drbd_test.go | 121 ++++++++++++++++++ .../scenarios/wave2-05-drbd-state-recovery.md | 4 +- 2 files changed, 124 insertions(+), 1 deletion(-) diff --git a/pkg/satellite/reconciler_drbd_test.go b/pkg/satellite/reconciler_drbd_test.go index 22661e7..8761f6e 100644 --- a/pkg/satellite/reconciler_drbd_test.go +++ b/pkg/satellite/reconciler_drbd_test.go @@ -4369,3 +4369,124 @@ func TestApplyResFileUsesKernelHostnameNotLINSTORName(t *testing.T) { badOn, got) } } + +// TestApplyRendersPingTimeoutIntoNetBlock pins scenario 5.W03 +// (wave2-05 §5.W03, UG9 §"Setting DRBD options for node connections"). +// `DrbdOptions/Net/ping-timeout` is set at any scope the operator +// reaches it from — `linstor controller set-property`, `linstor rd +// drbd-options --ping-timeout`, or `linstor node-connection +// drbd-peer-options --ping-timeout`. By the time it arrives at the +// satellite the effective-props resolver has already folded the +// scope hierarchy into one flat bag on `DesiredResource.DrbdOptions`, +// so the renderer's contract is scope-agnostic: any +// `DrbdOptions/Net/` entry lands verbatim inside the rendered +// `net { … }` block with the section prefix stripped (the form +// drbdadm parses). +// +// `ping-timeout 500;` is the canonical example from UG9 — DRBD +// encodes the value in 1/10-second units, so `500` is 50.0 s. We +// assert against a 2-replica RD on n1↔n2 to mirror the production +// shape where every per-peer net option must reach a mesh of N>1 +// peers (a single-replica .res has no `connection { … }` block at +// all and would let a renderer regression that only fires on multi- +// peer paths slip through). +// +// Pinning the `net {` framing — rather than the bare `ping-timeout` +// substring — keeps the assertion honest: a regression that dropped +// the prop from `splitDRBDOptions` but left a hand-typed default +// elsewhere in the renderer would still leave the substring present +// at the wrong scope. +func TestApplyRendersPingTimeoutIntoNetBlock(t *testing.T) { + dir := t.TempDir() + fx := storage.NewFakeExec() + fx.Expect("lvs --config devices { filter=['r|^/dev/drbd|','r|^/dev/zd|'] } --noheadings -o lv_name vg/pvc-ping_00000", + storage.FakeResponse{Stdout: []byte("")}) + + thin := lvm.NewThin(lvm.ThinConfig{VolumeGroup: "vg", ThinPool: "tp"}, fx) + rec := satellite.NewReconciler(satellite.ReconcilerConfig{ + Providers: map[string]storage.Provider{"thin1": thin}, + Adm: drbd.NewAdm(fx), + StateDir: dir, + NodeName: "n1", + }) + + _, err := rec.Apply(t.Context(), []*intent.DesiredResource{ + { + Name: "pvc-ping", + NodeName: "n1", + Volumes: []*intent.DesiredVolume{ + {VolumeNumber: 0, SizeKib: 1024 * 1024, StoragePool: "thin1"}, + }, + Peers: []string{"n2"}, + DrbdOptions: map[string]string{ + "port": "7000", + "node-id": "0", + "address": "10.0.0.1", + "minor": "1000", + "peer.n2.address": "10.0.0.2", + "peer.n2.node-id": "1", + "peer.n2.port": "7000", + + // 5.W03 payload: the effective-props resolver folded + // the operator-set net-option into this bag regardless + // of which scope (`linstor controller set-property` / + // `linstor rd drbd-options --ping-timeout` / `linstor + // node-connection drbd-peer-options --ping-timeout`) + // they reached it from. From here on the contract is + // scope-agnostic. + "DrbdOptions/Net/ping-timeout": "500", + }, + }, + }) + if err != nil { + t.Fatalf("Apply: %v", err) + } + + body, err := os.ReadFile(filepath.Join(dir, "pvc-ping.res")) + if err != nil { + t.Fatalf("read .res: %v", err) + } + + got := string(body) + + // The 2-replica RD must render a connection mesh — without one + // any per-peer net option is moot. Assert the framing first so a + // renderer regression that suppresses the mesh entirely produces + // a clear error rather than a misleading "missing ping-timeout" + // follow-on. + if !strings.Contains(got, "connection {") { + t.Errorf(".res missing connection{} mesh; body=%s", got) + } + + // Must contain a top-level `net { … ping-timeout 500; … }` block + // — the scenario's load-bearing assertion against `grep + // ping-timeout /var/lib/linstor.d/pvc-ping.res`. Pinning the + // `net {` framing (not just the bare `ping-timeout 500;` + // substring) keeps the assertion honest: a regression that + // dropped the prop from splitDRBDOptions but left a hand-typed + // default elsewhere in the renderer would still leave the + // substring present at the wrong scope. + netStart := strings.Index(got, "net {") + if netStart < 0 { + t.Fatalf(".res missing net{} block; body=%s", got) + } + + netEnd := strings.Index(got[netStart:], "}") + if netEnd < 0 { + t.Fatalf("net{} block not delimited; body=%s", got) + } + + netBlock := got[netStart : netStart+netEnd] + if !strings.Contains(netBlock, "ping-timeout 500;") { + t.Errorf("net{} block missing `ping-timeout 500;`; net-block=%q\nfull=%s", netBlock, got) + } + + // Defence in depth: the LINSTOR prop key must NOT leak verbatim + // into the .res — splitDRBDOptions strips the `DrbdOptions/Net/` + // prefix before handing the bare key to the ConfFileBuilder. + // drbdadm rejects the unstripped form with "expected: cpu-mask + // | ... but got 'DrbdOptions'". + if strings.Contains(got, "DrbdOptions/Net/ping-timeout") { + t.Errorf("unstripped LINSTOR prop key leaked into .res; body=%s", got) + } +} diff --git a/tests/scenarios/wave2-05-drbd-state-recovery.md b/tests/scenarios/wave2-05-drbd-state-recovery.md index 70abde3..d083e25 100644 --- a/tests/scenarios/wave2-05-drbd-state-recovery.md +++ b/tests/scenarios/wave2-05-drbd-state-recovery.md @@ -34,13 +34,15 @@ PATCH `rd ` props → `.res` `net { protocol C; }` on every replica. Hierarc `linstor rd drbd-options --unset-protocol backups` → prop deleted; next adjust returns option to DRBD default. Same `--unset-` syntax for `drbd-options` and `drbd-peer-options`. -### 5.W03 `node-connection drbd-peer-options --ping-timeout` — S +### 5.W03 `node-connection drbd-peer-options --ping-timeout` — S ✓ - **Priority:** P2 **Target:** unit + e2e **Complexity:** L - **Source:** UG9 §"Setting DRBD options for node connections" (lines 3386-3398) via tests/scenarios/day2-drbd-peer-options-node-connection.md Per (nodeA, nodeB) pair, applies to every RD's connection between them. Resource-level / RD-level options take precedence. DRBD time encoding is 1/10 of a second (`--ping-timeout 299` = 29.9s). +**Unit:** `pkg/satellite.TestApplyRendersPingTimeoutIntoNetBlock` pins the renderer-side contract — a `DesiredResource.DrbdOptions["DrbdOptions/Net/ping-timeout"]="500"` on a 2-replica RD (n1↔n2) flows through `splitDRBDOptions` into the `net { … }` block of the rendered `.res` as a verbatim `ping-timeout 500;` line, with the `DrbdOptions/Net/` prefix stripped (the form drbdadm parses). The hand-off from the controller-side effective-props resolver — which folds controller-scope, RD-scope, and node-connection-scope keys into one flat bag before dispatch — is scope-agnostic by construction, so one renderer test covers all three set-property paths. + ### 5.W04 `resource-connection drbd-peer-options --max-buffers` — S - **Priority:** P2 **Target:** unit + e2e **Complexity:** L