Skip to content

Commit 63442c8

Browse files
kvapsclaude
andcommitted
feat(controller): node evacuate/lost actually migrates replicas (Phase 8.3)
Eviction was flag-only: the REST endpoint marked EVICTED on the Node CRD and that was it. No migration. A drain-this-node operation followed by a maintenance reboot would silently corrupt the PVC's redundancy until somebody noticed. Implemented: - pkg/placer: shared autoplacer extracted from pkg/rest/autoplace.go so both the REST handler and the eviction reconciler use the same data path. Existing-replicas count now excludes EVICTED/LOST hosts so a 2-replica RD with one evicted source actually triggers the add-replica path. - internal/controller.NodeReconciler: watches Node CRDs, on EVICTED enumerates Resources on that node and calls placer.Place per RD to create a replacement on a healthy peer. EVICTED leaves the source intact (the operator finalises after replacement is UpToDate); LOST also deletes the source so the cluster heals without operator action. - cmd/main.go: store init moved before reconciler wiring so NodeReconciler can share the store with REST. Tests: - TestNodeReconciler_EvictedTriggersMigration: 3-node cluster, evict one of two replica hosts, assert a third replica lands on the remaining healthy node. - TestNodeReconciler_LostDeletesSourceResource: source Resource CRD goes away when the node is also flagged LOST. Phase 8.3 first two items closed. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent fa07a46 commit 63442c8

6 files changed

Lines changed: 780 additions & 411 deletions

File tree

PLAN.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,8 @@ The phases above closed the MVP slice and the csi-sanity REST contract. A deep a
410410

411411
### 8.3 Replica lifecycle
412412

413-
- [ ] **`linstor node evacuate` actually migrates replicas** (not just sets the flag). Eviction must enumerate Resources on the node, autoplace diskful copies elsewhere first, then drop the local replica. Today: flag-only, no migration. e2e: 3-node cluster, one PVC, evacuate one node, verify replica reappears on the third.
414-
- [ ] **`linstor node lost` recovery**. Same migration logic as evacuate but assumes the node never returns — should free its TCP-port/node-id allocations. Tested by tearing the satellite pod down hard and asserting the cluster heals without operator intervention.
413+
- [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.
414+
- [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.
416416
- [ ] **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.
417417
- [ ] **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.

cmd/main.go

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -194,9 +194,27 @@ func main() {
194194
os.Exit(1)
195195
}
196196

197+
// Construct the store before reconciler wiring so the
198+
// NodeReconciler can drive eviction-triggered migration via the
199+
// shared placer. Same instance the REST server uses below.
200+
var st store.Store
201+
202+
switch storeKind {
203+
case "k8s":
204+
st = storek8s.New(mgr.GetClient())
205+
case "memory":
206+
st = store.NewInMemory()
207+
208+
setupLog.Info("Using in-memory store; data will be lost on restart")
209+
default:
210+
setupLog.Error(nil, "Unknown --store value", "store", storeKind)
211+
os.Exit(1)
212+
}
213+
197214
if err := (&controller.NodeReconciler{
198215
Client: mgr.GetClient(),
199216
Scheme: mgr.GetScheme(),
217+
Store: st,
200218
}).SetupWithManager(mgr); err != nil {
201219
setupLog.Error(err, "Failed to create controller", "controller", "node")
202220
os.Exit(1)
@@ -249,23 +267,6 @@ func main() {
249267
os.Exit(1)
250268
}
251269

252-
// REST persistence: Kubernetes CRDs (default) or in-process map (tests).
253-
// Both implementations satisfy pkg/store.Store; the same test suite (in
254-
// pkg/store/storetest) exercises both, so behaviour cannot diverge.
255-
var st store.Store
256-
257-
switch storeKind {
258-
case "k8s":
259-
st = storek8s.New(mgr.GetClient())
260-
case "memory":
261-
st = store.NewInMemory()
262-
263-
setupLog.Info("Using in-memory store; data will be lost on restart")
264-
default:
265-
setupLog.Error(nil, "Unknown --store value", "store", storeKind)
266-
os.Exit(1)
267-
}
268-
269270
if err := mgr.Add(&rest.Server{Addr: restAddr, Store: st}); err != nil {
270271
setupLog.Error(err, "Failed to register REST API server")
271272
os.Exit(1)
Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
/*
2+
Copyright 2026 Cozystack contributors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package controller_test
18+
19+
import (
20+
"context"
21+
"testing"
22+
23+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
24+
"k8s.io/apimachinery/pkg/types"
25+
ctrl "sigs.k8s.io/controller-runtime"
26+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
27+
28+
blockstoriov1alpha1 "github.com/cozystack/blockstor/api/v1alpha1"
29+
controllerpkg "github.com/cozystack/blockstor/internal/controller"
30+
apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
31+
"github.com/cozystack/blockstor/pkg/store"
32+
)
33+
34+
// TestNodeReconciler_EvictedTriggersMigration: when a Node is flagged
35+
// EVICTED, every Resource on it gets a replacement created on a
36+
// non-evicted node. Existing peer count + new replica must match the
37+
// RG's place_count.
38+
func TestNodeReconciler_EvictedTriggersMigration(t *testing.T) {
39+
t.Parallel()
40+
41+
ctx := context.Background()
42+
scheme := newScheme(t)
43+
cli := fake.NewClientBuilder().WithScheme(scheme).Build()
44+
45+
st := store.NewInMemory()
46+
47+
// Two storage pools on three nodes; n1 is the evicted one.
48+
for _, name := range []string{"n1", "n2", "n3"} {
49+
if err := st.Nodes().Create(ctx, &apiv1.Node{Name: name, Type: apiv1.NodeTypeSatellite}); err != nil {
50+
t.Fatalf("seed node: %v", err)
51+
}
52+
53+
if err := st.StoragePools().Create(ctx, &apiv1.StoragePool{
54+
StoragePoolName: "pool",
55+
NodeName: name,
56+
ProviderKind: apiv1.StoragePoolKindLVMThin,
57+
}); err != nil {
58+
t.Fatalf("seed pool: %v", err)
59+
}
60+
}
61+
62+
// RG with place_count=2.
63+
if err := st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{
64+
Name: "rg",
65+
SelectFilter: apiv1.AutoSelectFilter{
66+
PlaceCount: 2,
67+
StoragePool: "pool",
68+
},
69+
}); err != nil {
70+
t.Fatalf("seed RG: %v", err)
71+
}
72+
73+
if err := st.ResourceDefinitions().Create(ctx, &apiv1.ResourceDefinition{
74+
Name: "pvc-1",
75+
ResourceGroupName: "rg",
76+
}); err != nil {
77+
t.Fatalf("seed RD: %v", err)
78+
}
79+
80+
// Two replicas exist: n1 + n2. We're going to evict n1.
81+
for _, node := range []string{"n1", "n2"} {
82+
if err := st.Resources().Create(ctx, &apiv1.Resource{Name: "pvc-1", NodeName: node}); err != nil {
83+
t.Fatalf("seed resource: %v", err)
84+
}
85+
}
86+
87+
// Mark n1 evicted via the store + Node CRD on the fake client.
88+
n1 := &apiv1.Node{
89+
Name: "n1",
90+
Type: apiv1.NodeTypeSatellite,
91+
Flags: []string{apiv1.NodeFlagEvicted},
92+
}
93+
94+
if err := st.Nodes().Update(ctx, n1); err != nil {
95+
t.Fatalf("flag n1 evicted: %v", err)
96+
}
97+
98+
if err := cli.Create(ctx, &blockstoriov1alpha1.Node{
99+
ObjectMeta: metav1.ObjectMeta{Name: "n1"},
100+
Spec: blockstoriov1alpha1.NodeSpec{
101+
Type: apiv1.NodeTypeSatellite,
102+
Flags: []string{apiv1.NodeFlagEvicted},
103+
},
104+
}); err != nil {
105+
t.Fatalf("create n1 CRD: %v", err)
106+
}
107+
108+
rec := &controllerpkg.NodeReconciler{Client: cli, Scheme: scheme, Store: st}
109+
110+
_, err := rec.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "n1"}})
111+
if err != nil {
112+
t.Fatalf("Reconcile: %v", err)
113+
}
114+
115+
got, err := st.Resources().ListByDefinition(ctx, "pvc-1")
116+
if err != nil {
117+
t.Fatalf("list: %v", err)
118+
}
119+
120+
// EVICTED is the soft "drain me" hint: the migration adds a
121+
// replacement on a healthy node but leaves the source replica
122+
// in place — the operator decides when to actually remove it
123+
// (typically once the new replica is UpToDate). For LOST, the
124+
// source is deleted in the same reconcile pass.
125+
//
126+
// So after one EVICTED reconcile we expect 3 replicas: original
127+
// n1 + n2 + the freshly placed replacement on n3.
128+
if len(got) != 3 {
129+
t.Fatalf("replica count: got %d, want 3; entries=%v", len(got), got)
130+
}
131+
132+
gotNodes := map[string]bool{}
133+
for _, r := range got {
134+
gotNodes[r.NodeName] = true
135+
}
136+
137+
for _, want := range []string{"n1", "n2", "n3"} {
138+
if !gotNodes[want] {
139+
t.Errorf("expected replica on %s after migration; got %v", want, got)
140+
}
141+
}
142+
}
143+
144+
// TestNodeReconciler_LostDeletesSourceResource: when a Node is flagged
145+
// LOST, the Resource on it is deleted via the K8s API in addition to
146+
// the migration trigger.
147+
func TestNodeReconciler_LostDeletesSourceResource(t *testing.T) {
148+
t.Parallel()
149+
150+
ctx := context.Background()
151+
scheme := newScheme(t)
152+
153+
resCRD := &blockstoriov1alpha1.Resource{
154+
ObjectMeta: metav1.ObjectMeta{Name: "pvc-1.n1"},
155+
Spec: blockstoriov1alpha1.ResourceSpec{
156+
ResourceDefinitionName: "pvc-1",
157+
NodeName: "n1",
158+
},
159+
}
160+
161+
cli := fake.NewClientBuilder().WithScheme(scheme).
162+
WithObjects(
163+
&blockstoriov1alpha1.Node{
164+
ObjectMeta: metav1.ObjectMeta{Name: "n1"},
165+
Spec: blockstoriov1alpha1.NodeSpec{
166+
Type: apiv1.NodeTypeSatellite,
167+
Flags: []string{apiv1.NodeFlagEvicted, apiv1.NodeFlagLost},
168+
},
169+
},
170+
resCRD,
171+
).
172+
Build()
173+
174+
st := store.NewInMemory()
175+
176+
for _, name := range []string{"n1", "n2"} {
177+
_ = st.Nodes().Create(ctx, &apiv1.Node{Name: name, Type: apiv1.NodeTypeSatellite})
178+
_ = st.StoragePools().Create(ctx, &apiv1.StoragePool{
179+
StoragePoolName: "pool",
180+
NodeName: name,
181+
ProviderKind: apiv1.StoragePoolKindLVMThin,
182+
})
183+
}
184+
185+
_ = st.Nodes().Update(ctx, &apiv1.Node{
186+
Name: "n1",
187+
Type: apiv1.NodeTypeSatellite,
188+
Flags: []string{apiv1.NodeFlagEvicted, apiv1.NodeFlagLost},
189+
})
190+
_ = st.ResourceDefinitions().Create(ctx, &apiv1.ResourceDefinition{Name: "pvc-1"})
191+
_ = st.Resources().Create(ctx, &apiv1.Resource{Name: "pvc-1", NodeName: "n1"})
192+
193+
rec := &controllerpkg.NodeReconciler{Client: cli, Scheme: scheme, Store: st}
194+
195+
_, err := rec.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "n1"}})
196+
if err != nil {
197+
t.Fatalf("Reconcile: %v", err)
198+
}
199+
200+
// The Resource CRD on the lost node must be gone (or have
201+
// DeletionTimestamp set if it had a finalizer).
202+
got := &blockstoriov1alpha1.Resource{}
203+
204+
err = cli.Get(ctx, types.NamespacedName{Name: "pvc-1.n1"}, got)
205+
if err == nil && got.DeletionTimestamp.IsZero() {
206+
t.Errorf("expected Resource on LOST node to be deleted; still present: %v", got)
207+
}
208+
}

0 commit comments

Comments
 (0)