Surface L4Route service-settings faithfully (groundwork for #8; on-screen fix needs cluster data)

[jgruberf5]

Status: groundwork, not a merge-ready fix for #8 — see the review thread

This started as "replace the declared backend weight with the analyzer's computed effectiveWeight." @mwiget's review showed that reading is a guess that conflicts with an existing, F5-docs-grounded consumer (F5AIAnalyzerViewer), and would silently no-op or show a convincing wrong answer in production. So this PR no longer changes the displayed weight, and it does not close #8. It keeps only what's defensible without a cluster sample.

What the review established (all correct)

k8s.f5.com/service-settings is already parsed by F5AIAnalyzerViewer.tsx (on staging), and it reads the annotation as pool-keyed ({"pool-3": {ip: weight}}, straight from F5 docs), rendering each pool · ip as a percentage of the grand total — never collapsing to a per-service absolute. My original backend cut disagreed three ways, each a silent fallback to the declared weight that looks exactly like the bug unfixed:

1. Key is a pool, not a service. My per-backendRef lookup keyed by service name → every lookup misses in production → effectiveWeight always None → UI shows the declared weight again. The tests passed only because they used service-name keys — the same wrong assumption under test.
2. Sum isn't a comparable number. The docs example's per-pod weights sum to 100 within one pool. sum() would render "weight 100 / weight 100".
3. isinstance(int) too strict vs the existing Number(w) — drops floats/numeric-strings silently.

What this PR now does

• Removes the guess: effectiveWeight, the per-backend attribution, _build_backend, the build_route_ref_map propagation, the two frontend render swaps, and the type fields — all backed out.
• Parses faithfully: _parse_service_settings preserves the pool-keyed structure and coerces weights tolerantly (_coerce_weight: int/float/numeric-string, matching Number(w); bool excluded), aligned with the existing consumer so the two readers can't disagree.
• Surfaces it where it belongs: the parsed annotation rides on the route as serviceSettings (the same shape F5AIAnalyzerViewer reads), available for a validated consumer without another backend change. Backends keep the declared weight — unchanged on screen.

What's needed to actually fix #8

The cluster check @mwiget asked for: dump the raw annotation from a route with ≥2 weighted backends across multiple pods (not just the reporter's single-backend case). That one sample settles all three questions — pool↔service mapping, within-pool vs between-service semantics, and numeric type — and only then can the topology/backends views show a correct number. Until then, #8 stays open.

Relates to #8 (does not close it).

https://claude.ai/code/session_01UpRYiFserdBE5ESHn759N4

[jgruberf5]

Self-review

I did this review before opening the PR, and it changed the shape of the change — so this documents what I checked and the calls I made.

The review caught that a backend-only fix wouldn't fix anything

My first cut was backend-only: parse the annotation, put effectiveWeight/analyzerWeights on the topology backend dict, done. Reviewing it, that fixes the data but not what the user sees — nothing consumed the new fields. So I traced every consumer:

1. F5BNKTopologyViewer renders be.weight directly from the topology tree.
2. BackendsCollection renders ref.weight, where ref comes from build_route_ref_map — which walks the same topology backend dicts and copies only weight.

So there were two display surfaces and one shared upstream, and the backend-only change would have left both showing the declared weight. That's why the PR also touches build_route_ref_map (propagate both fields) and both components (effectiveWeight ?? weight). TrafficFlowOverview also iterates route.backends, but only for name/namespace — I checked, it renders no weight, so it's correctly untouched.

Two backend types on the frontend — easy to miss one

F5BNKTopologyViewer defines its own local TopologyBackend, separate from f5bnk.ts's TopologyRouteBackend. tsc caught that I'd updated only the shared one; I added the fields to the local copy too. Flagging because anyone extending this later hits the same duplication.

The one design call, flagged again

effectiveWeight = sum(per_pod.values()). The annotation is per-pod; the views show a per-service backend. Sum is faithful for the reporter's data (one weighted pod per service → sum == that weight) and matches the 1/99 split in the issue. If a service ever has several pods with different analyzer weights under one backend, "sum" may not be the number a human wants — so I kept analyzerWeights on the payload and the type, so the FE can switch to max/avg/per-pod without a backend change.

Fallback semantics I made sure of

effectiveWeight/analyzerWeights are present-and-None when there's no annotation, not absent. That lets the UI's ?? fall through to the declared weight, and lets a reader tell "analyzer weighted this 0" from "analyzer didn't weight it" — a distinction that would vanish if I'd omitted the keys.

What I could not verify

Rendering against a live analyzer. The tests prove the data flows and the components prefer the right field; they can't prove the analyzer emits the annotation shape I parsed. That's the "Environment validation needed" note on the PR, and it's the real last mile — worth one apply against dynamo-system before merge.

[mwiget]

The plumbing is clean — annotation parsed once per route, both numbers carried through build_route_ref_map so the second surface can't drift from the first, declared weight preserved as fallback, and both frontends using the same effectiveWeight ?? weight expression. No complaints about the mechanics.

But this repo already reads this annotation, and it reads it differently.

frontend-v2/src/components/k8s/F5AIAnalyzerViewer.tsx (on staging, lines 244-303) parses k8s.f5.com/service-settings today. The PR body says the annotation was parsed "nowhere in the backend (grep-confirmed)" — true as written, and I confirmed it, but the qualifier is carrying a lot of weight. Two live interpretations of the same annotation in one product is itself the #8 class of bug, so these need to be reconciled before either is trusted.

Three specific ways they disagree, all sharing one failure mode: silent fallback to the declared weight, which looks exactly like the bug still being unfixed.

1. The top-level key: pool name or service name?

F5AIAnalyzerViewer documents the shape straight from F5 docs as {"pool-3":{"10.244.114.53":33,"10.244.114.54":34,"10.244.99.91":33}} and treats the key as a pool. _build_backend looks it up with analyzer_weights.get(br.get("name")) — a backendRef name, i.e. a Service. If SPK keys by pool and the pool name isn't identical to the service name, every lookup misses, effectiveWeight is always None, and the UI quietly shows the declared weight again. Nothing errors, nothing logs, and the tests pass because they construct the annotation with service-name keys — the assumption under test is the same one that would be wrong.

2. Summing per-pod weights may not produce a comparable number.

In the documented example the three per-pod weights sum to 100 for a single pool — they look like a distribution within a pool, not a share between pools. If that's the semantic, sum(per_pod.values()) returns ~100 for every backend regardless of its actual share, and a two-backend route renders "weight 100 / weight 100" — a different wrong answer than the one you're fixing, and a more convincing one.

Note F5AIAnalyzerViewer never collapses to an absolute per-service number for exactly this reason: it keeps each pool · ip separate and renders each as a percentage of the grand total across all pools. That's the interpretation that survives either semantic.

Your reporter sample {"vlm-vllm-agg-vlmfrontend": {"10.244.123.12": 99}} doesn't settle it — a single pod at 99 is consistent with both readings (99 of a 100-point within-pool split, or 99 against some other service's 1), which is why the single-backend case can't distinguish them.

3. isinstance(w, int) is stricter than the existing consumer's Number(w).

Line 258 drops any weight the analyzer emits as a float or a numeric string; clean comes out empty, the service is omitted from out, and it's the silent fallback again. The existing parser accepts both. Whatever the annotation really contains, the two readers should agree — and this one should probably match the more permissive existing behaviour.

---

What would settle it: the cluster check you already flagged, with one addition — dump the raw annotation from a route with two or more weighted backends and multiple pods, not just the reporter's single-backend case. That one sample answers all three questions at once: whether the key matches the service name, whether the per-pod weights sum to 100 per pool or partition across pools, and what numeric type they arrive as. Until then effectiveWeight's definition is a guess, and the failure mode is invisible.

I'd also reconcile with F5AIAnalyzerViewer either way — if your reading is right, that component is showing wrong percentages today and should be fixed alongside; if its reading is right, _build_backend needs to change. Worth resolving in this PR rather than leaving two answers in the tree.

Everything else reads correct: the malformed-annotation paths return {} and degrade safely, the declared weight is preserved for backward compatibility, analyzerWeights is exposed raw so a caller can reinterpret without another backend change, and the frontend's info badge makes analyzer-derived numbers distinguishable at a glance — which is a good touch, since it means an operator can see which reading produced the number on screen.

[mwiget]

This looks up the analyzer map by backendRef.name — a Service name. But F5AIAnalyzerViewer.tsx:249 documents the annotation's top-level key as a pool name, quoting F5 docs: {"pool-3":{"10.244.114.53":33,...}}.

If SPK keys by pool, every lookup here misses and effectiveWeight is permanently None — the UI silently shows the declared weight, i.e. #8 unfixed, with no error and green tests. The tests can't catch it because they build the annotation with service-name keys, which is the assumption under test.

[mwiget]

The sum is only meaningful if per-pod weights partition a service's share. In the F5 docs example quoted in F5AIAnalyzerViewer.tsx:249 the three pod weights sum to 100 within one pool, which reads as a distribution within the pool — under that semantic this returns ~100 for every backend, and a two-backend route shows "weight 100 / weight 100".

F5AIAnalyzerViewer avoids the question by never collapsing to an absolute number: it keeps each pool · ip and renders a percentage of the grand total. Worth confirming against a multi-backend, multi-pod route before fixing the definition here.

[mwiget]

isinstance(w, int) drops floats and numeric strings; clean then comes out empty, the service is left out of out, and the caller falls back to the declared weight with no signal.

The existing consumer uses Number(w) (F5AIAnalyzerViewer.tsx:280), which accepts both. The two readers of this annotation should agree, and the permissive version is the safer default given neither of us has seen the raw value.

[jgruberf5]

You're right, on all three, and thank you — this is exactly the catch that needed to happen before anything shipped. I've backed the guess out (latest push) and reframed the PR to groundwork; it no longer closes #8, and I've converted it to draft since the real fix is blocked on the cluster sample you described.

What I changed:

• Removed effectiveWeight, the per-backendRef attribution, _build_backend, the build_route_ref_map propagation, both frontend render swaps, and the type fields. None of it survives your point Module lifecycle control: destroy blast radius, disabled gating, real cancel, step output #1 alone: keying by service name when the annotation keys by pool means every lookup misses in production, effectiveWeight is always None, and the UI quietly shows the declared weight — a "fix" that closes the issue while doing nothing. And you nailed why the tests didn't catch it: they constructed the annotation with service-name keys, so the assumption under test was the bug.

• _parse_service_settings now parses faithfully: pool-keyed structure preserved, weights coerced tolerantly via _coerce_weight (int/float/numeric-string, matching your Number(w); bool excluded since it's an int subclass but never a weight). This is deliberately aligned with F5AIAnalyzerViewer so the two readers of this annotation can't drift — which was your deeper point: two live interpretations in one product is itself the Displayed weights are not reflecting the correct values #8 class of bug.

• The parsed annotation rides on the route as serviceSettings — the same pool-keyed shape F5AIAnalyzerViewer already reads — so a validated consumer can use it without another backend change. Backends keep the declared weight; nothing on screen changes.

On your point #2 specifically — I agree the single-backend reporter sample can't distinguish "99 of a 100-point within-pool split" from "99 vs 1 between services," and that F5AIAnalyzerViewer's percentage-of-grand-total is the reading that survives either. That's the strongest argument for not collapsing to a per-service absolute anywhere until the data says so, which is why I didn't keep a percentage version either — attributing even a percentage to a specific backendRef still needs the pool→service mapping I can't verify.

On reconciling F5AIAnalyzerViewer: I left it unchanged, because its reading is the trusted one (F5 docs + it never collapses to an absolute). If the cluster sample later shows its percentage math is off, that's a separate change against real data rather than another guess.

I don't have cluster access to pull the multi-backend/multi-pod annotation, so I've left #8 open with that as the explicit next step. If someone can dump kubectl get l4route -n dynamo-system -o json from a route with two or more weighted backends, I'll finish the on-screen fix against it.

[mwiget]

Approving the re-scoped PR. Backing the interpretation out rather than patching around it was the right call, and retitling plus downgrading Fixes #8 to "Relates to #8 (does not close it)" matters as much as the code — a merge that auto-closed #8 while the reporter still saw the wrong number on screen would have been the worst outcome available here.

I verified the revert is complete: helpers.py, BackendsCollection.tsx, F5BNKTopologyViewer.tsx and f5bnk.ts are byte-identical to staging again, and _build_backend is gone. What remains is one additive field.

On the parse itself:

• _coerce_weight checks bool first, which is necessary and easy to get wrong — isinstance(True, int) is True in Python, so without that branch a JSON true would have become weight 1.0. Matching Number(w) for int/float/numeric-string means the two readers of this annotation can no longer disagree about what counts as a weight, which was the third finding.
• Keeping the pool-keyed structure verbatim is the part that makes this safe to merge unresolved: serviceSettings asserts nothing about pool↔service mapping or within-pool vs between-service semantics, so nothing downstream can silently inherit a guess.
• I checked the route (routes/k8s/f5bnk.py:120) declares no response_model, so the new key actually reaches API consumers rather than being dropped by a schema — the same trap that would have made this dead weight.

The field has no consumer yet, which I'd normally push back on. It's justified here because the alternative is a second backend change once the cluster sample arrives, and because the shape is now the one F5AIAnalyzerViewer already reads — so a validated consumer can be written against it directly. Worth revisiting if that sample doesn't materialise; unconsumed data ages badly.

CI: P1 unit, P2 component and P2 legacy all green. P3 · Integration Tests · Backend was still running — it doesn't touch this path, and I'm watching it.

#8 stays open, and the cluster dump from a route with ≥2 weighted backends across multiple pods is still the thing that unblocks the real fix.