Skip to content

feat: EPP peer discovery via EndpointSlices - #2341

Open
evacchi wants to merge 5 commits into
llm-d:mainfrom
evacchi:epp-peer-discovery
Open

feat: EPP peer discovery via EndpointSlices#2341
evacchi wants to merge 5 commits into
llm-d:mainfrom
evacchi:epp-peer-discovery

Conversation

@evacchi

@evacchi evacchi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind feature

What this PR does / why we need it:

Adds Kubernetes-based discovery of peer EPP replicas. It follows the approach outlined in #1892 (comment): a controller-runtime reconciler that watches the EPP Service's EndpointSlices to track the live set of replicas.

Follow-up: a file-based k8s-independent discovery mechanism.

  • Mirrors endpoint discovery: PeerMetadata, PeerStore, PeerNotifier, and an in-memory MemoryPeerStore. The reconciler populates the store; nothing consumes it in this PR (will be implemented in another follow-up).
  • EPPPeerReconciler: aggregates the ready endpoints across the Service's EndpointSlices, excludes this replica, and drives peer add/update/delete through the notifier. It runs on every replica (leader election disabled) since each replica needs its own peer view.
  • Opt-in via --enable-peer-discovery, which requires --epp-peer-service naming the EPP's own Service. Off by default: no behavior change and no extra RBAC unless enabled.
  • Scopes the EndpointSlice informer to the pool namespace and Service so the existing namespaced Role suffices; adds discovery.k8s.io/endpointslices permission to the kustomize component and the Helm chart.

Out of scope (follow-ups): the distributed CrossReplicaSyncer that consumes the peer set, and file-based peer discovery for non-Kubernetes environments.

Testing:

  • Unit: reconciler add/update/delete diffing, ready/self exclusion, TargetRef ID fallback; MemoryPeerStore; options validation (enable without service fails); leader-election guard (never leader-gated).
  • Integration (envtest): peers appear, update, and drop as EndpointSlices are created, an endpoint goes NotReady, and a slice is deleted.
  • Manual (kind): verified against real EndpointSlice churn; steady state is one peer per replica with self excluded.

Which issue(s) this PR fixes:

Part of #1892, related to #1946.

Release note (write NONE if no user-facing change):

Add opt-in discovery of peer EPP replicas via the EPP Service's EndpointSlices,
enabled with the --enable-peer-discovery and --epp-peer-service flags. 
Will enable cross-replica state synchronization for active-active EPP and has no
effect unless enabled. 
When enabled, the EPP requires get/list/watch on discovery.k8s.io/endpointslices:
Helm charts and kustomize manifests update RBAC policies accordingly:
- Helm is gated behind --set router.epp.peerDiscovery.enabled=true
- Kustomize provides overlays

@evacchi
evacchi requested a review from a team as a code owner August 10, 2026 09:55
@evacchi
evacchi requested review from ahg-g and elevran and a lite review from Copilot August 10, 2026 09:55
@github-actions github-actions Bot added kind/feature Categorizes issue or PR as related to a new feature. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Aug 10, 2026
@evacchi evacchi changed the title refactor: EPP peer discovery via EndpointSlices feat: EPP peer discovery via EndpointSlices Aug 10, 2026
@github-actions github-actions Bot added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/feature Categorizes issue or PR as related to a new feature. labels Aug 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds an opt-in, Kubernetes-native mechanism for discovering peer EPP replicas by watching the EPP Service’s EndpointSlice objects, storing the live peer set for future cross-replica state synchronization work.

Changes:

  • Introduce EPPPeerReconciler (controller-runtime) to aggregate ready endpoints across EndpointSlices, exclude self, and emit add/update/delete via a PeerNotifier.
  • Add PeerMetadata/PeerStore abstractions and an in-memory MemoryPeerStore, plus unit + envtest-style integration coverage.
  • Wire feature gating via --enable-peer-discovery + --epp-peer-service, and extend RBAC to allow get/list/watch on discovery.k8s.io/endpointslices.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/integration/epp/epp_peer_discovery_integration_test.go Integration test exercising the reconciler against envtest by manually creating/updating/deleting EndpointSlices.
pkg/epp/statesync/peerstore.go Adds MemoryPeerStore implementation for storing live peer membership.
pkg/epp/statesync/peerstore_test.go Unit tests for MemoryPeerStore upsert/delete semantics and deterministic ordering.
pkg/epp/server/runserver.go Wires peer discovery reconciler into the server runner when enabled; instantiates the in-memory peer store.
pkg/epp/server/options.go Adds CLI flags and validation for opt-in peer discovery configuration.
pkg/epp/server/options_test.go Adds validation test ensuring peer discovery requires specifying the peer Service.
pkg/epp/server/controller_manager.go Scopes the controller-runtime cache for EndpointSlices to the pool namespace + Service label when enabled.
pkg/epp/server/controller_config.go Extends controller config to carry the peer Service name for cache scoping.
pkg/epp/framework/interface/datalayer/peer.go Introduces peer discovery interfaces/types (PeerMetadata, PeerStore, PeerNotifier).
pkg/epp/controller/setup_test.go Adds a test asserting peer discovery is never leader-election gated.
pkg/epp/controller/epp_peer_reconciler.go Implements the EndpointSlice-based peer discovery reconciler and diffing against prior peer state.
pkg/epp/controller/epp_peer_reconciler_test.go Unit tests for peer aggregation, self-exclusion, diffing behavior, and ID fallback logic.
deploy/components/inference-gateway/rbac.yaml Adds EndpointSlice read permissions to the kustomize component Role.
config/charts/routerlib/templates/_rbac.yaml Adds EndpointSlice read permissions to the Helm Role template.
cmd/epp/runner/runner.go Plumbs enablement/config into controller config + runner, and captures POD_IP for self-exclusion.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +52 to +54
- apiGroups: ["discovery.k8s.io"]
resources: ["endpointslices"]
verbs: ["get", "watch", "list"]
Comment thread deploy/components/inference-gateway/rbac.yaml Outdated
Comment thread test/integration/epp/epp_peer_discovery_integration_test.go Outdated
Comment thread pkg/epp/server/options.go
//
// Peer discovery (active-active state synchronization).
//
EnablePeerDiscovery bool // Enables discovery of peer EPP replicas. Requires PeerServiceName.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this flag, i mean isnt PeerServiceName enough to enable the feature

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah debatable, I added that because implicitly enabling when PeerServiceName is present seemed brittle, but I am not strongly opinionated

// endpointReady reports whether an endpoint is serving. A nil Ready condition is
// treated as not ready.
func endpointReady(ep *discoveryv1.Endpoint) bool {
return ep.Conditions.Ready != nil && *ep.Conditions.Ready

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should not check serving instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

incorrect doc string, Ready is when it is serving and not terminating, so I think Ready should be the appropriate one (you don't use a peer that's serving but about to terminate) -- updated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, the one in termination should be skipped

Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
@evacchi
evacchi force-pushed the epp-peer-discovery branch from 5a15125 to b793749 Compare August 10, 2026 15:35
Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
@github-actions github-actions Bot added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/feature Categorizes issue or PR as related to a new feature. labels Aug 10, 2026
…verlay)

Signed-off-by: Edoardo Vacchi <evacchi@users.noreply.github.com>
@github-actions github-actions Bot added kind/feature Categorizes issue or PR as related to a new feature. and removed kind/feature Categorizes issue or PR as related to a new feature. labels Aug 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/feature Categorizes issue or PR as related to a new feature. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants