Skip to content

Commit be16610

Browse files
authored
Introduce virtual host snapshots (#6105)
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 -->
1 parent 3ef8c77 commit be16610

28 files changed

+646
-344
lines changed

xds/src/main/java/com/linecorp/armeria/xds/ClusterResourceNode.java

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import static com.google.common.base.Strings.isNullOrEmpty;
2020
import static com.linecorp.armeria.xds.XdsType.CLUSTER;
21-
import static java.util.Objects.requireNonNull;
2221

2322
import java.util.Objects;
2423

@@ -28,16 +27,10 @@
2827
import io.envoyproxy.envoy.config.cluster.v3.Cluster.EdsClusterConfig;
2928
import io.envoyproxy.envoy.config.core.v3.ConfigSource;
3029
import io.envoyproxy.envoy.config.endpoint.v3.ClusterLoadAssignment;
31-
import io.envoyproxy.envoy.config.route.v3.Route;
32-
import io.envoyproxy.envoy.config.route.v3.VirtualHost;
3330
import io.grpc.Status;
3431

3532
final class ClusterResourceNode extends AbstractResourceNodeWithPrimer<ClusterXdsResource> {
3633

37-
@Nullable
38-
private final VirtualHost virtualHost;
39-
@Nullable
40-
private final Route route;
4134
private final int index;
4235
private final EndpointSnapshotWatcher snapshotWatcher = new EndpointSnapshotWatcher();
4336
private final SnapshotWatcher<ClusterSnapshot> parentWatcher;
@@ -49,19 +42,15 @@ final class ClusterResourceNode extends AbstractResourceNodeWithPrimer<ClusterXd
4942
ResourceNodeType resourceNodeType) {
5043
super(xdsBootstrap, configSource, CLUSTER, resourceName, primer, parentWatcher, resourceNodeType);
5144
this.parentWatcher = parentWatcher;
52-
virtualHost = null;
53-
route = null;
5445
index = -1;
5546
}
5647

5748
ClusterResourceNode(@Nullable ConfigSource configSource,
5849
String resourceName, XdsBootstrapImpl xdsBootstrap,
59-
@Nullable RouteXdsResource primer, SnapshotWatcher<ClusterSnapshot> parentWatcher,
60-
VirtualHost virtualHost, Route route, int index, ResourceNodeType resourceNodeType) {
50+
@Nullable VirtualHostXdsResource primer, SnapshotWatcher<ClusterSnapshot> parentWatcher,
51+
int index, ResourceNodeType resourceNodeType) {
6152
super(xdsBootstrap, configSource, CLUSTER, resourceName, primer, parentWatcher, resourceNodeType);
6253
this.parentWatcher = parentWatcher;
63-
this.virtualHost = requireNonNull(virtualHost, "virtualHost");
64-
this.route = requireNonNull(route, "route");
6554
this.index = index;
6655
}
6756

@@ -86,7 +75,8 @@ public void doOnChanged(ClusterXdsResource resource) {
8675
children().add(node);
8776
xdsBootstrap().subscribe(node);
8877
} else {
89-
parentWatcher.snapshotUpdated(new ClusterSnapshot(resource));
78+
final ClusterSnapshot clusterSnapshot = new ClusterSnapshot(resource);
79+
parentWatcher.snapshotUpdated(clusterSnapshot);
9080
}
9181
}
9282

@@ -100,8 +90,8 @@ public void snapshotUpdated(EndpointSnapshot newSnapshot) {
10090
if (!Objects.equals(newSnapshot.xdsResource().primer(), current)) {
10191
return;
10292
}
103-
parentWatcher.snapshotUpdated(
104-
new ClusterSnapshot(current, newSnapshot, virtualHost, route, index));
93+
final ClusterSnapshot clusterSnapshot = new ClusterSnapshot(current, newSnapshot, index);
94+
parentWatcher.snapshotUpdated(clusterSnapshot);
10595
}
10696

10797
@Override

xds/src/main/java/com/linecorp/armeria/xds/ClusterRoot.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public final class ClusterRoot extends AbstractRoot<ClusterSnapshot> {
3939
if (cluster != null) {
4040
node = staticCluster(xdsBootstrap, resourceName, this, cluster);
4141
} else {
42-
final ConfigSource configSource = configSourceMapper.cdsConfigSource(null, resourceName);
42+
final ConfigSource configSource = configSourceMapper.cdsConfigSource(resourceName);
4343
node = new ClusterResourceNode(configSource, resourceName, xdsBootstrap,
4444
null, this, ResourceNodeType.DYNAMIC);
4545
xdsBootstrap.subscribe(node);

xds/src/main/java/com/linecorp/armeria/xds/ClusterSnapshot.java

Lines changed: 6 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
import com.linecorp.armeria.common.annotation.UnstableApi;
2424

2525
import io.envoyproxy.envoy.config.cluster.v3.Cluster;
26-
import io.envoyproxy.envoy.config.route.v3.Route;
27-
import io.envoyproxy.envoy.config.route.v3.VirtualHost;
2826

2927
/**
3028
* A snapshot of a {@link Cluster} resource.
@@ -34,27 +32,17 @@ public final class ClusterSnapshot implements Snapshot<ClusterXdsResource> {
3432
private final ClusterXdsResource clusterXdsResource;
3533
@Nullable
3634
private final EndpointSnapshot endpointSnapshot;
37-
@Nullable
38-
private final VirtualHost virtualHost;
39-
@Nullable
40-
private final Route route;
4135
private final int index;
4236

43-
ClusterSnapshot(ClusterXdsResource clusterXdsResource, EndpointSnapshot endpointSnapshot,
44-
@Nullable VirtualHost virtualHost, @Nullable Route route, int index) {
37+
ClusterSnapshot(ClusterXdsResource clusterXdsResource,
38+
@Nullable EndpointSnapshot endpointSnapshot, int index) {
4539
this.clusterXdsResource = clusterXdsResource;
4640
this.endpointSnapshot = endpointSnapshot;
47-
this.virtualHost = virtualHost;
48-
this.route = route;
4941
this.index = index;
5042
}
5143

5244
ClusterSnapshot(ClusterXdsResource clusterXdsResource) {
53-
this.clusterXdsResource = clusterXdsResource;
54-
endpointSnapshot = null;
55-
virtualHost = null;
56-
route = null;
57-
index = -1;
45+
this(clusterXdsResource, null, -1);
5846
}
5947

6048
@Override
@@ -70,22 +58,6 @@ public EndpointSnapshot endpointSnapshot() {
7058
return endpointSnapshot;
7159
}
7260

73-
/**
74-
* The {@link VirtualHost} this {@link Cluster} belongs to.
75-
*/
76-
@Nullable
77-
public VirtualHost virtualHost() {
78-
return virtualHost;
79-
}
80-
81-
/**
82-
* The {@link Route} this {@link Cluster} belongs to.
83-
*/
84-
@Nullable
85-
public Route route() {
86-
return route;
87-
}
88-
8961
int index() {
9062
return index;
9163
}
@@ -99,15 +71,13 @@ public boolean equals(Object object) {
9971
return false;
10072
}
10173
final ClusterSnapshot that = (ClusterSnapshot) object;
102-
return index == that.index && Objects.equal(clusterXdsResource, that.clusterXdsResource) &&
103-
Objects.equal(endpointSnapshot, that.endpointSnapshot) &&
104-
Objects.equal(virtualHost, that.virtualHost) &&
105-
Objects.equal(route, that.route);
74+
return Objects.equal(clusterXdsResource, that.clusterXdsResource) &&
75+
Objects.equal(endpointSnapshot, that.endpointSnapshot);
10676
}
10777

10878
@Override
10979
public int hashCode() {
110-
return Objects.hashCode(clusterXdsResource, endpointSnapshot, virtualHost, route, index);
80+
return Objects.hashCode(clusterXdsResource, endpointSnapshot);
11181
}
11282

11383
@Override
@@ -116,9 +86,6 @@ public String toString() {
11686
.omitNullValues()
11787
.add("clusterXdsResource", clusterXdsResource)
11888
.add("endpointSnapshot", endpointSnapshot)
119-
.add("virtualHost", virtualHost)
120-
.add("route", route)
121-
.add("index", index)
12289
.toString();
12390
}
12491
}

xds/src/main/java/com/linecorp/armeria/xds/CompositeXdsStream.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ final class CompositeXdsStream implements XdsStream {
3434
CompositeXdsStream(GrpcClientBuilder clientBuilder, Node node, Backoff backoff,
3535
EventExecutor eventLoop, XdsResponseHandler handler,
3636
SubscriberStorage subscriberStorage) {
37-
for (XdsType type: XdsType.values()) {
37+
for (XdsType type: XdsType.discoverableTypes()) {
3838
final SotwXdsStream stream = new SotwXdsStream(
3939
SotwDiscoveryStub.basic(type, clientBuilder), node, backoff, eventLoop,
4040
handler, subscriberStorage, EnumSet.of(type));

xds/src/main/java/com/linecorp/armeria/xds/ConfigSourceMapper.java

Lines changed: 3 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ final class ConfigSourceMapper {
3232
@Nullable
3333
private final ConfigSource bootstrapAdsConfig;
3434
@Nullable
35-
private ConfigSource parentConfigSource;
35+
private final ConfigSource parentConfigSource;
3636

3737
ConfigSourceMapper(Bootstrap bootstrap) {
3838
this(bootstrap, null);
@@ -78,18 +78,7 @@ ConfigSource edsConfigSource(@Nullable ConfigSource configSource, String resourc
7878
throw new IllegalArgumentException("Cannot find an EDS config source for " + resourceName);
7979
}
8080

81-
ConfigSource cdsConfigSource(@Nullable ConfigSource configSource, String resourceName) {
82-
if (configSource != null) {
83-
if (configSource.hasApiConfigSource()) {
84-
return configSource;
85-
}
86-
if (configSource.hasSelf() && parentConfigSource != null) {
87-
return parentConfigSource;
88-
}
89-
if (configSource.hasAds() && bootstrapAdsConfig != null) {
90-
return bootstrapAdsConfig;
91-
}
92-
}
81+
ConfigSource cdsConfigSource(String resourceName) {
9382
if (bootstrapCdsConfig != null && bootstrapCdsConfig.hasApiConfigSource()) {
9483
return bootstrapCdsConfig;
9584
}
@@ -114,18 +103,7 @@ ConfigSource rdsConfigSource(@Nullable ConfigSource configSource, String resourc
114103
throw new IllegalArgumentException("Cannot find an RDS config source for route: " + resourceName);
115104
}
116105

117-
ConfigSource ldsConfigSource(@Nullable ConfigSource configSource, String resourceName) {
118-
if (configSource != null) {
119-
if (configSource.hasApiConfigSource()) {
120-
return configSource;
121-
}
122-
if (configSource.hasSelf() && parentConfigSource != null) {
123-
return parentConfigSource;
124-
}
125-
if (configSource.hasAds() && bootstrapAdsConfig != null) {
126-
return bootstrapAdsConfig;
127-
}
128-
}
106+
ConfigSource ldsConfigSource(String resourceName) {
129107
if (bootstrapLdsConfig != null && bootstrapLdsConfig.hasApiConfigSource()) {
130108
return bootstrapLdsConfig;
131109
}

xds/src/main/java/com/linecorp/armeria/xds/ListenerRoot.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public final class ListenerRoot extends AbstractRoot<ListenerSnapshot> {
3939
node = new ListenerResourceNode(null, resourceName, xdsBootstrap, this, ResourceNodeType.STATIC);
4040
node.onChanged(listenerXdsResource);
4141
} else {
42-
final ConfigSource configSource = configSourceMapper.ldsConfigSource(null, resourceName);
42+
final ConfigSource configSource = configSourceMapper.ldsConfigSource(resourceName);
4343
node = new ListenerResourceNode(configSource, resourceName, xdsBootstrap,
4444
this, ResourceNodeType.DYNAMIC);
4545
xdsBootstrap.subscribe(node);
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Copyright 2025 LY Corporation
3+
*
4+
* LINE Corporation licenses this file to you under the Apache License,
5+
* version 2.0 (the "License"); you may not use this file except in compliance
6+
* with the License. You may obtain a copy of the License at:
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
12+
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
13+
* License for the specific language governing permissions and limitations
14+
* under the License.
15+
*/
16+
17+
package com.linecorp.armeria.xds;
18+
19+
import java.util.Objects;
20+
21+
import com.google.common.base.MoreObjects;
22+
23+
import com.linecorp.armeria.common.annotation.Nullable;
24+
25+
import io.envoyproxy.envoy.config.route.v3.Route;
26+
import io.envoyproxy.envoy.config.route.v3.RouteAction;
27+
28+
/**
29+
* Represents a {@link Route}.
30+
*/
31+
public final class RouteEntry {
32+
33+
private final Route route;
34+
@Nullable
35+
private final ClusterSnapshot clusterSnapshot;
36+
37+
RouteEntry(Route route, @Nullable ClusterSnapshot clusterSnapshot) {
38+
this.route = route;
39+
this.clusterSnapshot = clusterSnapshot;
40+
}
41+
42+
/**
43+
* The {@link Route}.
44+
*/
45+
public Route route() {
46+
return route;
47+
}
48+
49+
/**
50+
* The {@link ClusterSnapshot} that is represented by {@link RouteAction#getCluster()}.
51+
* If the {@link RouteAction} does not reference a cluster, the returned value may be {@code null}.
52+
*/
53+
@Nullable
54+
public ClusterSnapshot clusterSnapshot() {
55+
return clusterSnapshot;
56+
}
57+
58+
@Override
59+
public boolean equals(Object o) {
60+
if (this == o) {
61+
return true;
62+
}
63+
if (o == null || getClass() != o.getClass()) {
64+
return false;
65+
}
66+
final RouteEntry that = (RouteEntry) o;
67+
return Objects.equals(route, that.route) &&
68+
Objects.equals(clusterSnapshot, that.clusterSnapshot);
69+
}
70+
71+
@Override
72+
public int hashCode() {
73+
return Objects.hash(route, clusterSnapshot);
74+
}
75+
76+
@Override
77+
public String toString() {
78+
return MoreObjects.toStringHelper(this)
79+
.add("route", route)
80+
.add("clusterSnapshot", clusterSnapshot)
81+
.toString();
82+
}
83+
}

0 commit comments

Comments
 (0)