diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java index 456e9339de..8700af0805 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java +++ b/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 { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Matcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Matcher.java index ad82daf661..f0c5bd7fd9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Matcher.java +++ b/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); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Updater.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Updater.java index 6984f788f8..62199e1ede 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/Updater.java +++ b/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); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java index 1c3c7d0822..25d7c098d5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.java +++ b/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); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 8ce3560fdc..8bd534ab9a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/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); + } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java index 6302c20e6d..3ae424cdfb 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java +++ b/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(); + } } diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java index b118b07f0c..6c71252bb5 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java +++ b/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,7 +33,7 @@ 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() @@ -40,16 +41,19 @@ protected Secret desired(MySQLSchema schema, Context context) { .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