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

Design: Endpoint slice support #5703

Conversation

clayton-gonsalves
Copy link
Contributor

Design proposal to address #2696

Signed-off-by: Clayton Gonsalves <clayton.gonsalves@reddit.com>
@clayton-gonsalves clayton-gonsalves requested a review from a team as a code owner August 28, 2023 13:09
@clayton-gonsalves clayton-gonsalves requested review from tsaarni and stevesloka and removed request for a team August 28, 2023 13:09
@clayton-gonsalves clayton-gonsalves added the kind/design Categorizes issue or PR as related to design. label Aug 28, 2023
@clayton-gonsalves clayton-gonsalves requested review from skriss, sunjayBhatia and a team and removed request for tsaarni and stevesloka August 28, 2023 13:09
@clayton-gonsalves clayton-gonsalves added the release-note/none-required Marks a PR as not requiring a release note. Should only be used for very small changes. label Aug 28, 2023
@codecov
Copy link

codecov bot commented Aug 28, 2023

Codecov Report

Merging #5703 (45a8a96) into main (68bafab) will decrease coverage by 0.04%.
Report is 24 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5703      +/-   ##
==========================================
- Coverage   78.60%   78.56%   -0.04%     
==========================================
  Files         138      138              
  Lines       19155    19163       +8     
==========================================
  Hits        15056    15056              
- Misses       3812     3820       +8     
  Partials      287      287              

see 2 files with indirect coverage changes

Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

Thanks for the design doc @clayton-gonsalves, apologies for the delay in reviewing. Added a few comments/questions.

design/endpoint-slice-support.md Outdated Show resolved Hide resolved
design/endpoint-slice-support.md Show resolved Hide resolved
design/endpoint-slice-support.md Show resolved Hide resolved
design/endpoint-slice-support.md Show resolved Hide resolved
design/endpoint-slice-support.md Show resolved Hide resolved
design/endpoint-slice-support.md Show resolved Hide resolved
Signed-off-by: Clayton Gonsalves <clayton.gonsalves@reddit.com>
Copy link
Member

@skriss skriss left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @clayton-gonsalves

Copy link
Member

@tsaarni tsaarni left a comment

Choose a reason for hiding this comment

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

Since the processing of EndpointSlices and Endpoints would be mutually exclusive maybe it could be mentioned that Endpoints where user has disabled mirroring will not be processed by Contour.

Signed-off-by: Clayton Gonsalves <clayton.gonsalves@reddit.com>
@clayton-gonsalves
Copy link
Contributor Author

Since the processing of EndpointSlices and Endpoints would be mutually exclusive maybe it could be mentioned that Endpoints where user has disabled mirroring will not be processed by Contour.

addressed in 45a8a96

Copy link
Member

@tsaarni tsaarni left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@skriss skriss merged commit b4be1ef into projectcontour:main Sep 15, 2023
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/design Categorizes issue or PR as related to design. release-note/none-required Marks a PR as not requiring a release note. Should only be used for very small changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants