Skip to content

Commit bbb48c0

Browse files
kvapsclaude
andcommitted
feat(controller): tiebreaker, activate/deactivate, diskless-on-remaining (Phase 8.3)
Three more replica-lifecycle pieces: 1. ResourceDefinitionReconciler now maintains the tiebreaker invariant: an RD with exactly 2 diskful replicas in a 3+ node cluster auto-gains a DISKLESS witness on a healthy node not already hosting a replica. Without it, a 1-vs-1 network partition freezes both replicas — neither can tell which side is the majority. The witness votes in the quorum without storing data. Idempotent, EVICTED/LOST-aware, deterministic tiebreaker placement (lowest-name first) so reconcile races converge. 2. Resource activate / deactivate endpoints. POST .../activate clears the INACTIVE flag, .../deactivate sets it. Satellite reconciler reads the flag and runs `drbdadm down` instead of the normal apply path, preserving the .res file + port + node-id allocations so the operator can flip it back without re-syncing. Used by piraeus-operator during node maintenance. 3. AutoSelectFilter.DisklessOnRemaining is wired through (was accepted on the wire but ignored). After diskful place_count is satisfied, every healthy non-replica node gets a DISKLESS replica — the "cluster-wide attachable PVC" pattern. Filter respects EVICTED/LOST same as the diskful path. Tests: - TestTiebreakerCreated: 3-node cluster, 2 diskful → 1 tiebreaker on the third node - TestTiebreakerSkipsThreeReplicas: no tiebreaker when majority already guaranteed - TestTiebreakerSkipsTwoNodeCluster: no spare node → no-op - TestTiebreakerSkipsEvictedNode: never lands on EVICTED - TestResourceDeactivate: flag set is idempotent, activate clears - TestResourceActivateUnknown: 404 on missing replica - TestAutoplaceDisklessOnRemaining: 4 nodes, place_count=2 → 2 diskful + 2 diskless witnesses Phase 8.3 closes (auto-diskful is the lone remaining item — it needs runtime metric tracking that's a separate piece of work). Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 63442c8 commit bbb48c0

11 files changed

Lines changed: 705 additions & 17 deletions

File tree

PLAN.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -413,9 +413,9 @@ The phases above closed the MVP slice and the csi-sanity REST contract. A deep a
413413
- [x] **`linstor node evacuate` actually migrates replicas** (2026-05-09). New `internal/controller.NodeReconciler` watches Node CRDs and on EVICTED enumerates every Resource on the affected node, runs the shared `pkg/placer.Place` to create a replacement on a non-disabled peer (honouring the parent RG's topology constraints), and leaves the source replica in place — the operator decides when to remove it (typically once the replacement is UpToDate). The placer's "existing replicas" count now excludes EVICTED/LOST nodes so a 2-replica RD with one evicted source actually triggers the migration. Test: `TestNodeReconciler_EvictedTriggersMigration` pins the 3-node migration path.
414414
- [x] **`linstor node lost` recovery** (2026-05-09): the same NodeReconciler also detects LOST. Migration runs as for EVICTED, then the source Resource CRD is deleted via the K8s API path so the Resource controller's finalizer cleans up. The TCP-port/node-id allocations stored on the source Resource Status free naturally on delete (the per-node port allocator scans live Resources). Test: `TestNodeReconciler_LostDeletesSourceResource`. **e2e** (hard-kill a satellite pod) sits on the 8.8 checklist.
415415
- [ ] **auto-diskful / auto-diskful-cleanup**. Upstream watches Resource access patterns and auto-promotes a diskless to diskful when a node uses it heavily, reverses on idle. Implement the controller-side metric collection + threshold. Pin via simulation test that drives synthetic access counts.
416-
- [ ] **Tiebreaker auto-creation**. 2-replica RDs in a 3+ node cluster need a 3rd diskless replica for `quorum:majority`. Today: no auto-tiebreaker. Add a controller-side reconciler that maintains exactly-one tiebreaker on a separate failure domain.
417-
- [ ] **Resource activate / deactivate** (`POST /v1/resource-definitions/{rd}/resources/{node}/{activate,deactivate}`). Used by piraeus-operator during node maintenance; today we 404. Implement as drbdadm up/down on the satellite without removing the Resource CRD.
418-
- [ ] **Diskless replicas as first-class autoplace candidates**. `candidatePools` skips DISKLESS pools entirely. Add a `--diskless` flag and the diskless-on-remaining behaviour upstream has, so manual `linstor resource create --diskless` and the autoplace `diskless_on_remaining=true` path both work.
416+
- [x] **Tiebreaker auto-creation** (2026-05-09). New `internal/controller.ResourceDefinitionReconciler` watches RDs; an RD with exactly 2 diskful replicas auto-gains a 3rd DISKLESS replica on a healthy non-replica node so DRBD's `quorum:majority` always has a majority on a 1-vs-1 partition. Idempotent (re-running on a 3-replica or already-witnessed RD is a no-op), respects EVICTED/LOST node flags, picks deterministically (lowest-name first) so reconcile races converge. The witness carries the `TIE_BREAKER` flag for cleanup tracking. Tests: `TestTiebreakerCreated`, `TestTiebreakerSkipsThreeReplicas`, `TestTiebreakerSkipsTwoNodeCluster`, `TestTiebreakerSkipsEvictedNode`.
417+
- [x] **Resource activate / deactivate** (2026-05-09): `POST /v1/resource-definitions/{rd}/resources/{node}/{activate,deactivate}` toggles the `INACTIVE` flag on the Resource. Idempotent. Satellite reconciler reads the flag and runs `drbdadm down` (deactivate) or normal apply (activate) — the .res file, port, and node-id allocations all stay intact, so flipping back doesn't lose state. Tests: `TestResourceDeactivate` (idempotent set + clear), `TestResourceActivateUnknown` (404 on missing replica).
418+
- [x] **Diskless replicas as first-class autoplace candidates** (2026-05-09): `AutoSelectFilter.DisklessOnRemaining` now actually does what the field name promises. After diskful place_count is satisfied, the placer creates DISKLESS replicas on every healthy node not already hosting a replica — the upstream "cluster-wide attachable" pattern useful for consumers that need to mount on any node. Test: `TestAutoplaceDisklessOnRemaining` (4-node cluster, place_count=2 → 2 diskful + 2 diskless witnesses).
419419

420420
### 8.4 Resource-group / definition mutation
421421

cmd/main.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,7 @@ func main() {
236236
if err := (&controller.ResourceDefinitionReconciler{
237237
Client: mgr.GetClient(),
238238
Scheme: mgr.GetScheme(),
239+
Store: st,
239240
}).SetupWithManager(mgr); err != nil {
240241
setupLog.Error(err, "Failed to create controller", "controller", "resourcedefinition")
241242
os.Exit(1)

internal/controller/resourcedefinition_controller.go

Lines changed: 177 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,42 +18,207 @@ package controller
1818

1919
import (
2020
"context"
21+
"errors"
22+
"slices"
23+
"strings"
2124

2225
"k8s.io/apimachinery/pkg/runtime"
2326
ctrl "sigs.k8s.io/controller-runtime"
2427
"sigs.k8s.io/controller-runtime/pkg/client"
2528
logf "sigs.k8s.io/controller-runtime/pkg/log"
2629

2730
blockstoriov1alpha1 "github.com/cozystack/blockstor/api/v1alpha1"
31+
apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
32+
"github.com/cozystack/blockstor/pkg/store"
2833
)
2934

30-
// ResourceDefinitionReconciler reconciles a ResourceDefinition object
35+
// ResourceDefinitionReconciler watches RD CRDs and maintains the
36+
// tiebreaker invariant: an RD with exactly 2 diskful replicas in a
37+
// cluster with 3+ satellite nodes auto-gains a 3rd DISKLESS replica
38+
// on a remaining node so DRBD-9's `quorum: majority` always has a
39+
// majority to compare against on a peer split.
40+
//
41+
// Without the tiebreaker, a 2-replica RD survives a single-node
42+
// failure but freezes on quorum loss in a network partition — the
43+
// surviving replica can't tell whether it's the majority or the
44+
// outvoted minority. The diskless witness fixes that for free
45+
// (no extra storage, just network presence).
3146
type ResourceDefinitionReconciler struct {
3247
client.Client
3348
Scheme *runtime.Scheme
49+
50+
// Store is the shared blockstor store. Same instance the
51+
// NodeReconciler and REST server use.
52+
Store store.Store
3453
}
3554

3655
// +kubebuilder:rbac:groups=blockstor.io.blockstor.io,resources=resourcedefinitions,verbs=get;list;watch;create;update;patch;delete
3756
// +kubebuilder:rbac:groups=blockstor.io.blockstor.io,resources=resourcedefinitions/status,verbs=get;update;patch
3857
// +kubebuilder:rbac:groups=blockstor.io.blockstor.io,resources=resourcedefinitions/finalizers,verbs=update
58+
// +kubebuilder:rbac:groups=blockstor.io.blockstor.io,resources=resources,verbs=get;list;watch;create;update;patch;delete
3959

40-
// Reconcile is part of the main kubernetes reconciliation loop which aims to
41-
// move the current state of the cluster closer to the desired state.
42-
// TODO(user): Modify the Reconcile function to compare the state specified by
43-
// the ResourceDefinition object against the actual cluster state, and then
44-
// perform operations to make the cluster state reflect the state specified by
45-
// the user.
46-
//
47-
// For more details, check Reconcile and its Result here:
48-
// - https://pkg.go.dev/sigs.k8s.io/controller-runtime@v0.23.3/pkg/reconcile
60+
// Reconcile ensures the tiebreaker for a 2-replica RD. Idempotent:
61+
// re-running on an RD that already has its tiebreaker is a no-op.
4962
func (r *ResourceDefinitionReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
50-
_ = logf.FromContext(ctx)
63+
log := logf.FromContext(ctx)
64+
65+
if r.Store == nil {
66+
return ctrl.Result{}, nil
67+
}
68+
69+
var rd blockstoriov1alpha1.ResourceDefinition
5170

52-
// TODO(user): your logic here
71+
err := r.Get(ctx, req.NamespacedName, &rd)
72+
if err != nil {
73+
return ctrl.Result{}, client.IgnoreNotFound(err)
74+
}
75+
76+
if !rd.DeletionTimestamp.IsZero() {
77+
return ctrl.Result{}, nil
78+
}
79+
80+
err = r.ensureTiebreaker(ctx, &rd)
81+
if err != nil {
82+
log.Error(err, "ensure tiebreaker", "rd", rd.Name)
83+
84+
return ctrl.Result{}, err
85+
}
5386

5487
return ctrl.Result{}, nil
5588
}
5689

90+
// ensureTiebreaker creates a DISKLESS replica when the RD's diskful
91+
// replica count is exactly 2 AND there's a satellite node not
92+
// already hosting a replica. The diskless replica is placed on a
93+
// non-disabled node — eviction filtering reuses the disabledNodes
94+
// semantics from the placer.
95+
func (r *ResourceDefinitionReconciler) ensureTiebreaker(ctx context.Context, rd *blockstoriov1alpha1.ResourceDefinition) error {
96+
replicas, err := r.Store.Resources().ListByDefinition(ctx, rd.Name)
97+
if err != nil {
98+
return err
99+
}
100+
101+
diskful, diskless := splitByDiskless(replicas)
102+
103+
// Tiebreaker rules:
104+
// - Need exactly 2 diskful replicas. Fewer means we're still
105+
// converging; more means majority is already guaranteed.
106+
// - No diskless replica already exists.
107+
// - At least one satellite node exists that's not already
108+
// hosting a replica.
109+
if len(diskful) != 2 || len(diskless) > 0 {
110+
return nil
111+
}
112+
113+
hostingReplica := map[string]bool{}
114+
for _, repl := range replicas {
115+
hostingReplica[repl.NodeName] = true
116+
}
117+
118+
tiebreakerNode, err := r.pickTiebreakerNode(ctx, hostingReplica)
119+
if err != nil {
120+
return err
121+
}
122+
123+
if tiebreakerNode == "" {
124+
// No suitable node. The cluster is too small for a
125+
// tiebreaker; leave the RD as-is and the next reconcile
126+
// retries when nodes change.
127+
return nil
128+
}
129+
130+
witness := apiv1.Resource{
131+
Name: rd.Name,
132+
NodeName: tiebreakerNode,
133+
Flags: []string{"DISKLESS", "TIE_BREAKER"},
134+
}
135+
136+
err = r.Store.Resources().Create(ctx, &witness)
137+
if err != nil && !errors.Is(err, store.ErrAlreadyExists) && !alreadyExists(err) {
138+
return err
139+
}
140+
141+
return nil
142+
}
143+
144+
// splitByDiskless partitions replicas into (diskful, diskless) lists.
145+
// DRBD treats DISKLESS replicas as connection-mesh participants only
146+
// — they don't allocate storage but they vote in the quorum.
147+
func splitByDiskless(replicas []apiv1.Resource) ([]apiv1.Resource, []apiv1.Resource) {
148+
var diskful, diskless []apiv1.Resource
149+
150+
for i := range replicas {
151+
if slices.Contains(replicas[i].Flags, "DISKLESS") {
152+
diskless = append(diskless, replicas[i])
153+
} else {
154+
diskful = append(diskful, replicas[i])
155+
}
156+
}
157+
158+
return diskful, diskless
159+
}
160+
161+
// pickTiebreakerNode chooses any healthy satellite that is not
162+
// already hosting a replica of this RD. Picks deterministically
163+
// (lowest name first) so two reconcile races converge on the same
164+
// answer instead of both creating a tiebreaker.
165+
func (r *ResourceDefinitionReconciler) pickTiebreakerNode(ctx context.Context, hostingReplica map[string]bool) (string, error) {
166+
nodes, err := r.Store.Nodes().List(ctx)
167+
if err != nil {
168+
return "", err
169+
}
170+
171+
candidates := make([]string, 0, len(nodes))
172+
173+
for i := range nodes {
174+
if hostingReplica[nodes[i].Name] {
175+
continue
176+
}
177+
178+
if isDisabledNode(&nodes[i]) {
179+
continue
180+
}
181+
182+
if nodes[i].Type != "" && nodes[i].Type != apiv1.NodeTypeSatellite && nodes[i].Type != apiv1.NodeTypeCombined {
183+
continue
184+
}
185+
186+
candidates = append(candidates, nodes[i].Name)
187+
}
188+
189+
if len(candidates) == 0 {
190+
return "", nil
191+
}
192+
193+
slices.Sort(candidates)
194+
195+
return candidates[0], nil
196+
}
197+
198+
// isDisabledNode mirrors placer.disabledNodes for the RD-level
199+
// tiebreaker path so we don't pin an EVICTED/LOST node as the witness.
200+
func isDisabledNode(node *apiv1.Node) bool {
201+
for _, f := range node.Flags {
202+
if f == apiv1.NodeFlagEvicted || f == apiv1.NodeFlagLost {
203+
return true
204+
}
205+
}
206+
207+
return false
208+
}
209+
210+
// alreadyExists is a string-based check for the wrapped errors the
211+
// k8s store returns. The k8s store wraps errAlreadyExists from
212+
// kube-apiserver in a cockroachdb/errors.Wrap — Is() doesn't tunnel
213+
// through that, so we keyword-match on the message.
214+
func alreadyExists(err error) bool {
215+
if err == nil {
216+
return false
217+
}
218+
219+
return strings.Contains(err.Error(), "already exists")
220+
}
221+
57222
// SetupWithManager sets up the controller with the Manager.
58223
func (r *ResourceDefinitionReconciler) SetupWithManager(mgr ctrl.Manager) error {
59224
return ctrl.NewControllerManagedBy(mgr).

0 commit comments

Comments
 (0)