-
Notifications
You must be signed in to change notification settings - Fork 309
Use actual lease duration from Kubernetes object in leader election #1707
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
base: master
Are you sure you want to change the base?
Conversation
Changed LeaderElector.TryAcquireOrRenew to use the lease duration from the Kubernetes object (oldLeaderElectionRecord.LeaseDurationSeconds) instead of the configured lease duration (config.LeaseDuration). This fixes the issue where graceful step-downs (where leaders set lease duration to 1 second) were being ignored by C# clients, which continued to wait for the full configured lease duration before attempting to acquire leadership. This brings the C# implementation in line with the Go client-go library behavior. Fixes kubernetes-client/csharp#1819 Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Copilot The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
1 similar comment
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Copilot The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@copilot could you please add ut |
Added LeaderElectionUsesActualLeaseDurationFromKubernetesObject test to verify that the leader election logic correctly uses the actual lease duration from the Kubernetes object (LeaseDurationSeconds) instead of the configured lease duration. This test specifically validates the graceful step-down scenario where a leader sets the lease duration to 1 second to signal it has stepped down, ensuring that other clients respect this short duration rather than waiting for the full configured duration. Co-authored-by: tg123 <170430+tg123@users.noreply.github.com>
Added unit test in commit 826586f. The 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.
Pull request overview
This PR fixes a bug where the leader election mechanism was using the locally configured lease duration instead of the actual lease duration from the Kubernetes object. This prevented proper handling of graceful leader step-downs, where outgoing leaders signal their departure by setting the lease duration to 1 second. The fix aligns the C# implementation with the Go client-go behavior.
Key changes:
- Modified lease expiration check to use the actual
LeaseDurationSecondsvalue from the Kubernetes object instead of the configured duration - Added comprehensive unit test to validate that graceful step-downs with 1-second lease durations work correctly
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/KubernetesClient/LeaderElection/LeaderElector.cs | Updated TryAcquireOrRenew method to use oldLeaderElectionRecord.LeaseDurationSeconds instead of config.LeaseDuration when checking if a lease has expired |
| tests/KubernetesClient.Tests/LeaderElection/LeaderElectionTests.cs | Added test LeaderElectionUsesActualLeaseDurationFromKubernetesObject that validates the fix by ensuring a 1-second lease duration from the Kubernetes object is respected instead of a 10-second configured duration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/KubernetesClient.Tests/LeaderElection/LeaderElectionTests.cs
Outdated
Show resolved
Hide resolved
…s.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
LeaderElector.TryAcquireOrRenewwas using the locally configured lease duration instead of the value from the Kubernetes object, breaking graceful step-downs where leaders signal their departure by setting lease duration to 1 second.Changes
oldLeaderElectionRecord.LeaseDurationSecondsfrom the Kubernetes object instead ofconfig.LeaseDurationwhen checking lease expirationLeaderElectionUsesActualLeaseDurationFromKubernetesObjectto verify the fixThis aligns the C# implementation with the Go client-go behavior.
Testing
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.