Skip to content

don't remove xds watch if we didn't respond #586

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.

Sign up for GitHub

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

Closed
andy-fong wants to merge 1 commit into from
Closed

don't remove xds watch if we didn't respond #586

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 9 additions & 6 deletions pkg/api/v1/control-plane/cache/simple.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,10 +168,12 @@ func (cache *snapshotCache) SetSnapshot(node string, snapshot Snapshot) {
// empty resource - intended behavior is for the resources to be cleared
if resources != nil {
// snapshot has been initialized and exists
cache.respond(watch.Request, watch.Response, resources, version)

// discard the watch
watches.Delete(pi)
if cache.respond(watch.Request, watch.Response, resources, version) {
// discard the watch
watches.Delete(pi)
} else if cache.log != nil {
cache.log.Debugf("skipped deleting watch %v", watch.Request.ResourceNames)
}
}
}
}
Expand Down Expand Up @@ -306,15 +308,15 @@ func (cache *snapshotCache) cancelWatch(nodeID string, watchIndex PriorityIndex)

// Respond to a watch with the snapshot value. The value channel should have capacity not to block.
// TODO(kuat) do not respond always, see issue https://github.com/envoyproxy/go-control-plane/issues/46
func (cache *snapshotCache) respond(request Request, value chan Response, resources map[string]Resource, version string) {
func (cache *snapshotCache) respond(request Request, value chan Response, resources map[string]Resource, version string) bool {
// for ADS, the request names must match the snapshot names
// if they do not, then the watch is never responded, and it is expected that envoy makes another request
if len(request.ResourceNames) != 0 && cache.ads {
if err := Superset(nameSet(request.ResourceNames), resources); err != nil {
if cache.log != nil {
cache.log.Debugf("ADS mode: not responding to request: %v", err)
}
return
return false
}
}
if cache.log != nil {
Expand All @@ -325,6 +327,7 @@ func (cache *snapshotCache) respond(request Request, value chan Response, resour
tag.Insert(KeyType, request.GetTypeUrl()),
}, MResponses.M(1))
value <- createResponse(request, resources, version)
return true
}

func createResponse(request Request, resources map[string]Resource, version string) Response {
Expand Down