From e7f8ca05c166f5ae5bd1b9542a940461066ae390 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 5 Apr 2022 21:42:42 +0200 Subject: [PATCH 1/5] fix: start timer as daemon to avoid prolonging the app's life --- .../processing/event/source/timer/TimerEventSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSource.java index cadebc3deb..28acfdc3ac 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSource.java @@ -20,7 +20,7 @@ public class TimerEventSource implements ResourceEventAware { private static final Logger log = LoggerFactory.getLogger(TimerEventSource.class); - private final Timer timer = new Timer(); + private final Timer timer = new Timer(true); private final AtomicBoolean running = new AtomicBoolean(); private final Map onceTasks = new ConcurrentHashMap<>(); From 27d42b0905d762689f3101b0dbd0169a3981ac78 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 5 Apr 2022 21:56:05 +0200 Subject: [PATCH 2/5] fix: exit operator on exception when starting Fixes #1136 --- .../io/javaoperatorsdk/operator/Operator.java | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 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 bcd0d7ade0..3fb69b7058 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 @@ -85,20 +85,25 @@ public List getControllers() { * and start the cluster monitoring processes. */ public void start() { - controllers.shouldStart(); - - final var version = ConfigurationServiceProvider.instance().getVersion(); - log.info( - "Operator SDK {} (commit: {}) built on {} starting...", - version.getSdkVersion(), - version.getCommit(), - version.getBuiltTime()); - - final var clientVersion = Version.clientVersion(); - log.info("Client version: {}", clientVersion); - - ExecutorServiceManager.init(); - controllers.start(); + try { + controllers.shouldStart(); + + final var version = ConfigurationServiceProvider.instance().getVersion(); + log.info( + "Operator SDK {} (commit: {}) built on {} starting...", + version.getSdkVersion(), + version.getCommit(), + version.getBuiltTime()); + + final var clientVersion = Version.clientVersion(); + log.info("Client version: {}", clientVersion); + + ExecutorServiceManager.init(); + controllers.start(); + } catch (Exception e) { + log.error("Error starting operator", e); + System.exit(1); + } } @Override From 6c9720780271933bc7e9e8d5ef3e58dc622505c9 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 6 Apr 2022 18:34:52 +0200 Subject: [PATCH 3/5] feat: extract missing CRD exception check to ReconcilerUtils Fixes #1139 --- .../operator/ReconcilerUtils.java | 48 +++++++++++++++++++ .../ControllerResourceEventSource.java | 15 +----- .../source/informer/InformerWrapper.java | 9 +++- .../operator/ReconcilerUtilsTest.java | 25 ++++++++++ 4 files changed, 83 insertions(+), 14 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java index d697422ef7..1135b9b887 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java @@ -4,9 +4,14 @@ import java.io.InputStream; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.Locale; +import java.util.function.Predicate; +import java.util.regex.Pattern; +import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.utils.Serialization; import io.javaoperatorsdk.operator.api.reconciler.Constants; import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; @@ -115,4 +120,47 @@ public static T loadYaml(Class clazz, Class loader, String yaml) { } } + public static void handleKubernetesClientException(Exception e, String resourceTypeName) { + if (e instanceof MissingCRDException) { + throw ((MissingCRDException) e); + } + + if (e instanceof KubernetesClientException) { + KubernetesClientException ke = (KubernetesClientException) e; + if (404 == ke.getCode()) { + // only throw MissingCRDException if the 404 error occurs on the target CRD + if (resourceTypeName.equals(ke.getFullResourceName()) + || matchesResourceType(resourceTypeName, ke)) { + throw new MissingCRDException(resourceTypeName, null, e.getMessage(), e); + } + } + } + } + + private static boolean matchesResourceType(String resourceTypeName, + KubernetesClientException exception) { + final var fullResourceName = exception.getFullResourceName(); + if (fullResourceName != null) { + return resourceTypeName.equals(fullResourceName); + } else { + // extract matching information from URI in the message if available + final var message = exception.getMessage(); + final var regex = Pattern.compile(".*http(s?)://[^/]*/api(s?)/(\\S*).*").matcher(message); + if (regex.matches()) { + var group = regex.group(3); + if (group.endsWith(".")) { + group = group.substring(0, group.length() - 1); + } + final var segments = Arrays.stream(group.split("/")).filter(Predicate.not(String::isEmpty)) + .collect(Collectors.toUnmodifiableList()); + if (segments.size() != 3) { + return false; + } + final var targetResourceName = segments.get(2) + "." + segments.get(0); + return resourceTypeName.equals(targetResourceName); + } + } + return false; + } + } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java index 25c3b17b08..c3ffc326bf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java @@ -8,13 +8,13 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; -import io.javaoperatorsdk.operator.MissingCRDException; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.MDCUtils; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.informer.ManagedInformerEventSource; +import static io.javaoperatorsdk.operator.ReconcilerUtils.handleKubernetesClientException; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getName; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getUID; import static io.javaoperatorsdk.operator.processing.KubernetesResourceUtils.getVersion; @@ -50,7 +50,7 @@ public void start() { try { super.start(); } catch (KubernetesClientException e) { - handleKubernetesClientException(e); + handleKubernetesClientException(e, controller.getConfiguration().getResourceTypeName()); throw e; } } @@ -90,17 +90,6 @@ public void onDelete(T resource, boolean b) { eventReceived(ResourceAction.DELETED, resource, null); } - private void handleKubernetesClientException(Exception e) { - KubernetesClientException ke = (KubernetesClientException) e; - if (404 == ke.getCode()) { - // only throw MissingCRDException if the 404 error occurs on the target CRD - final var targetCRDName = controller.getConfiguration().getResourceTypeName(); - if (targetCRDName.equals(ke.getFullResourceName())) { - throw new MissingCRDException(targetCRDName, null, e.getMessage(), e); - } - } - } - @Override public Optional getSecondaryResource(T primary) { throw new IllegalStateException("This method should not be called here. Primary: " + primary); 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 dfe38d53d4..e0f3108660 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 @@ -13,6 +13,7 @@ import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import io.fabric8.kubernetes.client.informers.cache.Cache; import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.processing.LifecycleAware; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.IndexerResourceCache; @@ -31,7 +32,13 @@ public InformerWrapper(SharedIndexInformer informer) { @Override public void start() throws OperatorException { - informer.run(); + try { + informer.run(); + } catch (Exception e) { + ReconcilerUtils.handleKubernetesClientException(e, + HasMetadata.getFullResourceName(informer.getApiTypeClass())); + throw e; + } } @Override diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java index 5081767980..5cbe1a4441 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsTest.java @@ -2,20 +2,29 @@ import org.junit.jupiter.api.Test; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.PodSpec; import io.fabric8.kubernetes.api.model.PodTemplateSpec; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.api.model.apps.DeploymentSpec; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.client.KubernetesClientException; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; import io.javaoperatorsdk.operator.sample.simple.TestCustomReconciler; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static io.javaoperatorsdk.operator.ReconcilerUtils.getDefaultFinalizerName; import static io.javaoperatorsdk.operator.ReconcilerUtils.getDefaultNameFor; import static io.javaoperatorsdk.operator.ReconcilerUtils.getDefaultReconcilerName; +import static io.javaoperatorsdk.operator.ReconcilerUtils.handleKubernetesClientException; import static io.javaoperatorsdk.operator.ReconcilerUtils.isFinalizerValid; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; class ReconcilerUtilsTest { @@ -89,4 +98,20 @@ private Deployment createTestDeployment() { podTemplateSpec.getSpec().setHostname("localhost"); return deployment; } + + @Test + void handleKubernetesExceptionShouldThrowMissingCRDExceptionWhenAppropriate() { + assertThrows(MissingCRDException.class, () -> handleKubernetesClientException( + new KubernetesClientException( + "Failure executing: GET at: https://kubernetes.docker.internal:6443/apis/tomcatoperator.io/v1/tomcats. Message: Not Found.", + 404, null), + HasMetadata.getFullResourceName(Tomcat.class))); + } + + @Group("tomcatoperator.io") + @Version("v1") + @ShortNames("tc") + private static class Tomcat extends CustomResource implements Namespaced { + + } } From ef016c5f17490be121821d4518979bdfafef02dc Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 6 Apr 2022 18:52:00 +0200 Subject: [PATCH 4/5] fix: propagate exceptions and stop on start exception --- .../main/java/io/javaoperatorsdk/operator/Operator.java | 7 ++----- .../io/javaoperatorsdk/operator/processing/Controller.java | 1 + 2 files changed, 3 insertions(+), 5 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 3fb69b7058..9765ede762 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 @@ -102,7 +102,8 @@ public void start() { controllers.start(); } catch (Exception e) { log.error("Error starting operator", e); - System.exit(1); + stop(); + throw e; } } @@ -209,10 +210,6 @@ public synchronized void start() { } public synchronized void stop() { - if (!started) { - return; - } - this.controllers.values().parallelStream().forEach(closeable -> { log.debug("closing {}", closeable); closeable.stop(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 4d0482b5d6..63b3c85a9c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -332,6 +332,7 @@ public void start() throws OperatorException { log.info("'{}' controller started, pending event sources initialization", controllerName); } catch (MissingCRDException e) { + stop(); throwMissingCRDException(crdName, specVersion, controllerName); } } From b94470043be3b4cc3234dfa847f6f45755998dca Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 7 Apr 2022 09:29:26 +0200 Subject: [PATCH 5/5] chore: regexp input is under control so marking as false positive --- .../java/io/javaoperatorsdk/operator/ReconcilerUtils.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java index 1135b9b887..d23362b2b0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtils.java @@ -145,7 +145,9 @@ private static boolean matchesResourceType(String resourceTypeName, } else { // extract matching information from URI in the message if available final var message = exception.getMessage(); - final var regex = Pattern.compile(".*http(s?)://[^/]*/api(s?)/(\\S*).*").matcher(message); + final var regex = Pattern + .compile(".*http(s?)://[^/]*/api(s?)/(\\S*).*") // NOSONAR: input is controlled + .matcher(message); if (regex.matches()) { var group = regex.group(3); if (group.endsWith(".")) {