diff --git a/backend/services/bnk/topology.py b/backend/services/bnk/topology.py index 1a4db70..bb77390 100644 --- a/backend/services/bnk/topology.py +++ b/backend/services/bnk/topology.py @@ -9,6 +9,8 @@ and returns a structured topology dict. """ +import json +import logging import re from typing import Any @@ -39,6 +41,8 @@ # --------------------------------------------------------------------------- +logger = logging.getLogger(__name__) + def analyze_topology(data: dict[str, Any]) -> dict[str, Any]: """Build the gateway topology response from raw BNK data.""" resources = data["resources"] @@ -187,6 +191,20 @@ def _match_routes_to_listener( route_spec = route.get("spec", {}) route_ns = route_meta.get("namespace", "") + # The analyzer records the weights it actually computed in the + # k8s.f5.com/service-settings annotation, NOT in + # spec.rules[].backendRefs[].weight (the author's declared intent). We + # surface the annotation faithfully at the route level as + # ``serviceSettings`` -- the same pool-keyed shape F5AIAnalyzerViewer + # already reads -- so a validated consumer can reinterpret it without + # another backend change (#8). We deliberately do NOT collapse it into a + # per-backendRef ``effectiveWeight`` here: the annotation is keyed by + # POOL, a backendRef is a Service, and the per-pod weights may be a + # within-pool distribution rather than a between-service share -- so the + # pool->service attribution and the collapse both need a real + # multi-backend/multi-pod cluster sample to settle (see PR discussion). + service_settings = _parse_service_settings(route_meta) + for parent in route_spec.get("parentRefs", []): parent_ns = parent.get("namespace", route_ns) if parent.get("name") != gw_name or parent_ns != gw_ns: @@ -215,6 +233,7 @@ def _match_routes_to_listener( "kind": route_kind, "hostnames": route_spec.get("hostnames", []), "backends": backends, + "serviceSettings": service_settings or None, "analyzers": _match_analyzers( analyzers, route_name, route_kind, route_ns, gw_ns, ), @@ -223,6 +242,67 @@ def _match_routes_to_listener( return routes_data +SERVICE_SETTINGS_ANNOTATION = "k8s.f5.com/service-settings" + + +def _coerce_weight(value: Any) -> float | None: + """Tolerant numeric coercion, matching F5AIAnalyzerViewer's ``Number(w)``. + + The annotation's weights arrive as ints in the samples we have, but the + existing frontend consumer accepts floats and numeric strings too, and the + two readers of this annotation must not disagree on what counts as a weight + (a stricter reader silently drops values and falls back to the declared + weight -- invisibly). bool is excluded: it is an int subclass but never a + weight. + """ + if isinstance(value, bool): + return None + if isinstance(value, (int, float)): + return float(value) + if isinstance(value, str): + try: + return float(value) + except ValueError: + return None + return None + + +def _parse_service_settings(route_meta: dict) -> dict[str, dict[str, float]]: + """Faithful parse of the k8s.f5.com/service-settings annotation. + + Per F5 docs the shape is ``{pool_name: {pod_ip: weight}}`` -- e.g. + ``{"pool-3": {"10.244.114.53": 33, "10.244.114.54": 34, "10.244.99.91": 33}}``. + The top-level key is a POOL, not necessarily a Service name; the per-pod + weights may be a within-pool distribution. We return the structure as-is + (keys preserved, weights coerced tolerantly) rather than interpreting it, + because the interpretation is exactly what's unsettled (#8) and the existing + consumer, F5AIAnalyzerViewer, already reads the same annotation this way. + Returns {} when the annotation is absent or unparseable. + """ + raw = (route_meta.get("annotations") or {}).get(SERVICE_SETTINGS_ANNOTATION) + if not raw: + return {} + try: + parsed = json.loads(raw) + except (ValueError, TypeError): + logger.warning("Unparseable %s on route %s", SERVICE_SETTINGS_ANNOTATION, + route_meta.get("name")) + return {} + if not isinstance(parsed, dict): + return {} + out: dict[str, dict[str, float]] = {} + for pool, ip_weights in parsed.items(): + if isinstance(ip_weights, dict): + clean = {} + for ip, w in ip_weights.items(): + cw = _coerce_weight(w) + if cw is not None: + clean[str(ip)] = cw + if clean: + out[str(pool)] = clean + return out + + def _match_analyzers( analyzers: list[dict], route_name: str, diff --git a/backend/tests/unit/test_bnk_topology.py b/backend/tests/unit/test_bnk_topology.py index 6daaec6..ebb6556 100644 --- a/backend/tests/unit/test_bnk_topology.py +++ b/backend/tests/unit/test_bnk_topology.py @@ -12,10 +12,12 @@ _build_data_plane, _build_egress, _build_vlan, + _coerce_weight, _match_analyzers, _match_net_policies, _match_routes_to_listener, _match_sec_policies, + _parse_service_settings, analyze_topology, resolve_list_refs, ) @@ -225,6 +227,87 @@ def test_route_not_matching_different_gateway(self): assert result == [] +# --------------------------------------------------------------------------- +# analyzer weights (#8) — k8s.f5.com/service-settings annotation +# --------------------------------------------------------------------------- + + +def _l4route_with_settings(name, gw_name, gw_ns, backends, settings_json=None): + r = _resource(name, gw_ns, spec={ + "parentRefs": [{"name": gw_name, "namespace": gw_ns}], + "rules": [{"backendRefs": backends}], + }) + if settings_json is not None: + r["metadata"]["annotations"] = {"k8s.f5.com/service-settings": settings_json} + return r + + +class TestServiceSettingsParsing: + """Faithful, tolerant parse of k8s.f5.com/service-settings. + + We surface the annotation as-is (pool-keyed, weights coerced), matching how + F5AIAnalyzerViewer already reads it. We deliberately do NOT collapse it into + a per-backend effectiveWeight here: the top-level key is a POOL (not + necessarily a Service), and whether the per-pod weights are a within-pool + distribution or a between-service share is unsettled without a real + multi-backend/multi-pod cluster sample (#8). + """ + + def test_parse_preserves_pool_keyed_structure(self): + # The F5-docs shape: top-level key is a pool, value is {pod_ip: weight}. + meta = {"name": "r", "annotations": {"k8s.f5.com/service-settings": + '{"pool-3": {"10.244.114.53": 33, "10.244.114.54": 34, "10.244.99.91": 33}}'}} + parsed = _parse_service_settings(meta) + assert parsed == {"pool-3": {"10.244.114.53": 33.0, "10.244.114.54": 34.0, "10.244.99.91": 33.0}} + + def test_parse_missing_or_bad_annotation_is_empty(self): + assert _parse_service_settings({"name": "r"}) == {} + assert _parse_service_settings({"annotations": {"k8s.f5.com/service-settings": "not json"}}) == {} + assert _parse_service_settings({"annotations": {"k8s.f5.com/service-settings": "[1,2]"}}) == {} + + def test_numeric_coercion_matches_the_existing_consumer(self): + # F5AIAnalyzerViewer accepts Number(w): int, float, and numeric strings. + # A stricter reader would silently drop these and fall back to the + # declared weight -- the exact invisible failure mode to avoid. + assert _coerce_weight(99) == 99.0 + assert _coerce_weight(33.5) == 33.5 + assert _coerce_weight("42") == 42.0 + assert _coerce_weight("not-a-number") is None + assert _coerce_weight(True) is None # bool is not a weight + assert _coerce_weight(None) is None + + def test_parse_coerces_float_and_string_weights(self): + meta = {"name": "r", "annotations": {"k8s.f5.com/service-settings": + '{"pool-1": {"10.0.0.1": "50", "10.0.0.2": 50.0}}'}} + parsed = _parse_service_settings(meta) + assert parsed == {"pool-1": {"10.0.0.1": 50.0, "10.0.0.2": 50.0}} + + def test_route_carries_raw_service_settings_end_to_end(self): + # The parsed annotation rides on the route as serviceSettings; backends + # keep the DECLARED weight (the on-screen collapse is deferred to cluster + # validation, so we do not overwrite it with a guess). + route = _l4route_with_settings( + "l4route-vlm-internal-gw1", "gw", "f5-bnk", + backends=[{"name": "vlm-vllm-agg-vlmfrontend", "port": 8000, "weight": 1}], + settings_json='{"pool-3": {"10.244.123.12": 99}}', + ) + result = _match_routes_to_listener([(route, "L4Route")], [], "gw", "f5-bnk", "l4") + assert len(result) == 1 + assert result[0]["serviceSettings"] == {"pool-3": {"10.244.123.12": 99.0}} + # Declared weight preserved; no guessed effectiveWeight on the backend. + backend = result[0]["backends"][0] + assert backend["weight"] == 1 + assert "effectiveWeight" not in backend + + def test_route_without_annotation_has_none_service_settings(self): + route = _l4route_with_settings( + "plain", "gw", "f5-bnk", + backends=[{"name": "svc", "port": 80, "weight": 50}], + ) + result = _match_routes_to_listener([(route, "HTTPRoute")], [], "gw", "f5-bnk", "l4") + assert result[0]["serviceSettings"] is None + + # --------------------------------------------------------------------------- # _match_analyzers # ---------------------------------------------------------------------------