From ae33c53fccb4b599814722e598b81415da079bd4 Mon Sep 17 00:00:00 2001 From: Dennis-Mircea Ciupitu Date: Wed, 27 May 2026 17:43:10 +0300 Subject: [PATCH 1/2] fix: shutdown hook deadlock under leader election and deprecate Operator#installShutdownHook(Duration) Signed-off-by: Dennis-Mircea Ciupitu --- .../operator/LeaderElectionManager.java | 51 ++++++++++++++- .../io/javaoperatorsdk/operator/Operator.java | 63 +++++++++++++++---- .../operator/LeaderElectionManagerTest.java | 12 ++++ 3 files changed, 112 insertions(+), 14 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index 4788aff385..b28b0e0b8d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -17,8 +17,10 @@ import java.util.Arrays; import java.util.Collection; +import java.util.HashSet; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Predicate; import java.util.stream.Collectors; @@ -36,6 +38,41 @@ import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.LeaderElectionConfiguration; +/** + * Manages the leader-election lifecycle for an {@link Operator} instance. Leader election ensures + * that, in a high-availability setup with multiple replicas of the same operator, only one replica + * at a time actively reconciles resources. The replica currently holding the lease is referred to + * as the leader, and the others stand by until the lease becomes available. + * + *

Leader election is opt-in. It is enabled when a {@link LeaderElectionConfiguration} is + * supplied via {@link + * io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider#withLeaderElectionConfiguration(LeaderElectionConfiguration) + * ConfigurationServiceOverrider#withLeaderElectionConfiguration(LeaderElectionConfiguration)}. The + * configuration controls the lease name, namespace, durations, and optional user-supplied {@link + * LeaderCallbacks}. + * + *

Internally this class wraps a Fabric8 {@link LeaderElector} that coordinates via a Kubernetes + * {@code Lease} resource (group {@value #COORDINATION_GROUP}, resource {@value #LEASES_RESOURCE}). + * When this pod acquires the lease, {@link #startLeading()} starts event processing on the + * controller manager. When the lease is lost or the leader-election future is cancelled, {@link + * #stopLeading()} is invoked. + * + *

{@link #stopLeading()} behaves differently depending on how it was triggered: + * + *

+ * + *

The lifecycle methods {@link #start()} and {@link #stop()} are called by {@link Operator} as + * part of {@link Operator#start()} and {@link Operator#stop()} respectively. Users typically do not + * interact with this class directly. + */ public class LeaderElectionManager { private static final Logger log = LoggerFactory.getLogger(LeaderElectionManager.class); @@ -53,6 +90,10 @@ public class LeaderElectionManager { private final ConfigurationService configurationService; private String leaseNamespace; private String leaseName; + // Set in stop() before cancelling the leader-election future. Checked in stopLeading() so that + // a graceful shutdown does not call System.exit, which would otherwise deadlock against the + // JVM shutdown hook lock when stop() is invoked from a JVM shutdown hook. + private final AtomicBoolean stoppingGracefully = new AtomicBoolean(false); LeaderElectionManager( ControllerManager controllerManager, ConfigurationService configurationService) { @@ -118,7 +159,11 @@ private void startLeading() { controllerManager.startEventProcessing(); } - private void stopLeading() { + protected void stopLeading() { + if (stoppingGracefully.get()) { + log.info("Stopped leading for identity: {} during graceful shutdown.", identity); + return; + } if (configurationService.getLeaderElectionConfiguration().orElseThrow().isExitOnStopLeading()) { log.info("Stopped leading for identity: {}. Exiting.", identity); // When leader stops leading the process ends immediately to prevent multiple reconciliations @@ -147,6 +192,7 @@ public void start() { } public void stop() { + stoppingGracefully.set(true); if (leaderElectionFuture != null) { leaderElectionFuture.cancel(false); } @@ -170,7 +216,8 @@ private void checkLeaseAccess() { .flatMap(Collection::stream) .distinct() .collect(Collectors.toList()); - if (verbsAllowed.contains(UNIVERSAL_VALUE) || verbsAllowed.containsAll(verbsRequired)) { + if (verbsAllowed.contains(UNIVERSAL_VALUE) + || new HashSet<>(verbsAllowed).containsAll(verbsRequired)) { return; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index fa8b4620bf..9fb19129f7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -130,23 +130,38 @@ protected ConfigurationService initConfigurationService( } /** - * Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. Note - * that graceful shutdown is usually not needed, but some {@link Reconciler} implementations might - * require it. + * Adds a JVM shutdown hook that automatically calls {@link #stop()} when the application shuts + * down. The shutdown timeout used while waiting for in-flight reconciliations to complete is + * taken from {@link + * io.javaoperatorsdk.operator.api.config.ConfigurationService#reconciliationTerminationTimeout()}. + * Configure it via {@link + * io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider#withReconciliationTerminationTimeout(Duration) + * ConfigurationServiceOverrider#withReconciliationTerminationTimeout(Duration)}. * - *

Note that you might want to tune "terminationGracePeriodSeconds" for the Pod running the - * controller. + *

The hook is registered regardless of whether leader election is enabled. A leader pod + * receiving {@code SIGTERM} will therefore release its lease cleanly so that a standby replica + * can take over without waiting for lease expiry. * - * @param gracefulShutdownTimeout timeout to wait for executor threads to complete actual - * reconciliations + *

NOTE: You may also want to tune the Pod's {@code terminationGracePeriodSeconds} to be + * at least as long as the configured {@code reconciliationTerminationTimeout}, plus a small + * buffer for the rest of the shutdown sequence (releasing the leader-election lease and closing + * the Kubernetes client). If the grace period elapses before {@link #stop()} returns, the kubelet + * sends {@code SIGKILL}, in-flight reconciliations are abandoned, and any held leader-election + * lease is not released cleanly. */ + public void installShutdownHook() { + Runtime.getRuntime().addShutdownHook(new Thread(this::stop)); + } + + /** + * @deprecated The {@code gracefulShutdownTimeout} argument is ignored. Use {@link + * #installShutdownHook()} instead. + * @param gracefulShutdownTimeout ignored, kept only for source and binary compatibility + */ + @Deprecated(forRemoval = true) @SuppressWarnings("unused") public void installShutdownHook(Duration gracefulShutdownTimeout) { - if (!leaderElectionManager.isLeaderElectionEnabled()) { - Runtime.getRuntime().addShutdownHook(new Thread(this::stop)); - } else { - log.warn("Leader election is on, shutdown hook will not be installed."); - } + installShutdownHook(); } public KubernetesClient getKubernetesClient() { @@ -188,6 +203,30 @@ public synchronized void start() { } } + /** + * Stops the operator and releases its resources. The shutdown sequence is: + * + *

    + *
  1. Stop the controller manager, halting reconciliation of all registered controllers. + *
  2. Stop the executor service manager, waiting up to {@link + * io.javaoperatorsdk.operator.api.config.ConfigurationService#reconciliationTerminationTimeout()} + * for in-flight reconciliations to complete. + *
  3. Stop the leader-election manager, cancelling the leader-election future and releasing any + * held lease. + *
  4. Close the {@link KubernetesClient} if {@link + * io.javaoperatorsdk.operator.api.config.ConfigurationService#closeClientOnStop()} is + * {@code true} (the default). + *
+ * + *

It is safe to call this method from a JVM shutdown hook (see {@link #installShutdownHook()}) + * as the graceful-shutdown path coordinates with the leader-election callbacks so that {@code + * System.exit} is not invoked while the JVM is already shutting down. + * + *

If the operator was never successfully started, this method only stops the executor service + * manager so that no thread pools are leaked. + * + * @throws OperatorException if an error occurs during shutdown + */ @Override public void stop() throws OperatorException { Duration reconciliationTerminationTimeout = diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java index 510890e56e..a885d7604c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/LeaderElectionManagerTest.java @@ -109,6 +109,18 @@ void testInitPermissionsMultipleRulesWithResourceName(@TempDir Path tempDir) thr assertTrue(leaderElectionManager.isLeaderElectionEnabled()); } + @Test + void stopLeadingDoesNotInvokeSystemExitWhenStopWasCalledFirst() { + // When stop() is called before the onStopLeading callback fires (which is what happens when + // stop()'s future cancellation triggers the callback), stopLeading() must skip + // System.exit(1). Otherwise calling stop() from inside a JVM shutdown hook deadlocks against + // the java.lang.Shutdown class lock. If this regression is ever reintroduced, this test + // method would terminate the JUnit JVM via System.exit(1) instead of failing cleanly. + final var leaderElectionManager = leaderElectionManager(null); + leaderElectionManager.stop(); + leaderElectionManager.stopLeading(); + } + @Test void testFailedToInitMissingPermission(@TempDir Path tempDir) throws IOException { var namespace = "foo"; From b135032b4e29473d3c5c1ef25f98032039d275b8 Mon Sep 17 00:00:00 2001 From: Dennis-Mircea Ciupitu Date: Thu, 28 May 2026 11:33:12 +0300 Subject: [PATCH 2/2] fix: Address review comments Signed-off-by: Dennis-Mircea Ciupitu --- .../operator/LeaderElectionManager.java | 23 ++++++------------- .../io/javaoperatorsdk/operator/Operator.java | 19 +++++++++++---- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java index b28b0e0b8d..7b0a446e81 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/LeaderElectionManager.java @@ -15,9 +15,8 @@ */ package io.javaoperatorsdk.operator; -import java.util.Arrays; import java.util.Collection; -import java.util.HashSet; +import java.util.List; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.atomic.AtomicBoolean; @@ -51,12 +50,6 @@ * configuration controls the lease name, namespace, durations, and optional user-supplied {@link * LeaderCallbacks}. * - *

Internally this class wraps a Fabric8 {@link LeaderElector} that coordinates via a Kubernetes - * {@code Lease} resource (group {@value #COORDINATION_GROUP}, resource {@value #LEASES_RESOURCE}). - * When this pod acquires the lease, {@link #startLeading()} starts event processing on the - * controller manager. When the lease is lost or the leader-election future is cancelled, {@link - * #stopLeading()} is invoked. - * *

{@link #stopLeading()} behaves differently depending on how it was triggered: * *