fix: shut down operator when failing to start (#1137)

* fix: start timer as daemon to avoid prolonging the app's life

* fix: exit operator on exception when starting

Fixes #1136

* feat: extract missing CRD exception check to ReconcilerUtils

Fixes #1139

* fix: propagate exceptions and stop on start exception

* chore: regexp input is under control so marking as false positive

Author: metacosm

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java

@@ -85,20 +85,26 @@ public List<Controller> 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);
+      stop();
+      throw e;
+    }
   }
 
   @Override
@@ -204,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();

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,49 @@ public static <T> T loadYaml(Class<T> 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*).*") // NOSONAR: input is controlled
+          .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;
+  }
+
 }

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);
     }
   }

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<T> getSecondaryResource(T primary) {
     throw new IllegalStateException("This method should not be called here. Primary: " + primary);

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<T> 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

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/timer/TimerEventSource.java

@@ -20,7 +20,7 @@ public class TimerEventSource<R extends HasMetadata>
     implements ResourceEventAware<R> {
   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<ResourceID, EventProducerTimeTask> onceTasks = new ConcurrentHashMap<>();
 

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<Void, Void> implements Namespaced {
+
+  }
 }