diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java index 957d78ac7d..2c8e5af017 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java @@ -11,6 +11,7 @@ import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; +import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result; import io.javaoperatorsdk.operator.processing.event.ResourceID; @Ignore @@ -20,61 +21,40 @@ public abstract class AbstractDependentResource private final boolean creatable = this instanceof Creator; private final boolean updatable = this instanceof Updater; - private final boolean bulk = this instanceof BulkDependentResource; protected Creator creator; protected Updater updater; - protected BulkDependentResource bulkDependentResource; private ResourceDiscriminator resourceDiscriminator; + private final DependentResourceReconciler dependentResourceReconciler; - @SuppressWarnings({"unchecked", "rawtypes"}) + @SuppressWarnings({"unchecked"}) protected AbstractDependentResource() { creator = creatable ? (Creator) this : null; updater = updatable ? (Updater) this : null; - bulkDependentResource = bulk ? (BulkDependentResource) this : null; + dependentResourceReconciler = this instanceof BulkDependentResource + ? new BulkDependentResourceReconciler<>((BulkDependentResource) this) + : new SingleDependentResourceReconciler<>(this); } - + /** + * Overriding classes are strongly encouraged to call this implementation as part of their + * implementation, as they otherwise risk breaking functionality. + * + * @param primary the primary resource for which we want to reconcile the dependent state + * @param context {@link Context} providing useful contextual information + * @return the reconciliation result + */ @Override public ReconcileResult reconcile(P primary, Context

context) { - if (bulk) { - final var targetResources = bulkDependentResource.desiredResources(primary, context); - - Map actualResources = - bulkDependentResource.getSecondaryResources(primary, context); - - deleteBulkResourcesIfRequired(targetResources.keySet(), actualResources, primary, context); - final List> results = new ArrayList<>(targetResources.size()); - - targetResources.forEach((key, resource) -> { - results.add(reconcileIndexAware(primary, actualResources.get(key), resource, key, context)); - }); - - return ReconcileResult.aggregatedResult(results); - } else { - var actualResource = getSecondaryResource(primary, context); - return reconcileIndexAware(primary, actualResource.orElse(null), null, null, context); - } + return dependentResourceReconciler.reconcile(primary, context); } - @SuppressWarnings({"rawtypes"}) - protected void deleteBulkResourcesIfRequired(Set targetKeys, Map actualResources, - P primary, Context

context) { - actualResources.forEach((key, value) -> { - if (!targetKeys.contains(key)) { - bulkDependentResource.deleteBulkResource(primary, value, key, context); - } - }); - } - - protected ReconcileResult reconcileIndexAware(P primary, R actualResource, R desiredResource, - String key, - Context

context) { + protected ReconcileResult reconcile(P primary, R actualResource, Context

context) { if (creatable || updatable) { if (actualResource == null) { if (creatable) { - var desired = bulkAwareDesired(primary, desiredResource, context); + var desired = desired(primary, context); throwIfNull(desired, primary, "Desired"); logForOperation("Creating", primary, desired); var createdResource = handleCreate(desired, primary, context); @@ -82,16 +62,9 @@ protected ReconcileResult reconcileIndexAware(P primary, R actualResource, R } } else { if (updatable) { - final Matcher.Result match; - if (bulk) { - match = - bulkDependentResource.match(actualResource, desiredResource, primary, key, context); - } else { - match = updater.match(actualResource, primary, context); - } + final Matcher.Result match = match(actualResource, primary, context); if (!match.matched()) { - final var desired = - match.computedDesired().orElse(bulkAwareDesired(primary, desiredResource, context)); + final var desired = match.computedDesired().orElse(desired(primary, context)); throwIfNull(desired, primary, "Desired"); logForOperation("Updating", primary, desired); var updatedResource = handleUpdate(actualResource, desired, primary, context); @@ -110,9 +83,8 @@ protected ReconcileResult reconcileIndexAware(P primary, R actualResource, R return ReconcileResult.noOperation(actualResource); } - private R bulkAwareDesired(P primary, R alreadyComputedDesire, Context

context) { - return bulk ? alreadyComputedDesire - : desired(primary, context); + public Result match(R resource, P primary, Context

context) { + return updater.match(resource, primary, context); } @Override @@ -179,12 +151,7 @@ protected R desired(P primary, Context

context) { } public void delete(P primary, Context

context) { - if (bulk) { - var actualResources = bulkDependentResource.getSecondaryResources(primary, context); - deleteBulkResourcesIfRequired(Collections.emptySet(), actualResources, primary, context); - } else { - handleDelete(primary, context); - } + dependentResourceReconciler.delete(primary, context); } protected void handleDelete(P primary, Context

context) { @@ -200,11 +167,8 @@ protected boolean isCreatable() { return creatable; } + @SuppressWarnings("unused") protected boolean isUpdatable() { return updatable; } - - protected boolean isBulk() { - return bulk; - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResource.java index fe8da3a091..5912a0f75c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResource.java @@ -17,10 +17,27 @@ public interface BulkDependentResource extends Creator, Deleter

{ /** - * @return number of resources to create + * Retrieves a map of desired secondary resources associated with the specified primary resource, + * identified by an arbitrary key. + * + * @param primary the primary resource with which we want to identify which secondary resources + * are associated + * @param context the {@link Context} associated with the current reconciliation + * @return a Map associating desired secondary resources with the specified primary via arbitrary + * identifiers */ Map desiredResources(P primary, Context

context); + /** + * Retrieves the actual secondary resources currently existing on the server and associated with + * the specified primary resource. + * + * @param primary the primary resource for which we want to retrieve the associated secondary + * resources + * @param context the {@link Context} associated with the current reconciliation + * @return a Map associating actual secondary resources with the specified primary via arbitrary + * identifiers + */ Map getSecondaryResources(P primary, Context

context); /** @@ -39,14 +56,15 @@ public interface BulkDependentResource * {@link Context}. * * @param actualResource the resource we want to determine whether it's matching the desired state + * @param desired the resource's desired state * @param primary the primary resource from which the desired state is inferred - * @param key key of the resource * @param context the context in which the resource is being matched * @return a {@link Result} encapsulating whether the resource matched its desired state and this * associated state if it was computed as part of the matching process. Use the static * convenience methods ({@link Result#nonComputed(boolean)} and * {@link Result#computed(boolean, Object)}) */ - Result match(R actualResource, R desired, P primary, String key, Context

context); - + default Result match(R actualResource, R desired, P primary, Context

context) { + return Matcher.Result.computed(desired.equals(actualResource), desired); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResourceReconciler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResourceReconciler.java new file mode 100644 index 0000000000..d0e3128983 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResourceReconciler.java @@ -0,0 +1,135 @@ +package io.javaoperatorsdk.operator.processing.dependent; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; +import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; +import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result; +import io.javaoperatorsdk.operator.processing.event.ResourceID; + +class BulkDependentResourceReconciler + implements DependentResourceReconciler { + + private final BulkDependentResource bulkDependentResource; + + BulkDependentResourceReconciler(BulkDependentResource bulkDependentResource) { + this.bulkDependentResource = bulkDependentResource; + } + + @Override + public ReconcileResult reconcile(P primary, Context

context) { + final var desiredResources = bulkDependentResource.desiredResources(primary, context); + Map actualResources = bulkDependentResource.getSecondaryResources(primary, context); + + // remove existing resources that are not needed anymore according to the primary state + deleteBulkResourcesIfRequired(desiredResources.keySet(), actualResources, primary, context); + + final List> results = new ArrayList<>(desiredResources.size()); + final var updatable = bulkDependentResource instanceof Updater; + desiredResources.forEach((key, value) -> { + final var instance = + updatable ? new UpdatableBulkDependentResourceInstance<>(bulkDependentResource, value) + : new BulkDependentResourceInstance<>(bulkDependentResource, value); + results.add(instance.reconcile(primary, actualResources.get(key), context)); + }); + + return ReconcileResult.aggregatedResult(results); + } + + @Override + public void delete(P primary, Context

context) { + var actualResources = bulkDependentResource.getSecondaryResources(primary, context); + deleteBulkResourcesIfRequired(Collections.emptySet(), actualResources, primary, context); + } + + private void deleteBulkResourcesIfRequired(Set expectedKeys, + Map actualResources, P primary, Context

context) { + actualResources.forEach((key, value) -> { + if (!expectedKeys.contains(key)) { + bulkDependentResource.deleteBulkResource(primary, value, key, context); + } + }); + } + + /** + * Exposes a dynamically-created instance of the bulk dependent resource precursor as an + * AbstractDependentResource so that we can reuse its reconciliation logic. + * + * @param + * @param

+ */ + private static class BulkDependentResourceInstance + extends AbstractDependentResource + implements Creator, Deleter

{ + private final BulkDependentResource bulkDependentResource; + private final R desired; + + private BulkDependentResourceInstance(BulkDependentResource bulkDependentResource, + R desired) { + this.bulkDependentResource = bulkDependentResource; + this.desired = desired; + } + + @SuppressWarnings("unchecked") + private AbstractDependentResource asAbstractDependentResource() { + return (AbstractDependentResource) bulkDependentResource; + } + + @Override + protected R desired(P primary, Context

context) { + return desired; + } + + @SuppressWarnings("unchecked") + public R update(R actual, R desired, P primary, Context

context) { + return ((Updater) bulkDependentResource).update(actual, desired, primary, context); + } + + @Override + public Result match(R resource, P primary, Context

context) { + return bulkDependentResource.match(resource, desired, primary, context); + } + + @Override + protected void onCreated(ResourceID primaryResourceId, R created) { + asAbstractDependentResource().onCreated(primaryResourceId, created); + } + + @Override + protected void onUpdated(ResourceID primaryResourceId, R updated, R actual) { + asAbstractDependentResource().onUpdated(primaryResourceId, updated, actual); + } + + @Override + public Class resourceType() { + return asAbstractDependentResource().resourceType(); + } + + @Override + public R create(R desired, P primary, Context

context) { + return bulkDependentResource.create(desired, primary, context); + } + } + + /** + * Makes sure that the instance implements Updater if its precursor does as well. + * + * @param + * @param

+ */ + private static class UpdatableBulkDependentResourceInstance + extends BulkDependentResourceInstance implements Updater { + + private UpdatableBulkDependentResourceInstance( + BulkDependentResource bulkDependentResource, + R desired) { + super(bulkDependentResource, desired); + } + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceReconciler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceReconciler.java new file mode 100644 index 0000000000..ff687ff474 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceReconciler.java @@ -0,0 +1,20 @@ +package io.javaoperatorsdk.operator.processing.dependent; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; + +/** + * An internal interface that abstracts away how to reconcile dependent resources, in particular + * when they can be dynamically created based on the state provided by the primary resource (e.g. + * the primary resource dictates which/how many secondary resources need to be created). + * + * @param the type of the secondary resource to be reconciled + * @param

the primary resource type + */ +interface DependentResourceReconciler { + + ReconcileResult reconcile(P primary, Context

context); + + void delete(P primary, Context

context); +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/SingleDependentResourceReconciler.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/SingleDependentResourceReconciler.java new file mode 100644 index 0000000000..20b37b3c7e --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/SingleDependentResourceReconciler.java @@ -0,0 +1,26 @@ +package io.javaoperatorsdk.operator.processing.dependent; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; + +class SingleDependentResourceReconciler + implements DependentResourceReconciler { + + private final AbstractDependentResource instance; + + SingleDependentResourceReconciler(AbstractDependentResource dependentResource) { + this.instance = dependentResource; + } + + @Override + public ReconcileResult reconcile(P primary, Context

context) { + final var maybeActual = instance.getSecondaryResource(primary, context); + return instance.reconcile(primary, maybeActual.orElse(null), context); + } + + @Override + public void delete(P primary, Context

context) { + instance.handleDelete(primary, context); + } +} 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 82df9dbbb9..8674da97d2 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 @@ -139,7 +139,8 @@ public Result match(R actualResource, P primary, Context

context) { return matcher.match(actualResource, primary, context); } - public Result match(R actualResource, R desired, P primary, String key, Context

context) { + @SuppressWarnings("unused") + public Result match(R actualResource, R desired, P primary, Context

context) { return GenericKubernetesResourceMatcher.match(desired, actualResource, false); } @@ -148,6 +149,7 @@ protected void handleDelete(P primary, Context

context) { resource.ifPresent(r -> client.resource(r).delete()); } + @SuppressWarnings("unused") public void deleteBulkResource(P primary, R resource, String key, Context

context) { client.resource(resource).delete(); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkDependentResource.java index 3a13e29707..ec8161bae8 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkDependentResource.java @@ -6,7 +6,6 @@ import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.dependent.BulkDependentResource; import io.javaoperatorsdk.operator.processing.dependent.BulkUpdater; -import io.javaoperatorsdk.operator.processing.dependent.Matcher; import io.javaoperatorsdk.operator.processing.dependent.external.PollingDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.sample.bulkdependent.BulkDependentTestCustomResource; @@ -92,12 +91,4 @@ public void deleteBulkResource(BulkDependentTestCustomResource primary, External Context context) { externalServiceMock.delete(resource.getId()); } - - @Override - public Matcher.Result match(ExternalResource actualResource, - ExternalResource desired, - BulkDependentTestCustomResource primary, String index, - Context context) { - return Matcher.Result.computed(desired.equals(actualResource), desired); - } }