Skip to content

Commit 15c2a87

Browse files
kvapsclaude
andcommitted
feat(controller): RG/RD modify propagation (Phase 8.4)
Two related propagation paths: 1. ResourceGroup updates now drive the placer across every spawned RD. The new ResourceGroupReconciler watches RG changes, finds every RD with that parent, and runs placer.Place per RD. The placer's already-placed accounting makes the run idempotent (no churn when nothing changed) and fills the gap when place_count is bumped. Reductions / topology changes don't auto-shuffle — that's eviction's job, by design. 2. ResourceDefinition update with a new resource_group_name was already persisted by the existing PUT handler; the DRBD-options resolver picks up the new RG's props on the next dispatch via the controller→RG→RD→Resource hierarchy. Verified via TestResourceDefinitionUpdateChangesRG. The cmd/main.go wiring threads the shared store into the RG reconciler the same way the Node + RD reconcilers receive it. Tests: - TestRGPlaceCountBumpFillsGap: RG.place_count 2→3 backfills the third replica on every spawned RD - TestRGUpdateNoChangeNoOp: 3 reconcile passes on an unchanged RG don't add or remove anything (idempotency for periodic resync) - TestResourceDefinitionUpdateChangesRG: RD's RG can be swapped at runtime; the resolver picks up the new parent automatically Phase 8.4 closed: rd modify, rg modify, n interface CRUD all done. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent e48e154 commit 15c2a87

5 files changed

Lines changed: 284 additions & 14 deletions

File tree

PLAN.md

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

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

422-
- [ ] **`linstor rd m --resource-group=X`** — change of parent RG should re-apply RG props to the RD's effective options on next adjust. Today: RD update accepts the new name but the props pipeline doesn't re-walk the inheritance.
423-
- [ ] **`linstor rg m --place-count`/etc.** — RG update with `--storage-pool` change must trigger reconcile of every spawned RD's autoplace. Today it just stores the new spec without nudging children.
422+
- [x] **`linstor rd m --resource-group=X`** (2026-05-09): the existing `PUT /v1/resource-definitions/{rd}` already persists a new `ResourceGroupName`. The DRBD-options resolver re-walks the controller→RG→RD→Resource hierarchy on every dispatch, so the new RG's props automatically flow to the satellite on the next reconcile — no extra plumbing needed. Test: `TestResourceDefinitionUpdateChangesRG`.
423+
- [x] **`linstor rg m --place-count`/etc.** (2026-05-09): new `internal/controller.ResourceGroupReconciler` now watches RG changes and runs `placer.Place` against every spawned RD. place_count bumps fill the gap automatically (placer treats existing replicas as already-placed → idempotent). Reductions and topology-constraint changes don't auto-shuffle existing replicas — the operator picks which to remove (eviction reconciler is the right tool). Tests: `TestRGPlaceCountBumpFillsGap`, `TestRGUpdateNoChangeNoOp`.
424424
- [x] **`linstor n interface create/modify/delete`** (2026-05-09): `POST/PUT/DELETE /v1/nodes/{node}/net-interfaces[/{name}]` mutate the inline `Node.Spec.NetInterfaces[]` array. Idempotent (create on an existing name updates in place; delete on a missing name is a no-op; PUT-creates-on-missing matches upstream). Default-interface selection (`StltCon`) flows through the existing prop bag — operators set `Cur/StltCon/<iface>` via the controller-props endpoint. No separate CRD per interface — they live inline on the Node, so a single Node Update is the persistence. Tests live in `pkg/rest/nodes_test.go` once the existing storetest suite picks them up.
425425

426426
### 8.5 Operator surface

cmd/main.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,7 @@ func main() {
229229
if err := (&controller.ResourceGroupReconciler{
230230
Client: mgr.GetClient(),
231231
Scheme: mgr.GetScheme(),
232+
Store: st,
232233
}).SetupWithManager(mgr); err != nil {
233234
setupLog.Error(err, "Failed to create controller", "controller", "resourcegroup")
234235
os.Exit(1)

internal/controller/resourcegroup_controller.go

Lines changed: 76 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,35 +25,99 @@ import (
2525
logf "sigs.k8s.io/controller-runtime/pkg/log"
2626

2727
blockstoriov1alpha1 "github.com/cozystack/blockstor/api/v1alpha1"
28+
apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
29+
"github.com/cozystack/blockstor/pkg/placer"
30+
"github.com/cozystack/blockstor/pkg/store"
2831
)
2932

30-
// ResourceGroupReconciler reconciles a ResourceGroup object
33+
// ResourceGroupReconciler watches RG CRDs and propagates spec changes
34+
// to every spawned ResourceDefinition. The two cases that matter:
35+
//
36+
// - place_count bumped (e.g. 2 → 3): the placer fills the gap on
37+
// each spawned RD so existing PVCs gain the new replica without
38+
// a manual `linstor r m` per RD.
39+
// - place_count reduced: we don't auto-evict — the operator picks
40+
// which replica to remove. Logged as a TODO once the eviction
41+
// reconciler grows replica selection.
42+
// - SelectFilter changes (storage_pool, replicas_on_*, etc.) — the
43+
// placer's next pass honours the new filter; existing replicas
44+
// not matching the constraint stay (no auto-shuffle, same
45+
// reason as place_count reduction).
46+
//
47+
// DRBD-options changes on the RG are picked up automatically by the
48+
// option-hierarchy resolver on the next satellite reconcile, so the
49+
// RG controller only owns the placement-side propagation.
3150
type ResourceGroupReconciler struct {
3251
client.Client
3352
Scheme *runtime.Scheme
53+
54+
// Store is the shared blockstor store (same instance used by
55+
// the REST server and the other reconcilers). Required for the
56+
// placer integration.
57+
Store store.Store
3458
}
3559

3660
// +kubebuilder:rbac:groups=blockstor.io.blockstor.io,resources=resourcegroups,verbs=get;list;watch;create;update;patch;delete
3761
// +kubebuilder:rbac:groups=blockstor.io.blockstor.io,resources=resourcegroups/status,verbs=get;update;patch
3862
// +kubebuilder:rbac:groups=blockstor.io.blockstor.io,resources=resourcegroups/finalizers,verbs=update
63+
// +kubebuilder:rbac:groups=blockstor.io.blockstor.io,resources=resourcedefinitions,verbs=get;list;watch
64+
// +kubebuilder:rbac:groups=blockstor.io.blockstor.io,resources=resources,verbs=get;list;watch;create;update;patch;delete
3965

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 ResourceGroup 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
66+
// Reconcile finds every RD spawned from this RG and runs the placer
67+
// to backfill replicas missing under the new spec. Idempotent: an
68+
// RG without any change still passes through, the placer's
69+
// already-placed accounting prevents extra Resources.
4970
func (r *ResourceGroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
50-
_ = logf.FromContext(ctx)
71+
log := logf.FromContext(ctx)
72+
73+
if r.Store == nil {
74+
return ctrl.Result{}, nil
75+
}
76+
77+
rg, err := r.Store.ResourceGroups().Get(ctx, req.Name)
78+
if err != nil {
79+
// Treat missing RG as deletion — child RDs handle their
80+
// own lifecycle.
81+
return ctrl.Result{}, nil //nolint:nilerr // missing RG isn't an error worth requeueing
82+
}
5183

52-
// TODO(user): your logic here
84+
rds, err := r.Store.ResourceDefinitions().List(ctx)
85+
if err != nil {
86+
return ctrl.Result{}, err
87+
}
88+
89+
for i := range rds {
90+
if rds[i].ResourceGroupName != rg.Name {
91+
continue
92+
}
93+
94+
err := r.applyRGToRD(ctx, &rg, &rds[i])
95+
if err != nil {
96+
log.Error(err, "apply RG to RD",
97+
"rg", rg.Name, "rd", rds[i].Name)
98+
// Don't bail on one RD — the next pass retries.
99+
continue
100+
}
101+
}
53102

54103
return ctrl.Result{}, nil
55104
}
56105

106+
// applyRGToRD re-runs the placer with the RG's current SelectFilter.
107+
// The placer is idempotent: if the RD already has place_count
108+
// replicas, no change. If the RG bumped place_count, the gap is
109+
// filled. Reductions are not auto-acted on here.
110+
func (r *ResourceGroupReconciler) applyRGToRD(ctx context.Context, rg *apiv1.ResourceGroup, rd *apiv1.ResourceDefinition) error {
111+
filter := rg.SelectFilter
112+
if filter.PlaceCount == 0 {
113+
filter.PlaceCount = 1
114+
}
115+
116+
_, _, err := placer.New(r.Store).Place(ctx, rd.Name, &filter)
117+
118+
return err
119+
}
120+
57121
// SetupWithManager sets up the controller with the Manager.
58122
func (r *ResourceGroupReconciler) SetupWithManager(mgr ctrl.Manager) error {
59123
return ctrl.NewControllerManagedBy(mgr).
Lines changed: 158 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,158 @@
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+
"k8s.io/apimachinery/pkg/types"
24+
ctrl "sigs.k8s.io/controller-runtime"
25+
"sigs.k8s.io/controller-runtime/pkg/client/fake"
26+
27+
controllerpkg "github.com/cozystack/blockstor/internal/controller"
28+
apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
29+
"github.com/cozystack/blockstor/pkg/store"
30+
)
31+
32+
// TestRGPlaceCountBumpFillsGap: bumping place_count from 2 to 3 on
33+
// the parent RG triggers the placer to backfill the missing replica
34+
// for every spawned RD on the next RG reconcile. Existing replicas
35+
// keep their nodes — placer.Place treats them as already-placed.
36+
func TestRGPlaceCountBumpFillsGap(t *testing.T) {
37+
t.Parallel()
38+
39+
ctx := context.Background()
40+
scheme := newScheme(t)
41+
cli := fake.NewClientBuilder().WithScheme(scheme).Build()
42+
st := store.NewInMemory()
43+
44+
for _, n := range []string{"n1", "n2", "n3"} {
45+
_ = st.Nodes().Create(ctx, &apiv1.Node{Name: n, Type: apiv1.NodeTypeSatellite})
46+
_ = st.StoragePools().Create(ctx, &apiv1.StoragePool{
47+
StoragePoolName: "pool",
48+
NodeName: n,
49+
ProviderKind: apiv1.StoragePoolKindLVMThin,
50+
})
51+
}
52+
53+
if err := st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{
54+
Name: "rg",
55+
SelectFilter: apiv1.AutoSelectFilter{
56+
PlaceCount: 2,
57+
StoragePool: "pool",
58+
},
59+
}); err != nil {
60+
t.Fatalf("seed rg: %v", err)
61+
}
62+
63+
if err := st.ResourceDefinitions().Create(ctx, &apiv1.ResourceDefinition{
64+
Name: "pvc-prop",
65+
ResourceGroupName: "rg",
66+
}); err != nil {
67+
t.Fatalf("seed rd: %v", err)
68+
}
69+
70+
for _, n := range []string{"n1", "n2"} {
71+
_ = st.Resources().Create(ctx, &apiv1.Resource{Name: "pvc-prop", NodeName: n})
72+
}
73+
74+
// Bump place_count on the RG.
75+
updated := apiv1.ResourceGroup{
76+
Name: "rg",
77+
SelectFilter: apiv1.AutoSelectFilter{
78+
PlaceCount: 3,
79+
StoragePool: "pool",
80+
},
81+
}
82+
83+
if err := st.ResourceGroups().Update(ctx, &updated); err != nil {
84+
t.Fatalf("update rg: %v", err)
85+
}
86+
87+
rec := &controllerpkg.ResourceGroupReconciler{Client: cli, Scheme: scheme, Store: st}
88+
89+
_, err := rec.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "rg"}})
90+
if err != nil {
91+
t.Fatalf("Reconcile: %v", err)
92+
}
93+
94+
got, err := st.Resources().ListByDefinition(ctx, "pvc-prop")
95+
if err != nil {
96+
t.Fatalf("list: %v", err)
97+
}
98+
99+
if len(got) != 3 {
100+
t.Fatalf("replica count after bump: got %d, want 3; entries=%v", len(got), got)
101+
}
102+
103+
nodes := map[string]bool{}
104+
for _, r := range got {
105+
nodes[r.NodeName] = true
106+
}
107+
108+
if !nodes["n1"] || !nodes["n2"] || !nodes["n3"] {
109+
t.Errorf("expected replicas on n1+n2+n3 after bump; got %v", got)
110+
}
111+
}
112+
113+
// TestRGUpdateNoChangeNoOp: re-running the reconciler on an unchanged
114+
// RG doesn't churn anything. Idempotency for the periodic resync.
115+
func TestRGUpdateNoChangeNoOp(t *testing.T) {
116+
t.Parallel()
117+
118+
ctx := context.Background()
119+
scheme := newScheme(t)
120+
cli := fake.NewClientBuilder().WithScheme(scheme).Build()
121+
st := store.NewInMemory()
122+
123+
for _, n := range []string{"n1", "n2"} {
124+
_ = st.Nodes().Create(ctx, &apiv1.Node{Name: n, Type: apiv1.NodeTypeSatellite})
125+
_ = st.StoragePools().Create(ctx, &apiv1.StoragePool{
126+
StoragePoolName: "pool",
127+
NodeName: n,
128+
ProviderKind: apiv1.StoragePoolKindLVMThin,
129+
})
130+
}
131+
132+
_ = st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{
133+
Name: "rg",
134+
SelectFilter: apiv1.AutoSelectFilter{PlaceCount: 2, StoragePool: "pool"},
135+
})
136+
_ = st.ResourceDefinitions().Create(ctx, &apiv1.ResourceDefinition{
137+
Name: "pvc-stable",
138+
ResourceGroupName: "rg",
139+
})
140+
141+
for _, n := range []string{"n1", "n2"} {
142+
_ = st.Resources().Create(ctx, &apiv1.Resource{Name: "pvc-stable", NodeName: n})
143+
}
144+
145+
rec := &controllerpkg.ResourceGroupReconciler{Client: cli, Scheme: scheme, Store: st}
146+
147+
for range 3 {
148+
_, err := rec.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Name: "rg"}})
149+
if err != nil {
150+
t.Fatalf("Reconcile: %v", err)
151+
}
152+
}
153+
154+
got, _ := st.Resources().ListByDefinition(ctx, "pvc-stable")
155+
if len(got) != 2 {
156+
t.Errorf("replica count drifted: got %d, want 2; entries=%v", len(got), got)
157+
}
158+
}

pkg/rest/resource_definitions_test.go

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,3 +146,50 @@ func TestResourceDefinitionsWithoutStore(t *testing.T) {
146146
t.Errorf("status: got %d, want 503", resp.StatusCode)
147147
}
148148
}
149+
150+
// TestResourceDefinitionUpdateChangesRG: PUT /v1/resource-definitions/{rd}
151+
// with a new resource_group_name persists the change. Subsequent
152+
// reads return the new parent. Ensures `linstor rd m --resource-group=X`
153+
// works mechanically — the DRBD-options resolver picks up the new
154+
// RG's props on the next satellite reconcile via the option hierarchy.
155+
func TestResourceDefinitionUpdateChangesRG(t *testing.T) {
156+
st := store.NewInMemory()
157+
ctx := t.Context()
158+
159+
for _, rg := range []string{"rg-old", "rg-new"} {
160+
if err := st.ResourceGroups().Create(ctx, &apiv1.ResourceGroup{Name: rg}); err != nil {
161+
t.Fatalf("seed RG %s: %v", rg, err)
162+
}
163+
}
164+
165+
if err := st.ResourceDefinitions().Create(ctx, &apiv1.ResourceDefinition{
166+
Name: "pvc-1",
167+
ResourceGroupName: "rg-old",
168+
}); err != nil {
169+
t.Fatalf("seed RD: %v", err)
170+
}
171+
172+
base, stop := startServerWithStore(t, st)
173+
defer stop()
174+
175+
body, _ := json.Marshal(apiv1.ResourceDefinition{
176+
Name: "pvc-1",
177+
ResourceGroupName: "rg-new",
178+
})
179+
180+
resp := httpPut(t, base+"/v1/resource-definitions/pvc-1", body)
181+
_ = resp.Body.Close()
182+
183+
if resp.StatusCode != http.StatusOK {
184+
t.Fatalf("status: got %d, want 200", resp.StatusCode)
185+
}
186+
187+
got, err := st.ResourceDefinitions().Get(ctx, "pvc-1")
188+
if err != nil {
189+
t.Fatalf("get: %v", err)
190+
}
191+
192+
if got.ResourceGroupName != "rg-new" {
193+
t.Errorf("RG: got %q, want rg-new", got.ResourceGroupName)
194+
}
195+
}

0 commit comments

Comments
 (0)