-
Notifications
You must be signed in to change notification settings - Fork 147
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(networking): cleanup service/endpoint if needed #3078
fix(networking): cleanup service/endpoint if needed #3078
Conversation
37a000c
to
6e6e488
Compare
b8e8bae
to
2411a2e
Compare
codeFactor checking failure is not related to this PR. |
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.
This looks reasonable to me.
ce14bfc
to
7e4a0a9
Compare
552adef
to
072656f
Compare
ed2d224
to
c3c1673
Compare
c3c1673
to
ac704dd
Compare
ac704dd
to
d5f67ad
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.
LGTM.
Need @c3y1huang's final check and approval. Thank you. |
d5f67ad
to
52691ad
Compare
52691ad
to
4832e8c
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.
LGTM
We meet a corner case that the service/endpoint would not be cleanup. That will cause the service keep the ClusterIP `None`. With this config, the endpoint of sharemanager would not correct. So the CSI driver cannot perform the mountpoint well. We would like to have a checking mechanism to know if the service/ endpoint did not cleanup. Then we will cleanup the service/endpoint to ensure the correct endpoint. Remove the cleanup function in the setting controller, we could do the cleanup on the sm controller Signed-off-by: Vicente Cheng <vicente.cheng@suse.com> Co-authored-by: Chin-Ya Huang <chin-ya.huang@suse.com> Co-authored-by: Derek Su <derek.su@suse.com>
4832e8c
to
4e88d72
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.
LGTM, thank you for the fix.
@mergify backport v1.7.x |
✅ Backports have been created
|
Which issue(s) this PR fixes:
Issue longhorn/longhorn#9272
What this PR does / why we need it:
We meet a corner case that the service/endpoint would not be cleanup. That will cause the service keep the ClusterIP
None
. With this config, the endpoint of sharemanager would not be correct, so the CSI driver cannot perform the mountpoint well.We would like to have a checking mechanism to know if the service/endpoint did not cleanup. Then we will cleanup the service/endpoint to ensure the correct endpoint.
Special notes for your reviewer:
Additional documentation or context