Skip to content

Commit 272d28f

Browse files
cache: bug fix to requested resource does not exist handling
we need to keep watch active for when resource is finally created so we can notify client. #219 Signed-off-by: staticfinalzero <[email protected]>
1 parent eaca1a4 commit 272d28f

File tree

2 files changed

+61
-4
lines changed

2 files changed

+61
-4
lines changed

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

+3-4
Original file line numberDiff line numberDiff line change
@@ -377,10 +377,9 @@ protected void respondWithSpecificOrder(T group,
377377
version);
378378
}
379379

380-
respond(watch, snapshot, group);
381-
382-
// Discard the watch. A new watch will be created for future snapshots once envoy ACKs the response.
383-
return true;
380+
// If we respond with an actual message, we can proceed to discard the watch.
381+
// A new watch will be created for future snapshots once envoy ACKs the response.
382+
return respond(watch, snapshot, group);
384383
}
385384

386385
// Do not discard the watch. The request version is the same as the snapshot version, so we wait to respond.

cache/src/test/java/io/envoyproxy/controlplane/cache/v3/SimpleCacheTest.java

+58
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,64 @@ public void watchIsLeftOpenIfNotRespondedImmediately() {
476476
assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
477477
}
478478

479+
@Test
480+
public void watchIsLeftOpenIfNotRespondedImmediatelyAndThroughSubsequentSetEmptySnapshots() {
481+
SimpleCache<String> cache = new SimpleCache<>(new SingleNodeGroup());
482+
cache.setSnapshot(SingleNodeGroup.GROUP, Snapshot.create(
483+
ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), VERSION1));
484+
485+
ResponseTracker responseTracker = new ResponseTracker();
486+
Watch watch = cache.createWatch(
487+
true,
488+
XdsRequest.create(DiscoveryRequest.newBuilder()
489+
.setNode(Node.getDefaultInstance())
490+
.setTypeUrl(ROUTE_TYPE_URL)
491+
.addAllResourceNames(Collections.singleton(ROUTE_NAME))
492+
.build()),
493+
Collections.singleton(ROUTE_NAME),
494+
responseTracker);
495+
496+
assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
497+
assertThat(cache.statusInfo(SingleNodeGroup.GROUP).numWatches()).isEqualTo(1);
498+
499+
cache.setSnapshot(SingleNodeGroup.GROUP, Snapshot.create(
500+
ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), VERSION2));
501+
502+
assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
503+
assertThat(cache.statusInfo(SingleNodeGroup.GROUP).numWatches()).isEqualTo(1);
504+
}
505+
506+
@Test
507+
public void watchIsLeftOpenIfNotRespondedImmediatelyAndLaterSetSnapshotSendsUpdate() {
508+
SimpleCache<String> cache = new SimpleCache<>(new SingleNodeGroup());
509+
cache.setSnapshot(SingleNodeGroup.GROUP, Snapshot.create(
510+
ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), VERSION1));
511+
512+
ResponseTracker responseTracker = new ResponseTracker();
513+
Watch watch = cache.createWatch(
514+
true,
515+
XdsRequest.create(DiscoveryRequest.newBuilder()
516+
.setNode(Node.getDefaultInstance())
517+
.setTypeUrl(ROUTE_TYPE_URL)
518+
.addAllResourceNames(SNAPSHOT1.resources(ROUTE_TYPE_URL).keySet())
519+
.build()),
520+
Collections.emptySet(),
521+
responseTracker);
522+
523+
assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
524+
assertThat(cache.statusInfo(SingleNodeGroup.GROUP).numWatches()).isEqualTo(1);
525+
526+
cache.setSnapshot(SingleNodeGroup.GROUP, Snapshot.create(
527+
ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), ImmutableList.of(), VERSION2));
528+
529+
assertThatWatchIsOpenWithNoResponses(new WatchAndTracker(watch, responseTracker));
530+
assertThat(cache.statusInfo(SingleNodeGroup.GROUP).numWatches()).isEqualTo(1);
531+
532+
cache.setSnapshot(SingleNodeGroup.GROUP, SNAPSHOT1);
533+
534+
assertThatWatchReceivesSnapshot(new WatchAndTracker(watch, responseTracker), SNAPSHOT1);
535+
}
536+
479537
@Test
480538
public void getSnapshot() {
481539
SimpleCache<String> cache = new SimpleCache<>(new SingleNodeGroup());

0 commit comments

Comments
 (0)