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

fix(discovery): when endpoint_slices updated, old endpoints all flush_all/flush_expired #11577

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

dongjiang1989
Copy link
Contributor

@dongjiang1989 dongjiang1989 commented Sep 13, 2024

Description

Fixes # (endpoint muti slices)

Bad case:
If the number of nodes of a certain service is too large, for example, it is split into two endpoint slices:
slice1: There are pod1 and pod2.
slice2: There are pod3 and pod4.
If the pod is re-scheduled and only slice1 or slice2 is updated, the upstream will only be updated to pod1 + pod2 or pod3 + pod4.

When watch_endpoint_slices_schema is true(endpoint_slice model), remove informer.pre_list and informer.post_list

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@dosubot dosubot bot added size:XS This PR changes 0-9 lines, ignoring generated files. bug Something isn't working labels Sep 13, 2024
@dongjiang1989 dongjiang1989 changed the title Fix(discovery): fixendpoint slices flush_all/flush_expired bug Fix(discovery): when endpoint_slices updated, old endpoints all flush_all/flush_expired Sep 13, 2024
@dongjiang1989 dongjiang1989 changed the title Fix(discovery): when endpoint_slices updated, old endpoints all flush_all/flush_expired fix(discovery): when endpoint_slices updated, old endpoints all flush_all/flush_expired Sep 13, 2024
Signed-off-by: dongjiang1989 <[email protected]>
Copy link
Contributor

@shreemaan-abhishek shreemaan-abhishek left a comment

Choose a reason for hiding this comment

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

please add test cases and explain the changes in detail and how it solves the issue.

@dongjiang1989
Copy link
Contributor Author

please add test cases and explain the changes in detail and how it solves the issue.

Thanks @shreemaan-abhishek .
Add the changes done.

@dongjiang1989
Copy link
Contributor Author

/ok-to-test

@shreemaan-abhishek
Copy link
Contributor

please add test cases

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:XS This PR changes 0-9 lines, ignoring generated files. labels Oct 22, 2024
@dongjiang1989
Copy link
Contributor Author

/ok-to-test

@dongjiang1989
Copy link
Contributor Author

please add test cases

Thanks @shreemaan-abhishek .
Add e2e cases done.

Signed-off-by: dongjiang1989 <[email protected]>
Signed-off-by: dongjiang1989 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants