fix: disable leader election for single replica deployment - #2767
Conversation
This change avoids unnecessary leader-election overhead, removing currently lease renewal failures upon API downtime. Signed-off-by: tginer <tginer@redhat.com>
|
/ok to test 0712179 |
| nvidia.com/gpu-driver-upgrade-drain.skip: "true" | ||
| spec: | ||
| replicas: 1 | ||
| replicas: {{ .Values.operator.replicas | default 1 }} |
There was a problem hiding this comment.
This also needs to be added to values.yaml (maybe commented) so that users can find this.
| args: | ||
| {{- if gt (int (.Values.operator.replicas | default 1)) 1 }} | ||
| - --leader-elect | ||
| {{- end }} |
There was a problem hiding this comment.
What happens when two gpu-operator controllers come up at same time during upgrade (old controller and new controller)?
There was a problem hiding this comment.
In my opinion leader election is critical under multiple replicas deployed. With a single replica and a very brief overlap period, acting without a leader seems fairly safe.
The tests I performed resulted in:
- The overlap window was between 10 - 15 seconds
- Kubernetes reconciliation is idempotent
- Kubernetes
resourceVersionwould prevent conflicts as pod B trying to update old version would result in a re-try and read new resourceVersion
Basically the two pods were briefly trying to do the same idempotent work and the old pod was terminated in a 10-15 second-window.
I replicated the idea implemented by NVIDIA in Mellanox/network-operator#2304 but the network operator AI agent bot also raised your concern in my identical PR in the NNO, see Mellanox/network-operator#3056
There was a problem hiding this comment.
Concurrent reconciles by different versions of the operator are almost never a good thing, even if they are idempotent in theory. It can be a source of unexpected bugs and we are sensitive to unnecessary daemonset rollouts (especially triggering unnecessary driver upgrades). So this change does not look safe to me.
Instead of disabling leader election, how about setting a larger --leader-lease-renew-deadline=60s value? That will make the operator tolerate any API outages up to 60s while still holding the lease. That way the operator doesn't enter a crash loop if it can't renew the lease in 10s (controller-runtime default) and doesn't delay ClusterPolicy processing due to exponential backoff. The OLM bundle already does this:
gpu-operator/bundle/manifests/gpu-operator-certified.clusterserviceversion.yaml
Lines 1081 to 1082 in 10ee5b3
3 participants
This change avoids unnecessary leader-election overhead, removing currently lease renewal failures upon API downtime.
Description
Issue Observed
In single-node clusters, the gpu-operator enters a CrashLoopBackOff cycle after every node reboot due to leader election lease renewal failures during transient API outages. This adds unnecessary recovery time before ClusterPolicy is reconciled.
The --leader-elect flag is unconditionally hardcoded in the Helm Deployment template, even though the default deployment runs a single replica. With only one replica, leader election provides zero availability benefit — there is no second replica to fail over to. Instead, every transient kube-apiserver disruption (expected during SNO MCP reconciliation) triggers a fatal leader election loss → os.Exit(1) → CrashLoopBackOff.
Change proposal
This PR makes --leader-elect conditional on the replica count. It is only passed as an argument in case replica count is greater than 1.
As a precedent, the NVIDIA Network Operator - Node Feature Discovery already implemented this pattern in Mellanox/network-operator#2304
Checklist
make lint)make validate-generated-assets)make validate-modules)Testing
The logs demonstrate that from previous ~5 pod restarts, after the fix there is a single restart (caused by node rebot). Leader Election logs attempting to acquire the lease, failing to update lock, failing to renew the lease, leader election lost are no longer present.