Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce type parameters where applicable for xDS classes. #5410

Merged
merged 7 commits into from
Jan 30, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import io.envoyproxy.envoy.config.core.v3.ConfigSource;
import io.grpc.Status;

abstract class AbstractResourceNode<T> implements ResourceNode<AbstractResourceHolder> {
abstract class AbstractResourceNode<T extends XdsResource> implements ResourceNode<T> {

private final Deque<ResourceNode<?>> children = new ArrayDeque<>();

Expand All @@ -33,21 +33,18 @@ abstract class AbstractResourceNode<T> implements ResourceNode<AbstractResourceH
private final ConfigSource configSource;
private final XdsType type;
private final String resourceName;
@Nullable
private final ResourceHolder primer;
private final SnapshotWatcher<? super T> parentWatcher;
private final SnapshotWatcher<?> parentWatcher;
private final ResourceNodeType resourceNodeType;
@Nullable
private AbstractResourceHolder current;
private T current;

AbstractResourceNode(XdsBootstrapImpl xdsBootstrap, @Nullable ConfigSource configSource,
XdsType type, String resourceName, @Nullable ResourceHolder primer,
SnapshotWatcher<? super T> parentWatcher, ResourceNodeType resourceNodeType) {
XdsType type, String resourceName,
SnapshotWatcher<?> parentWatcher, ResourceNodeType resourceNodeType) {
this.xdsBootstrap = xdsBootstrap;
this.configSource = configSource;
this.type = type;
this.resourceName = resourceName;
this.primer = primer;
this.parentWatcher = parentWatcher;
this.resourceNodeType = resourceNodeType;
}
Expand All @@ -56,12 +53,17 @@ XdsBootstrapImpl xdsBootstrap() {
return xdsBootstrap;
}

private void setCurrent(@Nullable AbstractResourceHolder current) {
@Override
public ConfigSource configSource() {
return configSource;
}

private void setCurrent(@Nullable T current) {
this.current = current;
}

@Override
public AbstractResourceHolder currentResourceHolder() {
public T currentResource() {
return current;
}

Expand All @@ -82,10 +84,8 @@ public void onResourceDoesNotExist(XdsType type, String resourceName) {
}

@Override
public final void onChanged(AbstractResourceHolder update) {
public void onChanged(T update) {
assert update.type() == type();

update = update.withPrimer(primer);
setCurrent(update);

final Deque<ResourceNode<?>> prevChildren = new ArrayDeque<>(children);
Expand All @@ -98,7 +98,7 @@ public final void onChanged(AbstractResourceHolder update) {
}
}

abstract void doOnChanged(ResourceHolder update);
abstract void doOnChanged(T update);

@Override
public void close() {
Expand All @@ -107,18 +107,14 @@ public void close() {
}
children.clear();
if (resourceNodeType == ResourceNodeType.DYNAMIC) {
xdsBootstrap.unsubscribe(configSource, this);
xdsBootstrap.unsubscribe(this);
}
}

Deque<ResourceNode<?>> children() {
return children;
}

SnapshotWatcher<? super T> parentWatcher() {
return parentWatcher;
}

@Override
public XdsType type() {
return type;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* Copyright 2023 LINE Corporation
*
* LINE Corporation licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/

package com.linecorp.armeria.xds;

import com.linecorp.armeria.common.annotation.Nullable;

import io.envoyproxy.envoy.config.core.v3.ConfigSource;

abstract class AbstractResourceNodeWithPrimer<T extends XdsResourceWithPrimer<T>>
extends AbstractResourceNode<T> {

@Nullable
private final XdsResource primer;

AbstractResourceNodeWithPrimer(XdsBootstrapImpl xdsBootstrap, @Nullable ConfigSource configSource,
XdsType type, String resourceName, @Nullable XdsResource primer,
SnapshotWatcher<?> parentWatcher, ResourceNodeType resourceNodeType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're going to try to make everything type-safe, shouldn't parentWatcher also be handled?

Suggested change
SnapshotWatcher<?> parentWatcher, ResourceNodeType resourceNodeType) {
SnapshotWatcher<? extends Snapshot<T>> parentWatcher, ResourceNodeType resourceNodeType) {

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the type is not used in this class and its parent class, I didn't add it. But let me add it for clarity. 😉

super(xdsBootstrap, configSource, type, resourceName, parentWatcher, resourceNodeType);
this.primer = primer;
}

@Override
public void onChanged(T update) {
assert update.type() == type();
super.onChanged(update.withPrimer(primer));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import io.grpc.Status;
import io.netty.util.concurrent.EventExecutor;

abstract class AbstractRoot<T extends Snapshot<? extends ResourceHolder>>
abstract class AbstractRoot<T extends Snapshot<? extends XdsResource>>
implements SnapshotWatcher<T>, SafeCloseable {

private static final Logger logger = LoggerFactory.getLogger(AbstractRoot.class);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,21 @@ final class BootstrapApiConfigs {
}
}

ConfigSource remapConfigSource(XdsType type, @Nullable ConfigSource configSource,
String resourceName) {
ConfigSource configSource(XdsType type, String resourceName, ResourceNode<?> node) {
if (type == XdsType.LISTENER) {
return ldsConfigSource(configSource);
return ldsConfigSource(node);
} else if (type == XdsType.ROUTE) {
return rdsConfigSource(configSource, resourceName);
return rdsConfigSource(node, resourceName);
} else if (type == XdsType.CLUSTER) {
return cdsConfigSource(configSource, resourceName);
return cdsConfigSource(node, resourceName);
} else {
assert type == XdsType.ENDPOINT;
return edsConfigSource(configSource, resourceName);
return edsConfigSource(node, resourceName);
}
}

ConfigSource edsConfigSource(@Nullable ConfigSource configSource, String resourceName) {
ConfigSource edsConfigSource(ResourceNode<?> node, String resourceName) {
final ConfigSource configSource = node.configSource();
if (configSource != null && configSource.hasApiConfigSource()) {
return configSource;
}
Expand All @@ -74,7 +74,8 @@ ConfigSource edsConfigSource(@Nullable ConfigSource configSource, String resourc
throw new IllegalArgumentException("Cannot find an EDS config source for " + resourceName);
}

ConfigSource cdsConfigSource(@Nullable ConfigSource configSource, String resourceName) {
ConfigSource cdsConfigSource(ResourceNode<?> node, String resourceName) {
final ConfigSource configSource = node.configSource();
if (configSource != null && configSource.hasApiConfigSource()) {
return configSource;
}
Expand All @@ -90,7 +91,8 @@ ConfigSource cdsConfigSource(@Nullable ConfigSource configSource, String resourc
throw new IllegalArgumentException("Cannot find a CDS config source for route: " + resourceName);
}

ConfigSource rdsConfigSource(@Nullable ConfigSource configSource, String resourceName) {
ConfigSource rdsConfigSource(ResourceNode<?> node, String resourceName) {
final ConfigSource configSource = node.configSource();
if (configSource != null && configSource.hasApiConfigSource()) {
return configSource;
}
Expand All @@ -100,7 +102,8 @@ ConfigSource rdsConfigSource(@Nullable ConfigSource configSource, String resourc
throw new IllegalArgumentException("Cannot find an RDS config source for route: " + resourceName);
}

ConfigSource ldsConfigSource(@Nullable ConfigSource configSource) {
ConfigSource ldsConfigSource(ResourceNode<?> node) {
final ConfigSource configSource = node.configSource();
if (configSource != null && configSource.hasApiConfigSource()) {
return configSource;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
import io.envoyproxy.envoy.config.cluster.v3.Cluster;
import io.grpc.Status;

final class BootstrapClusters implements SnapshotWatcher<Snapshot<?>> {
final class BootstrapClusters implements SnapshotWatcher<ClusterSnapshot> {

private final Map<String, ClusterSnapshot> clusterSnapshots = new HashMap<>();

Expand All @@ -46,10 +46,8 @@ final class BootstrapClusters implements SnapshotWatcher<Snapshot<?>> {
}

@Override
public void snapshotUpdated(Snapshot<?> newSnapshot) {
assert newSnapshot instanceof ClusterSnapshot;
final ClusterSnapshot clusterSnapshot = (ClusterSnapshot) newSnapshot;
clusterSnapshots.put(clusterSnapshot.holder().name(), clusterSnapshot);
public void snapshotUpdated(ClusterSnapshot newSnapshot) {
clusterSnapshots.put(newSnapshot.xdsResource().name(), newSnapshot);
}

@Nullable
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,84 +30,82 @@
import io.envoyproxy.envoy.config.route.v3.VirtualHost;
import io.grpc.Status;

final class ClusterResourceNode extends AbstractResourceNode<ClusterSnapshot> {
final class ClusterResourceNode extends AbstractResourceNodeWithPrimer<ClusterXdsResource> {

@Nullable
private final VirtualHost virtualHost;
@Nullable
private final Route route;
private final int index;
private final EndpointSnapshotWatcher snapshotWatcher = new EndpointSnapshotWatcher();
private final SnapshotWatcher<ClusterSnapshot> parentWatcher;

ClusterResourceNode(@Nullable ConfigSource configSource,
String resourceName, XdsBootstrapImpl xdsBootstrap,
@Nullable ResourceHolder primer, SnapshotWatcher<? super ClusterSnapshot> parentWatcher,
@Nullable RouteXdsResource primer,
SnapshotWatcher<ClusterSnapshot> parentWatcher,
ResourceNodeType resourceNodeType) {
super(xdsBootstrap, configSource, CLUSTER, resourceName, primer, parentWatcher, resourceNodeType);
this.parentWatcher = parentWatcher;
virtualHost = null;
route = null;
index = -1;
}

ClusterResourceNode(@Nullable ConfigSource configSource,
String resourceName, XdsBootstrapImpl xdsBootstrap,
@Nullable ResourceHolder primer, SnapshotWatcher<ClusterSnapshot> parentWatcher,
@Nullable RouteXdsResource primer, SnapshotWatcher<ClusterSnapshot> parentWatcher,
VirtualHost virtualHost, Route route, int index, ResourceNodeType resourceNodeType) {
super(xdsBootstrap, configSource, CLUSTER, resourceName, primer, parentWatcher, resourceNodeType);
this.parentWatcher = parentWatcher;
this.virtualHost = requireNonNull(virtualHost, "virtualHost");
this.route = requireNonNull(route, "route");
this.index = index;
}

@Override
public void doOnChanged(ResourceHolder update) {
final ClusterResourceHolder holder = (ClusterResourceHolder) update;
final Cluster cluster = holder.resource();
public void doOnChanged(ClusterXdsResource resource) {
final Cluster cluster = resource.resource();
if (cluster.hasLoadAssignment()) {
final ClusterLoadAssignment loadAssignment = cluster.getLoadAssignment();
final EndpointResourceNode node =
StaticResourceUtils.staticEndpoint(xdsBootstrap(), cluster.getName(),
holder, snapshotWatcher, loadAssignment);
resource, snapshotWatcher, loadAssignment);
children().add(node);
} else if (cluster.hasEdsClusterConfig()) {
final ConfigSource configSource = cluster.getEdsClusterConfig().getEdsConfig();
final EndpointResourceNode node =
new EndpointResourceNode(configSource, cluster.getName(), xdsBootstrap(), holder,
new EndpointResourceNode(configSource, cluster.getName(), xdsBootstrap(), resource,
snapshotWatcher, ResourceNodeType.DYNAMIC);
children().add(node);
xdsBootstrap().subscribe(configSource, node);
xdsBootstrap().subscribe(node);
} else {
parentWatcher().snapshotUpdated(new ClusterSnapshot(holder));
parentWatcher.snapshotUpdated(new ClusterSnapshot(resource));
}
}

@Override
public ClusterResourceHolder currentResourceHolder() {
return (ClusterResourceHolder) super.currentResourceHolder();
}

private class EndpointSnapshotWatcher implements SnapshotWatcher<EndpointSnapshot> {
@Override
public void snapshotUpdated(EndpointSnapshot newSnapshot) {
final ClusterResourceHolder current = currentResourceHolder();
final ClusterXdsResource current = currentResource();
if (current == null) {
return;
}
if (!Objects.equals(newSnapshot.holder().primer(), current)) {
if (!Objects.equals(newSnapshot.xdsResource().primer(), current)) {
return;
}
parentWatcher().snapshotUpdated(
parentWatcher.snapshotUpdated(
new ClusterSnapshot(current, newSnapshot, virtualHost, route, index));
}

@Override
public void onError(XdsType type, Status status) {
parentWatcher().onError(type, status);
parentWatcher.onError(type, status);
}

@Override
public void onMissing(XdsType type, String resourceName) {
parentWatcher().onMissing(type, resourceName);
parentWatcher.onMissing(type, resourceName);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,38 +16,28 @@

package com.linecorp.armeria.xds;

import com.google.protobuf.Message;

import io.envoyproxy.envoy.config.cluster.v3.Cluster;
import io.envoyproxy.envoy.config.cluster.v3.Cluster.EdsClusterConfig;
import io.envoyproxy.envoy.config.cluster.v3.ClusterOrBuilder;

final class ClusterResourceParser extends ResourceParser {
final class ClusterResourceParser extends ResourceParser<ClusterXdsResource, Cluster> {

static final ClusterResourceParser INSTANCE = new ClusterResourceParser();

private ClusterResourceParser() {}

@Override
ClusterResourceHolder parse(Message message) {
if (!(message instanceof Cluster)) {
throw new IllegalArgumentException("message not type of Cluster");
}
final Cluster cluster = (Cluster) message;
final ClusterResourceHolder holder = new ClusterResourceHolder(cluster);
ClusterXdsResource parse(Cluster cluster) {
final ClusterXdsResource resource = new ClusterXdsResource(cluster);
if (cluster.hasEdsClusterConfig()) {
final EdsClusterConfig eds = cluster.getEdsClusterConfig();
XdsConverterUtil.validateConfigSource(eds.getEdsConfig());
}
return holder;
return resource;
}

@Override
String name(Message message) {
if (!(message instanceof Cluster)) {
throw new IllegalArgumentException("message not type of Cluster");
}
return ((ClusterOrBuilder) message).getName();
String name(Cluster cluster) {
return cluster.getName();
}

@Override
Expand Down
3 changes: 3 additions & 0 deletions xds/src/main/java/com/linecorp/armeria/xds/ClusterRoot.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@

package com.linecorp.armeria.xds;

import com.linecorp.armeria.common.annotation.UnstableApi;

import io.envoyproxy.envoy.config.cluster.v3.Cluster;

/**
* A root node representing a {@link Cluster}.
* Users may query the latest value of this resource or add a watcher to be notified of changes.
* Note that it is important to close this resource to avoid leaking connections to the control plane server.
*/
@UnstableApi
public final class ClusterRoot extends AbstractRoot<ClusterSnapshot> {

private final ClusterResourceNode node;
Expand Down
Loading
Loading