From b156a565a680dc86a506be9c5649852002da6131 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 27 Jul 2022 13:26:40 +0200 Subject: [PATCH 01/48] feat: improvements on caching and dependent resources --- .../operator/processing/dependent/AbstractDependentResource.java | 1 + .../dependent/kubernetes/KubernetesDependentResource.java | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) 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 1abfb3df4b..cb9dda80bb 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 @@ -32,6 +32,7 @@ public AbstractDependentResource() { updater = updatable ? (Updater) this : null; } + @SuppressWarnings("unchecked") @Override public ReconcileResult reconcile(P primary, Context

context) { Optional maybeActual = getSecondaryResource(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 328a061e6b..64621e1cf3 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 @@ -71,7 +71,6 @@ private void configureWith(String labelSelector, Set namespaces, .build(); configureWith(new InformerEventSource<>(ic, context)); - } @SuppressWarnings("unchecked") From e20f185acd27d91857fbf66f191406437abdefdc Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 27 Jul 2022 14:26:13 +0200 Subject: [PATCH 02/48] wip --- .../api/reconciler/DefaultContext.java | 7 +++++++ .../reconciler/ResourceListDiscriminator.java | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index cb7f4ae63b..5c71f62d2b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -48,6 +48,13 @@ public Optional getSecondaryResource(Class expectedType, String eventS .getSecondaryResource(primaryResource); } + // todo implement + @Override + public Optional getSecondaryResource(Class expectedType, + ResourceDiscriminator discriminator) { + return Optional.empty(); + } + @Override public Optional getSecondaryResource(Class expectedType, ResourceDiscriminator discriminator) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java new file mode 100644 index 0000000000..da6f173268 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java @@ -0,0 +1,19 @@ +package io.javaoperatorsdk.operator.api.reconciler; + +import java.util.Optional; +import java.util.Set; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; + +public abstract class ResourceListDiscriminator + implements ResourceDiscriminator { + @Override + public Optional distinguish(Class resource, Context

context, + EventSourceRetriever

eventSourceManager) { + var resources = context.getSecondaryResources(resource); + return distinguish(resources); + } + + abstract Optional distinguish(Set resourceList); +} From 9103864d0cc90c50dcdfec2ca0baccb5d6333c23 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 28 Jul 2022 13:11:18 +0200 Subject: [PATCH 03/48] kubernetes dependent resource configuration --- .../AnnotationControllerConfiguration.java | 1 - .../reconciler/VoidResourceDiscriminator.java | 16 ++++++++++++++++ .../KubernetesDependentResourceConfig.java | 1 - 3 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index dbd09a32cc..1ea2923584 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -24,7 +24,6 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; -import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java new file mode 100644 index 0000000000..0caec9361d --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java @@ -0,0 +1,16 @@ +package io.javaoperatorsdk.operator.api.reconciler; + +import java.util.Optional; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; + +public class VoidResourceDiscriminator + implements ResourceDiscriminator { + + @Override + public Optional distinguish(Class resource, Context

context, + EventSourceRetriever

eventSourceManager) { + throw new UnsupportedOperationException(); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java index e2a2c0f684..b9f16d61c9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java @@ -75,7 +75,6 @@ public OnAddFilter onAddFilter() { return onAddFilter; } - public OnUpdateFilter onUpdateFilter() { return onUpdateFilter; } From 07418468e6325ef5f9545f47e38a68a30934e699 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 28 Jul 2022 14:58:09 +0200 Subject: [PATCH 04/48] IT fix --- .../operator/api/reconciler/DefaultContext.java | 4 ++-- .../operator/api/reconciler/ResourceListDiscriminator.java | 7 ++++--- .../operator/api/reconciler/VoidResourceDiscriminator.java | 2 +- .../processing/dependent/AbstractDependentResource.java | 1 + 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index 5c71f62d2b..80ee9f3a50 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -48,11 +48,11 @@ public Optional getSecondaryResource(Class expectedType, String eventS .getSecondaryResource(primaryResource); } - // todo implement @Override public Optional getSecondaryResource(Class expectedType, ResourceDiscriminator discriminator) { - return Optional.empty(); + return discriminator.distinguish(expectedType, primaryResource, this, + controller.getEventSourceManager()); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java index da6f173268..fbb86dbd04 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java @@ -6,14 +6,15 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; +// todo this requires rather a matcher (name+namespace) as input public abstract class ResourceListDiscriminator implements ResourceDiscriminator { @Override - public Optional distinguish(Class resource, Context

context, + public Optional distinguish(Class resource, P primary, Context

context, EventSourceRetriever

eventSourceManager) { var resources = context.getSecondaryResources(resource); - return distinguish(resources); + return distinguish(primary, resources); } - abstract Optional distinguish(Set resourceList); + protected abstract Optional distinguish(P primary, Set resourceList); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java index 0caec9361d..3a627df109 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java @@ -9,7 +9,7 @@ public class VoidResourceDiscriminator implements ResourceDiscriminator { @Override - public Optional distinguish(Class resource, Context

context, + public Optional distinguish(Class resource, P primary, Context

context, EventSourceRetriever

eventSourceManager) { throw new UnsupportedOperationException(); } 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 cb9dda80bb..852fb23b03 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 @@ -24,6 +24,7 @@ public abstract class AbstractDependentResource protected Creator creator; protected Updater updater; + // todo discuss, rather implement this as interface? private ResourceDiscriminator resourceDiscriminator; @SuppressWarnings("unchecked") From 2758551136db20f1f574c4ed99dfea79e2ae9f71 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 29 Jul 2022 09:31:48 +0200 Subject: [PATCH 05/48] fixed ITs --- .../reconciler/ResourceListDiscriminator.java | 20 ------------------- .../KubernetesDependentResourceConfig.java | 6 ++++++ .../MultipleDependentResourceReconciler.java | 1 - 3 files changed, 6 insertions(+), 21 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java deleted file mode 100644 index fbb86dbd04..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceListDiscriminator.java +++ /dev/null @@ -1,20 +0,0 @@ -package io.javaoperatorsdk.operator.api.reconciler; - -import java.util.Optional; -import java.util.Set; - -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; - -// todo this requires rather a matcher (name+namespace) as input -public abstract class ResourceListDiscriminator - implements ResourceDiscriminator { - @Override - public Optional distinguish(Class resource, P primary, Context

context, - EventSourceRetriever

eventSourceManager) { - var resources = context.getSecondaryResources(resource); - return distinguish(primary, resources); - } - - protected abstract Optional distinguish(P primary, Set resourceList); -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java index b9f16d61c9..c7674dd1a7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java @@ -91,4 +91,10 @@ public GenericFilter genericFilter() { public ResourceDiscriminator getResourceDiscriminator() { return resourceDiscriminator; } + + public

KubernetesDependentResourceConfig setResourceDiscriminator( + ResourceDiscriminator resourceDiscriminator) { + this.resourceDiscriminator = resourceDiscriminator; + return this; + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multipledependentresource/MultipleDependentResourceReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multipledependentresource/MultipleDependentResourceReconciler.java index 49f5ee64c1..0994e6b9b0 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multipledependentresource/MultipleDependentResourceReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/multipledependentresource/MultipleDependentResourceReconciler.java @@ -29,7 +29,6 @@ public class MultipleDependentResourceReconciler public MultipleDependentResourceReconciler() { firstDependentResourceConfigMap = new MultipleDependentResourceConfigMap(FIRST_CONFIG_MAP_ID); - secondDependentResourceConfigMap = new MultipleDependentResourceConfigMap(SECOND_CONFIG_MAP_ID); firstDependentResourceConfigMap From 50bd91670d892fb1050a8e1e2903ea81c5346d45 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 29 Jul 2022 13:48:16 +0200 Subject: [PATCH 06/48] index based discriminator --- .../api/reconciler/ResourceDiscriminator.java | 1 - .../reconciler/VoidResourceDiscriminator.java | 2 +- .../kubernetes/KubernetesDependentResource.java | 16 +++++++++------- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java index 072e7d8078..eb947fa440 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java @@ -7,5 +7,4 @@ public interface ResourceDiscriminator { Optional distinguish(Class resource, P primary, Context

context); - } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java index 3a627df109..a02c0c6a95 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java @@ -10,7 +10,7 @@ public class VoidResourceDiscriminator @Override public Optional distinguish(Class resource, P primary, Context

context, - EventSourceRetriever

eventSourceManager) { + EventSourceRetriever

eventSourceRetriever) { throw new UnsupportedOperationException(); } } 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 64621e1cf3..03a5cdb40b 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 @@ -64,13 +64,15 @@ private void configureWith(String labelSelector, Set namespaces, namespaces = context.getControllerConfiguration().getNamespaces(); } - var ic = InformerConfiguration.from(resourceType()) - .withLabelSelector(labelSelector) - .withSecondaryToPrimaryMapper(getSecondaryToPrimaryMapper()) - .withNamespaces(namespaces, inheritNamespacesOnChange) - .build(); - - configureWith(new InformerEventSource<>(ic, context)); + if (eventSource() == null) { + var ic = InformerConfiguration.from(resourceType()) + .withLabelSelector(labelSelector) + .withSecondaryToPrimaryMapper(getSecondaryToPrimaryMapper()) + .withNamespaces(namespaces, inheritNamespacesOnChange) + .build(); + + configureWith(new InformerEventSource<>(ic, context)); + } } @SuppressWarnings("unchecked") From 97ce295d7b1e374ac6cad6cdd49877d5c1fe8941 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 29 Jul 2022 14:13:03 +0200 Subject: [PATCH 07/48] IT fix --- .../kubernetes/KubernetesDependentResource.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) 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 03a5cdb40b..328a061e6b 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 @@ -64,15 +64,14 @@ private void configureWith(String labelSelector, Set namespaces, namespaces = context.getControllerConfiguration().getNamespaces(); } - if (eventSource() == null) { - var ic = InformerConfiguration.from(resourceType()) - .withLabelSelector(labelSelector) - .withSecondaryToPrimaryMapper(getSecondaryToPrimaryMapper()) - .withNamespaces(namespaces, inheritNamespacesOnChange) - .build(); - - configureWith(new InformerEventSource<>(ic, context)); - } + var ic = InformerConfiguration.from(resourceType()) + .withLabelSelector(labelSelector) + .withSecondaryToPrimaryMapper(getSecondaryToPrimaryMapper()) + .withNamespaces(namespaces, inheritNamespacesOnChange) + .build(); + + configureWith(new InformerEventSource<>(ic, context)); + } @SuppressWarnings("unchecked") From 0718875d6da414f6e9c5842ac9f19a2986010587 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 29 Jul 2022 16:03:01 +0200 Subject: [PATCH 08/48] wip --- .../operator/api/reconciler/ResourceDiscriminator.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java index eb947fa440..85fef3642b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java @@ -4,6 +4,8 @@ import io.fabric8.kubernetes.api.model.HasMetadata; +// todo is discriminator a good name? it not just discriminates but also reads from cache +// todo discuss a List version of this (for reconciler but also for batch processing?) public interface ResourceDiscriminator { Optional distinguish(Class resource, P primary, Context

context); From b71c7c9d87b6fd182cefdecf0d7074cb2805f118 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 26 Aug 2022 10:15:34 +0200 Subject: [PATCH 09/48] fixes from rebase from next --- .../operator/api/config/AnnotationControllerConfiguration.java | 1 + 1 file changed, 1 insertion(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 1ea2923584..dbd09a32cc 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -24,6 +24,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; +import io.javaoperatorsdk.operator.processing.dependent.workflow.Condition; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilters; From 6337e44a54cdf2c3198a95f725b473d0e88afd6d Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 5 Sep 2022 15:04:49 +0200 Subject: [PATCH 10/48] fix after rebase --- .../api/config/AnnotationControllerConfiguration.java | 5 +++-- .../dependent/workflow/WorkflowCleanupExecutor.java | 1 - 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index dbd09a32cc..b40631c603 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -45,13 +45,14 @@ public class AnnotationControllerConfiguration

private static final String KUBE_DEPENDENT_NAME = KubernetesDependent.class.getSimpleName(); protected final Reconciler

reconciler; - private final ControllerConfiguration annotation; + private final io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration annotation; private List specs; private Class

resourceClass; public AnnotationControllerConfiguration(Reconciler

reconciler) { this.reconciler = reconciler; - this.annotation = reconciler.getClass().getAnnotation(ControllerConfiguration.class); + this.annotation = reconciler.getClass() + .getAnnotation(io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration.class); if (annotation == null) { throw new OperatorException("Missing mandatory @" + CONTROLLER_CONFIG_ANNOTATION + " annotation for reconciler: " + reconciler); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java index 45541b91d6..ac08d2d874 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/WorkflowCleanupExecutor.java @@ -125,7 +125,6 @@ public void run() { } } - private synchronized void handleDependentCleaned( DependentResourceNode dependentResourceNode) { var dependOns = dependentResourceNode.getDependsOn(); From 01c33ec8b3f68e85d4f2fd1564204fabfbda89d8 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 6 Sep 2022 16:46:08 +0200 Subject: [PATCH 11/48] event source provider to context --- .../operator/api/reconciler/DefaultContext.java | 3 +-- .../operator/api/reconciler/VoidResourceDiscriminator.java | 4 +--- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index 80ee9f3a50..9b6099238e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -51,8 +51,7 @@ public Optional getSecondaryResource(Class expectedType, String eventS @Override public Optional getSecondaryResource(Class expectedType, ResourceDiscriminator discriminator) { - return discriminator.distinguish(expectedType, primaryResource, this, - controller.getEventSourceManager()); + return discriminator.distinguish(expectedType, primaryResource, this); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java index a02c0c6a95..0a5fd1f965 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java @@ -3,14 +3,12 @@ import java.util.Optional; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; public class VoidResourceDiscriminator implements ResourceDiscriminator { @Override - public Optional distinguish(Class resource, P primary, Context

context, - EventSourceRetriever

eventSourceRetriever) { + public Optional distinguish(Class resource, P primary, Context

context) { throw new UnsupportedOperationException(); } } From b701eb6f5d4417875f830280bfacb93c796ad90d Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 16 Sep 2022 09:48:32 +0200 Subject: [PATCH 12/48] todo fixes --- .../operator/api/reconciler/ResourceDiscriminator.java | 2 -- .../processing/dependent/AbstractDependentResource.java | 1 - 2 files changed, 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java index 85fef3642b..eb947fa440 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceDiscriminator.java @@ -4,8 +4,6 @@ import io.fabric8.kubernetes.api.model.HasMetadata; -// todo is discriminator a good name? it not just discriminates but also reads from cache -// todo discuss a List version of this (for reconciler but also for batch processing?) public interface ResourceDiscriminator { Optional distinguish(Class resource, P primary, Context

context); 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 852fb23b03..cb9dda80bb 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 @@ -24,7 +24,6 @@ public abstract class AbstractDependentResource protected Creator creator; protected Updater updater; - // todo discuss, rather implement this as interface? private ResourceDiscriminator resourceDiscriminator; @SuppressWarnings("unchecked") From 562f7d20a7c264678f8ab5a8f9c5eeeb42ca1091 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 27 Sep 2022 09:22:15 +0200 Subject: [PATCH 13/48] remove void discriminator --- .../api/reconciler/VoidResourceDiscriminator.java | 14 -------------- 1 file changed, 14 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java deleted file mode 100644 index 0a5fd1f965..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/VoidResourceDiscriminator.java +++ /dev/null @@ -1,14 +0,0 @@ -package io.javaoperatorsdk.operator.api.reconciler; - -import java.util.Optional; - -import io.fabric8.kubernetes.api.model.HasMetadata; - -public class VoidResourceDiscriminator - implements ResourceDiscriminator { - - @Override - public Optional distinguish(Class resource, P primary, Context

context) { - throw new UnsupportedOperationException(); - } -} From b414d580a2bddba7ff048f7b155052470e94359e Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 8 Sep 2022 17:29:42 +0200 Subject: [PATCH 14/48] fix: bulk creation of dependent resource directly in abstract resource --- .../dependent/AbstractDependentResource.java | 95 +++++++++++++++++-- .../BulkResourceDiscriminatorFactory.java | 10 ++ .../processing/dependent/Updater.java | 9 ++ 3 files changed, 104 insertions(+), 10 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkResourceDiscriminatorFactory.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 cb9dda80bb..a0bccdf891 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,5 +1,7 @@ package io.javaoperatorsdk.operator.processing.dependent; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import org.slf4j.Logger; @@ -24,7 +26,9 @@ public abstract class AbstractDependentResource protected Creator creator; protected Updater updater; - private ResourceDiscriminator resourceDiscriminator; + protected List> resourceDiscriminator = new ArrayList<>(1); + // used just for bulk creation + protected BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory; @SuppressWarnings("unchecked") public AbstractDependentResource() { @@ -32,14 +36,44 @@ public AbstractDependentResource() { updater = updatable ? (Updater) this : null; } - @SuppressWarnings("unchecked") @Override public ReconcileResult reconcile(P primary, Context

context) { - Optional maybeActual = getSecondaryResource(primary, context); + var count = count(primary, context); + if (isBulkResourceCreation(primary, context)) { + initDiscriminators(count); + } + for (int i = 0; i < count; i++) { + reconcileWithIndex(primary, i, context); + } + // todo result + return null; + } + + private void initDiscriminators(int count) { + if (resourceDiscriminator.size() == count) { + return; + } + if (resourceDiscriminator.size() < count) { + for (int i = resourceDiscriminator.size() - 1; i < count; i++) { + resourceDiscriminator.add(bulkResourceDiscriminatorFactory.createResourceDiscriminator(i)); + } + } + if (resourceDiscriminator.size() < count) { + for (int i = resourceDiscriminator.size() - 1; i < count; i++) { + resourceDiscriminator.add(bulkResourceDiscriminatorFactory.createResourceDiscriminator(i)); + } + } + if (resourceDiscriminator.size() > count) { + resourceDiscriminator.subList(count, resourceDiscriminator.size()).clear(); + } + } + + protected ReconcileResult reconcileWithIndex(P primary, int i, Context

context) { + Optional maybeActual = getSecondaryResource(primary, i, context); if (creatable || updatable) { if (maybeActual.isEmpty()) { if (creatable) { - var desired = desired(primary, context); + var desired = desired(primary, i, context); throwIfNull(desired, primary, "Desired"); logForOperation("Creating", primary, desired); var createdResource = handleCreate(desired, primary, context); @@ -48,7 +82,8 @@ public ReconcileResult reconcile(P primary, Context

context) { } else { final var actual = maybeActual.get(); if (updatable) { - final var match = updater.match(actual, primary, context); + // todo simplify matcher? + final var match = updater.match(actual, primary, i, context); if (!match.matched()) { final var desired = match.computedDesired().orElse(desired(primary, context)); throwIfNull(desired, primary, "Desired"); @@ -69,8 +104,20 @@ public ReconcileResult reconcile(P primary, Context

context) { } public Optional getSecondaryResource(P primary, Context

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

context) { + if (index > 0 && resourceDiscriminator.isEmpty()) { + throw new IllegalStateException( + "Handling resources in bulk bot no resource discriminators set."); + } + if (!isBulkResourceCreation(primary, context)) { + return getSecondaryResource(primary, context); + } + + return context.getSecondaryResource(resourceType(), resourceDiscriminator.get(index)); } private void throwIfNull(R desired, P primary, String descriptor) { @@ -130,12 +177,40 @@ protected R desired(P primary, Context

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

context) { + if (!isBulkResourceCreation(primary, context)) { + return desired(primary, context); + } else { + throw new IllegalStateException( + "desired() with index method must be implemented for bulk DependentResource creation"); + } + } + + public AbstractDependentResource setResourceDiscriminator( ResourceDiscriminator resourceDiscriminator) { - this.resourceDiscriminator = resourceDiscriminator; + this.resourceDiscriminator.add(resourceDiscriminator); + return this; } public ResourceDiscriminator getResourceDiscriminator() { - return resourceDiscriminator; + return resourceDiscriminator.get(0); + } + + protected int count(P primary, Context

context) { + return 1; + } + + protected boolean isBulkResourceCreation(P primary, Context

context) { + return false; + } + + public BulkResourceDiscriminatorFactory getBulkResourceDiscriminatorFactory() { + return bulkResourceDiscriminatorFactory; + } + + public AbstractDependentResource setBulkResourceDiscriminatorFactory( + BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory) { + this.bulkResourceDiscriminatorFactory = bulkResourceDiscriminatorFactory; + return this; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkResourceDiscriminatorFactory.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkResourceDiscriminatorFactory.java new file mode 100644 index 0000000000..8b9b6e968d --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkResourceDiscriminatorFactory.java @@ -0,0 +1,10 @@ +package io.javaoperatorsdk.operator.processing.dependent; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; + +public interface BulkResourceDiscriminatorFactory { + + ResourceDiscriminator createResourceDiscriminator(int index); + +} 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 828f9ad785..19480f253b 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,4 +8,13 @@ public interface Updater { R update(R actual, R desired, P primary, Context

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

context); + + // todo change to simple desired matching? + default Result match(R actualResource, P primary, int index, Context

context) { + if (index == 0) { + return match(actualResource, primary, context); + } else { + throw new IllegalStateException("Implement this"); + } + } } From 3b34d1e40aaf51387792056d71afa1b3a96af0a6 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 9 Sep 2022 08:43:44 +0200 Subject: [PATCH 15/48] wip --- .../operator/config/runtime/StandaloneBulkDependentIT.java | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/StandaloneBulkDependentIT.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/StandaloneBulkDependentIT.java new file mode 100644 index 0000000000..ef27c9c2e1 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/StandaloneBulkDependentIT.java @@ -0,0 +1,4 @@ +package io.javaoperatorsdk.operator.config.runtime; + +public class StandaloneBulkDependentIT { +} From c7f3671dd0343ab845862a895f6a7fe21728573b Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 9 Sep 2022 11:13:17 +0200 Subject: [PATCH 16/48] wip --- .../javaoperatorsdk/operator/StandaloneBulkDependentIT.java | 4 ++++ .../operator/config/runtime/StandaloneBulkDependentIT.java | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/StandaloneBulkDependentIT.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java new file mode 100644 index 0000000000..0ee8ef49e2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -0,0 +1,4 @@ +package io.javaoperatorsdk.operator; + +public class StandaloneBulkDependentIT { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/StandaloneBulkDependentIT.java deleted file mode 100644 index ef27c9c2e1..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/runtime/StandaloneBulkDependentIT.java +++ /dev/null @@ -1,4 +0,0 @@ -package io.javaoperatorsdk.operator.config.runtime; - -public class StandaloneBulkDependentIT { -} From 759b2e15d008ebd86d52ea529ee11660f8612e0b Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 9 Sep 2022 11:18:54 +0200 Subject: [PATCH 17/48] wip to start IT --- .../io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index 0ee8ef49e2..56ac867aa8 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -1,4 +1,5 @@ package io.javaoperatorsdk.operator; public class StandaloneBulkDependentIT { + } From 1b7e71422a2189ebe62d82bad0d063620b409ea8 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 9 Sep 2022 16:09:49 +0200 Subject: [PATCH 18/48] fixes, progress --- .../reconciler/dependent/ReconcileResult.java | 39 +++++++---- .../dependent/AbstractDependentResource.java | 18 +++-- .../processing/dependent/Updater.java | 6 +- .../KubernetesDependentResource.java | 6 +- .../operator/StandaloneBulkDependentIT.java | 47 ++++++++++++- .../ConfigMapBulkDependentResource.java | 69 +++++++++++++++++++ .../StandaloneBulkDependentReconciler.java | 58 ++++++++++++++++ ...daloneBulkDependentTestCustomResource.java | 15 ++++ .../StandaloneBulkDependentTestSpec.java | 15 ++++ 9 files changed, 246 insertions(+), 27 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestSpec.java 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 c83da1c8ea..65b6e8adfd 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 @@ -1,14 +1,16 @@ package io.javaoperatorsdk.operator.api.reconciler.dependent; -import java.util.Optional; +import java.util.*; +import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.processing.event.ResourceID; public class ReconcileResult { - private final R resource; - private final Operation operation; + private Map resourceOperations = new HashMap<>(1); + + public ReconcileResult() {} public static ReconcileResult resourceCreated(T resource) { return new ReconcileResult<>(resource, Operation.CREATED); @@ -24,23 +26,34 @@ public static ReconcileResult noOperation(T resource) { @Override public String toString() { - return getResource() - .map(r -> r instanceof HasMetadata ? ResourceID.fromResource((HasMetadata) r) : r) - .orElse("no resource") - + " -> " + operation; + return resourceOperations.entrySet().stream().collect(Collectors.toMap( + e -> e instanceof HasMetadata ? ResourceID.fromResource((HasMetadata) e) : e, + Map.Entry::getValue)) + .toString(); } private ReconcileResult(R resource, Operation operation) { - this.resource = resource; - this.operation = operation; + resourceOperations.put(resource, operation); + } + + public Optional getSingleResource() { + return resourceOperations.entrySet().stream().findFirst().map(Map.Entry::getKey); } - public Optional getResource() { - return Optional.ofNullable(resource); + public Operation getSingleOperation() { + return resourceOperations.entrySet().stream().findFirst().map(Map.Entry::getValue) + .orElseThrow(); } - public Operation getOperation() { - return operation; + public Map getResourceOperations() { + return resourceOperations; + } + + public void addReconcileResult(ReconcileResult result) { + result.getSingleResource().ifPresent(r -> { + resourceOperations.put(r, result.getSingleOperation()); + }); + } public enum Operation { 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 a0bccdf891..2f90e6e5e2 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,11 +42,12 @@ public ReconcileResult reconcile(P primary, Context

context) { if (isBulkResourceCreation(primary, context)) { initDiscriminators(count); } + ReconcileResult result = new ReconcileResult<>(); for (int i = 0; i < count; i++) { - reconcileWithIndex(primary, i, context); + var res = reconcileWithIndex(primary, i, context); + result.addReconcileResult(res); } - // todo result - return null; + return result; } private void initDiscriminators(int count) { @@ -83,7 +84,12 @@ protected ReconcileResult reconcileWithIndex(P primary, int i, Context

con final var actual = maybeActual.get(); if (updatable) { // todo simplify matcher? - final var match = updater.match(actual, primary, i, context); + final Matcher.Result match; + if (isBulkResourceCreation(primary, context)) { + match = updater.match(actual, primary, i, context); + } else { + match = updater.match(actual, primary, context); + } if (!match.matched()) { final var desired = match.computedDesired().orElse(desired(primary, context)); throwIfNull(desired, primary, "Desired"); @@ -188,7 +194,9 @@ protected R desired(P primary, int index, Context

context) { public AbstractDependentResource setResourceDiscriminator( ResourceDiscriminator resourceDiscriminator) { - this.resourceDiscriminator.add(resourceDiscriminator); + if (resourceDiscriminator != null) { + this.resourceDiscriminator.add(resourceDiscriminator); + } return this; } 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 19480f253b..dd36be0492 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 @@ -11,10 +11,6 @@ public interface Updater { // todo change to simple desired matching? default Result match(R actualResource, P primary, int index, Context

context) { - if (index == 0) { - return match(actualResource, primary, context); - } else { - throw new IllegalStateException("Implement this"); - } + throw new IllegalStateException("Implement this for bulk matching"); } } 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 328a061e6b..5b450603d9 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 @@ -163,8 +163,10 @@ protected InformerEventSource createEventSource(EventSourceContext

cont onUpdateFilter = kubernetesDependentResourceConfig.onUpdateFilter(); onDeleteFilter = kubernetesDependentResourceConfig.onDeleteFilter(); genericFilter = kubernetesDependentResourceConfig.genericFilter(); - setResourceDiscriminator(kubernetesDependentResourceConfig.getResourceDiscriminator()); - + var discriminator = kubernetesDependentResourceConfig.getResourceDiscriminator(); + if (discriminator != null) { + setResourceDiscriminator(discriminator); + } configureWith(kubernetesDependentResourceConfig.labelSelector(), kubernetesDependentResourceConfig.namespaces(), !kubernetesDependentResourceConfig.wereNamespacesConfigured(), context); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index 56ac867aa8..006dc9dd53 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -1,5 +1,48 @@ package io.javaoperatorsdk.operator; -public class StandaloneBulkDependentIT { - +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.standalonebulkdependent.StandaloneBulkDependentReconciler; +import io.javaoperatorsdk.operator.sample.standalonebulkdependent.StandaloneBulkDependentTestCustomResource; +import io.javaoperatorsdk.operator.sample.standalonebulkdependent.StandaloneBulkDependentTestSpec; + +import static io.javaoperatorsdk.operator.sample.standalonebulkdependent.ConfigMapBulkDependentResource.LABEL_KEY; +import static io.javaoperatorsdk.operator.sample.standalonebulkdependent.ConfigMapBulkDependentResource.LABEL_VALUE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class StandaloneBulkDependentIT { + + public static final String TEST_RESOURCE_NAME = "test"; + public static final int NUMBER_OF_CONFIG_MAPS = 3; + + @RegisterExtension + LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension.builder().withReconciler(new StandaloneBulkDependentReconciler()) + .build(); + + @Test + void managesBulkConfigMaps() { + operator.create(testResource()); + + await().untilAsserted(() -> { + var cms = operator.getKubernetesClient().configMaps().inNamespace(operator.getNamespace()) + .withLabel(LABEL_KEY, LABEL_VALUE) + .list().getItems(); + assertThat(cms).hasSize(NUMBER_OF_CONFIG_MAPS); + }); + } + + private StandaloneBulkDependentTestCustomResource testResource() { + StandaloneBulkDependentTestCustomResource cr = new StandaloneBulkDependentTestCustomResource(); + cr.setMetadata(new ObjectMeta()); + cr.getMetadata().setName(TEST_RESOURCE_NAME); + cr.setSpec(new StandaloneBulkDependentTestSpec()); + cr.getSpec().setNumberOfResources(NUMBER_OF_CONFIG_MAPS); + return cr; + } + } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java new file mode 100644 index 0000000000..3df6c7223b --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java @@ -0,0 +1,69 @@ +package io.javaoperatorsdk.operator.sample.standalonebulkdependent; + +import java.util.Map; +import java.util.Optional; +import java.util.stream.Collectors; + +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.processing.dependent.BulkResourceDiscriminatorFactory; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; + +public class ConfigMapBulkDependentResource + extends CRUDKubernetesDependentResource { + + public static final String LABEL_KEY = "bulk"; + public static final String LABEL_VALUE = "true"; + + public ConfigMapBulkDependentResource() { + super(ConfigMap.class); + setBulkResourceDiscriminatorFactory( + new BulkResourceDiscriminatorFactory() { + @Override + public ResourceDiscriminator createResourceDiscriminator( + 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)); + } + }; + } + }); + } + + @Override + protected ConfigMap desired(StandaloneBulkDependentTestCustomResource primary, + int index, Context context) { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new ObjectMetaBuilder() + .withName(primary.getMetadata().getName() + "-" + index) + .withNamespace(primary.getMetadata().getNamespace()) + .withLabels(Map.of(LABEL_KEY, LABEL_VALUE)) + .build()); + configMap.setData(Map.of("number", "" + index)); + return configMap; + } + + @Override + protected boolean isBulkResourceCreation(StandaloneBulkDependentTestCustomResource primary, + Context context) { + return true; + } + + @Override + protected int count(StandaloneBulkDependentTestCustomResource primary, + Context context) { + return primary.getSpec().getNumberOfResources(); + } + + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentReconciler.java new file mode 100644 index 0000000000..bf2b3cbf09 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentReconciler.java @@ -0,0 +1,58 @@ +package io.javaoperatorsdk.operator.sample.standalonebulkdependent; + +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.junit.KubernetesClientAware; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; + +@ControllerConfiguration +public class StandaloneBulkDependentReconciler + implements Reconciler, TestExecutionInfoProvider, + EventSourceInitializer, KubernetesClientAware { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + private ConfigMapBulkDependentResource dependent; + private KubernetesClient kubernetesClient; + + public StandaloneBulkDependentReconciler() { + dependent = new ConfigMapBulkDependentResource(); + } + + @Override + public UpdateControl reconcile( + StandaloneBulkDependentTestCustomResource resource, + Context context) { + numberOfExecutions.addAndGet(1); + + dependent.reconcile(resource, context); + + return UpdateControl.noUpdate(); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + + @Override + public Map prepareEventSources( + EventSourceContext context) { + return EventSourceInitializer + .nameEventSources(dependent.initEventSource(context)); + } + + @Override + public KubernetesClient getKubernetesClient() { + return kubernetesClient; + } + + @Override + public void setKubernetesClient(KubernetesClient kubernetesClient) { + this.kubernetesClient = kubernetesClient; + dependent.setKubernetesClient(kubernetesClient); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestCustomResource.java new file mode 100644 index 0000000000..4ae13f945b --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestCustomResource.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.standalonebulkdependent; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("sbd") +public class StandaloneBulkDependentTestCustomResource + extends CustomResource + implements Namespaced { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestSpec.java new file mode 100644 index 0000000000..521da411c3 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestSpec.java @@ -0,0 +1,15 @@ +package io.javaoperatorsdk.operator.sample.standalonebulkdependent; + +public class StandaloneBulkDependentTestSpec { + + private Integer numberOfResources; + + public Integer getNumberOfResources() { + return numberOfResources; + } + + public StandaloneBulkDependentTestSpec setNumberOfResources(Integer numberOfResources) { + this.numberOfResources = numberOfResources; + return this; + } +} From 953d2748ed1351f4f51e50380606ef816d303f2c Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 12 Sep 2022 10:54:20 +0200 Subject: [PATCH 19/48] wp --- .../dependent/AbstractDependentResource.java | 16 ++++++-- .../ConfigMapBulkDependentResource.java | 38 ++++++------------- 2 files changed, 24 insertions(+), 30 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 2f90e6e5e2..bbc816c3ab 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 @@ -38,7 +38,7 @@ public AbstractDependentResource() { @Override public ReconcileResult reconcile(P primary, Context

context) { - var count = count(primary, context); + var count = count(primary, context).orElse(1); if (isBulkResourceCreation(primary, context)) { initDiscriminators(count); } @@ -204,12 +204,20 @@ public ResourceDiscriminator getResourceDiscriminator() { return resourceDiscriminator.get(0); } - protected int count(P primary, Context

context) { - return 1; + /** + * @param primary resource + * @param context actual context + * @return empty optional if it's not a bulk resource management, number of instances otherwise. + */ + protected Optional count(P primary, Context

context) { + return Optional.empty(); } + /** + * Override in case the count() is a more heavy + */ protected boolean isBulkResourceCreation(P primary, Context

context) { - return false; + return count(primary, context).isPresent(); } public BulkResourceDiscriminatorFactory getBulkResourceDiscriminatorFactory() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java index 3df6c7223b..7d13f67b50 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java @@ -7,8 +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.processing.dependent.BulkResourceDiscriminatorFactory; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; public class ConfigMapBulkDependentResource @@ -20,22 +18,16 @@ public class ConfigMapBulkDependentResource public ConfigMapBulkDependentResource() { super(ConfigMap.class); setBulkResourceDiscriminatorFactory( - new BulkResourceDiscriminatorFactory() { - @Override - public ResourceDiscriminator createResourceDiscriminator( - 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)); - } - }; + index -> (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)); } }); } @@ -54,15 +46,9 @@ protected ConfigMap desired(StandaloneBulkDependentTestCustomResource primary, } @Override - protected boolean isBulkResourceCreation(StandaloneBulkDependentTestCustomResource primary, + protected Optional count(StandaloneBulkDependentTestCustomResource primary, Context context) { - return true; - } - - @Override - protected int count(StandaloneBulkDependentTestCustomResource primary, - Context context) { - return primary.getSpec().getNumberOfResources(); + return Optional.of(primary.getSpec().getNumberOfResources()); } From 98da2e1e36e85444b57935c223f2067cf3287b66 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 12 Sep 2022 12:58:39 +0200 Subject: [PATCH 20/48] matcher --- .../dependent/AbstractDependentResource.java | 3 +- .../dependent/DesiredEqualsMatcher.java | 6 + .../processing/dependent/Matcher.java | 15 +++ .../processing/dependent/Updater.java | 1 - .../GenericKubernetesResourceMatcher.java | 103 ++++++++++++------ .../KubernetesDependentResource.java | 5 + 6 files changed, 97 insertions(+), 36 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 bbc816c3ab..b7c5130de1 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 @@ -83,7 +83,6 @@ protected ReconcileResult reconcileWithIndex(P primary, int i, Context

con } else { final var actual = maybeActual.get(); if (updatable) { - // todo simplify matcher? final Matcher.Result match; if (isBulkResourceCreation(primary, context)) { match = updater.match(actual, primary, i, context); @@ -151,7 +150,7 @@ protected R handleCreate(R desired, P primary, Context

context) { } /** - * Allows sub-classes to perform additional processing (e.g. caching) on the created resource if + * Allows subclasses to perform additional processing (e.g. caching) on the created resource if * needed. * * @param primaryResourceId the {@link ResourceID} of the primary resource associated with the 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 459d7951d6..1d3b34a47b 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,4 +16,10 @@ 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 750fe89cbf..835f76ab3a 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,4 +95,19 @@ 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 dd36be0492..06b3cb52f6 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 @@ -9,7 +9,6 @@ public interface Updater { Result match(R actualResource, P primary, Context

context); - // todo change to simple desired matching? 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 e294b1c938..56b5bc7a59 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 @@ -24,17 +24,42 @@ private GenericKubernetesResourceMatcher(KubernetesDependentResource depen static Matcher matcherFor( Class resourceType, KubernetesDependentResource dependentResource) { if (Secret.class.isAssignableFrom(resourceType)) { - return (actual, primary, context) -> { - final var desired = dependentResource.desired(primary, context); - return Result.computed( - ResourceComparators.compareSecretData((Secret) desired, (Secret) actual), desired); + 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, context); + return Result.computed( + ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource), + desired); + } }; } else if (ConfigMap.class.isAssignableFrom(resourceType)) { - return (actual, primary, context) -> { - final var desired = dependentResource.desired(primary, context); - return Result.computed( - ResourceComparators.compareConfigMapData((ConfigMap) desired, (ConfigMap) actual), - desired); + 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); @@ -43,32 +68,18 @@ static Matcher matcherFor( @Override public Result match(R actualResource, P primary, Context

context) { - return match(dependentResource, actualResource, primary, context, false); + var desired = dependentResource.desired(primary, context); + return match(desired, actualResource, false); } - /** - * Determines whether the specified actual resource matches the desired state defined by the - * specified {@link KubernetesDependentResource} based on the observed state of the associated - * specified primary resource. - * - * @param dependentResource the {@link KubernetesDependentResource} implementation used to - * computed the desired state associated with the specified primary resource - * @param actualResource the observed dependent resource for which we want to determine whether it - * matches the desired state or not - * @param primary the primary resource from which we want to compute the desired state - * @param context the {@link Context} instance within which this method is called - * @param considerMetadata {@code true} to consider the metadata of the actual resource when - * determining if it matches the desired state, {@code false} if matching should occur only - * considering the spec of the resources - * @return a {@link io.javaoperatorsdk.operator.processing.dependent.Matcher.Result} object - * @param the type of resource we want to determine whether they match or not - * @param

the type of primary resources associated with the secondary resources we want to - * match - */ - public static Result match( - KubernetesDependentResource dependentResource, R actualResource, P primary, - Context

context, boolean considerMetadata) { - final var desired = dependentResource.desired(primary, context); + @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) { if (considerMetadata) { final var desiredMetadata = desired.getMetadata(); final var actualMetadata = actualResource.getMetadata(); @@ -95,4 +106,30 @@ public static Result match( } return Result.computed(true, desired); } + + /** + * Determines whether the specified actual resource matches the desired state defined by the + * specified {@link KubernetesDependentResource} based on the observed state of the associated + * specified primary resource. + * + * @param dependentResource the {@link KubernetesDependentResource} implementation used to + * computed the desired state associated with the specified primary resource + * @param actualResource the observed dependent resource for which we want to determine whether it + * matches the desired state or not + * @param primary the primary resource from which we want to compute the desired state + * @param context the {@link Context} instance within which this method is called + * @param considerMetadata {@code true} to consider the metadata of the actual resource when + * determining if it matches the desired state, {@code false} if matching should occur only + * considering the spec of the resources + * @return a {@link io.javaoperatorsdk.operator.processing.dependent.Matcher.Result} object + * @param the type of resource we want to determine whether they match or not + * @param

the type of primary resources associated with the secondary resources we want to + * match + */ + public static Result match( + KubernetesDependentResource dependentResource, R actualResource, P primary, + Context

context, boolean considerMetadata) { + final var desired = dependentResource.desired(primary, context); + return match(desired, actualResource, considerMetadata); + } } 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 5b450603d9..3a3ce558f2 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 @@ -217,6 +217,11 @@ 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) { eventSource().prepareForCreateOrUpdateEventFiltering(resourceID, desired); } From 40c3e47bbe514cfe559e4856a57457e2924916c3 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 12 Sep 2022 15:43:56 +0200 Subject: [PATCH 21/48] test passes --- .../dependent/AbstractDependentResource.java | 49 +++++++++++++------ .../KubernetesDependentResource.java | 9 ++++ .../operator/StandaloneBulkDependentIT.java | 20 +++++++- .../ConfigMapBulkDependentResource.java | 5 ++ 4 files changed, 68 insertions(+), 15 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 b7c5130de1..74b035c59c 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 @@ -40,27 +40,41 @@ public AbstractDependentResource() { public ReconcileResult reconcile(P primary, Context

context) { var count = count(primary, context).orElse(1); if (isBulkResourceCreation(primary, context)) { - initDiscriminators(count); + cleanupBulkResourcesIfRequired(count, resourceDiscriminator.size(), primary, context); + adjustDiscriminators(count); } ReconcileResult result = new ReconcileResult<>(); for (int i = 0; i < count; i++) { - var res = reconcileWithIndex(primary, i, context); + var res = reconcileIndexAware(primary, i, context); result.addReconcileResult(res); } return result; } - private void initDiscriminators(int count) { - if (resourceDiscriminator.size() == count) { + private void cleanupBulkResourcesIfRequired(int targetCount, int actualCount, P primary, + Context

context) { + if (targetCount >= actualCount) { return; } - if (resourceDiscriminator.size() < count) { - for (int i = resourceDiscriminator.size() - 1; i < count; i++) { - resourceDiscriminator.add(bulkResourceDiscriminatorFactory.createResourceDiscriminator(i)); - } + for (int i = targetCount; i < actualCount; i++) { + var resource = getSecondaryResourceIndexAware(primary, i, context); + var index = i; + resource.ifPresent(r -> { + deleteBulkResourceWithIndex(primary, r, index, context); + }); + } + } + + protected void deleteBulkResourceWithIndex(P primary, R resource, int i, Context

context) { + throw new IllegalStateException("Implement if handling bulk resources."); + } + + private void adjustDiscriminators(int count) { + if (resourceDiscriminator.size() == count) { + return; } if (resourceDiscriminator.size() < count) { - for (int i = resourceDiscriminator.size() - 1; i < count; i++) { + for (int i = resourceDiscriminator.size(); i < count; i++) { resourceDiscriminator.add(bulkResourceDiscriminatorFactory.createResourceDiscriminator(i)); } } @@ -69,12 +83,12 @@ private void initDiscriminators(int count) { } } - protected ReconcileResult reconcileWithIndex(P primary, int i, Context

context) { - Optional maybeActual = getSecondaryResource(primary, i, context); + protected ReconcileResult reconcileIndexAware(P primary, int i, Context

context) { + Optional maybeActual = getSecondaryResourceIndexAware(primary, i, context); if (creatable || updatable) { if (maybeActual.isEmpty()) { if (creatable) { - var desired = desired(primary, i, context); + var desired = desiredIndexAware(primary, i, context); throwIfNull(desired, primary, "Desired"); logForOperation("Creating", primary, desired); var createdResource = handleCreate(desired, primary, context); @@ -90,7 +104,8 @@ protected ReconcileResult reconcileWithIndex(P primary, int i, Context

con match = updater.match(actual, primary, context); } if (!match.matched()) { - final var desired = match.computedDesired().orElse(desired(primary, context)); + final var desired = + match.computedDesired().orElse(desiredIndexAware(primary, i, context)); throwIfNull(desired, primary, "Desired"); logForOperation("Updating", primary, desired); var updatedResource = handleUpdate(actual, desired, primary, context); @@ -108,12 +123,18 @@ protected ReconcileResult reconcileWithIndex(P primary, int i, Context

con return ReconcileResult.noOperation(maybeActual.orElse(null)); } + private R desiredIndexAware(P primary, int i, Context

context) { + return isBulkResourceCreation(primary, context) ? desired(primary, i, context) + : desired(primary, context); + } + + // todo check public Optional getSecondaryResource(P primary, Context

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

context) { + protected Optional getSecondaryResourceIndexAware(P primary, int index, Context

context) { if (index > 0 && resourceDiscriminator.isEmpty()) { throw new IllegalStateException( "Handling resources in bulk bot no resource discriminators set."); 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 3a3ce558f2..450491bad8 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 @@ -134,10 +134,19 @@ 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) { + return matcher.match(actualResource, primary, index, context); + } + public void delete(P primary, Context

context) { getSecondaryResource(primary, context).ifPresent(r -> client.resource(r).delete()); } + @Override + protected 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) { log.debug("{} target resource with type: {}, with id: {}", diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index 006dc9dd53..6e02c838c9 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -27,15 +27,33 @@ class StandaloneBulkDependentIT { @Test void managesBulkConfigMaps() { operator.create(testResource()); + assertNumberOfConfigMaps(3); + updateSpecWithNumber(1); + assertNumberOfConfigMaps(1); + + updateSpecWithNumber(5); + assertNumberOfConfigMaps(5); + + operator.delete(testResource()); + assertNumberOfConfigMaps(0); + } + + void assertNumberOfConfigMaps(int n) { await().untilAsserted(() -> { var cms = operator.getKubernetesClient().configMaps().inNamespace(operator.getNamespace()) .withLabel(LABEL_KEY, LABEL_VALUE) .list().getItems(); - assertThat(cms).hasSize(NUMBER_OF_CONFIG_MAPS); + assertThat(cms).hasSize(n); }); } + private void updateSpecWithNumber(int n) { + var resource = testResource(); + resource.getSpec().setNumberOfResources(n); + operator.replace(resource); + } + private StandaloneBulkDependentTestCustomResource testResource() { StandaloneBulkDependentTestCustomResource cr = new StandaloneBulkDependentTestCustomResource(); cr.setMetadata(new ObjectMeta()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java index 7d13f67b50..9f6a82ba40 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java @@ -4,6 +4,9 @@ import java.util.Optional; import java.util.stream.Collectors; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -12,6 +15,8 @@ public class ConfigMapBulkDependentResource extends CRUDKubernetesDependentResource { + private final static Logger log = LoggerFactory.getLogger(ConfigMapBulkDependentResource.class); + public static final String LABEL_KEY = "bulk"; public static final String LABEL_VALUE = "true"; From ceca37c3519173b9f6fd873468bab36c6ec24f7a Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 13 Sep 2022 13:02:26 +0200 Subject: [PATCH 22/48] bulk dependent resource to an interface --- .../dependent/AbstractDependentResource.java | 58 +++++-------------- .../dependent/BulkDependentResource.java | 18 ++++++ .../KubernetesDependentResource.java | 3 +- .../ConfigMapBulkDependentResource.java | 42 ++++++++------ 4 files changed, 56 insertions(+), 65 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResource.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 74b035c59c..0721bd4d7f 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 @@ -22,24 +22,25 @@ public abstract class AbstractDependentResource protected final boolean creatable = this instanceof Creator; protected final boolean updatable = this instanceof Updater; + protected final boolean bulk = this instanceof BulkDependentResource; protected Creator creator; protected Updater updater; + protected BulkDependentResource bulkDependentResource; protected List> resourceDiscriminator = new ArrayList<>(1); - // used just for bulk creation - protected BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory; @SuppressWarnings("unchecked") public AbstractDependentResource() { creator = creatable ? (Creator) this : null; updater = updatable ? (Updater) this : null; + bulkDependentResource = bulk ? (BulkDependentResource) this : null; } @Override public ReconcileResult reconcile(P primary, Context

context) { - var count = count(primary, context).orElse(1); - if (isBulkResourceCreation(primary, context)) { + var count = bulk ? bulkDependentResource.count(primary, context) : 1; + if (bulk) { cleanupBulkResourcesIfRequired(count, resourceDiscriminator.size(), primary, context); adjustDiscriminators(count); } @@ -59,23 +60,19 @@ private void cleanupBulkResourcesIfRequired(int targetCount, int actualCount, P for (int i = targetCount; i < actualCount; i++) { var resource = getSecondaryResourceIndexAware(primary, i, context); var index = i; - resource.ifPresent(r -> { - deleteBulkResourceWithIndex(primary, r, index, context); - }); + resource.ifPresent( + r -> bulkDependentResource.deleteBulkResourceWithIndex(primary, r, index, context)); } } - protected void deleteBulkResourceWithIndex(P primary, R resource, int i, Context

context) { - throw new IllegalStateException("Implement if handling bulk resources."); - } - private void adjustDiscriminators(int count) { if (resourceDiscriminator.size() == count) { return; } if (resourceDiscriminator.size() < count) { for (int i = resourceDiscriminator.size(); i < count; i++) { - resourceDiscriminator.add(bulkResourceDiscriminatorFactory.createResourceDiscriminator(i)); + resourceDiscriminator.add(bulkDependentResource.bulkResourceDiscriminatorFactory() + .createResourceDiscriminator(i)); } } if (resourceDiscriminator.size() > count) { @@ -98,7 +95,7 @@ protected ReconcileResult reconcileIndexAware(P primary, int i, Context

co final var actual = maybeActual.get(); if (updatable) { final Matcher.Result match; - if (isBulkResourceCreation(primary, context)) { + if (bulk) { match = updater.match(actual, primary, i, context); } else { match = updater.match(actual, primary, context); @@ -124,7 +121,7 @@ protected ReconcileResult reconcileIndexAware(P primary, int i, Context

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

context) { - return isBulkResourceCreation(primary, context) ? desired(primary, i, context) + return bulk ? desired(primary, i, context) : desired(primary, context); } @@ -139,7 +136,7 @@ protected Optional getSecondaryResourceIndexAware(P primary, int index, Conte throw new IllegalStateException( "Handling resources in bulk bot no resource discriminators set."); } - if (!isBulkResourceCreation(primary, context)) { + if (!bulk) { return getSecondaryResource(primary, context); } @@ -204,7 +201,7 @@ protected R desired(P primary, Context

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

context) { - if (!isBulkResourceCreation(primary, context)) { + if (!bulk) { return desired(primary, context); } else { throw new IllegalStateException( @@ -220,33 +217,4 @@ public AbstractDependentResource setResourceDiscriminator( return this; } - public ResourceDiscriminator getResourceDiscriminator() { - return resourceDiscriminator.get(0); - } - - /** - * @param primary resource - * @param context actual context - * @return empty optional if it's not a bulk resource management, number of instances otherwise. - */ - protected Optional count(P primary, Context

context) { - return Optional.empty(); - } - - /** - * Override in case the count() is a more heavy - */ - protected boolean isBulkResourceCreation(P primary, Context

context) { - return count(primary, context).isPresent(); - } - - public BulkResourceDiscriminatorFactory getBulkResourceDiscriminatorFactory() { - return bulkResourceDiscriminatorFactory; - } - - public AbstractDependentResource setBulkResourceDiscriminatorFactory( - BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory) { - this.bulkResourceDiscriminatorFactory = bulkResourceDiscriminatorFactory; - return this; - } } 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 new file mode 100644 index 0000000000..df5c2b920b --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResource.java @@ -0,0 +1,18 @@ +package io.javaoperatorsdk.operator.processing.dependent; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; + +public interface BulkDependentResource { + + int count(P primary, Context

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

context) { + throw new IllegalStateException("Implement if the dependent resource is a creator or updater"); + } + + void deleteBulkResourceWithIndex(P primary, R resource, int i, Context

context); + + BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory(); + +} 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 450491bad8..432d111676 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 @@ -142,8 +142,7 @@ public void delete(P primary, Context

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

context) { + public void deleteBulkResourceWithIndex(P primary, R resource, int i, Context

context) { client.resource(resource).delete(); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java index 9f6a82ba40..89b69edb41 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java @@ -10,35 +10,38 @@ 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.processing.dependent.BulkDependentResource; +import io.javaoperatorsdk.operator.processing.dependent.BulkResourceDiscriminatorFactory; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; public class ConfigMapBulkDependentResource - extends CRUDKubernetesDependentResource { + extends CRUDKubernetesDependentResource + implements BulkDependentResource { private final static Logger log = LoggerFactory.getLogger(ConfigMapBulkDependentResource.class); public static final String LABEL_KEY = "bulk"; public static final String LABEL_VALUE = "true"; + private BulkResourceDiscriminatorFactory factory = + index -> (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 ConfigMapBulkDependentResource() { super(ConfigMap.class); - setBulkResourceDiscriminatorFactory( - index -> (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)); - } - }); } @Override - protected ConfigMap desired(StandaloneBulkDependentTestCustomResource primary, + public ConfigMap desired(StandaloneBulkDependentTestCustomResource primary, int index, Context context) { ConfigMap configMap = new ConfigMap(); configMap.setMetadata(new ObjectMetaBuilder() @@ -51,10 +54,13 @@ protected ConfigMap desired(StandaloneBulkDependentTestCustomResource primary, } @Override - protected Optional count(StandaloneBulkDependentTestCustomResource primary, + public int count(StandaloneBulkDependentTestCustomResource primary, Context context) { - return Optional.of(primary.getSpec().getNumberOfResources()); + return primary.getSpec().getNumberOfResources(); } - + @Override + public BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory() { + return factory; + } } From abccb4ec609bb7fe2a96891b2b8d090ae1bb7d19 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 14 Sep 2022 16:20:45 +0200 Subject: [PATCH 23/48] wip --- .../dependent/kubernetes/KubernetesDependentResource.java | 1 + .../io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) 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 432d111676..c1d3693795 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 @@ -138,6 +138,7 @@ public Result match(R actualResource, P primary, int index, Context

contex return matcher.match(actualResource, primary, index, context); } + // todo delete bulk support public void delete(P primary, Context

context) { getSecondaryResource(primary, context).ifPresent(r -> client.resource(r).delete()); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index 6e02c838c9..4e7c8ae489 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -44,7 +44,8 @@ void assertNumberOfConfigMaps(int n) { var cms = operator.getKubernetesClient().configMaps().inNamespace(operator.getNamespace()) .withLabel(LABEL_KEY, LABEL_VALUE) .list().getItems(); - assertThat(cms).hasSize(n); + assertThat(cms).withFailMessage("Number of items is still: " + cms.size()) + .hasSize(n); }); } From 1fe3486e7421eab7a31698a68250f9525c8bbf29 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 14 Sep 2022 16:22:54 +0200 Subject: [PATCH 24/48] test improvement --- .../operator/StandaloneBulkDependentIT.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index 4e7c8ae489..c2d709ee8a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -1,5 +1,7 @@ package io.javaoperatorsdk.operator; +import java.time.Duration; + import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; @@ -40,13 +42,14 @@ void managesBulkConfigMaps() { } void assertNumberOfConfigMaps(int n) { - await().untilAsserted(() -> { - var cms = operator.getKubernetesClient().configMaps().inNamespace(operator.getNamespace()) - .withLabel(LABEL_KEY, LABEL_VALUE) - .list().getItems(); - assertThat(cms).withFailMessage("Number of items is still: " + cms.size()) - .hasSize(n); - }); + await().atMost(Duration.ofSeconds(20)) + .untilAsserted(() -> { + var cms = operator.getKubernetesClient().configMaps().inNamespace(operator.getNamespace()) + .withLabel(LABEL_KEY, LABEL_VALUE) + .list().getItems(); + assertThat(cms).withFailMessage("Number of items is still: " + cms.size()) + .hasSize(n); + }); } private void updateSpecWithNumber(int n) { From 83224c8a8648cb11f7b78db69f5df86d7d3bf914 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 14 Sep 2022 17:34:36 +0200 Subject: [PATCH 25/48] note --- .../operator/processing/dependent/AbstractDependentResource.java | 1 + 1 file changed, 1 insertion(+) 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 0721bd4d7f..7f131afd50 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 @@ -41,6 +41,7 @@ public AbstractDependentResource() { public ReconcileResult reconcile(P primary, Context

context) { var count = bulk ? bulkDependentResource.count(primary, context) : 1; if (bulk) { + //todo do this just if it is deleter? cleanupBulkResourcesIfRequired(count, resourceDiscriminator.size(), primary, context); adjustDiscriminators(count); } From 2d627f7d174910ab09992b5f186b2a8a23a7ef6a Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 15 Sep 2022 15:26:32 +0200 Subject: [PATCH 26/48] wip --- .../dependent/AbstractDependentResource.java | 10 +++------- 1 file changed, 3 insertions(+), 7 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 7f131afd50..6c452ea6bf 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 @@ -41,7 +41,7 @@ public AbstractDependentResource() { public ReconcileResult reconcile(P primary, Context

context) { var count = bulk ? bulkDependentResource.count(primary, context) : 1; if (bulk) { - //todo do this just if it is deleter? + // todo do this just if it is deleter? cleanupBulkResourcesIfRequired(count, resourceDiscriminator.size(), primary, context); adjustDiscriminators(count); } @@ -202,12 +202,8 @@ protected R desired(P primary, Context

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

context) { - if (!bulk) { - return desired(primary, context); - } else { - throw new IllegalStateException( - "desired() with index method must be implemented for bulk DependentResource creation"); - } + throw new IllegalStateException( + "Must be implemented for bulk DependentResource creation"); } public AbstractDependentResource setResourceDiscriminator( From 580c62fc11af6109162256044c210a9c2e03d356 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 20 Sep 2022 14:59:08 +0200 Subject: [PATCH 27/48] rebase on next --- .../processing/dependent/AbstractDependentResource.java | 8 ++++++++ 1 file changed, 8 insertions(+) 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 6c452ea6bf..821be1dfee 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 @@ -214,4 +214,12 @@ public AbstractDependentResource setResourceDiscriminator( return this; } + public ResourceDiscriminator getResourceDiscriminator() { + if (this.resourceDiscriminator.isEmpty()) { + return null; + } else { + return this.resourceDiscriminator.get(0); + } + } + } From 5339b04645f47d1fb2aee7a173818510681fbfba Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 20 Sep 2022 15:12:04 +0200 Subject: [PATCH 28/48] increates test timeout --- .../io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index c2d709ee8a..7928c44f1a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -42,7 +42,7 @@ void managesBulkConfigMaps() { } void assertNumberOfConfigMaps(int n) { - await().atMost(Duration.ofSeconds(20)) + await().atMost(Duration.ofSeconds(30)) .untilAsserted(() -> { var cms = operator.getKubernetesClient().configMaps().inNamespace(operator.getNamespace()) .withLabel(LABEL_KEY, LABEL_VALUE) From 9d17abae4162b75c0a2399411e9fdcd99c157cee Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 20 Sep 2022 15:36:12 +0200 Subject: [PATCH 29/48] comment --- .../io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java | 1 + 1 file changed, 1 insertion(+) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index 7928c44f1a..30e3c82549 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -42,6 +42,7 @@ void managesBulkConfigMaps() { } void assertNumberOfConfigMaps(int n) { + // this test was failing with a lower timeout on GitHub, probably the garbage collection was slower there. await().atMost(Duration.ofSeconds(30)) .untilAsserted(() -> { var cms = operator.getKubernetesClient().configMaps().inNamespace(operator.getNamespace()) From 1704be9b647eebe2737db3f149753634d4a9f6ba Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 20 Sep 2022 16:12:30 +0200 Subject: [PATCH 30/48] fix format --- .../operator/StandaloneBulkDependentIT.java | 13 +++++++------ .../ConfigMapBulkDependentResource.java | 2 +- .../StandaloneBulkDependentReconciler.java | 2 +- .../StandaloneBulkDependentTestCustomResource.java | 2 +- .../StandaloneBulkDependentTestSpec.java | 2 +- 5 files changed, 11 insertions(+), 10 deletions(-) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/{standalonebulkdependent => bulkdependent}/ConfigMapBulkDependentResource.java (97%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/{standalonebulkdependent => bulkdependent}/StandaloneBulkDependentReconciler.java (96%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/{standalonebulkdependent => bulkdependent}/StandaloneBulkDependentTestCustomResource.java (87%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/{standalonebulkdependent => bulkdependent}/StandaloneBulkDependentTestSpec.java (82%) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index 30e3c82549..c30988d00d 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -7,12 +7,12 @@ import io.fabric8.kubernetes.api.model.ObjectMeta; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; -import io.javaoperatorsdk.operator.sample.standalonebulkdependent.StandaloneBulkDependentReconciler; -import io.javaoperatorsdk.operator.sample.standalonebulkdependent.StandaloneBulkDependentTestCustomResource; -import io.javaoperatorsdk.operator.sample.standalonebulkdependent.StandaloneBulkDependentTestSpec; +import io.javaoperatorsdk.operator.sample.bulkdependent.StandaloneBulkDependentReconciler; +import io.javaoperatorsdk.operator.sample.bulkdependent.StandaloneBulkDependentTestCustomResource; +import io.javaoperatorsdk.operator.sample.bulkdependent.StandaloneBulkDependentTestSpec; -import static io.javaoperatorsdk.operator.sample.standalonebulkdependent.ConfigMapBulkDependentResource.LABEL_KEY; -import static io.javaoperatorsdk.operator.sample.standalonebulkdependent.ConfigMapBulkDependentResource.LABEL_VALUE; +import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapBulkDependentResource.LABEL_KEY; +import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapBulkDependentResource.LABEL_VALUE; import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; @@ -42,7 +42,8 @@ void managesBulkConfigMaps() { } void assertNumberOfConfigMaps(int n) { - // this test was failing with a lower timeout on GitHub, probably the garbage collection was slower there. + // this test was failing with a lower timeout on GitHub, probably the garbage collection was + // slower there. await().atMost(Duration.ofSeconds(30)) .untilAsserted(() -> { var cms = operator.getKubernetesClient().configMaps().inNamespace(operator.getNamespace()) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java similarity index 97% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java index 89b69edb41..6daa108624 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/ConfigMapBulkDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample.standalonebulkdependent; +package io.javaoperatorsdk.operator.sample.bulkdependent; import java.util.Map; import java.util.Optional; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java similarity index 96% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentReconciler.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java index bf2b3cbf09..adf3b7e517 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample.standalonebulkdependent; +package io.javaoperatorsdk.operator.sample.bulkdependent; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestCustomResource.java similarity index 87% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestCustomResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestCustomResource.java index 4ae13f945b..8fc9559a17 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestCustomResource.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample.standalonebulkdependent; +package io.javaoperatorsdk.operator.sample.bulkdependent; import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestSpec.java similarity index 82% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestSpec.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestSpec.java index 521da411c3..5e48beae5b 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonebulkdependent/StandaloneBulkDependentTestSpec.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestSpec.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator.sample.standalonebulkdependent; +package io.javaoperatorsdk.operator.sample.bulkdependent; public class StandaloneBulkDependentTestSpec { From fed173dfe2d5c488ada871d9e85bc31e57afbd05 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 20 Sep 2022 16:23:31 +0200 Subject: [PATCH 31/48] wip --- .../ManagedBulkDependentReconciler.java | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java new file mode 100644 index 0000000000..aa5ab84446 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java @@ -0,0 +1,25 @@ +package io.javaoperatorsdk.operator.sample.bulkdependent; + +import java.util.concurrent.atomic.AtomicInteger; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; + +@ControllerConfiguration +public class ManagedBulkDependentReconciler + implements Reconciler { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + StandaloneBulkDependentTestCustomResource resource, + Context context) throws Exception { + + numberOfExecutions.addAndGet(1); + + return UpdateControl.noUpdate(); + } +} From c7db869b6fdcb19cd4acf97b7a2a9ae9a6e2b83d Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 21 Sep 2022 11:17:40 +0200 Subject: [PATCH 32/48] delete, other improvements --- docs/assets/js/uikit.js | 2 +- .../dependent/AbstractDependentResource.java | 6 ++-- .../dependent/BulkDependentResource.java | 28 ++++++++++++++++--- .../KubernetesDependentResource.java | 11 +++++--- .../operator/StandaloneBulkDependentIT.java | 1 + .../ManagedBulkDependentReconciler.java | 3 +- 6 files changed, 38 insertions(+), 13 deletions(-) diff --git a/docs/assets/js/uikit.js b/docs/assets/js/uikit.js index 5b1acf9275..665c8c0522 100644 --- a/docs/assets/js/uikit.js +++ b/docs/assets/js/uikit.js @@ -8214,7 +8214,7 @@ updateAria: function(toggled) { attr(this.$el, 'aria-expanded', isBoolean(toggled) - ? toggled + ? toggled// todo delete bulk support : isToggled(this.target, this.cls) ); } 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 821be1dfee..aa6e1a0f5f 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 @@ -34,6 +34,7 @@ public abstract class AbstractDependentResource public AbstractDependentResource() { creator = creatable ? (Creator) this : null; updater = updatable ? (Updater) this : null; + bulkDependentResource = bulk ? (BulkDependentResource) this : null; } @@ -41,8 +42,7 @@ public AbstractDependentResource() { public ReconcileResult reconcile(P primary, Context

context) { var count = bulk ? bulkDependentResource.count(primary, context) : 1; if (bulk) { - // todo do this just if it is deleter? - cleanupBulkResourcesIfRequired(count, resourceDiscriminator.size(), primary, context); + deleteBulkResourcesIfRequired(count, resourceDiscriminator.size(), primary, context); adjustDiscriminators(count); } ReconcileResult result = new ReconcileResult<>(); @@ -53,7 +53,7 @@ public ReconcileResult reconcile(P primary, Context

context) { return result; } - private void cleanupBulkResourcesIfRequired(int targetCount, int actualCount, P primary, + protected void deleteBulkResourcesIfRequired(int targetCount, int actualCount, P primary, Context

context) { if (targetCount >= actualCount) { return; 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 df5c2b920b..c89af4b481 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 @@ -2,17 +2,37 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; -public interface BulkDependentResource { +/** + * Manages dynamic number of resources created for a primary resource. Since the point of a bulk + * dependent resource is to manage the number of secondary resources dynamically it implement + * {@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

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

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

context) { - throw new IllegalStateException("Implement if the dependent resource is a creator or updater"); - } + R desired(P primary, int index, 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 context actual context + */ void deleteBulkResourceWithIndex(P primary, R resource, int i, Context

context); + /** + * @return a discriminator factor that helps to create a discriminator for a certain resource with + * an index + */ BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory(); } 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 c1d3693795..e81d3ec376 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 @@ -138,9 +138,13 @@ public Result match(R actualResource, P primary, int index, Context

contex return matcher.match(actualResource, primary, index, context); } - // todo delete bulk support public void delete(P primary, Context

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

context) { @@ -158,8 +162,7 @@ protected Resource prepare(R desired, P primary, String actionName) { } else if (useDefaultAnnotationsToIdentifyPrimary()) { addDefaultSecondaryToPrimaryMapperAnnotations(desired, primary); } - Class targetClass = (Class) desired.getClass(); - return client.resources(targetClass).inNamespace(desired.getMetadata().getNamespace()) + return client.resource(desired).inNamespace(desired.getMetadata().getNamespace()) .resource(desired); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index c30988d00d..5d9f2e643d 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -26,6 +26,7 @@ class StandaloneBulkDependentIT { LocallyRunOperatorExtension.builder().withReconciler(new StandaloneBulkDependentReconciler()) .build(); + // TODO update, deleter - no GC test, external bulk resource @Test void managesBulkConfigMaps() { operator.create(testResource()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java index aa5ab84446..466e4cb159 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java @@ -6,8 +6,9 @@ import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; -@ControllerConfiguration +@ControllerConfiguration(dependents = @Dependent(type = ConfigMapBulkDependentResource.class)) public class ManagedBulkDependentReconciler implements Reconciler { From da8541efeff69252bbfe93d78cd95f807debc716 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 21 Sep 2022 13:36:39 +0200 Subject: [PATCH 33/48] manage tests, refactored ITs --- .../operator/BulkDependentTestBase.java | 68 +++++++++++++++++++ .../operator/ManagedBulkDependentIT.java | 20 ++++++ .../operator/StandaloneBulkDependentIT.java | 64 ++--------------- ...a => BulkDependentTestCustomResource.java} | 4 +- ...stSpec.java => BulkDependentTestSpec.java} | 4 +- .../ConfigMapBulkDependentResource.java | 16 ++--- .../ManagedBulkDependentReconciler.java | 8 +-- .../StandaloneBulkDependentReconciler.java | 12 ++-- 8 files changed, 115 insertions(+), 81 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/ManagedBulkDependentIT.java rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/{StandaloneBulkDependentTestCustomResource.java => BulkDependentTestCustomResource.java} (77%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/{StandaloneBulkDependentTestSpec.java => BulkDependentTestSpec.java} (64%) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java new file mode 100644 index 0000000000..697cd2fc9b --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java @@ -0,0 +1,68 @@ +package io.javaoperatorsdk.operator; + +import java.time.Duration; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.bulkdependent.BulkDependentTestCustomResource; +import io.javaoperatorsdk.operator.sample.bulkdependent.BulkDependentTestSpec; + +import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapBulkDependentResource.LABEL_KEY; +import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapBulkDependentResource.LABEL_VALUE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +public abstract class BulkDependentTestBase { + + public static final String TEST_RESOURCE_NAME = "test"; + public static final int INITIAL_NUMBER_OF_CONFIG_MAPS = 3; + + @Test + public void managesBulkConfigMaps() { + extension().create(testResource()); + assertNumberOfConfigMaps(3); + + updateSpecWithNumber(1); + assertNumberOfConfigMaps(1); + + updateSpecWithNumber(5); + assertNumberOfConfigMaps(5); + + extension().delete(testResource()); + assertNumberOfConfigMaps(0); + } + + private void assertNumberOfConfigMaps(int n) { + // this test was failing with a lower timeout on GitHub, probably the garbage collection was + // slower there. + await().atMost(Duration.ofSeconds(30)) + .untilAsserted(() -> { + var cms = + extension().getKubernetesClient().configMaps().inNamespace(extension().getNamespace()) + .withLabel(LABEL_KEY, LABEL_VALUE) + .list().getItems(); + assertThat(cms).withFailMessage("Number of items is still: " + cms.size()) + .hasSize(n); + }); + } + + private BulkDependentTestCustomResource testResource() { + BulkDependentTestCustomResource cr = new BulkDependentTestCustomResource(); + cr.setMetadata(new ObjectMeta()); + cr.getMetadata().setName(TEST_RESOURCE_NAME); + cr.setSpec(new BulkDependentTestSpec()); + cr.getSpec().setNumberOfResources(INITIAL_NUMBER_OF_CONFIG_MAPS); + return cr; + } + + + private void updateSpecWithNumber(int n) { + var resource = testResource(); + resource.getSpec().setNumberOfResources(n); + extension().replace(resource); + } + + abstract LocallyRunOperatorExtension extension(); +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ManagedBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ManagedBulkDependentIT.java new file mode 100644 index 0000000000..4c38714dbe --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ManagedBulkDependentIT.java @@ -0,0 +1,20 @@ +package io.javaoperatorsdk.operator; + +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.bulkdependent.ManagedBulkDependentReconciler; + +class ManagedBulkDependentIT extends BulkDependentTestBase { + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(new ManagedBulkDependentReconciler()) + .build(); + + + @Override + LocallyRunOperatorExtension extension() { + return extension; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java index 5d9f2e643d..b0f999f2a4 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java @@ -1,73 +1,19 @@ package io.javaoperatorsdk.operator; -import java.time.Duration; - -import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; -import io.fabric8.kubernetes.api.model.ObjectMeta; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import io.javaoperatorsdk.operator.sample.bulkdependent.StandaloneBulkDependentReconciler; -import io.javaoperatorsdk.operator.sample.bulkdependent.StandaloneBulkDependentTestCustomResource; -import io.javaoperatorsdk.operator.sample.bulkdependent.StandaloneBulkDependentTestSpec; - -import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapBulkDependentResource.LABEL_KEY; -import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapBulkDependentResource.LABEL_VALUE; -import static org.assertj.core.api.Assertions.assertThat; -import static org.awaitility.Awaitility.await; -class StandaloneBulkDependentIT { - - public static final String TEST_RESOURCE_NAME = "test"; - public static final int NUMBER_OF_CONFIG_MAPS = 3; +class StandaloneBulkDependentIT extends BulkDependentTestBase { @RegisterExtension - LocallyRunOperatorExtension operator = + LocallyRunOperatorExtension extension = LocallyRunOperatorExtension.builder().withReconciler(new StandaloneBulkDependentReconciler()) .build(); - // TODO update, deleter - no GC test, external bulk resource - @Test - void managesBulkConfigMaps() { - operator.create(testResource()); - assertNumberOfConfigMaps(3); - - updateSpecWithNumber(1); - assertNumberOfConfigMaps(1); - - updateSpecWithNumber(5); - assertNumberOfConfigMaps(5); - - operator.delete(testResource()); - assertNumberOfConfigMaps(0); + @Override + LocallyRunOperatorExtension extension() { + return extension; } - - void assertNumberOfConfigMaps(int n) { - // this test was failing with a lower timeout on GitHub, probably the garbage collection was - // slower there. - await().atMost(Duration.ofSeconds(30)) - .untilAsserted(() -> { - var cms = operator.getKubernetesClient().configMaps().inNamespace(operator.getNamespace()) - .withLabel(LABEL_KEY, LABEL_VALUE) - .list().getItems(); - assertThat(cms).withFailMessage("Number of items is still: " + cms.size()) - .hasSize(n); - }); - } - - private void updateSpecWithNumber(int n) { - var resource = testResource(); - resource.getSpec().setNumberOfResources(n); - operator.replace(resource); - } - - private StandaloneBulkDependentTestCustomResource testResource() { - StandaloneBulkDependentTestCustomResource cr = new StandaloneBulkDependentTestCustomResource(); - cr.setMetadata(new ObjectMeta()); - cr.getMetadata().setName(TEST_RESOURCE_NAME); - cr.setSpec(new StandaloneBulkDependentTestSpec()); - cr.getSpec().setNumberOfResources(NUMBER_OF_CONFIG_MAPS); - return cr; - } - } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestCustomResource.java similarity index 77% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestCustomResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestCustomResource.java index 8fc9559a17..68e6297f8c 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestCustomResource.java @@ -9,7 +9,7 @@ @Group("sample.javaoperatorsdk") @Version("v1") @ShortNames("sbd") -public class StandaloneBulkDependentTestCustomResource - extends CustomResource +public class BulkDependentTestCustomResource + extends CustomResource implements Namespaced { } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestSpec.java similarity index 64% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestSpec.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestSpec.java index 5e48beae5b..62b8b0fceb 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentTestSpec.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestSpec.java @@ -1,6 +1,6 @@ package io.javaoperatorsdk.operator.sample.bulkdependent; -public class StandaloneBulkDependentTestSpec { +public class BulkDependentTestSpec { private Integer numberOfResources; @@ -8,7 +8,7 @@ public Integer getNumberOfResources() { return numberOfResources; } - public StandaloneBulkDependentTestSpec setNumberOfResources(Integer numberOfResources) { + public BulkDependentTestSpec setNumberOfResources(Integer numberOfResources) { this.numberOfResources = numberOfResources; return this; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java index 6daa108624..c404f1b4dc 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java @@ -15,14 +15,14 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; public class ConfigMapBulkDependentResource - extends CRUDKubernetesDependentResource - implements BulkDependentResource { + extends CRUDKubernetesDependentResource + implements BulkDependentResource { private final static Logger log = LoggerFactory.getLogger(ConfigMapBulkDependentResource.class); public static final String LABEL_KEY = "bulk"; public static final String LABEL_VALUE = "true"; - private BulkResourceDiscriminatorFactory factory = + private BulkResourceDiscriminatorFactory factory = index -> (resource, primary, context) -> { var resources = context.getSecondaryResources(resource).stream() .filter(r -> r.getMetadata().getName().endsWith("-" + index)) @@ -41,8 +41,8 @@ public ConfigMapBulkDependentResource() { } @Override - public ConfigMap desired(StandaloneBulkDependentTestCustomResource primary, - int index, Context context) { + public ConfigMap desired(BulkDependentTestCustomResource primary, + int index, Context context) { ConfigMap configMap = new ConfigMap(); configMap.setMetadata(new ObjectMetaBuilder() .withName(primary.getMetadata().getName() + "-" + index) @@ -54,13 +54,13 @@ public ConfigMap desired(StandaloneBulkDependentTestCustomResource primary, } @Override - public int count(StandaloneBulkDependentTestCustomResource primary, - Context context) { + public int count(BulkDependentTestCustomResource primary, + Context context) { return primary.getSpec().getNumberOfResources(); } @Override - public BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory() { + public BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory() { return factory; } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java index 466e4cb159..4d22a6300a 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java @@ -10,14 +10,14 @@ @ControllerConfiguration(dependents = @Dependent(type = ConfigMapBulkDependentResource.class)) public class ManagedBulkDependentReconciler - implements Reconciler { + implements Reconciler { private final AtomicInteger numberOfExecutions = new AtomicInteger(0); @Override - public UpdateControl reconcile( - StandaloneBulkDependentTestCustomResource resource, - Context context) throws Exception { + public UpdateControl reconcile( + BulkDependentTestCustomResource resource, + Context context) throws Exception { numberOfExecutions.addAndGet(1); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java index adf3b7e517..db6dca8590 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java @@ -11,8 +11,8 @@ @ControllerConfiguration public class StandaloneBulkDependentReconciler - implements Reconciler, TestExecutionInfoProvider, - EventSourceInitializer, KubernetesClientAware { + implements Reconciler, TestExecutionInfoProvider, + EventSourceInitializer, KubernetesClientAware { private final AtomicInteger numberOfExecutions = new AtomicInteger(0); @@ -24,9 +24,9 @@ public StandaloneBulkDependentReconciler() { } @Override - public UpdateControl reconcile( - StandaloneBulkDependentTestCustomResource resource, - Context context) { + public UpdateControl reconcile( + BulkDependentTestCustomResource resource, + Context context) { numberOfExecutions.addAndGet(1); dependent.reconcile(resource, context); @@ -40,7 +40,7 @@ public int getNumberOfExecutions() { @Override public Map prepareEventSources( - EventSourceContext context) { + EventSourceContext context) { return EventSourceInitializer .nameEventSources(dependent.initEventSource(context)); } From 714f42c36955927032d5bb105ad51166895dc9fa Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 21 Sep 2022 14:50:30 +0200 Subject: [PATCH 34/48] additionl IT --- .../operator/BulkDependentDeleterIT.java | 19 ++++++++++ .../operator/BulkDependentTestBase.java | 37 ++++++++++++++++++- .../bulkdependent/BulkDependentTestSpec.java | 10 +++++ .../CRUDConfigMapBulkDependentResource.java | 7 ++++ ...onfigMapDeleterBulkDependentResource.java} | 28 +++++++++----- .../ManagedBulkDependentReconciler.java | 3 +- .../ManagedDeleterBulkReconciler.java | 20 ++++++++++ .../StandaloneBulkDependentReconciler.java | 4 +- 8 files changed, 112 insertions(+), 16 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentDeleterIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/CRUDConfigMapBulkDependentResource.java rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/{ConfigMapBulkDependentResource.java => ConfigMapDeleterBulkDependentResource.java} (68%) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedDeleterBulkReconciler.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentDeleterIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentDeleterIT.java new file mode 100644 index 0000000000..dce6df8b98 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentDeleterIT.java @@ -0,0 +1,19 @@ +package io.javaoperatorsdk.operator; + +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.bulkdependent.ManagedDeleterBulkReconciler; + +public class BulkDependentDeleterIT extends BulkDependentTestBase { + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(new ManagedDeleterBulkReconciler()) + .build(); + + @Override + LocallyRunOperatorExtension extension() { + return extension; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java index 697cd2fc9b..0a5cfee161 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java @@ -8,9 +8,10 @@ import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; import io.javaoperatorsdk.operator.sample.bulkdependent.BulkDependentTestCustomResource; import io.javaoperatorsdk.operator.sample.bulkdependent.BulkDependentTestSpec; +import io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapDeleterBulkDependentResource; -import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapBulkDependentResource.LABEL_KEY; -import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapBulkDependentResource.LABEL_VALUE; +import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapDeleterBulkDependentResource.LABEL_KEY; +import static io.javaoperatorsdk.operator.sample.bulkdependent.ConfigMapDeleterBulkDependentResource.LABEL_VALUE; import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; @@ -18,6 +19,8 @@ public abstract class BulkDependentTestBase { public static final String TEST_RESOURCE_NAME = "test"; public static final int INITIAL_NUMBER_OF_CONFIG_MAPS = 3; + public static final String INITIAL_ADDITIONAL_DATA = "initialData"; + public static final String NEW_VERSION_OF_ADDITIONAL_DATA = "newVersionOfAdditionalData"; @Test public void managesBulkConfigMaps() { @@ -34,6 +37,16 @@ public void managesBulkConfigMaps() { assertNumberOfConfigMaps(0); } + @Test + public void updatesData() { + extension().create(testResource()); + assertNumberOfConfigMaps(3); + assertAdditionalDataOnConfigMaps(INITIAL_ADDITIONAL_DATA); + + updateSpecWithNewAdditionalData(NEW_VERSION_OF_ADDITIONAL_DATA); + assertAdditionalDataOnConfigMaps(NEW_VERSION_OF_ADDITIONAL_DATA); + } + private void assertNumberOfConfigMaps(int n) { // this test was failing with a lower timeout on GitHub, probably the garbage collection was // slower there. @@ -48,15 +61,35 @@ private void assertNumberOfConfigMaps(int n) { }); } + private void assertAdditionalDataOnConfigMaps(String expectedValue) { + await().atMost(Duration.ofSeconds(30)) + .untilAsserted(() -> { + var cms = + extension().getKubernetesClient().configMaps().inNamespace(extension().getNamespace()) + .withLabel(LABEL_KEY, LABEL_VALUE) + .list().getItems(); + cms.forEach(cm -> { + assertThat(cm.getData().get(ConfigMapDeleterBulkDependentResource.ADDITIONAL_DATA_KEY)) + .isEqualTo(expectedValue); + }); + }); + } + private BulkDependentTestCustomResource testResource() { BulkDependentTestCustomResource cr = new BulkDependentTestCustomResource(); cr.setMetadata(new ObjectMeta()); cr.getMetadata().setName(TEST_RESOURCE_NAME); cr.setSpec(new BulkDependentTestSpec()); cr.getSpec().setNumberOfResources(INITIAL_NUMBER_OF_CONFIG_MAPS); + cr.getSpec().setAdditionalData(INITIAL_ADDITIONAL_DATA); return cr; } + private void updateSpecWithNewAdditionalData(String data) { + var resource = testResource(); + resource.getSpec().setAdditionalData(data); + extension().replace(resource); + } private void updateSpecWithNumber(int n) { var resource = testResource(); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestSpec.java index 62b8b0fceb..5266950b41 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestSpec.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/BulkDependentTestSpec.java @@ -3,6 +3,7 @@ public class BulkDependentTestSpec { private Integer numberOfResources; + private String additionalData; public Integer getNumberOfResources() { return numberOfResources; @@ -12,4 +13,13 @@ public BulkDependentTestSpec setNumberOfResources(Integer numberOfResources) { this.numberOfResources = numberOfResources; return this; } + + public BulkDependentTestSpec setAdditionalData(String additionalData) { + this.additionalData = additionalData; + return this; + } + + public String getAdditionalData() { + return additionalData; + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/CRUDConfigMapBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/CRUDConfigMapBulkDependentResource.java new file mode 100644 index 0000000000..83cec0bb69 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/CRUDConfigMapBulkDependentResource.java @@ -0,0 +1,7 @@ +package io.javaoperatorsdk.operator.sample.bulkdependent; + +import io.javaoperatorsdk.operator.api.reconciler.dependent.GarbageCollected; + +public class CRUDConfigMapBulkDependentResource extends ConfigMapDeleterBulkDependentResource + implements GarbageCollected { +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java similarity index 68% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java index c404f1b4dc..eab06ac61d 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapBulkDependentResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ConfigMapDeleterBulkDependentResource.java @@ -4,24 +4,31 @@ import java.util.Optional; import java.util.stream.Collectors; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - 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.dependent.Deleter; import io.javaoperatorsdk.operator.processing.dependent.BulkDependentResource; import io.javaoperatorsdk.operator.processing.dependent.BulkResourceDiscriminatorFactory; +import io.javaoperatorsdk.operator.processing.dependent.Creator; +import io.javaoperatorsdk.operator.processing.dependent.Updater; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; -public class ConfigMapBulkDependentResource - extends CRUDKubernetesDependentResource - implements BulkDependentResource { - - private final static Logger log = LoggerFactory.getLogger(ConfigMapBulkDependentResource.class); +/** + * Not using CRUDKubernetesDependentResource so the delete functionality can be tested. + */ +public class ConfigMapDeleterBulkDependentResource + extends + KubernetesDependentResource + implements Creator, + Updater, + Deleter, + BulkDependentResource { public static final String LABEL_KEY = "bulk"; public static final String LABEL_VALUE = "true"; + public static final String ADDITIONAL_DATA_KEY = "additionalData"; private BulkResourceDiscriminatorFactory factory = index -> (resource, primary, context) -> { var resources = context.getSecondaryResources(resource).stream() @@ -36,7 +43,7 @@ public class ConfigMapBulkDependentResource } }; - public ConfigMapBulkDependentResource() { + public ConfigMapDeleterBulkDependentResource() { super(ConfigMap.class); } @@ -49,7 +56,8 @@ public ConfigMap desired(BulkDependentTestCustomResource primary, .withNamespace(primary.getMetadata().getNamespace()) .withLabels(Map.of(LABEL_KEY, LABEL_VALUE)) .build()); - configMap.setData(Map.of("number", "" + index)); + configMap.setData( + Map.of("number", "" + index, ADDITIONAL_DATA_KEY, primary.getSpec().getAdditionalData())); return configMap; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java index 4d22a6300a..3b2acd942e 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedBulkDependentReconciler.java @@ -8,7 +8,7 @@ import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; -@ControllerConfiguration(dependents = @Dependent(type = ConfigMapBulkDependentResource.class)) +@ControllerConfiguration(dependents = @Dependent(type = CRUDConfigMapBulkDependentResource.class)) public class ManagedBulkDependentReconciler implements Reconciler { @@ -20,7 +20,6 @@ public UpdateControl reconcile( Context context) throws Exception { numberOfExecutions.addAndGet(1); - return UpdateControl.noUpdate(); } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedDeleterBulkReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedDeleterBulkReconciler.java new file mode 100644 index 0000000000..e759bdd200 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/ManagedDeleterBulkReconciler.java @@ -0,0 +1,20 @@ +package io.javaoperatorsdk.operator.sample.bulkdependent; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; + +@ControllerConfiguration( + dependents = @Dependent(type = ConfigMapDeleterBulkDependentResource.class)) +public class ManagedDeleterBulkReconciler implements Reconciler { + @Override + public UpdateControl reconcile( + BulkDependentTestCustomResource resource, + Context context) + throws Exception { + + return UpdateControl.noUpdate(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java index db6dca8590..4033583340 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/StandaloneBulkDependentReconciler.java @@ -16,11 +16,11 @@ public class StandaloneBulkDependentReconciler private final AtomicInteger numberOfExecutions = new AtomicInteger(0); - private ConfigMapBulkDependentResource dependent; + private ConfigMapDeleterBulkDependentResource dependent; private KubernetesClient kubernetesClient; public StandaloneBulkDependentReconciler() { - dependent = new ConfigMapBulkDependentResource(); + dependent = new CRUDConfigMapBulkDependentResource(); } @Override From 57428da84c0471a8ab690b44a48d85c38fa44aa4 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 21 Sep 2022 17:06:00 +0200 Subject: [PATCH 35/48] external resource --- .../dependent/AbstractDependentResource.java | 6 +- .../processing/dependent/BulkUpdater.java | 13 +++ .../KubernetesDependentResource.java | 2 +- .../ExternalBulkDependentResource.java | 104 ++++++++++++++++++ .../external/ExternalResource.java | 37 +++++++ .../external/ExternalServiceMock.java | 39 +++++++ 6 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkUpdater.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkDependentResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalServiceMock.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 aa6e1a0f5f..af5f1e02e6 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 AbstractDependentResource() { public ReconcileResult reconcile(P primary, Context

context) { var count = bulk ? bulkDependentResource.count(primary, context) : 1; if (bulk) { - deleteBulkResourcesIfRequired(count, resourceDiscriminator.size(), primary, context); + deleteBulkResourcesIfRequired(count, lastKnownBulkSize(), primary, context); adjustDiscriminators(count); } ReconcileResult result = new ReconcileResult<>(); @@ -222,4 +222,8 @@ public ResourceDiscriminator getResourceDiscriminator() { } } + protected int lastKnownBulkSize() { + return resourceDiscriminator.size(); + } + } 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 new file mode 100644 index 0000000000..d5ad2957d2 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkUpdater.java @@ -0,0 +1,13 @@ +package io.javaoperatorsdk.operator.processing.dependent; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.reconciler.Context; + +public interface BulkUpdater extends Updater { + + default Matcher.Result match(R actualResource, P primary, Context

context) { + throw new IllegalStateException(); + } + + 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/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index e81d3ec376..1e2fbf2df0 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,7 +140,7 @@ public Result match(R actualResource, P primary, int index, Context

contex public void delete(P primary, Context

context) { if (bulk) { - deleteBulkResourcesIfRequired(0, resourceDiscriminator.size(), primary, context); + deleteBulkResourcesIfRequired(0, lastKnownBulkSize(), primary, context); } else { var resource = getSecondaryResource(primary, context); resource.ifPresent(r -> client.resource(r).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 new file mode 100644 index 0000000000..fc95bebdb7 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkDependentResource.java @@ -0,0 +1,104 @@ +package io.javaoperatorsdk.operator.sample.bulkdependent.external; + +import java.util.HashMap; +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.processing.dependent.*; +import io.javaoperatorsdk.operator.processing.dependent.external.PollingDependentResource; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.CacheKeyMapper; +import io.javaoperatorsdk.operator.sample.bulkdependent.BulkDependentTestCustomResource; + +public class ExternalBulkDependentResource + extends PollingDependentResource + implements BulkDependentResource, + BulkUpdater { + + public static final String EXTERNAL_RESOURCE_NAME_DELIMITER = "#"; + + private ExternalServiceMock externalServiceMock = ExternalServiceMock.getInstance(); + + public ExternalBulkDependentResource() { + super(ExternalResource.class, CacheKeyMapper.singleResourceCacheKeyMapper()); + } + + @Override + public Map> fetchResources() { + // todo + Map> result = new HashMap<>(); + var resources = externalServiceMock.listResources(); + + return result; + } + + @Override + public void delete(BulkDependentTestCustomResource primary, + Context context) { + deleteBulkResourcesIfRequired(0, lastKnownBulkSize(), primary, context); + } + + @Override + public int count(BulkDependentTestCustomResource primary, + Context context) { + return primary.getSpec().getNumberOfResources(); + } + + @Override + public void deleteBulkResourceWithIndex(BulkDependentTestCustomResource primary, + ExternalResource resource, int i, Context context) { + externalServiceMock.delete(resource.getId()); + } + + @Override + public BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory() { + return index -> (resource, primary, context) -> { + ExternalResource collect = context.getSecondaryResources(resource).stream() + .filter(r -> r.getId().endsWith(EXTERNAL_RESOURCE_NAME_DELIMITER + index)) + .collect(Collectors.collectingAndThen( + Collectors.toList(), + list -> { + if (list.size() > 1) { + throw new IllegalStateException("Found more than 1 object: " + list); + } + return list.get(0); + })); + return Optional.ofNullable(collect); + }; + } + + @Override + public ExternalResource desired(BulkDependentTestCustomResource primary, int index, + Context context) { + return new ExternalResource(toExternalResourceId(primary, index), "" + index); + } + + @Override + public ExternalResource create(ExternalResource desired, BulkDependentTestCustomResource primary, + Context context) { + return externalServiceMock.create(desired); + } + + @Override + public ExternalResource update(ExternalResource actual, ExternalResource desired, + BulkDependentTestCustomResource primary, Context context) { + return externalServiceMock.update(desired); + } + + @Override + public Matcher.Result match(ExternalResource actualResource, + BulkDependentTestCustomResource primary, + int 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; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalResource.java new file mode 100644 index 0000000000..935fd99e47 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalResource.java @@ -0,0 +1,37 @@ +package io.javaoperatorsdk.operator.sample.bulkdependent.external; + +import java.util.Objects; + +public class ExternalResource { + + private String id; + private String data; + + public ExternalResource(String id, String data) { + this.id = id; + this.data = data; + } + + public String getId() { + return id; + } + + public String getData() { + return data; + } + + @Override + public boolean equals(Object o) { + if (this == o) + return true; + if (o == null || getClass() != o.getClass()) + return false; + ExternalResource that = (ExternalResource) o; + return Objects.equals(id, that.id) && Objects.equals(data, that.data); + } + + @Override + public int hashCode() { + return Objects.hash(id, data); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalServiceMock.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalServiceMock.java new file mode 100644 index 0000000000..e73062ccf2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalServiceMock.java @@ -0,0 +1,39 @@ +package io.javaoperatorsdk.operator.sample.bulkdependent.external; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; + +public class ExternalServiceMock { + + private static ExternalServiceMock serviceMock = new ExternalServiceMock(); + + private Map resourceMap = new ConcurrentHashMap<>(); + + public ExternalResource create(ExternalResource externalResource) { + resourceMap.put(externalResource.getId(), externalResource); + return externalResource; + } + + public Optional read(String id) { + return Optional.ofNullable(resourceMap.get(id)); + } + + public ExternalResource update(ExternalResource externalResource) { + return resourceMap.put(externalResource.getId(), externalResource); + } + + public Optional delete(String id) { + return Optional.ofNullable(resourceMap.remove(id)); + } + + public List listResources() { + return new ArrayList<>(resourceMap.values()); + } + + public static ExternalServiceMock getInstance() { + return serviceMock; + } +} From 13ff73ebd9378fd625cdbe614302a7e0aa6719b5 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 22 Sep 2022 14:44:08 +0200 Subject: [PATCH 36/48] external resource IT --- .../BulkDependentDeleterIT.java | 2 +- .../BulkDependentTestBase.java | 17 +++++- .../BulkExternalDependentIT.java | 56 +++++++++++++++++++ .../ManagedBulkDependentIT.java | 2 +- .../StandaloneBulkDependentIT.java | 2 +- .../ExternalBulkDependentResource.java | 35 ++++++------ .../ExternalBulkResourceReconciler.java | 19 +++++++ 7 files changed, 109 insertions(+), 24 deletions(-) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/{ => bulkdependent}/BulkDependentDeleterIT.java (91%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/{ => bulkdependent}/BulkDependentTestBase.java (86%) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkExternalDependentIT.java rename operator-framework/src/test/java/io/javaoperatorsdk/operator/{ => bulkdependent}/ManagedBulkDependentIT.java (91%) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/{ => bulkdependent}/StandaloneBulkDependentIT.java (91%) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkResourceReconciler.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentDeleterIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkDependentDeleterIT.java similarity index 91% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentDeleterIT.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkDependentDeleterIT.java index dce6df8b98..a934bdd1f3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentDeleterIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkDependentDeleterIT.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator; +package io.javaoperatorsdk.operator.bulkdependent; import org.junit.jupiter.api.extension.RegisterExtension; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkDependentTestBase.java similarity index 86% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkDependentTestBase.java index 0a5cfee161..605731623c 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/BulkDependentTestBase.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkDependentTestBase.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator; +package io.javaoperatorsdk.operator.bulkdependent; import java.time.Duration; @@ -75,7 +75,7 @@ private void assertAdditionalDataOnConfigMaps(String expectedValue) { }); } - private BulkDependentTestCustomResource testResource() { + public static BulkDependentTestCustomResource testResource() { BulkDependentTestCustomResource cr = new BulkDependentTestCustomResource(); cr.setMetadata(new ObjectMeta()); cr.getMetadata().setName(TEST_RESOURCE_NAME); @@ -91,11 +91,24 @@ private void updateSpecWithNewAdditionalData(String data) { extension().replace(resource); } + public static void updateSpecWithNewAdditionalData(LocallyRunOperatorExtension extension, + String data) { + var resource = testResource(); + resource.getSpec().setAdditionalData(data); + extension.replace(resource); + } + private void updateSpecWithNumber(int n) { var resource = testResource(); resource.getSpec().setNumberOfResources(n); extension().replace(resource); } + public static void updateSpecWithNumber(LocallyRunOperatorExtension extension, int n) { + var resource = testResource(); + resource.getSpec().setNumberOfResources(n); + extension.replace(resource); + } + abstract LocallyRunOperatorExtension extension(); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkExternalDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkExternalDependentIT.java new file mode 100644 index 0000000000..29f66e8205 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/BulkExternalDependentIT.java @@ -0,0 +1,56 @@ +package io.javaoperatorsdk.operator.bulkdependent; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.bulkdependent.external.ExternalBulkResourceReconciler; +import io.javaoperatorsdk.operator.sample.bulkdependent.external.ExternalServiceMock; + +import static io.javaoperatorsdk.operator.bulkdependent.BulkDependentTestBase.*; +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class BulkExternalDependentIT { + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder().withReconciler(new ExternalBulkResourceReconciler()) + .build(); + + ExternalServiceMock externalServiceMock = ExternalServiceMock.getInstance(); + + @Test + void managesExternalBulkResources() { + extension.create(testResource()); + assertResourceNumberAndData(3, INITIAL_ADDITIONAL_DATA); + + updateSpecWithNumber(extension, 1); + assertResourceNumberAndData(1, INITIAL_ADDITIONAL_DATA); + + updateSpecWithNumber(extension, 5); + assertResourceNumberAndData(5, INITIAL_ADDITIONAL_DATA); + + extension.delete(testResource()); + assertResourceNumberAndData(0, INITIAL_ADDITIONAL_DATA); + } + + + @Test + void handlesResourceUpdates() { + extension.create(testResource()); + assertResourceNumberAndData(3, INITIAL_ADDITIONAL_DATA); + + updateSpecWithNewAdditionalData(extension, NEW_VERSION_OF_ADDITIONAL_DATA); + assertResourceNumberAndData(3, NEW_VERSION_OF_ADDITIONAL_DATA); + } + + private void assertResourceNumberAndData(int n, String data) { + await().untilAsserted(() -> { + var resources = externalServiceMock.listResources(); + assertThat(resources).hasSize(n); + assertThat(resources).allMatch(r -> r.getData().equals(data)); + }); + } + +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ManagedBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/ManagedBulkDependentIT.java similarity index 91% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/ManagedBulkDependentIT.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/ManagedBulkDependentIT.java index 4c38714dbe..7f074ac8f5 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ManagedBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/ManagedBulkDependentIT.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator; +package io.javaoperatorsdk.operator.bulkdependent; import org.junit.jupiter.api.extension.RegisterExtension; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/StandaloneBulkDependentIT.java similarity index 91% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/StandaloneBulkDependentIT.java index b0f999f2a4..683cc1662b 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/StandaloneBulkDependentIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/bulkdependent/StandaloneBulkDependentIT.java @@ -1,4 +1,4 @@ -package io.javaoperatorsdk.operator; +package io.javaoperatorsdk.operator.bulkdependent; import org.junit.jupiter.api.extension.RegisterExtension; 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 fc95bebdb7..0638c658c2 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,16 +1,12 @@ package io.javaoperatorsdk.operator.sample.bulkdependent.external; -import java.util.HashMap; -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; import io.javaoperatorsdk.operator.processing.dependent.*; import io.javaoperatorsdk.operator.processing.dependent.external.PollingDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; -import io.javaoperatorsdk.operator.processing.event.source.CacheKeyMapper; import io.javaoperatorsdk.operator.sample.bulkdependent.BulkDependentTestCustomResource; public class ExternalBulkDependentResource @@ -23,15 +19,18 @@ public class ExternalBulkDependentResource private ExternalServiceMock externalServiceMock = ExternalServiceMock.getInstance(); public ExternalBulkDependentResource() { - super(ExternalResource.class, CacheKeyMapper.singleResourceCacheKeyMapper()); + super(ExternalResource.class, ExternalResource::getId); } @Override public Map> fetchResources() { - // todo Map> result = new HashMap<>(); var resources = externalServiceMock.listResources(); - + resources.stream().forEach(er -> { + var resourceID = toResourceID(er); + result.putIfAbsent(resourceID, new HashSet<>()); + result.get(resourceID).add(er); + }); return result; } @@ -56,24 +55,17 @@ public void deleteBulkResourceWithIndex(BulkDependentTestCustomResource primary, @Override public BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory() { return index -> (resource, primary, context) -> { - ExternalResource collect = context.getSecondaryResources(resource).stream() + return context.getSecondaryResources(resource).stream() .filter(r -> r.getId().endsWith(EXTERNAL_RESOURCE_NAME_DELIMITER + index)) - .collect(Collectors.collectingAndThen( - Collectors.toList(), - list -> { - if (list.size() > 1) { - throw new IllegalStateException("Found more than 1 object: " + list); - } - return list.get(0); - })); - return Optional.ofNullable(collect); + .collect(Collectors.toList()).stream().findFirst(); }; } @Override public ExternalResource desired(BulkDependentTestCustomResource primary, int index, Context context) { - return new ExternalResource(toExternalResourceId(primary, index), "" + index); + return new ExternalResource(toExternalResourceId(primary, index), + primary.getSpec().getAdditionalData()); } @Override @@ -101,4 +93,9 @@ private static String toExternalResourceId(BulkDependentTestCustomResource prima 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]); + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkResourceReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkResourceReconciler.java new file mode 100644 index 0000000000..2543422d74 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/bulkdependent/external/ExternalBulkResourceReconciler.java @@ -0,0 +1,19 @@ +package io.javaoperatorsdk.operator.sample.bulkdependent.external; + +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; +import io.javaoperatorsdk.operator.sample.bulkdependent.BulkDependentTestCustomResource; + +@ControllerConfiguration(dependents = @Dependent(type = ExternalBulkDependentResource.class)) +public class ExternalBulkResourceReconciler implements Reconciler { + + @Override + public UpdateControl reconcile( + BulkDependentTestCustomResource resource, Context context) + throws Exception { + return UpdateControl.noUpdate(); + } +} From 6984571ce74d1f5dcae2f5f2aa5d75987d60843f Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 22 Sep 2022 14:55:47 +0200 Subject: [PATCH 37/48] docs --- .../dependent/AbstractDependentResource.java | 12 ++---------- .../operator/processing/dependent/BulkUpdater.java | 7 +++++++ 2 files changed, 9 insertions(+), 10 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 af5f1e02e6..accc1a24fe 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 @@ -82,7 +82,8 @@ private void adjustDiscriminators(int count) { } protected ReconcileResult reconcileIndexAware(P primary, int i, Context

context) { - Optional maybeActual = getSecondaryResourceIndexAware(primary, i, context); + Optional maybeActual = bulk ? getSecondaryResourceIndexAware(primary, i, context) + : getSecondaryResource(primary, context); if (creatable || updatable) { if (maybeActual.isEmpty()) { if (creatable) { @@ -126,21 +127,12 @@ private R desiredIndexAware(P primary, int i, Context

context) { : desired(primary, context); } - // todo check 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) { - if (index > 0 && resourceDiscriminator.isEmpty()) { - throw new IllegalStateException( - "Handling resources in bulk bot no resource discriminators set."); - } - if (!bulk) { - return getSecondaryResource(primary, context); - } - return context.getSecondaryResource(resourceType(), resourceDiscriminator.get(index)); } 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 d5ad2957d2..9c00b47d0c 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 @@ -3,6 +3,13 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.reconciler.Context; +/** + * Helper for the Bulk Dependent Resources to make it more explicit that bulk needs to only + * implement the index aware match method. + * + * @param secondary resource type + * @param

primary resource type + */ public interface BulkUpdater extends Updater { default Matcher.Result match(R actualResource, P primary, Context

context) { From d5e814d44df449e07f5ded4e9f134b29df845fac Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 27 Sep 2022 16:36:18 +0200 Subject: [PATCH 38/48] refactor: clean-up --- .../api/config/AnnotationControllerConfiguration.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index b40631c603..dbd09a32cc 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -45,14 +45,13 @@ public class AnnotationControllerConfiguration

private static final String KUBE_DEPENDENT_NAME = KubernetesDependent.class.getSimpleName(); protected final Reconciler

reconciler; - private final io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration annotation; + private final ControllerConfiguration annotation; private List specs; private Class

resourceClass; public AnnotationControllerConfiguration(Reconciler

reconciler) { this.reconciler = reconciler; - this.annotation = reconciler.getClass() - .getAnnotation(io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration.class); + this.annotation = reconciler.getClass().getAnnotation(ControllerConfiguration.class); if (annotation == null) { throw new OperatorException("Missing mandatory @" + CONTROLLER_CONFIG_ANNOTATION + " annotation for reconciler: " + reconciler); From 4e44a5054f48547a75b09d584ce5a235b758eddc Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 27 Sep 2022 17:04:45 +0200 Subject: [PATCH 39/48] refactor: make ReconcileResult immutable --- .../reconciler/dependent/ReconcileResult.java | 33 ++++++++++++------- .../dependent/AbstractDependentResource.java | 16 +++++---- 2 files changed, 31 insertions(+), 18 deletions(-) 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 65b6e8adfd..2b5905b214 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 @@ -8,9 +8,7 @@ public class ReconcileResult { - private Map resourceOperations = new HashMap<>(1); - - public ReconcileResult() {} + private final Map resourceOperations; public static ReconcileResult resourceCreated(T resource) { return new ReconcileResult<>(resource, Operation.CREATED); @@ -24,6 +22,21 @@ public static ReconcileResult noOperation(T resource) { return new ReconcileResult<>(resource, Operation.NONE); } + @SafeVarargs + public static ReconcileResult aggregatedResult(ReconcileResult... results) { + if (results == null) { + throw new IllegalArgumentException("Should provide results to aggregate"); + } + if (results.length == 1) { + return results[0]; + } + final Map operations = new HashMap<>(results.length); + for (ReconcileResult res : results) { + res.getSingleResource().ifPresent(r -> operations.put(r, res.getSingleOperation())); + } + return new ReconcileResult<>(operations); + } + @Override public String toString() { return resourceOperations.entrySet().stream().collect(Collectors.toMap( @@ -33,7 +46,11 @@ public String toString() { } private ReconcileResult(R resource, Operation operation) { - resourceOperations.put(resource, operation); + resourceOperations = Map.of(resource, operation); + } + + private ReconcileResult(Map operations) { + resourceOperations = Collections.unmodifiableMap(operations); } public Optional getSingleResource() { @@ -45,17 +62,11 @@ public Operation getSingleOperation() { .orElseThrow(); } + @SuppressWarnings("unused") public Map getResourceOperations() { return resourceOperations; } - public void addReconcileResult(ReconcileResult result) { - result.getSingleResource().ifPresent(r -> { - resourceOperations.put(r, result.getSingleOperation()); - }); - - } - public enum Operation { CREATED, UPDATED, NONE } 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 accc1a24fe..b2f92d55a6 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 @@ -40,17 +40,19 @@ public AbstractDependentResource() { @Override public ReconcileResult reconcile(P primary, Context

context) { - var count = bulk ? bulkDependentResource.count(primary, context) : 1; if (bulk) { + final var count = bulkDependentResource.count(primary, context); deleteBulkResourcesIfRequired(count, lastKnownBulkSize(), primary, context); adjustDiscriminators(count); + @SuppressWarnings("unchecked") + final ReconcileResult[] results = new ReconcileResult[count]; + for (int i = 0; i < count; i++) { + results[i] = reconcileIndexAware(primary, i, context); + } + return ReconcileResult.aggregatedResult(results); + } else { + return reconcileIndexAware(primary, 0, context); } - ReconcileResult result = new ReconcileResult<>(); - for (int i = 0; i < count; i++) { - var res = reconcileIndexAware(primary, i, context); - result.addReconcileResult(res); - } - return result; } protected void deleteBulkResourcesIfRequired(int targetCount, int actualCount, P primary, From 408fc9c62a54bc45efd197c5ff389f1066ec2141 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 27 Sep 2022 18:58:46 +0200 Subject: [PATCH 40/48] fix: do not create map if resource is null --- .../operator/api/reconciler/dependent/ReconcileResult.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 2b5905b214..468e14e8ea 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 @@ -46,7 +46,7 @@ public String toString() { } private ReconcileResult(R resource, Operation operation) { - resourceOperations = Map.of(resource, operation); + resourceOperations = resource != null ? Map.of(resource, operation) : Collections.emptyMap(); } private ReconcileResult(Map operations) { From cabe07dd6d46a811150e0019f9919df712eaa0f4 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 28 Sep 2022 11:33:33 +0200 Subject: [PATCH 41/48] fix: make things work again after rebase --- .../operator/api/reconciler/DefaultContext.java | 6 ------ .../dependent/kubernetes/KubernetesDependentResource.java | 3 +-- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java index 9b6099238e..cb7f4ae63b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/DefaultContext.java @@ -54,12 +54,6 @@ public Optional getSecondaryResource(Class expectedType, return discriminator.distinguish(expectedType, primaryResource, this); } - @Override - public Optional getSecondaryResource(Class expectedType, - ResourceDiscriminator discriminator) { - return discriminator.distinguish(expectedType, primaryResource, this); - } - @Override public ControllerConfiguration

getControllerConfiguration() { return controllerConfiguration; 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 1e2fbf2df0..3738a2e7d2 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 @@ -162,8 +162,7 @@ protected Resource prepare(R desired, P primary, String actionName) { } else if (useDefaultAnnotationsToIdentifyPrimary()) { addDefaultSecondaryToPrimaryMapperAnnotations(desired, primary); } - return client.resource(desired).inNamespace(desired.getMetadata().getNamespace()) - .resource(desired); + return client.resource(desired).inNamespace(desired.getMetadata().getNamespace()); } @Override From 72ebce3dd6a6593b0f7e515b3a2cf13e0f828340 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 28 Sep 2022 12:01:15 +0200 Subject: [PATCH 42/48] refactor: simplify by removing discriminator factory --- .../dependent/AbstractDependentResource.java | 5 ++- .../dependent/BulkDependentResource.java | 7 ++-- .../BulkResourceDiscriminatorFactory.java | 10 ------ ...ConfigMapDeleterBulkDependentResource.java | 32 +++++++++---------- .../ExternalBulkDependentResource.java | 22 ++++++------- 5 files changed, 30 insertions(+), 46 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkResourceDiscriminatorFactory.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 b2f92d55a6..078fc60b66 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 @@ -28,7 +28,7 @@ public abstract class AbstractDependentResource protected Updater updater; protected BulkDependentResource bulkDependentResource; - protected List> resourceDiscriminator = new ArrayList<>(1); + private final List> resourceDiscriminator = new ArrayList<>(1); @SuppressWarnings("unchecked") public AbstractDependentResource() { @@ -74,8 +74,7 @@ private void adjustDiscriminators(int count) { } if (resourceDiscriminator.size() < count) { for (int i = resourceDiscriminator.size(); i < count; i++) { - resourceDiscriminator.add(bulkDependentResource.bulkResourceDiscriminatorFactory() - .createResourceDiscriminator(i)); + resourceDiscriminator.add(bulkDependentResource.getResourceDiscriminator(i)); } } if (resourceDiscriminator.size() > count) { 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 c89af4b481..1f2688f5cb 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 @@ -2,6 +2,7 @@ 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; /** @@ -29,10 +30,6 @@ public interface BulkDependentResource extends Creator */ void deleteBulkResourceWithIndex(P primary, R resource, int i, Context

context); - /** - * @return a discriminator factor that helps to create a discriminator for a certain resource with - * an index - */ - BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory(); + ResourceDiscriminator getResourceDiscriminator(int index); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkResourceDiscriminatorFactory.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkResourceDiscriminatorFactory.java deleted file mode 100644 index 8b9b6e968d..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkResourceDiscriminatorFactory.java +++ /dev/null @@ -1,10 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent; - -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.reconciler.ResourceDiscriminator; - -public interface BulkResourceDiscriminatorFactory { - - ResourceDiscriminator createResourceDiscriminator(int index); - -} 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 eab06ac61d..a7fbd9cb98 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,12 +7,11 @@ 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.BulkResourceDiscriminatorFactory; import io.javaoperatorsdk.operator.processing.dependent.Creator; import io.javaoperatorsdk.operator.processing.dependent.Updater; -import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; /** @@ -29,19 +28,6 @@ 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"; - private BulkResourceDiscriminatorFactory factory = - index -> (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 ConfigMapDeleterBulkDependentResource() { super(ConfigMap.class); @@ -68,7 +54,19 @@ public int count(BulkDependentTestCustomResource primary, } @Override - public BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory() { - return factory; + 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)); + } + }; } } 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 0638c658c2..110626a923 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 @@ -4,6 +4,7 @@ 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.external.PollingDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -16,7 +17,7 @@ public class ExternalBulkDependentResource public static final String EXTERNAL_RESOURCE_NAME_DELIMITER = "#"; - private ExternalServiceMock externalServiceMock = ExternalServiceMock.getInstance(); + private final ExternalServiceMock externalServiceMock = ExternalServiceMock.getInstance(); public ExternalBulkDependentResource() { super(ExternalResource.class, ExternalResource::getId); @@ -26,7 +27,7 @@ public ExternalBulkDependentResource() { public Map> fetchResources() { Map> result = new HashMap<>(); var resources = externalServiceMock.listResources(); - resources.stream().forEach(er -> { + resources.forEach(er -> { var resourceID = toResourceID(er); result.putIfAbsent(resourceID, new HashSet<>()); result.get(resourceID).add(er); @@ -52,15 +53,6 @@ public void deleteBulkResourceWithIndex(BulkDependentTestCustomResource primary, externalServiceMock.delete(resource.getId()); } - @Override - public BulkResourceDiscriminatorFactory bulkResourceDiscriminatorFactory() { - return index -> (resource, primary, context) -> { - return context.getSecondaryResources(resource).stream() - .filter(r -> r.getId().endsWith(EXTERNAL_RESOURCE_NAME_DELIMITER + index)) - .collect(Collectors.toList()).stream().findFirst(); - }; - } - @Override public ExternalResource desired(BulkDependentTestCustomResource primary, int index, Context context) { @@ -98,4 +90,12 @@ private ResourceID toResourceID(ExternalResource externalResource) { var parts = externalResource.getId().split(EXTERNAL_RESOURCE_NAME_DELIMITER); return new ResourceID(parts[0], parts[1]); } + + @Override + public ResourceDiscriminator getResourceDiscriminator( + int index) { + return (resource, primary, context) -> context.getSecondaryResources(resource).stream() + .filter(r -> r.getId().endsWith(EXTERNAL_RESOURCE_NAME_DELIMITER + index)) + .collect(Collectors.toList()).stream().findFirst(); + } } From b5f982fd250361a1dfb11dd9db2a55cad0f1933c Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 28 Sep 2022 16:05:26 +0200 Subject: [PATCH 43/48] fix: properly use indexed version --- .../dependent/kubernetes/GenericKubernetesResourceMatcher.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 56b5bc7a59..bb066b5b24 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 @@ -35,7 +35,7 @@ public Result match(R actualResource, P primary, Context

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

context) { - final var desired = dependentResource.desired(primary, context); + final var desired = dependentResource.desired(primary, index, context); return Result.computed( ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource), desired); From da8163b0d02d1c8ca847ea5f7c856b2aaeaa864b Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 29 Sep 2022 15:06:31 +0200 Subject: [PATCH 44/48] revert strange comment --- docs/assets/js/uikit.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/assets/js/uikit.js b/docs/assets/js/uikit.js index 665c8c0522..5b1acf9275 100644 --- a/docs/assets/js/uikit.js +++ b/docs/assets/js/uikit.js @@ -8214,7 +8214,7 @@ updateAria: function(toggled) { attr(this.$el, 'aria-expanded', isBoolean(toggled) - ? toggled// todo delete bulk support + ? toggled : isToggled(this.target, this.cls) ); } From 1d303a0d5dd34d9b8412588505bc9d4142324da7 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Mon, 29 Aug 2022 20:16:38 +0200 Subject: [PATCH 45/48] fix: make it clearer when CRD file isn't found --- .../operator/junit/LocallyRunOperatorExtension.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java index e2f4234453..fdd158275f 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java @@ -151,6 +151,9 @@ protected void before(ExtensionContext context) { private void applyCrd(String resourceTypeName) { String path = "/META-INF/fabric8/" + resourceTypeName + "-v1.yml"; try (InputStream is = getClass().getResourceAsStream(path)) { + if (is == null) { + throw new IllegalStateException("Cannot find CRD at " + path); + } final var crd = getKubernetesClient().load(is); crd.createOrReplace(); Thread.sleep(CRD_READY_WAIT); // readiness is not applicable for CRD, just wait a little From e2e6b18537847bbc4ebaf828fd460859e4c225e8 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 29 Sep 2022 14:48:53 +0200 Subject: [PATCH 46/48] refactor: isolate index handling to BulkDependentResource interface --- .../dependent/AbstractDependentResource.java | 93 ++++++++----------- .../dependent/BulkDependentResource.java | 21 ++++- .../processing/dependent/BulkUpdater.java | 10 +- .../dependent/DesiredEqualsMatcher.java | 6 -- .../processing/dependent/Matcher.java | 15 --- .../processing/dependent/Updater.java | 4 - .../GenericKubernetesResourceMatcher.java | 51 +++------- .../KubernetesDependentResource.java | 16 ++-- ...ConfigMapDeleterBulkDependentResource.java | 27 +++--- .../ExternalBulkDependentResource.java | 19 ++-- 10 files changed, 106 insertions(+), 156 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 078fc60b66..6e937268d9 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; - - private final List> resourceDiscriminator = new ArrayList<>(1); + private final BulkDependentResource bulkDependentResource; + 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..359a434a39 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 @@ -24,42 +24,19 @@ private GenericKubernetesResourceMatcher(KubernetesDependentResource depen 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); - } + return (actualResource, primary, context) -> { + final var desired = dependentResource.desired(primary, 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); - } + return (actualResource, primary, context) -> { + final var desired = dependentResource.desired(primary, context); + return Result.computed( + ResourceComparators.compareConfigMapData((ConfigMap) desired, + (ConfigMap) actualResource), + desired); }; } else { return new GenericKubernetesResourceMatcher(dependentResource); @@ -72,12 +49,6 @@ 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) { if (considerMetadata) { 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 3738a2e7d2..44897dbf06 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 @@ -135,23 +135,19 @@ 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) { log.debug("{} target resource with type: {}, with id: {}", actionName, @@ -192,7 +188,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(); } From d30b9f7ebbfad75b1f44f316fc4a716afc9a06e0 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 29 Sep 2022 15:54:15 +0200 Subject: [PATCH 47/48] Revert "refactor: isolate index handling to BulkDependentResource interface" This reverts commit e2e6b18537847bbc4ebaf828fd460859e4c225e8. --- .../dependent/AbstractDependentResource.java | 93 +++++++++++-------- .../dependent/BulkDependentResource.java | 21 +---- .../processing/dependent/BulkUpdater.java | 10 +- .../dependent/DesiredEqualsMatcher.java | 6 ++ .../processing/dependent/Matcher.java | 15 +++ .../processing/dependent/Updater.java | 4 + .../GenericKubernetesResourceMatcher.java | 51 +++++++--- .../KubernetesDependentResource.java | 16 ++-- ...ConfigMapDeleterBulkDependentResource.java | 27 +++--- .../ExternalBulkDependentResource.java | 19 ++-- 10 files changed, 156 insertions(+), 106 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 6e937268d9..078fc60b66 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,5 +1,7 @@ package io.javaoperatorsdk.operator.processing.dependent; +import java.util.ArrayList; +import java.util.List; import java.util.Optional; import org.slf4j.Logger; @@ -18,15 +20,15 @@ public abstract class AbstractDependentResource implements DependentResource { private static final Logger log = LoggerFactory.getLogger(AbstractDependentResource.class); - private final boolean creatable = this instanceof Creator; - private final boolean updatable = this instanceof Updater; - private final boolean bulk = this instanceof BulkDependentResource; + protected final boolean creatable = this instanceof Creator; + protected final boolean updatable = this instanceof Updater; + protected final boolean bulk = this instanceof BulkDependentResource; protected Creator creator; protected Updater updater; - private final BulkDependentResource bulkDependentResource; - private ResourceDiscriminator resourceDiscriminator; - private int currentCount; + protected BulkDependentResource bulkDependentResource; + + private final List> resourceDiscriminator = new ArrayList<>(1); @SuppressWarnings("unchecked") public AbstractDependentResource() { @@ -40,33 +42,48 @@ public AbstractDependentResource() { public ReconcileResult reconcile(P primary, Context

context) { if (bulk) { final var count = bulkDependentResource.count(primary, context); - deleteBulkResourcesIfRequired(count, primary, context); + deleteBulkResourcesIfRequired(count, lastKnownBulkSize(), primary, context); + adjustDiscriminators(count); @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, P primary, Context

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

context) { + if (targetCount >= actualCount) { return; } - for (int i = targetCount; i < currentCount; i++) { - var resource = bulkDependentResource.getSecondaryResource(primary, i, context); + for (int i = targetCount; i < actualCount; i++) { + var resource = getSecondaryResourceIndexAware(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 ? bulkDependentResource.getSecondaryResource(primary, i, context) + Optional maybeActual = bulk ? getSecondaryResourceIndexAware(primary, i, context) : getSecondaryResource(primary, context); if (creatable || updatable) { if (maybeActual.isEmpty()) { @@ -82,7 +99,7 @@ protected ReconcileResult reconcileIndexAware(P primary, int i, Context

co if (updatable) { final Matcher.Result match; if (bulk) { - match = bulkDependentResource.match(actual, primary, i, context); + match = updater.match(actual, primary, i, context); } else { match = updater.match(actual, primary, context); } @@ -107,12 +124,17 @@ 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 == null ? context.getSecondaryResource(resourceType()) - : resourceDiscriminator.distinguish(resourceType(), primary, 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)); } private void throwIfNull(R desired, P primary, String descriptor) { @@ -173,35 +195,28 @@ 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"); - } - - public void delete(P primary, Context

context) { - if (bulk) { - deleteBulkResourcesIfRequired(0, primary, context); - } else { - handleDelete(primary, context); - } - } - - protected void handleDelete(P primary, Context

context) { - throw new IllegalStateException("delete method be implemented if Deleter trait is supported"); + throw new IllegalStateException( + "Must be implemented for bulk DependentResource creation"); } - public void setResourceDiscriminator( + public AbstractDependentResource setResourceDiscriminator( ResourceDiscriminator resourceDiscriminator) { - this.resourceDiscriminator = resourceDiscriminator; + if (resourceDiscriminator != null) { + this.resourceDiscriminator.add(resourceDiscriminator); + } + return this; } - protected boolean isCreatable() { - return creatable; + public ResourceDiscriminator getResourceDiscriminator() { + if (this.resourceDiscriminator.isEmpty()) { + return null; + } else { + return this.resourceDiscriminator.get(0); + } } - protected boolean isUpdatable() { - return updatable; + protected int lastKnownBulkSize() { + return resourceDiscriminator.size(); } - 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 64a174e201..1f2688f5cb 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,11 +1,9 @@ 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 @@ -32,21 +30,6 @@ public interface BulkDependentResource extends Creator */ void deleteBulkResourceWithIndex(P primary, R resource, int i, 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); - - Optional getSecondaryResource(P primary, int index, Context

context); + ResourceDiscriminator getResourceDiscriminator(int index); } 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 ee8f08a68d..9c00b47d0c 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,12 +13,8 @@ public interface BulkUpdater extends Updater { default Matcher.Result match(R actualResource, P primary, Context

context) { - 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"); + throw new IllegalStateException(); } + + 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 459d7951d6..1d3b34a47b 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,4 +16,10 @@ 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 750fe89cbf..835f76ab3a 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,4 +95,19 @@ 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 828f9ad785..06b3cb52f6 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,4 +8,8 @@ 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 359a434a39..bb066b5b24 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 @@ -24,19 +24,42 @@ private GenericKubernetesResourceMatcher(KubernetesDependentResource depen static Matcher matcherFor( Class resourceType, KubernetesDependentResource dependentResource) { if (Secret.class.isAssignableFrom(resourceType)) { - return (actualResource, primary, context) -> { - final var desired = dependentResource.desired(primary, context); - return Result.computed( - ResourceComparators.compareSecretData((Secret) desired, (Secret) actualResource), - desired); + 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 (actualResource, primary, context) -> { - final var desired = dependentResource.desired(primary, context); - return Result.computed( - ResourceComparators.compareConfigMapData((ConfigMap) desired, - (ConfigMap) actualResource), - desired); + 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); @@ -49,6 +72,12 @@ 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) { if (considerMetadata) { 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 44897dbf06..3738a2e7d2 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 @@ -135,19 +135,23 @@ public Result match(R actualResource, P primary, Context

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); + return matcher.match(actualResource, primary, index, context); } - protected void handleDelete(P primary, Context

context) { - var resource = getSecondaryResource(primary, context); - resource.ifPresent(r -> client.resource(r).delete()); + 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()); + } } 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) { log.debug("{} target resource with type: {}, with id: {}", actionName, @@ -188,7 +192,7 @@ protected InformerEventSource createEventSource(EventSourceContext

cont } private boolean useDefaultAnnotationsToIdentifyPrimary() { - return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && isCreatable(); + return !(this instanceof SecondaryToPrimaryMapper) && !garbageCollected && creatable; } 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 d8f85bb10a..a7fbd9cb98 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,6 +7,7 @@ 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; @@ -53,17 +54,19 @@ public int count(BulkDependentTestCustomResource primary, } @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)); - } + 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)); + } + }; } } 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..110626a923 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,16 +1,11 @@ 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; -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.api.reconciler.ResourceDiscriminator; +import io.javaoperatorsdk.operator.processing.dependent.*; import io.javaoperatorsdk.operator.processing.dependent.external.PollingDependentResource; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.sample.bulkdependent.BulkDependentTestCustomResource; @@ -43,7 +38,7 @@ public Map> fetchResources() { @Override public void delete(BulkDependentTestCustomResource primary, Context context) { - deleteBulkResourcesIfRequired(0, primary, context); + deleteBulkResourcesIfRequired(0, lastKnownBulkSize(), primary, context); } @Override @@ -97,9 +92,9 @@ private ResourceID toResourceID(ExternalResource externalResource) { } @Override - public Optional getSecondaryResource(BulkDependentTestCustomResource primary, - int index, Context context) { - return context.getSecondaryResources(resourceType()).stream() + public ResourceDiscriminator getResourceDiscriminator( + int index) { + return (resource, primary, context) -> context.getSecondaryResources(resource).stream() .filter(r -> r.getId().endsWith(EXTERNAL_RESOURCE_NAME_DELIMITER + index)) .collect(Collectors.toList()).stream().findFirst(); } From 71b07e965d11c9d1ce06b8563bae67219ae1ed5c Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 29 Sep 2022 16:11:43 +0200 Subject: [PATCH 48/48] feat: make it clearer when the CRD file isn't found (#1503) * feat: make it clearer when the CRD file isn't found * fix: only apply CRD when there is a CRD to apply --- .../operator/junit/LocallyRunOperatorExtension.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java index fdd158275f..f7627a5555 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java @@ -134,7 +134,10 @@ protected void before(ExtensionContext context) { ref.controllerConfigurationOverrider.accept(oconfig); } - applyCrd(config.getResourceTypeName()); + // only try to apply a CRD for the reconciler if it is associated to a CR + if (CustomResource.class.isAssignableFrom(config.getResourceClass())) { + applyCrd(config.getResourceTypeName()); + } if (ref.reconciler instanceof KubernetesClientAware) { ((KubernetesClientAware) ref.reconciler).setKubernetesClient(kubernetesClient);