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

connectivity: Improve coverage of Ingress service #2414

Merged
merged 1 commit into from
May 6, 2024

Conversation

giorio94
Copy link
Member

@giorio94 giorio94 commented Mar 15, 2024

Please refer to the individual commits for additional details.

Blocked by cilium/cilium#31653

@sayboras sayboras added the needs-rebase This PR needs to be rebased because it has merge conflicts. label Mar 17, 2024
@giorio94 giorio94 force-pushed the mio/ingress-backend-service branch from efc290a to eff1aba Compare March 18, 2024 10:26
@giorio94
Copy link
Member Author

Rebased to fix conflict.

@giorio94 giorio94 removed the needs-rebase This PR needs to be rebased because it has merge conflicts. label Mar 18, 2024
@sayboras sayboras added the needs-rebase This PR needs to be rebased because it has merge conflicts. label Mar 24, 2024
There's been some back and forth [1,2] on which service should be the
backend of the ingress, as echo-other-node allows to cover the cluster
mesh case as well, but is not available on single-node clusters. Given
that they provide a different coverage, and we just discovered a bug
which only happens when targeting a backend on the same node of the
client, let's start creating two ingresses to cover both cases.

[1]: f7a8822 ("connectivity: make ingress target echo-other-node deployment")
[2]: 7e7fa6a ("ingress: Update backend service for Ingress")

Signed-off-by: Marco Iorio <[email protected]>
@giorio94 giorio94 force-pushed the mio/ingress-backend-service branch from eff1aba to b5b3530 Compare March 27, 2024 17:47
@giorio94 giorio94 removed the needs-rebase This PR needs to be rebased because it has merge conflicts. label Mar 27, 2024
@sayboras sayboras changed the title Improve coverage of Ingress service connectivity: Improve coverage of Ingress service May 6, 2024
@sayboras
Copy link
Member

sayboras commented May 6, 2024

Testing was done as part of cilium/cilium#32367

@sayboras sayboras marked this pull request as ready for review May 6, 2024 09:59
@sayboras sayboras requested a review from a team as a code owner May 6, 2024 09:59
Copy link
Member

@sayboras sayboras left a comment

Choose a reason for hiding this comment

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

Thanks a lot and lgtm ✔️

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label May 6, 2024
@sayboras sayboras merged commit f1a6316 into cilium:main May 6, 2024
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants