feat: matcher avoiding creating the desired state when possible

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

@@ -36,19 +36,23 @@ public void reconcile(P primary, Context context) {
     final var updatable = isUpdatable(primary, context);
     if (creatable || updatable) {
       var maybeActual = getResource(primary);
-      var desired = desired(primary, context);
       if (maybeActual.isEmpty()) {
         if (creatable) {
+          var desired = desired(primary, context);
           log.debug("Creating dependent {} for primary {}", desired, primary);
           creator.create(desired, primary, context);
         }
       } else {
         final var actual = maybeActual.get();
-        if (updatable && !updater.match(actual, desired, context)) {
-          log.debug("Updating dependent {} for primary {}", desired, primary);
-          updater.update(actual, desired, primary, context);
+        if (updatable) {
+          final var match = updater.match(actual, primary, context);
+          if (!match.matched()) {
+            final var desired = match.computedDesired().orElse(desired(primary, context));
+            log.debug("Updating dependent {} for primary {}", desired, primary);
+            updater.update(actual, desired, primary, context);
+          }
         } else {
-          log.debug("Update skipped for dependent {} as it matched the existing one", desired);
+          log.debug("Update skipped for dependent {} as it matched the existing one", actual);
         }
       }
     } else {

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

@@ -1,7 +1,36 @@
 package io.javaoperatorsdk.operator.api.reconciler.dependent;
 
+import java.util.Optional;
+
+import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 
-public interface Matcher<R> {
-  boolean match(R actualResource, R desiredResource, Context context);
+public interface Matcher<R, P extends HasMetadata> {
+  interface Result<R> {
+    boolean matched();
+
+    default Optional<R> computedDesired() {
+      return Optional.empty();
+    }
+
+    static <T> Result<T> nonComputed(boolean matched) {
+      return () -> matched;
+    }
+
+    static <T> Result<T> computed(boolean matched, T computedDesired) {
+      return new Result<>() {
+        @Override
+        public boolean matched() {
+          return matched;
+        }
+
+        @Override
+        public Optional<T> computedDesired() {
+          return Optional.of(computedDesired);
+        }
+      };
+    }
+  }
+
+  Result<R> match(R actualResource, P primary, Context context);
 }

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

@@ -1,18 +1,22 @@
 package io.javaoperatorsdk.operator.api.reconciler.dependent;
 
-import java.util.Objects;
-
 import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher.Result;
 
 @SuppressWarnings("rawtypes")
 public interface Updater<R, P extends HasMetadata> {
-  Updater NOOP = (actual, desired, primary, context) -> {
+  Updater NOOP = new Updater() {
+    @Override
+    public void update(Object actual, Object desired, HasMetadata primary, Context context) {}
+
+    @Override
+    public Result match(Object actualResource, HasMetadata primary, Context context) {
+      return Result.nonComputed(true);
+    }
   };
 
   void update(R actual, R desired, P primary, Context context);
 
-  default boolean match(R actualResource, R desiredResource, Context context) {
-    return Objects.equals(actualResource, desiredResource);
-  }
+  Result<R> match(R actualResource, P primary, Context context);
 }

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

@@ -8,37 +8,52 @@
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher;
 
-public class GenericKubernetesResourceMatcher<R extends HasMetadata> implements Matcher<R> {
+public class GenericKubernetesResourceMatcher<R extends HasMetadata, P extends HasMetadata>
+    implements Matcher<R, P> {
 
-  private GenericKubernetesResourceMatcher() {}
+  private final KubernetesDependentResource<R, P> dependentResource;
 
-  @SuppressWarnings({"rawtypes", "unchecked"})
-  public static <R extends HasMetadata> Matcher<R> matcherFor(Class<R> resourceType) {
+  private GenericKubernetesResourceMatcher(KubernetesDependentResource<R, P> dependentResource) {
+    this.dependentResource = dependentResource;
+  }
+
+  @SuppressWarnings({"unchecked", "rawtypes"})
+  static <R extends HasMetadata, P extends HasMetadata> Matcher<R, P> matcherFor(
+      Class<R> resourceType, KubernetesDependentResource<R, P> dependentResource) {
     if (Secret.class.isAssignableFrom(resourceType)) {
-      return (actual, desired, context) -> ResourceComparators.compareSecretData((Secret) desired,
-          (Secret) actual);
+      return (actual, primary, context) -> {
+        final var desired = dependentResource.desired(primary, context);
+        return Result.computed(
+            ResourceComparators.compareSecretData((Secret) desired, (Secret) actual), desired);
+      };
     } else if (ConfigMap.class.isAssignableFrom(resourceType)) {
-      return (actual, desired, context) -> ResourceComparators
-          .compareConfigMapData((ConfigMap) desired, (ConfigMap) actual);
+      return (actual, primary, context) -> {
+        final var desired = dependentResource.desired(primary, context);
+        return Result.computed(
+            ResourceComparators.compareConfigMapData((ConfigMap) desired, (ConfigMap) actual),
+            desired);
+      };
     } else {
-      return new GenericKubernetesResourceMatcher();
+      return new GenericKubernetesResourceMatcher(dependentResource);
     }
   }
 
   @Override
-  public boolean match(R actualResource, R desiredResource, Context context) {
+  public Result<R> match(R actualResource, P primary, Context context) {
     final var objectMapper = context.getConfigurationService().getObjectMapper();
+    final var desired = dependentResource.desired(primary, context);
+
     // reflection will be replaced by this:
     // https://github.com/fabric8io/kubernetes-client/issues/3816
-    var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desiredResource));
+    var desiredSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(desired));
     var actualSpecNode = objectMapper.valueToTree(ReconcilerUtils.getSpec(actualResource));
     var diffJsonPatch = JsonDiff.asJson(desiredSpecNode, actualSpecNode);
     for (int i = 0; i < diffJsonPatch.size(); i++) {
       String operation = diffJsonPatch.get(i).get("op").asText();
       if (!operation.equals("add")) {
-        return false;
+        return Result.computed(false, desired);
       }
     }
-    return true;
+    return Result.computed(true, desired);
   }
 }

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

@@ -21,7 +21,9 @@
 import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.KubernetesClientAware;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher.Result;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.ResourceUpdatePreProcessor;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.Updater;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 import io.javaoperatorsdk.operator.processing.event.source.AssociatedSecondaryResourceIdentifier;
 import io.javaoperatorsdk.operator.processing.event.source.EventSource;
@@ -39,14 +41,24 @@ public abstract class KubernetesDependentResource<R extends HasMetadata, P exten
   protected KubernetesClient client;
   private InformerEventSource<R, P> informerEventSource;
   private boolean addOwnerReference;
-  private final Matcher<R> matcher;
+  private final Matcher<R, P> matcher;
   private final ResourceUpdatePreProcessor<R> processor;
 
   @SuppressWarnings("unchecked")
   public KubernetesDependentResource() {
-    init(this::create, this::update, this::delete);
-    matcher = this instanceof Matcher ? (Matcher<R>) this
-        : GenericKubernetesResourceMatcher.matcherFor(resourceType());
+    init(this::create, new Updater<>() {
+      @Override
+      public void update(R actual, R desired, P primary, Context context) {
+        KubernetesDependentResource.this.update(actual, desired, primary, context);
+      }
+
+      @Override
+      public Result<R> match(R actualResource, P primary, Context context) {
+        return KubernetesDependentResource.this.match(actualResource, primary, context);
+      }
+    }, this::delete);
+    matcher = this instanceof Matcher ? (Matcher<R, P>) this
+        : GenericKubernetesResourceMatcher.matcherFor(resourceType(), this);
     processor = this instanceof ResourceUpdatePreProcessor
         ? (ResourceUpdatePreProcessor<R>) this
         : GenericResourceUpdatePreProcessor.processorFor(resourceType());
@@ -117,8 +129,8 @@ public void update(R actual, R target, P primary, Context context) {
     }
   }
 
-  public boolean match(R actualResource, R desiredResource, Context context) {
-    return matcher.match(actualResource, desiredResource, context);
+  public Result<R> match(R actualResource, P primary, Context context) {
+    return matcher.match(actualResource, primary, context);
   }
 
   public void delete(P primary, Context context) {
@@ -173,4 +185,9 @@ public Optional<R> getResource(P primaryResource) {
   public void setKubernetesClient(KubernetesClient kubernetesClient) {
     this.client = kubernetesClient;
   }
+
+  @Override
+  protected R desired(P primary, Context context) {
+    return super.desired(primary, context);
+  }
 }

operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java

@@ -1,8 +1,12 @@
 package io.javaoperatorsdk.operator.processing.dependent.kubernetes;
 
+import java.util.Optional;
+
 import org.junit.jupiter.api.Test;
 
+import io.fabric8.kubernetes.api.model.HasMetadata;
 import io.fabric8.kubernetes.api.model.apps.Deployment;
+import io.fabric8.kubernetes.api.model.apps.DeploymentBuilder;
 import io.javaoperatorsdk.operator.ReconcilerUtils;
 import io.javaoperatorsdk.operator.api.config.ConfigurationService;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
@@ -24,29 +28,40 @@ class GenericKubernetesResourceMatcherTest {
 
   @Test
   void checksIfDesiredValuesAreTheSame() {
-    var target1 = createDeployment();
-    var desired1 = createDeployment();
-    final var matcher = GenericKubernetesResourceMatcher.matcherFor(Deployment.class);
-    assertThat(matcher.match(target1, desired1, context)).isTrue();
-
-    var target2 = createDeployment();
-    var desired2 = createDeployment();
-    target2.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
-    assertThat(matcher.match(target2, desired2, context))
+    var actual = createDeployment();
+    final var desired = createDeployment();
+    final var matcher = GenericKubernetesResourceMatcher.matcherFor(Deployment.class,
+        new KubernetesDependentResource<>() {
+          @Override
+          protected Deployment desired(HasMetadata primary, Context context) {
+            final var currentCase = Optional.ofNullable(primary)
+                .map(p -> p.getMetadata().getLabels().get("case"))
+                .orElse(null);
+            var d = desired;
+            if ("removed".equals(currentCase)) {
+              d = createDeployment();
+              d.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
+            }
+            return d;
+          }
+        });
+    assertThat(matcher.match(actual, null, context).matched()).isTrue();
+    assertThat(matcher.match(actual, null, context).computedDesired().isPresent()).isTrue();
+    assertThat(matcher.match(actual, null, context).computedDesired().get()).isEqualTo(desired);
+
+    actual.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
+    assertThat(matcher.match(actual, null, context).matched())
         .withFailMessage("Additive changes should be ok")
         .isTrue();
 
-    var target3 = createDeployment();
-    var desired3 = createDeployment();
-    desired3.getSpec().getTemplate().getMetadata().getLabels().put("new-key", "val");
-    assertThat(matcher.match(target3, desired3, context))
+    actual = createDeployment();
+    assertThat(matcher.match(actual, createPrimary("removed"), context).matched())
         .withFailMessage("Removed value should not be ok")
         .isFalse();
 
-    var target4 = createDeployment();
-    var desired4 = createDeployment();
-    target4.getSpec().setReplicas(2);
-    assertThat(matcher.match(target4, desired4, context))
+    actual = createDeployment();
+    actual.getSpec().setReplicas(2);
+    assertThat(matcher.match(actual, null, context).matched())
         .withFailMessage("Changed values are not ok")
         .isFalse();
   }
@@ -55,4 +70,12 @@ Deployment createDeployment() {
     return ReconcilerUtils.loadYaml(
         Deployment.class, GenericKubernetesResourceMatcherTest.class, "nginx-deployment.yaml");
   }
+
+  HasMetadata createPrimary(String caseName) {
+    return new DeploymentBuilder()
+        .editOrNewMetadata()
+        .addToLabels("case", caseName)
+        .endMetadata()
+        .build();
+  }
 }

sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java

@@ -8,6 +8,7 @@
 import io.fabric8.kubernetes.api.model.SecretBuilder;
 import io.javaoperatorsdk.operator.api.reconciler.Context;
 import io.javaoperatorsdk.operator.api.reconciler.dependent.Creator;
+import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher.Result;
 import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource;
 import io.javaoperatorsdk.operator.processing.event.ResourceID;
 import io.javaoperatorsdk.operator.processing.event.source.AssociatedSecondaryResourceIdentifier;
@@ -32,24 +33,27 @@ protected Secret desired(MySQLSchema schema, Context context) {
     final var password = RandomStringUtils
         .randomAlphanumeric(16); // NOSONAR: we don't need cryptographically-strong randomness here
     final var name = schema.getMetadata().getName();
-    final var secretName = String.format(SECRET_FORMAT, name);
+    final var secretName = getSecretName(name);
     final var userName = String.format(USERNAME_FORMAT, name);
 
     return new SecretBuilder()
         .withNewMetadata()
         .withName(secretName)
         .withNamespace(schema.getMetadata().getNamespace())
         .endMetadata()
-        .addToData(
-            MYSQL_SECRET_USERNAME, encode(userName))
-        .addToData(
-            MYSQL_SECRET_PASSWORD, encode(password))
+        .addToData(MYSQL_SECRET_USERNAME, encode(userName))
+        .addToData(MYSQL_SECRET_PASSWORD, encode(password))
         .build();
   }
 
+  private String getSecretName(String name) {
+    return String.format(SECRET_FORMAT, name);
+  }
+
   @Override
-  public boolean match(Secret actual, Secret target, Context context) {
-    return ResourceID.fromResource(actual).equals(ResourceID.fromResource(target));
+  public Result<Secret> match(Secret actual, MySQLSchema primary, Context context) {
+    final var desiredSecretName = getSecretName(primary.getMetadata().getName());
+    return Result.nonComputed(actual.getMetadata().getName().equals(desiredSecretName));
   }
 
   @Override