Skip to content

Implement endpoint slice resolution for ate api connections for HA - #582

Open
Michael Aspinwall (michaelasp) wants to merge 1 commit into
agent-substrate:mainfrom
michaelasp:endpointGRPCRes
Open

Implement endpoint slice resolution for ate api connections for HA#582
Michael Aspinwall (michaelasp) wants to merge 1 commit into
agent-substrate:mainfrom
michaelasp:endpointGRPCRes

Conversation

@michaelasp

Copy link
Copy Markdown

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"}}

@michaelasp

Copy link
Copy Markdown
Author

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

@EItanya

Eitan Yarmush (EItanya) commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

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.

@michaelasp

Michael Aspinwall (michaelasp) commented Jul 29, 2026

Copy link
Copy Markdown
Author

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?

@EItanya

Copy link
Copy Markdown
Collaborator

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:

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

@michaelasp

Michael Aspinwall (michaelasp) commented Jul 29, 2026

Copy link
Copy Markdown
Author

What if we have 2 code paths:

  1. keep the old path when no k8s resolver can be built
  2. 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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider replacing DNS-based round robin + forced reconnects with a k8s-aware gRPC resolver for ateapi clients

2 participants