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: leader election stop not called #2059

Merged
merged 4 commits into from
Sep 19, 2023
Merged

Conversation

csviri
Copy link
Collaborator

@csviri csviri commented Sep 18, 2023

No description provided.

Signed-off-by: Attila Mészáros <csviri@gmail.com>
@csviri csviri requested review from metacosm and shawkins September 18, 2023 07:35
@csviri csviri changed the title fix: leader election parallel issue fix: leader election stop not called Sep 18, 2023
Signed-off-by: Attila Mészáros <csviri@gmail.com>
@csviri csviri requested a review from metacosm September 18, 2023 11:10
@@ -67,7 +67,7 @@ private void init(LeaderElectionConfiguration config) {
config.getRenewDeadline(),
config.getRetryPeriod(),
leaderCallbacks(),
true,
false, // this is required to receive stop event in all cases
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why, though?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

what do you mean? we want to get those all cases

Copy link
Collaborator

Choose a reason for hiding this comment

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

What I mean is that the comment doesn't explain why changing the value to false makes sure that stop is called…

Copy link
Collaborator Author

@csviri csviri Sep 18, 2023

Choose a reason for hiding this comment

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

That should be part of the fabric8 client docs rather, actually not there but it's in the go docs?
https://pkg.go.dev/k8s.io/client-go/tools/leaderelection#LeaderElectionConfig
Should I link this there you think?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe but mostly explain why this "release on cancel" setting has an impact on the lifecycle of our manager.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

extended the comment, but basically this should be very straghforward, that the onStop is called, also if the lock is released.

Signed-off-by: Attila Mészáros <csviri@gmail.com>
@csviri csviri requested a review from metacosm September 19, 2023 08:48
@csviri csviri merged commit 609a55a into main Sep 19, 2023
@csviri csviri deleted the leader-election-stop-issue branch September 19, 2023 10:07
Copy link

@ashangit ashangit left a comment

Choose a reason for hiding this comment

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

@csviri
Copy link
Collaborator Author

csviri commented Sep 19, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Leader election mechanism does not call all the time the stopLeading action
3 participants