From 8cf411b9ba4e14fb342977ff90783882e8693016 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 6 Oct 2022 10:28:30 +0200 Subject: [PATCH 1/6] refactor: isolate bulk dependent resource handling more --- .../dependent/AbstractDependentResource.java | 72 +++----------- .../dependent/BulkDependentResource.java | 9 +- .../BulkDependentResourceReconciler.java | 97 +++++++++++++++++++ .../DependentResourceReconciler.java | 12 +++ .../SingleDependentResourceReconciler.java | 26 +++++ .../KubernetesDependentResource.java | 1 + 6 files changed, 159 insertions(+), 58 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResourceReconciler.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceReconciler.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/SingleDependentResourceReconciler.java 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..60b85c1d6a 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,32 @@ 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); } - @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 resource, 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); @@ -83,15 +55,10 @@ 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); - } + match = match(resource, primary, context); if (!match.matched()) { final var desired = - match.computedDesired().orElse(bulkAwareDesired(primary, desiredResource, context)); + match.computedDesired().orElse(desired(primary, context)); throwIfNull(desired, primary, "Desired"); logForOperation("Updating", primary, desired); var updatedResource = handleUpdate(actualResource, desired, primary, context); @@ -110,9 +77,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 +145,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 +161,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..25f4e01320 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,7 +17,14 @@ 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 bulk secondary resources with the specified primary via arbitrary + * identifiers */ Map desiredResources(P primary, Context

context); 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..11c7dbf80c --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResourceReconciler.java @@ -0,0 +1,97 @@ +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.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 targetKeys = bulkDependentResource.desiredResourceKeys(primary, context); + Map actualResources = + bulkDependentResource.getSecondaryResources(primary, context); + + deleteBulkResourcesIfRequired(targetKeys, actualResources, primary, context); + + final List> results = new ArrayList<>(targetKeys.size()); + actualResources.forEach((key, value) -> { + final var instance = new BulkDependentResourceInstance<>(key, bulkDependentResource); + results.add(instance.reconcile(primary, value, 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); + } + + protected void deleteBulkResourcesIfRequired(Set expectedKeys, + Map actualResources, P primary, Context

context) { + actualResources.forEach((key, value) -> { + if (!expectedKeys.contains(key)) { + bulkDependentResource.deleteBulkResource(primary, value, key, context); + } + }); + } + + private static class BulkDependentResourceInstance + extends AbstractDependentResource { + private final String key; + private final BulkDependentResource bulkDependentResource; + + private BulkDependentResourceInstance(String key, + BulkDependentResource bulkDependentResource) { + this.key = key; + this.bulkDependentResource = bulkDependentResource; + } + + @SuppressWarnings("unchecked") + private AbstractDependentResource asAbstractDependentResource() { + return (AbstractDependentResource) bulkDependentResource; + } + + @Override + protected R desired(P primary, Context

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

context) { + return bulkDependentResource.match(resource, primary, key, 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(); + } + } +} 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..112905d926 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceReconciler.java @@ -0,0 +1,12 @@ +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; + +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..4fc4168cbc 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 @@ -148,6 +148,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(); } From bfe3e670cf081b44a6567172781c1eb2029c7ee6 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 7 Oct 2022 14:28:32 +0200 Subject: [PATCH 2/6] refactor: adapt after rebase --- .../dependent/AbstractDependentResource.java | 4 ++-- .../dependent/BulkDependentResource.java | 7 ++++--- .../BulkDependentResourceReconciler.java | 20 +++++++++---------- .../KubernetesDependentResource.java | 3 ++- .../ExternalBulkDependentResource.java | 9 --------- 5 files changed, 18 insertions(+), 25 deletions(-) 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 60b85c1d6a..ce916887dc 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 @@ -42,7 +42,7 @@ public ReconcileResult reconcile(P primary, Context

context) { return dependentResourceReconciler.reconcile(primary, context); } - protected ReconcileResult reconcile(P primary, R resource, Context

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

context) { if (creatable || updatable) { if (actualResource == null) { if (creatable) { @@ -55,7 +55,7 @@ protected ReconcileResult reconcile(P primary, R resource, Context

context } else { if (updatable) { final Matcher.Result match; - match = match(resource, primary, context); + match = match(actualResource, primary, context); if (!match.matched()) { final var desired = match.computedDesired().orElse(desired(primary, context)); 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 25f4e01320..6c2e796c1d 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 @@ -46,14 +46,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 index 11c7dbf80c..d28fc5084e 100644 --- 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 @@ -23,15 +23,15 @@ class BulkDependentResourceReconciler @Override public ReconcileResult reconcile(P primary, Context

context) { - final var targetKeys = bulkDependentResource.desiredResourceKeys(primary, context); + final var desiredResources = bulkDependentResource.desiredResources(primary, context); Map actualResources = bulkDependentResource.getSecondaryResources(primary, context); - deleteBulkResourcesIfRequired(targetKeys, actualResources, primary, context); + deleteBulkResourcesIfRequired(desiredResources.keySet(), actualResources, primary, context); - final List> results = new ArrayList<>(targetKeys.size()); + final List> results = new ArrayList<>(desiredResources.size()); actualResources.forEach((key, value) -> { - final var instance = new BulkDependentResourceInstance<>(key, bulkDependentResource); + final var instance = new BulkDependentResourceInstance<>(bulkDependentResource, value); results.add(instance.reconcile(primary, value, context)); }); @@ -55,13 +55,13 @@ protected void deleteBulkResourcesIfRequired(Set expectedKeys, private static class BulkDependentResourceInstance extends AbstractDependentResource { - private final String key; private final BulkDependentResource bulkDependentResource; + private final R desired; - private BulkDependentResourceInstance(String key, - BulkDependentResource bulkDependentResource) { - this.key = key; + private BulkDependentResourceInstance(BulkDependentResource bulkDependentResource, + R desired) { this.bulkDependentResource = bulkDependentResource; + this.desired = desired; } @SuppressWarnings("unchecked") @@ -71,12 +71,12 @@ private AbstractDependentResource asAbstractDependentResource() { @Override protected R desired(P primary, Context

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

context) { - return bulkDependentResource.match(resource, primary, key, context); + return bulkDependentResource.match(resource, desired, primary, context); } @Override 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 4fc4168cbc..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); } 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); - } } From bf96682aba9b13edb560537d94a2f81ae3badf71 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 7 Oct 2022 15:38:11 +0200 Subject: [PATCH 3/6] fix: iterate over desired instead of actual, handle updatable case --- .../dependent/AbstractDependentResource.java | 3 +- .../BulkDependentResourceReconciler.java | 32 ++++++++++++++++--- 2 files changed, 29 insertions(+), 6 deletions(-) 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 ce916887dc..e51ce5f9c2 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 @@ -57,8 +57,7 @@ protected ReconcileResult reconcile(P primary, R actualResource, Context

c final Matcher.Result match; match = match(actualResource, primary, context); if (!match.matched()) { - final var desired = - match.computedDesired().orElse(desired(primary, 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); 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 index d28fc5084e..dfd53011ae 100644 --- 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 @@ -8,6 +8,7 @@ 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; @@ -30,9 +31,12 @@ public ReconcileResult reconcile(P primary, Context

context) { deleteBulkResourcesIfRequired(desiredResources.keySet(), actualResources, primary, context); final List> results = new ArrayList<>(desiredResources.size()); - actualResources.forEach((key, value) -> { - final var instance = new BulkDependentResourceInstance<>(bulkDependentResource, value); - results.add(instance.reconcile(primary, value, context)); + 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); @@ -54,7 +58,8 @@ protected void deleteBulkResourcesIfRequired(Set expectedKeys, } private static class BulkDependentResourceInstance - extends AbstractDependentResource { + extends AbstractDependentResource + implements Creator, Deleter

{ private final BulkDependentResource bulkDependentResource; private final R desired; @@ -73,6 +78,10 @@ private AbstractDependentResource asAbstractDependentResource() { 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) { @@ -93,5 +102,20 @@ protected void onUpdated(ResourceID primaryResourceId, R updated, R actual) { public Class resourceType() { return asAbstractDependentResource().resourceType(); } + + @Override + public R create(R desired, P primary, Context

context) { + return bulkDependentResource.create(desired, primary, context); + } + } + + private static class UpdatableBulkDependentResourceInstance + extends BulkDependentResourceInstance implements Updater { + + private UpdatableBulkDependentResourceInstance( + BulkDependentResource bulkDependentResource, + R desired) { + super(bulkDependentResource, desired); + } } } From abd9186047396ca07b9971ea98a1e2688c0b81f4 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 7 Oct 2022 15:48:55 +0200 Subject: [PATCH 4/6] fix: format [skip ci] --- .../processing/dependent/BulkDependentResourceReconciler.java | 1 + 1 file changed, 1 insertion(+) 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 index dfd53011ae..e08a89ba2b 100644 --- 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 @@ -78,6 +78,7 @@ private AbstractDependentResource asAbstractDependentResource() { 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); From 5f0407c8a12142eed6adee6414d3d5eba5686eda Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 12 Oct 2022 10:48:09 +0200 Subject: [PATCH 5/6] docs: add more documentation / comments --- .../dependent/AbstractDependentResource.java | 11 +++++++++-- .../dependent/BulkDependentResource.java | 12 +++++++++++- .../BulkDependentResourceReconciler.java | 19 ++++++++++++++++--- .../DependentResourceReconciler.java | 8 ++++++++ 4 files changed, 44 insertions(+), 6 deletions(-) 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 e51ce5f9c2..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 @@ -37,6 +37,14 @@ protected AbstractDependentResource() { : 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) { return dependentResourceReconciler.reconcile(primary, context); @@ -54,8 +62,7 @@ protected ReconcileResult reconcile(P primary, R actualResource, Context

c } } else { if (updatable) { - final Matcher.Result match; - match = match(actualResource, primary, context); + final Matcher.Result match = match(actualResource, primary, context); if (!match.matched()) { final var desired = match.computedDesired().orElse(desired(primary, context)); throwIfNull(desired, primary, "Desired"); 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 6c2e796c1d..fadafca9db 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 @@ -23,11 +23,21 @@ public interface BulkDependentResource * @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 bulk secondary resources with the specified primary via arbitrary + * @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); /** 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 index e08a89ba2b..04080f4224 100644 --- 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 @@ -25,9 +25,9 @@ class BulkDependentResourceReconciler @Override public ReconcileResult reconcile(P primary, Context

context) { final var desiredResources = bulkDependentResource.desiredResources(primary, context); - Map actualResources = - bulkDependentResource.getSecondaryResources(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()); @@ -48,7 +48,7 @@ public void delete(P primary, Context

context) { deleteBulkResourcesIfRequired(Collections.emptySet(), actualResources, primary, context); } - protected void deleteBulkResourcesIfRequired(Set expectedKeys, + private void deleteBulkResourcesIfRequired(Set expectedKeys, Map actualResources, P primary, Context

context) { actualResources.forEach((key, value) -> { if (!expectedKeys.contains(key)) { @@ -57,6 +57,13 @@ protected void deleteBulkResourcesIfRequired(Set expectedKeys, }); } + /** + * 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

{ @@ -110,6 +117,12 @@ public R create(R desired, P primary, Context

context) { } } + /** + * Makes sure that the instance implements Updater if its precursor does as well. + * + * @param + * @param

+ */ private static class UpdatableBulkDependentResourceInstance extends BulkDependentResourceInstance implements Updater { 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 index 112905d926..ff687ff474 100644 --- 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 @@ -4,6 +4,14 @@ 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); From 7d92eaaa71f1115c462574b6c8584336636be7c1 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 12 Oct 2022 11:33:09 +0200 Subject: [PATCH 6/6] fix: format --- .../operator/processing/dependent/BulkDependentResource.java | 2 +- .../processing/dependent/BulkDependentResourceReconciler.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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 fadafca9db..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 @@ -31,7 +31,7 @@ public interface BulkDependentResource /** * 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 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 index 04080f4224..d0e3128983 100644 --- 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 @@ -60,7 +60,7 @@ private void deleteBulkResourcesIfRequired(Set expectedKeys, /** * Exposes a dynamically-created instance of the bulk dependent resource precursor as an * AbstractDependentResource so that we can reuse its reconciliation logic. - * + * * @param * @param

*/ @@ -119,7 +119,7 @@ public R create(R desired, P primary, Context

context) { /** * Makes sure that the instance implements Updater if its precursor does as well. - * + * * @param * @param

*/