Consider a simpler Matcher interface

[metacosm]

Right now, the Matcher interface defines a single method: public boolean match(Deployment actual, Deployment desired, Context context). This works quite nicely for the default implementation. However, in many cases, when people actually want to implement their own optimised version, it would be more effective and efficient to get the primary resource instead of the desired state as the desired state might be quite complex (e.g. a Deployment) and navigating to the salient parts, not easy.

Contrast the following implementations:

• Using the current interface:

public boolean match(Deployment actual, Deployment desired, Context context) {
    final var maybeDesiredContainer = desired.getSpec().getTemplate().getSpec().getContainers()
        .stream()
        .findFirst();
    if (maybeDesiredContainer.isEmpty()) {
      return true;
    }
    final var desiredContainer = maybeDesiredContainer.get();

    final var container = actual.getSpec().getTemplate().getSpec().getContainers()
        .stream()
        .findFirst();
    return container.map(c -> desiredContainer.getImage().equals(c.getImage())
            && desiredContainer.getEnv().equals(c.getEnv()))
        .orElse(false);
  }

• Using a version that would pass the primary resource instead:

public boolean match(Deployment actual, ExposedApp primary, Context context) {
    final var spec = primary.getSpec();
    Optional<Container> container = actual.getSpec().getTemplate().getSpec().getContainers()
        .stream()
        .findFirst();
    return container.map(
            c -> spec.getImageRef().equals(c.getImage()) && (spec.getEnv() == null || spec.getEnv()
                .equals(convert(c.getEnv()))))
        .orElse(false);
  }

  private Map<String, String> convert(List<EnvVar> envVars) {
    final var result = new HashMap<String, String>(envVars.size());
    envVars.forEach(
        envVar -> result.put(envVar.getName().toLowerCase(Locale.ROOT), envVar.getValue()));
    return result;
  }

I think the second implementation is clearer as to what is going on exactly and the relation to the primary resource state clearer. Granted, in this second case, we do need to convert part of the actual state to something that matches what is provided by the primary resource (that's the purpose of the convert method) but the relation between the primary resource state and whether the dependent state matches it is more explicit in this case.

This would also have the added benefit of not having to compute the full desired state if we can determine quickly that the actual state matches the expectations set by the primary resource as AbstractDependentResource.reconcile could be rewritten as follows:

public void reconcile(P primary, Context context) {
    final var creatable = isCreatable(primary, context);
    final var updatable = isUpdatable(primary, context);
    if (creatable || updatable) {
      var maybeActual = getResource(primary);
      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, primary, context)) {
          var desired = 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", actual);
        }
      }
    } else {
      log.debug(
          "Dependent {} is read-only, implement Creator and/or Updater interfaces to modify it",
          getClass().getSimpleName());
    }
  }

[csviri]

The match is quite a generic problem. Or a problem that could be solved in a very generic way, based on desire and the actual state (what we already do in the current implementation). Hopefully most of the users will be just use the default implementation out of the box.

I absolutely agree that some might need the primary to optimize for some special cases. On the other hand removing the desired would mean in generic implementation, that we would need to compute it there, and if not matches compute it again on update.

So IMHO would do just a compromise between. Adding the primary but not removing the desired from the interface.

Creating desired is just a POJO mapping basically from primary to desired, what is an extremely fast and non resource intensive operation, not sure if optimizing that makes sense in this case.

[metacosm]

The match is quite a generic problem. Or a problem that could be solved in a very generic way, based on desire and the actual state (what we already do in the current implementation). Hopefully most of the users will be just use the default implementation out of the box.

The default implementation is not neutral, though, at least for Kubernetes resources: it serialises both desired and actual resources then performs a JSON diff, which can potentially be rather intensive for large or often changing resources.

I absolutely agree that some might need the primary to optimize for some special cases. On the other hand removing the desired would mean in generic implementation, that we would need to compute it there, and if not matches compute it again on update.

The generic implementation is only used for Kubernetes dependents where, usually, computing the desired state is less resource-intensive than the actual comparison. That said, with some changes to the Matcher interface we could avoid this issue altogether.

So IMHO would do just a compromise between. Adding the primary but not removing the desired from the interface.

Creating desired is just a POJO mapping basically from primary to desired, what is an extremely fast and non resource intensive operation, not sure if optimizing that makes sense in this case.

It's not always true, though, if the desired state is complex to compute or if it deals with external resources

[csviri]

The default implementation is not neutral, though, at least for Kubernetes resources: it serialises both desired and actual resources then performs a JSON diff, which can potentially be rather intensive for large or often changing resources.

Yes, we should measure this part, to see the actual numbers.

The generic implementation is only used for Kubernetes dependents where, usually, computing the desired state is less resource-intensive than the actual comparison. That said, with some changes to the Matcher interface we could avoid this issue altogether.

Not sure I understand what you mean here. For non kubernetes like MySQL Schema, usually equals is used so, actual.equals(desired) this is what we do already for caching for polling event sources. So it's logical to do it also here. Otherwise we might want to re-thing the matching also in caches.