Implement endpoint slice resolution for ate api connections for HA by michaelasp · Pull Request #582 · agent-substrate/substrate

Michael Aspinwall (michaelasp) · 2026-07-29T13:44:52Z

Fixes #565

Replaces standard DNS resolution (dns:///) and the forced 1-minute MaxConnectionAge reconnect workaround with a native, event-driven Kubernetes gRPC resolver (internal/k8sresolver). The resolver uses client-go's SharedInformerFactory and EndpointSliceLister to watch discoveryv1.EndpointSlice resources in real time.

Verification

Created a kind cluster to test changes by scaling the deployments up and down and verified the new servers were connected to by clients.

1. Kind Cluster Scale-Out & EndpointSlice Discovery

Click to view kubectl scale and EndpointSlice output
$ kubectl scale deployment/ate-api-server -n ate-system --replicas=4
deployment.apps/ate-api-server scaled

$ kubectl get pods -n ate-system -l app=ate-api-server
NAME                              READY   STATUS    RESTARTS   AGE
ate-api-server-6bcd9fddbb-gxxth   1/1     Running   0          3m8s
ate-api-server-6bcd9fddbb-jgwbw   1/1     Running   0          3m8s
ate-api-server-6bcd9fddbb-m8nbx   1/1     Running   0          7m20s
ate-api-server-6bcd9fddbb-vwm2g   1/1     Running   0          7m20s

$ kubectl get endpointslices -n ate-system -l kubernetes.io/service-name=api
NAME        ADDRESSTYPE   PORTS   ENDPOINTS
api-dmzfc   IPv4          443     10.244.0.15,10.244.0.19,10.244.0.31,10.244.0.32

2. Proof of Active gRPC Load-Balancing Across All Replicas

    [pod/ate-api-server-6bcd9fddbb-gxxth] {"time":"2026-07-29T13:20:49Z","level":"INFO","msg":"Handle RPC","method":"/ateapi.Control/ListActors","principal":{"ID":"spiffe://cluster.local/ns/ate-system/sa/atenet-router"}}
    [pod/ate-api-server-6bcd9fddbb-jgwbw] {"time":"2026-07-29T13:20:46Z","level":"INFO","msg":"Handle RPC","method":"/ateapi.Control/ListActors","principal":{"ID":"spiffe://cluster.local/ns/ate-system/sa/atenet-router"}}
    [pod/ate-api-server-6bcd9fddbb-m8nbx] {"time":"2026-07-29T13:20:48Z","level":"INFO","msg":"Handle RPC","method":"/ateapi.Control/ListActors","principal":{"ID":"spiffe://cluster.local/ns/ate-system/sa/atenet-router"}}
    [pod/ate-api-server-6bcd9fddbb-vwm2g] {"time":"2026-07-29T13:20:47Z","level":"INFO","msg":"Handle RPC","method":"/ateapi.Control/ListActors","principal":{"ID":"spiffe://cluster.local/ns/ate-system/sa/atenet-router"}}

Michael Aspinwall (michaelasp) · 2026-07-29T14:24:04Z

TLS error caused e2e to fail, changed the url to match.

Eitan Yarmush (EItanya) · 2026-07-29T17:01:06Z

Initial agent review:

dns:/// remains supported by gRPC, and adding the k8s resolver does not cause DNS targets to read EndpointSlices. However, this PR makes K8sClient unconditionally required by ateapiauth.DialOptions, even when the caller continues to use a dns:/// target. That unnecessarily couples out-of-cluster/DNS clients to Kubernetes client configuration, despite them never using the k8s:/// resolver.

Could the Kubernetes client requirement be made conditional/opt-in so only callers selecting k8s:/// need Kubernetes API access and EndpointSlice RBAC? External clients should remain able to use DNS, a load balancer, or another stable endpoint without Kubernetes API credentials.

Additional findings from the initial pass:

boomer-glutton now defaults to k8s:///, but the benchmark pod uses the default service account and its manifest does not grant EndpointSlice list/watch. Resolution will therefore fail in the existing benchmark deployment.
The resolver uses the port from the dial target for each pod IP and only consults EndpointSlice.Ports when that target port parses as zero. A Kubernetes Service port can differ from its endpoint/target port, so this can dial the wrong pod port. The current Service happens to use 443 for both.
ctrl.SetLogger(zap.New(zap.UseDevMode(true))) was removed from atecontroller while making the resolver changes. This appears unrelated and may suppress controller-runtime logging; it should likely be retained.
EndpointSlice authorization/list-watch failures happen inside the informer after Build succeeds. Without surfacing cache-sync failure to the caller, a misconfigured client may sit unresolved instead of failing startup with a useful RBAC error.

Michael Aspinwall (michaelasp) · 2026-07-29T17:27:35Z

Initial agent review:

dns:/// remains supported by gRPC, and adding the k8s resolver does not cause DNS targets to read EndpointSlices. However, this PR makes K8sClient unconditionally required by ateapiauth.DialOptions, even when the caller continues to use a dns:/// target. That unnecessarily couples out-of-cluster/DNS clients to Kubernetes client configuration, despite them never using the k8s:/// resolver.

Could the Kubernetes client requirement be made conditional/opt-in so only callers selecting k8s:/// need Kubernetes API access and EndpointSlice RBAC? External clients should remain able to use DNS, a load balancer, or another stable endpoint without Kubernetes API credentials.

Ok I made it so that if we provide a kubernetes client we create a resolver. Although on the apiserver side of things, we used to set the keepalive in order to close connections to allow for us to pick up new endpoints, however that is not optimal. This PR removes that in order to prevent spurious connection drops. This change would make DNS clients not be able tell if new apiservers have come up until they restarted. Not sure if we should account for that. Julian Gutierrez Oschmann (@juli4n) made the change in order to add the keepalive, wdyt?

Eitan Yarmush (EItanya) · 2026-07-29T17:31:20Z

Initial agent review:
dns:/// remains supported by gRPC, and adding the k8s resolver does not cause DNS targets to read EndpointSlices. However, this PR makes K8sClient unconditionally required by ateapiauth.DialOptions, even when the caller continues to use a dns:/// target. That unnecessarily couples out-of-cluster/DNS clients to Kubernetes client configuration, despite them never using the k8s:/// resolver.
Could the Kubernetes client requirement be made conditional/opt-in so only callers selecting k8s:/// need Kubernetes API access and EndpointSlice RBAC? External clients should remain able to use DNS, a load balancer, or another stable endpoint without Kubernetes API credentials.

Ok I made it so that if we provide a kubernetes client we create a resolver. Although on the apiserver side of things, we used to set the keepalive in order to close connections to allow for us to pick up new endpoints, however that is not optimal. This PR removes that in order to prevent spurious connection drops. This change would make DNS clients not be able tell if new apiservers have come up until they restarted. Not sure if we should account for that. Julian Gutierrez Oschmann (Julian Gutierrez Oschmann (@juli4n)) made the change in order to add the keepalive, wdyt?

What if we have 2 code paths:

keep the old path when no k8s resolver can be built
use the new one when it can

Michael Aspinwall (michaelasp) · 2026-07-29T18:22:50Z

What if we have 2 code paths:

keep the old path when no k8s resolver can be built

use the new one when it can

The issue is that this is on 2 sides and we can have many clients to one server. The server can be configured with how long to keep the connection alive(default infinity) but was turned down to kill a connection every minute to allow dns based clients to find new replicas. This is bad for keeping a live connection, and was reported as an issue in GRPC, grpc/grpc#12295

The endpoint resolver fixes that but doesn't help if clients use the dns backend without the resolver. I think that we should expect that if clients connect with no resolver then they do not get the ability to dynamically obtain backends. Maybe we could create a MaxConnectionAge of something large, ~1 hr or so just to ensure that eventually those clients would obtain backends?

Eitan Yarmush (EItanya) · 2026-07-29T19:27:55Z

What if we have 2 code paths:

keep the old path when no k8s resolver can be built

use the new one when it can

The issue is that this is on 2 sides and we can have many clients to one server. The server can be configured with how long to keep the connection alive(default infinity) but was turned down to kill a connection every minute to allow dns based clients to find new replicas. This is bad for keeping a live connection, and was reported as an issue in GRPC, grpc/grpc#12295

The endpoint resolver fixes that but doesn't help if clients use the dns backend without the resolver. I think that we should expect that if clients connect with no resolver then they do not get the ability to dynamically obtain backends. Maybe we could create a MaxConnectionAge of something large, ~1 hr or so just to ensure that eventually those clients would obtain backends?

That sounds ok to me. But maybe stepping back a second what would be the typical way to solve something like this?

Michael Aspinwall (michaelasp) · 2026-07-29T20:53:56Z

That sounds ok to me. But maybe stepping back a second what would be the typical way to solve something like this?

This is mostly an issue that discovery of endpoints happens at the beginning of a connection instantiation with DNS and grpc creates a long lived connection that unless dropped will not reobtain endpoints. We could maybe also create a DNS resolver that obtains endpoints via polling or some other mechanism, but if we consider that most clients will be able to obtain native kubernetes endpoints then this should be enough to eventually balance out any non native client and allow for clients that do want/need immediate access to all endpoints the ability to do so.

I added a hour connection drop so it should reconnect infrequently enough to not cause any issues for clients but allow for any DNS based client to ~eventually obtain IPs.

Benjamin Elder (BenTheElder)

Thanks!

AI first pass with some minor issues:

Benjamin Elder (BenTheElder) · 2026-07-29T21:00:07Z

Should also fix the CI / linter findings.

Julian Gutierrez Oschmann (juli4n) · 2026-07-29T21:55:42Z

+		}
+	})
+
+	update := func(_ any) { r.updateState() }


Maybe just:

reg, err := inf.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{ AddFunc: func(_ any) { r.updateState() }, UpdateFunc: func(_, _ any) { r.updateState() }, DeleteFunc: func(_ any) { r.updateState() }, }

There is no much value in the update wrapper as UpdateFunc needs to also inline the closure.

Done! Also added an error check here, that might have been bad.

Benjamin Elder (BenTheElder) · 2026-07-31T01:10:10Z

+		selector: selector,
+	}
+
+	_ = inf.Informer().SetWatchErrorHandler(func(_ *cache.Reflector, err error) {


🤖 nit 🟢 – Installing a handler here replaces client-go's default one, which was the thing logging "failed to list/watch". Watch errors now reach only cc.ReportError, so they surface through failing RPCs and nowhere else — an idle ClientConn produces no operator-visible signal at all, which is the case where a missing-RBAC misconfiguration is easiest to miss. A slog.Error alongside the ReportError keeps both.

Ah good catch, made it so this logs an error as well. Thanks!

Julian Gutierrez Oschmann (juli4n) self-requested a review July 29, 2026 13:47

Michael Aspinwall (michaelasp) force-pushed the endpointGRPCRes branch from a8e119e to 9ecd49d Compare July 29, 2026 14:22

Michael Aspinwall (michaelasp) force-pushed the endpointGRPCRes branch from 9ecd49d to a104c05 Compare July 29, 2026 16:52

Michael Aspinwall (michaelasp) force-pushed the endpointGRPCRes branch from a104c05 to 8bbb12d Compare July 29, 2026 17:26

Michael Aspinwall (michaelasp) force-pushed the endpointGRPCRes branch from 8bbb12d to a931a05 Compare July 29, 2026 18:33

Michael Aspinwall (michaelasp) force-pushed the endpointGRPCRes branch 2 times, most recently from 6e861cd to 60ca9c6 Compare July 29, 2026 20:52

Benjamin Elder (BenTheElder) reviewed Jul 29, 2026

View reviewed changes

Comment thread internal/k8sresolver/resolver.go

Comment thread internal/k8sresolver/resolver.go Outdated

Michael Aspinwall (michaelasp) force-pushed the endpointGRPCRes branch from 60ca9c6 to 13e664d Compare July 29, 2026 21:26

Julian Gutierrez Oschmann (juli4n) reviewed Jul 29, 2026

View reviewed changes

Michael Aspinwall (michaelasp) force-pushed the endpointGRPCRes branch from 13e664d to d90e940 Compare July 29, 2026 22:07

Benjamin Elder (BenTheElder) reviewed Jul 31, 2026

View reviewed changes

Implement endpoint slice resolution for ate api connections for HA

5b7e4f7

Michael Aspinwall (michaelasp) force-pushed the endpointGRPCRes branch from d90e940 to 5b7e4f7 Compare July 31, 2026 09:37

Uh oh!

Conversation

Michael Aspinwall (michaelasp) commented Jul 29, 2026

Verification

1. Kind Cluster Scale-Out & EndpointSlice Discovery

2. Proof of Active gRPC Load-Balancing Across All Replicas

Uh oh!

Michael Aspinwall (michaelasp) commented Jul 29, 2026

Uh oh!

Eitan Yarmush (EItanya) commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Michael Aspinwall (michaelasp) commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Eitan Yarmush (EItanya) commented Jul 29, 2026

Uh oh!

Michael Aspinwall (michaelasp) commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Eitan Yarmush (EItanya) commented Jul 29, 2026

Uh oh!

Michael Aspinwall (michaelasp) commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Benjamin Elder (BenTheElder) commented Jul 29, 2026

Uh oh!

Julian Gutierrez Oschmann (juli4n) Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Michael Aspinwall (michaelasp) Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Benjamin Elder (BenTheElder) Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Michael Aspinwall (michaelasp) Jul 31, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Eitan Yarmush (EItanya) commented Jul 29, 2026 •

edited

Loading

Michael Aspinwall (michaelasp) commented Jul 29, 2026 •

edited

Loading

Michael Aspinwall (michaelasp) commented Jul 29, 2026 •

edited

Loading

Michael Aspinwall (michaelasp) commented Jul 29, 2026 •

edited

Loading

Julian Gutierrez Oschmann (juli4n) Jul 29, 2026 •

edited

Loading