From 163ecdaa8bdc22db5249cec13111e52b5b4f8cf2 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 27 Oct 2022 13:08:54 +0200 Subject: [PATCH 1/2] fix: default stop handler exists only if informer started --- .../api/config/ConfigurationService.java | 4 ++- .../operator/InformerRelatedBehaviorITS.java | 26 +++++++++++++++---- 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 16faa0e9f0..9f4bde5beb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -184,7 +184,9 @@ default Duration cacheSyncTimeout() { */ default Optional getInformerStoppedHandler() { return Optional.of((informer, ex) -> { - if (ex != null) { + // hasSynced is checked to verify that informer already started. If not started, in case + // of a fatal error the operator will stop, no need for explicit exit. + if (ex != null && informer.hasSynced()) { Logger log = LoggerFactory.getLogger(ConfigurationService.class); log.error("Fatal error in informer: {}. Stopping the operator", informer, ex); System.exit(1); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java index bfc0e4c02f..7c9a045146 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java @@ -41,7 +41,7 @@ class InformerRelatedBehaviorITS { KubernetesClient adminClient = new KubernetesClientBuilder().build(); InformerRelatedBehaviorTestReconciler reconciler; String actualNamespace; - volatile boolean stopHandlerCalled = false; + volatile boolean stopHandlerCalledReplacement = false; @BeforeEach void beforeEach(TestInfo testInfo) { @@ -136,7 +136,19 @@ void callsStopHandlerOnStartupFail() { assertThrows(OperatorException.class, () -> startOperator(true)); - await().untilAsserted(() -> assertThat(stopHandlerCalled).isTrue()); + await().untilAsserted(() -> assertThat(stopHandlerCalledReplacement).isTrue()); + } + + @Test + void notExitingWithDefaultStopHandlerIfErrorHappensOnStartup() { + setNoCustomResourceAccess(); + adminClient.resource(testCustomResource()).createOrReplace(); + + assertThrows(OperatorException.class, () -> startOperator(true, false)); + + // note that we just basically check here that the default handler does not call system exit. + // Thus, the test does not terminate before to assert. + await().untilAsserted(() -> assertThat(stopHandlerCalledReplacement).isFalse()); } private static void waitForWatchReconnect() { @@ -183,6 +195,10 @@ KubernetesClient clientUsingServiceAccount() { } Operator startOperator(boolean stopOnInformerErrorDuringStartup) { + return startOperator(stopOnInformerErrorDuringStartup, true); + } + + Operator startOperator(boolean stopOnInformerErrorDuringStartup, boolean addStopHandler) { ConfigurationServiceProvider.reset(); reconciler = new InformerRelatedBehaviorTestReconciler(); @@ -190,9 +206,9 @@ Operator startOperator(boolean stopOnInformerErrorDuringStartup) { co -> { co.withStopOnInformerErrorDuringStartup(stopOnInformerErrorDuringStartup); co.withCacheSyncTimeout(Duration.ofMillis(3000)); - co.withInformerStoppedHandler((informer, ex) -> { - stopHandlerCalled = true; - }); + if (addStopHandler) { + co.withInformerStoppedHandler((informer, ex) -> stopHandlerCalledReplacement = true); + } }); operator.register(reconciler); operator.installShutdownHook(); From b4072ae46ed78911cbde4ef84d44e94117c64597 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 27 Oct 2022 15:37:10 +0200 Subject: [PATCH 2/2] refactor based on PR comment --- .../operator/InformerRelatedBehaviorITS.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java index 7c9a045146..30e69a1471 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerRelatedBehaviorITS.java @@ -41,7 +41,7 @@ class InformerRelatedBehaviorITS { KubernetesClient adminClient = new KubernetesClientBuilder().build(); InformerRelatedBehaviorTestReconciler reconciler; String actualNamespace; - volatile boolean stopHandlerCalledReplacement = false; + volatile boolean replacementStopHandlerCalled = false; @BeforeEach void beforeEach(TestInfo testInfo) { @@ -136,7 +136,7 @@ void callsStopHandlerOnStartupFail() { assertThrows(OperatorException.class, () -> startOperator(true)); - await().untilAsserted(() -> assertThat(stopHandlerCalledReplacement).isTrue()); + await().untilAsserted(() -> assertThat(replacementStopHandlerCalled).isTrue()); } @Test @@ -148,7 +148,7 @@ void notExitingWithDefaultStopHandlerIfErrorHappensOnStartup() { // note that we just basically check here that the default handler does not call system exit. // Thus, the test does not terminate before to assert. - await().untilAsserted(() -> assertThat(stopHandlerCalledReplacement).isFalse()); + await().untilAsserted(() -> assertThat(replacementStopHandlerCalled).isFalse()); } private static void waitForWatchReconnect() { @@ -207,7 +207,7 @@ Operator startOperator(boolean stopOnInformerErrorDuringStartup, boolean addStop co.withStopOnInformerErrorDuringStartup(stopOnInformerErrorDuringStartup); co.withCacheSyncTimeout(Duration.ofMillis(3000)); if (addStopHandler) { - co.withInformerStoppedHandler((informer, ex) -> stopHandlerCalledReplacement = true); + co.withInformerStoppedHandler((informer, ex) -> replacementStopHandlerCalled = true); } }); operator.register(reconciler);