[POC] Introduce XdsPreprocessor
#6096
Draft
Conversation
This file contains 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
jrhee17
added a commit
that referenced
this pull request
Mar 27, 2025
Motivation: We are planning on deprecating `XdsEndpointGroup` and introducing `XdsPreprocessor` at #6096. Previously, `XdsEndpointGroup` created sub-`EndpointGroup`s which consume resources. (e.g. `HealthCheckedEndpointGroup`, `DnsEndpointGroup`, etc..) assuming that all xDS clients will share a common `EndpointGroup`. However, `XdsPreprocessor` cannot be shared among all client types. For instance, thrift clients will need to use `RpcPreprocessor` whereas web clients will need to use normal `Preprocessor`s. I propose that `XdsBootstrap` contains a single `ClusterManager` which caches all `EndpointGroup`s derived from `Cluster`s. This `ClusterManager` is aligned with the xDS API [cluster_manager](https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/bootstrap/v3/bootstrap.proto#envoy-v3-api-msg-config-bootstrap-v3-clustermanager) and is aligned with the [upstream implementation](https://github.com/envoyproxy/envoy/blob/e3a97f1a81a65b168c5cf822d4a0861958f94162/source/common/upstream/cluster_manager_impl.h#L243) as well. One prerequisite for the introduction of `ClusterManager` is the ability to cache `Cluster`s. This is difficult since the current implementation of `ClusterSnapshot` also contains a reference to the containing `Route`. https://github.com/line/armeria/blob/533f798efdf827326c3a8d70ce21d13f20245c20/xds/src/main/java/com/linecorp/armeria/xds/ClusterSnapshot.java#L82 I propose that `VirtualHostSnapshot` and `RouteEntry` is introduced, and snapshot mappings are unidirectional. This direction aligns with a possible future implementation of [VHDS](https://www.envoyproxy.io/docs/envoy/latest/configuration/http/http_conn_man/vhds) as well. As this is a refactor of the previous implementation, there is no expected change in behavior except for subset load balancing (which actually matches upstream now) Modifications: - Introduced `VirtualHostSnapshot` which is equivalent to `VirtualHost`, and `RouteEntry` which is equivalent to `Route`. - Refactored `ClusterManager` and `SubsetLoadBalancer` to not use `ClusterSnapshot#route` - `SubsetLoadBalancer` now is aligned with the upstream implementation ([ref](https://github.com/envoyproxy/envoy/blob/e3a97f1a81a65b168c5cf822d4a0861958f94162/source/extensions/load_balancing_policies/subset/subset_lb.cc#L478)) - The previous test case for skipping failure on empty metadata is removed as it does not match the upstream implementation. - (Misc) Cleaned up the `ConfigSourceMapper` implementation by removing unused fields and the corresponding logic. Result: - The `ClusterSnapshot` API is prepared for the introduction of a central `ClusterManager` maintained by `XdsBootstrapImpl` <!-- Visit this URL to learn more about how to write a pull request description: https://armeria.dev/community/developer-guide#how-to-write-pull-request-description -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
Explain why you're making this change and what problem you're trying to solve.
Modifications:
Result: