Skip to content

Commit 64b4cd5

Browse files
authored
fix: default exitOnStopLeading to true in LeaderElectionConfigurationBuilder (#3619)
LeaderElectionConfigurationBuilder.build() defaulted exitOnStopLeading to false, contradicting the deprecated LeaderElectionConfiguration constructors and the LeaderElectionManager javadoc, which both treat true as the default. Operators configured through the builder, including those configured via the josdk.leader-election.* properties handled by ConfigLoader, therefore kept running after losing the lead, risking two instances reconciling in parallel. Introduce EXIT_ON_STOP_LEADING_DEFAULT_VALUE, use it from build() and the deprecated constructors, and route build() and buildForTest(boolean) through a common private build(boolean) instead of build() delegating to buildForTest(false).
1 parent e4d61e4 commit 64b4cd5

3 files changed

Lines changed: 43 additions & 4 deletions

File tree

‎docs/content/en/docs/documentation/operations/leader-election.md‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,16 @@ See details under [configurations](configuration.md) page.
4444
the lease.
4545
2. Once leadership is acquired, event processing begins normally.
4646
3. If leadership is lost (e.g. the leader pod becomes unresponsive), another instance acquires the lease
47-
and takes over reconciliation. The instance that lost the lead is terminated (`System.exit()`)
47+
and takes over reconciliation. The instance that lost the lead is terminated (`System.exit(1)`), so
48+
that it is restarted by Kubernetes and no two instances reconcile the same resources in parallel.
49+
This does not happen on a graceful shutdown (`Operator.stop()`), only when the lead is lost while
50+
the operator is running.
51+
52+
{{% alert title="Note" color="primary" %}}
53+
Exiting on lost leadership is always on in production. `LeaderElectionConfigurationBuilder` exposes
54+
`buildForTest(boolean exitOnStopLeading)` to turn it off, but as the name says this is only meant for
55+
tests, where terminating the JVM would kill the test run.
56+
{{% /alert %}}
4857

4958
### Identity and Namespace Inference
5059

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfiguration.java‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ public class LeaderElectionConfiguration {
2525
public static final Duration LEASE_DURATION_DEFAULT_VALUE = Duration.ofSeconds(15);
2626
public static final Duration RENEW_DEADLINE_DEFAULT_VALUE = Duration.ofSeconds(10);
2727
public static final Duration RETRY_PERIOD_DEFAULT_VALUE = Duration.ofSeconds(2);
28+
public static final boolean EXIT_ON_STOP_LEADING_DEFAULT_VALUE = true;
2829

2930
private final String leaseName;
3031
private final String leaseNamespace;
@@ -50,7 +51,7 @@ public LeaderElectionConfiguration(String leaseName, String leaseNamespace, Stri
5051
RETRY_PERIOD_DEFAULT_VALUE,
5152
identity,
5253
null,
53-
true);
54+
EXIT_ON_STOP_LEADING_DEFAULT_VALUE);
5455
}
5556

5657
/**
@@ -79,7 +80,15 @@ public LeaderElectionConfiguration(
7980
Duration leaseDuration,
8081
Duration renewDeadline,
8182
Duration retryPeriod) {
82-
this(leaseName, leaseNamespace, leaseDuration, renewDeadline, retryPeriod, null, null, true);
83+
this(
84+
leaseName,
85+
leaseNamespace,
86+
leaseDuration,
87+
renewDeadline,
88+
retryPeriod,
89+
null,
90+
null,
91+
EXIT_ON_STOP_LEADING_DEFAULT_VALUE);
8392
}
8493

8594
/**
@@ -133,6 +142,12 @@ public Optional<LeaderCallbacks> getLeaderCallbacks() {
133142
return Optional.ofNullable(leaderCallbacks);
134143
}
135144

145+
/**
146+
* Whether the process should exit (via {@code System.exit(1)}) when this instance stops leading
147+
* outside of a graceful shutdown. Defaults to {@value #EXIT_ON_STOP_LEADING_DEFAULT_VALUE};
148+
* {@code false} is only meant for testing purposes, see {@link
149+
* LeaderElectionConfigurationBuilder#buildForTest(boolean)}.
150+
*/
136151
public boolean isExitOnStopLeading() {
137152
return exitOnStopLeading;
138153
}

‎operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/LeaderElectionConfigurationBuilder.java‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,26 @@ public LeaderElectionConfigurationBuilder withExitOnStopLeading(boolean exitOnSt
8181
+ " instead");
8282
}
8383

84+
/**
85+
* Builds the configuration with {@code exitOnStopLeading} set to {@value
86+
* LeaderElectionConfiguration#EXIT_ON_STOP_LEADING_DEFAULT_VALUE}, meaning that the process exits
87+
* when this instance stops leading outside of a graceful shutdown, so that another replica can
88+
* take over without two instances reconciling in parallel.
89+
*/
8490
public LeaderElectionConfiguration build() {
85-
return buildForTest(false);
91+
return build(EXIT_ON_STOP_LEADING_DEFAULT_VALUE);
8692
}
8793

94+
/**
95+
* Same as {@link #build()}, but allows turning off the exit on stop leading behavior. This should
96+
* only be used for testing purposes, since without exiting, two instances might reconcile the
97+
* same resources in parallel.
98+
*/
8899
public LeaderElectionConfiguration buildForTest(boolean exitOnStopLeading) {
100+
return build(exitOnStopLeading);
101+
}
102+
103+
private LeaderElectionConfiguration build(boolean exitOnStopLeading) {
89104
return new LeaderElectionConfiguration(
90105
leaseName,
91106
leaseNamespace,

0 commit comments

Comments
 (0)