Skip to content

Commit 01e74fd

Browse files
kvapsclaude
andcommitted
refactor(k8s-store): unify CRD composite-key separator on . (PhysicalDevice)
Other composite-key CRDs (StoragePool, Resource, Snapshot) all name themselves `<key1>.<key2>` and route through `k8s.Name()` for slug discipline. PhysicalDevice was the odd one out — used `-` and a hand-rolled sanitiser. Operators rely on the dot separator for cross-kind grep (`kubectl get … | grep '<node>\.'` works for storagepools + resources + snapshots; PhysicalDevice broke the pattern). Moves PhysicalDeviceCRDName from pkg/satellite/discovery.go to pkg/store/k8s/physicaldevices.go where the analogous functions live, and replaces the hand-rolled slug with the standard `Name(node + "." + stableID)` shape. Pinned by a new test in crdname_test.go covering the pass-through and slugified paths. The PhysicalDevice type doc-comment now points operators at the cross-CRD convention so future readers know not to deviate. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Andrei Kvapil <kvapss@gmail.com>
1 parent d51b069 commit 01e74fd

5 files changed

Lines changed: 46 additions & 76 deletions

File tree

api/v1alpha1/physicaldevice_types.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,10 @@ type PhysicalDevice struct {
180180
metav1.TypeMeta `json:",inline"`
181181

182182
// metadata is a standard object metadata. The expected name
183-
// shape is `<nodeName>-<stable-id-slug>`; the metadata.labels
183+
// shape is `<nodeName>.<stable-id-slug>` (matching every
184+
// other composite-key CRD in the project — Resource,
185+
// StoragePool, Snapshot — so operators can grep across
186+
// kinds with a single `<node>.` pattern); the metadata.labels
184187
// must carry `blockstor.io/node=<nodeName>` so per-node
185188
// filters work via label selectors.
186189
// +optional

pkg/satellite/discovery.go

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -105,47 +105,6 @@ func slugifyForLSStableID(in string) string {
105105
return b.String()
106106
}
107107

108-
// PhysicalDeviceCRDName composes the `metadata.name` for a
109-
// PhysicalDevice CRD: `<nodeName>-<stable-id-slug>`. K8s name
110-
// constraints (RFC 1123 label, lowercase alphanumeric + `-`,
111-
// max 63 chars per segment) drive the further sanitisation:
112-
// underscores become hyphens, every other invalid char is
113-
// dropped, the whole thing is lowercased, and we cap at 63 chars
114-
// after the node-name + separator (truncating the slug if
115-
// needed; collisions are vanishingly unlikely with stable-id
116-
// entropy).
117-
func PhysicalDeviceCRDName(nodeName, stableID string) string {
118-
const (
119-
maxK8sName = 253
120-
stableIDMaxLength = 200
121-
)
122-
123-
slug := strings.ToLower(strings.ReplaceAll(stableID, "_", "-"))
124-
125-
var b strings.Builder
126-
127-
for _, r := range slug {
128-
switch {
129-
case r >= 'a' && r <= 'z',
130-
r >= '0' && r <= '9',
131-
r == '-':
132-
b.WriteRune(r)
133-
}
134-
}
135-
136-
clean := b.String()
137-
if len(clean) > stableIDMaxLength {
138-
clean = clean[:stableIDMaxLength]
139-
}
140-
141-
out := strings.ToLower(nodeName) + "-" + clean
142-
if len(out) > maxK8sName {
143-
out = out[:maxK8sName]
144-
}
145-
146-
return out
147-
}
148-
149108
// DiscoveryAction is one entry in the diff plan the discovery
150109
// loop computes. Pure data — the actual store mutations happen
151110
// in the loop's caller using a `store.PhysicalDeviceStore`.

pkg/satellite/discovery_test.go

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -102,40 +102,6 @@ func TestPickStableIDByPathFallback(t *testing.T) {
102102
}
103103
}
104104

105-
// TestPhysicalDeviceCRDNameSanitises pins the k8s name shape:
106-
// underscores → hyphens, uppercase → lowercase, anything outside
107-
// `[a-z0-9-]` dropped, max 253 chars total.
108-
func TestPhysicalDeviceCRDNameSanitises(t *testing.T) {
109-
t.Parallel()
110-
111-
cases := []struct {
112-
node, stable, want string
113-
}{
114-
{
115-
node: "n1",
116-
stable: "wwn-0x5000c500a3b1c2d3",
117-
want: "n1-wwn-0x5000c500a3b1c2d3",
118-
},
119-
{
120-
node: "N1-Worker",
121-
stable: "scsi-SATA_Samsung_SSD_980_PRO_S6BUNS0R123456",
122-
want: "n1-worker-scsi-sata-samsung-ssd-980-pro-s6buns0r123456",
123-
},
124-
{
125-
node: "n2",
126-
stable: "nvme-INTEL@SSDPF21Q400GB_PHM2!1234",
127-
want: "n2-nvme-intelssdpf21q400gb-phm21234",
128-
},
129-
}
130-
131-
for _, tc := range cases {
132-
got := satellite.PhysicalDeviceCRDName(tc.node, tc.stable)
133-
if got != tc.want {
134-
t.Errorf("%s + %s: got %q, want %q", tc.node, tc.stable, got, tc.want)
135-
}
136-
}
137-
}
138-
139105
// TestDiscoveryDiffCreate pins the new-device case: a device in
140106
// `discovered` that isn't in `existing` becomes an OpCreate.
141107
func TestDiscoveryDiffCreate(t *testing.T) {

pkg/store/k8s/crdname_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,29 @@ func TestSetOriginalName_PassThroughDoesNotAnnotate(t *testing.T) {
117117
t.Errorf("OriginalName = %q, want %q", got, original)
118118
}
119119
}
120+
121+
// TestPhysicalDeviceCRDNameUsesDotSeparator pins the cross-CRD
122+
// naming convention: every composite-key CRD in the project
123+
// (StoragePool, Resource, Snapshot, PhysicalDevice) uses
124+
// `<key1>.<key2>` and routes through `Name()` for slug
125+
// discipline. Operators rely on this — `kubectl get … | grep
126+
// '<node>\.' ` works across kinds. A regression that switched
127+
// PhysicalDevice to a hyphen separator would silently break that
128+
// grep workflow.
129+
func TestPhysicalDeviceCRDNameUsesDotSeparator(t *testing.T) {
130+
t.Parallel()
131+
132+
// RFC1123-clean inputs pass through verbatim with the dot.
133+
got := k8s.PhysicalDeviceCRDName("n1", "wwn-0xabcdef0123456789")
134+
if got != "n1.wwn-0xabcdef0123456789" {
135+
t.Errorf("got %q, want n1.wwn-0xabcdef0123456789", got)
136+
}
137+
138+
// Inputs with characters k8s rejects (uppercase, underscore,
139+
// at-sign) get slugified by Name() — same path other
140+
// composite-key CRDs travel through.
141+
got = k8s.PhysicalDeviceCRDName("N1-Worker", "scsi-SATA_Samsung_SSD_980_PRO_S6BUNS0R")
142+
if !strings.Contains(got, "n1-worker") {
143+
t.Errorf("got %q, want substring n1-worker (slugified node)", got)
144+
}
145+
}

pkg/store/k8s/physicaldevices.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,22 @@ type physicalDevices struct {
3535
c ctrlclient.Client
3636
}
3737

38+
// PhysicalDeviceCRDName encodes the (node, stable-id) composite
39+
// key into a single CRD name. Uses the same `<node>.<key>` shape
40+
// as `crdName` (StoragePool), `resourceCRDName` (Resource), and
41+
// `snapshotCRDName` (Snapshot) — every composite-key CRD in the
42+
// project follows this convention so operators can grep for
43+
// `node.something` and find every related CRD across kinds.
44+
//
45+
// Run through `Name()` to slugify: stable identifiers carry
46+
// upstream udev shapes (`wwn-0x...`, `scsi-SATA_<vendor>_...`)
47+
// with characters k8s names reject; `Name()` lowercases,
48+
// substitutes invalid runes with `-`, and prepends an 8-char
49+
// SHA256 prefix when slugification was lossy. Phase 10.7.
50+
func PhysicalDeviceCRDName(nodeName, stableID string) string {
51+
return Name(nodeName + "." + stableID)
52+
}
53+
3854
func (s *physicalDevices) List(ctx context.Context) ([]apiv1.PhysicalDevice, error) {
3955
var crdList crdv1alpha1.PhysicalDeviceList
4056

0 commit comments

Comments
 (0)