Add xds-kubernetes module for Kubernetes-based xDS endpoint discovery - #6914
Open
jrhee17 wants to merge 4 commits into
Open
Add xds-kubernetes module for Kubernetes-based xDS endpoint discovery#6914jrhee17 wants to merge 4 commits into
xds-kubernetes module for Kubernetes-based xDS endpoint discovery#6914jrhee17 wants to merge 4 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds a Kubernetes xDS cluster module. It defines cluster configuration, discovers Kubernetes endpoints, maps them to Envoy assignments, registers the extension, and validates POD-mode routing with mocked Kubernetes resources. It also changes Kubernetes client factory shutdown to asynchronous closure. ChangesKubernetes xDS integration
Kubernetes client shutdown
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant XdsBootstrap
participant KubernetesClusterTypeFactory
participant KubernetesAPI
participant KubernetesEndpointMapper
participant Backend
XdsBootstrap->>KubernetesClusterTypeFactory: createEndpointStream(cluster config)
KubernetesClusterTypeFactory->>KubernetesAPI: watch service endpoints
KubernetesAPI-->>KubernetesClusterTypeFactory: endpoint updates
KubernetesClusterTypeFactory->>KubernetesEndpointMapper: map endpoints
KubernetesEndpointMapper-->>KubernetesClusterTypeFactory: ClusterLoadAssignment
KubernetesClusterTypeFactory-->>XdsBootstrap: EndpointSnapshot
XdsBootstrap->>Backend: route /hello
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
xds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/KubernetesClusterTypeFactory.java (1)
164-169: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMap the endpoint mode with an explicit
switch.The
elsebranch maps every value other thanNODE_PORTtoPOD. This includesUNRECOGNIZEDand any mode added to the proto later. A new proto mode would silently resolve toPOD.Use a
switchon the proto enum and reject unknown values. A static import or a local alias also removes the fully qualified reference on Line 165.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/KubernetesClusterTypeFactory.java` around lines 164 - 169, Replace the if/else mapping in KubernetesClusterTypeFactory with an explicit switch over config.getMode(), mapping NODE_PORT and POD directly to their corresponding builder modes and rejecting UNRECOGNIZED or any unsupported future values instead of defaulting to POD. Use a static import or local alias for KubernetesEndpointMode to remove the fully qualified reference.it/xds-client/src/test/java/com/linecorp/armeria/xds/it/KubernetesClusterTypeIntegrationTest.java (2)
86-96: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the discovered Kubernetes endpoints.
The mapper lambda on Line 88 ignores the
endpointsparameter and always returns a fixedClusterLoadAssignmentthat points atbackendServer. The assertion on Line 96 therefore passes whenever any endpoint update fires, regardless of what theKubernetesEndpointGroupdiscovered.The test does not verify the pod IP
10.0.0.1created on Line 107, thePODmode, or the service-to-pod resolution. Those are the behaviors this integration test is meant to cover.Capture the received
endpointsin the mapper and assert their content.💚 Proposed change to assert discovered endpoints
`@Test` void basicEndpointDiscovery() { final Bootstrap bootstrap = bootstrapYaml(client.getMasterUrl().toString()); + final BlockingQueue<List<Endpoint>> discovered = new LinkedBlockingQueue<>(); final KubernetesClusterTypeFactory factory = KubernetesClusterTypeFactory.of( client.getConfiguration(), - (clusterName, endpoints) -> backendCla(clusterName)); + (clusterName, endpoints) -> { + discovered.add(ImmutableList.copyOf(endpoints)); + return backendCla(clusterName); + }); try (XdsBootstrap xdsBootstrap = XdsBootstrap.builder(bootstrap) .extensionFactories(factory) .build(); XdsHttpPreprocessor preprocessor = XdsHttpPreprocessor.ofListener("listener1", xdsBootstrap)) { final BlockingWebClient webClient = WebClient.of(preprocessor).blocking(); assertThat(webClient.get("/hello").contentUtf8()).isEqualTo("world"); } + await().untilAsserted(() -> assertThat(discovered) + .anySatisfy(endpoints -> assertThat(endpoints) + .extracting(Endpoint::host) + .contains("10.0.0.1"))); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/KubernetesClusterTypeIntegrationTest.java` around lines 86 - 96, Update the KubernetesClusterTypeFactory mapper in the XdsBootstrap setup to capture the discovered endpoints instead of ignoring the endpoints parameter, and assert that the received KubernetesEndpointGroup data resolves the POD-mode service to pod IP 10.0.0.1. Ensure the test validates the discovered endpoint content before or alongside the existing /hello assertion, rather than always returning the fixed backendCla result.
100-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSet the namespace explicitly on the created resources.
createK8sResourcescreates Deployment, Service, and Pod without a namespace, while the bootstrap YAML expectsnamespace: test. Call.inNamespace("test")on each resource operation, or set the namespace in the correspondingObjectMeta, so the test does not depend on the mock client default.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/KubernetesClusterTypeIntegrationTest.java` around lines 100 - 108, Update createK8sResources to explicitly create the Deployment, Service, and Pod in the "test" namespace by applying inNamespace("test") to each resource operation, or by setting that namespace in each resource's ObjectMeta; do not rely on the mock client default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@xds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/DefaultKubernetesEndpointMapper.java`:
- Around line 41-44: Update the public map method in
DefaultKubernetesEndpointMapper to call Objects.requireNonNull on both
clusterName and endpoints, using each parameter name as its message, before
creating the LocalityLbEndpoints builder or processing endpoints.
- Around line 40-41: Annotate the public override method map in
DefaultKubernetesEndpointMapper with `@UnstableApi`, since the containing class is
not annotated and no exception applies.
In
`@xds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/KubernetesClusterTypeFactory.java`:
- Around line 132-142: Move the newConfigBuilder(config) call inside the
switchMapEager callback so each GenericSecretSnapshot emission creates a fresh
ConfigBuilder. Apply withOauthToken only when secretSnapshot.credential() is
non-null, then pass that per-emission builder to createSnapshot; keep the
no-credential path creating a builder once for its single snapshot.
---
Nitpick comments:
In
`@it/xds-client/src/test/java/com/linecorp/armeria/xds/it/KubernetesClusterTypeIntegrationTest.java`:
- Around line 86-96: Update the KubernetesClusterTypeFactory mapper in the
XdsBootstrap setup to capture the discovered endpoints instead of ignoring the
endpoints parameter, and assert that the received KubernetesEndpointGroup data
resolves the POD-mode service to pod IP 10.0.0.1. Ensure the test validates the
discovered endpoint content before or alongside the existing /hello assertion,
rather than always returning the fixed backendCla result.
- Around line 100-108: Update createK8sResources to explicitly create the
Deployment, Service, and Pod in the "test" namespace by applying
inNamespace("test") to each resource operation, or by setting that namespace in
each resource's ObjectMeta; do not rely on the mock client default.
In
`@xds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/KubernetesClusterTypeFactory.java`:
- Around line 164-169: Replace the if/else mapping in
KubernetesClusterTypeFactory with an explicit switch over config.getMode(),
mapping NODE_PORT and POD directly to their corresponding builder modes and
rejecting UNRECOGNIZED or any unsupported future values instead of defaulting to
POD. Use a static import or local alias for KubernetesEndpointMode to remove the
fully qualified reference.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4b14f239-1c98-455d-b69b-155558c538b5
📒 Files selected for processing (14)
it/xds-client/build.gradleit/xds-client/src/test/java/com/linecorp/armeria/xds/it/KubernetesClusterTypeIntegrationTest.javasettings.gradlexds-api/src/main/proto/armeria/xds/kubernetes/kubernetes_cluster_config.protoxds-kubernetes/build.gradlexds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/DefaultKubernetesEndpointMapper.javaxds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/KubernetesClusterTypeFactory.javaxds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/KubernetesClusterTypeFactoryProvider.javaxds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/KubernetesEndpointMapper.javaxds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/KubernetesTypeRegistryPackageProvider.javaxds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/package-info.javaxds-kubernetes/src/main/resources/META-INF/services/com.linecorp.armeria.xds.XdsExtensionFactoryProviderxds-kubernetes/src/main/resources/META-INF/services/com.linecorp.armeria.xds.XdsTypeRegistryPackageProviderxds-kubernetes/src/test/java/com/linecorp/armeria/xds/kubernetes/DefaultKubernetesEndpointMapperTest.java
Contributor
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
xds-kubernetes/src/test/java/com/linecorp/armeria/xds/kubernetes/DefaultKubernetesEndpointMapperTest.java (1)
80-93: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover the same host with different ports.
All test inputs use port
30000. A mapper that deduplicates by host only would still pass this test. Add the same host with a second port and assert that both host-port pairs remain while the exact duplicate is removed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xds-kubernetes/src/test/java/com/linecorp/armeria/xds/kubernetes/DefaultKubernetesEndpointMapperTest.java` around lines 80 - 93, Update the deduplication test around DefaultKubernetesEndpointMapper.get().map to include the same host with a second port alongside the exact duplicate. Assert that both distinct host-port endpoints remain and the exact duplicate is removed, verifying deduplication uses the complete host-port pair rather than host alone.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@xds-kubernetes/src/test/java/com/linecorp/armeria/xds/kubernetes/DefaultKubernetesEndpointMapperTest.java`:
- Around line 80-93: Update the deduplication test around
DefaultKubernetesEndpointMapper.get().map to include the same host with a second
port alongside the exact duplicate. Assert that both distinct host-port
endpoints remain and the exact duplicate is removed, verifying deduplication
uses the complete host-port pair rather than host alone.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6bcacd3b-ae56-4a92-9c4f-3e6c33075990
📒 Files selected for processing (2)
xds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/DefaultKubernetesEndpointMapper.javaxds-kubernetes/src/test/java/com/linecorp/armeria/xds/kubernetes/DefaultKubernetesEndpointMapperTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- xds-kubernetes/src/main/java/com/linecorp/armeria/xds/kubernetes/DefaultKubernetesEndpointMapper.java
jrhee17
marked this pull request as ready for review
August 11, 2026 02:55
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6914 +/- ##
============================================
- Coverage 74.46% 0 -74.47%
============================================
Files 1963 0 -1963
Lines 82437 0 -82437
Branches 10764 0 -10764
============================================
- Hits 61385 0 -61385
+ Misses 15918 0 -15918
+ Partials 5134 0 -5134 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Tick the box to add this pull request to the merge queue (same as
|
minwoox
approved these changes
Aug 11, 2026
| * with equal weight, priority 0, and | ||
| * {@link io.envoyproxy.envoy.config.core.v3.HealthStatus#HEALTHY HEALTHY} status. | ||
| */ | ||
| static KubernetesEndpointMapper ofDefault() { |
| import io.envoyproxy.envoy.config.endpoint.v3.LbEndpoint; | ||
| import io.envoyproxy.envoy.config.endpoint.v3.LocalityLbEndpoints; | ||
|
|
||
| final class DefaultKubernetesEndpointMapper implements KubernetesEndpointMapper { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
Users running xDS-based service discovery in Kubernetes environments have no built-in way to resolve xDS clusters using Kubernetes endpoints directly. This module bridges
KubernetesEndpointGroup(which watches K8s Pods/Services) with the xDSClusterTypeFactoryinterface (which producesEndpointSnapshotfromClusterLoadAssignmentprotobufs).Modifications:
xds-api/src/main/proto/armeria/xds/kubernetes/kubernetes_cluster_config.protodefiningKubernetesClusterConfigmessage andKubernetesEndpointModeenumxds-kubernetesmodule with:KubernetesClusterTypeFactory— aClusterTypeFactoryimplementation (armeria.cluster.kubernetes) that unpacksKubernetesClusterConfigfrom the cluster'styped_config, creates aKubernetesEndpointGroup, and bridges endpoint updates toSnapshotStream<EndpointSnapshot>. Supports optional SDS credential for K8s API authentication with automatic recreation on secret rotation.KubernetesEndpointMapper— a@FunctionalInterfacefor convertingList<Endpoint>toClusterLoadAssignment, withofDefault()providing a default mapping (single locality, equal weight, HEALTHY status)Result: