Skip to content

Commit f68a103

Browse files
kvapsclaude
kvaps
and
claude
committed
docs(architecture): pin Spec/Status discipline + load-bearing invariants (Phase 10.2 step)
New docs/architecture.md captures the rules that, if violated, silently corrupt cluster state — the parts a tour of the code won't surface. Sections: * Spec/Status discipline + per-field cheatsheet (which half each typed Resource / Node field lives on, and why). * Multi-writer Status pattern (controller writes allocator outputs; satellite writes observed state) → server-side apply with distinct field managers. * Hierarchy resolver — Controller→RG→RD→Resource override chain with nil-vs-set pointer discipline. * Wire (golinstor-shaped Props) vs CRD (typed DRBDOptions + ExtraProps) format boundary at the k8s store transcoder. * DRBD initial-sync skip pipeline as a worked example of the whole stack respecting the rules end-to-end. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent a4eb129 commit f68a103

2 files changed

Lines changed: 167 additions & 1 deletion

File tree

‎PLAN.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ and per-object configuration lives in typed Spec / Status fields.
535535

536536
- [x] Audit every site where observed state currently writes to `Spec.Props` (2026-05-10). `applyObserved` now sets `state.DrbdState` directly on the typed `apiv1.ResourceState`; the k8s store routes the whole write through `.Status().Update()` to `Resource.Status.DrbdState`. SetState's legacy `drbdProps map[string]string` parameter is gone. No production Spec.Props side-writes remain on the observation path.
537537
- [ ] Add `ResourceVolumeStatus.CurrentGi string` (also covered by Phase 8.1 follow-up) and `ResourceVolumeStatus.HistoryGi []string` if split-brain visibility matters for the UI.
538-
- [ ] Document the Spec/Status split rule in `docs/architecture.md`: anything observed by the satellite from the kernel goes to Status, never Spec; anything the operator or controller asks the satellite to do goes to Spec, never Status.
538+
- [x] Document the Spec/Status split rule in `docs/architecture.md` (2026-05-10). Includes the cheatsheet table per typed field, the multi-writer Status / SSA story, the hierarchy-resolver nil-vs-set discipline, the wire-vs-CRD format boundary, and the DRBD initial-sync skip pipeline as a worked example of the rule.
539539
- [ ] Use Kubernetes server-side-apply field managers when both controller (writes parts of Status — e.g. allocator outputs `DRBDPort`) and satellite (writes other parts of Status — `DiskState`, `CurrentGi`) touch the same Status, so neither clobbers the other. Today the controller uses regular `Update` which can clobber Status writes that landed between Get and Update.
540540

541541
### 10.3 — Typed fields replace `Spec.Props map[string]string`

‎docs/architecture.md‎

Lines changed: 166 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,166 @@
1+
# Blockstor architecture
2+
3+
This document captures load-bearing invariants the codebase relies
4+
on. It is NOT a tour of the code — for that, follow the call graph
5+
from `cmd/main.go` (controller) and `cmd/satellite/main.go`
6+
(satellite). The pieces below are the rules whose violation would
7+
quietly corrupt cluster state.
8+
9+
## Spec / Status discipline
10+
11+
A blockstor CRD has two halves:
12+
13+
* **Spec** is _desired state_. Operators, REST handlers, and the
14+
controller's own placement logic write here. The satellite
15+
**never writes Spec**.
16+
* **Status** is _observed state_. The satellite (and the
17+
controller's allocators that derive values from Spec) write here.
18+
The user-facing REST API never writes Status — it returns it
19+
read-only.
20+
21+
The rule:
22+
23+
> Anything the satellite reads from the kernel (`drbdsetup events2`,
24+
> `drbdadm status`, `lvs`, …) lives on **Status**. Anything the
25+
> operator or controller asks the satellite to do lives on **Spec**.
26+
27+
A naive whole-object `Update` is unsafe whenever both halves are
28+
written by different actors — a Spec mutation in flight would
29+
clobber a concurrent Status write and vice versa. Status writes
30+
**must** go through the Status subresource (`Status().Update()`);
31+
Spec writes use the regular `Update()` path.
32+
33+
This rule is enforced by code review for now; once Phase 10.2
34+
lands the satellite-side reconciler entirely (its writes will
35+
naturally route through Status only), it becomes mechanical.
36+
37+
### Field placement cheatsheet
38+
39+
| Field | Half | Rationale |
40+
|---|---|---|
41+
| `Resource.Spec.NodeName` | Spec | Operator-chosen placement target. |
42+
| `Resource.Spec.Flags` | Spec | DISKLESS / TIE_BREAKER are operator-controlled. |
43+
| `Resource.Spec.StoragePool` | Spec | Allocator output written by controller. |
44+
| `Resource.Spec.Volumes[i].SeedFromGi` | Spec | Controller-picked DRBD GI to stamp on first activation. |
45+
| `Resource.Status.InUse` | Status | Reflects `drbdsetup status` role. |
46+
| `Resource.Status.DrbdState` | Status | Reflects `events2` connection state. |
47+
| `Resource.Status.DRBDPort` / `DRBDMinor` / `DRBDNodeID` | Status | Allocator-derived; immutable per replica once set. |
48+
| `Resource.Status.Volumes[i].DiskState` | Status | Per-volume kernel state. |
49+
| `Resource.Status.Volumes[i].CurrentGi` | Status | DRBD generation identifier observed by the satellite. |
50+
| `Node.Status.ConnectionStatus` | Status | Set on Hello — reflects whether the satellite has dialled in. |
51+
52+
A field that fits neither half (rare — typically a transient
53+
debounce hint) lives in an annotation, not Spec or Status.
54+
55+
### Multi-writer Status (server-side apply)
56+
57+
Some Status fields are written by **both** the controller (e.g.
58+
allocator output → `DRBDPort`) and the satellite (e.g.
59+
`DiskState`, `CurrentGi`). A regular `.Status().Update()` from
60+
either side rewrites the **whole** Status subresource, which can
61+
clobber the other side's writes that landed between Get and
62+
Update.
63+
64+
Phase 10.2 routes those writes through Kubernetes Server-Side
65+
Apply with distinct field managers (`blockstor-controller`,
66+
`blockstor-satellite`) so each side only touches the fields it
67+
owns. See `pkg/store/k8s/resources.go` `SetState` for the
68+
satellite-side writer; the controller-side writer lives in
69+
`internal/controller`.
70+
71+
## Hierarchy resolver
72+
73+
DRBD configuration follows an upstream-LINSTOR-shaped override
74+
chain:
75+
76+
```
77+
Controller → ResourceGroup → ResourceDefinition → Resource
78+
(broadest) (narrowest)
79+
```
80+
81+
Lower scopes override higher scopes per non-nil field. The typed
82+
implementation lives in `pkg/drbd/typed_resolver.go`
83+
(`ResolveDRBDOptions`); the legacy string-keyed implementation
84+
(`ResolveOptions`) is still used as a fallback for any
85+
`Spec.Props` data not yet migrated to the typed `DRBDOptions`
86+
struct. See `internal/controller/resource_controller.go`'s
87+
`resolveEffectiveProps` for the merge.
88+
89+
`*int32` and `*bool` use nil-vs-set discipline:
90+
91+
* `nil` means "not overridden at this scope, inherit from parent".
92+
* Any non-nil value (including the zero value) means "explicitly
93+
set, do not inherit".
94+
95+
A regression that did `if *src.X { out.X = src.X }` would silently
96+
drop explicit-`false` overrides, e.g. an RD that intentionally
97+
sets `AllowTwoPrimaries=false` would inherit a parent RG's `true`.
98+
The pinning tests for this are in `pkg/drbd/typed_resolver_test.go`.
99+
100+
## Wire format vs CRD storage
101+
102+
Two shapes coexist in the codebase:
103+
104+
1. **Wire shape** — `pkg/api/v1` types, identical to upstream
105+
LINSTOR's REST API. golinstor and external callers see this
106+
verbatim. Property bags live as `props map[string]string`.
107+
2. **CRD shape** — `api/v1alpha1` types, the typed structures
108+
blockstor persists in Kubernetes. DRBD configuration lives in
109+
`Spec.DRBDOptions` (typed) + `Spec.ExtraProps` (forward-compat
110+
for keys we haven't typed yet).
111+
112+
The k8s store (`pkg/store/k8s/`) is the boundary. Its
113+
`drbd_transcode.go` parses the wire `props` bag into typed CRD
114+
fields on Create/Update; the inverse direction re-emits typed
115+
fields back into `props` on GET so golinstor sees the unchanged
116+
shape. Unknown DrbdOptions/* keys round-trip through ExtraProps
117+
without loss.
118+
119+
## DRBD initial-sync skip
120+
121+
Adding a third replica to a 2-replica resource without
122+
intervention would trigger a full resync of the entire backing
123+
device — hours on multi-TiB volumes. The skip pipeline (Phase
124+
8.1):
125+
126+
1. Satellite's `events2` observer parses `current-uuid` from
127+
each device frame and surfaces it in Status as
128+
`Resource.Status.Volumes[i].CurrentGi`.
129+
2. Controller's `ensureSeedFromGi` picks the lowest-named
130+
UpToDate peer's CurrentGi when allocating a new replica and
131+
stamps it on the new replica's `Spec.Volumes[i].SeedFromGi`.
132+
3. Dispatcher threads SeedFromGi through the satellite gRPC
133+
contract (`DesiredVolume.seed_from_gi`).
134+
4. Satellite reconciler's `applyDRBD` runs
135+
136+
```
137+
drbdmeta --force <res>/<vol> v09 <device> internal set-gi <gi>:<gi>:0:0
138+
```
139+
140+
between `drbdadm create-md` and `drbdadm adjust` on first
141+
activation. With matching `current_uuid`+`bitmap_uuid` the
142+
GI handshake on first connect sees the new peer as
143+
already-in-sync and skips the full sync.
144+
145+
The pipeline is end-to-end gated by
146+
`tests/e2e/replica-add-no-resync.sh`.
147+
148+
## Controllers and reconcilers
149+
150+
* `internal/controller.ResourceReconciler` — Resource CRDs.
151+
Allocates DRBD node-id / port / minor; picks SeedFromGi;
152+
promotes DISKLESS to diskful when actively used; dispatches
153+
the desired-state to the satellite via gRPC.
154+
* `internal/controller.ResourceDefinitionReconciler` — RD CRDs.
155+
Auto-creates `DISKLESS+TIE_BREAKER` witnesses when an RD has
156+
even diskful replicas; sets the resource-level quorum policy.
157+
* `internal/controller.ResourceGroupReconciler` /
158+
`NodeReconciler` / `StoragePoolReconciler` /
159+
`SnapshotReconciler` — currently scaffolded but largely no-op
160+
past CRD persistence; reconcile logic lives in the dispatcher
161+
+ satellite.
162+
163+
Phase 10.1 lifts the satellite's gRPC-driven reconciler logic
164+
into `pkg/satellite/controllers/` controller-runtime reconcilers
165+
that watch the apiserver directly; that change retires the
166+
`pkg/dispatcher/` + `pkg/satellitecontroller/` layers entirely.

0 commit comments

Comments
 (0)