Fixes loss of updates after multiple on-demand vhds requests

[dmitri-d]

Please see #12158 for more details about the issue.

[dmitri-d]

The idea of this PR is to watch on prefixes used in on-demand vhds. On subscription start a watch is created using route config name as the prefix. New requests update a watch with an alias, which is removed when a reply is received; the watch retains the prefix at all times, however. Any spontaneous update for an already resolved vhost, or an update that adds a new vhost will be handled by a watch that matches the prefix in resource names contained in the update.

This bit is ugly and done to prevent sending of prefixes instead of resource names on subscription start: https://github.com/envoyproxy/envoy/pull/12819/files#diff-050ecb76fd3d458fd208a27e7c1a4e23R141. Instead, we send an empty request. This has implications on server ability to respond with a non-empty initial state.

This needs more tests.

ping @htuch, @stevenzzzz

[dmitri-d]

Also ping @chaoqin-li1123

[htuch]

@dmitri-d thanks for working on this bug. I'm trying to grok the logic here, the main question I have is why do we need to go through the alias subscribe/unsubscribe dance for new resources when we don't need to do this for resource updates? If we're going to propagate all prefix matched updates to the VHDS provider, can't the VHDS provider maintain all the alias information locally? I.e. we can get watches out of the business of reasoning about aliases.

[htuch]

@chaoqin-li1123 some variant of this might be useful for glob matching when subscribing to glob collections in the work you are doing. For udpa:// URLs, it's not a prefix on the path component (ignoring context parameters, which should be exact match).

[dmitri-d]

the main question I have is why do we need to go through the alias subscribe/unsubscribe dance for new resources when we don't need to do this for resource updates?

The thought did occur to me; delta xds is tightly coupled with watch_map and subscription-state, the dance you are referring to is a way to create xds requests without changing watch-map and subscription-state interfaces. Let me see if I can skip creation of an alias watch without changing the interfaces much.

[dmitri-d]

• added requestOnDemandUpdate() grpc-mux interface that bypasses watch-map completely

@htuch: is this what you had in mind?

[htuch]

@dmitri-d yes, exactly, this is a really nice simplification. Can you clean up the PR for review, e.g. comments, and get CI passing? I'll give a detailed review then, but this approach looks like the right stepping stone for glob collections and fixes the VHDS issue.
/wait

[dmitri-d]

@htuch: I think this is ready for a review.

[dmitri-d]

• responded to feedback

[dmitri-d]

ping @htuch

[htuch]

LGTM. @chaoqin-li1123 can you take a pass to see if this aligns with your plans around udpa:// support?

[htuch]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[htuch]

LGTM, thanks!