Skip to content

Commit c75d114

Browse files
committed
fixes
Signed-off-by: Sebastian Schepens <[email protected]>
1 parent f6b5f3b commit c75d114

File tree

4 files changed

+72
-24
lines changed

4 files changed

+72
-24
lines changed

cache/src/main/java/io/envoyproxy/controlplane/cache/Resources.java

+15-13
Original file line numberDiff line numberDiff line change
@@ -103,19 +103,21 @@ public static class V3 {
103103
ROUTE,
104104
SECRET);
105105

106-
public static final Map<String, String> V3_TYPE_URLS_TO_V2 = ImmutableMap.of(
107-
Resources.V3.CLUSTER_TYPE_URL, Resources.V2.CLUSTER_TYPE_URL,
108-
Resources.V3.ENDPOINT_TYPE_URL, Resources.V2.ENDPOINT_TYPE_URL,
109-
Resources.V3.LISTENER_TYPE_URL, Resources.V2.LISTENER_TYPE_URL,
110-
Resources.V3.ROUTE_TYPE_URL, Resources.V2.ROUTE_TYPE_URL,
111-
Resources.V3.SECRET_TYPE_URL, Resources.V2.SECRET_TYPE_URL);
112-
113-
public static final Map<String, String> V2_TYPE_URLS_TO_V3 = ImmutableMap.of(
114-
Resources.V2.CLUSTER_TYPE_URL, Resources.V3.CLUSTER_TYPE_URL,
115-
Resources.V2.ENDPOINT_TYPE_URL, Resources.V3.ENDPOINT_TYPE_URL,
116-
Resources.V2.LISTENER_TYPE_URL, Resources.V3.LISTENER_TYPE_URL,
117-
Resources.V2.ROUTE_TYPE_URL, Resources.V3.ROUTE_TYPE_URL,
118-
Resources.V2.SECRET_TYPE_URL, Resources.V3.SECRET_TYPE_URL);
106+
public static final Map<String, String> V3_TYPE_URLS_TO_V2 = ImmutableMap.<String, String>builder()
107+
.put(Resources.V3.CLUSTER_TYPE_URL, Resources.V2.CLUSTER_TYPE_URL)
108+
.put(Resources.V3.ENDPOINT_TYPE_URL, Resources.V2.ENDPOINT_TYPE_URL)
109+
.put(Resources.V3.LISTENER_TYPE_URL, Resources.V2.LISTENER_TYPE_URL)
110+
.put(Resources.V3.ROUTE_TYPE_URL, Resources.V2.ROUTE_TYPE_URL)
111+
.put(Resources.V3.SECRET_TYPE_URL, Resources.V2.SECRET_TYPE_URL)
112+
.build();
113+
114+
public static final Map<String, String> V2_TYPE_URLS_TO_V3 = ImmutableMap.<String, String>builder()
115+
.put(Resources.V2.CLUSTER_TYPE_URL, Resources.V3.CLUSTER_TYPE_URL)
116+
.put(Resources.V2.ENDPOINT_TYPE_URL, Resources.V3.ENDPOINT_TYPE_URL)
117+
.put(Resources.V2.LISTENER_TYPE_URL, Resources.V3.LISTENER_TYPE_URL)
118+
.put(Resources.V2.ROUTE_TYPE_URL, Resources.V3.ROUTE_TYPE_URL)
119+
.put(Resources.V2.SECRET_TYPE_URL, Resources.V3.SECRET_TYPE_URL)
120+
.build();
119121

120122
public static final Map<String, ResourceType> TYPE_URLS_TO_RESOURCE_TYPE =
121123
new ImmutableMap.Builder<String, ResourceType>()

cache/src/main/java/io/envoyproxy/controlplane/cache/SimpleCache.java

+51-3
Original file line numberDiff line numberDiff line change
@@ -364,10 +364,11 @@ public Collection<T> groups() {
364364
public synchronized void setSnapshot(T group, U snapshot) {
365365
// we take a writeLock to prevent watches from being created while we update the snapshot
366366
ConcurrentMap<ResourceType, CacheStatusInfo<T>> status;
367+
U previousSnapshot;
367368
writeLock.lock();
368369
try {
369370
// Update the existing snapshot entry.
370-
snapshots.put(group, snapshot);
371+
previousSnapshot = snapshots.put(group, snapshot);
371372
status = statuses.get(group);
372373
} finally {
373374
writeLock.unlock();
@@ -379,7 +380,7 @@ public synchronized void setSnapshot(T group, U snapshot) {
379380

380381
// Responses should be in specific order and typeUrls has a list of resources in the right
381382
// order.
382-
respondWithSpecificOrder(group, snapshot, status);
383+
respondWithSpecificOrder(group, previousSnapshot, snapshot, status);
383384
}
384385

385386
/**
@@ -403,7 +404,7 @@ public StatusInfo statusInfo(T group) {
403404

404405
@VisibleForTesting
405406
protected void respondWithSpecificOrder(T group,
406-
U snapshot,
407+
U previousSnapshot, U snapshot,
407408
ConcurrentMap<ResourceType, CacheStatusInfo<T>> statusMap) {
408409
for (ResourceType resourceType : RESOURCE_TYPES_IN_ORDER) {
409410
CacheStatusInfo<T> status = statusMap.get(resourceType);
@@ -435,6 +436,53 @@ protected void respondWithSpecificOrder(T group,
435436
// Do not discard the watch. The request version is the same as the snapshot version, so we wait to respond.
436437
return false;
437438
});
439+
440+
Map<String, SnapshotResource<?>> previousResources = previousSnapshot == null
441+
? Collections.emptyMap()
442+
: previousSnapshot.resources(resourceType);
443+
Map<String, SnapshotResource<?>> snapshotResources = snapshot.resources(resourceType);
444+
445+
Map<String, SnapshotResource<?>> snapshotChangedResources = snapshotResources.entrySet()
446+
.stream()
447+
.filter(entry -> {
448+
SnapshotResource<?> snapshotResource = previousResources.get(entry.getKey());
449+
return snapshotResource == null || !snapshotResource.version().equals(entry.getValue().version());
450+
})
451+
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
452+
453+
Set<String> snapshotRemovedResources = previousResources.keySet()
454+
.stream()
455+
.filter(s -> !snapshotResources.containsKey(s))
456+
.collect(Collectors.toSet());
457+
458+
status.deltaWatchesRemoveIf((id, watch) -> {
459+
String version = snapshot.version(watch.request().getResourceType(), Collections.emptyList());
460+
461+
if (!watch.version().equals(version)) {
462+
if (LOGGER.isDebugEnabled()) {
463+
LOGGER.debug("responding to open watch {}[{}] with new version {}",
464+
id,
465+
String.join(", ", watch.trackedResources().keySet()),
466+
version);
467+
}
468+
469+
List<String> removedResources = snapshotRemovedResources.stream()
470+
.filter(s -> watch.trackedResources().get(s) != null)
471+
.collect(Collectors.toList());
472+
473+
ResponseState responseState = respondDeltaTracked(watch,
474+
snapshotChangedResources,
475+
removedResources,
476+
version,
477+
group);
478+
// Discard the watch if it was responded or cancelled.
479+
// A new watch will be created for future snapshots once envoy ACKs the response.
480+
return ResponseState.RESPONDED.equals(responseState) || ResponseState.CANCELLED.equals(responseState);
481+
}
482+
483+
// Do not discard the watch. The request version is the same as the snapshot version, so we wait to respond.
484+
return false;
485+
});
438486
}
439487
}
440488

server/src/main/java/io/envoyproxy/controlplane/server/AdsDeltaDiscoveryRequestStreamObserver.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -115,10 +115,9 @@ Set<String> pendingResources(String typeUrl) {
115115

116116
@Override
117117
boolean isWildcard(String typeUrl) {
118-
return typeUrl.equals(Resources.V2.CLUSTER_TYPE_URL)
119-
|| typeUrl.equals(Resources.V3.CLUSTER_TYPE_URL)
120-
|| typeUrl.equals(Resources.V2.LISTENER_TYPE_URL)
121-
|| typeUrl.equals(Resources.V3.LISTENER_TYPE_URL);
118+
Resources.ResourceType resourceType = Resources.TYPE_URLS_TO_RESOURCE_TYPE.get(typeUrl);
119+
return Resources.ResourceType.CLUSTER.equals(resourceType)
120+
|| Resources.ResourceType.LISTENER.equals(resourceType);
122121
}
123122

124123
@Override

server/src/main/java/io/envoyproxy/controlplane/server/XdsDeltaDiscoveryRequestStreamObserver.java

+3-4
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,9 @@ public class XdsDeltaDiscoveryRequestStreamObserver<V, X, Y> extends DeltaDiscov
3434
super(defaultTypeUrl, responseObserver, streamId, executor, discoveryServer);
3535
this.trackedResources = new HashMap<>();
3636
this.pendingResources = new HashSet<>();
37-
this.isWildcard = defaultTypeUrl.equals(Resources.V2.CLUSTER_TYPE_URL)
38-
|| defaultTypeUrl.equals(Resources.V3.CLUSTER_TYPE_URL)
39-
|| defaultTypeUrl.equals(Resources.V2.LISTENER_TYPE_URL)
40-
|| defaultTypeUrl.equals(Resources.V3.LISTENER_TYPE_URL);
37+
Resources.ResourceType resourceType = Resources.TYPE_URLS_TO_RESOURCE_TYPE.get(defaultTypeUrl);
38+
this.isWildcard = Resources.ResourceType.CLUSTER.equals(resourceType)
39+
|| Resources.ResourceType.LISTENER.equals(resourceType);
4140
this.responses = new ConcurrentHashMap<>();
4241
}
4342

0 commit comments

Comments
 (0)