feat: distinguish resources based on desired state (#2252)

Signed-off-by: Attila Mészáros <[email protected]>
Signed-off-by: Attila Mészáros <[email protected]>

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java

@@ -49,6 +49,16 @@ default Optional<? extends ResourceEventSource<R, P>> eventSource(
     return Optional.empty();
   }
 
+  /**
+   * Retrieves the secondary resource (if it exists) associated with the specified primary resource
+   * for this DependentResource.
+   *
+   * @param primary the primary resource for which we want to retrieve the secondary resource
+   *        associated with this DependentResource
+   * @param context the current {@link Context} in which the operation is called
+   * @return the secondary resource or {@link Optional#empty()} if it doesn't exist
+   * @throws IllegalStateException if more than one secondary is found to match the primary resource
+   */
   default Optional<R> getSecondaryResource(P primary, Context<P> context) {
     return Optional.empty();
   }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java

@@ -1,6 +1,7 @@
 package io.javaoperatorsdk.operator.processing.dependent;
 
 import java.util.Optional;
+import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -104,8 +105,39 @@ protected ReconcileResult<R> reconcile(P primary, R actualResource, Context<P> c
 
   @Override
   public Optional<R> getSecondaryResource(P primary, Context<P> context) {
-    return resourceDiscriminator == null ? context.getSecondaryResource(resourceType())
-        : resourceDiscriminator.distinguish(resourceType(), primary, context);
+    if (resourceDiscriminator != null) {
+      return resourceDiscriminator.distinguish(resourceType(), primary, context);
+    } else {
+      var secondaryResources = context.getSecondaryResources(resourceType());
+      if (secondaryResources.isEmpty()) {
+        return Optional.empty();
+      } else {
+        return selectManagedSecondaryResource(secondaryResources, primary, context);
+      }
+    }
+  }
+
+  /**
+   * Selects the actual secondary resource matching the desired state derived from the primary
+   * resource when several resources of the same type are found in the context. This method allows
+   * for optimized implementations in subclasses since this default implementation will check each
+   * secondary candidates for equality with the specified desired state, which might end up costly.
+   *
+   * @param secondaryResources to select the target resource from
+   *
+   * @return the matching secondary resource or {@link Optional#empty()} if none matches
+   * @throws IllegalStateException if more than one candidate is found, in which case some other
+   *         mechanism might be necessary to distinguish between candidate secondary resources
+   */
+  protected Optional<R> selectManagedSecondaryResource(Set<R> secondaryResources, P primary,
+      Context<P> context) {
+    R desired = desired(primary, context);
+    var targetResources = secondaryResources.stream().filter(r -> r.equals(desired)).toList();
+    if (targetResources.size() > 1) {
+      throw new IllegalStateException(
+          "More than one secondary resource related to primary: " + targetResources);
+    }
+    return targetResources.isEmpty() ? Optional.empty() : Optional.of(targetResources.get(0));
   }
 
   private void throwIfNull(R desired, P primary, String descriptor) {
@@ -173,8 +205,7 @@ protected void handleDelete(P primary, R secondary, Context<P> context) {
         "handleDelete method must be implemented if Deleter trait is supported");
   }
 
-  public void setResourceDiscriminator(
-      ResourceDiscriminator<R, P> resourceDiscriminator) {
+  public void setResourceDiscriminator(ResourceDiscriminator<R, P> resourceDiscriminator) {
     this.resourceDiscriminator = resourceDiscriminator;
   }
 

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

@@ -1,6 +1,7 @@
 package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
 
 import java.util.Map;
+import java.util.Objects;
 import java.util.Optional;
 import java.util.Set;
 
@@ -285,6 +286,29 @@ protected void addSecondaryToPrimaryMapperAnnotations(R desired, P primary, Stri
     }
   }
 
+  @Override
+  protected Optional<R> selectManagedSecondaryResource(Set<R> secondaryResources, P primary,
+      Context<P> context) {
+    ResourceID managedResourceID = managedSecondaryResourceID(primary, context);
+    return secondaryResources.stream()
+        .filter(r -> r.getMetadata().getName().equals(managedResourceID.getName()) &&
+            Objects.equals(r.getMetadata().getNamespace(),
+                managedResourceID.getNamespace().orElse(null)))
+        .findFirst();
+  }
+
+  /**
+   * Override this method in order to optimize and not compute the desired when selecting the target
+   * secondary resource. Simply, a static ResourceID can be returned.
+   *
+   * @param primary resource
+   * @param context of current reconciliation
+   * @return id of the target managed resource
+   */
+  protected ResourceID managedSecondaryResourceID(P primary, Context<P> context) {
+    return ResourceID.fromResource(desired(primary, context));
+  }
+
   protected boolean addOwnerReference() {
     return garbageCollected;
   }

operator-framework/src/test/java/io/javaoperatorsdk/operator/ExternalStateBulkIT.java

@@ -34,7 +34,7 @@ class ExternalStateBulkIT {
           .build();
 
   @Test
-  void reconcilesResourceWithPersistentState() throws InterruptedException {
+  void reconcilesResourceWithPersistentState() {
     var resource = operator.create(testResource());
     assertResources(resource, INITIAL_TEST_DATA, INITIAL_BULK_SIZE);
 

operator-framework/src/test/java/io/javaoperatorsdk/operator/MultipleDependentResourceIT.java

@@ -1,62 +1,80 @@
 package io.javaoperatorsdk.operator;
 
 import java.time.Duration;
-import java.util.stream.IntStream;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 import io.fabric8.kubernetes.api.model.ConfigMap;
 import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
 import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
-import io.javaoperatorsdk.operator.sample.multipledependentresource.MultipleDependentResourceConfigMap;
 import io.javaoperatorsdk.operator.sample.multipledependentresource.MultipleDependentResourceCustomResource;
 import io.javaoperatorsdk.operator.sample.multipledependentresource.MultipleDependentResourceReconciler;
+import io.javaoperatorsdk.operator.sample.multipledependentresource.MultipleDependentResourceSpec;
+import io.javaoperatorsdk.operator.sample.multipledrsametypenodiscriminator.*;
 
+import static io.javaoperatorsdk.operator.sample.multipledependentresource.MultipleDependentResourceConfigMap.DATA_KEY;
+import static io.javaoperatorsdk.operator.sample.multipledependentresource.MultipleDependentResourceConfigMap.getConfigMapName;
+import static io.javaoperatorsdk.operator.sample.multipledependentresource.MultipleDependentResourceReconciler.FIRST_CONFIG_MAP_ID;
+import static io.javaoperatorsdk.operator.sample.multipledependentresource.MultipleDependentResourceReconciler.SECOND_CONFIG_MAP_ID;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.awaitility.Awaitility.await;
 
-class MultipleDependentResourceIT {
+public class MultipleDependentResourceIT {
+
+  public static final String CHANGED_VALUE = "changed value";
+  public static final String INITIAL_VALUE = "initial value";
 
-  public static final String TEST_RESOURCE_NAME = "multipledependentresource-testresource";
   @RegisterExtension
-  LocallyRunOperatorExtension operator =
+  LocallyRunOperatorExtension extension =
       LocallyRunOperatorExtension.builder()
-          .withReconciler(MultipleDependentResourceReconciler.class)
-          .waitForNamespaceDeletion(true)
+          .withReconciler(new MultipleDependentResourceReconciler())
           .build();
 
   @Test
-  void twoConfigMapsHaveBeenCreated() {
-    MultipleDependentResourceCustomResource customResource = createTestCustomResource();
-    operator.create(customResource);
-
-    var reconciler = operator.getReconcilerOfType(MultipleDependentResourceReconciler.class);
-
-    await().pollDelay(Duration.ofMillis(300))
-        .until(() -> reconciler.getNumberOfExecutions() <= 1);
-
-    IntStream.of(MultipleDependentResourceReconciler.FIRST_CONFIG_MAP_ID,
-        MultipleDependentResourceReconciler.SECOND_CONFIG_MAP_ID).forEach(configMapId -> {
-          ConfigMap configMap =
-              operator.get(ConfigMap.class, customResource.getConfigMapName(configMapId));
-          assertThat(configMap).isNotNull();
-          assertThat(configMap.getMetadata().getName())
-              .isEqualTo(customResource.getConfigMapName(configMapId));
-          assertThat(configMap.getData().get(MultipleDependentResourceConfigMap.DATA_KEY))
-              .isEqualTo(String.valueOf(configMapId));
-        });
-  }
+  void handlesCRUDOperations() {
+    var res = extension.create(testResource());
+
+    await().untilAsserted(() -> {
+      var cm1 = extension.get(ConfigMap.class, getConfigMapName(FIRST_CONFIG_MAP_ID));
+      var cm2 = extension.get(ConfigMap.class, getConfigMapName(SECOND_CONFIG_MAP_ID));
+
+      assertThat(cm1).isNotNull();
+      assertThat(cm2).isNotNull();
+      assertThat(cm1.getData()).containsEntry(DATA_KEY, INITIAL_VALUE);
+      assertThat(cm2.getData()).containsEntry(DATA_KEY, INITIAL_VALUE);
+    });
+
+    res.getSpec().setValue(CHANGED_VALUE);
+    res = extension.replace(res);
+
+    await().untilAsserted(() -> {
+      var cm1 = extension.get(ConfigMap.class, getConfigMapName(FIRST_CONFIG_MAP_ID));
+      var cm2 = extension.get(ConfigMap.class, getConfigMapName(SECOND_CONFIG_MAP_ID));
 
-  public MultipleDependentResourceCustomResource createTestCustomResource() {
-    MultipleDependentResourceCustomResource resource =
-        new MultipleDependentResourceCustomResource();
-    resource.setMetadata(
-        new ObjectMetaBuilder()
-            .withName(TEST_RESOURCE_NAME)
-            .withNamespace(operator.getNamespace())
-            .build());
-    return resource;
+      assertThat(cm1.getData()).containsEntry(DATA_KEY, CHANGED_VALUE);
+      assertThat(cm2.getData()).containsEntry(DATA_KEY, CHANGED_VALUE);
+    });
+
+    extension.delete(res);
+
+    await().timeout(Duration.ofSeconds(120)).untilAsserted(() -> {
+      var cm1 = extension.get(ConfigMap.class, getConfigMapName(FIRST_CONFIG_MAP_ID));
+      var cm2 = extension.get(ConfigMap.class, getConfigMapName(SECOND_CONFIG_MAP_ID));
+
+      assertThat(cm1).isNull();
+      assertThat(cm2).isNull();
+    });
   }
 
+  MultipleDependentResourceCustomResource testResource() {
+    var res = new MultipleDependentResourceCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName("test1")
+        .build());
+    res.setSpec(new MultipleDependentResourceSpec());
+    res.getSpec().setValue(INITIAL_VALUE);
+
+    return res;
+  }
 }

operator-framework/src/test/java/io/javaoperatorsdk/operator/MultipleDependentResourceWithNoDiscriminatorIT.java

@@ -0,0 +1,65 @@
+package io.javaoperatorsdk.operator;
+
+import java.time.Duration;
+import java.util.stream.IntStream;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.multipledependentresourcewithdiscriminator.MultipleDependentResourceConfigMap;
+import io.javaoperatorsdk.operator.sample.multipledependentresourcewithdiscriminator.MultipleDependentResourceCustomResourceWithDiscriminator;
+import io.javaoperatorsdk.operator.sample.multipledependentresourcewithdiscriminator.MultipleDependentResourceWithDiscriminatorReconciler;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+class MultipleDependentResourceWithNoDiscriminatorIT {
+
+  public static final String TEST_RESOURCE_NAME = "multipledependentresource-testresource";
+  @RegisterExtension
+  LocallyRunOperatorExtension operator =
+      LocallyRunOperatorExtension.builder()
+          .withReconciler(MultipleDependentResourceWithDiscriminatorReconciler.class)
+          .waitForNamespaceDeletion(true)
+          .build();
+
+  @Test
+  void twoConfigMapsHaveBeenCreated() {
+    MultipleDependentResourceCustomResourceWithDiscriminator customResource =
+        createTestCustomResource();
+    operator.create(customResource);
+
+    var reconciler =
+        operator.getReconcilerOfType(MultipleDependentResourceWithDiscriminatorReconciler.class);
+
+    await().pollDelay(Duration.ofMillis(300))
+        .until(() -> reconciler.getNumberOfExecutions() <= 1);
+
+    IntStream.of(MultipleDependentResourceWithDiscriminatorReconciler.FIRST_CONFIG_MAP_ID,
+        MultipleDependentResourceWithDiscriminatorReconciler.SECOND_CONFIG_MAP_ID)
+        .forEach(configMapId -> {
+          ConfigMap configMap =
+              operator.get(ConfigMap.class, customResource.getConfigMapName(configMapId));
+          assertThat(configMap).isNotNull();
+          assertThat(configMap.getMetadata().getName())
+              .isEqualTo(customResource.getConfigMapName(configMapId));
+          assertThat(configMap.getData().get(MultipleDependentResourceConfigMap.DATA_KEY))
+              .isEqualTo(String.valueOf(configMapId));
+        });
+  }
+
+  public MultipleDependentResourceCustomResourceWithDiscriminator createTestCustomResource() {
+    MultipleDependentResourceCustomResourceWithDiscriminator resource =
+        new MultipleDependentResourceCustomResourceWithDiscriminator();
+    resource.setMetadata(
+        new ObjectMetaBuilder()
+            .withName(TEST_RESOURCE_NAME)
+            .withNamespace(operator.getNamespace())
+            .build());
+    return resource;
+  }
+
+}

operator-framework/src/test/java/io/javaoperatorsdk/operator/MultipleManagedDependentNoDiscriminatorIT.java

@@ -0,0 +1,81 @@
+package io.javaoperatorsdk.operator;
+
+import java.time.Duration;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+
+import io.fabric8.kubernetes.api.model.ConfigMap;
+import io.fabric8.kubernetes.api.model.ObjectMetaBuilder;
+import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension;
+import io.javaoperatorsdk.operator.sample.multipledrsametypenodiscriminator.*;
+
+import static io.javaoperatorsdk.operator.sample.multipledrsametypenodiscriminator.MultipleManagedDependentSameTypeNoDiscriminatorReconciler.DATA_KEY;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.awaitility.Awaitility.await;
+
+public class MultipleManagedDependentNoDiscriminatorIT {
+
+  public static final String RESOURCE_NAME = "test1";
+  public static final String INITIAL_VALUE = "initial_value";
+  public static final String CHANGED_VALUE = "changed_value";
+
+  @RegisterExtension
+  LocallyRunOperatorExtension extension =
+      LocallyRunOperatorExtension.builder()
+          .withReconciler(new MultipleManagedDependentSameTypeNoDiscriminatorReconciler())
+          .build();
+
+  @Test
+  void handlesCRUDOperations() {
+    var res = extension.create(testResource());
+
+    await().untilAsserted(() -> {
+      var cm1 = extension.get(ConfigMap.class,
+          RESOURCE_NAME + MultipleManagedDependentNoDiscriminatorConfigMap1.NAME_SUFFIX);
+      var cm2 = extension.get(ConfigMap.class,
+          RESOURCE_NAME + MultipleManagedDependentNoDiscriminatorConfigMap2.NAME_SUFFIX);
+
+      assertThat(cm1).isNotNull();
+      assertThat(cm2).isNotNull();
+      assertThat(cm1.getData()).containsEntry(DATA_KEY, INITIAL_VALUE);
+      assertThat(cm2.getData()).containsEntry(DATA_KEY, INITIAL_VALUE);
+    });
+
+    res.getSpec().setValue(CHANGED_VALUE);
+    res = extension.replace(res);
+
+    await().untilAsserted(() -> {
+      var cm1 = extension.get(ConfigMap.class,
+          RESOURCE_NAME + MultipleManagedDependentNoDiscriminatorConfigMap1.NAME_SUFFIX);
+      var cm2 = extension.get(ConfigMap.class,
+          RESOURCE_NAME + MultipleManagedDependentNoDiscriminatorConfigMap2.NAME_SUFFIX);
+
+      assertThat(cm1.getData()).containsEntry(DATA_KEY, CHANGED_VALUE);
+      assertThat(cm2.getData()).containsEntry(DATA_KEY, CHANGED_VALUE);
+    });
+
+    extension.delete(res);
+
+    await().timeout(Duration.ofSeconds(60)).untilAsserted(() -> {
+      var cm1 = extension.get(ConfigMap.class,
+          RESOURCE_NAME + MultipleManagedDependentNoDiscriminatorConfigMap1.NAME_SUFFIX);
+      var cm2 = extension.get(ConfigMap.class,
+          RESOURCE_NAME + MultipleManagedDependentNoDiscriminatorConfigMap2.NAME_SUFFIX);
+
+      assertThat(cm1).isNull();
+      assertThat(cm2).isNull();
+    });
+  }
+
+  MultipleManagedDependentNoDiscriminatorCustomResource testResource() {
+    var res = new MultipleManagedDependentNoDiscriminatorCustomResource();
+    res.setMetadata(new ObjectMetaBuilder()
+        .withName(RESOURCE_NAME)
+        .build());
+    res.setSpec(new MultipleManagedDependentNoDiscriminatorSpec());
+    res.getSpec().setValue(INITIAL_VALUE);
+    return res;
+  }
+
+}