Skip to content

Commit 939318e

Browse files
kvapsclaude
andcommitted
fix(satellite): drop section-less DrbdOptions/* from .res render
piraeus-operator stuffs LINSTOR-controller-only knobs (e.g. DrbdOptions/AutoEvictAllowEviction) under the DrbdOptions/ namespace via /v1/controller/properties. The satellite was treating any DrbdOptions/<key> with no section subpath as a resource-level DRBD option and writing it into the rendered options{} block — which drbdadm rejects with "Parse error: ... but got 'AutoEvictAllowEvicti...'". That parse error makes drbdadm fail on EVERY .res in /etc/drbd.d/, so a single leaky resource takes the whole node down for new RDs. Convention upstream: real DRBD options live at DrbdOptions/<Section>/<Key>; section-less DrbdOptions/<Key> is a LINSTOR-only knob and should not flow to the satellite's .res output. Test: TestApplyDropsLinstorOnlyOptions pins the regression. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 71bf4d3 commit 939318e

2 files changed

Lines changed: 82 additions & 6 deletions

File tree

pkg/satellite/reconciler.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -524,10 +524,18 @@ func buildResFile(dr *satellitepb.DesiredResource, localNode, localAddr string,
524524
// into per-section maps. Per-replica wiring (port/node-id/peer.*.…)
525525
// is dropped — those are not user-tunable knobs. Anything under
526526
// `DrbdOptions/Net/...` lands on the `net { }` block, anything else
527-
// under `DrbdOptions/...` lands on the resource-level `options { }`
528-
// block (drbd's catch-all). The ConfFileBuilder writes the keys
529-
// verbatim with the `DrbdOptions/<Section>/` prefix stripped — that's
530-
// the form drbdadm expects.
527+
// under `DrbdOptions/<Section>/...` lands on the resource-level
528+
// `options { }` block (drbd's catch-all). The ConfFileBuilder writes
529+
// the keys verbatim with the `DrbdOptions/<Section>/` prefix stripped
530+
// — that's the form drbdadm expects.
531+
//
532+
// Section-less keys (`DrbdOptions/<Key>` with nothing after the
533+
// prefix beyond a single segment) are LINSTOR-controller-only props
534+
// — e.g. `DrbdOptions/AutoEvictAllowEviction` is consumed by the
535+
// LINSTOR controller's auto-eviction logic, NOT by DRBD. Writing
536+
// those into the .res file makes drbdadm fail with "expected:
537+
// cpu-mask | on-no-data-accessible | ... but got: <name>". Drop
538+
// them on the satellite side; the convention upstream is the same.
531539
func splitDRBDOptions(opts map[string]string) (map[string]string, map[string]string) {
532540
netOpts := map[string]string{}
533541
resOpts := map[string]string{}
@@ -540,8 +548,8 @@ func splitDRBDOptions(opts map[string]string) (map[string]string, map[string]str
540548

541549
section, rawKey, hasSection := strings.Cut(rest, "/")
542550
if !hasSection {
543-
resOpts[rest] = value
544-
551+
// LINSTOR-only key (no DRBD section subpath); these
552+
// don't belong in the rendered .res. See doc comment.
545553
continue
546554
}
547555

pkg/satellite/reconciler_drbd_test.go

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,3 +314,71 @@ func TestApplyRendersAllowTwoPrimaries(t *testing.T) {
314314
t.Errorf(".res missing allow-two-primaries; body=%s", body)
315315
}
316316
}
317+
318+
// TestApplyDropsLinstorOnlyOptions: section-less DrbdOptions/* keys
319+
// (e.g. DrbdOptions/AutoEvictAllowEviction set by piraeus-operator
320+
// via /v1/controller/properties) must NOT land in the rendered .res
321+
// — they're LINSTOR-controller-only knobs and drbdadm rejects the
322+
// whole file with "Parse error: ... but got 'AutoEvictAllowEviction'"
323+
// on the next `drbdadm primary`. Regression for stand-side smoke
324+
// failure observed 2026-05-09.
325+
func TestApplyDropsLinstorOnlyOptions(t *testing.T) {
326+
dir := t.TempDir()
327+
fx := storage.NewFakeExec()
328+
fx.Expect("lvs --noheadings -o lv_name vg/pvc-noeviction_00000",
329+
storage.FakeResponse{Stdout: []byte("")})
330+
331+
thin := lvm.NewThin(lvm.ThinConfig{VolumeGroup: "vg", ThinPool: "tp"}, fx)
332+
rec := satellite.NewReconciler(satellite.ReconcilerConfig{
333+
Providers: map[string]storage.Provider{"thin1": thin},
334+
Adm: drbd.NewAdm(fx),
335+
StateDir: dir,
336+
NodeName: "n1",
337+
})
338+
339+
_, err := rec.Apply(t.Context(), []*satellitepb.DesiredResource{
340+
{
341+
Name: "pvc-noeviction",
342+
NodeName: "n1",
343+
Volumes: []*satellitepb.DesiredVolume{
344+
{VolumeNumber: 0, SizeKib: 1024 * 1024, StoragePool: "thin1"},
345+
},
346+
DrbdOptions: map[string]string{
347+
"port": "7000", "node-id": "0", "address": "10.0.0.1", "minor": "1000",
348+
349+
// Section-less: must be dropped
350+
"DrbdOptions/AutoEvictAllowEviction": "false",
351+
// Section-less: must be dropped
352+
"DrbdOptions/AutoplaceTarget": "3",
353+
// Real DRBD option: must land in net{} block
354+
"DrbdOptions/Net/rr-conflict": "retry-connect",
355+
// Real DRBD option: must land in options{} block
356+
"DrbdOptions/Resource/on-no-quorum": "suspend-io",
357+
},
358+
},
359+
})
360+
if err != nil {
361+
t.Fatalf("Apply: %v", err)
362+
}
363+
364+
body, err := os.ReadFile(filepath.Join(dir, "pvc-noeviction.res"))
365+
if err != nil {
366+
t.Fatalf("read .res: %v", err)
367+
}
368+
369+
if strings.Contains(string(body), "AutoEvictAllowEviction") {
370+
t.Errorf("LINSTOR-only key leaked into .res; body=%s", body)
371+
}
372+
373+
if strings.Contains(string(body), "AutoplaceTarget") {
374+
t.Errorf("LINSTOR-only key (AutoplaceTarget) leaked into .res; body=%s", body)
375+
}
376+
377+
if !strings.Contains(string(body), "rr-conflict retry-connect;") {
378+
t.Errorf("real DRBD net option missing; body=%s", body)
379+
}
380+
381+
if !strings.Contains(string(body), "on-no-quorum suspend-io;") {
382+
t.Errorf("real DRBD resource option missing; body=%s", body)
383+
}
384+
}

0 commit comments

Comments
 (0)