-
Notifications
You must be signed in to change notification settings - Fork 3.4k
clustermesh: fix MCS-API service export cache not properly deleted #36892
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
clustermesh: fix MCS-API service export cache not properly deleted #36892
Conversation
Signed-off-by: Arthur Outhenin-Chalandre <arthur@cri.epita.fr>
FYI I spotted this with the MCS-API conformance test: "Exporting a ClusterIP service should create a ServiceImport of type ClusterSetIP in the service's namespace in each cluster. Unexporting should delete the ServiceImport". I was already running the conformance test (https://github.com/kubernetes-sigs/mcs-api/tree/master/conformance) regularly but it turns out I was not running it correctly in the past and I was effectively running it on only one of my cluster instead of two... which does test a few things but as you might expect leave a few things untested... I will also start looking relatively soon at integrating this conformance test in the CI, the only thing is that we currently don't pass an (optional) test that checks for EndpointSlice because we only sync EndpointSlice if the user opted in with an annotation or if the service is headless and the conformance tests doesn't know about this Cilium specific details. But will try to figure out the best path forward to integrate this conformance test which would help us getting a "free" e2etest suite and catch a few more bugs like this one! |
/test |
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.
Good catch, thanks!
That would be great, thanks! Feel free to ask if you need help with the CI. As for the currently failing test, I guess that one possibility in the short term could be just skipping it, so that we still get all the others for free. |
This fixes a bug where we where not properly deleting the service export cache on a delete event from remote cluster which for instance prevents ServiceImport to be properly deleted in some/most cases.