-
Notifications
You must be signed in to change notification settings - Fork 688
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
Added support for endpointslices #5798
base: master
Are you sure you want to change the base?
Added support for endpointslices #5798
Conversation
025c59e
to
59c79e3
Compare
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.
@ajaychoudhary-hotstar, many thanks for diving into this! The mechanics of how you're dealing with EndpointSlices look reasonable to me, but I need to make your life a little harder, I'm afraid -- I've run into at least one case where there were no EndpointSlices but there were Endpoints, so I think we should probably look at EndpointSlices if present and Endpoints if not. Does that sound sane?
Signed-off-by: ajaychoudhary-hotstar <[email protected]>
Signed-off-by: ajaychoudhary-hotstar <[email protected]>
Signed-off-by: ajaychoudhary-hotstar <[email protected]>
Signed-off-by: ajaychoudhary-hotstar <[email protected]>
1c6e7fd
to
90cabf7
Compare
Thanks, @kflynn Have added endpoints as fallback. |
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.
@ajaychoudhary-hotstar, I think this looks good, many thanks! Let me sort out CI for it...
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.
Nice, and thanks for the PR! 🙂 🚀
Signed-off-by: ajaychoudhary-hotstar <[email protected]>
Signed-off-by: ajaychoudhary-hotstar <[email protected]>
Description
Currently Ingress is doing service discovery from endpoint objects, there is is hard limit of 1K IP address that k8s endpoint objects can hold which breaks load balancing if some backends scale more than 1K.
Related Issues
(#5167)
Testing
Tested in lower env, scaled the backend system to 1.2K pods, and noticed the same 1.2K in backend metrics envoy_cluster_membership_total.
Checklist
Does my change need to be backported to a previous release?
I made sure to update
CHANGELOG.md
.Remember, the CHANGELOG needs to mention:
This is unlikely to impact how the Ambassador performs at scale.
Remember, things that might have an impact at scale include:
My change is adequately tested.
Remember when considering testing:
I updated
CONTRIBUTING.md
with any special dev tricks I had to use to work on this code efficiently.The changes in this PR have been reviewed for security concerns and adherence to security best practices.