diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ReconcileResult.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ReconcileResult.java index 468e14e8ea..66d982f01d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ReconcileResult.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ReconcileResult.java @@ -22,15 +22,14 @@ public static ReconcileResult noOperation(T resource) { return new ReconcileResult<>(resource, Operation.NONE); } - @SafeVarargs - public static ReconcileResult aggregatedResult(ReconcileResult... results) { + public static ReconcileResult aggregatedResult(List> results) { if (results == null) { throw new IllegalArgumentException("Should provide results to aggregate"); } - if (results.length == 1) { - return results[0]; + if (results.size() == 1) { + return results.get(0); } - final Map operations = new HashMap<>(results.length); + final Map operations = new HashMap<>(results.size()); for (ReconcileResult res : results) { res.getSingleResource().ifPresent(r -> operations.put(r, res.getSingleOperation())); } 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 201f08add6..1786c3a568 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,6 +1,6 @@ package io.javaoperatorsdk.operator.processing.dependent; -import java.util.Optional; +import java.util.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -26,76 +26,75 @@ public abstract class AbstractDependentResource protected Updater updater; protected BulkDependentResource bulkDependentResource; private ResourceDiscriminator resourceDiscriminator; - private int currentCount; - @SuppressWarnings("unchecked") - public AbstractDependentResource() { + @SuppressWarnings({"unchecked", "rawtypes"}) + protected AbstractDependentResource() { creator = creatable ? (Creator) this : null; updater = updatable ? (Updater) this : null; - bulkDependentResource = bulk ? (BulkDependentResource) this : null; + bulkDependentResource = bulk ? (BulkDependentResource) this : null; } + @Override public ReconcileResult reconcile(P primary, Context

context) { if (bulk) { - final var count = bulkDependentResource.count(primary, context); - 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); + final var targetKeys = bulkDependentResource.targetKeys(primary, context); + Map actualResources = + bulkDependentResource.getSecondaryResources(primary, context); + + deleteBulkResourcesIfRequired(targetKeys, actualResources, primary, context); + final List> results = new ArrayList<>(targetKeys.size()); + + for (String key : targetKeys) { + results.add(reconcileIndexAware(primary, actualResources.get(key), key, context)); } - currentCount = count; return ReconcileResult.aggregatedResult(results); } else { - return reconcileIndexAware(primary, 0, context); + var actualResource = getSecondaryResource(primary, context); + return reconcileIndexAware(primary, actualResource.orElse(null), null, context); } } - protected void deleteBulkResourcesIfRequired(int targetCount, P primary, Context

context) { - if (targetCount >= currentCount) { - return; - } - 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)); - } + @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, int i, Context

context) { - Optional maybeActual = bulk ? bulkDependentResource.getSecondaryResource(primary, i, context) - : getSecondaryResource(primary, context); + protected ReconcileResult reconcileIndexAware(P primary, R resource, String key, + Context

context) { if (creatable || updatable) { - if (maybeActual.isEmpty()) { + if (resource == null) { if (creatable) { - var desired = desiredIndexAware(primary, i, context); + var desired = desiredIndexAware(primary, key, context); throwIfNull(desired, primary, "Desired"); logForOperation("Creating", primary, desired); var createdResource = handleCreate(desired, primary, context); return ReconcileResult.resourceCreated(createdResource); } } else { - final var actual = maybeActual.get(); if (updatable) { final Matcher.Result match; if (bulk) { - match = bulkDependentResource.match(actual, primary, i, context); + match = bulkDependentResource.match(resource, primary, key, context); } else { - match = updater.match(actual, primary, context); + match = updater.match(resource, primary, context); } if (!match.matched()) { final var desired = - match.computedDesired().orElse(desiredIndexAware(primary, i, context)); + match.computedDesired().orElse(desiredIndexAware(primary, key, context)); throwIfNull(desired, primary, "Desired"); logForOperation("Updating", primary, desired); - var updatedResource = handleUpdate(actual, desired, primary, context); + var updatedResource = handleUpdate(resource, desired, primary, context); return ReconcileResult.resourceUpdated(updatedResource); } } else { - log.debug("Update skipped for dependent {} as it matched the existing one", actual); + log.debug("Update skipped for dependent {} as it matched the existing one", resource); } } } else { @@ -103,13 +102,15 @@ protected ReconcileResult reconcileIndexAware(P primary, int i, Context

co "Dependent {} is read-only, implement Creator and/or Updater interfaces to modify it", getClass().getSimpleName()); } - return ReconcileResult.noOperation(maybeActual.orElse(null)); + return ReconcileResult.noOperation(resource); } - private R desiredIndexAware(P primary, int i, Context

context) { - return bulk ? desired(primary, i, context) : desired(primary, context); + private R desiredIndexAware(P primary, String key, Context

context) { + return bulk ? bulkDependentResource.desired(primary, key, context) + : desired(primary, context); } + @Override public Optional getSecondaryResource(P primary, Context

context) { return resourceDiscriminator == null ? context.getSecondaryResource(resourceType()) : resourceDiscriminator.distinguish(resourceType(), primary, context); @@ -172,13 +173,10 @@ protected R desired(P primary, Context

context) { "desired method must be implemented if this DependentResource can be created and/or updated"); } - protected R desired(P primary, int index, Context

context) { - throw new IllegalStateException("Must be implemented for bulk DependentResource creation"); - } - public void delete(P primary, Context

context) { if (bulk) { - deleteBulkResourcesIfRequired(0, primary, context); + var actualResources = bulkDependentResource.getSecondaryResources(primary, context); + deleteBulkResourcesIfRequired(Collections.emptySet(), actualResources, primary, context); } else { handleDelete(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 64a174e201..1c7189293b 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,6 +1,7 @@ package io.javaoperatorsdk.operator.processing.dependent; -import java.util.Optional; +import java.util.Map; +import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -13,24 +14,27 @@ * {@link Creator} and {@link Deleter} interfaces out of the box. A concrete dependent resource can * implement additionally also {@link Updater}. */ -public interface BulkDependentResource extends Creator, Deleter

{ +public interface BulkDependentResource + extends Creator, Deleter

{ /** * @return number of resources to create */ - int count(P primary, Context

context); + Set targetKeys(P primary, Context

context); - R desired(P primary, int index, Context

context); + Map getSecondaryResources(P primary, Context

context); + + R desired(P primary, String key, Context

context); /** * Used to delete resource if the desired count is lower than the actual count of a resource. * * @param primary resource * @param resource actual resource from the cache for the index - * @param i index of the resource + * @param key key of the resource * @param context actual context */ - void deleteBulkResourceWithIndex(P primary, R resource, int i, Context

context); + void deleteBulkResource(P primary, R resource, String key, Context

context); /** * Determines whether the specified secondary resource matches the desired state with target index @@ -39,14 +43,13 @@ public interface BulkDependentResource extends Creator * * @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 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, P primary, int index, Context

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

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

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 1aaed6f12e..97ad7fa226 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,9 +139,9 @@ public Result match(R actualResource, P primary, Context

context) { return matcher.match(actualResource, primary, context); } - public Result match(R actualResource, P primary, int index, Context

context) { - final var desired = desired(primary, index, context); - return GenericKubernetesResourceMatcher.match(desired, actualResource, false); + public Result match(R actualResource, P primary, String key, Context

context) { + final var desired = bulkDependentResource.desired(primary, key, context); + return GenericKubernetesResourceMatcher.match((R) desired, actualResource, false); } protected void handleDelete(P primary, Context

context) { @@ -149,7 +149,8 @@ protected void handleDelete(P primary, Context

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

context) { + + public void deleteBulkResource(P primary, R resource, String key, Context

context) { client.resource(resource).delete(); } @@ -229,11 +230,6 @@ protected R desired(P primary, Context

context) { return super.desired(primary, context); } - @Override - protected R desired(P primary, int index, Context

context) { - return super.desired(primary, index, context); - } - private void prepareEventFiltering(R desired, ResourceID resourceID) { ((InformerEventSource) eventSource().orElseThrow()) .prepareForCreateOrUpdateEventFiltering(resourceID, desired); 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 d8f85bb10a..e41b38c97e 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 @@ -1,8 +1,6 @@ package io.javaoperatorsdk.operator.sample.bulkdependent; -import java.util.Map; -import java.util.Optional; -import java.util.stream.Collectors; +import java.util.*; import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; @@ -27,43 +25,49 @@ public class ConfigMapDeleterBulkDependentResource public static final String LABEL_KEY = "bulk"; public static final String LABEL_VALUE = "true"; public static final String ADDITIONAL_DATA_KEY = "additionalData"; + public static final String INDEX_DELIMITER = "-"; public ConfigMapDeleterBulkDependentResource() { super(ConfigMap.class); } @Override - public ConfigMap desired(BulkDependentTestCustomResource primary, - int index, Context context) { + public Set targetKeys(BulkDependentTestCustomResource primary, + Context context) { + var number = primary.getSpec().getNumberOfResources(); + Set res = new HashSet<>(); + for (int i = 0; i < number; i++) { + res.add(Integer.toString(i)); + } + return res; + } + + @Override + public ConfigMap desired(BulkDependentTestCustomResource primary, String key, + Context context) { ConfigMap configMap = new ConfigMap(); configMap.setMetadata(new ObjectMetaBuilder() - .withName(primary.getMetadata().getName() + "-" + index) + .withName(primary.getMetadata().getName() + INDEX_DELIMITER + key) .withNamespace(primary.getMetadata().getNamespace()) .withLabels(Map.of(LABEL_KEY, LABEL_VALUE)) .build()); configMap.setData( - Map.of("number", "" + index, ADDITIONAL_DATA_KEY, primary.getSpec().getAdditionalData())); + Map.of("number", "" + key, ADDITIONAL_DATA_KEY, primary.getSpec().getAdditionalData())); return configMap; } @Override - public int count(BulkDependentTestCustomResource primary, + public Map getSecondaryResources(BulkDependentTestCustomResource primary, Context context) { - return primary.getSpec().getNumberOfResources(); - } - - @Override - 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)); - } + var configMaps = context.getSecondaryResources(ConfigMap.class); + Map result = new HashMap<>(configMaps.size()); + configMaps.forEach(cm -> { + String name = cm.getMetadata().getName(); + if (name.startsWith(primary.getMetadata().getName())) { + String key = name.substring(name.lastIndexOf(INDEX_DELIMITER) + 1); + result.put(key, cm); + } + }); + return result; } } 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 ed486f6f34..79b861d060 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,10 +1,6 @@ package io.javaoperatorsdk.operator.sample.bulkdependent.external; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.stream.Collectors; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -41,66 +37,72 @@ public Map> fetchResources() { } @Override - public void delete(BulkDependentTestCustomResource primary, + public ExternalResource create(ExternalResource desired, BulkDependentTestCustomResource primary, Context context) { - deleteBulkResourcesIfRequired(0, primary, context); + return externalServiceMock.create(desired); } @Override - public int count(BulkDependentTestCustomResource primary, - Context context) { - return primary.getSpec().getNumberOfResources(); + public ExternalResource update(ExternalResource actual, ExternalResource desired, + BulkDependentTestCustomResource primary, Context context) { + return externalServiceMock.update(desired); + } + + private static String toExternalResourceId(BulkDependentTestCustomResource primary, String i) { + return primary.getMetadata().getName() + EXTERNAL_RESOURCE_NAME_DELIMITER + + primary.getMetadata().getNamespace() + + EXTERNAL_RESOURCE_NAME_DELIMITER + i; + } + + private ResourceID toResourceID(ExternalResource externalResource) { + var parts = externalResource.getId().split(EXTERNAL_RESOURCE_NAME_DELIMITER); + return new ResourceID(parts[0], parts[1]); } @Override - public void deleteBulkResourceWithIndex(BulkDependentTestCustomResource primary, - ExternalResource resource, int i, Context context) { - externalServiceMock.delete(resource.getId()); + public Set targetKeys(BulkDependentTestCustomResource primary, + Context context) { + var number = primary.getSpec().getNumberOfResources(); + Set res = new HashSet<>(); + for (int i = 0; i < number; i++) { + res.add(Integer.toString(i)); + } + return res; } @Override - public ExternalResource desired(BulkDependentTestCustomResource primary, int index, + public Map getSecondaryResources( + BulkDependentTestCustomResource primary, Context context) { - return new ExternalResource(toExternalResourceId(primary, index), - primary.getSpec().getAdditionalData()); + return context.getSecondaryResources(resourceType()).stream() + .filter(r -> r.getId() + .startsWith(primary.getMetadata().getName() + EXTERNAL_RESOURCE_NAME_DELIMITER + + primary.getMetadata().getNamespace() + + EXTERNAL_RESOURCE_NAME_DELIMITER)) + .collect(Collectors.toMap( + r -> r.getId().substring(r.getId().lastIndexOf(EXTERNAL_RESOURCE_NAME_DELIMITER) + 1), + r -> r)); } @Override - public ExternalResource create(ExternalResource desired, BulkDependentTestCustomResource primary, + public ExternalResource desired(BulkDependentTestCustomResource primary, String key, Context context) { - return externalServiceMock.create(desired); + return new ExternalResource(toExternalResourceId(primary, key), + primary.getSpec().getAdditionalData()); } @Override - public ExternalResource update(ExternalResource actual, ExternalResource desired, - BulkDependentTestCustomResource primary, Context context) { - return externalServiceMock.update(desired); + public void deleteBulkResource(BulkDependentTestCustomResource primary, ExternalResource resource, + String key, + Context context) { + externalServiceMock.delete(resource.getId()); } @Override public Matcher.Result match(ExternalResource actualResource, - BulkDependentTestCustomResource primary, - int index, Context context) { + BulkDependentTestCustomResource primary, String index, + Context context) { var desired = desired(primary, index, context); return Matcher.Result.computed(desired.equals(actualResource), desired); } - - private static String toExternalResourceId(BulkDependentTestCustomResource primary, int i) { - return primary.getMetadata().getName() + EXTERNAL_RESOURCE_NAME_DELIMITER + - primary.getMetadata().getNamespace() + - EXTERNAL_RESOURCE_NAME_DELIMITER + i; - } - - private ResourceID toResourceID(ExternalResource externalResource) { - var parts = externalResource.getId().split(EXTERNAL_RESOURCE_NAME_DELIMITER); - return new ResourceID(parts[0], parts[1]); - } - - @Override - 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(); - } }