From 39902855f829dfad13f8f3c5128fe207be125bd0 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 7 Jan 2022 14:26:39 +0100 Subject: [PATCH 1/4] fix: minor improvements --- .../operator/processing/event/EventSourceManager.java | 6 +++--- .../event/source/controller/ResourceEventFilters.java | 2 -- .../event/source/polling/PerResourcePollingEventSource.java | 3 +++ .../operator/sample/MySQLSchemaOperator.java | 2 ++ .../io/javaoperatorsdk/operator/sample/TomcatOperator.java | 1 + .../io/javaoperatorsdk/operator/sample/WebPageOperator.java | 1 + 6 files changed, 10 insertions(+), 5 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 f158eccf68..55f42cd438 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 @@ -40,7 +40,7 @@ public class EventSourceManager implements LifecycleAware EventSourceManager(EventProcessor eventProcessor) { this.eventProcessor = eventProcessor; controller = null; - registerEventSource(eventSources.initRetryEventSource()); + registerEventSource(eventSources.retryEventSource()); } public EventSourceManager(Controller controller) { @@ -48,7 +48,7 @@ public EventSourceManager(Controller controller) { // controller event source needs to be available before we create the event processor final var controllerEventSource = eventSources.initControllerEventSource(controller); this.eventProcessor = new EventProcessor<>(this); - registerEventSource(eventSources.initRetryEventSource()); + registerEventSource(eventSources.retryEventSource()); registerEventSource(controllerEventSource); } @@ -184,7 +184,7 @@ ControllerResourceEventSource initControllerEventSource(Controller control return controllerResourceEventSource; } - TimerEventSource initRetryEventSource() { + TimerEventSource retryEventSource() { return retryAndRescheduleTimerEventSource; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ResourceEventFilters.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ResourceEventFilters.java index 43fe410fbc..d287361c2b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ResourceEventFilters.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ResourceEventFilters.java @@ -25,8 +25,6 @@ public final class ResourceEventFilters { private static final ResourceEventFilter GENERATION_AWARE = (configuration, oldResource, newResource) -> { final var generationAware = configuration.isGenerationAware(); - // todo: change this to check for HasStatus (or similar) when - // https://github.com/fabric8io/kubernetes-client/issues/3586 is fixed if (newResource instanceof CustomResource) { var newCustomResource = (CustomResource) newResource; final var status = newCustomResource.getStatus(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java index 0afcb1afae..a7d2442866 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java @@ -99,6 +99,9 @@ public void onResourceDeleted(R resource) { private void checkAndRegisterTask(R resource) { var resourceID = ResourceID.fromResource(resource); + // Events are coming always from the same thread, no need for explicit locking for timerTasks + // related + // (multiple) operations in this method if (timerTasks.get(resourceID) == null && (registerPredicate == null || registerPredicate.test(resource))) { var task = new TimerTask() { diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java index e8f25c84bd..fbb9ae2b65 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java @@ -27,8 +27,10 @@ public static void main(String[] args) throws IOException { KubernetesClient client = new DefaultKubernetesClient(config); Operator operator = new Operator(client, DefaultConfigurationService.instance()); operator.register(new MySQLSchemaReconciler(client, MySQLDbConfig.loadFromEnvironmentVars())); + operator.installShutdownHook(); operator.start(); + new FtBasic(new TkFork(new FkRegex("/health", "ALL GOOD!")), 8080).start(Exit.NEVER); } } diff --git a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java index a7a7ca40ff..487183dfe5 100644 --- a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java +++ b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java @@ -27,6 +27,7 @@ public static void main(String[] args) throws IOException { Operator operator = new Operator(client, DefaultConfigurationService.instance()); operator.register(new TomcatReconciler(client)); operator.register(new WebappReconciler(client)); + operator.installShutdownHook(); operator.start(); new FtBasic(new TkFork(new FkRegex("/health", "ALL GOOD.")), 8080).start(Exit.NEVER); diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java index 4940119ba9..768fd26a72 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java @@ -27,6 +27,7 @@ public static void main(String[] args) throws IOException { KubernetesClient client = new DefaultKubernetesClient(config); Operator operator = new Operator(client, DefaultConfigurationService.instance()); operator.register(new WebPageReconciler(client)); + operator.installShutdownHook(); operator.start(); new FtBasic(new TkFork(new FkRegex("/health", "ALL GOOD!")), 8080).start(Exit.NEVER); From 8b856700c385481fa2971f05ca987c5736eb677d Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 7 Jan 2022 14:47:47 +0100 Subject: [PATCH 2/4] docs: comment improved --- .../event/source/polling/PerResourcePollingEventSource.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java index a7d2442866..05f7266e16 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java @@ -97,11 +97,11 @@ public void onResourceDeleted(R resource) { cache.remove(resourceID); } + // This method is always called from the same Thread with for the same resource, + // since events from ResourceEventAware only the thread of the informer. This is important + // because otherwise there will be a race condition related to the timerTasks. private void checkAndRegisterTask(R resource) { var resourceID = ResourceID.fromResource(resource); - // Events are coming always from the same thread, no need for explicit locking for timerTasks - // related - // (multiple) operations in this method if (timerTasks.get(resourceID) == null && (registerPredicate == null || registerPredicate.test(resource))) { var task = new TimerTask() { From 6a85fe9d747fcad34c1e4d4eecd3872557f0f56f Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 7 Jan 2022 14:57:35 +0100 Subject: [PATCH 3/4] fix: comment --- .../event/source/polling/PerResourcePollingEventSource.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java index 05f7266e16..229d2e5d5e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java @@ -97,8 +97,8 @@ public void onResourceDeleted(R resource) { cache.remove(resourceID); } - // This method is always called from the same Thread with for the same resource, - // since events from ResourceEventAware only the thread of the informer. This is important + // This method is always called from the same Thread for the same resource, + // since events from ResourceEventAware are propagated from the thread of the informer. This is important // because otherwise there will be a race condition related to the timerTasks. private void checkAndRegisterTask(R resource) { var resourceID = ResourceID.fromResource(resource); From 5b6e9d94c3ae2335c07d1d352911400f910c0b95 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 7 Jan 2022 15:00:08 +0100 Subject: [PATCH 4/4] fix: format --- .../event/source/polling/PerResourcePollingEventSource.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java index 229d2e5d5e..465b3c617a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/polling/PerResourcePollingEventSource.java @@ -98,7 +98,8 @@ public void onResourceDeleted(R resource) { } // This method is always called from the same Thread for the same resource, - // since events from ResourceEventAware are propagated from the thread of the informer. This is important + // since events from ResourceEventAware are propagated from the thread of the informer. This is + // important // because otherwise there will be a race condition related to the timerTasks. private void checkAndRegisterTask(R resource) { var resourceID = ResourceID.fromResource(resource);