From 235169c5548bcf7f230329b12cd258b2d4725b55 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 11:11:32 +0200 Subject: [PATCH 01/13] refactor: fix generics --- .../operator/processing/DefaultEventHandler.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index 5c21747623..567dc75698 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -146,8 +146,8 @@ private boolean submitReconciliationExecution(CustomResourceID customResourceUid if (!controllerUnderExecution && latestCustomResource.isPresent()) { setUnderExecutionProcessing(customResourceUid); - ExecutionScope executionScope = - new ExecutionScope( + ExecutionScope executionScope = + new ExecutionScope<>( latestCustomResource.get(), retryInfo(customResourceUid)); eventMarker.unMarkEventReceived(customResourceUid); From 40c21b2b10480d847e990d739425c5a465ef42b3 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 11:12:13 +0200 Subject: [PATCH 02/13] fix: restore thread's original name This probably isn't needed but it's nicer to clean after yourself :) --- .../operator/processing/DefaultEventHandler.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index 567dc75698..b7a5b791aa 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -360,10 +360,17 @@ private ControllerExecution(ExecutionScope executionScope) { @Override public void run() { // change thread name for easier debugging - Thread.currentThread().setName("EventHandler-" + controllerName); - PostExecutionControl postExecutionControl = - eventDispatcher.handleExecution(executionScope); - eventProcessingFinished(executionScope, postExecutionControl); + final var thread = Thread.currentThread(); + final var name = thread.getName(); + try { + thread.setName("EventHandler-" + controllerName); + PostExecutionControl postExecutionControl = + eventDispatcher.handleExecution(executionScope); + eventProcessingFinished(executionScope, postExecutionControl); + } finally { + // restore original name + thread.setName(name); + } } @Override From 93b7fa537249106f4aebdefeba889698c775503b Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 14:17:16 +0200 Subject: [PATCH 03/13] refactor: move Metrics to api.monitoring package --- .../operator/{ => monitoring}/micrometer/MicrometerMetrics.java | 2 +- .../operator/api/config/ConfigurationService.java | 2 +- .../operator/api/config/ConfigurationServiceOverrider.java | 2 +- .../javaoperatorsdk/operator/{ => api/monitoring}/Metrics.java | 2 +- .../operator/processing/ConfiguredController.java | 2 +- .../operator/processing/EventDispatcherTest.java | 2 +- .../event/internal/CustomResourceEventSourceTest.java | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) rename micrometer-support/src/main/java/io/javaoperatorsdk/operator/{ => monitoring}/micrometer/MicrometerMetrics.java (97%) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/{ => api/monitoring}/Metrics.java (93%) diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java similarity index 97% rename from micrometer-support/src/main/java/io/javaoperatorsdk/operator/micrometer/MicrometerMetrics.java rename to micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java index 3c8074f8f8..651616e5e8 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java @@ -3,7 +3,7 @@ import java.util.Collections; import java.util.Map; -import io.javaoperatorsdk.operator.Metrics; +import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.DefaultEventHandler.EventMonitor; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; 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 ec61108bf2..fb53f7eaae 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 @@ -6,8 +6,8 @@ import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.Metrics; import io.javaoperatorsdk.operator.api.ResourceController; +import io.javaoperatorsdk.operator.api.monitoring.Metrics; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index f1faef44f8..2cecfb552d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -4,8 +4,8 @@ import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.Metrics; import io.javaoperatorsdk.operator.api.ResourceController; +import io.javaoperatorsdk.operator.api.monitoring.Metrics; public class ConfigurationServiceOverrider { private final ConfigurationService original; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Metrics.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java similarity index 93% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Metrics.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java index a3f3ddccb5..b08913deea 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Metrics.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator; +package io.javaoperatorsdk.operator.api.monitoring; import java.util.Map; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java index 3ce90ebb67..bf0f29525b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ConfiguredController.java @@ -11,7 +11,6 @@ import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.javaoperatorsdk.operator.CustomResourceUtils; -import io.javaoperatorsdk.operator.Metrics.ControllerExecution; import io.javaoperatorsdk.operator.MissingCRDException; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.Context; @@ -19,6 +18,7 @@ import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.UpdateControl; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution; import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java index b709c026e1..da32b467e0 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java @@ -10,7 +10,6 @@ import org.mockito.ArgumentMatchers; import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.Metrics; import io.javaoperatorsdk.operator.TestUtils; import io.javaoperatorsdk.operator.api.Context; import io.javaoperatorsdk.operator.api.DeleteControl; @@ -19,6 +18,7 @@ import io.javaoperatorsdk.operator.api.UpdateControl; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java index a24c32a547..b8e4381f3b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java @@ -9,10 +9,10 @@ import io.fabric8.kubernetes.api.model.KubernetesResourceList; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.javaoperatorsdk.operator.Metrics; import io.javaoperatorsdk.operator.TestUtils; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.DefaultControllerConfiguration; +import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.ConfiguredController; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.EventHandler; From 35b9309fe91ba7f03e1c6e285b1165aee9906cbe Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 14:18:07 +0200 Subject: [PATCH 04/13] refactor: move MicrometerMetrics to monitoring package --- .../operator/monitoring/micrometer/MicrometerMetrics.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java index 651616e5e8..a0dbbeb005 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.micrometer; +package io.javaoperatorsdk.operator.monitoring.micrometer; import java.util.Collections; import java.util.Map; From e371a858943b3d276a0055f76b60f3d5a11a6e70 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 14:19:54 +0200 Subject: [PATCH 05/13] refactor: extract EventMonitor to api.monitoring package --- .../micrometer/MicrometerMetrics.java | 2 +- .../operator/api/monitoring/EventMonitor.java | 19 +++++++++++++ .../operator/api/monitoring/Metrics.java | 5 +--- .../processing/DefaultEventHandler.java | 27 +++++-------------- 4 files changed, 27 insertions(+), 26 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java index a0dbbeb005..7128670e8e 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java @@ -3,8 +3,8 @@ import java.util.Collections; import java.util.Map; +import io.javaoperatorsdk.operator.api.monitoring.EventMonitor; import io.javaoperatorsdk.operator.api.monitoring.Metrics; -import io.javaoperatorsdk.operator.processing.DefaultEventHandler.EventMonitor; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; import io.micrometer.core.instrument.MeterRegistry; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java new file mode 100644 index 0000000000..e56895ad77 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java @@ -0,0 +1,19 @@ +package io.javaoperatorsdk.operator.api.monitoring; + +import io.javaoperatorsdk.operator.processing.event.CustomResourceID; +import io.javaoperatorsdk.operator.processing.event.Event; + +public interface EventMonitor { + + EventMonitor NOOP = new EventMonitor() { + @Override + public void processedEvent(CustomResourceID uid, Event event) {} + + @Override + public void failedEvent(CustomResourceID uid, Event event) {} + }; + + void processedEvent(CustomResourceID uid, Event event); + + void failedEvent(CustomResourceID uid, Event event); +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java index b08913deea..ddbb469c92 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java @@ -2,9 +2,6 @@ import java.util.Map; -import io.javaoperatorsdk.operator.processing.DefaultEventHandler; -import io.javaoperatorsdk.operator.processing.DefaultEventHandler.EventMonitor; - public interface Metrics { Metrics NOOP = new Metrics() {}; @@ -31,7 +28,7 @@ default void incrementProcessedEventsNumber() {} return map; } - default DefaultEventHandler.EventMonitor getEventMonitor() { + default EventMonitor getEventMonitor() { return EventMonitor.NOOP; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index b7a5b791aa..1ba7f61a18 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -16,6 +16,7 @@ import io.javaoperatorsdk.operator.api.RetryInfo; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; +import io.javaoperatorsdk.operator.api.monitoring.EventMonitor; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager; import io.javaoperatorsdk.operator.processing.event.Event; @@ -92,23 +93,6 @@ public void setEventSourceManager(DefaultEventSourceManager eventSourceManage this.eventSourceManager = eventSourceManager; } - /* - * TODO: promote this interface to top-level, probably create a `monitoring` package? - */ - public interface EventMonitor { - EventMonitor NOOP = new EventMonitor() { - @Override - public void processedEvent(CustomResourceID uid, Event event) {} - - @Override - public void failedEvent(CustomResourceID uid, Event event) {} - }; - - void processedEvent(CustomResourceID uid, Event event); - - void failedEvent(CustomResourceID uid, Event event); - } - private EventMonitor monitor() { // todo: remove us of static monitor, only here for backwards compatibility return DefaultEventHandler.monitor != EventMonitor.NOOP ? DefaultEventHandler.monitor @@ -125,13 +109,14 @@ public void handleEvent(Event event) { return; } final var monitor = monitor(); - monitor.processedEvent(event.getRelatedCustomResourceID(), event); + final var resourceID = event.getRelatedCustomResourceID(); + monitor.processedEvent(resourceID, event); handleEventMarking(event); - if (!eventMarker.deleteEventPresent(event.getRelatedCustomResourceID())) { - submitReconciliationExecution(event.getRelatedCustomResourceID()); + if (!eventMarker.deleteEventPresent(resourceID)) { + submitReconciliationExecution(resourceID); } else { - cleanupForDeletedEvent(event.getRelatedCustomResourceID()); + cleanupForDeletedEvent(resourceID); } } finally { lock.unlock(); From e01ccfc66a1654329ed63ec93fda5481fb62f246 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 14:24:31 +0200 Subject: [PATCH 06/13] refactor: remove deprecated static monitor field --- .../operator/processing/DefaultEventHandler.java | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index 1ba7f61a18..3509b3b872 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -38,9 +38,6 @@ public class DefaultEventHandler> implements Even private static final Logger log = LoggerFactory.getLogger(DefaultEventHandler.class); - @Deprecated - private static EventMonitor monitor = EventMonitor.NOOP; - private final Set underProcessing = new HashSet<>(); private final EventDispatcher eventDispatcher; private final Retry retry; @@ -94,9 +91,7 @@ public void setEventSourceManager(DefaultEventSourceManager eventSourceManage } private EventMonitor monitor() { - // todo: remove us of static monitor, only here for backwards compatibility - return DefaultEventHandler.monitor != EventMonitor.NOOP ? DefaultEventHandler.monitor - : eventMonitor; + return eventMonitor; } @Override From 43ffc86bdd74ca3df12301af2afe5516d578640b Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 14:40:42 +0200 Subject: [PATCH 07/13] chore: remove obsolete class --- .../operator/EventListUtils.java | 22 ------------------- 1 file changed, 22 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/EventListUtils.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/EventListUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/EventListUtils.java deleted file mode 100644 index 0fe18bdae0..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/EventListUtils.java +++ /dev/null @@ -1,22 +0,0 @@ -package io.javaoperatorsdk.operator; - -import java.util.List; - -import io.javaoperatorsdk.operator.processing.event.Event; -import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; -import io.javaoperatorsdk.operator.processing.event.internal.ResourceAction; - -public class EventListUtils { - - public static boolean containsCustomResourceDeletedEvent(List events) { - return events.stream() - .anyMatch( - e -> { - if (e instanceof CustomResourceEvent) { - return ((CustomResourceEvent) e).getAction() == ResourceAction.DELETED; - } else { - return false; - } - }); - } -} From 96db12310c7b316c82e27fe46db38f895c31b7b3 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 18:13:01 +0200 Subject: [PATCH 08/13] refactor: add Type on Event Type is an extended version of what was previously called ResourceAction. Since all events have types now, we can also remove CustomResourceEvent. --- .../processing/DefaultEventHandler.java | 5 +--- .../processing/event/DefaultEvent.java | 11 +++++++- .../operator/processing/event/Event.java | 9 +++++++ .../event/internal/CustomResourceEvent.java | 27 ------------------- .../internal/CustomResourceEventSource.java | 14 +++++----- .../event/internal/InformerEventSource.java | 3 ++- .../event/internal/ResourceAction.java | 5 ---- .../event/internal/TimerEventSource.java | 3 ++- .../processing/DefaultEventHandlerTest.java | 17 +++++------- .../processing/EventDispatcherTest.java | 14 +++++----- .../CustomResourceEventFilterTest.java | 13 ++++----- .../CustomResourceEventSourceTest.java | 23 ++++++++-------- 12 files changed, 65 insertions(+), 79 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEvent.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/ResourceAction.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index 3509b3b872..8621454ad5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -21,8 +21,6 @@ import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager; import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.EventHandler; -import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; -import io.javaoperatorsdk.operator.processing.event.internal.ResourceAction; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import io.javaoperatorsdk.operator.processing.retry.RetryExecution; @@ -150,8 +148,7 @@ private boolean submitReconciliationExecution(CustomResourceID customResourceUid } private void handleEventMarking(Event event) { - if (event instanceof CustomResourceEvent && - ((CustomResourceEvent) event).getAction() == ResourceAction.DELETED) { + if (event.isDeleteEvent()) { eventMarker.markDeleteEventReceived(event); } else if (!eventMarker.deleteEventPresent(event.getRelatedCustomResourceID())) { eventMarker.markEventReceived(event); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEvent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEvent.java index 7c55939da5..b5a79282f0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEvent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEvent.java @@ -4,9 +4,12 @@ public class DefaultEvent implements Event { private final CustomResourceID relatedCustomResource; + private final Type type; - public DefaultEvent(CustomResourceID targetCustomResource) { + public DefaultEvent(CustomResourceID targetCustomResource, + Type type) { this.relatedCustomResource = targetCustomResource; + this.type = type; } @Override @@ -14,10 +17,16 @@ public CustomResourceID getRelatedCustomResourceID() { return relatedCustomResource; } + @Override + public Type getType() { + return type; + } + @Override public String toString() { return "DefaultEvent{" + "relatedCustomResource=" + relatedCustomResource + + ", type=" + type + '}'; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java index b0d51a37c0..14803e763f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/Event.java @@ -4,4 +4,13 @@ public interface Event { CustomResourceID getRelatedCustomResourceID(); + Type getType(); + + default boolean isDeleteEvent() { + return Type.DELETED == getType(); + } + + enum Type { + ADDED, UPDATED, DELETED, OTHER, UNKNOWN + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEvent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEvent.java deleted file mode 100644 index 0c20369e0a..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEvent.java +++ /dev/null @@ -1,27 +0,0 @@ -package io.javaoperatorsdk.operator.processing.event.internal; - -import io.javaoperatorsdk.operator.processing.event.CustomResourceID; -import io.javaoperatorsdk.operator.processing.event.DefaultEvent; - -public class CustomResourceEvent extends DefaultEvent { - - private final ResourceAction action; - - public CustomResourceEvent(ResourceAction action, - CustomResourceID customResourceID) { - super(customResourceID); - this.action = action; - } - - @Override - public String toString() { - return "CustomResourceEvent{" + - "action=" + action + - '}'; - } - - public ResourceAction getAction() { - return action; - } - -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java index b9c4c5b6a3..c926b65841 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSource.java @@ -19,6 +19,8 @@ import io.javaoperatorsdk.operator.processing.ResourceCache; import io.javaoperatorsdk.operator.processing.event.AbstractEventSource; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; +import io.javaoperatorsdk.operator.processing.event.DefaultEvent; +import io.javaoperatorsdk.operator.processing.event.Event.Type; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; @@ -117,13 +119,13 @@ public void close() throws IOException { } } - public void eventReceived(ResourceAction action, T customResource, T oldResource) { + public void eventReceived(Type type, T customResource, T oldResource) { log.debug( "Event received for resource: {}", getName(customResource)); if (filter.acceptChange(controller.getConfiguration(), oldResource, customResource)) { - eventHandler.handleEvent( - new CustomResourceEvent(action, CustomResourceID.fromResource(customResource))); + eventHandler + .handleEvent(new DefaultEvent(CustomResourceID.fromResource(customResource), type)); } else { log.debug( "Skipping event handling resource {} with version: {}", @@ -134,17 +136,17 @@ public void eventReceived(ResourceAction action, T customResource, T oldResource @Override public void onAdd(T resource) { - eventReceived(ResourceAction.ADDED, resource, null); + eventReceived(Type.ADDED, resource, null); } @Override public void onUpdate(T oldCustomResource, T newCustomResource) { - eventReceived(ResourceAction.UPDATED, newCustomResource, oldCustomResource); + eventReceived(Type.UPDATED, newCustomResource, oldCustomResource); } @Override public void onDelete(T resource, boolean b) { - eventReceived(ResourceAction.DELETED, resource, null); + eventReceived(Type.DELETED, resource, null); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java index 42b1c94084..ef87ea74ce 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/InformerEventSource.java @@ -14,6 +14,7 @@ import io.javaoperatorsdk.operator.processing.event.AbstractEventSource; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEvent; +import io.javaoperatorsdk.operator.processing.event.Event.Type; public class InformerEventSource extends AbstractEventSource { @@ -82,7 +83,7 @@ private void propagateEvent(T object) { return; } uids.forEach(uid -> { - DefaultEvent event = new DefaultEvent(CustomResourceID.fromResource(object)); + DefaultEvent event = new DefaultEvent(CustomResourceID.fromResource(object), Type.OTHER); this.eventHandler.handleEvent(event); }); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/ResourceAction.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/ResourceAction.java deleted file mode 100644 index 6fbc233133..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/ResourceAction.java +++ /dev/null @@ -1,5 +0,0 @@ -package io.javaoperatorsdk.operator.processing.event.internal; - -public enum ResourceAction { - ADDED, UPDATED, DELETED -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSource.java index 5015df4281..9070f1a6f5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/internal/TimerEventSource.java @@ -14,6 +14,7 @@ import io.javaoperatorsdk.operator.processing.event.AbstractEventSource; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEvent; +import io.javaoperatorsdk.operator.processing.event.Event.Type; public class TimerEventSource> extends AbstractEventSource { private static final Logger log = LoggerFactory.getLogger(TimerEventSource.class); @@ -95,7 +96,7 @@ public EventProducerTimeTask(CustomResourceID customResourceUid) { public void run() { if (running.get()) { log.debug("Producing event for custom resource id: {}", customResourceUid); - eventHandler.handleEvent(new DefaultEvent(customResourceUid)); + eventHandler.handleEvent(new DefaultEvent(customResourceUid, Type.OTHER)); } } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java index f231852537..ddb0a4a3e9 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java @@ -16,15 +16,14 @@ import io.javaoperatorsdk.operator.processing.event.DefaultEvent; import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager; import io.javaoperatorsdk.operator.processing.event.Event; -import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; +import io.javaoperatorsdk.operator.processing.event.Event.Type; import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEventSource; -import io.javaoperatorsdk.operator.processing.event.internal.ResourceAction; import io.javaoperatorsdk.operator.processing.event.internal.TimerEventSource; import io.javaoperatorsdk.operator.processing.retry.GenericRetry; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static io.javaoperatorsdk.operator.TestUtils.testCustomResource; -import static io.javaoperatorsdk.operator.processing.event.internal.ResourceAction.DELETED; +import static io.javaoperatorsdk.operator.processing.event.Event.Type.DELETED; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; @@ -203,8 +202,7 @@ public void doNotFireEventsIfClosing() { @Test public void cleansUpWhenDeleteEventReceivedAndNoEventPresent() { - Event deleteEvent = - new CustomResourceEvent(DELETED, prepareCREvent().getRelatedCustomResourceID()); + Event deleteEvent = new DefaultEvent(prepareCREvent().getRelatedCustomResourceID(), DELETED); defaultEventHandler.handleEvent(deleteEvent); @@ -303,19 +301,18 @@ private CustomResourceID eventAlreadyUnderProcessing() { return event.getRelatedCustomResourceID(); } - private CustomResourceEvent prepareCREvent() { + private DefaultEvent prepareCREvent() { return prepareCREvent(new CustomResourceID(UUID.randomUUID().toString(), TEST_NAMESPACE)); } - private CustomResourceEvent prepareCREvent(CustomResourceID uid) { + private DefaultEvent prepareCREvent(CustomResourceID uid) { TestCustomResource customResource = testCustomResource(uid); when(resourceCacheMock.getCustomResource(eq(uid))).thenReturn(Optional.of(customResource)); - return new CustomResourceEvent(ResourceAction.UPDATED, - CustomResourceID.fromResource(customResource)); + return new DefaultEvent(CustomResourceID.fromResource(customResource), Type.UPDATED); } private Event nonCREvent(CustomResourceID relatedCustomResourceUid) { - return new DefaultEvent(relatedCustomResourceUid); + return new DefaultEvent(relatedCustomResourceUid, Type.OTHER); } private void overrideData(CustomResourceID id, CustomResource applyTo) { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java index da32b467e0..2e8950fe27 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java @@ -20,12 +20,12 @@ import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; +import io.javaoperatorsdk.operator.processing.event.DefaultEvent; import io.javaoperatorsdk.operator.processing.event.Event; -import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; -import io.javaoperatorsdk.operator.processing.event.internal.ResourceAction; +import io.javaoperatorsdk.operator.processing.event.Event.Type; -import static io.javaoperatorsdk.operator.processing.event.internal.ResourceAction.ADDED; -import static io.javaoperatorsdk.operator.processing.event.internal.ResourceAction.UPDATED; +import static io.javaoperatorsdk.operator.processing.event.Event.Type.ADDED; +import static io.javaoperatorsdk.operator.processing.event.Event.Type.UPDATED; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -329,9 +329,9 @@ private void removeFinalizers(CustomResource customResource) { } public ExecutionScope executionScopeWithCREvent( - ResourceAction action, CustomResource resource, Event... otherEvents) { - CustomResourceEvent event = - new CustomResourceEvent(action, CustomResourceID.fromResource(resource)); + Type type, CustomResource resource, Event... otherEvents) { + DefaultEvent event = + new DefaultEvent(CustomResourceID.fromResource(resource), type); List eventList = new ArrayList<>(1 + otherEvents.length); eventList.add(event); eventList.addAll(Arrays.asList(otherEvents)); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java index 3e9e413f1e..e05fb60c61 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventFilterTest.java @@ -14,6 +14,7 @@ import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.DefaultControllerConfiguration; import io.javaoperatorsdk.operator.processing.ConfiguredController; +import io.javaoperatorsdk.operator.processing.event.Event.Type; import io.javaoperatorsdk.operator.processing.event.EventHandler; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; @@ -51,13 +52,13 @@ public void eventFilteredByCustomPredicate() { cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(ResourceAction.UPDATED, cr, null); + eventSource.eventReceived(Type.UPDATED, cr, null); verify(eventHandler, times(1)).handleEvent(any()); cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(ResourceAction.UPDATED, cr, cr); + eventSource.eventReceived(Type.UPDATED, cr, cr); verify(eventHandler, times(1)).handleEvent(any()); } @@ -84,13 +85,13 @@ public void eventFilteredByCustomPredicateAndGenerationAware() { cr.getMetadata().setGeneration(2L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(ResourceAction.UPDATED, cr, cr2); + eventSource.eventReceived(Type.UPDATED, cr, cr2); verify(eventHandler, times(1)).handleEvent(any()); cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("2"); - eventSource.eventReceived(ResourceAction.UPDATED, cr, cr); + eventSource.eventReceived(Type.UPDATED, cr, cr); verify(eventHandler, times(1)).handleEvent(any()); } @@ -114,13 +115,13 @@ public void eventNotFilteredByCustomPredicateIfFinalizerIsRequired() { cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(ResourceAction.UPDATED, cr, cr); + eventSource.eventReceived(Type.UPDATED, cr, cr); verify(eventHandler, times(1)).handleEvent(any()); cr.getMetadata().setGeneration(1L); cr.getStatus().setConfigMapStatus("1"); - eventSource.eventReceived(ResourceAction.UPDATED, cr, cr); + eventSource.eventReceived(Type.UPDATED, cr, cr); verify(eventHandler, times(2)).handleEvent(any()); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java index b8e4381f3b..dae94e80ba 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/internal/CustomResourceEventSourceTest.java @@ -15,6 +15,7 @@ import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.ConfiguredController; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; +import io.javaoperatorsdk.operator.processing.event.Event.Type; import io.javaoperatorsdk.operator.processing.event.EventHandler; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; @@ -48,11 +49,11 @@ public void skipsEventHandlingIfGenerationNotIncreased() { TestCustomResource oldCustomResource = TestUtils.testCustomResource(); oldCustomResource.getMetadata().setFinalizers(List.of(FINALIZER)); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource, + customResourceEventSource.eventReceived(Type.UPDATED, customResource, oldCustomResource); verify(eventHandler, times(1)).handleEvent(any()); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource, + customResourceEventSource.eventReceived(Type.UPDATED, customResource, customResource); verify(eventHandler, times(1)).handleEvent(any()); } @@ -61,13 +62,13 @@ public void skipsEventHandlingIfGenerationNotIncreased() { public void dontSkipEventHandlingIfMarkedForDeletion() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResourceEventSource.eventReceived(Type.UPDATED, customResource1, customResource1); verify(eventHandler, times(1)).handleEvent(any()); // mark for deletion customResource1.getMetadata().setDeletionTimestamp(LocalDateTime.now().toString()); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResourceEventSource.eventReceived(Type.UPDATED, customResource1, customResource1); verify(eventHandler, times(2)).handleEvent(any()); } @@ -76,12 +77,12 @@ public void dontSkipEventHandlingIfMarkedForDeletion() { public void normalExecutionIfGenerationChanges() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResourceEventSource.eventReceived(Type.UPDATED, customResource1, customResource1); verify(eventHandler, times(1)).handleEvent(any()); customResource1.getMetadata().setGeneration(2L); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResourceEventSource.eventReceived(Type.UPDATED, customResource1, customResource1); verify(eventHandler, times(2)).handleEvent(any()); } @@ -94,11 +95,11 @@ public void handlesAllEventIfNotGenerationAware() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResourceEventSource.eventReceived(Type.UPDATED, customResource1, customResource1); verify(eventHandler, times(1)).handleEvent(any()); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResourceEventSource.eventReceived(Type.UPDATED, customResource1, customResource1); verify(eventHandler, times(2)).handleEvent(any()); } @@ -107,7 +108,7 @@ public void handlesAllEventIfNotGenerationAware() { public void eventWithNoGenerationProcessedIfNoFinalizer() { TestCustomResource customResource1 = TestUtils.testCustomResource(); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource1, + customResourceEventSource.eventReceived(Type.UPDATED, customResource1, customResource1); verify(eventHandler, times(1)).handleEvent(any()); @@ -119,7 +120,7 @@ public void handlesNextEventIfWhitelisted() { customResource.getMetadata().setFinalizers(List.of(FINALIZER)); customResourceEventSource.whitelistNextEvent(CustomResourceID.fromResource(customResource)); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource, + customResourceEventSource.eventReceived(Type.UPDATED, customResource, customResource); verify(eventHandler, times(1)).handleEvent(any()); @@ -130,7 +131,7 @@ public void notHandlesNextEventIfNotWhitelisted() { TestCustomResource customResource = TestUtils.testCustomResource(); customResource.getMetadata().setFinalizers(List.of(FINALIZER)); - customResourceEventSource.eventReceived(ResourceAction.UPDATED, customResource, + customResourceEventSource.eventReceived(Type.UPDATED, customResource, customResource); verify(eventHandler, times(0)).handleEvent(any()); From 0c30922c780bb43f9658b029738348c4295c33f2 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 21:10:51 +0200 Subject: [PATCH 09/13] refactor: add triggering Event on ExecutionScope and propagate it --- .../processing/DefaultEventHandler.java | 22 +++++++++---------- .../operator/processing/ExecutionScope.java | 9 +++++++- .../processing/DefaultEventHandlerTest.java | 21 ++++++++++++------ .../processing/EventDispatcherTest.java | 15 +++---------- 4 files changed, 36 insertions(+), 31 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index 8621454ad5..cf059c203e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -107,16 +107,17 @@ public void handleEvent(Event event) { handleEventMarking(event); if (!eventMarker.deleteEventPresent(resourceID)) { - submitReconciliationExecution(resourceID); + submitReconciliationExecution(event); } else { - cleanupForDeletedEvent(resourceID); + cleanupForDeletedEvent(event); } } finally { lock.unlock(); } } - private boolean submitReconciliationExecution(CustomResourceID customResourceUid) { + private boolean submitReconciliationExecution(Event event) { + final var customResourceUid = event.getRelatedCustomResourceID(); boolean controllerUnderExecution = isControllerUnderExecution(customResourceUid); Optional latestCustomResource = resourceCache.getCustomResource(customResourceUid); @@ -127,7 +128,7 @@ private boolean submitReconciliationExecution(CustomResourceID customResourceUid ExecutionScope executionScope = new ExecutionScope<>( latestCustomResource.get(), - retryInfo(customResourceUid)); + retryInfo(customResourceUid), event); eventMarker.unMarkEventReceived(customResourceUid); log.debug("Executing events for custom resource. Scope: {}", executionScope); executor.execute(new ControllerExecution(executionScope)); @@ -179,18 +180,16 @@ void eventProcessingFinished( if (isRetryConfigured() && postExecutionControl.exceptionDuringExecution() && !eventMarker.deleteEventPresent(customResourceID)) { handleRetryOnException(executionScope); - // todo revisit monitoring since events are not present anymore - // final var monitor = monitor(); executionScope.getEvents().forEach(e -> - // monitor.failedEvent(executionScope.getCustomResourceID(), e)); + monitor().failedEvent(customResourceID, executionScope.getTriggeringEvent()); return; } cleanupOnSuccessfulExecution(executionScope); if (eventMarker.deleteEventPresent(customResourceID)) { - cleanupForDeletedEvent(executionScope.getCustomResourceID()); + cleanupForDeletedEvent(executionScope.getTriggeringEvent()); } else { if (eventMarker.eventPresent(customResourceID)) { if (isCacheReadyForInstantReconciliation(executionScope, postExecutionControl)) { - submitReconciliationExecution(customResourceID); + submitReconciliationExecution(executionScope.getTriggeringEvent()); } else { postponeReconciliationAndHandleCacheSyncEvent(customResourceID); } @@ -257,7 +256,7 @@ private void handleRetryOnException(ExecutionScope executionScope) { if (eventPresent) { log.debug("New events exists for for resource id: {}", customResourceID); - submitReconciliationExecution(customResourceID); + submitReconciliationExecution(executionScope.getTriggeringEvent()); return; } Optional nextDelay = execution.nextDelay(); @@ -296,7 +295,8 @@ private RetryExecution getOrInitRetryExecution(ExecutionScope executionScope) return retryExecution; } - private void cleanupForDeletedEvent(CustomResourceID customResourceUid) { + private void cleanupForDeletedEvent(Event event) { + final var customResourceUid = event.getRelatedCustomResourceID(); eventSourceManager.cleanupForCustomResource(customResourceUid); eventMarker.cleanup(customResourceUid); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ExecutionScope.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ExecutionScope.java index 6cf05e9308..fd4890f6f5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ExecutionScope.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/ExecutionScope.java @@ -3,16 +3,19 @@ import io.fabric8.kubernetes.client.CustomResource; import io.javaoperatorsdk.operator.api.RetryInfo; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; +import io.javaoperatorsdk.operator.processing.event.Event; public class ExecutionScope> { // the latest custom resource from cache private final R customResource; private final RetryInfo retryInfo; + private final Event triggeringEvent; - public ExecutionScope(R customResource, RetryInfo retryInfo) { + ExecutionScope(R customResource, RetryInfo retryInfo, Event triggeringEvent) { this.customResource = customResource; this.retryInfo = retryInfo; + this.triggeringEvent = triggeringEvent; } public R getCustomResource() { @@ -23,6 +26,10 @@ public CustomResourceID getCustomResourceID() { return CustomResourceID.fromResource(customResource); } + public Event getTriggeringEvent() { + return triggeringEvent; + } + @Override public String toString() { return "ExecutionScope{" diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java index ddb0a4a3e9..d14befdbb1 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/DefaultEventHandlerTest.java @@ -24,6 +24,8 @@ import static io.javaoperatorsdk.operator.TestUtils.testCustomResource; import static io.javaoperatorsdk.operator.processing.event.Event.Type.DELETED; +import static io.javaoperatorsdk.operator.processing.event.Event.Type.OTHER; +import static io.javaoperatorsdk.operator.processing.event.Event.Type.UPDATED; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.*; @@ -90,7 +92,8 @@ public void ifExecutionInProgressWaitsUntilItsFinished() throws InterruptedExcep public void schedulesAnEventRetryOnException() { TestCustomResource customResource = testCustomResource(); - ExecutionScope executionScope = new ExecutionScope(customResource, null); + ExecutionScope executionScope = new ExecutionScope(customResource, null, + new DefaultEvent(CustomResourceID.fromResource(customResource), UPDATED)); PostExecutionControl postExecutionControl = PostExecutionControl.exceptionDuringExecution(new RuntimeException("test")); @@ -213,9 +216,9 @@ public void cleansUpWhenDeleteEventReceivedAndNoEventPresent() { @Test public void cleansUpAfterExecutionIfOnlyDeleteEventMarkLeft() { var cr = testCustomResource(); - var crEvent = prepareCREvent(CustomResourceID.fromResource(cr)); + var crEvent = new DefaultEvent(CustomResourceID.fromResource(cr), DELETED); eventMarker.markDeleteEventReceived(crEvent.getRelatedCustomResourceID()); - var executionScope = new ExecutionScope(cr, null); + var executionScope = new ExecutionScope(cr, null, crEvent); defaultEventHandler.eventProcessingFinished(executionScope, PostExecutionControl.defaultDispatch()); @@ -236,7 +239,8 @@ public void whitelistNextEventIfTheCacheIsNotPropagatedAfterAnUpdate() { when(defaultEventSourceManagerMock.getCustomResourceEventSource()) .thenReturn(mockCREventSource); - defaultEventHandler.eventProcessingFinished(new ExecutionScope(cr, null), + defaultEventHandler.eventProcessingFinished( + new ExecutionScope(cr, null, new DefaultEvent(crID, UPDATED)), PostExecutionControl.customResourceUpdated(updatedCr)); verify(mockCREventSource, times(1)).whitelistNextEvent(eq(crID)); @@ -251,12 +255,13 @@ public void dontWhitelistsEventWhenOtherChangeDuringExecution() { var otherChangeCR = testCustomResource(crID); otherChangeCR.getMetadata().setResourceVersion("3"); var mockCREventSource = mock(CustomResourceEventSource.class); + final var event = new DefaultEvent(crID, Type.UPDATED); eventMarker.markEventReceived(crID); when(resourceCacheMock.getCustomResource(eq(crID))).thenReturn(Optional.of(otherChangeCR)); when(defaultEventSourceManagerMock.getCustomResourceEventSource()) .thenReturn(mockCREventSource); - defaultEventHandler.eventProcessingFinished(new ExecutionScope(cr, null), + defaultEventHandler.eventProcessingFinished(new ExecutionScope(cr, null, event), PostExecutionControl.customResourceUpdated(updatedCr)); verify(mockCREventSource, times(0)).whitelistNextEvent(eq(crID)); @@ -272,7 +277,8 @@ public void dontWhitelistsEventIfUpdatedEventInCache() { when(defaultEventSourceManagerMock.getCustomResourceEventSource()) .thenReturn(mockCREventSource); - defaultEventHandler.eventProcessingFinished(new ExecutionScope(cr, null), + defaultEventHandler.eventProcessingFinished( + new ExecutionScope(cr, null, new DefaultEvent(crID, UPDATED)), PostExecutionControl.customResourceUpdated(cr)); verify(mockCREventSource, times(0)).whitelistNextEvent(eq(crID)); @@ -283,7 +289,8 @@ public void cancelScheduleOnceEventsOnSuccessfulExecution() { var crID = new CustomResourceID("test-cr", TEST_NAMESPACE); var cr = testCustomResource(crID); - defaultEventHandler.eventProcessingFinished(new ExecutionScope(cr, null), + defaultEventHandler.eventProcessingFinished( + new ExecutionScope(cr, null, new DefaultEvent(crID, OTHER)), PostExecutionControl.defaultDispatch()); verify(retryTimerEventSourceMock, times(1)).cancelOnceSchedule(eq(crID)); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java index 2e8950fe27..1574c4efa6 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/EventDispatcherTest.java @@ -1,9 +1,5 @@ package io.javaoperatorsdk.operator.processing; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; - import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -21,7 +17,6 @@ import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEvent; -import io.javaoperatorsdk.operator.processing.event.Event; import io.javaoperatorsdk.operator.processing.event.Event.Type; import static io.javaoperatorsdk.operator.processing.event.Event.Type.ADDED; @@ -279,7 +274,7 @@ public int getAttemptCount() { public boolean isLastAttempt() { return true; } - })); + }, new DefaultEvent(CustomResourceID.fromResource(testCustomResource), UPDATED))); ArgumentCaptor> contextArgumentCaptor = ArgumentCaptor.forClass(Context.class); @@ -328,13 +323,9 @@ private void removeFinalizers(CustomResource customResource) { customResource.getMetadata().getFinalizers().clear(); } - public ExecutionScope executionScopeWithCREvent( - Type type, CustomResource resource, Event... otherEvents) { + public ExecutionScope executionScopeWithCREvent(Type type, CustomResource resource) { DefaultEvent event = new DefaultEvent(CustomResourceID.fromResource(resource), type); - List eventList = new ArrayList<>(1 + otherEvents.length); - eventList.add(event); - eventList.addAll(Arrays.asList(otherEvents)); - return new ExecutionScope(resource, null); + return new ExecutionScope(resource, null, event); } } From 24d09026d53fb6f3b6f855de961a89c3033fcde0 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 21:15:52 +0200 Subject: [PATCH 10/13] refactor: simplify EvenMonitor methods --- .../monitoring/micrometer/MicrometerMetrics.java | 5 ++--- .../operator/api/monitoring/EventMonitor.java | 9 ++++----- .../operator/processing/DefaultEventHandler.java | 4 ++-- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java index 7128670e8e..f89c4bfe1d 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java @@ -5,7 +5,6 @@ import io.javaoperatorsdk.operator.api.monitoring.EventMonitor; import io.javaoperatorsdk.operator.api.monitoring.Metrics; -import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; import io.micrometer.core.instrument.MeterRegistry; import io.micrometer.core.instrument.Timer; @@ -16,12 +15,12 @@ public class MicrometerMetrics implements Metrics { private final MeterRegistry registry; private final EventMonitor monitor = new EventMonitor() { @Override - public void processedEvent(CustomResourceID uid, Event event) { + public void processedEvent(Event event) { incrementProcessedEventsNumber(); } @Override - public void failedEvent(CustomResourceID uid, Event event) { + public void failedEvent(Event event) { incrementControllerRetriesNumber(); } }; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java index e56895ad77..59a3b20db1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java @@ -1,19 +1,18 @@ package io.javaoperatorsdk.operator.api.monitoring; -import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.Event; public interface EventMonitor { EventMonitor NOOP = new EventMonitor() { @Override - public void processedEvent(CustomResourceID uid, Event event) {} + public void processedEvent(Event event) {} @Override - public void failedEvent(CustomResourceID uid, Event event) {} + public void failedEvent(Event event) {} }; - void processedEvent(CustomResourceID uid, Event event); + void processedEvent(Event event); - void failedEvent(CustomResourceID uid, Event event); + void failedEvent(Event event); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index cf059c203e..c592b880b3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -103,7 +103,7 @@ public void handleEvent(Event event) { } final var monitor = monitor(); final var resourceID = event.getRelatedCustomResourceID(); - monitor.processedEvent(resourceID, event); + monitor.processedEvent(event); handleEventMarking(event); if (!eventMarker.deleteEventPresent(resourceID)) { @@ -180,7 +180,7 @@ void eventProcessingFinished( if (isRetryConfigured() && postExecutionControl.exceptionDuringExecution() && !eventMarker.deleteEventPresent(customResourceID)) { handleRetryOnException(executionScope); - monitor().failedEvent(customResourceID, executionScope.getTriggeringEvent()); + monitor().failedEvent(executionScope.getTriggeringEvent()); return; } cleanupOnSuccessfulExecution(executionScope); From 045ae1030e24b2d224b4ad0073772fb40defdd1a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 22:36:57 +0200 Subject: [PATCH 11/13] refactor: remove EventMonitor altogether and improve counter reporting --- .../micrometer/MicrometerMetrics.java | 48 +++++++++++-------- .../operator/api/monitoring/EventMonitor.java | 18 ------- .../operator/api/monitoring/Metrics.java | 16 +++---- .../processing/DefaultEventHandler.java | 27 +++++------ 4 files changed, 48 insertions(+), 61 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java diff --git a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java index f89c4bfe1d..354942c399 100644 --- a/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java +++ b/micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java @@ -1,9 +1,10 @@ package io.javaoperatorsdk.operator.monitoring.micrometer; import java.util.Collections; +import java.util.LinkedList; +import java.util.List; import java.util.Map; -import io.javaoperatorsdk.operator.api.monitoring.EventMonitor; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.event.Event; import io.micrometer.core.instrument.MeterRegistry; @@ -13,17 +14,6 @@ public class MicrometerMetrics implements Metrics { public static final String PREFIX = "operator.sdk."; private final MeterRegistry registry; - private final EventMonitor monitor = new EventMonitor() { - @Override - public void processedEvent(Event event) { - incrementProcessedEventsNumber(); - } - - @Override - public void failedEvent(Event event) { - incrementControllerRetriesNumber(); - } - }; public MicrometerMetrics(MeterRegistry registry) { this.registry = registry; @@ -63,21 +53,37 @@ public void incrementControllerRetriesNumber() { } - public void incrementProcessedEventsNumber() { - registry - .counter( - PREFIX + "total.events.received", "events", "totalEvents", "type", - "eventsReceived") - .increment(); + public void processingEvent(Event event) { + incrementCounter(event, "events.received"); + } + public void processedEvent(Event event) { + incrementCounter(event, "events.processed"); + } + + public void failedEvent(Event event, RuntimeException exception) { + var cause = exception.getCause(); + if (cause == null) { + cause = exception; + } else if (cause instanceof RuntimeException) { + cause = cause.getCause() != null ? cause.getCause() : cause; + } + incrementCounter(event, "events.failed", "exception", cause.getClass().getSimpleName()); } public > T monitorSizeOf(T map, String name) { return registry.gaugeMapSize(PREFIX + name + ".size", Collections.emptyList(), map); } - @Override - public EventMonitor getEventMonitor() { - return monitor; + private void incrementCounter(Event event, String counterName, String... additionalTags) { + final var id = event.getRelatedCustomResourceID(); + var tags = List.of("namespace", id.getNamespace().orElse(""), + "scope", id.getNamespace().isPresent() ? "namespace" : "cluster", + "type", event.getType().name()); + if (additionalTags != null && additionalTags.length > 0) { + tags = new LinkedList<>(tags); + tags.addAll(List.of(additionalTags)); + } + registry.counter(PREFIX + counterName, tags.toArray(new String[0])).increment(); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java deleted file mode 100644 index 59a3b20db1..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/EventMonitor.java +++ /dev/null @@ -1,18 +0,0 @@ -package io.javaoperatorsdk.operator.api.monitoring; - -import io.javaoperatorsdk.operator.processing.event.Event; - -public interface EventMonitor { - - EventMonitor NOOP = new EventMonitor() { - @Override - public void processedEvent(Event event) {} - - @Override - public void failedEvent(Event event) {} - }; - - void processedEvent(Event event); - - void failedEvent(Event event); -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java index ddbb469c92..4b005695e5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/monitoring/Metrics.java @@ -2,9 +2,17 @@ import java.util.Map; +import io.javaoperatorsdk.operator.processing.event.Event; + public interface Metrics { Metrics NOOP = new Metrics() {}; + default void processingEvent(Event event) {} + + default void processedEvent(Event event) {} + + default void failedEvent(Event event, RuntimeException exception) {} + interface ControllerExecution { String name(); @@ -20,15 +28,7 @@ default T timeControllerExecution(ControllerExecution execution) { return execution.execute(); } - default void incrementControllerRetriesNumber() {} - - default void incrementProcessedEventsNumber() {} - default > T monitorSizeOf(T map, String name) { return map; } - - default EventMonitor getEventMonitor() { - return EventMonitor.NOOP; - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index c592b880b3..cc78a0d78e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -16,7 +16,7 @@ import io.javaoperatorsdk.operator.api.RetryInfo; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; -import io.javaoperatorsdk.operator.api.monitoring.EventMonitor; +import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.processing.event.CustomResourceID; import io.javaoperatorsdk.operator.processing.event.DefaultEventSourceManager; import io.javaoperatorsdk.operator.processing.event.Event; @@ -43,7 +43,7 @@ public class DefaultEventHandler> implements Even private final ExecutorService executor; private final String controllerName; private final ReentrantLock lock = new ReentrantLock(); - private final EventMonitor eventMonitor; + private final Metrics metrics; private volatile boolean running; private final ResourceCache resourceCache; private DefaultEventSourceManager eventSourceManager; @@ -56,7 +56,7 @@ public DefaultEventHandler(ConfiguredController controller, ResourceCache controller.getConfiguration().getName(), new EventDispatcher<>(controller), GenericRetry.fromConfiguration(controller.getConfiguration().getRetryConfiguration()), - controller.getConfiguration().getConfigurationService().getMetrics().getEventMonitor(), + controller.getConfiguration().getConfigurationService().getMetrics(), new EventMarker()); } @@ -68,7 +68,7 @@ public DefaultEventHandler(ConfiguredController controller, ResourceCache private DefaultEventHandler(ResourceCache resourceCache, ExecutorService executor, String relatedControllerName, - EventDispatcher eventDispatcher, Retry retry, EventMonitor monitor, + EventDispatcher eventDispatcher, Retry retry, Metrics metrics, EventMarker eventMarker) { this.running = true; this.executor = @@ -80,7 +80,7 @@ private DefaultEventHandler(ResourceCache resourceCache, ExecutorService exec this.eventDispatcher = eventDispatcher; this.retry = retry; this.resourceCache = resourceCache; - this.eventMonitor = monitor != null ? monitor : EventMonitor.NOOP; + this.metrics = metrics != null ? metrics : Metrics.NOOP; this.eventMarker = eventMarker; } @@ -88,10 +88,6 @@ public void setEventSourceManager(DefaultEventSourceManager eventSourceManage this.eventSourceManager = eventSourceManager; } - private EventMonitor monitor() { - return eventMonitor; - } - @Override public void handleEvent(Event event) { lock.lock(); @@ -101,9 +97,8 @@ public void handleEvent(Event event) { log.debug("Skipping event: {} because the event handler is shutting down", event); return; } - final var monitor = monitor(); final var resourceID = event.getRelatedCustomResourceID(); - monitor.processedEvent(event); + metrics.processingEvent(event); handleEventMarking(event); if (!eventMarker.deleteEventPresent(resourceID)) { @@ -111,6 +106,8 @@ public void handleEvent(Event event) { } else { cleanupForDeletedEvent(event); } + + metrics.processedEvent(event); } finally { lock.unlock(); } @@ -179,8 +176,8 @@ void eventProcessingFinished( // Either way we don't want to retry. if (isRetryConfigured() && postExecutionControl.exceptionDuringExecution() && !eventMarker.deleteEventPresent(customResourceID)) { - handleRetryOnException(executionScope); - monitor().failedEvent(executionScope.getTriggeringEvent()); + handleRetryOnException(executionScope, + postExecutionControl.getRuntimeException().orElseThrow()); return; } cleanupOnSuccessfulExecution(executionScope); @@ -247,7 +244,8 @@ private void reScheduleExecutionIfInstructed(PostExecutionControl postExecuti * events (received meanwhile retry is in place or already in buffer) instantly or always wait * according to the retry timing if there was an exception. */ - private void handleRetryOnException(ExecutionScope executionScope) { + private void handleRetryOnException(ExecutionScope executionScope, + RuntimeException exception) { RetryExecution execution = getOrInitRetryExecution(executionScope); var customResourceID = executionScope.getCustomResourceID(); boolean eventPresent = eventMarker.eventPresent(customResourceID); @@ -267,6 +265,7 @@ private void handleRetryOnException(ExecutionScope executionScope) { "Scheduling timer event for retry with delay:{} for resource: {}", delay, customResourceID); + metrics.failedEvent(executionScope.getTriggeringEvent(), exception); eventSourceManager .getRetryAndRescheduleTimerEventSource() .scheduleOnce(executionScope.getCustomResource(), delay); From ec137dc1fff73515f6a95ab8e7d98d0caa444ca5 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 22:42:34 +0200 Subject: [PATCH 12/13] fix: more generics fixes --- .../processing/event/DefaultEventSourceManager.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java index d72bd6ec7e..41cb77a803 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/DefaultEventSourceManager.java @@ -15,7 +15,7 @@ import io.javaoperatorsdk.operator.processing.event.internal.TimerEventSource; public class DefaultEventSourceManager> - implements EventSourceManager { + implements EventSourceManager { private static final Logger log = LoggerFactory.getLogger(DefaultEventSourceManager.class); @@ -23,7 +23,7 @@ public class DefaultEventSourceManager> private final Set eventSources = Collections.synchronizedSet(new HashSet<>()); private DefaultEventHandler defaultEventHandler; private TimerEventSource retryAndRescheduleTimerEventSource; - private CustomResourceEventSource customResourceEventSource; + private CustomResourceEventSource customResourceEventSource; DefaultEventSourceManager(DefaultEventHandler defaultEventHandler) { init(defaultEventHandler); @@ -57,7 +57,7 @@ public void close() { try { eventSource.close(); } catch (Exception e) { - log.warn("Error closing {} -> {}", eventSource); + log.warn("Error closing {} -> {}", eventSource, e); } } eventSources.clear(); @@ -98,7 +98,7 @@ public void cleanupForCustomResource(CustomResourceID customResourceUid) { } } - public TimerEventSource getRetryAndRescheduleTimerEventSource() { + public TimerEventSource getRetryAndRescheduleTimerEventSource() { return retryAndRescheduleTimerEventSource; } From df6f66413feae65430157838ea1e8d0591e32c6a Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 21 Oct 2021 22:43:32 +0200 Subject: [PATCH 13/13] fix: minor clean-ups --- .../operator/processing/DefaultEventHandler.java | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java index cc78a0d78e..d1e24e3ca4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java @@ -113,7 +113,7 @@ public void handleEvent(Event event) { } } - private boolean submitReconciliationExecution(Event event) { + private void submitReconciliationExecution(Event event) { final var customResourceUid = event.getRelatedCustomResourceID(); boolean controllerUnderExecution = isControllerUnderExecution(customResourceUid); Optional latestCustomResource = @@ -129,7 +129,6 @@ private boolean submitReconciliationExecution(Event event) { eventMarker.unMarkEventReceived(customResourceUid); log.debug("Executing events for custom resource. Scope: {}", executionScope); executor.execute(new ControllerExecution(executionScope)); - return true; } else { log.debug( "Skipping executing controller for resource id: {}." @@ -141,7 +140,6 @@ private boolean submitReconciliationExecution(Event event) { log.warn("no custom resource found in cache for CustomResourceID: {}", customResourceUid); } - return false; } } @@ -222,14 +220,11 @@ private boolean isCacheReadyForInstantReconciliation(ExecutionScope execution if (cachedCustomResourceVersion.equals(customResourceVersionAfterExecution)) { return true; } - if (cachedCustomResourceVersion.equals(originalResourceVersion)) { - return false; - } // If the cached resource version equals neither the version before nor after execution // probably an update happened on the custom resource independent of the framework during // reconciliation. We cannot tell at this point if it happened before our update or before. // (Well we could if we would parse resource version, but that should not be done by definition) - return true; + return !cachedCustomResourceVersion.equals(originalResourceVersion); } private void reScheduleExecutionIfInstructed(PostExecutionControl postExecutionControl,