Repository navigation
fix: make the leader lease renew deadline configurable - #3256
Merged
Merged
Conversation
Contributor
|
stav-inbar
force-pushed
the
leader_lease
branch
from
September 17, 2026 15:50
7aed3e2 to
e98dac9
Compare
Member
|
@tginer PTAL |
Member
|
/retest-all |
Collaborator
|
/retest-nic_operator_kind |
4 similar comments
Collaborator
|
/retest-nic_operator_kind |
Collaborator
|
/retest-nic_operator_kind |
Collaborator
|
/retest-nic_operator_kind |
Collaborator
|
/retest-nic_operator_kind |
Collaborator
Author
|
/retest-nic_operator_helm |
1 similar comment
Collaborator
Author
|
/retest-nic_operator_helm |
Collaborator
|
@stav-inbar please rebase |
almaslennikov
approved these changes
Oct 6, 2026
A brief control-plane outage was enough to kill the operator. With the controller-runtime default renew deadline of 10s, a kube-apiserver restart that refused connections for a few seconds left the leader unable to renew its lease, so the manager returned "leader election lost", main exited, and the single-replica Deployment went into CrashLoopBackOff. Each restart then had to wait out the stale lease before reconciling again, delaying NicClusterPolicy by minutes for an outage that lasted seconds. Add --leader-lease-renew-deadline so the operator can ride out an outage instead of standing down, and default it to 60s in both the Helm chart and the kustomize manifests. Both kustomize paths have to carry the flag: the default overlay replaces the manager's argument list rather than merging into it, so setting it only in the base left `make deploy` on the 10s default. Leader election requires the lease duration to exceed the renew deadline or it refuses to start, so the lease duration is derived as the deadline plus 5s rather than exposed as a second flag that could be set to an invalid combination. Leaving the flag unset keeps the controller-runtime defaults, which is what happens on any deployment that does not pass it. Helm users can override it through operator.leaderElection.renewDeadline. That same validation also requires the deadline to exceed the retry period scaled by a jitter factor, 2.4s with the defaults in use, and it runs when the elector is built rather than when the manager is created. A value below it therefore let the operator come up and only then exit, which is the crash loop this flag exists to prevent, so the deadline is now rejected below 3s before the manager is built. Registering the command line flags moved into its own function, and the manager configuration is passed around as a struct, because the added check put main over the statement limit. Neither changes behavior. Note the trade-off this makes: a leader that dies without releasing its lease is now replaced after about 65s instead of about 15s, so operator restarts and upgrades pause reconciliation for that long. That is the cost of not treating a transient API error as fatal. Fixes Mellanox#3133 Signed-off-by: Stav Inbar <sinbar@nvidia.com>
almaslennikov
approved these changes
Oct 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A brief control-plane outage was enough to kill the operator. With the controller-runtime default renew deadline of 10s, a kube-apiserver restart that refused connections for a few seconds left the leader unable to renew its lease, so the manager returned "leader election lost", main exited, and the single-replica Deployment went into CrashLoopBackOff. Each restart then had to wait out the stale lease before reconciling again, delaying NicClusterPolicy by minutes for an outage that lasted seconds.
Add --leader-lease-renew-deadline so the operator can ride out an outage instead of standing down, and default it to 60s in both the Helm chart and the kustomize manifests. Leader election requires the lease duration to exceed the renew deadline or it refuses to start, so the lease duration is derived as the deadline plus 5s rather than exposed as a second flag that could be set to an invalid combination. Leaving the flag unset keeps the controller-runtime defaults, which is what happens on any deployment that does not pass it. Helm users can override it through operator.leaderElection.renewDeadline.
Note the trade-off this makes: a leader that dies without releasing its lease is now replaced after about 65s instead of about 15s, so operator restarts and upgrades pause reconciliation for that long. That is the cost of not treating a transient API error as fatal.
Fixes #3133