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 63e522ad7d..201f08add6 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 @@ -1,7 +1,5 @@ package io.javaoperatorsdk.operator.processing.dependent; -import java.util.ArrayList; -import java.util.List; import java.util.Optional; import org.slf4j.Logger; @@ -20,15 +18,15 @@ public abstract class AbstractDependentResource implements DependentResource { private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class); - protected final boolean creatable = this instanceof Creator; - protected final boolean updatable = this instanceof Updater; - protected final boolean bulk = this instanceof BulkDependentResource; + 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; - - protected List> resourceDiscriminator = new ArrayList<>(1); + private ResourceDiscriminator resourceDiscriminator; + private int currentCount; @SuppressWarnings("unchecked") public AbstractDependentResource() { @@ -42,48 +40,33 @@ public AbstractDependentResource() { public ReconcileResult reconcile(P primary, Context

context) { if (bulk) { final var count = bulkDependentResource.count(primary, context); - deleteBulkResourcesIfRequired(count, lastKnownBulkSize(), primary, context); - adjustDiscriminators(count); + deleteBulkResourcesIfRequired(count, primary, context); @SuppressWarnings("unchecked") final ReconcileResult[] results = new ReconcileResult[count]; for (int i = 0; i < count; i++) { results[i] = reconcileIndexAware(primary, i, context); } + currentCount = count; return ReconcileResult.aggregatedResult(results); } else { return reconcileIndexAware(primary, 0, context); } } - protected void deleteBulkResourcesIfRequired(int targetCount, int actualCount, P primary, - Context

context) { - if (targetCount >= actualCount) { + protected void deleteBulkResourcesIfRequired(int targetCount, P primary, Context

context) { + if (targetCount >= currentCount) { return; } - for (int i = targetCount; i < actualCount; i++) { - var resource = getSecondaryResourceIndexAware(primary, i, context); + for (int i = targetCount; i < currentCount; i++) { + var resource = bulkDependentResource.getSecondaryResource(primary, i, context); var index = i; resource.ifPresent( r -> bulkDependentResource.deleteBulkResourceWithIndex(primary, r, index, context)); } } - private void adjustDiscriminators(int count) { - if (resourceDiscriminator.size() == count) { - return; - } - if (resourceDiscriminator.size() < count) { - for (int i = resourceDiscriminator.size(); i < count; i++) { - resourceDiscriminator.add(bulkDependentResource.getResourceDiscriminator(i)); - } - } - if (resourceDiscriminator.size() > count) { - resourceDiscriminator.subList(count, resourceDiscriminator.size()).clear(); - } - } - protected ReconcileResult reconcileIndexAware(P primary, int i, Context

context) { - Optional maybeActual = bulk ? getSecondaryResourceIndexAware(primary, i, context) + Optional maybeActual = bulk ? bulkDependentResource.getSecondaryResource(primary, i, context) : getSecondaryResource(primary, context); if (creatable || updatable) { if (maybeActual.isEmpty()) { @@ -99,7 +82,7 @@ protected ReconcileResult reconcileIndexAware(P primary, int i, Context

co if (updatable) { final Matcher.Result match; if (bulk) { - match = updater.match(actual, primary, i, context); + match = bulkDependentResource.match(actual, primary, i, context); } else { match = updater.match(actual, primary, context); } @@ -124,17 +107,12 @@ protected ReconcileResult reconcileIndexAware(P primary, int i, Context

co } private R desiredIndexAware(P primary, int i, Context

context) { - return bulk ? desired(primary, i, context) - : desired(primary, context); + return bulk ? desired(primary, i, context) : desired(primary, context); } public Optional getSecondaryResource(P primary, Context

context) { - return resourceDiscriminator.isEmpty() ? context.getSecondaryResource(resourceType()) - : resourceDiscriminator.get(0).distinguish(resourceType(), primary, context); - } - - protected Optional getSecondaryResourceIndexAware(P primary, int index, Context

context) { - return context.getSecondaryResource(resourceType(), resourceDiscriminator.get(index)); + return resourceDiscriminator == null ? context.getSecondaryResource(resourceType()) + : resourceDiscriminator.distinguish(resourceType(), primary, context); } private void throwIfNull(R desired, P primary, String descriptor) { @@ -195,28 +173,35 @@ protected R desired(P primary, Context

context) { } protected R desired(P primary, int index, Context

context) { - throw new IllegalStateException( - "Must be implemented for bulk DependentResource creation"); + throw new IllegalStateException("Must be implemented for bulk DependentResource creation"); } - public AbstractDependentResource setResourceDiscriminator( - ResourceDiscriminator resourceDiscriminator) { - if (resourceDiscriminator != null) { - this.resourceDiscriminator.add(resourceDiscriminator); + public void delete(P primary, Context

context) { + if (bulk) { + deleteBulkResourcesIfRequired(0, primary, context); + } else { + handleDelete(primary, context); } - return this; } - public ResourceDiscriminator getResourceDiscriminator() { - if (this.resourceDiscriminator.isEmpty()) { - return null; - } else { - return this.resourceDiscriminator.get(0); - } + protected void handleDelete(P primary, Context

context) { + throw new IllegalStateException("delete method be implemented if Deleter trait is supported"); } - protected int lastKnownBulkSize() { - return resourceDiscriminator.size(); + public void setResourceDiscriminator( + ResourceDiscriminator resourceDiscriminator) { + this.resourceDiscriminator = resourceDiscriminator; } + protected boolean isCreatable() { + return creatable; + } + + 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 1f2688f5cb..64a174e201 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 @@ -1,9 +1,11 @@ package io.javaoperatorsdk.operator.processing.dependent; +import java.util.Optional; + import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; +import io.javaoperatorsdk.operator.processing.dependent.Matcher.Result; /** * Manages dynamic number of resources created for a primary resource. Since the point of a bulk @@ -30,6 +32,21 @@ public interface BulkDependentResource extends Creator */ void deleteBulkResourceWithIndex(P primary, R resource, int i, Context

context); - ResourceDiscriminator getResourceDiscriminator(int index); + /** + * Determines whether the specified secondary resource matches the desired state with target index + * of a bulk resource as defined from the specified primary resource, given the specified + * {@link Context}. + * + * @param actualResource the resource we want to determine whether it's matching the desired state + * @param primary the primary resource from which the desired state is inferred + * @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, P primary, int index, Context

context); + + Optional getSecondaryResource(P primary, int index, Context

context); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkUpdater.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkUpdater.java index 9c00b47d0c..ee8f08a68d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkUpdater.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkUpdater.java @@ -13,8 +13,12 @@ public interface BulkUpdater extends Updater { default Matcher.Result match(R actualResource, P primary, Context

context) { - throw new IllegalStateException(); + if (!(this instanceof BulkDependentResource)) { + throw new IllegalStateException( + BulkUpdater.class.getSimpleName() + " interface should only be implemented by " + + BulkDependentResource.class.getSimpleName() + " implementations"); + } + throw new IllegalStateException("This method should not be called from a " + + BulkDependentResource.class.getSimpleName() + " implementation"); } - - Matcher.Result match(R actualResource, P primary, int index, Context

context); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DesiredEqualsMatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DesiredEqualsMatcher.java index 1d3b34a47b..459d7951d6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DesiredEqualsMatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DesiredEqualsMatcher.java @@ -16,10 +16,4 @@ public Result match(R actualResource, P primary, Context

context) { var desired = abstractDependentResource.desired(primary, context); return Result.computed(actualResource.equals(desired), desired); } - - @Override - public Result match(R actualResource, P primary, int index, Context

context) { - var desired = abstractDependentResource.desired(primary, index, context); - return Result.computed(actualResource.equals(desired), desired); - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/Matcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/Matcher.java index 835f76ab3a..750fe89cbf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/Matcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/Matcher.java @@ -95,19 +95,4 @@ public Optional computedDesired() { * {@link Result#computed(boolean, Object)}) */ Result match(R actualResource, P primary, Context

context); - - /** - * Determines whether the specified secondary resource matches the desired state with target index - * of a bulk resource as defined from the specified primary resource, given the specified - * {@link Context}. - * - * @param actualResource the resource we want to determine whether it's matching the desired state - * @param primary the primary resource from which the desired state is inferred - * @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, P primary, int index, Context

context); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/Updater.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/Updater.java index 06b3cb52f6..828f9ad785 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/Updater.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/Updater.java @@ -8,8 +8,4 @@ public interface Updater { R update(R actual, R desired, P primary, Context

context); Result match(R actualResource, P primary, Context

context); - - default Result match(R actualResource, P primary, int index, Context

context) { - throw new IllegalStateException("Implement this for bulk matching"); - } } 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 bb066b5b24..9952763e6a 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 @@ -23,47 +23,7 @@ private GenericKubernetesResourceMatcher(KubernetesDependentResource depen @SuppressWarnings({"unchecked", "rawtypes"}) static Matcher matcherFor( Class resourceType, KubernetesDependentResource dependentResource) { - if (Secret.class.isAssignableFrom(resourceType)) { - return new Matcher<>() { - @Override - public Result match(R actualResource, P primary, Context

context) { - final var desired = dependentResource.desired(primary, context); - return Result.computed( - ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource), - desired); - } - - @Override - public Result match(R actualResource, P primary, int index, Context

context) { - final var desired = dependentResource.desired(primary, index, context); - return Result.computed( - ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource), - desired); - } - }; - } else if (ConfigMap.class.isAssignableFrom(resourceType)) { - return new Matcher<>() { - @Override - public Result match(R actualResource, P primary, Context

context) { - final var desired = dependentResource.desired(primary, context); - return Result.computed( - ResourceComparators.compareConfigMapData((ConfigMap) desired, - (ConfigMap) actualResource), - desired); - } - - @Override - public Result match(R actualResource, P primary, int index, Context

context) { - final var desired = dependentResource.desired(primary, index, context); - return Result.computed( - ResourceComparators.compareConfigMapData((ConfigMap) desired, - (ConfigMap) actualResource), - desired); - } - }; - } else { - return new GenericKubernetesResourceMatcher(dependentResource); - } + return new GenericKubernetesResourceMatcher(dependentResource); } @Override @@ -72,14 +32,8 @@ public Result match(R actualResource, P primary, Context

context) { return match(desired, actualResource, false); } - @Override - public Result match(R actualResource, P primary, int index, Context

context) { - var desired = dependentResource.desired(primary, index, context); - return match(desired, actualResource, false); - } - - public static Result match( - R desired, R actualResource, boolean considerMetadata) { + public static Result match(R desired, R actualResource, + boolean considerMetadata) { if (considerMetadata) { final var desiredMetadata = desired.getMetadata(); final var actualMetadata = actualResource.getMetadata(); @@ -91,20 +45,30 @@ public static Result match( } } - final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper(); + if (desired instanceof ConfigMap) { + return Result.computed( + ResourceComparators.compareConfigMapData((ConfigMap) desired, (ConfigMap) actualResource), + desired); + } else if (desired instanceof Secret) { + return Result.computed( + ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource), + desired); + } else { + final var objectMapper = ConfigurationServiceProvider.instance().getObjectMapper(); - // reflection will be replaced by this: - // https://github.com/fabric8io/kubernetes-client/issues/3816 - 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 Result.computed(false, desired); + // reflection will be replaced by this: + // https://github.com/fabric8io/kubernetes-client/issues/3816 + 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 Result.computed(false, desired); + } } + return Result.computed(true, desired); } - 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 d019e79a7f..1aaed6f12e 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 @@ -140,25 +140,20 @@ public Result match(R actualResource, P primary, Context

context) { } public Result match(R actualResource, P primary, int index, Context

context) { - return matcher.match(actualResource, primary, index, context); + final var desired = desired(primary, index, context); + return GenericKubernetesResourceMatcher.match(desired, actualResource, false); } - public void delete(P primary, Context

context) { - if (bulk) { - deleteBulkResourcesIfRequired(0, lastKnownBulkSize(), primary, context); - } else { - var resource = getSecondaryResource(primary, context); - resource.ifPresent(r -> client.resource(r).delete()); - } + protected void handleDelete(P primary, Context

context) { + var resource = getSecondaryResource(primary, context); + resource.ifPresent(r -> client.resource(r).delete()); } public void deleteBulkResourceWithIndex(P primary, R resource, int i, Context

context) { client.resource(resource).delete(); } - @SuppressWarnings("unchecked") - protected Resource prepare(R desired, - P primary, String actionName) { + protected Resource prepare(R desired, P primary, String actionName) { log.debug("{} target resource with type: {}, with id: {}", actionName, desired.getClass(), @@ -176,7 +171,6 @@ protected Resource prepare(R desired, protected InformerEventSource createEventSource(EventSourceContext

context) { if (kubernetesDependentResourceConfig != null) { // sets the filters for the dependent resource, which are applied by parent class - onAddFilter = kubernetesDependentResourceConfig.onAddFilter(); onUpdateFilter = kubernetesDependentResourceConfig.onUpdateFilter(); onDeleteFilter = kubernetesDependentResourceConfig.onDeleteFilter(); @@ -199,7 +193,7 @@ protected InformerEventSource createEventSource(EventSourceContext

cont } private boolean useDefaultAnnotationsToIdentifyPrimary() { - return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && creatable; + return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && isCreatable(); } private void addDefaultSecondaryToPrimaryMapperAnnotations(R desired, P primary) { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java index a7fbd9cb98..d8f85bb10a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java @@ -7,7 +7,6 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.processing.dependent.BulkDependentResource; import io.javaoperatorsdk.operator.processing.dependent.Creator; @@ -54,19 +53,17 @@ public int count(BulkDependentTestCustomResource primary, } @Override - public ResourceDiscriminator getResourceDiscriminator( - int index) { - return (resource, primary, context) -> { - var resources = context.getSecondaryResources(resource).stream() - .filter(r -> r.getMetadata().getName().endsWith("-" + index)) - .collect(Collectors.toList()); - if (resources.isEmpty()) { - return Optional.empty(); - } else if (resources.size() > 1) { - throw new IllegalStateException("More than one resource found for index:" + index); - } else { - return Optional.of(resources.get(0)); - } - }; + public Optional getSecondaryResource(BulkDependentTestCustomResource primary, + int index, Context context) { + var resources = context.getSecondaryResources(resourceType()).stream() + .filter(r -> r.getMetadata().getName().endsWith("-" + index)) + .collect(Collectors.toList()); + if (resources.isEmpty()) { + return Optional.empty(); + } else if (resources.size() > 1) { + throw new IllegalStateException("More than one resource found for index:" + index); + } else { + return Optional.of(resources.get(0)); + } } } 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 110626a923..ed486f6f34 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 @@ -1,11 +1,16 @@ package io.javaoperatorsdk.operator.sample.bulkdependent.external; -import java.util.*; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Optional; +import java.util.Set; import java.util.stream.Collectors; import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; -import io.javaoperatorsdk.operator.processing.dependent.*; +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; @@ -38,7 +43,7 @@ public Map> fetchResources() { @Override public void delete(BulkDependentTestCustomResource primary, Context context) { - deleteBulkResourcesIfRequired(0, lastKnownBulkSize(), primary, context); + deleteBulkResourcesIfRequired(0, primary, context); } @Override @@ -92,9 +97,9 @@ private ResourceID toResourceID(ExternalResource externalResource) { } @Override - public ResourceDiscriminator getResourceDiscriminator( - int index) { - return (resource, primary, context) -> context.getSecondaryResources(resource).stream() + public Optional getSecondaryResource(BulkDependentTestCustomResource primary, + int index, Context context) { + return context.getSecondaryResources(resourceType()).stream() .filter(r -> r.getId().endsWith(EXTERNAL_RESOURCE_NAME_DELIMITER + index)) .collect(Collectors.toList()).stream().findFirst(); }