From ccf57cc8837129b868fe498ea0ae7992e561716b Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 1 Feb 2023 10:39:24 +0100 Subject: [PATCH 1/5] fix: improvements on informer start and logging --- .../operator/processing/event/EventSourceManager.java | 2 +- .../event/source/informer/InformerManager.java | 9 ++++++++- .../source/informer/ManagedInformerEventSource.java | 7 +++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index a38ca3ec6e..6d161aca9e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -82,7 +82,7 @@ private static Function getThreadNamer(String stage) { return es -> { final var name = es.name(); return es.priority() + " " + stage + " -> " - + (es.isNameSet() ? name + " " + es.original().getClass().getSimpleName() : name); + + (es.isNameSet() ? name + " " + es.original().getClass() : es.original()); }; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 50ad1d6567..9d048e61cf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -25,6 +25,7 @@ import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.Cloner; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; +import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.processing.LifecycleAware; @@ -58,7 +59,13 @@ public InformerManager(MixedOperation, Resource> public void start() throws OperatorException { initSources(); // make sure informers are all started before proceeding further - sources.values().parallelStream().forEach(InformerWrapper::start); + ExecutorServiceManager.executeAndWaitForAllToComplete(sources.values().stream(), + iw -> { + iw.start(); + return null; + }, + iw -> "InformerStarter-" + iw.getTargetNamespace() + "-" + + configuration.getResourceClass().getSimpleName()); } private void initSources() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 40fb5ef436..b0b5b6313f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -163,4 +163,11 @@ public ResourceConfiguration getInformerConfiguration() { public C configuration() { return manager().configuration(); } + + @Override + public String toString() { + return getClass().getSimpleName() + "{" + + "resourceClass: " + configuration.getResourceClass().getSimpleName() + + "}"; + } } From f2f89de54e35f78bd798557cf62cfc46528a004c Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Feb 2023 16:08:57 +0100 Subject: [PATCH 2/5] additional logging --- .../io/javaoperatorsdk/operator/processing/Controller.java | 2 +- .../processing/event/source/informer/InformerManager.java | 1 + .../processing/event/source/informer/InformerWrapper.java | 6 ++++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 8753e0862b..f547fee778 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -335,7 +335,7 @@ public synchronized void start(boolean startEventProcessor) throws OperatorExcep if (startEventProcessor) { eventProcessor.start(); } - log.info("'{}' controller started, pending event sources initialization", controllerName); + log.info("'{}' controller started", controllerName); } catch (MissingCRDException e) { stop(); throwMissingCRDException(e.getCrdName(), e.getSpecVersion(), controllerName); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 9d048e61cf..31c394868a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -58,6 +58,7 @@ public InformerManager(MixedOperation, Resource> @Override public void start() throws OperatorException { initSources(); + // sources.values().parallelStream().forEach(InformerWrapper::start); // make sure informers are all started before proceeding further ExecutorServiceManager.executeAndWaitForAllToComplete(sources.values().stream(), iw -> { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java index 26ce893fba..65f8abf776 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java @@ -73,12 +73,18 @@ public void start() throws OperatorException { final var name = thread.getName(); try { thread.setName(informerInfo() + " " + thread.getId()); + log.debug("Starting informer for namespace: {} resource: {}", namespaceIdentifier, + informer.getApiTypeClass().getSimpleName()); var start = informer.start(); // note that in case we don't put here timeout and stopOnInformerErrorDuringStartup is // false, and there is a rbac issue the get never returns; therefore operator never really // starts + log.trace("Waiting informer to start namespace: {} resource: {}", namespaceIdentifier, + informer.getApiTypeClass().getSimpleName()); start.toCompletableFuture().get(configService.cacheSyncTimeout().toMillis(), TimeUnit.MILLISECONDS); + log.debug("Started informer for namespace: {} resource: {}", namespaceIdentifier, + informer.getApiTypeClass().getSimpleName()); } catch (TimeoutException | ExecutionException e) { if (configService.stopOnInformerErrorDuringStartup()) { log.error("Informer startup error. Operator will be stopped. Informer: {}", informer, e); From 6cb6bd2812d9e973f51d73a9262e08d92ed4e9bb Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Feb 2023 16:16:59 +0100 Subject: [PATCH 3/5] exception handling fix --- .../operator/api/config/ExecutorServiceManager.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java index c4193b9a2a..5dcba5975d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java @@ -16,6 +16,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import io.javaoperatorsdk.operator.OperatorException; + public class ExecutorServiceManager { private static final Logger log = LoggerFactory.getLogger(ExecutorServiceManager.class); private static ExecutorServiceManager instance; @@ -82,7 +84,16 @@ public static void executeAndWaitForAllToComplete(Stream stream, // restore original name thread.setName(name); } - }).collect(Collectors.toList())); + }).collect(Collectors.toList())).forEach(f -> { + try { + // to find out any exceptions + f.get(); + } catch (ExecutionException e) { + throw new OperatorException(e.getCause()); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + }); shutdown(instrumented); } catch (InterruptedException e) { Thread.currentThread().interrupt(); From 066f954260285fddcb1f574f9ca697186c704481 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 2 Feb 2023 16:19:02 +0100 Subject: [PATCH 4/5] remove comment --- .../processing/event/source/informer/InformerManager.java | 1 - 1 file changed, 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 31c394868a..9d048e61cf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -58,7 +58,6 @@ public InformerManager(MixedOperation, Resource> @Override public void start() throws OperatorException { initSources(); - // sources.values().parallelStream().forEach(InformerWrapper::start); // make sure informers are all started before proceeding further ExecutorServiceManager.executeAndWaitForAllToComplete(sources.values().stream(), iw -> { From e82b2a199087e4240d005f2c470bf58bc6434f19 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 2 Feb 2023 17:17:37 +0100 Subject: [PATCH 5/5] refactor: avoid retrieving name several times --- .../processing/event/source/informer/InformerWrapper.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java index 65f8abf776..1ea93adb70 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java @@ -73,18 +73,19 @@ public void start() throws OperatorException { final var name = thread.getName(); try { thread.setName(informerInfo() + " " + thread.getId()); + final var resourceName = informer.getApiTypeClass().getSimpleName(); log.debug("Starting informer for namespace: {} resource: {}", namespaceIdentifier, - informer.getApiTypeClass().getSimpleName()); + resourceName); var start = informer.start(); // note that in case we don't put here timeout and stopOnInformerErrorDuringStartup is // false, and there is a rbac issue the get never returns; therefore operator never really // starts log.trace("Waiting informer to start namespace: {} resource: {}", namespaceIdentifier, - informer.getApiTypeClass().getSimpleName()); + resourceName); start.toCompletableFuture().get(configService.cacheSyncTimeout().toMillis(), TimeUnit.MILLISECONDS); log.debug("Started informer for namespace: {} resource: {}", namespaceIdentifier, - informer.getApiTypeClass().getSimpleName()); + resourceName); } catch (TimeoutException | ExecutionException e) { if (configService.stopOnInformerErrorDuringStartup()) { log.error("Informer startup error. Operator will be stopped. Informer: {}", informer, e);