[sotw][issue-540] Return full state when applicable for watches in linear cache #856
Conversation
…ches subscribing to multiple resources Properly return the request in sotw responses to allow proper handling in callbacks Signed-off-by: Valerian Roche <[email protected]>
…near cache Signed-off-by: Valerian Roche <[email protected]>
valerian-roche
force-pushed
the
vr/resource-returned
branch
from
d6ec809 to
998c937
Compare
| @@ -327,8 +341,12 @@ func (cache *LinearCache) CreateWatch(request *Request, _ stream.StreamState, va | |||
| case err != nil: | |||
| stale = true | |||
| staleResources = request.GetResourceNames() | |||
| cache.log.Debugf("Watch is stale as version failed to parse %s", err.Error()) | |||
There was a problem hiding this comment.
nit: add a colon to separate error from the string: Watch is stale as version failed to parse: %s
Also I see some log lines are prefixed with [linear cache], you may want to add that here and at L348, too.
pkg/cache/v3/resource.go
Outdated
| // for some types. This is relied on by xds-grpc which is explicitly requesting clusters but expect | ||
| // to receive all existing resources |
There was a problem hiding this comment.
In a way, yes, this is a specificity of xDS gRPC, because xDS gRPC uses resource names with LDS and CDS, whereas IIUC, envoy does not and instead uses wildcards. For cases where Envoy does (ODCDS?) then I don't know how resource deletions are handled.
I would add here that for gRPC clients, not including a resource in LDS or CDS indicates that the resource has been deleted, as described at https://www.envoyproxy.io/docs/envoy/v1.28.0/api-docs/xds_protocol#deleting-resources.
Co-authored-by: Antoine Tollenaere <[email protected]> Signed-off-by: Valerian Roche <[email protected]>
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
|
This pull request has been automatically closed because it has not had activity in the last 37 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
This PR is currently based on #854, and include its diff too, but in a separate commit
Fixes #540
This PR is fixing issue #540: the xds protocol defines that for the resource types
ListenerandCluster, when using sotw watches, the cache must return as part of the response all existing resources matching the requests if not wildcard.Currently the control-plane will only return the modified resources on cache updates, and the protocol states that the client should interpret such a response as a deletion
In simple cache this distinction is not applicable as we currently always return all resources. While we may want to improve this behavior in the future (e.g. to avoid sending all clusters if only one was updated in delta, or to send a single route configuration in sotw on update), this is out of scope of this PR as this requires keeping more state