Skip to content

Commit e48e154

Browse files
kvapsclaude
andcommitted
feat(rest): node net-interface CRUD + correct HA-failover layer plan (Phase 8.4)
Two changes: 1. Per-interface CRUD on /v1/nodes/{node}/net-interfaces[/{name}]. Clusters with separate replication and management networks need to add/remove interfaces on a running Node without a full PUT-of-the-whole-Node round-trip. Idempotent (create on existing name updates in place; delete on missing is no-op; PUT-creates-on- missing matches upstream LINSTOR's `n interface modify` semantic). Mutates the inline Node.Spec.NetInterfaces[] array — no separate CRD per interface. Tests cover create/append, idempotent replace, path-name-wins on PUT, and idempotent delete. 2. PLAN.md correction for the HA-failover layer. drbd-reactor on its own only handles local systemd promotion — the Kubernetes- aware actions (pod eviction on StandAlone, mount-node migration on disk failure, force-detach coordination) live in a separate piraeus-ha-controller daemon that dials the LINSTOR REST API as a client. Plan now distinguishes the two: drbd-reactor configs are operator-shipped (ConfigMap / Talos extension), and the blockstor-side work is verifying piraeus-ha-controller against our REST surface end-to-end on the dev stand. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 5e2a6fe commit e48e154

4 files changed

Lines changed: 342 additions & 2 deletions

File tree

PLAN.md

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

422422
- [ ] **`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.
423423
- [ ] **`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.
424-
- [ ] **`linstor n interface create/modify/delete`** + setting the default `StltCon` interface. Schema has `NetInterfaces[]`; CRUD endpoints absent. Required for clusters with separate replication and management networks.
424+
- [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
427427

428428
- [x] **`linstor physical-storage` / `create-device-pool`** (2026-05-09): listed as **explicitly out-of-scope** for cozystack — pools are provisioned via Talos extensions / static node config, not at runtime. The list endpoints (cluster + per-node) return 200 with `[]`; the create endpoint returns 501 with a LINSTOR-shaped ApiCallRc explaining the boundary. Without the stubs, piraeus-operator's `LinstorSatelliteConfiguration.spec.storagePools` retry loop would 404 indefinitely. Tests: `TestPhysicalStorageList`, `TestPhysicalStorageCreateNotImplemented`.
429-
- [ ] **DRBD reactor integration**. Cozystack's RWX (Ganesha) and Postgres failover (Patroni) rely on `drbd-reactor`'s promoter+systemd plugins. Ship a default reactor config the satellite renders into `/etc/drbd-reactor.d/<rd>.toml` per spawned RD that needs it; surface promotion state back via a Status condition.
429+
- [ ] **HA failover layer (drbd-reactor + piraeus-ha-controller)**. Two cooperating components, both required for cozystack's RWX (Ganesha) + Postgres failover (Patroni) flows; we conflated them in the original audit. Status:
430+
- `drbd-reactor` — local daemon on each satellite, listens to `drbdsetup events2` and triggers systemd promoter units on Primary-acquired (e.g. `nfs-ganesha@<rd>.service`). Cozystack already ships it via Talos / DaemonSet; the only blockstor-side concern is whether operators want the satellite to render `/etc/drbd-reactor.d/<rd>.toml` per-RD or keep that static. Decision needed; default is to leave reactor config out of the satellite's responsibility (operators ship it via ConfigMap / extension).
431+
- `piraeus-ha-controller` ([upstream](https://github.com/piraeusdatastore/piraeus-ha-controller)) — separate Kubernetes-aware controller. Watches Pod / Node / LINSTOR-resource state and does the cluster-aware actions: pod eviction on DRBD `StandAlone`, force-detach when a backing device fails, mount-node migration when the consumer Pod reschedules. It dials the LINSTOR REST API as a client. Verify against blockstor: end-to-end run with piraeus-ha-controller against our REST surface, document which endpoints it hits, fix any gaps. No code changes from blockstor expected unless an endpoint shape diverges from upstream; the audit needs to happen on the dev stand.
430432
- [x] **`linstor advise`** (2026-05-09): `GET /v1/view/advise/resources` and `GET /v1/resource-definitions/{rd}/advise` return per-RD recommendations (top-N pools by free capacity, sorted desc) without persisting anything. Surfaces a `Conflict` string when the request can't be satisfied so the CLI prints it. Tests: `TestAdviseRD`, `TestAdviseRDInsufficient`.
431433
- [x] **`linstor query-size-info` / spaceinfo** (2026-05-09): `POST /v1/resource-groups/{rg}/query-size-info` answers `max_vlm_size_in_kib = FreeCapacity of the n-th-largest pool` (n = place_count) — the cap that all replicas can fit at once, the value golinstor's pre-flight uses. `POST /v1/query-all-size-info` returns the per-RG map in one shot. EVICTED/LOST nodes excluded from capacity. Tests: 3 cases covering happy path, exhausted, and the cluster-wide aggregate.
432434
- [x] **shared LUN provider (EXOS / SHARED)** (2026-05-09): documented as **explicitly out-of-scope**. cozystack runs DRBD over local block devices; the shared-LUN model (multiple satellites attaching to one SAN-exposed device) is the wrong primitive for the architecture. The provider-kind constants `OPENFLEX_TARGET` and `REMOTE_SPDK` already exist and surface as 501 in the storage-pool create path; no further wiring planned.

pkg/rest/net_interface_test.go

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,186 @@
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 rest
18+
19+
import (
20+
"encoding/json"
21+
"net/http"
22+
"testing"
23+
24+
apiv1 "github.com/cozystack/blockstor/pkg/api/v1"
25+
"github.com/cozystack/blockstor/pkg/store"
26+
)
27+
28+
// TestNetInterfaceCreateAppendsToNode adds a NetInterface to a Node
29+
// without disturbing the rest of the spec. The interface lives
30+
// inline on Node.Spec.NetInterfaces[]; we don't mint a separate CRD.
31+
func TestNetInterfaceCreateAppendsToNode(t *testing.T) {
32+
st := store.NewInMemory()
33+
ctx := t.Context()
34+
35+
if err := st.Nodes().Create(ctx, &apiv1.Node{
36+
Name: "n1",
37+
Type: apiv1.NodeTypeSatellite,
38+
NetInterfaces: []apiv1.NetInterface{
39+
{Name: "default", Address: "10.0.0.1"},
40+
},
41+
}); err != nil {
42+
t.Fatalf("seed node: %v", err)
43+
}
44+
45+
base, stop := startServerWithStore(t, st)
46+
defer stop()
47+
48+
body, _ := json.Marshal(apiv1.NetInterface{
49+
Name: "replication", Address: "10.10.0.1", SatellitePort: 7000,
50+
})
51+
52+
resp := httpPost(t, base+"/v1/nodes/n1/net-interfaces", body)
53+
_ = resp.Body.Close()
54+
55+
if resp.StatusCode != http.StatusOK {
56+
t.Fatalf("status: got %d, want 200", resp.StatusCode)
57+
}
58+
59+
got, err := st.Nodes().Get(ctx, "n1")
60+
if err != nil {
61+
t.Fatalf("get: %v", err)
62+
}
63+
64+
if len(got.NetInterfaces) != 2 {
65+
t.Fatalf("interface count: got %d, want 2", len(got.NetInterfaces))
66+
}
67+
68+
names := map[string]bool{}
69+
for _, n := range got.NetInterfaces {
70+
names[n.Name] = true
71+
}
72+
73+
if !names["default"] || !names["replication"] {
74+
t.Errorf("expected both default + replication; got %v", got.NetInterfaces)
75+
}
76+
}
77+
78+
// TestNetInterfaceCreateReplacesSameName: idempotent — second create
79+
// with the same name overwrites in place rather than duplicating.
80+
func TestNetInterfaceCreateReplacesSameName(t *testing.T) {
81+
st := store.NewInMemory()
82+
ctx := t.Context()
83+
84+
_ = st.Nodes().Create(ctx, &apiv1.Node{
85+
Name: "n1",
86+
Type: apiv1.NodeTypeSatellite,
87+
NetInterfaces: []apiv1.NetInterface{
88+
{Name: "default", Address: "10.0.0.1"},
89+
},
90+
})
91+
92+
base, stop := startServerWithStore(t, st)
93+
defer stop()
94+
95+
body, _ := json.Marshal(apiv1.NetInterface{Name: "default", Address: "192.168.1.5"})
96+
97+
resp := httpPost(t, base+"/v1/nodes/n1/net-interfaces", body)
98+
_ = resp.Body.Close()
99+
100+
got, _ := st.Nodes().Get(ctx, "n1")
101+
if len(got.NetInterfaces) != 1 {
102+
t.Errorf("idempotent create grew the list: %v", got.NetInterfaces)
103+
}
104+
105+
if got.NetInterfaces[0].Address != "192.168.1.5" {
106+
t.Errorf("address not overwritten: %v", got.NetInterfaces[0])
107+
}
108+
}
109+
110+
// TestNetInterfaceUpdatePathNameWins: the URL's {name} is the
111+
// authoritative interface identifier; a name in the body is ignored.
112+
func TestNetInterfaceUpdatePathNameWins(t *testing.T) {
113+
st := store.NewInMemory()
114+
ctx := t.Context()
115+
116+
_ = st.Nodes().Create(ctx, &apiv1.Node{
117+
Name: "n1",
118+
Type: apiv1.NodeTypeSatellite,
119+
NetInterfaces: []apiv1.NetInterface{
120+
{Name: "replication", Address: "10.10.0.1"},
121+
},
122+
})
123+
124+
base, stop := startServerWithStore(t, st)
125+
defer stop()
126+
127+
// Send body with a different name to prove the path wins.
128+
body, _ := json.Marshal(apiv1.NetInterface{Name: "evil", Address: "10.10.0.99"})
129+
130+
resp := httpPut(t, base+"/v1/nodes/n1/net-interfaces/replication", body)
131+
_ = resp.Body.Close()
132+
133+
if resp.StatusCode != http.StatusOK {
134+
t.Fatalf("status: got %d, want 200", resp.StatusCode)
135+
}
136+
137+
got, _ := st.Nodes().Get(ctx, "n1")
138+
if len(got.NetInterfaces) != 1 {
139+
t.Fatalf("count: got %d, want 1; interfaces=%v", len(got.NetInterfaces), got.NetInterfaces)
140+
}
141+
142+
if got.NetInterfaces[0].Name != "replication" {
143+
t.Errorf("name overridden by body: %v", got.NetInterfaces[0])
144+
}
145+
146+
if got.NetInterfaces[0].Address != "10.10.0.99" {
147+
t.Errorf("address not updated: %v", got.NetInterfaces[0])
148+
}
149+
}
150+
151+
// TestNetInterfaceDelete drops the interface; subsequent delete is a
152+
// no-op (idempotent).
153+
func TestNetInterfaceDelete(t *testing.T) {
154+
st := store.NewInMemory()
155+
ctx := t.Context()
156+
157+
_ = st.Nodes().Create(ctx, &apiv1.Node{
158+
Name: "n1",
159+
Type: apiv1.NodeTypeSatellite,
160+
NetInterfaces: []apiv1.NetInterface{
161+
{Name: "default", Address: "10.0.0.1"},
162+
{Name: "replication", Address: "10.10.0.1"},
163+
},
164+
})
165+
166+
base, stop := startServerWithStore(t, st)
167+
defer stop()
168+
169+
for range 2 {
170+
resp := httpDelete(t, base+"/v1/nodes/n1/net-interfaces/replication")
171+
_ = resp.Body.Close()
172+
173+
if resp.StatusCode != http.StatusNoContent {
174+
t.Fatalf("delete status: got %d, want 204", resp.StatusCode)
175+
}
176+
}
177+
178+
got, _ := st.Nodes().Get(ctx, "n1")
179+
if len(got.NetInterfaces) != 1 {
180+
t.Errorf("count: got %d, want 1; interfaces=%v", len(got.NetInterfaces), got.NetInterfaces)
181+
}
182+
183+
if got.NetInterfaces[0].Name != "default" {
184+
t.Errorf("wrong interface survived: %v", got.NetInterfaces[0])
185+
}
186+
}

pkg/rest/nodes.go

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,141 @@ func (s *Server) registerNodes(mux *http.ServeMux) {
3434
mux.HandleFunc("POST /v1/nodes", s.requireStore(s.handleNodeCreate))
3535
mux.HandleFunc("PUT /v1/nodes/{node}", s.requireStore(s.handleNodeUpdate))
3636
mux.HandleFunc("DELETE /v1/nodes/{node}", s.requireStore(s.handleNodeDelete))
37+
38+
// Per-interface CRUD: clusters with separate replication and
39+
// management networks need to add/remove NetInterfaces on a
40+
// running Node without a full PUT-of-the-whole-Node round-trip.
41+
// Maps onto Node.Spec.NetInterfaces[] inside the same Node CRD.
42+
mux.HandleFunc("POST /v1/nodes/{node}/net-interfaces",
43+
s.requireStore(s.handleNetInterfaceCreate))
44+
mux.HandleFunc("PUT /v1/nodes/{node}/net-interfaces/{name}",
45+
s.requireStore(s.handleNetInterfaceUpdate))
46+
mux.HandleFunc("DELETE /v1/nodes/{node}/net-interfaces/{name}",
47+
s.requireStore(s.handleNetInterfaceDelete))
48+
}
49+
50+
// handleNetInterfaceCreate appends a NetInterface to the Node's spec.
51+
// Idempotent: a second create with the same name updates in place.
52+
func (s *Server) handleNetInterfaceCreate(w http.ResponseWriter, r *http.Request) {
53+
mutateNetInterface(w, r, s, func(n *apiv1.Node, iface apiv1.NetInterface) error {
54+
for i := range n.NetInterfaces {
55+
if n.NetInterfaces[i].Name == iface.Name {
56+
n.NetInterfaces[i] = iface
57+
58+
return nil
59+
}
60+
}
61+
62+
n.NetInterfaces = append(n.NetInterfaces, iface)
63+
64+
return nil
65+
})
66+
}
67+
68+
// handleNetInterfaceUpdate is the per-name replace. The path's
69+
// {name} wins over any name in the body so callers can omit it.
70+
func (s *Server) handleNetInterfaceUpdate(w http.ResponseWriter, r *http.Request) {
71+
name := r.PathValue("name")
72+
73+
mutateNetInterface(w, r, s, func(n *apiv1.Node, iface apiv1.NetInterface) error {
74+
iface.Name = name
75+
76+
for i := range n.NetInterfaces {
77+
if n.NetInterfaces[i].Name == name {
78+
n.NetInterfaces[i] = iface
79+
80+
return nil
81+
}
82+
}
83+
84+
// Update on a missing interface is also a create — matches
85+
// upstream LINSTOR's PUT-creates semantic for `linstor n
86+
// interface modify`.
87+
n.NetInterfaces = append(n.NetInterfaces, iface)
88+
89+
return nil
90+
})
91+
}
92+
93+
// handleNetInterfaceDelete drops the named NetInterface. Missing →
94+
// no-op (idempotent).
95+
func (s *Server) handleNetInterfaceDelete(w http.ResponseWriter, r *http.Request) {
96+
nodeName := r.PathValue("node")
97+
name := r.PathValue("name")
98+
99+
node, err := s.Store.Nodes().Get(r.Context(), nodeName)
100+
if err != nil {
101+
writeStoreError(w, err)
102+
103+
return
104+
}
105+
106+
out := node.NetInterfaces[:0]
107+
108+
for i := range node.NetInterfaces {
109+
if node.NetInterfaces[i].Name == name {
110+
continue
111+
}
112+
113+
out = append(out, node.NetInterfaces[i])
114+
}
115+
116+
node.NetInterfaces = out
117+
118+
err = s.Store.Nodes().Update(r.Context(), &node)
119+
if err != nil {
120+
writeStoreError(w, err)
121+
122+
return
123+
}
124+
125+
w.WriteHeader(http.StatusNoContent)
126+
}
127+
128+
// mutateNetInterface decodes a NetInterface body, runs the supplied
129+
// mutation against the node's interface list, and persists. Used by
130+
// both create and update so the decoder + Get + Update plumbing stays
131+
// in one place.
132+
func mutateNetInterface(w http.ResponseWriter, r *http.Request, s *Server, mutate func(*apiv1.Node, apiv1.NetInterface) error) {
133+
nodeName := r.PathValue("node")
134+
135+
var iface apiv1.NetInterface
136+
137+
err := json.NewDecoder(r.Body).Decode(&iface)
138+
if err != nil {
139+
writeError(w, http.StatusBadRequest, err.Error())
140+
141+
return
142+
}
143+
144+
if iface.Name == "" && r.PathValue("name") == "" {
145+
writeError(w, http.StatusBadRequest, "interface name is required")
146+
147+
return
148+
}
149+
150+
node, err := s.Store.Nodes().Get(r.Context(), nodeName)
151+
if err != nil {
152+
writeStoreError(w, err)
153+
154+
return
155+
}
156+
157+
err = mutate(&node, iface)
158+
if err != nil {
159+
writeError(w, http.StatusInternalServerError, err.Error())
160+
161+
return
162+
}
163+
164+
err = s.Store.Nodes().Update(r.Context(), &node)
165+
if err != nil {
166+
writeStoreError(w, err)
167+
168+
return
169+
}
170+
171+
writeJSON(w, http.StatusOK, node)
37172
}
38173

39174
// requireStore guards endpoints that need persistence; it returns 503 if the

pkg/rest/nodes_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,3 +282,20 @@ func httpPatch(t *testing.T, addr string, body []byte) *http.Response {
282282

283283
return resp
284284
}
285+
286+
func httpPut(t *testing.T, addr string, body []byte) *http.Response {
287+
t.Helper()
288+
289+
req, err := http.NewRequestWithContext(t.Context(), http.MethodPut, addr, bytes.NewReader(body))
290+
if err != nil {
291+
t.Fatalf("new request: %v", err)
292+
}
293+
req.Header.Set("Content-Type", "application/json")
294+
295+
resp, err := http.DefaultClient.Do(req)
296+
if err != nil {
297+
t.Fatalf("do: %v", err)
298+
}
299+
300+
return resp
301+
}

0 commit comments

Comments
 (0)