Skip to content

Commit a79f721

Browse files
committed
docs/test: update README and fix lint in topology-extractor
README: remove stale 'at endpoint creation' phrasing; clarify that Hostname is sourced from spec.hostname or a configured pod label. Tests: drop always-constant parameters from helper functions (unparam); introduce constants for repeated string literals (goconst). Signed-off-by: Etai Lev Ran <elevran@gmail.com>
1 parent 3c07e37 commit a79f721

2 files changed

Lines changed: 59 additions & 53 deletions

File tree

pkg/epp/framework/plugins/datalayer/attribute/topology/README.md

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@ Carries the locality of an endpoint. Populated once at endpoint creation.
99

1010
- **Key**: `TopologyAttributeKey` (`Topology`)
1111
- **Fields**:
12-
- `Hostname`: The host name of the endpoint, derived from the Pod hostname
13-
field or from a user-configured label.
12+
- `Hostname`: The host name of the endpoint. Sourced from `spec.hostname`
13+
on the Pod object, or from a user-configured pod label.
1414

1515
## Producers
1616

1717
The following plugins produce this attribute:
1818

19-
- **`topology-extractor`** (Data Layer): Sets the `Topology` attribute on each
20-
endpoint when it is created, using the Pod hostname or a configured label.
19+
- **`topology-extractor`** (Data Layer): Sets the `Topology` attribute using
20+
`spec.hostname` from the Pod object, or the value of a configured pod label.

pkg/epp/framework/plugins/datalayer/extractor/topology/extractor_test.go

Lines changed: 55 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,14 @@ func makeDecoder(v any) *json.Decoder {
6363
return json.NewDecoder(bytes.NewReader(b))
6464
}
6565

66-
func readTopology(ep fwkdl.Endpoint, name string) (*attrtopology.Topology, bool) {
67-
dk := attrtopology.TopologyAttributeKey.WithNonEmptyProducerName(name).String()
66+
const (
67+
testPluginName = "test"
68+
testNamespace = "default"
69+
testNodeHostname = "node-hostname"
70+
)
71+
72+
func readTopology(ep fwkdl.Endpoint) (*attrtopology.Topology, bool) {
73+
dk := attrtopology.TopologyAttributeKey.WithNonEmptyProducerName(testPluginName).String()
6874
raw, ok := ep.GetAttributes().Get(dk)
6975
if !ok {
7076
return nil, false
@@ -75,21 +81,21 @@ func readTopology(ep fwkdl.Endpoint, name string) (*attrtopology.Topology, bool)
7581

7682
// newRankEndpoint creates an endpoint whose NamespacedName uses the rank suffix
7783
// matching the real datastore convention (pod.Name + "-rank-" + idx).
78-
func newRankEndpoint(podName, namespace string, rank int, labels map[string]string) fwkdl.Endpoint {
84+
func newRankEndpoint(podName string, rank int, labels map[string]string) fwkdl.Endpoint {
7985
epName := fmt.Sprintf("%s-rank-%d", podName, rank)
8086
return fwkdl.NewEndpoint(&fwkdl.EndpointMetadata{
81-
NamespacedName: types.NamespacedName{Name: epName, Namespace: namespace},
87+
NamespacedName: types.NamespacedName{Name: epName, Namespace: testNamespace},
8288
PodName: podName,
8389
Labels: labels,
8490
}, nil)
8591
}
8692

8793
// makePod builds an unstructured Pod with optional spec.hostname and readiness.
88-
func makePod(name, namespace, hostname string, ready bool) *unstructured.Unstructured {
94+
func makePod(name, hostname string, ready bool) *unstructured.Unstructured {
8995
u := &unstructured.Unstructured{}
9096
u.SetGroupVersionKind(podGVK)
9197
u.SetName(name)
92-
u.SetNamespace(namespace)
98+
u.SetNamespace(testNamespace)
9399
if hostname != "" {
94100
_ = unstructured.SetNestedField(u.Object, hostname, "spec", "hostname")
95101
}
@@ -144,12 +150,12 @@ func podEvent(t fwkdl.EventType, pod *unstructured.Unstructured) fwkdl.Notificat
144150
func TestLabel_EndpointHandler_LabelPresent(t *testing.T) {
145151
_, epH, _ := getHandlers(t, &params{HostnameLabel: "topology.hostname"})
146152

147-
ep := newRankEndpoint("worker-1", "default", 0, map[string]string{"topology.hostname": "rack-42"})
153+
ep := newRankEndpoint("worker-1", 0, map[string]string{"topology.hostname": "rack-42"})
148154
if err := epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep)); err != nil {
149155
t.Fatalf("Extract: %v", err)
150156
}
151157

152-
got, ok := readTopology(ep, "test")
158+
got, ok := readTopology(ep)
153159
if !ok {
154160
t.Fatal("expected Topology attribute")
155161
}
@@ -161,43 +167,43 @@ func TestLabel_EndpointHandler_LabelPresent(t *testing.T) {
161167
func TestLabel_EndpointHandler_LabelMissing(t *testing.T) {
162168
_, epH, _ := getHandlers(t, &params{HostnameLabel: "topology.hostname"})
163169

164-
ep := newRankEndpoint("worker-2", "default", 0, map[string]string{"other": "value"})
170+
ep := newRankEndpoint("worker-2", 0, map[string]string{"other": "value"})
165171
if err := epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep)); err != nil {
166172
t.Fatalf("Extract: %v", err)
167173
}
168-
if _, ok := readTopology(ep, "test"); ok {
174+
if _, ok := readTopology(ep); ok {
169175
t.Error("expected no Topology attribute when label is absent")
170176
}
171177
}
172178

173179
func TestLabel_EndpointHandler_NilLabels(t *testing.T) {
174180
_, epH, _ := getHandlers(t, &params{HostnameLabel: "topology.hostname"})
175181

176-
ep := newRankEndpoint("worker-3", "default", 0, nil)
182+
ep := newRankEndpoint("worker-3", 0, nil)
177183
if err := epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep)); err != nil {
178184
t.Fatalf("Extract: %v", err)
179185
}
180-
if _, ok := readTopology(ep, "test"); ok {
186+
if _, ok := readTopology(ep); ok {
181187
t.Error("expected no Topology attribute when pod has no labels")
182188
}
183189
}
184190

185191
func TestLabel_EndpointHandler_DeleteIsNoop(t *testing.T) {
186192
_, epH, _ := getHandlers(t, &params{HostnameLabel: "topology.hostname"})
187193

188-
ep := newRankEndpoint("worker-4", "default", 0, map[string]string{"topology.hostname": "rack-1"})
194+
ep := newRankEndpoint("worker-4", 0, map[string]string{"topology.hostname": "rack-1"})
189195
if err := epH.Extract(context.Background(), epEvent(fwkdl.EventDelete, ep)); err != nil {
190196
t.Fatalf("Extract: %v", err)
191197
}
192-
if _, ok := readTopology(ep, "test"); ok {
198+
if _, ok := readTopology(ep); ok {
193199
t.Error("expected no Topology attribute for delete event")
194200
}
195201
}
196202

197203
func TestLabel_PodHandler_IsNoop(t *testing.T) {
198204
_, _, podH := getHandlers(t, &params{HostnameLabel: "topology.hostname"})
199205

200-
pod := makePod("worker-1", "default", "actual-hostname", true)
206+
pod := makePod("worker-1", "actual-hostname", true)
201207
if err := podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, pod)); err != nil {
202208
t.Fatalf("Extract: %v", err)
203209
}
@@ -208,50 +214,50 @@ func TestLabel_PodHandler_IsNoop(t *testing.T) {
208214
func TestNoLabel_PodFirst_SingleEndpoint(t *testing.T) {
209215
_, epH, podH := getHandlers(t, nil)
210216

211-
pod := makePod("worker-5", "default", "node-hostname", true)
217+
pod := makePod("worker-5", testNodeHostname, true)
212218
if err := podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, pod)); err != nil {
213219
t.Fatalf("podH.Extract: %v", err)
214220
}
215221

216-
ep := newRankEndpoint("worker-5", "default", 0, nil)
222+
ep := newRankEndpoint("worker-5", 0, nil)
217223
if err := epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep)); err != nil {
218224
t.Fatalf("epH.Extract: %v", err)
219225
}
220226

221-
got, ok := readTopology(ep, "test")
227+
got, ok := readTopology(ep)
222228
if !ok {
223229
t.Fatal("expected Topology attribute")
224230
}
225-
if got.Hostname != "node-hostname" {
226-
t.Errorf("hostname = %q, want %q", got.Hostname, "node-hostname")
231+
if got.Hostname != testNodeHostname {
232+
t.Errorf("hostname = %q, want %q", got.Hostname, testNodeHostname)
227233
}
228234
}
229235

230236
func TestNoLabel_PodFirst_MultiPort(t *testing.T) {
231237
_, epH, podH := getHandlers(t, nil)
232238

233239
// Pod notification fires once.
234-
pod := makePod("worker-6", "default", "node-hostname", true)
240+
pod := makePod("worker-6", testNodeHostname, true)
235241
if err := podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, pod)); err != nil {
236242
t.Fatalf("podH.Extract: %v", err)
237243
}
238244

239245
// Two rank endpoints arrive.
240-
ep0 := newRankEndpoint("worker-6", "default", 0, nil)
241-
ep1 := newRankEndpoint("worker-6", "default", 1, nil)
246+
ep0 := newRankEndpoint("worker-6", 0, nil)
247+
ep1 := newRankEndpoint("worker-6", 1, nil)
242248
for i, ep := range []fwkdl.Endpoint{ep0, ep1} {
243249
if err := epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep)); err != nil {
244250
t.Fatalf("epH.Extract rank %d: %v", i, err)
245251
}
246252
}
247253

248254
for i, ep := range []fwkdl.Endpoint{ep0, ep1} {
249-
got, ok := readTopology(ep, "test")
255+
got, ok := readTopology(ep)
250256
if !ok {
251257
t.Fatalf("rank %d: expected Topology attribute", i)
252258
}
253-
if got.Hostname != "node-hostname" {
254-
t.Errorf("rank %d: hostname = %q, want %q", i, got.Hostname, "node-hostname")
259+
if got.Hostname != testNodeHostname {
260+
t.Errorf("rank %d: hostname = %q, want %q", i, got.Hostname, testNodeHostname)
255261
}
256262
}
257263
}
@@ -261,48 +267,48 @@ func TestNoLabel_PodFirst_MultiPort(t *testing.T) {
261267
func TestNoLabel_EndpointFirst_SingleEndpoint(t *testing.T) {
262268
_, epH, podH := getHandlers(t, nil)
263269

264-
ep := newRankEndpoint("worker-7", "default", 0, nil)
270+
ep := newRankEndpoint("worker-7", 0, nil)
265271
if err := epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep)); err != nil {
266272
t.Fatalf("epH.Extract: %v", err)
267273
}
268274

269-
pod := makePod("worker-7", "default", "node-hostname", true)
275+
pod := makePod("worker-7", testNodeHostname, true)
270276
if err := podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, pod)); err != nil {
271277
t.Fatalf("podH.Extract: %v", err)
272278
}
273279

274-
got, ok := readTopology(ep, "test")
280+
got, ok := readTopology(ep)
275281
if !ok {
276282
t.Fatal("expected Topology attribute")
277283
}
278-
if got.Hostname != "node-hostname" {
279-
t.Errorf("hostname = %q, want %q", got.Hostname, "node-hostname")
284+
if got.Hostname != testNodeHostname {
285+
t.Errorf("hostname = %q, want %q", got.Hostname, testNodeHostname)
280286
}
281287
}
282288

283289
func TestNoLabel_EndpointFirst_MultiPort(t *testing.T) {
284290
_, epH, podH := getHandlers(t, nil)
285291

286-
ep0 := newRankEndpoint("worker-8", "default", 0, nil)
287-
ep1 := newRankEndpoint("worker-8", "default", 1, nil)
292+
ep0 := newRankEndpoint("worker-8", 0, nil)
293+
ep1 := newRankEndpoint("worker-8", 1, nil)
288294
for i, ep := range []fwkdl.Endpoint{ep0, ep1} {
289295
if err := epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep)); err != nil {
290296
t.Fatalf("epH.Extract rank %d: %v", i, err)
291297
}
292298
}
293299

294-
pod := makePod("worker-8", "default", "node-hostname", true)
300+
pod := makePod("worker-8", testNodeHostname, true)
295301
if err := podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, pod)); err != nil {
296302
t.Fatalf("podH.Extract: %v", err)
297303
}
298304

299305
for i, ep := range []fwkdl.Endpoint{ep0, ep1} {
300-
got, ok := readTopology(ep, "test")
306+
got, ok := readTopology(ep)
301307
if !ok {
302308
t.Fatalf("rank %d: expected Topology attribute", i)
303309
}
304-
if got.Hostname != "node-hostname" {
305-
t.Errorf("rank %d: hostname = %q, want %q", i, got.Hostname, "node-hostname")
310+
if got.Hostname != testNodeHostname {
311+
t.Errorf("rank %d: hostname = %q, want %q", i, got.Hostname, testNodeHostname)
306312
}
307313
}
308314
}
@@ -313,17 +319,17 @@ func TestNoLabel_NotReadyPod_NotCached(t *testing.T) {
313319
ext, epH, podH := getHandlers(t, nil)
314320

315321
// Not-ready pod notification — hostname must not be cached.
316-
pod := makePod("worker-9", "default", "node-hostname", false)
322+
pod := makePod("worker-9", testNodeHostname, false)
317323
if err := podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, pod)); err != nil {
318324
t.Fatalf("podH.Extract: %v", err)
319325
}
320326

321-
ep := newRankEndpoint("worker-9", "default", 0, nil)
327+
ep := newRankEndpoint("worker-9", 0, nil)
322328
if err := epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep)); err != nil {
323329
t.Fatalf("epH.Extract: %v", err)
324330
}
325331

326-
if _, ok := readTopology(ep, "test"); ok {
332+
if _, ok := readTopology(ep); ok {
327333
t.Error("expected no Topology attribute for not-ready pod")
328334
}
329335

@@ -341,7 +347,7 @@ func TestNoLabel_NotReadyEvictsCache(t *testing.T) {
341347
podKey := types.NamespacedName{Name: "worker-10", Namespace: "default"}
342348

343349
// Pod is ready — hostname cached.
344-
ready := makePod("worker-10", "default", "node-hostname", true)
350+
ready := makePod("worker-10", testNodeHostname, true)
345351
_ = podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, ready))
346352

347353
ext.mu.Lock()
@@ -352,7 +358,7 @@ func TestNoLabel_NotReadyEvictsCache(t *testing.T) {
352358
}
353359

354360
// Pod becomes not-ready — cache evicted.
355-
notReady := makePod("worker-10", "default", "node-hostname", false)
361+
notReady := makePod("worker-10", testNodeHostname, false)
356362
_ = podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, notReady))
357363

358364
ext.mu.Lock()
@@ -368,16 +374,16 @@ func TestNoLabel_NotReadyEvictsCache(t *testing.T) {
368374
func TestNoLabel_EmptyHostname_NoAttribute(t *testing.T) {
369375
_, epH, podH := getHandlers(t, nil)
370376

371-
ep := newRankEndpoint("worker-11", "default", 0, nil)
377+
ep := newRankEndpoint("worker-11", 0, nil)
372378
_ = epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep))
373379

374380
// Ready pod but spec.hostname not set.
375-
pod := makePod("worker-11", "default", "", true)
381+
pod := makePod("worker-11", "", true)
376382
if err := podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, pod)); err != nil {
377383
t.Fatalf("podH.Extract: %v", err)
378384
}
379385

380-
if _, ok := readTopology(ep, "test"); ok {
386+
if _, ok := readTopology(ep); ok {
381387
t.Error("expected no Topology attribute when spec.hostname is empty")
382388
}
383389
}
@@ -386,7 +392,7 @@ func TestNoLabel_PodDelete_ClearsHostnameCache(t *testing.T) {
386392
ext, _, podH := getHandlers(t, nil)
387393

388394
podKey := types.NamespacedName{Name: "worker-12", Namespace: "default"}
389-
ready := makePod("worker-12", "default", "node-hostname", true)
395+
ready := makePod("worker-12", testNodeHostname, true)
390396
_ = podH.Extract(context.Background(), podEvent(fwkdl.EventAddOrUpdate, ready))
391397

392398
ext.mu.Lock()
@@ -396,7 +402,7 @@ func TestNoLabel_PodDelete_ClearsHostnameCache(t *testing.T) {
396402
t.Fatal("precondition: expected hostname cached")
397403
}
398404

399-
deleted := makePod("worker-12", "default", "", false)
405+
deleted := makePod("worker-12", "", false)
400406
if err := podH.Extract(context.Background(), podEvent(fwkdl.EventDelete, deleted)); err != nil {
401407
t.Fatalf("podH.Extract delete: %v", err)
402408
}
@@ -413,8 +419,8 @@ func TestNoLabel_EndpointDelete_RemovesFromMap(t *testing.T) {
413419
ext, epH, _ := getHandlers(t, nil)
414420

415421
podKey := types.NamespacedName{Name: "worker-13", Namespace: "default"}
416-
ep0 := newRankEndpoint("worker-13", "default", 0, nil)
417-
ep1 := newRankEndpoint("worker-13", "default", 1, nil)
422+
ep0 := newRankEndpoint("worker-13", 0, nil)
423+
ep1 := newRankEndpoint("worker-13", 1, nil)
418424

419425
_ = epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep0))
420426
_ = epH.Extract(context.Background(), epEvent(fwkdl.EventAddOrUpdate, ep1))

0 commit comments

Comments
 (0)