fix(store): preserve per-volume DRBDMinor across VD-scoped REST modify (Bug 433)

[Andrei Kvapil (kvaps)]

What

A legal, in-bounds VD-scoped modify — vd set-size (grow) or vd set-property — silently dropped the per-volume DRBDMinor (the /dev/drbd<N> device identity) from RD.Spec.VolumeDefinitions[].

Both VD-scoped store write paths (volumeDefinitions.Update and PatchVolumeDefinitionSpec) rebuild the inline entry through wireToCRDVD, whose wire shape apiv1.VolumeDefinition carries no DRBDMinor — so the round-trip zeroed it. In isolation the controller re-allocates the same minor (self-heals), but once a lower minor is freed by routine RD churn, the allocate-if-nil pass hands the resized volume that freed lower minor instead. The result is a permanent device-identity change on a live volume, driven purely by a resize / property edit — violating the CRD's own contract that a non-nil minor "is NEVER overwritten … preserved across a REST modify".

This is the same wire-rebuild-drops-operator-only-field class as the carry-across family (Bug 206 / 208 / 209). The RD-scoped path preserves the whole VolumeDefinitions slice wholesale (so the minor rode along for free); only the VD-scoped element rebuild dropped it.

Fix

Route both write-backs through a new wireToCRDVDPreserving, which carries the operator-only DRBDMinor across the rebuild.

ResourceDefinitionVolume is now registered in the typed-field carry-across tripwire — it was wrongly documented as having no operator-only fields, the gap that let this ship — so a future operator-only field on the VD element is a build-stop failure unless it is explicitly classified.

Tests (fail-on-bug, proven both directions)

• L1 store unit (pkg/store/k8s/drbd_minor_carry_bug433_test.go): the minor survives resize / props / Update. FAILs on the pre-fix tree (minor comes back nil), PASSes with the carry-across.
• Integration / envtest (tests/integration/group_blue_drbd_minor_bug433_test.go): the full REST → store → controller path. Pre-fix the minor diverges 20001 → 20000 (the freed lower minor); with the fix it holds at 20001.
• L6 cli-matrix + L7 replay (vd-modify-preserves-drbd-minor.{sh,yaml} + a new drbd_minor replay await kind): the operator-CLI codification per the CLI-bug-fix protocol. On-stand execution is deferred — the DRBD stand is unavailable this session — so these are authored but not run here; the store unit and the envtest integration are the deterministic proof.

Each regression was verified to FAIL on the pre-fix tree and PASS with the fix.

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 430f677d-2c41-4d04-a9b2-4380d5fda4ca

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch blue/round5-vd-modify-drops-drbd-minor

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gemini-code-assist]

Code Review

This pull request resolves Bug 433 by ensuring that the operator-only DRBDMinor field (the DRBD device identity) is preserved during volume-definition-scoped updates and patches. The fix introduces a wireToCRDVDPreserving helper function to carry the minor across the wire rebuild, and adds extensive unit, integration, and end-to-end replay tests to prevent regression. The review feedback suggests adding a defensive nil check for the prev pointer in the new helper function to prevent potential nil pointer dereferences.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

To enforce defensive programming and prevent potential nil pointer dereferences in the future, it is safer to add a nil check for the prev pointer before accessing its fields. While the current callers pass a non-nil pointer, safeguarding this helper function ensures robustness against future refactoring or reuse.

Suggested change

	func wireToCRDVDPreserving(prev *crdv1alpha1.ResourceDefinitionVolume, wire *apiv1.VolumeDefinition) crdv1alpha1.ResourceDefinitionVolume {
	out := wireToCRDVD(wire)
	out.DRBDMinor = prev.DRBDMinor
	return out
	}
	func wireToCRDVDPreserving(prev *crdv1alpha1.ResourceDefinitionVolume, wire *apiv1.VolumeDefinition) crdv1alpha1.ResourceDefinitionVolume {
	out := wireToCRDVD(wire)
	if prev != nil {
	out.DRBDMinor = prev.DRBDMinor
	}
	return out
	}