Skip to content

Commit a4eb129

Browse files
kvapsclaude
andcommitted
feat(k8s-store): fold topology.blockstor.io/* labels into Aux/ props (Phase 10.3)
Operators can now set placement topology on Node CRDs as native Kubernetes labels — `topology.blockstor.io/zone`, `/rack`, etc. — and the k8s store's crdToWireNode folds them back into the wire `Props["Aux/<key>"]` shape so the autoplacer's existing replicas_on_same / replicas_on_different filters keep working without code changes. Precedence rule: an explicit Props value wins on conflict (matches the autoplacer's auxKey() lookup, which reads Props verbatim — the labels path is purely additive). The full Aux/ removal — drop the prop side, switch the placer to client.MatchingLabels — is a follow-up once piraeus-operator + cozystack stop writing the prop on CREATE. Until then the additive path is the safer migration. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent 5aaf411 commit a4eb129

3 files changed

Lines changed: 163 additions & 4 deletions

File tree

PLAN.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -551,7 +551,7 @@ LINSTOR boundary.
551551
- [x] `Node.Spec.DRBDPortRange / DRBDMinorRange { Min, Max int32 }` (2026-05-10). Typed PortRange struct on NodeSpec; ResourceReconciler.nodeRange resolves typed first with legacy "min-max" Props fallback. CRD enum/min validation in place via kubebuilder markers.
552552
- [x] `AutoTieBreaker *bool` (2026-05-10). Lives on `DRBDResourceOptions.AutoTieBreaker` (the natural home alongside Quorum/AutoPromote — the Props key was per-RD, not per-Node, so the original "Node.Spec" plan-line was a misclassification). Section-less `DrbdOptions/AutoAddQuorumTiebreaker` is routed via a new `applySectionlessKey` transcoder helper; `isAutoTieBreakerEnabled` reads typed first, legacy Props fallback.
553553
- [x] `Resource.Spec.StoragePool string` (2026-05-10). Typed field on Resource; dispatcher's buildVolumes + autoplacer dispatch read typed first; auto-diskful promotion stamps both typed + legacy Props key; k8s store transcodes both ways.
554-
- [ ] Topology: drop `Props["Aux/zone"]` / `"Aux/rack"`. Use Kubernetes-native `metadata.labels["topology.blockstor.io/zone"]` etc. Autoplacer reads labels via `client.MatchingLabels`. This is also the upgrade path to label-aware features like `topologySpreadConstraints` later.
554+
- [x] Topology labels (2026-05-10, additive). The k8s store's `crdToWireNode` folds every `topology.blockstor.io/<key>` label on the Node CRD into `Props["Aux/<key>"]` so the autoplacer's existing `replicas_on_same` / `replicas_on_different` filters keep working without code changes. Operators can now set the standard Kubernetes label form on Node CRDs and the placer surfaces it as if it were an Aux/ prop. Pinned via 4 unit tests (label→Aux/ fold, Props wins on conflict, hasTopologyLabels true/false). Full Aux/ removal (drop the prop-side entirely, switch the placer to `client.MatchingLabels`) is a follow-up once piraeus-operator + cozystack callers stop writing the prop on CREATE — until then keeping the legacy path is the safer migration.
555555
- [x] REST shim transcoder (2026-05-10). `pkg/store/k8s/drbd_transcode.go` splits the wire `props` bag into `Spec.DRBDOptions` (recognised keys) + `Spec.ExtraProps` (unknown DrbdOptions/* + non-DRBD residual keys); inverse re-emits typed back to `props` on GET. Pinned by 7 unit tests (per-section recognition, unknown-key fallthrough, parse-error fallback, round-trip losslessness, bool spelling tolerance). Controller's `resolveEffectiveProps` now walks typed via `ResolveDRBDOptions` and flattens via `drbd.TypedDRBDOptionsToProps` for the dispatcher → satellite renders the same .res file from typed CRD fields rather than string-keyed Props.
556556
- [x] Validation via kubebuilder enums (2026-05-10). `Net.Protocol`, `Disk.OnIOError`, `Resource.Quorum`, `Resource.OnNoQuorum`, `Net.AfterSb0Pri/1Pri/2Pri` all carry `+kubebuilder:validation:Enum=...` markers; CRD manifests regenerated. Pinned via `internal/controller/drbd_admission_test.go` — 8 ginkgo specs that submit garbage values against a live envtest apiserver and assert it rejects at admission.
557557

pkg/store/k8s/nodes.go

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,17 +167,25 @@ func (n *nodes) Delete(ctx context.Context, name string) error {
167167
// crdToWireNode flattens a Node CRD into the LINSTOR REST shape.
168168
// Phase 10.3: re-emits typed `Spec.SatelliteEndpoint` back into the
169169
// wire `Props["SatelliteEndpoint"]` so golinstor + the dispatcher's
170-
// fallback path keep seeing the same shape on GET.
170+
// fallback path keep seeing the same shape on GET. Native
171+
// `topology.blockstor.io/<key>` labels on the Node CRD ALSO get
172+
// folded back into `Props["Aux/<key>"]` so the autoplacer's
173+
// existing replicas_on_same / replicas_on_different filters keep
174+
// working unchanged across the labels migration.
171175
func crdToWireNode(crd *crdv1alpha1.Node) apiv1.Node {
172176
props := crd.Spec.Props
173177

174-
if crd.Spec.SatelliteEndpoint != "" {
178+
if crd.Spec.SatelliteEndpoint != "" || hasTopologyLabels(crd.Labels) {
175179
props = maps.Clone(props)
176180
if props == nil {
177181
props = map[string]string{}
178182
}
179183

180-
props["SatelliteEndpoint"] = crd.Spec.SatelliteEndpoint
184+
if crd.Spec.SatelliteEndpoint != "" {
185+
props["SatelliteEndpoint"] = crd.Spec.SatelliteEndpoint
186+
}
187+
188+
foldTopologyLabels(props, crd.Labels)
181189
}
182190

183191
out := apiv1.Node{
@@ -255,3 +263,51 @@ func wireToCRDNodeSpec(in *apiv1.Node) crdv1alpha1.NodeSpec {
255263

256264
return spec
257265
}
266+
267+
// TopologyLabelPrefix is the native Kubernetes label namespace
268+
// blockstor uses for topology placement keys (zone, rack, …).
269+
// Replaces upstream LINSTOR's `Props["Aux/<key>"]` shape so that
270+
// the autoplacer can use `client.MatchingLabels` selectors and the
271+
// keys feed into `topologySpreadConstraints` for free. Phase 10.3.
272+
const TopologyLabelPrefix = "topology.blockstor.io/"
273+
274+
// hasTopologyLabels reports whether any of the Node's metadata
275+
// labels lives under the blockstor topology prefix. Cheap pre-
276+
// check so we only allocate a fresh Props map when we actually
277+
// have labels to fold in.
278+
func hasTopologyLabels(labels map[string]string) bool {
279+
for k := range labels {
280+
if strings.HasPrefix(k, TopologyLabelPrefix) {
281+
return true
282+
}
283+
}
284+
285+
return false
286+
}
287+
288+
// foldTopologyLabels copies every `topology.blockstor.io/<key>`
289+
// label into `props["Aux/<key>"]` so existing readers (autoplacer
290+
// auxKey lookups, golinstor over the wire) see the topology
291+
// information without any changes. Mutates props in place.
292+
//
293+
// The Aux/ Props side stays the source of truth for now — when a
294+
// caller writes via the wire API the legacy key path is what gets
295+
// persisted. The label-side path is purely additive (operators
296+
// who set the native label see it surface as an Aux/ prop on
297+
// GET); future phases will flip the source-of-truth direction
298+
// once linstor-csi etc. learn to set labels directly.
299+
func foldTopologyLabels(props, labels map[string]string) {
300+
for label, value := range labels {
301+
if !strings.HasPrefix(label, TopologyLabelPrefix) {
302+
continue
303+
}
304+
305+
auxKey := "Aux/" + strings.TrimPrefix(label, TopologyLabelPrefix)
306+
// Don't clobber an explicit Props value — the operator may
307+
// have set both and the Props side wins (matches the
308+
// auxKey() lookup precedence in the placer).
309+
if _, exists := props[auxKey]; !exists {
310+
props[auxKey] = value
311+
}
312+
}
313+
}
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
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 k8s
18+
19+
import (
20+
"testing"
21+
)
22+
23+
// TestFoldTopologyLabelsAddsAuxProp pins the migration shim: a Node
24+
// with `topology.blockstor.io/zone=us-east-1a` set as a native
25+
// Kubernetes label MUST surface as `Props["Aux/zone"]` on the wire
26+
// so the autoplacer's existing replicas_on_same / replicas_on_different
27+
// filters keep working without changes. Phase 10.3.
28+
func TestFoldTopologyLabelsAddsAuxProp(t *testing.T) {
29+
t.Parallel()
30+
31+
props := map[string]string{}
32+
labels := map[string]string{
33+
"topology.blockstor.io/zone": "us-east-1a",
34+
"topology.blockstor.io/rack": "r5",
35+
"unrelated": "ignored",
36+
}
37+
38+
foldTopologyLabels(props, labels)
39+
40+
if props["Aux/zone"] != "us-east-1a" {
41+
t.Errorf("Aux/zone: got %q, want us-east-1a", props["Aux/zone"])
42+
}
43+
44+
if props["Aux/rack"] != "r5" {
45+
t.Errorf("Aux/rack: got %q, want r5", props["Aux/rack"])
46+
}
47+
48+
if _, ok := props["unrelated"]; ok {
49+
t.Errorf("unrelated label leaked into props")
50+
}
51+
}
52+
53+
// TestFoldTopologyLabelsPropsWinOnConflict pins the precedence
54+
// rule: when both the Props key and the matching label are set,
55+
// Props wins (matches the autoplacer's auxKey() lookup, which
56+
// reads Props verbatim — the labels path is purely additive).
57+
// A regression that overwrote Props would silently change
58+
// existing placement decisions.
59+
func TestFoldTopologyLabelsPropsWinOnConflict(t *testing.T) {
60+
t.Parallel()
61+
62+
props := map[string]string{
63+
"Aux/zone": "explicit-from-props",
64+
}
65+
labels := map[string]string{
66+
"topology.blockstor.io/zone": "from-label-should-lose",
67+
}
68+
69+
foldTopologyLabels(props, labels)
70+
71+
if props["Aux/zone"] != "explicit-from-props" {
72+
t.Errorf("Aux/zone: got %q, want explicit-from-props (Props overrides labels)", props["Aux/zone"])
73+
}
74+
}
75+
76+
// TestHasTopologyLabelsTrue / False pin the cheap pre-check: skip
77+
// the props-clone allocation entirely when there's nothing to fold
78+
// in. A regression that always allocated would slow every
79+
// crdToWireNode call on clusters with no topology labels at all.
80+
func TestHasTopologyLabelsTrue(t *testing.T) {
81+
t.Parallel()
82+
83+
if !hasTopologyLabels(map[string]string{"topology.blockstor.io/zone": "us-east-1a"}) {
84+
t.Errorf("expected true for prefixed label")
85+
}
86+
}
87+
88+
func TestHasTopologyLabelsFalse(t *testing.T) {
89+
t.Parallel()
90+
91+
cases := []map[string]string{
92+
nil,
93+
{},
94+
{"unrelated": "x"},
95+
{"topology.kubernetes.io/zone": "x"}, // standard k8s label, NOT ours
96+
}
97+
98+
for i, labels := range cases {
99+
if hasTopologyLabels(labels) {
100+
t.Errorf("case %d: expected false for %v", i, labels)
101+
}
102+
}
103+
}

0 commit comments

Comments
 (0)