fix: potential bug in informers (#833)

operator-framework · Jan 14, 2022 · 9184633 · 9184633
1 parent a22596e
commit 9184633
Show file tree

Hide file tree

Showing 4 changed files with 50 additions and 9 deletions.
diff --git a/...ava/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/...ava/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java
@@ -83,6 +83,13 @@ public void onAdd(T t) {
 
       @Override
       public void onUpdate(T oldObject, T newObject) {
+        if (newObject == null) {
+          // this is a fix for this potential issue with informer:
+          // https://github.com/java-operator-sdk/java-operator-sdk/issues/830
+          propagateEvent(oldObject);
+          return;
+        }
+
         if (InformerEventSource.this.skipUpdateEventPropagationIfNoChange &&
             oldObject.getMetadata().getResourceVersion()
                 .equals(newObject.getMetadata().getResourceVersion())) {

diff --git a/...r-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java b/...r-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java
@@ -140,6 +140,9 @@ public <T extends HasMetadata> T replace(Class<T> type, T resource) {
     return kubernetesClient.resources(type).inNamespace(namespace).replace(resource);
   }
 
+  public <T extends HasMetadata> boolean delete(Class<T> type, T resource) {
+    return kubernetesClient.resources(type).inNamespace(namespace).delete(resource);
+  }
 
   @SuppressWarnings("unchecked")
   protected void before(ExtensionContext context) {

diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/InformerEventSourceIT.java
@@ -13,9 +13,9 @@
 import io.javaoperatorsdk.operator.sample.informereventsource.InformerEventSourceTestCustomReconciler;
 import io.javaoperatorsdk.operator.sample.informereventsource.InformerEventSourceTestCustomResource;
 
-import static io.javaoperatorsdk.operator.sample.informereventsource.InformerEventSourceTestCustomReconciler.RELATED_RESOURCE_NAME;
-import static io.javaoperatorsdk.operator.sample.informereventsource.InformerEventSourceTestCustomReconciler.TARGET_CONFIG_MAP_KEY;
+import static io.javaoperatorsdk.operator.sample.informereventsource.InformerEventSourceTestCustomReconciler.*;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.fail;
 import static org.awaitility.Awaitility.await;
 
 public class InformerEventSourceIT {
@@ -32,7 +32,7 @@ public class InformerEventSourceIT {
           .build();
 
   @Test
-  public void testUsingInformerToWatchChangesOfConfigMap() {
+  void testUsingInformerToWatchChangesOfConfigMap() {
     var customResource = initialCustomResource();
     customResource = operator.create(InformerEventSourceTestCustomResource.class, customResource);
     ConfigMap configMap =
@@ -45,6 +45,25 @@ public void testUsingInformerToWatchChangesOfConfigMap() {
     waitForCRStatusValue(UPDATE_STATUS_MESSAGE);
   }
 
+  @Test
+  void deletingSecondaryResource() {
+    var customResource = initialCustomResource();
+    customResource = operator.create(InformerEventSourceTestCustomResource.class, customResource);
+    ConfigMap configMap =
+        operator.create(ConfigMap.class, relatedConfigMap(customResource.getMetadata().getName()));
+    waitForCRStatusValue(INITIAL_STATUS_MESSAGE);
+
+    boolean res = operator.delete(ConfigMap.class, configMap);
+    if (!res) {
+      fail("Unable to delete configmap");
+    }
+
+    waitForCRStatusValue(MISSING_CONFIG_MAP);
+    assertThat(((InformerEventSourceTestCustomReconciler) operator.getReconcilers().get(0))
+        .getNumberOfExecutions())
+            .isEqualTo(3);
+  }
+
   private ConfigMap relatedConfigMap(String relatedResourceAnnotation) {
     ConfigMap configMap = new ConfigMap();
 

diff --git a/...ratorsdk/operator/sample/informereventsource/InformerEventSourceTestCustomReconciler.java b/...ratorsdk/operator/sample/informereventsource/InformerEventSourceTestCustomReconciler.java
@@ -1,6 +1,8 @@
 package io.javaoperatorsdk.operator.sample.informereventsource;
 
 import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -34,8 +36,10 @@ public class InformerEventSourceTestCustomReconciler implements
 
   public static final String RELATED_RESOURCE_NAME = "relatedResourceName";
   public static final String TARGET_CONFIG_MAP_KEY = "targetStatus";
+  public static final String MISSING_CONFIG_MAP = "Missing Config Map";
 
   private KubernetesClient kubernetesClient;
+  private final AtomicInteger numberOfExecutions = new AtomicInteger(0);
 
   @Override
   public List<EventSource> prepareEventSources(
@@ -48,16 +52,20 @@ public List<EventSource> prepareEventSources(
   public UpdateControl<InformerEventSourceTestCustomResource> reconcile(
       InformerEventSourceTestCustomResource resource,
       Context context) {
+    numberOfExecutions.incrementAndGet();
 
+    resource.setStatus(new InformerEventSourceTestCustomResourceStatus());
     // Reading the config map from the informer not from the API
     // name of the config map same as custom resource for sake of simplicity
-    ConfigMap configMap = context.getSecondaryResource(ConfigMap.class)
-        .orElseThrow(() -> new IllegalStateException("Config map should be present."));
+    Optional<ConfigMap> configMap = context.getSecondaryResource(ConfigMap.class);
+    if (configMap.isEmpty()) {
+      resource.getStatus().setConfigMapValue(MISSING_CONFIG_MAP);
+    } else {
+      String targetStatus = configMap.get().getData().get(TARGET_CONFIG_MAP_KEY);
+      LOGGER.debug("Setting target status for CR: {}", targetStatus);
+      resource.getStatus().setConfigMapValue(targetStatus);
+    }
 
-    String targetStatus = configMap.getData().get(TARGET_CONFIG_MAP_KEY);
-    LOGGER.debug("Setting target status for CR: {}", targetStatus);
-    resource.setStatus(new InformerEventSourceTestCustomResourceStatus());
-    resource.getStatus().setConfigMapValue(targetStatus);
     return UpdateControl.updateStatus(resource);
   }
 
@@ -70,4 +78,8 @@ public KubernetesClient getKubernetesClient() {
   public void setKubernetesClient(KubernetesClient kubernetesClient) {
     this.kubernetesClient = kubernetesClient;
   }
+
+  public int getNumberOfExecutions() {
+    return numberOfExecutions.get();
+  }
 }