From 4d1528cf20ee93e233233300b8f01be3c3de2369 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 14 Dec 2022 15:01:59 +0100 Subject: [PATCH 1/5] fix: make it clearer that the thread is re-interrupted --- .../processing/event/source/informer/InformerWrapper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ccb1d268ca..a872e9daa3 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 @@ -86,7 +86,7 @@ public void start() throws OperatorException { log.warn("Informer startup error. Will periodically retry. Informer: {}", informer, e); } } catch (InterruptedException e) { - Thread.currentThread().interrupt(); + thread.interrupt(); throw new IllegalStateException(e); } finally { // restore original name From d07c932e9b251339d76ca824a6c58d6e13d93268 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 14 Dec 2022 15:17:40 +0100 Subject: [PATCH 2/5] refactor: simplify map creation --- .../dependent/workflow/DefaultWorkflow.java | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java index a151ff0947..84fe404558 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java @@ -6,8 +6,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.Function; -import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -65,21 +63,24 @@ protected DefaultWorkflow(Map dependentResourceNo } @SuppressWarnings("unchecked") - private Map toMap( - Set dependentResourceNodes) { - return dependentResourceNodes.stream() - .peek(drn -> { - // add cycle detection? - if (drn.getDependsOn().isEmpty()) { - topLevelResources.add(drn); - } else { - for (DependentResourceNode dependsOn : (List) drn - .getDependsOn()) { - bottomLevelResource.remove(dependsOn); - } - } - }) - .collect(Collectors.toMap(DependentResourceNode::getName, Function.identity())); + private Map toMap(Set nodes) { + if(nodes == null || nodes.isEmpty()) { + return Collections.emptyMap(); + } + + final var map = new HashMap(nodes.size()); + for (DependentResourceNode node : nodes) { + // add cycle detection? + if (node.getDependsOn().isEmpty()) { + topLevelResources.add(node); + } else { + for (DependentResourceNode dependsOn : (List) node.getDependsOn()) { + bottomLevelResource.remove(dependsOn); + } + } + map.put(node.getName(), node); + } + return map; } @Override From 53d287db3835b2048edbfd2669d8837c0b57bf5b Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 14 Dec 2022 15:18:35 +0100 Subject: [PATCH 3/5] fix: remove unused variable --- .../event/source/informer/InformerManager.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 94788e6706..34f89dd68b 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 @@ -1,6 +1,11 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; -import java.util.*; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import java.util.function.Predicate; @@ -55,8 +60,6 @@ void initSources(MixedOperation, Resource> clien this.eventHandler = eventHandler; final var targetNamespaces = configuration.getEffectiveNamespaces(); - final var labelSelector = configuration.getLabelSelector(); - if (ResourceConfiguration.allNamespacesWatched(targetNamespaces)) { var source = createEventSourceForNamespace(WATCH_ALL_NAMESPACES); log.debug("Registered {} -> {} for any namespace", this, source); From c9985bce7fbdcd284789ee47d4151e84a67816da Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 14 Dec 2022 15:24:10 +0100 Subject: [PATCH 4/5] chore: clean-ups --- .../java/io/javaoperatorsdk/operator/Operator.java | 5 ++++- .../operator/api/config/ConfigurationService.java | 1 + .../operator/config/BaseConfigurationServiceTest.java | 10 +++++----- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 8a4b6850ac..f337c84f2a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -76,7 +76,10 @@ public Operator(KubernetesClient kubernetesClient, ConfigurationService configur .ifPresent(c -> leaderElectionManager.init(c, this.kubernetesClient)); } - /** Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. */ + /** + * Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. + * @deprecated This feature should not be used anymore + */ @Deprecated(forRemoval = true) public void installShutdownHook() { if (!leaderElectionManager.isLeaderElectionEnabled()) { 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 45d7162184..1b356ea60d 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 @@ -195,6 +195,7 @@ default Optional getInformerStoppedHandler() { }); } + @SuppressWarnings("rawtypes") default ManagedWorkflowFactory getWorkflowFactory() { return ManagedWorkflowFactory.DEFAULT; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java index 22af954661..339d2a02b4 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java @@ -126,8 +126,8 @@ void getDependentResources() { @Test void missingAnnotationThrowsException() { - Assertions.assertThrows(OperatorException.class, - () -> configFor(new MissingAnnotationReconciler())); + final var reconciler = new MissingAnnotationReconciler(); + Assertions.assertThrows(OperatorException.class, () -> configFor(reconciler)); } @SuppressWarnings("rawtypes") @@ -145,7 +145,8 @@ private Optional findByNameOptional( @Test void tryingToAddDuplicatedDependentsWithoutNameShouldFail() { - assertThrows(IllegalArgumentException.class, () -> configFor(new DuplicatedDepReconciler())); + final var reconciler = new DuplicatedDepReconciler(); + assertThrows(IllegalArgumentException.class, () -> configFor(reconciler)); } @Test @@ -179,8 +180,7 @@ void checkDefaultRateAndRetryConfigurations() { @Test void configuringRateAndRetryViaAnnotationsShouldWork() { - var config = - configFor(new ConfigurableRateLimitAndRetryReconciler()); + var config = configFor(new ConfigurableRateLimitAndRetryReconciler()); final var retry = config.getRetry(); final var testRetry = assertInstanceOf(TestRetry.class, retry); assertEquals(12, testRetry.getValue()); From 59b00b1c85e928d2ea2b58a2467b7f0311a72e1d Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 14 Dec 2022 15:28:21 +0100 Subject: [PATCH 5/5] fix: format --- .../src/main/java/io/javaoperatorsdk/operator/Operator.java | 1 + .../processing/dependent/workflow/DefaultWorkflow.java | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index f337c84f2a..12437155e9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -78,6 +78,7 @@ public Operator(KubernetesClient kubernetesClient, ConfigurationService configur /** * Adds a shutdown hook that automatically calls {@link #stop()} when the app shuts down. + * * @deprecated This feature should not be used anymore */ @Deprecated(forRemoval = true) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java index 84fe404558..3a5291c8b8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/DefaultWorkflow.java @@ -64,10 +64,10 @@ protected DefaultWorkflow(Map dependentResourceNo @SuppressWarnings("unchecked") private Map toMap(Set nodes) { - if(nodes == null || nodes.isEmpty()) { + if (nodes == null || nodes.isEmpty()) { return Collections.emptyMap(); } - + final var map = new HashMap(nodes.size()); for (DependentResourceNode node : nodes) { // add cycle detection?