From ae8efca8d6039ca721146d3bd01a3f4f605568c0 Mon Sep 17 00:00:00 2001 From: Konstantins Feofantovs Date: Sat, 29 Aug 2026 11:16:27 +0300 Subject: [PATCH] feat: topology-aware routing is a request the cluster can silently decline (ADR 0051) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting `spec.trafficDistribution` asks for zone-local traffic; the EndpointSlice controller decides whether to arrange it, and declines when the endpoints are distributed too unevenly against zone capacity. There is no event, no condition and no status field — a Service whose routing works and one whose routing was refused read identically, while the second keeps paying for cross-zone transfer. `workload_policy` now carries, per Service, the field, the topology-mode annotation, and ready-endpoint counts by zone with how many the cluster gave a hint. The finding is one comparison: a mode set against `hinted: 0`. Endpoint addresses, targetRef, node and hostname are removed as the object enters the cache, so the agent never holds a pod IP — the join to a pod is never made, which is what keeps the counts clear of invariant 6. Counts are per address family: a dual-stack Service lists each pod twice, and summing doubles every zone uniformly, leaving the skew plausible and every number wrong. Adds `discovery.k8s.io/endpointslices` to the ClusterRole. Without it the cache never syncs, `endpoint_slices` names itself in `unavailable_sources`, and the Services still ship (ADR 0033). go test -race ./... && make lint && make chart-lint # green make policy-e2e # "service web asked for PreferSameZone; 2 ready # endpoints, 2 hinted, zones map[e2e-zone-a:2]" Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_011YRKuL97KRndaopfpixrhp --- .../templates/controller-rbac.yaml | 3 + docs/adr/0032-cluster-policy.md | 2 +- ...pology-routing-is-asked-and-not-granted.md | 118 +++++++++++ docs/adr/README.md | 1 + docs/backend-requirements.md | 12 ++ docs/prose_test.go | 14 +- docs/security.md | 5 +- internal/chartrender/guardrail_test.go | 1 + internal/collector/cachetransform.go | 23 +++ internal/collector/cachetransform_test.go | 59 ++++++ internal/collector/pods.go | 5 + internal/collector/policy.go | 126 +++++++++++- internal/collector/policy_test.go | 193 +++++++++++++++++- internal/sink/spool_test.go | 7 + .../sink/testdata/workload-policy.golden.json | 14 +- test/e2e/policy_test.go | 71 ++++++- 16 files changed, 635 insertions(+), 19 deletions(-) create mode 100644 docs/adr/0051-topology-routing-is-asked-and-not-granted.md diff --git a/charts/runtime-agent/templates/controller-rbac.yaml b/charts/runtime-agent/templates/controller-rbac.yaml index 37235b2..fea432c 100644 --- a/charts/runtime-agent/templates/controller-rbac.yaml +++ b/charts/runtime-agent/templates/controller-rbac.yaml @@ -55,6 +55,9 @@ rules: - apiGroups: [""] resources: [services] verbs: [get, list, watch] + - apiGroups: [discovery.k8s.io] + resources: [endpointslices] + verbs: [get, list, watch] - apiGroups: [policy] resources: [poddisruptionbudgets] verbs: [get, list, watch] diff --git a/docs/adr/0032-cluster-policy.md b/docs/adr/0032-cluster-policy.md index 1a2152a..89c57aa 100644 --- a/docs/adr/0032-cluster-policy.md +++ b/docs/adr/0032-cluster-policy.md @@ -4,7 +4,7 @@ Date: 2026-08-24 Status: Accepted Amends: 0031 §7 -Amended by: 0033, 0048 +Amended by: 0033, 0048, 0051 Adds the payload kinds `workload_policy` and `cluster_policy`. Adds six resources to the ClusterRole — the first time the agent's read access has widened since it diff --git a/docs/adr/0051-topology-routing-is-asked-and-not-granted.md b/docs/adr/0051-topology-routing-is-asked-and-not-granted.md new file mode 100644 index 0000000..356fae5 --- /dev/null +++ b/docs/adr/0051-topology-routing-is-asked-and-not-granted.md @@ -0,0 +1,118 @@ +# 0051. Topology-aware routing is a request the cluster can silently decline + +Date: 2026-08-29 + +Status: Accepted + +Amends: 0032 §1 + +Adds `spec.trafficDistribution`, the topology-mode annotation, and per-zone +endpoint counts to the Services already carried by `workload_policy`. Adds one +resource to the ClusterRole (`discovery.k8s.io/endpointslices`) and one case to +the informer transform of ADR 0046. + +## Context + +A customer who wants traffic to stay inside an availability zone sets one field +and is told nothing further. `spec.trafficDistribution: PreferSameZone` is a +request: the EndpointSlice controller decides whether to act on it, and it +declines whenever the endpoints are distributed too unevenly against the zones' +CPU capacity — a Service with four replicas in one zone and one in another gets +no hints at all. There is no event, no condition, and no status field. The +`kubectl get svc` output of a Service whose routing works and one whose routing +was refused is identical. + +The cost of the refusal is a cross-zone data transfer bill that the customer +believes they have already eliminated. It is also entirely invisible to every +tool that reads Services, because the answer is not on the Service. + +The agent already reports Services (ADR 0048 §4) — name, type, headless, the two +traffic policies. It reports none of what says whether the routing the customer +asked for exists. + +`workload_metadata.pod.nodes` gives replica placement per node, and +`node_metadata` gives each node's zone, so a determined backend can already +compute where the replicas are. It cannot compute what the EndpointSlice +controller decided, which is the half that turns a placement observation into a +finding. + +## Decision + +**1. Endpoint zones are counted, and nothing that identifies an endpoint is +read.** The counts are ready endpoints per zone, per Service, taken from +`endpoints[].zone`. `addresses` (pod IPs), `targetRef` (a pod's name and UID), +`nodeName`, `hostname` and the deprecated topology map are removed in the +informer transform, so they never enter the cache — "not collected" rather than +"collected and dropped" (CLAUDE.md invariant 4, ADR 0046). `targetRef` is not +read at all, which is what keeps this clear of invariant 6: there is no pod +whose identity could leak, because the join to a pod is never made. + +The counts are a property of the Service's routing, in the class of the +node-level totals that are collected regardless of workload filters. They are +attached only to Services already resolved to an admitted workload, so a Service +selecting nothing the agent collects is still never named. + +This also bounds memory. A Service with thousands of endpoints holds thousands +of addresses; the agent would have carried them for the life of the process to +count something none of them answers. + +**2. One entry per address family, never summed.** A dual-stack Service lists +each pod once in an IPv4 slice and once in an IPv6 slice. Merging them into one +zone map doubles every zone — uniformly, so the skew still looks right while +every absolute number is wrong. The payload carries `endpoints[]` keyed by +`address_type`, and the obligation not to sum across them is stated in +`backend-requirements.md`. + +**3. Both the field and the annotation are kept verbatim, bounded, not +normalized.** The field's own vocabulary is in motion — `PreferClose` is +deprecated in favour of `PreferSameZone`, and `PreferSameNode` joined them — so a +closed set in the agent would be a list to chase across a fleet the agent cannot +update. Passing the value through costs nothing and dates correctly. + +The same holds for the two annotations that asked for this before the field +existed and are still set on live clusters. A value outside Kubernetes' +vocabulary — `auto` where `Auto` was meant, `enabled` where nothing of the sort +exists — configures nothing while looking configured, and that is the same +finding as a declined hint arriving by a different route. Normalizing to a +closed vocabulary would erase it. Both are bounded at 32 characters and dropped +whole past that, the rule ADR 0019 set for build settings. + +**4. An unset ready condition counts as ready.** `conditions.ready` is a +pointer, and the API instructs a consumer that does not understand the condition +to treat the endpoint as ready. Reading `nil` as not-ready would under-count +every zone in the cluster, quietly and everywhere. + +**5. A Service with no slices carries no endpoint block.** Not a block of zeros: +zero ready endpoints is a claim about a Service, and "the agent has not seen +this Service's slices" is a claim about the agent. The distinction is ADR 0048 +§2's, and here it matters because a new informer's cache fills after the first +metadata flush. + +## Consequences + +**Easier.** The finding is one comparison on one payload: a set +`traffic_distribution` or `topology_mode` against `hinted: 0`. It needs no +history, no profiling and no second read, so it belongs to the first-hour report +rather than to week two. The zone counts also answer, for the first time, where +a Service's traffic can actually land — which the placement fields describe for +pods and never for routes. + +**Harder / given up.** One more resource in the ClusterRole, and it is the first +in the `discovery.k8s.io` group. EndpointSlices are also the most numerous +object class the agent now watches — a large cluster has more of them than pods — +so the transform is doing real work on every event, not only guarding a promise. + +The counts describe every ready endpoint of the Service, including ones backed +by pods that opted out of collection individually. That is deliberate: routing +does not respect the agent's filters, and a zone distribution computed over the +admitted subset would be a distribution of nothing real. No such pod is named or +counted separately; the number is an aggregate over the Service. + +**Not changed.** No new payload kind and no registry row: these are fields on +`workload_policy`, which stays structural, superseding, one per cluster. The +degradation path is the existing one — without the grant the cache never syncs, +`endpoint_slices` appears in `unavailable_sources`, and the Services still ship +without the block (ADR 0033). + +This ADR records decisions implemented in the same pull request, per the process +in [`README.md`](README.md). diff --git a/docs/adr/README.md b/docs/adr/README.md index 6921fc5..1607633 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -108,3 +108,4 @@ line to paste. - [0048. Four first-hour findings name fields the agent did not collect](0048-findings-name-the-fields-they-need.md) - [0049. Revisions belong to whatever controls the ReplicaSet, and the payload is renamed for it](0049-revisions-are-not-only-deployments.md) - [0050. The GOMAXPROCS finding rests on a build fact, not on the toolchain version](0050-godebug-defaults-are-a-build-fact.md) +- [0051. Topology-aware routing is a request the cluster can silently decline](0051-topology-routing-is-asked-and-not-granted.md) diff --git a/docs/backend-requirements.md b/docs/backend-requirements.md index e298f22..9cc73f9 100644 --- a/docs/backend-requirements.md +++ b/docs/backend-requirements.md @@ -360,6 +360,18 @@ in nothing. A `GODEBUG` variable in `workload_metadata.runtime_env` overrides both at startup; it is a fact about a workload, this is a fact about a build, and the backend MUST NOT merge them into one value. +**`workload_policy` Service endpoint counts are per address family and MUST NOT +be summed across them** ([ADR 0051](adr/0051-topology-routing-is-asked-and-not-granted.md)). +A dual-stack Service lists each pod once in an IPv4 slice and once in an IPv6 +slice, so adding the families doubles every zone. `zones` counts ready endpoints +by zone; `unzoned` counts those on a node carrying no zone label and MUST NOT be +folded into a zone named `""`. `hinted` is read together with +`traffic_distribution` (or `topology_mode`): a `hinted` of 0 where neither is set +means nothing, and where one is set means the cluster declined to arrange the +routing that was asked for — which is the finding. The `endpoints` array is +absent when the agent has not seen the Service's slices, which is not the same +as a Service with no ready endpoints, and MUST NOT be rendered as zero. + **`node_metadata.nodes[].architecture` is what `go_build.settings.GOARCH` is compared against.** A build's target architecture and microarchitecture level mean nothing on their own; the comparison is the finding. The field is the diff --git a/docs/prose_test.go b/docs/prose_test.go index 2491285..4953d17 100644 --- a/docs/prose_test.go +++ b/docs/prose_test.go @@ -23,19 +23,19 @@ const maxCommentRun = 8 // The two documents that grow with every payload kind, at the size each landed // once the restated ADRs came out (ADR 0044). `security.md` is deliberately not -// back at ADR 0024's 750: kinds have been added since, each a table row and a -// paragraph. `backend-requirements.md` restates decisions on purpose — its -// reader implements ingest and reads neither our ADRs nor `security.md`. -// Ceilings rose for obligations the documents did not have before: 920 → 930 -// (ADR 0048), then 930 → 940 and 460 → 475 for a field whose absence the -// backend would otherwise read as a value (ADR 0050). +// back at ADR 0024's 750: kinds have been added since. `backend-requirements.md` +// restates decisions on purpose — its reader implements ingest and reads neither +// our ADRs nor `security.md`. Ceilings rose for obligations the documents did +// not have before: 920 → 930 (ADR 0048), 930 → 940 and 460 → 475 for a field +// whose absence the backend would read as a value (ADR 0050), 475 → 495 for +// counts it would otherwise sum across address families (ADR 0051). var ceilings = []struct { file string lines int section int }{ {"security.md", 940, 300}, - {"backend-requirements.md", 475, 330}, + {"backend-requirements.md", 495, 345}, } func TestNoCommentRunIsLongerThanAPointerToAnADR(t *testing.T) { diff --git a/docs/security.md b/docs/security.md index 256a75e..3524b87 100644 --- a/docs/security.md +++ b/docs/security.md @@ -140,7 +140,8 @@ bound to nothing and its token is not mounted — so it appears in no row here. | `replicasets`, `deployments`, `statefulsets`, `daemonsets`, `jobs`, `cronjobs` | get/list/watch | Resolve the `ownerReferences` chain (Pod → ReplicaSet → Deployment) so findings are aggregated per workload, not per pod; read each workload's own annotations to honor an opt-out written there ([ADR 0028](adr/0028-workload-level-opt-out.md)); and read finished Jobs' timings and outcomes for `job_runs` ([ADR 0029](adr/0029-job-runs.md)) | | `nodes` | get/list/watch | `allocatable`/`capacity` for node idle computation; labels `node.kubernetes.io/instance-type` and capacity-type (spot/on-demand) for the cost model; `status.nodeInfo.kernelVersion` to report whether nodes meet the kernel floor for the eBPF profile (`CAP_BPF` requires kernel 5.8+, see [§7](#7-node-privileges)) | | `namespaces` | get/list/watch | Evaluate namespace allow/deny filters and the opt-out annotation | -| `services` | get/list/watch | Report that traffic is routed to a workload at all ([ADR 0048](adr/0048-findings-name-the-fields-they-need.md)): one replica is a batch size or an outage depending on whether anything sends it requests, and a declared `containerPort` is an announcement rather than a route. The selector is resolved against pods that passed your filters and never reported as such. Addresses are not read — a ClusterIP is compared against `None` to tell a headless Service apart, and nothing else | +| `services` | get/list/watch | Report that traffic is routed to a workload at all ([ADR 0048](adr/0048-findings-name-the-fields-they-need.md)): one replica is a batch size or an outage depending on whether anything sends it requests, and a declared `containerPort` is an announcement rather than a route. The selector is resolved against pods that passed your filters and never reported as such. Addresses are not read — a ClusterIP is compared against `None` to tell a headless Service apart, and nothing else. Also read: `spec.trafficDistribution` and the topology-mode annotation, which say whether you asked for traffic to stay inside a zone | +| `endpointslices` (`discovery.k8s.io`) | get/list/watch | Count, per Service, how many ready endpoints sit in each zone and how many carry the routing hint the cluster decides to give ([ADR 0051](adr/0051-topology-routing-is-asked-and-not-granted.md)). Setting `trafficDistribution` is a request the EndpointSlice controller may silently decline, and nothing on the Service says which happened. **Endpoint addresses are pod IPs and are never read**: they, the `targetRef` naming the pod, its node and its hostname are removed as the object enters the cache, so the agent never holds them (ADR 0046). Only counts leave, and no endpoint is identified | | `poddisruptionbudgets`, `horizontalpodautoscalers` | get/list/watch | Report what bounds a workload from outside its own spec ([ADR 0032](adr/0032-cluster-policy.md)): a budget can forbid eviction outright, so a node covered by one cannot be drained at all; an autoscaler targeting CPU *utilization* targets a percentage of the request, so the request cannot be read safely without it. A budget's selector is resolved against pods that passed your filters and never reported as such | | `limitranges`, `resourcequotas` | get/list/watch | Distinguish a request a team chose from one the namespace supplied on their behalf, and report the ceiling a namespace sits under and how much of it is spent (ADR 0032). Read only for namespaces that pass your filters | | `persistentvolumeclaims`, `storageclasses` | get/list/watch | A bound claim on a zonal volume pins its pod to that zone, which no field of the pod spec states (ADR 0032). A StorageClass's `parameters` are **not** read — that is the one field here carrying provider configuration (endpoints, key identifiers); what is read is how the class behaves: provisioner, reclaim policy, `volumeBindingMode`, expansion | @@ -491,7 +492,7 @@ test fails if a kind ships without appearing here (ADR 0022). | `job_runs` | Per hour: each Job that finished — when it started and finished, whether it succeeded, the failure reason, its pod success/failure counts, and its declared `parallelism`/`completions`/`backoffLimit` | **yes** | | `workload_revisions` | Per flush: each ReplicaSet of a collected workload that manages ReplicaSets — a Deployment, or a custom resource such as an Argo Rollout ([ADR 0049](adr/0049-revisions-are-not-only-deployments.md)) — with when it was created, its desired/current/ready replica counts, each container's image reference, and a revision number for a Deployment only | **yes** | | `workload_metadata` | Declared shape per (namespace, workload, container, image digest): image, requests, limits, ports, probe schedules without what any probe checks, the five runtime knobs of the field-minimization list below, QoS, replica counts by phase, by node, and by unscheduled reason, the pod's reduced placement constraints (ADR 0031), and — for a Deployment, StatefulSet or DaemonSet — the update strategy by which the workload replaces its own replicas ([ADR 0048](adr/0048-findings-name-the-fields-they-need.md)). A workload of any other kind carries no strategy and nothing standing in for one | no | -| `workload_policy` | Per collected workload, what bounds it from outside its own spec: disruption budgets covering it, autoscalers driving it, the volume claims its pods mount (ADR 0032), and the Services routing to it (ADR 0048); plus `unavailable_sources`, naming any of those the agent could not read — never granted, or no longer being fed (ADR 0033, ADR 0035) | not directly — but a claim is named, and a StatefulSet's claim is `