Support distinct endpoints and Pod/Node metadata mapping for xDS K8s endpoints - #1326
Conversation
…endpoints Motivation: The xDS control plane converts Kubernetes service endpoints into Envoy ClusterLoadAssignment resources, but had two gaps: in NODE_PORT mode multiple Pods on the same node produce duplicate LbEndpoints, and LbEndpoint.metadata was never populated, so users could not drive Envoy subset load balancing from Kubernetes Pod/Node labels or annotations. Modifications: - Add `distinct_endpoint` to `ServiceEndpointWatcher`; when true, endpoints that share the same host:port are collapsed into a single LbEndpoint. - Add `metadata_mapping` (repeated `MetadataMapping`) to `ServiceEndpointWatcher`. Each rule copies a Pod/Node label or annotation into `LbEndpoint.metadata`, selecting an exact `source_key` or a `source_key_prefix` (original keys kept), under a configurable `metadata_namespace` (default `envoy.lb`) and `metadata_key`. - Add `KubernetesEndpointConverter` centralizing endpoint-to-LbEndpoint conversion (dedup + metadata) via Armeria `KubernetesResourceAccess`. - Route both build sites (background fetching service and preview) through the converter, removing duplicated conversion code. - Validate metadata mappings on create/update, rejecting malformed rules with INVALID_ARGUMENT. - Add tests for exact/prefix metadata copy, custom namespace, annotations, endpoint dedup, and validation errors. Result: Users can deduplicate Kubernetes endpoints and carry Pod/Node labels and annotations as Envoy endpoint metadata by configuring the watcher. Both fields are optional; existing configurations are unaffected.
71667a3 to
a8f1ed0
Compare
📝 WalkthroughWalkthroughAdds Kubernetes endpoint metadata mapping support: new ChangesKubernetes endpoint metadata mapping
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant XdsKubernetesService
participant KubernetesEndpointConverter
participant KubernetesResourceAccess
participant EnvoyLbEndpoint
XdsKubernetesService->>XdsKubernetesService: validateMetadataMappings(watcher)
XdsKubernetesService->>KubernetesEndpointConverter: addLbEndpoints(builder, endpoints, watcher)
loop each Kubernetes endpoint
KubernetesEndpointConverter->>KubernetesEndpointConverter: skip if no ports / dedupe by host:port
KubernetesEndpointConverter->>KubernetesResourceAccess: objectMeta(endpoint, resourceType)
KubernetesResourceAccess-->>KubernetesEndpointConverter: Pod/Node ObjectMeta
KubernetesEndpointConverter->>KubernetesEndpointConverter: buildMetadata(metadataMappingList, objectMeta)
KubernetesEndpointConverter->>EnvoyLbEndpoint: build LbEndpoint(address, metadata)
end
KubernetesEndpointConverter-->>XdsKubernetesService: populated LocalityLbEndpoints
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
xds/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointConverter.java (1)
86-136: 🚀 Performance & Scalability | 🔵 TrivialMinor: repeated Pod/Node lookups when multiple mappings target the same resource.
objectMeta(endpoint, mapping.getResourceType())is invoked once perMetadataMapping; if several mappings target the sameresource_type, the same Pod/Node lookup is repeated. Caching per resource type per endpoint (e.g. a smallEnumMap) would avoid the redundant calls.🤖 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/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointConverter.java` around lines 86 - 136, buildMetadata repeatedly calls objectMeta(endpoint, mapping.getResourceType()) for every MetadataMapping, causing redundant Pod/Node lookups when multiple mappings share the same resource type. Cache the ObjectMeta per resource type within buildMetadata, using a small per-endpoint map keyed by mapping.getResourceType(), and reuse the cached value inside the loop while preserving the current metadata assembly logic.xds/src/main/proto/centraldogma/xds/k8s/v1/xds_kubernetes.proto (1)
129-139: 📐 Maintainability & Code Quality | 🔵 TrivialOptional: prefix enum values with the enum name.
POD/NODEandLABEL/ANNOTATIONshare C++/Python scope with sibling enums in the same message per protobuf style guide; prefixing (e.g.RESOURCE_TYPE_POD) avoids future name collisions if more enums are added toMetadataMapping.🤖 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/src/main/proto/centraldogma/xds/k8s/v1/xds_kubernetes.proto` around lines 129 - 139, Prefix the enum values in MetadataMapping’s ResourceType and EntryType to avoid shared C++/Python scope collisions with sibling enums; update the definitions of ResourceType and EntryType so POD/NODE and LABEL/ANNOTATION use enum-name-prefixed identifiers consistently. Keep the zero-value unspecified members aligned with the same naming pattern, and make sure any generated references in xds_kubernetes.proto are updated to the new enum symbols.xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointMetadataTest.java (1)
200-221: 📐 Maintainability & Code Quality | 🔵 TrivialConsider covering update-path validation too.
rejectInvalidMetadataMappingonly exercisescreateAggregator;updateKubernetesEndpointAggregatorgoes through the samevalidateKubernetesEndpointAndPush/validateMetadataMappingscode path, so a corresponding update-rejection test would round out coverage, though the risk is low given shared implementation.🤖 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/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointMetadataTest.java` around lines 200 - 221, Add update-path coverage for invalid metadata mapping validation in KubernetesEndpointMetadataTest, since rejectInvalidMetadataMapping only verifies createAggregator. Reuse the existing invalid MetadataMapping cases and assert that updateKubernetesEndpointAggregator (through ServiceEndpointWatcher / watcher setup and validateKubernetesEndpointAndPush) also returns BAD_REQUEST with INVALID_ARGUMENT, so the shared validateMetadataMappings path is exercised for updates too.
🤖 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/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointConverter.java`:
- Around line 86-136: buildMetadata repeatedly calls objectMeta(endpoint,
mapping.getResourceType()) for every MetadataMapping, causing redundant Pod/Node
lookups when multiple mappings share the same resource type. Cache the
ObjectMeta per resource type within buildMetadata, using a small per-endpoint
map keyed by mapping.getResourceType(), and reuse the cached value inside the
loop while preserving the current metadata assembly logic.
In `@xds/src/main/proto/centraldogma/xds/k8s/v1/xds_kubernetes.proto`:
- Around line 129-139: Prefix the enum values in MetadataMapping’s ResourceType
and EntryType to avoid shared C++/Python scope collisions with sibling enums;
update the definitions of ResourceType and EntryType so POD/NODE and
LABEL/ANNOTATION use enum-name-prefixed identifiers consistently. Keep the
zero-value unspecified members aligned with the same naming pattern, and make
sure any generated references in xds_kubernetes.proto are updated to the new
enum symbols.
In
`@xds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointMetadataTest.java`:
- Around line 200-221: Add update-path coverage for invalid metadata mapping
validation in KubernetesEndpointMetadataTest, since rejectInvalidMetadataMapping
only verifies createAggregator. Reuse the existing invalid MetadataMapping cases
and assert that updateKubernetesEndpointAggregator (through
ServiceEndpointWatcher / watcher setup and validateKubernetesEndpointAndPush)
also returns BAD_REQUEST with INVALID_ARGUMENT, so the shared
validateMetadataMappings path is exercised for updates too.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 10bdf480-5dad-4f2f-a88c-ffbc361d224f
📒 Files selected for processing (5)
xds/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointConverter.javaxds/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesEndpointFetchingService.javaxds/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesService.javaxds/src/main/proto/centraldogma/xds/k8s/v1/xds_kubernetes.protoxds/src/test/java/com/linecorp/centraldogma/xds/k8s/v1/KubernetesEndpointMetadataTest.java
Motivation:
The xDS control plane converts Kubernetes service endpoints into Envoy ClusterLoadAssignment resources, but had two gaps: in NODE_PORT mode multiple Pods on the same node produce duplicate LbEndpoints, and LbEndpoint.metadata was never populated, so users could not drive Envoy subset load balancing from Kubernetes Pod/Node labels or annotations.
Modifications:
distinct_endpointtoServiceEndpointWatcher; when true, endpoints that share the same host:port are collapsed into a single LbEndpoint.metadata_mapping(repeatedMetadataMapping) toServiceEndpointWatcher. Each rule copies a Pod/Node label or annotation intoLbEndpoint.metadata, selecting an exactsource_keyor asource_key_prefix(original keys kept), under a configurablemetadata_namespace(defaultenvoy.lb) andmetadata_key.KubernetesEndpointConvertercentralizing endpoint-to-LbEndpoint conversion (dedup + metadata) via ArmeriaKubernetesResourceAccess.Result:
Users can deduplicate Kubernetes endpoints and carry Pod/Node labels and annotations as Envoy endpoint metadata by configuring the watcher. Both fields are optional; existing configurations are unaffected.