Skip to content
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.

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

Provide trigger to proactively push resources to the client #1

Merged
merged 2 commits into from
Jul 26, 2022

Conversation

valerian-roche
Copy link
Collaborator

@valerian-roche valerian-roche commented Jul 22, 2022

This PR is adding a feature we use to workaround an issue in envoy:

  • when a cluster is updated in envoy (e.g. during a cds update), if the load-balancing or connection part is touched, envoy creates a new cluster and requests endpoints for the given eds key (same is applicable if a new cluster is added)
  • if the eds key has already been requested by this client, a bug in the delta-ads implementation on envoy side will drop the request without returning any endpoint. As a consequence, the new cluster (or the updated one) will be left with no endpoints and envoy will log an error (mentioning a CLA stream timeout)

To work-around this issue, we are adding an extension allowing the user to trigger a "force" push of resources when a response is sent. This force-push has the given properties:

  • the push can be for the same or another type. If for the same type user should be aware some resources may end up being sent twice in two consecutive type responses
  • the push can refer to multiple resources but only one type. Attempting to call it multiple time should work but is definitely not guaranteed
  • the response with those resources will be sent after the current response. This is a guarantee and is critical for this workaround to work
  • the resources will be sent if and only if they are subscribed to (either explicitly or through wildcard)
  • if at the time of the trigger there is no watch for the given type, the request for a push is kept and will be applied on the first request of the type. It means initialVersions for the resource will be ignored if set, and it will therefore always be returned if known

@valerian-roche valerian-roche changed the title Vr/trigger Provide trigger to proactively push resources to the client Jul 22, 2022
Copy link

@s-matyukevich s-matyukevich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, The description of the similar PR in the old fork has more useful context IMO, so I want to reference it here for future reviewers valerian-roche#3 (comment)

@atollena
Copy link

Small suggestion to ease following what's going on for reviewers:

This PR is adding a feature we use to workaround envoyproxy/envoy#13009 in envoy.

rather than just "a bug". There's a lot of context in that thread.

@valerian-roche valerian-roche merged commit 73555a8 into DataDog:dd/main Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants