-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add cluster extension #6118
Add cluster extension #6118
Conversation
…sters Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
…nto add-cluster-extension Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
…-extension Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very excited for this. @htuch can you do a quick API pass on this to see if this will meet Google needs? @envoyproxy/maintainers in case anyone else wants to take a look. @HenryYYang I will take a full pass once the function is deleted per offline discussion. Thank you!
/wait
@mattklein123 this seems general enough to be useful for anything we'd want to do in terms of customer clusters. OTOH, I think our main interest today is making LB more extensible, which is orthogonal to cluster extensibility. I.e. we'd like to push out the existing LB to extensions and add our own, without needing to change from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks this is great! Flushing some more comments.
/wait
Signed-off-by: Henry Yang <[email protected]>
🔨 |
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks this is really excellent work. A few small changes and then I think we are ready to ship!
/wait
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
… header file Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
Signed-off-by: Henry Yang <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fantastic work! Thank you @HenryYYang!
@HenryYYang when you do your next PR for the new extension cluster, can you also take a look at https://github.com/envoyproxy/envoy/blob/master/REPO_LAYOUT.md and update it? I think we might be missing a few extensions, not just the clusters. Thank you!
For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md
Description: Add cluster factory to allow creating and registering custom cluster.
Risk Level: medium
Testing: Added unit tests and integration tests
Docs Changes:
Release Notes: Added.
#5780