From 5d2e3a325edd308b7ace3663ee605f60689110cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 31 Jul 2026 14:44:49 +0200 Subject: [PATCH 1/3] feat: pool informers so controllers and event sources can share them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every InformerEventSource used to create its own SharedIndexInformer, so an operator whose controllers all watch the same secondary type - ConfigMap and Secret being the usual suspects - opened one watch connection and kept one cache per controller for the very same resources. Informers are now handed out by an InformerPool obtained from the ConfigurationService, keyed by an InformerClassifier. Event sources whose classifiers are equal are backed by one informer; the pool reference counts its users and stops the informer once the last one releases it. The classifier is made up of everything that decides what an informer watches and how: the KubernetesClient instance (compared by identity, since two clients for the same API server may still differ in credentials, impersonation or TLS material), the resource class or the group/version/kind for generic resources, the namespace, the label, field and shard selectors, and the item store. Two components are deliberately not part of that identity. The informerListLimit is excluded, so event sources that disagree only on it still share an informer, keeping the limit of whichever one created it and logging a warning. Indexers are excluded because they can be added to a running informer: they are registered under a name qualified with the controller and event source that added them, so index names stay private to an event source while callers keep using their own names, and they are removed again when that event source releases the informer. Two strategies ship: DefaultInformerPool shares as described and is the default, NonSharingInformerPool creates a dedicated informer per event source for anyone wanting to opt out. Either is selected with ConfigurationServiceOverrider#withInformerPool, and a custom strategy extends AbstractInformerPool, which already creates the informers from a classifier, starts them and waits for their caches to sync, leaving the subclass only the question of whether and when an informer is shared. Consequently informer creation and startup moved out of InformerWrapper and InformerManager into the pool, InformerManager acquires and releases informers instead of owning them, and it removes its own event handler and indexers from an informer that keeps running for others. An event source registered dynamically against an already running shared informer needs no special handling: the client replays the cache contents to a newly added handler. Also in support of the above: ConfigurationService#informerPool, an InformerEventSource constructor that no longer needs an EventSourceContext (the one taking it is deprecated), the resource group/version/kind on InformerConfiguration, equality and toString on FieldSelector, and equality of GroupVersionKindPlural made consistent with its hashCode so that an unspecified plural no longer splits informers. The pooling itself is production ready; the configuration API around it is marked experimental and may still change. Covered by unit tests for the pools, the classifier, the wrapper and the manager, and by integration tests for sharing, dynamic registration and de-registration that each run against both strategies. Signed-off-by: Attila Mészáros --- .../content/en/docs/documentation/eventing.md | 68 +++- .../config/AbstractConfigurationService.java | 16 + .../api/config/ConfigurationService.java | 21 ++ .../config/ConfigurationServiceOverrider.java | 28 ++ .../api/config/informer/FieldSelector.java | 18 + .../informer/InformerConfiguration.java | 37 +- .../InformerEventSourceConfiguration.java | 3 +- .../operator/processing/GroupVersionKind.java | 4 + .../kubernetes/GroupVersionKindPlural.java | 9 +- .../processing/event/EventSourceManager.java | 6 +- .../controller/ControllerEventSource.java | 2 +- .../source/informer/InformerEventSource.java | 20 +- .../source/informer/InformerManager.java | 183 +++++----- .../source/informer/InformerWrapper.java | 166 ++++----- .../informer/ManagedInformerEventSource.java | 36 +- .../informer/pool/AbstractInformerPool.java | 201 +++++++++++ .../informer/pool/DefaultInformerPool.java | 133 ++++++++ .../informer/pool/InformerClassifier.java | 143 ++++++++ .../source/informer/pool/InformerPool.java | 98 ++++++ .../informer/pool/NonSharingInformerPool.java | 88 +++++ .../operator/MockKubernetesClient.java | 12 +- .../api/config/InformerConfigurationTest.java | 33 ++ .../operator/processing/ControllerTest.java | 12 +- .../processing/GroupVersionKindTest.java | 12 + .../event/EventSourceManagerTest.java | 11 +- .../event/ReconciliationDispatcherTest.java | 17 +- .../controller/ControllerEventSourceTest.java | 7 +- .../informer/InformerEventSourceTest.java | 27 +- .../InformerManagerConcurrentReleaseTest.java | 145 ++++++++ .../source/informer/InformerWrapperTest.java | 129 +++++++ ...nformerEventSourceNamespaceChangeTest.java | 132 ++++++++ ...edInformerEventSourceStartFailureTest.java | 155 +++++++++ .../pool/DefaultInformerPoolTest.java | 154 +++++++++ .../informer/pool/InformerClassifierTest.java | 315 ++++++++++++++++++ .../pool/NonSharingInformerPoolTest.java | 143 ++++++++ .../basic/AbstractSharedInformerIT.java | 107 ++++++ ...onSharingInformerPoolSharedInformerIT.java | 39 +++ .../basic/SharedInformerCustomResource1.java | 28 ++ .../basic/SharedInformerCustomResource2.java | 28 ++ .../informerpool/basic/SharedInformerIT.java | 38 +++ .../basic/SharedInformerReconciler1.java | 63 ++++ .../basic/SharedInformerReconciler2.java | 62 ++++ .../basic/SharedInformerStatus.java | 29 ++ .../AbstractDeregisterSharedInformerIT.java | 99 ++++++ .../DeregisterPrimaryCustomResource.java | 29 ++ .../deregister/DeregisterReconciler.java | 73 ++++ .../DeregisterSharedInformerIT.java | 29 ++ .../deregister/DeregisterSpec.java | 37 ++ .../DeregisterWatchedCustomResource.java | 32 ++ ...nformerPoolDeregisterSharedInformerIT.java | 34 ++ .../AbstractDynamicSharedInformerIT.java | 140 ++++++++ .../dynamic/DynamicSharedInformerIT.java | 37 ++ ...cSharedInformerPrimaryCustomResource1.java | 29 ++ ...cSharedInformerPrimaryCustomResource2.java | 29 ++ .../DynamicSharedInformerReconciler.java | 81 +++++ ...amicSharedInformerThirdCustomResource.java | 34 ++ ...ngInformerPoolDynamicSharedInformerIT.java | 41 +++ .../StaticSharedInformerReconciler.java | 73 ++++ .../StandaloneDependentResourceIT.java | 7 + 59 files changed, 3541 insertions(+), 241 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifier.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerPool.java create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapperTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceNamespaceChangeTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceStartFailureTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPoolTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifierTest.java create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPoolTest.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/AbstractSharedInformerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource1.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource2.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler1.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler2.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerStatus.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/AbstractDeregisterSharedInformerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSharedInformerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterWatchedCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/NonSharingInformerPoolDeregisterSharedInformerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/AbstractDynamicSharedInformerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerThirdCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/NonSharingInformerPoolDynamicSharedInformerIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/StaticSharedInformerReconciler.java diff --git a/docs/content/en/docs/documentation/eventing.md b/docs/content/en/docs/documentation/eventing.md index d2a104737b..340946d393 100644 --- a/docs/content/en/docs/documentation/eventing.md +++ b/docs/content/en/docs/documentation/eventing.md @@ -346,4 +346,70 @@ for [primary resources](https://github.com/operator-framework/java-operator-sdk/ See also [CaffeineBoundedItemStores](https://github.com/operator-framework/java-operator-sdk/blob/main/caffeine-bounded-cache-support/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/CaffeineBoundedItemStores.java) -for more details. \ No newline at end of file +for more details. + +### Sharing Informers Between Controllers (Informer Pool) + +{{% alert title="Experimental" color="warning" %}} +Informer pooling is marked `@Experimental`: the feature itself is production ready, but its +configuration API may still change in a non-backwards-compatible way. +{{% /alert %}} + +By default JOSDK maintains an *informer pool* so that informers are **shared** across controllers +and event sources. When several `InformerEventSource`s (whether belonging to different controllers, +or dynamically registered at runtime) watch the same resource type with an equivalent configuration, +they are all backed by a single underlying `SharedIndexInformer` instead of one informer each. This +reduces memory usage and the number of watch connections opened against the API server — which +matters in operators where many controllers watch the same secondary resource type (for example +`ConfigMap` or `Secret`). + +Two event sources share an informer when their effective informer configuration matches on all of: + +- the `KubernetesClient` they watch through, compared by instance: normally every event source + resolves the operator's own client, but an event source watching another cluster brings its own + (see [multi-cluster](#informereventsource-multi-cluster-support)). Two separate client instances + never share an informer, not even when they connect to the same API server — they may differ in + credentials, impersonation or TLS material, and the informer keeps using the client it was created + from, +- the resource type (or the group/version/kind for generic resources), +- the watched namespace, +- the label, field and shard selectors, +- the configured [item store](#bounded-caches-for-informers). + +The `informerListLimit` is intentionally *not* part of this identity: if two otherwise-equivalent +event sources request a different list limit, the existing informer is reused (a warning is logged +and the first-configured limit is kept). Indexers are also not part of the identity: they are +registered on the shared informer under a name qualified with the controller and event source that +added them, so index names are private to an event source and cannot collide with those of another +one. You keep looking indexes up by the name you registered, and the indexers of an event source are +removed from the shared informer when it stops using it. + +The pool is reference-counted: the shared informer is created on first use and only stopped once the +last event source using it is de-registered (or its controller stops). Dynamically registering an +event source for a resource that is already backed by a running informer reuses that informer, and +the initial state already in its cache is replayed to the newly added handler. + +#### Selecting the pooling strategy + +The strategy is provided by the +[`InformerPool`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerPool.java) +configured on the `ConfigurationService`. Two implementations are available: + +- [`DefaultInformerPool`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.java) + (the default): shares informers as described above. +- [`NonSharingInformerPool`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java): + never shares informers, creating a dedicated informer for every event source. Use this to opt out + of pooling and restore the pre-pooling behavior. + +You can override the strategy through the `ConfigurationService`: + +```java +Operator operator = new Operator(overrider -> + overrider.withInformerPool(new NonSharingInformerPool())); +``` + +A custom strategy has to extend +[`AbstractInformerPool`](https://github.com/operator-framework/java-operator-sdk/blob/main/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.java), +which is what `withInformerPool` accepts: it already creates the informers from an +`InformerClassifier` and starts them, leaving the subclass to decide only whether and how they are +shared. `InformerPool` itself is just the narrower contract that the event sources consume. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java index a1b37d6fe9..46be5c59c9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java @@ -24,6 +24,9 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.AbstractInformerPool; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.DefaultInformerPool; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerPool; /** * An abstract implementation of {@link ConfigurationService} meant to ease custom implementations @@ -35,6 +38,7 @@ public class AbstractConfigurationService implements ConfigurationService { private KubernetesClient client; private Cloner cloner; private ExecutorServiceManager executorServiceManager; + private AbstractInformerPool informerPool; protected AbstractConfigurationService(Version version) { this(version, null); @@ -190,4 +194,16 @@ public ExecutorServiceManager getExecutorServiceManager() { } return executorServiceManager; } + + @Override + public synchronized InformerPool informerPool() { + // cached so that all controllers backed by this ConfigurationService share the same pool and + // can therefore share the underlying informers; synchronized so concurrent first-access from + // multiple controllers cannot create (and share out) more than one pool instance + if (informerPool == null) { + informerPool = new DefaultInformerPool(); + informerPool.setConfigurationService(this); + } + return informerPool; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 6ed9b7ff64..c2c1ba1918 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -36,6 +36,7 @@ import io.fabric8.kubernetes.client.utils.KubernetesSerialization; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.Experimental; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; @@ -43,6 +44,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowFactory; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerPool; /** An interface from which to retrieve configuration information. */ public interface ConfigurationService { @@ -476,4 +478,23 @@ default boolean useSSAToPatchPrimaryResource() { default boolean cloneSecondaryResourcesWhenGettingFromCache() { return false; } + + /** + * The informer pool used to create and (when using the default, sharing pool) share the informers + * backing the event sources of all controllers managed by this {@code ConfigurationService}. + * + *

Implementations must return the same instance on every call. The pool is + * effectively a per-{@code ConfigurationService} singleton: controllers share informers only if + * they resolve the same pool, and reference counting / informer shutdown are only correct if + * {@code getInformer} and {@code releaseInformer} operate on that same instance. This is + * intentionally not a {@code default} method, since a {@code default} could not cache the result + * and would hand out a fresh (unshared) pool on each call; {@link AbstractConfigurationService} + * provides a cached implementation backed by the default sharing pool. + * + * @return the informer pool for this configuration service + */ + @Experimental( + "Only the configuration API around informer pooling could still change in a" + + " non-backwards-compatible way, the pooling itself is prod ready.") + InformerPool informerPool(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java index cd9cdafb39..e07cd336f1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java @@ -28,7 +28,10 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.Operator; import io.javaoperatorsdk.operator.api.monitoring.Metrics; +import io.javaoperatorsdk.operator.api.reconciler.Experimental; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.AbstractInformerPool; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerPool; @SuppressWarnings({"unused", "UnusedReturnValue"}) public class ConfigurationServiceOverrider { @@ -53,6 +56,7 @@ public class ConfigurationServiceOverrider { private Set> defaultNonSSAResource; private Boolean useSSAToPatchPrimaryResource; private Boolean cloneSecondaryResourcesWhenGettingFromCache; + private InformerPool informerPool; @SuppressWarnings("rawtypes") private DependentResourceFactory dependentResourceFactory; @@ -176,6 +180,21 @@ public ConfigurationServiceOverrider withCloneSecondaryResourcesWhenGettingFromC return this; } + /** + * Overrides the informer pool strategy used to create/share the informers backing the event + * sources. When not set, the default (informer-sharing) pool is used. + * + *

Custom strategies extend {@link AbstractInformerPool}, which already takes care of creating + * and starting the informers. + */ + @Experimental( + "Only the configuration API around informer pooling could still change in a" + + " non-backwards-compatible way, the pooling itself is prod ready.") + public ConfigurationServiceOverrider withInformerPool(AbstractInformerPool informerPool) { + this.informerPool = informerPool; + return this; + } + public ConfigurationService build() { return new BaseConfigurationService(original.getVersion(), cloner, client) { @Override @@ -309,6 +328,15 @@ public boolean cloneSecondaryResourcesWhenGettingFromCache() { cloneSecondaryResourcesWhenGettingFromCache, ConfigurationService::cloneSecondaryResourcesWhenGettingFromCache); } + + @Override + public InformerPool informerPool() { + if (informerPool == null) { + return super.informerPool(); + } + informerPool.setConfigurationService(this); + return informerPool; + } }; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.java index 022bb59ef0..1ee1e4e4a7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.java @@ -17,6 +17,7 @@ import java.util.Arrays; import java.util.List; +import java.util.Objects; public class FieldSelector { private final List fields; @@ -38,4 +39,21 @@ public Field(String path, String value) { this(path, value, false); } } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) return false; + FieldSelector that = (FieldSelector) o; + return Objects.equals(fields, that.fields); + } + + @Override + public int hashCode() { + return Objects.hashCode(fields); + } + + @Override + public String toString() { + return "FieldSelector{" + "fields=" + fields + '}'; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index 6c92dcdcc1..9fe25c999d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -30,6 +30,7 @@ import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.Utils; import io.javaoperatorsdk.operator.api.reconciler.Constants; +import io.javaoperatorsdk.operator.processing.GroupVersionKind; import io.javaoperatorsdk.operator.processing.event.source.cache.BoundedItemStore; import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter; @@ -42,6 +43,7 @@ public class InformerConfiguration { private final Builder builder = new Builder(); private final Class resourceClass; + private final GroupVersionKind resourceGroupVersionKind; private final String resourceTypeName; private String name; private Set namespaces; @@ -59,6 +61,7 @@ public class InformerConfiguration { protected InformerConfiguration( Class resourceClass, + GroupVersionKind resourceGroupVersionKind, String name, Set namespaces, boolean followControllerNamespaceChanges, @@ -74,7 +77,7 @@ protected InformerConfiguration( Boolean comparableResourceVersions, // TODO for removal in major release Duration ghostResourceCacheCheckInterval) { - this(resourceClass); + this(resourceClass, resourceGroupVersionKind); this.name = name; this.namespaces = namespaces; this.followControllerNamespaceChanges = followControllerNamespaceChanges; @@ -90,9 +93,14 @@ protected InformerConfiguration( this.comparableResourceVersions = comparableResourceVersions; } - private InformerConfiguration(Class resourceClass) { + private InformerConfiguration(Class resourceClass, GroupVersionKind resourceGroupVersionKind) { this.resourceClass = resourceClass; + this.resourceGroupVersionKind = resourceGroupVersionKind; this.resourceTypeName = + // note the direction: this is true for GenericKubernetesResource, but also when the + // resource + // class is a supertype of it - i.e. a plain HasMetadata, for which no type name can be + // resolved from @Group/@Version annotations resourceClass.isAssignableFrom(GenericKubernetesResource.class) // in general this is irrelevant now for secondary resources it is used just by // controller @@ -101,10 +109,16 @@ private InformerConfiguration(Class resourceClass) { : ReconcilerUtilsInternal.getResourceTypeName(resourceClass); } + @SuppressWarnings({"rawtypes", "unchecked"}) + public static InformerConfiguration.Builder builder( + Class resourceClass, GroupVersionKind groupVersionKind) { + return new InformerConfiguration(resourceClass, groupVersionKind).builder; + } + @SuppressWarnings({"rawtypes", "unchecked"}) public static InformerConfiguration.Builder builder( Class resourceClass) { - return new InformerConfiguration(resourceClass).builder; + return new InformerConfiguration(resourceClass, null).builder; } @SuppressWarnings({"rawtypes", "unchecked"}) @@ -112,6 +126,7 @@ public static InformerConfiguration.Builder builder( InformerConfiguration original) { return new InformerConfiguration( original.resourceClass, + original.resourceGroupVersionKind, original.name, original.namespaces, original.followControllerNamespaceChanges, @@ -305,6 +320,10 @@ public Long getInformerListLimit() { return informerListLimit; } + public GroupVersionKind getResourceGroupVersionKind() { + return resourceGroupVersionKind; + } + public FieldSelector getFieldSelector() { return fieldSelector; } @@ -500,10 +519,20 @@ public Builder withInformerListLimit(Long informerListLimit) { } public Builder withFieldSelector(FieldSelector fieldSelector) { - InformerConfiguration.this.fieldSelector = fieldSelector; + // an empty selector filters nothing, so it must not be distinguishable from having none at + // all: the informer pool keys on the field selector, and the annotation path always builds + // one (@Informer#fieldSelector defaults to {}) where the programmatic path leaves it null, + // which would otherwise stop the two from sharing an informer + InformerConfiguration.this.fieldSelector = isEmpty(fieldSelector) ? null : fieldSelector; return this; } + private static boolean isEmpty(FieldSelector fieldSelector) { + return fieldSelector == null + || fieldSelector.getFields() == null + || fieldSelector.getFields().isEmpty(); + } + public Builder withComparableResourceVersions(boolean comparableResourceVersions) { InformerConfiguration.this.comparableResourceVersions = comparableResourceVersions; return this; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java index ab1ad2b8eb..b6f7939728 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java @@ -76,6 +76,7 @@ default boolean followControllerNamespaceChanges() {

PrimaryToSecondaryMapper

getPrimaryToSecondaryMapper(); + // todo deprecate Optional getGroupVersionKind(); default String name() { @@ -167,7 +168,7 @@ private Builder( this.resourceClass = resourceClass; this.groupVersionKind = groupVersionKind; this.primaryResourceClass = primaryResourceClass; - this.config = InformerConfiguration.builder(resourceClass); + this.config = InformerConfiguration.builder(resourceClass, groupVersionKind); } public Builder withName(String name) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.java index be3869a64f..7d182cf1e6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.java @@ -136,4 +136,8 @@ public int hashCode() { public String toString() { return toGVKString(); } + + public String getApiVersion() { + return apiVersion; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GroupVersionKindPlural.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GroupVersionKindPlural.java index a3ed4d2d97..728673ad25 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GroupVersionKindPlural.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GroupVersionKindPlural.java @@ -53,10 +53,11 @@ protected GroupVersionKindPlural(GroupVersionKind gvk, String plural) { @Override protected boolean specificEquals(GroupVersionKind that) { - if (plural == null) { - return true; - } - return that instanceof GroupVersionKindPlural gvkp && gvkp.plural.equals(plural); + // a GroupVersionKind that is not plural-aware carries no plural form, which is the same as an + // unspecified one: that keeps this consistent with hashCode(), which only mixes the plural in + // when it is present + final var thatPlural = that instanceof GroupVersionKindPlural gvkp ? gvkp.plural : null; + return Objects.equals(plural, thatPlural); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 441d3cf178..b271a385e0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -148,7 +148,7 @@ private Void stopEventSource(EventSource eventSource) { return null; } - @SuppressWarnings("rawtypes") + @SuppressWarnings({"rawtypes", "unchecked"}) public final synchronized void registerEventSource(EventSource eventSource) throws OperatorException { Objects.requireNonNull(eventSource, "EventSource must not be null"); @@ -250,7 +250,9 @@ public EventSource dynamicallyRegisterEventSource(EventSource ev } } // The start itself is blocking thus blocking only the threads which are attempt to start the - // actual event source. Think of this as a form of lock striping. + // actual event source. Think of this as a form of lock striping. Note that two event sources + // backed by the same pooled informer may reach this concurrently; starting an already started + // informer is a no-op. eventSource.start(); return eventSource; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java index 2f624d1150..13d199bb59 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java @@ -48,7 +48,7 @@ public class ControllerEventSource @SuppressWarnings({"unchecked", "rawtypes"}) public ControllerEventSource(Controller controller) { - super(NAME, controller.getCRClient(), controller.getConfiguration()); + super(NAME, controller.getConfiguration()); this.controller = controller; final var config = controller.getConfiguration(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index b03a22e894..cb0fdaa8dd 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -24,8 +24,6 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.KubernetesClient; -import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; @@ -54,20 +52,18 @@ public class InformerEventSource private final PrimaryToSecondaryIndex primaryToSecondaryIndex; private final PrimaryToSecondaryMapper

primaryToSecondaryMapper; + /** + * @deprecated use {@link InformerEventSource(InformerEventSourceConfiguration)} + */ + // todo migrate sample, separate PR? + @Deprecated(forRemoval = true) public InformerEventSource( InformerEventSourceConfiguration configuration, EventSourceContext

context) { - this(configuration, configuration.getKubernetesClient().orElse(context.getClient())); + this(configuration); } - @SuppressWarnings({"unchecked", "rawtypes"}) - InformerEventSource(InformerEventSourceConfiguration configuration, KubernetesClient client) { - super( - configuration.name(), - configuration - .getGroupVersionKind() - .map(gvk -> client.genericKubernetesResources(gvk.apiVersion(), gvk.getKind())) - .orElseGet(() -> (MixedOperation) client.resources(configuration.getResourceClass())), - configuration); + public InformerEventSource(InformerEventSourceConfiguration configuration) { + super(configuration.name(), configuration); // If there is a primary to secondary mapper there is no need for primary to secondary index. primaryToSecondaryMapper = configuration.getPrimaryToSecondaryMapper(); if (useSecondaryToPrimaryIndex()) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 8e7054b231..edb8bf718a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -26,50 +26,46 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.api.model.KubernetesResourceList; -import io.fabric8.kubernetes.client.dsl.FilterWatchListDeletable; -import io.fabric8.kubernetes.client.dsl.MixedOperation; -import io.fabric8.kubernetes.client.dsl.Resource; +import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.Informable; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; -import io.javaoperatorsdk.operator.processing.LifecycleAware; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.Cache; import io.javaoperatorsdk.operator.processing.event.source.IndexerResourceCache; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerClassifier; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerPool; import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_ALL_NAMESPACES; class InformerManager> - implements LifecycleAware, IndexerResourceCache { + implements IndexerResourceCache { private static final Logger log = LoggerFactory.getLogger(InformerManager.class); private final Map> sources = new ConcurrentHashMap<>(); private final C configuration; - private final MixedOperation, Resource> client; private final ResourceEventHandler eventHandler; private final Map>> indexers = new HashMap<>(); private ControllerConfiguration controllerConfiguration; + private InformerPool informerPool; + private KubernetesClient targetClient; - InformerManager( - MixedOperation, Resource> client, - C configuration, - ResourceEventHandler eventHandler) { - this.client = client; + InformerManager(C configuration, ResourceEventHandler eventHandler) { this.configuration = configuration; this.eventHandler = eventHandler; } void setControllerConfiguration(ControllerConfiguration controllerConfiguration) { this.controllerConfiguration = controllerConfiguration; + this.informerPool = controllerConfiguration.getConfigurationService().informerPool(); } - @Override public void start() throws OperatorException { initSources(); // make sure informers are all started before proceeding further @@ -78,8 +74,8 @@ public void start() throws OperatorException { .getExecutorServiceManager() .boundedExecuteAndWaitForAllToComplete( sources.values().stream(), - iw -> { - iw.start(); + wrapper -> { + start(wrapper); return null; }, iw -> @@ -96,25 +92,26 @@ private void initSources() { final var targetNamespaces = configuration.getInformerConfig().getEffectiveNamespaces(controllerConfiguration); if (InformerConfiguration.allNamespacesWatched(targetNamespaces)) { - var source = createEventSourceForNamespace(WATCH_ALL_NAMESPACES); + var source = getEventSourceForNamespace(WATCH_ALL_NAMESPACES); log.debug("Registered {} -> {} for any namespace", this, source); } else { targetNamespaces.forEach( ns -> { - final var source = createEventSourceForNamespace(ns); + final var source = getEventSourceForNamespace(ns); log.debug("Registered {} -> {} for namespace: {}", this, source, ns); }); } } public void changeNamespaces(Set namespaces) { - var sourcesToRemove = - sources.keySet().stream().filter(k -> !namespaces.contains(k)).collect(Collectors.toSet()); - log.debug("Stopped informer {} for namespaces: {}", this, sourcesToRemove); - sourcesToRemove.forEach(k -> sources.remove(k).stop()); - - var newNamespaces = - namespaces.stream().filter(ns -> !sources.containsKey(ns)).collect(Collectors.toList()); + var namespacesToRemove = + sources.keySet().stream() + .filter(ns -> !namespaces.contains(ns)) + .collect(Collectors.toSet()); + log.debug("Stopped informer {} for namespaces: {}", this, namespacesToRemove); + namespacesToRemove.forEach(this::releaseSource); + + var newNamespaces = namespaces.stream().filter(ns -> !sources.containsKey(ns)).toList(); if (newNamespaces.isEmpty()) { return; } @@ -125,79 +122,107 @@ public void changeNamespaces(Set namespaces) { .boundedExecuteAndWaitForAllToComplete( newNamespaces.stream(), ns -> { - final var source = createEventSourceForNamespace(ns); - source.start(); + final var source = getEventSourceForNamespace(ns); + // block until the informer's cache is synced (or the sync timeout elapses) + start(source); log.debug("Registered new {} -> {} for namespace: {}", this, source, ns); return null; }, ns -> "InformerStarter-" + ns + "-" + configuration.getResourceClass().getSimpleName()); } - private InformerWrapper createEventSourceForNamespace(String namespace) { + private void start(InformerWrapper informerWrapper) { + informerPool.start(informerWrapper.getInformer(), informerWrapper.getClassifier()); + } + + private InformerWrapper getEventSourceForNamespace(String namespaceIdentifier) { final InformerWrapper source; - final var labelSelector = configuration.getInformerConfig().getLabelSelector(); - final var shardSelector = configuration.getInformerConfig().getShardSelector(); - if (namespace.equals(WATCH_ALL_NAMESPACES)) { - final var filteredBySelectorClient = - client.inAnyNamespace().withLabelSelector(labelSelector).withShardSelector(shardSelector); - source = createEventSource(filteredBySelectorClient, eventHandler, WATCH_ALL_NAMESPACES); - } else { - source = - createEventSource( - client - .inNamespace(namespace) - .withLabelSelector(labelSelector) - .withShardSelector(shardSelector), - eventHandler, - namespace); - } + InformerClassifier classifier = getClassifier(namespaceIdentifier); + var informer = + informerPool.getInformer( + controllerConfiguration.getName(), + configuration.getInformerConfig().getName(), + classifier); + source = + new InformerWrapper<>( + informer, + namespaceIdentifier, + classifier, + controllerConfiguration.getName(), + configuration.getInformerConfig().getName()); + sources.put(namespaceIdentifier, source); source.addIndexers(indexers); + source.addEventHandler(eventHandler); return source; } - private InformerWrapper createEventSource( - FilterWatchListDeletable, Resource> filteredBySelectorClient, - ResourceEventHandler eventHandler, - String namespaceIdentifier) { - final var informerConfig = configuration.getInformerConfig(); + private InformerClassifier getClassifier(String namespaceIdentifier) { + KubernetesClient targetClient = getTargetClient(); + + return new InformerClassifier<>( + targetClient, + configuration.getInformerConfig().getLabelSelector(), + configuration.getInformerConfig().getShardSelector(), + namespaceIdentifier, + configuration.getResourceClass(), + configuration.getInformerConfig().getResourceGroupVersionKind(), + configuration.getInformerConfig().getFieldSelector(), + configuration.getInformerConfig().getInformerListLimit(), + configuration.getInformerConfig().getItemStore()); + } - if (informerConfig.getFieldSelector() != null - && !informerConfig.getFieldSelector().getFields().isEmpty()) { - for (var f : informerConfig.getFieldSelector().getFields()) { - if (f.negated()) { - filteredBySelectorClient = filteredBySelectorClient.withoutField(f.path(), f.value()); - } else { - filteredBySelectorClient = filteredBySelectorClient.withField(f.path(), f.value()); + private KubernetesClient getTargetClient() { + // resolved once: the client is part of the informer classifier's identity, so every classifier + // this manager builds (one per watched namespace, and more when namespaces change later on) has + // to see the very same instance. ConfigurationService#getKubernetesClient is expected to return + // a stable instance, but its default implementation does create a new client on every call. + if (targetClient == null) { + targetClient = controllerConfiguration.getConfigurationService().getKubernetesClient(); + if (configuration instanceof InformerEventSourceConfiguration iesc) { + var remoteClient = iesc.getKubernetesClient().orElse(null); + if (remoteClient != null) { + targetClient = remoteClient; } } } - - var informer = - Optional.ofNullable(informerConfig.getInformerListLimit()) - .map(filteredBySelectorClient::withLimit) - .orElse(filteredBySelectorClient) - .runnableInformer(0); - Optional.ofNullable(informerConfig.getItemStore()).ifPresent(informer::itemStore); - var source = - new InformerWrapper<>( - informer, controllerConfiguration.getConfigurationService(), namespaceIdentifier); - source.addEventHandler(eventHandler); - sources.put(namespaceIdentifier, source); - return source; + return targetClient; } - @Override public void stop() { - sources.forEach( - (ns, source) -> { - try { - log.debug("Stopping informer for namespace: {} -> {}", ns, source); - source.stop(); - } catch (Exception e) { - log.warn("Error stopping informer for namespace: {} -> {}", ns, source, e); - } - }); - sources.clear(); + sources + .keySet() + .forEach( + ns -> { + try { + log.debug("Stopping informer for namespace: {}", ns); + releaseSource(ns); + } catch (Exception e) { + log.warn("Error stopping informer for namespace: {}", ns, e); + } + }); + } + + /** + * Gives the informer backing the given namespace back to the pool, but only if this manager still + * holds it: removing it from {@link #sources} is what claims the right to release it. {@link + * #stop()} and {@link #changeNamespaces(Set)} can run concurrently, and since the pool + * reference-counts its informers, releasing the same namespace twice would consume a reference + * another controller still holds and make the pool stop an informer that is still in use. + */ + private void releaseSource(String namespaceIdentifier) { + var wrapper = sources.remove(namespaceIdentifier); + if (wrapper == null) { + return; + } + // the informer may be shared, in which case it keeps running and would otherwise hold on to + // this event source's indexers + wrapper.removeIndexers(); + informerPool + .releaseInformer( + controllerConfiguration.getName(), + configuration.getInformerConfig().getName(), + wrapper.getClassifier()) + .ifPresent(i -> i.removeEventHandler(eventHandler)); } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java index 541068aa93..9548e8c540 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java @@ -15,12 +15,12 @@ */ package io.javaoperatorsdk.operator.processing.event.source.informer; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; @@ -30,125 +30,39 @@ import io.fabric8.kubernetes.api.model.GenericKubernetesResource; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.informers.ExceptionHandler; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.fabric8.kubernetes.client.informers.SharedIndexInformer; import io.fabric8.kubernetes.client.informers.cache.Cache; -import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.health.InformerHealthIndicator; import io.javaoperatorsdk.operator.health.Status; -import io.javaoperatorsdk.operator.processing.LifecycleAware; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.IndexerResourceCache; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerClassifier; class InformerWrapper - implements LifecycleAware, IndexerResourceCache, InformerHealthIndicator { + implements IndexerResourceCache, InformerHealthIndicator { private static final Logger log = LoggerFactory.getLogger(InformerWrapper.class); private final SharedIndexInformer informer; private final Cache cache; private final String namespaceIdentifier; - private final ConfigurationService configurationService; + private final InformerClassifier informerClassifier; + private final String indexNamePrefix; + private final Set registeredIndexNames = ConcurrentHashMap.newKeySet(); public InformerWrapper( SharedIndexInformer informer, - ConfigurationService configurationService, - String namespaceIdentifier) { + String namespaceIdentifier, + InformerClassifier classifier, + String controllerName, + String eventSourceName) { this.informer = informer; this.namespaceIdentifier = namespaceIdentifier; this.cache = (Cache) informer.getStore(); - this.configurationService = configurationService; - } - - @Override - public void start() throws OperatorException { - try { - - // register stopped handler if we have one defined - configurationService - .getInformerStoppedHandler() - .ifPresent( - ish -> { - final var stopped = informer.stopped(); - if (stopped != null) { - stopped.handle( - (res, ex) -> { - ish.onStop(informer, ex); - return null; - }); - } else { - final var apiTypeClass = informer.getApiTypeClass(); - final var fullResourceName = HasMetadata.getFullResourceName(apiTypeClass); - final var version = HasMetadata.getVersion(apiTypeClass); - throw new IllegalStateException( - "Cannot retrieve 'stopped' callback to listen to informer stopping for" - + " informer for " - + fullResourceName - + "/" - + version); - } - }); - if (!configurationService.stopOnInformerErrorDuringStartup()) { - informer.exceptionHandler((b, t) -> !ExceptionHandler.isDeserializationException(t)); - } - // change thread name for easier debugging - final var thread = Thread.currentThread(); - final var name = thread.getName(); - try { - thread.setName(informerInfo() + " " + thread.getId()); - final var resourceName = informer.getApiTypeClass().getSimpleName(); - log.debug( - "Starting informer for namespace: {} resource: {}", namespaceIdentifier, resourceName); - var start = informer.start(); - // note that in case we don't put here timeout and stopOnInformerErrorDuringStartup is - // false, and there is a rbac issue the get never returns; therefore operator never really - // starts - log.trace( - "Waiting informer to start namespace: {} resource: {}", - namespaceIdentifier, - resourceName); - start - .toCompletableFuture() - .get(configurationService.cacheSyncTimeout().toMillis(), TimeUnit.MILLISECONDS); - log.debug( - "Started informer for namespace: {} resource: {}", namespaceIdentifier, resourceName); - } catch (TimeoutException | ExecutionException e) { - if (configurationService.stopOnInformerErrorDuringStartup()) { - log.error("Informer startup error. Operator will be stopped. Informer: {}", informer, e); - throw new OperatorException(e); - } else { - log.warn("Informer startup error. Will periodically retry. Informer: {}", informer, e); - } - } catch (InterruptedException e) { - thread.interrupt(); - throw new IllegalStateException(e); - } finally { - // restore original name - thread.setName(name); - } - - } catch (Exception e) { - ReconcilerUtilsInternal.handleKubernetesClientException( - e, HasMetadata.getFullResourceName(informer.getApiTypeClass())); - throw new OperatorException( - "Couldn't start informer for " + versionedFullResourceName() + " resources", e); - } - } - - private String versionedFullResourceName() { - final var apiTypeClass = informer.getApiTypeClass(); - if (apiTypeClass.isAssignableFrom(GenericKubernetesResource.class)) { - return GenericKubernetesResource.class.getSimpleName(); - } - return ReconcilerUtilsInternal.getResourceTypeNameWithVersion(apiTypeClass); - } - - @Override - public void stop() throws OperatorException { - informer.stop(); + this.informerClassifier = classifier; + this.indexNamePrefix = "josdk/" + controllerName + "/" + eventSourceName + "/"; } @Override @@ -187,12 +101,42 @@ public void addEventHandler(ResourceEventHandler eventHandler) { @Override public void addIndexers(Map>> indexers) { - informer.getIndexer().addIndexers(indexers); + Map>> qualified = new HashMap<>(); + indexers.forEach((name, indexer) -> qualified.put(qualify(name), indexer)); + informer.getIndexer().addIndexers(qualified); + registeredIndexNames.addAll(qualified.keySet()); + } + + /** + * Removes the indexers this event source added, to be called when its informer is released. A + * shared informer outlives the event sources that stop using it, so without this its indexer + * would keep both the index and the (possibly capturing) index function of every event source + * that ever used it, and re-registering the same event source later would be rejected as a name + * conflict. + */ + void removeIndexers() { + registeredIndexNames.forEach(name -> informer.getIndexer().removeIndexer(name)); + registeredIndexNames.clear(); } @Override public List byIndex(String indexName, String indexKey) { - return informer.getIndexer().byIndex(indexName, indexKey); + return informer.getIndexer().byIndex(qualify(indexName), indexKey); + } + + /** + * The informer can be shared by event sources of several controllers, while its indexer is a + * single namespace of index names: two event sources registering the same index name on it would + * be rejected by the client, and one could read the other's index. Names are therefore qualified + * with the event source that registered them. + * + *

This stays invisible to callers, who keep using their own names, but only for as long as + * this class remains the only place that talks to {@link SharedIndexInformer#getIndexer()}: + * adding, reading and removing all have to go through here so that the qualification stays + * symmetric. + */ + private String qualify(String indexName) { + return indexNamePrefix + indexName; } @Override @@ -201,7 +145,15 @@ public String toString() { } private String informerInfo() { - return "InformerWrapper [" + versionedFullResourceName() + "]"; + return "InformerWrapper [ " + versionedFullResourceName() + " ]"; + } + + private String versionedFullResourceName() { + final var apiTypeClass = informer.getApiTypeClass(); + if (GenericKubernetesResource.class.isAssignableFrom(apiTypeClass)) { + return GenericKubernetesResource.class.getSimpleName(); + } + return ReconcilerUtilsInternal.getResourceTypeNameWithVersion(apiTypeClass); } @Override @@ -237,4 +189,12 @@ public Status getStatus() { public String getTargetNamespace() { return namespaceIdentifier; } + + public InformerClassifier getClassifier() { + return informerClassifier; + } + + public SharedIndexInformer getInformer() { + return informer; + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 8352bef665..5945936ee1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -32,7 +32,6 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; @@ -51,7 +50,6 @@ import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; -@SuppressWarnings("rawtypes") public abstract class ManagedInformerEventSource< R extends HasMetadata, P extends HasMetadata, C extends Informable> extends AbstractEventSource @@ -70,13 +68,11 @@ public abstract class ManagedInformerEventSource< private final C configuration; private final Map>> indexers = new HashMap<>(); protected TemporaryResourceCache temporaryResourceCache; - protected MixedOperation client; - protected ManagedInformerEventSource(String name, MixedOperation client, C configuration) { + protected ManagedInformerEventSource(String name, C configuration) { super(configuration.getResourceClass(), name); this.comparableResourceVersions = configuration.getInformerConfig().isComparableResourceVersions(); - this.client = client; this.configuration = configuration; } @@ -85,10 +81,14 @@ protected InformerManager manager() { } @Override - public void changeNamespaces(Set namespaces) { - if (allowsNamespaceChanges()) { - manager().changeNamespaces(namespaces); + public synchronized void changeNamespaces(Set namespaces) { + // a stopped event source has released its informers and its manager holds no sources, so every + // requested namespace would look new: it would acquire and start pooled informers that nothing + // can ever release, since stop() short-circuits on a non-running event source + if (!isRunning() || !allowsNamespaceChanges()) { + return; } + manager().changeNamespaces(namespaces); } /** @@ -159,17 +159,31 @@ protected abstract void handleEvent( Boolean deletedFinalStateUnknown, Set relatedPrimaryIDs); - @SuppressWarnings("unchecked") @Override public synchronized void start() { if (isRunning()) { return; } temporaryResourceCache = new TemporaryResourceCache<>(comparableResourceVersions, this); - this.cache = new InformerManager<>(client, configuration, this); + this.cache = new InformerManager<>(configuration, this); cache.setControllerConfiguration(controllerConfiguration); cache.addIndexers(indexers); - manager().start(); + // A dynamically registered event source may join an already-running shared informer whose cache + // is already populated. Those pre-existing resources are still delivered to this newly added + // handler: the underlying Fabric8 informer replays the current cache contents to every handler + // at registration time (see SharedProcessor#addProcessorListener). Replaying them here as well + // would deliver every pre-existing resource twice. + try { + manager().start(); + } catch (RuntimeException e) { + // The manager acquires a pooled informer for every watched namespace before any of them is + // started, so a startup failure has to hand those references back here: super.start() is not + // reached, which leaves isRunning() false and makes stop() skip the release entirely. The + // pooled informer would then be referenced forever (never stopped, even on a clean shutdown) + // and a retried start() would acquire it a second time. + manager().stop(); + throw e; + } super.start(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.java new file mode 100644 index 0000000000..81f8f979c1 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.java @@ -0,0 +1,201 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer.pool; + +import java.util.Optional; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.api.model.GenericKubernetesResource; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.dsl.FilterWatchListDeletable; +import io.fabric8.kubernetes.client.dsl.MixedOperation; +import io.fabric8.kubernetes.client.informers.ExceptionHandler; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.reconciler.Experimental; + +import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_ALL_NAMESPACES; +import static io.javaoperatorsdk.operator.api.reconciler.Experimental.API_MIGHT_CHANGE; + +/** + * Base class for the informer pool strategies, and the type the configuration API accepts (see + * {@link io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider#withInformerPool}), + * so custom strategies are expected to extend this rather than to implement {@link InformerPool} + * directly. + * + *

Creating an informer from an {@link InformerClassifier}, starting it and waiting for its cache + * to sync, and holding on to the injected {@link ConfigurationService} are handled here. Subclasses + * are left with the actual strategy: whether an informer is handed out to more than one event + * source and, consequently, when it is stopped. + */ +@Experimental(API_MIGHT_CHANGE) +public abstract class AbstractInformerPool implements InformerPool { + + private static final Logger log = LoggerFactory.getLogger(AbstractInformerPool.class); + + protected ConfigurationService configurationService; + + public ConfigurationService getConfigurationService() { + return configurationService; + } + + @Override + public void setConfigurationService(ConfigurationService configurationService) { + this.configurationService = configurationService; + } + + /** + * Number of distinct informers currently held in the pool for the given resource type. With a + * sharing pool multiple controllers watching the same resource are backed by a single informer + * (so this returns {@code 1}), whereas a non-sharing pool creates one informer per user. + */ + public abstract long numberOfInformersForResource(Class resourceClass); + + @SuppressWarnings({"rawtypes", "unchecked"}) + protected SharedIndexInformer createInformer(InformerClassifier classifier) { + var client = classifier.client(); + + MixedOperation clientWithResource; + if (classifier.groupVersionKind() != null) { + clientWithResource = + client.genericKubernetesResources( + classifier.groupVersionKind().getApiVersion(), + classifier.groupVersionKind().getKind()); + } else { + clientWithResource = client.resources(classifier.resourceClass()); + } + + FilterWatchListDeletable filteredClient; + if (WATCH_ALL_NAMESPACES.equals(classifier.namespaceIdentifier())) { + filteredClient = clientWithResource.inAnyNamespace(); + } else { + filteredClient = clientWithResource.inNamespace(classifier.namespaceIdentifier()); + } + filteredClient = + (FilterWatchListDeletable) filteredClient.withLabelSelector(classifier.labelSelector()); + filteredClient = + (FilterWatchListDeletable) filteredClient.withShardSelector(classifier.shardSelector()); + + if (classifier.fieldSelector() != null && !classifier.fieldSelector().getFields().isEmpty()) { + for (var f : classifier.fieldSelector().getFields()) { + if (f.negated()) { + filteredClient = + (FilterWatchListDeletable) filteredClient.withoutField(f.path(), f.value()); + } else { + filteredClient = (FilterWatchListDeletable) filteredClient.withField(f.path(), f.value()); + } + } + } + + if (classifier.informerListLimit() != null) { + filteredClient = + (FilterWatchListDeletable) filteredClient.withLimit(classifier.informerListLimit()); + } + + var informer = filteredClient.runnableInformer(0); + + Optional.ofNullable(classifier.itemStore()).ifPresent(informer::itemStore); + + configurationService + .getInformerStoppedHandler() + .ifPresent( + ish -> { + final var stopped = informer.stopped(); + if (stopped != null) { + stopped.handle( + (res, ex) -> { + ish.onStop(informer, (Throwable) ex); + return null; + }); + } else { + final var apiTypeClass = informer.getApiTypeClass(); + final var fullResourceName = HasMetadata.getFullResourceName(apiTypeClass); + final var version = HasMetadata.getVersion(apiTypeClass); + throw new IllegalStateException( + "Cannot retrieve 'stopped' callback to listen to informer stopping for" + + " informer for " + + fullResourceName + + "/" + + version); + } + }); + if (!configurationService.stopOnInformerErrorDuringStartup()) { + informer.exceptionHandler((b, t) -> !ExceptionHandler.isDeserializationException(t)); + } + return informer; + } + + @Override + public void start( + SharedIndexInformer informer, InformerClassifier informerClassifier) { + // change thread name for easier debugging + final var thread = Thread.currentThread(); + final var name = thread.getName(); + try { + thread.setName( + "InformerInfo[" + informer.getApiTypeClass().getSimpleName() + "] " + thread.getId()); + final var resourceName = informer.getApiTypeClass().getSimpleName(); + var start = informer.start(); + // note that in case we don't put here timeout and stopOnInformerErrorDuringStartup is + // false, and there is a rbac issue the get never returns; therefore operator never really + // starts + log.trace( + "Waiting informer to start namespace: {} resource: {}", + informerClassifier.namespaceIdentifier(), + resourceName); + start + .toCompletableFuture() + .get(configurationService.cacheSyncTimeout().toMillis(), TimeUnit.MILLISECONDS); + log.debug( + "Started informer for namespace: {} resource: {}", + informerClassifier.namespaceIdentifier(), + resourceName); + } catch (TimeoutException | ExecutionException e) { + if (configurationService.stopOnInformerErrorDuringStartup()) { + log.error("Informer startup error. Operator will be stopped. Informer: {}", informer, e); + throw new OperatorException(e); + } else { + log.warn("Informer startup error. Will periodically retry. Informer: {}", informer, e); + } + } catch (InterruptedException e) { + thread.interrupt(); + throw new IllegalStateException(e); + } catch (Exception e) { + ReconcilerUtilsInternal.handleKubernetesClientException( + e, HasMetadata.getFullResourceName(informer.getApiTypeClass())); + throw new OperatorException( + "Couldn't start informer for " + versionedFullResourceName(informer) + " resources", e); + } finally { + // restore original name + thread.setName(name); + } + } + + private String versionedFullResourceName(SharedIndexInformer informer) { + final var apiTypeClass = informer.getApiTypeClass(); + if (GenericKubernetesResource.class.isAssignableFrom(apiTypeClass)) { + return GenericKubernetesResource.class.getSimpleName(); + } + return ReconcilerUtilsInternal.getResourceTypeNameWithVersion(apiTypeClass); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.java new file mode 100644 index 0000000000..4440cd17f9 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.java @@ -0,0 +1,133 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer.pool; + +import java.util.HashMap; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicInteger; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; + +public class DefaultInformerPool extends AbstractInformerPool { + + private static final Logger log = LoggerFactory.getLogger(DefaultInformerPool.class); + + /** A pooled informer together with the number of event sources currently sharing it. */ + private record PooledInformer(SharedIndexInformer informer, AtomicInteger referenceCount) {} + + private final Map, PooledInformer> informers = new HashMap<>(); + + @SuppressWarnings("unchecked") + @Override + public SharedIndexInformer getInformer( + String controllerName, String name, InformerClassifier classifier) { + SharedIndexInformer informer; + synchronized (this) { + var pooled = informers.get(classifier); + if (pooled == null) { + informer = createInformer(classifier); + informers.put(classifier, new PooledInformer(informer, new AtomicInteger(1))); + log.debug( + "Created new pooled informer for classifier: {}. Requested by controller: {}, event" + + " source: {}", + classifier, + controllerName, + name); + } else { + informer = (SharedIndexInformer) pooled.informer(); + informers.keySet().stream() + .filter(existing -> existing.differsOnlyByInformerListLimit(classifier)) + .findFirst() + .ifPresent( + existing -> + log.warn( + "Reusing informer for classifier {} that differs only by informerListLimit" + + " (existing: {}, requested: {}). The existing informerListLimit is" + + " kept.", + classifier, + existing.informerListLimit(), + classifier.informerListLimit())); + var referenceCount = pooled.referenceCount().incrementAndGet(); + log.debug( + "Reusing pooled informer for classifier: {}. Reference count now: {}. Requested by" + + " controller: {}, event source: {}", + classifier, + referenceCount, + controllerName, + name); + } + } + return informer; + } + + @SuppressWarnings("unchecked") + @Override + public synchronized Optional> releaseInformer( + String controllerName, String name, InformerClassifier classifier) { + var pooled = informers.get(classifier); + if (pooled == null) { + log.warn("No informer found in the pool for classifier: {}", classifier); + return Optional.empty(); + } + var informer = (SharedIndexInformer) pooled.informer(); + // Only the last controller sharing the informer stops it; the informer is still returned to the + // caller in every case so it can remove its own event handler from the (possibly still running) + // shared informer. + var referenceCount = pooled.referenceCount().decrementAndGet(); + if (referenceCount == 0) { + informers.remove(classifier); + informer.stop(); + log.debug( + "Released and stopped last-referenced pooled informer for classifier: {}. Released by" + + " controller: {}, event source: {}", + classifier, + controllerName, + name); + } else { + log.debug( + "Released pooled informer for classifier: {}, kept running. Reference count now: {}." + + " Released by controller: {}, event source: {}", + classifier, + referenceCount, + controllerName, + name); + } + return Optional.of(informer); + } + + /** Total number of distinct informers currently held in the pool. */ + synchronized int size() { + return informers.size(); + } + + /** + * Number of distinct informers currently held in the pool for the given resource type. When + * multiple controllers share a single informer for a resource, this returns {@code 1} for that + * resource type regardless of how many controllers use it. + */ + @Override + public synchronized long numberOfInformersForResource( + Class resourceClass) { + return informers.keySet().stream() + .filter(classifier -> resourceClass.equals(classifier.resourceClass())) + .count(); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifier.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifier.java new file mode 100644 index 0000000000..e4023a93e9 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifier.java @@ -0,0 +1,143 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer.pool; + +import java.util.Objects; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; +import io.javaoperatorsdk.operator.api.config.informer.FieldSelector; +import io.javaoperatorsdk.operator.processing.GroupVersionKind; + +/** + * Identifies the informer that backs an event source: two event sources whose classifiers are equal + * can be served by one shared informer. It also carries everything needed to create that informer, + * including the {@link KubernetesClient} to create it from. + * + *

Note that {@link #equals(Object)} and {@link #hashCode()} deliberately do not + * cover every record component: + * + *

+ * + *

The {@link #client()} takes part in equality by identity: event sources + * sharing an informer must be watching through the very same client, since the informer is created + * from (and keeps using) the client of whichever event source established it. Two separate clients + * are therefore never assumed to be interchangeable, not even when they connect to the same API + * server — they may well differ in credentials, impersonation or TLS material, and the pool cannot + * tell. + * + *

Note that this is also why nothing security relevant from the client's configuration is part + * of the classifier: instances end up in log messages and exception messages, so a credential held + * here would leak into those. + */ +public record InformerClassifier( + KubernetesClient client, + String labelSelector, + String shardSelector, + String namespaceIdentifier, + Class resourceClass, + GroupVersionKind groupVersionKind, + FieldSelector fieldSelector, + Long informerListLimit, + ItemStore itemStore) { + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof InformerClassifier that)) { + return false; + } + return client == that.client + && Objects.equals(labelSelector, that.labelSelector) + && Objects.equals(shardSelector, that.shardSelector) + && Objects.equals(namespaceIdentifier, that.namespaceIdentifier) + && Objects.equals(resourceClass, that.resourceClass) + && Objects.equals(groupVersionKind, that.groupVersionKind) + && Objects.equals(fieldSelector, that.fieldSelector) + && Objects.equals(itemStore, that.itemStore); + } + + @Override + public int hashCode() { + return Objects.hash( + System.identityHashCode(client), + labelSelector, + shardSelector, + namespaceIdentifier, + resourceClass, + groupVersionKind, + fieldSelector, + itemStore); + } + + /** + * Hand written instead of using the one generated for the record, so that the API server URL is + * part of it: classifiers show up in log and exception messages, where the client on its own + * identifies the instance but not the cluster it connects to. The URL is derived from the {@link + * #client()} rather than held as a component of its own, since it would be redundant for the + * identity and could only ever contradict the client. + */ + @Override + public String toString() { + return "InformerClassifier[client=" + + client + + " (" + + masterUrl() + + "), labelSelector=" + + labelSelector + + ", shardSelector=" + + shardSelector + + ", namespaceIdentifier=" + + namespaceIdentifier + + ", resourceClass=" + + (resourceClass != null ? resourceClass.getName() : null) + + ", groupVersionKind=" + + groupVersionKind + + ", fieldSelector=" + + fieldSelector + + ", informerListLimit=" + + informerListLimit + + ", itemStore=" + + itemStore + + "]"; + } + + private String masterUrl() { + if (client == null || client.getConfiguration() == null) { + return null; + } + return client.getConfiguration().getMasterUrl(); + } + + /** + * Checks whether this classifier and the other are equal in every attribute except for the {@link + * #informerListLimit()}, which differs between them. + */ + public boolean differsOnlyByInformerListLimit(InformerClassifier other) { + return equals(other) && !Objects.equals(informerListLimit, other.informerListLimit); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerPool.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerPool.java new file mode 100644 index 0000000000..404f14b5f4 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerPool.java @@ -0,0 +1,98 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer.pool; + +import java.util.Optional; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.reconciler.Experimental; + +/** + * The contract consumed by the event sources. Implementations must extend {@link + * AbstractInformerPool} — that is the type the configuration API accepts — which additionally + * handles informer creation, startup and the {@link ConfigurationService} injection. + */ +@Experimental( + "This is experimental only in the sense that the API could be improved in a" + + " non-backwards-compatible way. The feature we provide otherwise is prod ready.") +public interface InformerPool { + + /** + * The informer backing the event source identified by {@code controllerName} and {@code name}: a + * sharing pool returns the existing informer for an equal {@link InformerClassifier} if there is + * one and creates it otherwise, a non-sharing pool always creates a dedicated one. A newly + * created informer is created from the classifier's {@link InformerClassifier#client()}, which is + * part of the classifier's identity precisely so that a shared informer is only ever handed to + * event sources watching through that same client. + * + *

The returned informer is not started, callers are expected to call {@link + * #start(SharedIndexInformer, InformerClassifier)} afterwards. When joining an already running + * shared informer it may however be started and hold a populated cache already; handlers + * registered on it still receive the cache contents, so callers must not replay those themselves. + * + *

This registers the caller as a user of the informer and must therefore be paired with + * exactly one {@link #releaseInformer(String, String, InformerClassifier)} for the same + * controller name, event source name and classifier. Requesting an informer twice for the same + * combination without releasing it in between is a programming error: a sharing pool would count + * the caller twice and consequently never stop the informer, which is why {@link + * NonSharingInformerPool} rejects it outright. + */ + SharedIndexInformer getInformer( + String controllerName, String name, InformerClassifier classifier); + + /** + * Starts the informer (if not already started) and blocks until its cache has synced, or the + * configured {@link ConfigurationService#cacheSyncTimeout()} elapses. Callers are expected to + * invoke this after {@link #getInformer(String, String, InformerClassifier)} returns; the pool + * itself only registers/reference-counts the informer and does not block on cache sync + * internally. + */ + void start( + SharedIndexInformer informer, InformerClassifier classifier); + + /** + * Signals that the identified user (controller + event source name) no longer needs the informer + * for the given classifier. A sharing pool only stops the informer once its last user has + * released it, a non-sharing pool stops it right away. + * + *

The informer is returned in either case, even when it is left running for the remaining + * users, since the caller still has to remove its own event handler from it. Callers must not + * assume the returned informer is stopped, and must not stop it themselves. + * + * @return the released informer, or empty if the pool holds none for this user and classifier + */ + Optional> releaseInformer( + String controllerName, String name, InformerClassifier classifier); + + /** + * Binds this pool to the {@link ConfigurationService} it belongs to. Called by the framework when + * the pool is resolved from that configuration service, before the pool is used; users are not + * expected to call it themselves. + * + *

The pool needs the configuration service to create and start informers: the {@link + * ConfigurationService#cacheSyncTimeout()} to wait for, whether to {@link + * ConfigurationService#stopOnInformerErrorDuringStartup()}, and the {@link + * ConfigurationService#getInformerStoppedHandler()} to hook up. + * + *

Injecting it here, rather than requiring it as a constructor argument, is what keeps + * creating a pool a plain {@code new NonSharingInformerPool()} for users configuring one through + * {@link io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider#withInformerPool}. + * A pool instance therefore belongs to exactly one configuration service. + */ + void setConfigurationService(ConfigurationService configurationService); +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java new file mode 100644 index 0000000000..da7ac06ab6 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java @@ -0,0 +1,88 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer.pool; + +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.javaoperatorsdk.operator.OperatorException; + +@SuppressWarnings({"unchecked", "rawtypes"}) +public class NonSharingInformerPool extends AbstractInformerPool { + + private static final Logger log = LoggerFactory.getLogger(NonSharingInformerPool.class); + + private final Map informers = new ConcurrentHashMap(); + + @Override + public synchronized SharedIndexInformer getInformer( + String controllerName, String name, InformerClassifier classifier) { + var key = new ClassifierWithName(controllerName, name, classifier); + if (informers.containsKey(key)) { + throw new OperatorException( + "Informer already registered for controller: " + + controllerName + + ", event source: " + + name + + ", classifier: " + + classifier + + ". This pool creates a dedicated informer per controller/event source and never" + + " shares them, so requesting one twice for the same combination without releasing" + + " the previous one first would leak the earlier informer."); + } + var informer = createInformer(classifier); + informers.put(key, informer); + return informer; + } + + @Override + public Optional> releaseInformer( + String controllerName, String name, InformerClassifier classifier) { + var informer = informers.remove(new ClassifierWithName(controllerName, name, classifier)); + if (informer != null) { + informer.stop(); + } else { + log.warn("Informer was not found for classifier: {}", classifier); + } + return Optional.ofNullable(informer); + } + + /** Number of informers currently tracked (i.e. created but not yet released). */ + int size() { + return informers.size(); + } + + /** + * Number of distinct informers currently held for the given resource type. Since this pool never + * shares informers, this equals the number of registered users (controller + event source name) + * watching that resource type. + */ + @Override + public long numberOfInformersForResource(Class resourceClass) { + return informers.keySet().stream() + .filter(key -> resourceClass.equals(key.classifier().resourceClass())) + .count(); + } + + public record ClassifierWithName( + String controllerName, String name, InformerClassifier classifier) {} +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java index 61b434c0c4..3e5b872ba2 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java @@ -26,12 +26,12 @@ import io.fabric8.kubernetes.api.model.authorization.v1.ResourceRule; import io.fabric8.kubernetes.api.model.authorization.v1.SelfSubjectRulesReview; import io.fabric8.kubernetes.api.model.authorization.v1.SubjectRulesReviewStatus; +import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.V1ApiextensionAPIGroupDSL; import io.fabric8.kubernetes.client.dsl.AnyNamespaceOperation; import io.fabric8.kubernetes.client.dsl.ApiextensionsAPIGroupDSL; import io.fabric8.kubernetes.client.dsl.FilterWatchListDeletable; -import io.fabric8.kubernetes.client.dsl.Informable; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.NamespaceableResource; import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; @@ -112,9 +112,9 @@ public static KubernetesClient client( when(filterable.runnableInformer(anyLong())).thenReturn(informer); - Informable informable = mock(Informable.class); - when(filterable.withLimit(anyLong())).thenReturn(informable); - when(informable.runnableInformer(anyLong())).thenReturn(informer); + // The informer pool casts the result of withLimit() back to FilterWatchListDeletable, so it has + // to return the filterable mock (which is one) rather than a plain Informable mock. + when(filterable.withLimit(anyLong())).thenReturn(filterable); when(client.resources(clazz)).thenReturn(resources); when(client.leaderElector()) @@ -138,6 +138,10 @@ public static KubernetesClient client( final var serialization = new KubernetesSerialization(); when(client.getKubernetesSerialization()).thenReturn(serialization); + final var config = mock(Config.class); + when(config.getMasterUrl()).thenReturn("https://localhost:8443/"); + when(client.getConfiguration()).thenReturn(config); + return client; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/InformerConfigurationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/InformerConfigurationTest.java index 95b8465706..16e5ab578b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/InformerConfigurationTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/InformerConfigurationTest.java @@ -16,11 +16,13 @@ package io.javaoperatorsdk.operator.api.config; import java.util.Collections; +import java.util.List; import java.util.Set; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.config.informer.FieldSelector; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Constants; @@ -79,6 +81,37 @@ void nullShardSelectorByDefault() { assertNull(informerConfig.getShardSelector()); } + @Test + void nullFieldSelectorByDefault() { + final var informerConfig = InformerConfiguration.builder(ConfigMap.class).build(); + assertNull(informerConfig.getFieldSelector()); + } + + @Test + void emptyFieldSelectorIsNormalizedToNoFieldSelector() { + // the annotation path always builds a FieldSelector (@Informer#fieldSelector defaults to {}) + // while the programmatic path leaves it null. An empty selector filters nothing, so the two + // must not get classifiers that disagree and therefore refuse to share an informer + assertNull( + InformerConfiguration.builder(ConfigMap.class) + .withFieldSelector(new FieldSelector(List.of())) + .build() + .getFieldSelector()); + assertNull( + InformerConfiguration.builder(ConfigMap.class) + .withFieldSelector(new FieldSelector()) + .build() + .getFieldSelector()); + } + + @Test + void fieldSelectorIsSetOnBuilderWhenNotEmpty() { + final var fieldSelector = new FieldSelector(new FieldSelector.Field("metadata.name", "foo")); + final var informerConfig = + InformerConfiguration.builder(ConfigMap.class).withFieldSelector(fieldSelector).build(); + assertEquals(fieldSelector, informerConfig.getFieldSelector()); + } + @Test void shardSelectorIsSetOnBuilder() { final var informerConfig = diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java index 91d60f7aa7..b725f49132 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java @@ -62,6 +62,9 @@ class ControllerTest { @Test void crdShouldNotBeCheckedForNativeResources() { final var client = MockKubernetesClient.client(Secret.class); + final var configurationService = + ConfigurationService.newOverriddenConfigurationService( + this.configurationService, o -> o.withKubernetesClient(client)); final var configuration = MockControllerConfiguration.forResource(Secret.class, configurationService); final var controller = new Controller(reconciler, configuration, client); @@ -75,7 +78,8 @@ void notifiesMetricsWhenEventProcessorStarts() { final var metrics = mock(Metrics.class); final var configurationService = ConfigurationService.newOverriddenConfigurationService( - new BaseConfigurationService(), o -> o.withMetrics(metrics)); + new BaseConfigurationService(), + o -> o.withMetrics(metrics).withKubernetesClient(client)); final var configuration = MockControllerConfiguration.forResource(Secret.class, configurationService); final var controller = new Controller(reconciler, configuration, client); @@ -95,7 +99,8 @@ void doesNotNotifyMetricsWhenEventProcessorNotStarted() { final var metrics = mock(Metrics.class); final var configurationService = ConfigurationService.newOverriddenConfigurationService( - new BaseConfigurationService(), o -> o.withMetrics(metrics)); + new BaseConfigurationService(), + o -> o.withMetrics(metrics).withKubernetesClient(client)); final var configuration = MockControllerConfiguration.forResource(Secret.class, configurationService); final var controller = new Controller(reconciler, configuration, client); @@ -110,7 +115,8 @@ void crdShouldNotBeCheckedForCustomResourcesIfDisabled() { final var client = MockKubernetesClient.client(TestCustomResource.class); ConfigurationService configurationService = ConfigurationService.newOverriddenConfigurationService( - new BaseConfigurationService(), o -> o.checkingCRDAndValidateLocalModel(false)); + new BaseConfigurationService(), + o -> o.checkingCRDAndValidateLocalModel(false).withKubernetesClient(client)); final var configuration = MockControllerConfiguration.forResource(TestCustomResource.class, configurationService); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java index 9874740ae4..8324748207 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java @@ -96,6 +96,18 @@ void pluralShouldOverrideDefaultComputedVersionIfProvided() { assertThat(gvk.hashCode()).isNotEqualTo(original.hashCode()); } + @Test + void comparingAnExplicitPluralWithAnUnspecifiedOneIsSymmetricAndDoesNotThrow() { + final var original = new GroupVersionKind("josdk.io", "v1", "MyKind"); + final var withPlural = GroupVersionKindPlural.gvkWithPlural(original, "MyPlural"); + final var withoutPlural = GroupVersionKindPlural.gvkWithPlural(original, null); + + // an unspecified plural is not a wildcard: it carries no plural form, just like the plain + // GroupVersionKind it compares equal to + assertThat(withPlural).isNotEqualTo(withoutPlural); + assertThat(withoutPlural).isNotEqualTo(withPlural); + } + @Test void equals() { final var original = new GroupVersionKind("josdk.io", "v1", "MyKind"); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index 251a0e47ae..f47b72820f 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -24,6 +24,7 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -202,12 +203,14 @@ void changesNamespacesOnControllerAndInformerEventSources() { private EventSourceManager initManager() { final var configuration = MockControllerConfiguration.forResource(ConfigMap.class); - final var configService = new BaseConfigurationService(); + final var mockClient = MockKubernetesClient.client(ConfigMap.class); + final var configService = + ConfigurationService.newOverriddenConfigurationService( + new BaseConfigurationService(), + overrider -> overrider.withKubernetesClient(mockClient)); when(configuration.getConfigurationService()).thenReturn(configService); - final Controller controller = - new Controller( - mock(Reconciler.class), configuration, MockKubernetesClient.client(ConfigMap.class)); + final Controller controller = new Controller(mock(Reconciler.class), configuration, mockClient); return new EventSourceManager(controller); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index ac24375242..191836a0dc 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -122,6 +122,21 @@ private ReconciliationDispatcher init( boolean useFinalizer) { final Class resourceClass = (Class) customResource.getClass(); + final var kubernetesClient = MockKubernetesClient.client(resourceClass); + // The informer pool obtains its client from the ConfigurationService, so the mock client has to + // be set there as well (not only on the Controller); otherwise starting the informers would hit + // a real cluster. Cloner and SSA settings are re-supplied so the re-wrapping does not drop + // them. + configurationService = + ConfigurationService.newOverriddenConfigurationService( + configurationService, + overrider -> + overrider + .withKubernetesClient(kubernetesClient) + .withResourceCloner(configurationService.getResourceCloner()) + .withUseSSAToPatchPrimaryResource( + configurationService.useSSAToPatchPrimaryResource())); + configuration = configuration == null ? MockControllerConfiguration.forResource(resourceClass, configurationService) @@ -139,7 +154,7 @@ private ReconciliationDispatcher init( .thenReturn(Optional.of(Duration.ofHours(RECONCILIATION_MAX_INTERVAL))); Controller controller = - new Controller<>(reconciler, configuration, MockKubernetesClient.client(resourceClass)) { + new Controller<>(reconciler, configuration, kubernetesClient) { @Override public boolean useFinalizer() { return useFinalizer; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java index 38190a96dc..185b626161 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java @@ -27,6 +27,7 @@ import io.javaoperatorsdk.operator.ReconcilerUtilsInternal; import io.javaoperatorsdk.operator.TestUtils; import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.ResolvedControllerConfiguration; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; @@ -59,7 +60,11 @@ class ControllerEventSourceTest @BeforeEach public void setup() { - when(controllerConfig.getConfigurationService()).thenReturn(new BaseConfigurationService()); + var clientMock = MockKubernetesClient.client(TestCustomResource.class); + when(controllerConfig.getConfigurationService()) + .thenReturn( + ConfigurationService.newOverriddenConfigurationService( + new BaseConfigurationService(), o -> o.withKubernetesClient(clientMock))); var ic = mock(InformerConfiguration.class); when(controllerConfig.getInformerConfig()).thenReturn(ic); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java index 210ce52fcc..01ee25e44c 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java @@ -94,6 +94,10 @@ void setup() { when(informerEventSourceConfiguration.getResourceClass()).thenReturn(Deployment.class); when(informerConfig.isComparableResourceVersions()).thenReturn(true); when(informerConfig.getEffectiveNamespaces(any())).thenReturn(DEFAULT_NAMESPACES_SET); + // a plain Long-returning Mockito mock yields 0 here, but a real unconfigured informer has no + // list limit; without this the pool would take the withLimit(...) branch when creating the + // informer + when(informerConfig.getInformerListLimit()).thenReturn(null); informerEventSource = buildInformerEventSource(); } @@ -101,7 +105,7 @@ void setup() { private InformerEventSource buildInformerEventSource() { InformerEventSource eventSource = spy( - new InformerEventSource<>(informerEventSourceConfiguration, clientMock) { + new InformerEventSource<>(informerEventSourceConfiguration) { // mocking start @Override public synchronized void start() {} @@ -259,22 +263,25 @@ void ownUpdateEventIsDeferredDuringActiveFilter() { void informerStoppedHandlerShouldBeCalledWhenInformerStops() { final var exception = new RuntimeException("Informer stopped exceptionally!"); final var informerStoppedHandler = mock(InformerStoppedHandler.class); + // the informer is created by the pool, which uses the client from the configuration service, so + // the mock client has to be set there for its informer-start behavior to take effect + final var mockClient = + MockKubernetesClient.client( + Deployment.class, + unused -> { + throw exception; + }); var configuration = ConfigurationService.newOverriddenConfigurationService( new BaseConfigurationService(), - o -> o.withInformerStoppedHandler(informerStoppedHandler)); + o -> + o.withInformerStoppedHandler(informerStoppedHandler) + .withKubernetesClient(mockClient)); var mockControllerConfig = mock(ControllerConfiguration.class); when(mockControllerConfig.getConfigurationService()).thenReturn(configuration); - informerEventSource = - new InformerEventSource<>( - informerEventSourceConfiguration, - MockKubernetesClient.client( - Deployment.class, - unused -> { - throw exception; - })); + informerEventSource = new InformerEventSource<>(informerEventSourceConfiguration); informerEventSource.setControllerConfiguration(mockControllerConfig); // by default informer fails to start if there is an exception in the client on start. diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.java new file mode 100644 index 0000000000..a6906f85cf --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.java @@ -0,0 +1,145 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer; + +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.informers.ResourceEventHandler; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.DefaultInformerPool; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerClassifier; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * A pooled informer is reference counted, so releasing the same namespace twice consumes a + * reference another controller still holds. {@link InformerManager#stop()} and {@link + * InformerManager#changeNamespaces(Set)} can run concurrently, so removing the source from the + * manager has to be what claims the right to release it. + */ +@SuppressWarnings({"rawtypes", "unchecked"}) +class InformerManagerConcurrentReleaseTest { + + private static final String NAMESPACE = "ns1"; + + private final KubernetesClient clientMock = MockKubernetesClient.client(Deployment.class); + private final LatchingInformerPool pool = new LatchingInformerPool(); + private final InformerEventSourceConfiguration configuration = + mock(InformerEventSourceConfiguration.class); + private final ResourceEventHandler eventHandler = mock(ResourceEventHandler.class); + + @BeforeEach + void setup() { + final var informerConfig = mock(InformerConfiguration.class); + when(informerConfig.getEffectiveNamespaces(any())).thenReturn(Set.of(NAMESPACE)); + when(informerConfig.getInformerListLimit()).thenReturn(null); + when(configuration.getInformerConfig()).thenReturn(informerConfig); + when(configuration.getResourceClass()).thenReturn(Deployment.class); + } + + @Test + void concurrentStopAndNamespaceChangeReleaseTheInformerOnlyOnce() throws Exception { + var manager = + new InformerManager>( + configuration, eventHandler); + manager.setControllerConfiguration(controllerConfiguration()); + manager.start(); + // a second controller shares the very same informer and never releases it, so the pool has to + // keep it running no matter how the manager below is torn down + var informer = pool.getInformer("other-controller", "other-es", classifier()); + + // drop the only watched namespace on one thread; the pool blocks inside releaseInformer, which + // is the window in which stop() used to see the already-released source and release it again + var namespaceChange = new Thread(() -> manager.changeNamespaces(Set.of())); + namespaceChange.start(); + assertThat(pool.enteredRelease.await(5, TimeUnit.SECONDS)).isTrue(); + + manager.stop(); + + pool.proceed.countDown(); + namespaceChange.join(TimeUnit.SECONDS.toMillis(5)); + + assertThat(pool.releaseCount.get()) + .as("the same namespace must not be released twice") + .isEqualTo(1); + verify(informer, never()).stop(); + } + + private ControllerConfiguration controllerConfiguration() { + ConfigurationService configurationService = + ConfigurationService.newOverriddenConfigurationService( + new BaseConfigurationService(), + o -> o.withKubernetesClient(clientMock).withInformerPool(pool)); + var controllerConfiguration = mock(ControllerConfiguration.class); + when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); + when(controllerConfiguration.getName()).thenReturn("controller"); + return controllerConfiguration; + } + + /** Has to match what the manager builds for {@link #NAMESPACE} so it hits the same pool entry. */ + private InformerClassifier classifier() { + return new InformerClassifier<>( + clientMock, null, null, NAMESPACE, Deployment.class, null, null, null, null); + } + + /** Blocks inside the first release so the two teardown paths can be interleaved on purpose. */ + private static class LatchingInformerPool extends DefaultInformerPool { + + private final CountDownLatch enteredRelease = new CountDownLatch(1); + private final CountDownLatch proceed = new CountDownLatch(1); + private final AtomicInteger releaseCount = new AtomicInteger(); + private final AtomicBoolean blockNextRelease = new AtomicBoolean(true); + + @Override + public Optional> releaseInformer( + String controllerName, String name, InformerClassifier classifier) { + releaseCount.incrementAndGet(); + // deliberately blocking before delegating: releaseInformer is synchronized, so waiting inside + // it would just serialize the two threads instead of interleaving them + if (blockNextRelease.compareAndSet(true, false)) { + enteredRelease.countDown(); + try { + proceed.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + return super.releaseInformer(controllerName, name, classifier); + } + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapperTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapperTest.java new file mode 100644 index 0000000000..e175074125 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapperTest.java @@ -0,0 +1,129 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer; + +import java.util.List; +import java.util.Map; + +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.fabric8.kubernetes.client.informers.cache.Cache; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerClassifier; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * The informer behind a wrapper can be shared by event sources of several controllers, while its + * indexer is a single namespace of index names. The wrapper therefore qualifies index names with + * the event source that registered them, which only works as long as registration, lookup and + * removal all apply the same qualification. + */ +class InformerWrapperTest { + + private static final String INDEX_NAME = "my-index"; + private static final String INDEX_KEY = "key"; + + @SuppressWarnings("unchecked") + private final Cache indexer = mock(Cache.class); + + @Test + void indexNamesAreQualifiedOnRegistrationAndOnLookup() { + var wrapper = wrapper("controller", "event-source"); + + wrapper.addIndexers(Map.of(INDEX_NAME, r -> List.of(INDEX_KEY))); + wrapper.byIndex(INDEX_NAME, INDEX_KEY); + + var registered = registeredNames(); + // the caller's name is not what reaches the informer... + assertThat(registered).hasSize(1).allSatisfy(name -> assertThat(name).isNotEqualTo(INDEX_NAME)); + // ...but it is still recognizable in it, so that client side errors stay diagnosable + assertThat(registered) + .allSatisfy( + name -> + assertThat(name) + .contains("controller") + .contains("event-source") + .endsWith(INDEX_NAME)); + // and lookup asks for exactly the name that was registered + verify(indexer).byIndex(eq(registered.get(0)), eq(INDEX_KEY)); + } + + @Test + void twoEventSourcesRegisterTheSameIndexNameUnderDistinctQualifiedNames() { + // this is what keeps the client from rejecting the second one with an "Indexer conflict", and + // what keeps either of them from reading the other's index + wrapper("controller-1", "event-source") + .addIndexers(Map.of(INDEX_NAME, r -> List.of(INDEX_KEY))); + wrapper("controller-2", "event-source") + .addIndexers(Map.of(INDEX_NAME, r -> List.of(INDEX_KEY))); + wrapper("controller-1", "other-event-source") + .addIndexers(Map.of(INDEX_NAME, r -> List.of(INDEX_KEY))); + + assertThat(registeredNames()).hasSize(3).doesNotHaveDuplicates(); + } + + @Test + void removeIndexersDropsExactlyTheNamesThisWrapperRegistered() { + var wrapper = wrapper("controller", "event-source"); + wrapper.addIndexers(Map.of(INDEX_NAME, r -> List.of(INDEX_KEY))); + var registered = registeredNames().get(0); + + wrapper.removeIndexers(); + + verify(indexer).removeIndexer(registered); + } + + @Test + void removeIndexersIsANoopWithoutRegisteredIndexers() { + wrapper("controller", "event-source").removeIndexers(); + + verify(indexer, never()).removeIndexer(any()); + } + + @SuppressWarnings("unchecked") + private InformerWrapper wrapper(String controller, String eventSource) { + SharedIndexInformer informer = mock(SharedIndexInformer.class); + when(informer.getStore()).thenReturn(indexer); + when(informer.getIndexer()).thenReturn(indexer); + return new InformerWrapper<>( + informer, + "default", + new InformerClassifier<>( + null, null, null, "default", TestCustomResource.class, null, null, null, null), + controller, + eventSource); + } + + @SuppressWarnings("unchecked") + private List registeredNames() { + var captor = ArgumentCaptor.forClass(Map.class); + verify(indexer, atLeastOnce()).addIndexers(captor.capture()); + return captor.getAllValues().stream() + .flatMap(m -> m.keySet().stream()) + .map(String::valueOf) + .toList(); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceNamespaceChangeTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceNamespaceChangeTest.java new file mode 100644 index 0000000000..a1ee558036 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceNamespaceChangeTest.java @@ -0,0 +1,132 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer; + +import java.util.Set; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.fabric8.kubernetes.client.informers.cache.Cache; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.processing.event.EventHandler; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.DefaultInformerPool; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * A namespace change acquires and starts pooled informers, so it must not happen on an event source + * that is no longer running: {@link ManagedInformerEventSource#stop()} short-circuits on a + * non-running event source, which would leave those informers referenced with nothing left to + * release them. + */ +@SuppressWarnings({"rawtypes", "unchecked"}) +class ManagedInformerEventSourceNamespaceChangeTest { + + private final KubernetesClient clientMock = MockKubernetesClient.client(Deployment.class); + private final DefaultInformerPool pool = new DefaultInformerPool(); + private final InformerEventSourceConfiguration configuration = + mock(InformerEventSourceConfiguration.class); + + @BeforeEach + void setup() { + final var informerConfig = mock(InformerConfiguration.class); + when(informerConfig.getEffectiveNamespaces(any())).thenReturn(Set.of("ns1")); + when(informerConfig.getInformerListLimit()).thenReturn(null); + when(configuration.getInformerConfig()).thenReturn(informerConfig); + when(configuration.getResourceClass()).thenReturn(Deployment.class); + when(configuration.followControllerNamespaceChanges()).thenReturn(true); + final var secondaryToPrimaryMapper = mock(SecondaryToPrimaryMapper.class); + when(secondaryToPrimaryMapper.toPrimaryResourceIDs(any())) + .thenReturn(Set.of(new ResourceID("name", "ns1"))); + when(configuration.getSecondaryToPrimaryMapper()).thenReturn(secondaryToPrimaryMapper); + stubInformerStore(); + } + + @Test + void namespaceChangeOnAStoppedEventSourceAcquiresNoInformer() { + var eventSource = buildEventSource(); + eventSource.start(); + assertThat(pool.numberOfInformersForResource(Deployment.class)).isEqualTo(1); + eventSource.stop(); + assertThat(pool.numberOfInformersForResource(Deployment.class)).isZero(); + + eventSource.changeNamespaces(Set.of("ns2")); + + assertThat(pool.numberOfInformersForResource(Deployment.class)) + .as("a stopped event source must not acquire informers it can never release") + .isZero(); + } + + @Test + void namespaceChangeIsStillAppliedWhileRunning() { + var eventSource = buildEventSource(); + eventSource.start(); + + eventSource.changeNamespaces(Set.of("ns2")); + + assertThat(pool.numberOfInformersForResource(Deployment.class)).isEqualTo(1); + assertThat(eventSource.manager().isWatchingNamespace("ns2")).isTrue(); + assertThat(eventSource.manager().isWatchingNamespace("ns1")).isFalse(); + + eventSource.stop(); + assertThat(pool.numberOfInformersForResource(Deployment.class)).isZero(); + } + + /** + * A successfully started {@link InformerEventSource} lists its cache to populate the + * primary-to-secondary index, and the mock client leaves the informer's store unstubbed. + */ + private void stubInformerStore() { + SharedIndexInformer informer = + clientMock + .resources(Deployment.class) + .inNamespace("ns1") + .withLabelSelector((String) null) + .withShardSelector(null) + .runnableInformer(0); + when(informer.getStore()).thenReturn(mock(Cache.class)); + } + + private InformerEventSource buildEventSource() { + ConfigurationService configurationService = + ConfigurationService.newOverriddenConfigurationService( + new BaseConfigurationService(), + o -> o.withKubernetesClient(clientMock).withInformerPool(pool)); + var controllerConfiguration = mock(ControllerConfiguration.class); + when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); + when(controllerConfiguration.getName()).thenReturn("controller"); + + var eventSource = new InformerEventSource(configuration); + eventSource.setEventHandler(mock(EventHandler.class)); + eventSource.setControllerConfiguration(controllerConfiguration); + return eventSource; + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceStartFailureTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceStartFailureTest.java new file mode 100644 index 0000000000..a4d78ace74 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceStartFailureTest.java @@ -0,0 +1,155 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer; + +import java.util.Set; +import java.util.concurrent.atomic.AtomicBoolean; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.fabric8.kubernetes.client.informers.cache.Cache; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.processing.event.EventHandler; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.DefaultInformerPool; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerClassifier; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * The informers of an event source are acquired from the pool before any of them is started, so a + * failure while starting them has to release everything that was already acquired. Otherwise the + * pooled informer stays referenced forever: {@code start()} never marks the event source running, + * so {@code stop()} silently skips the release. + */ +@SuppressWarnings({"rawtypes", "unchecked"}) +class ManagedInformerEventSourceStartFailureTest { + + private static final Set NAMESPACES = Set.of("ns1", "ns2"); + + private final KubernetesClient clientMock = MockKubernetesClient.client(Deployment.class); + private final FailingOnceInformerPool pool = new FailingOnceInformerPool(); + private final InformerEventSourceConfiguration configuration = + mock(InformerEventSourceConfiguration.class); + + @BeforeEach + void setup() { + final var informerConfig = mock(InformerConfiguration.class); + when(informerConfig.getEffectiveNamespaces(any())).thenReturn(NAMESPACES); + // an unconfigured informer has no list limit, while a plain Long-returning mock would yield 0 + // here and send the pool down the withLimit(...) branch + when(informerConfig.getInformerListLimit()).thenReturn(null); + when(configuration.getInformerConfig()).thenReturn(informerConfig); + when(configuration.getResourceClass()).thenReturn(Deployment.class); + final var secondaryToPrimaryMapper = mock(SecondaryToPrimaryMapper.class); + when(secondaryToPrimaryMapper.toPrimaryResourceIDs(any())) + .thenReturn(Set.of(new ResourceID("name", "ns1"))); + when(configuration.getSecondaryToPrimaryMapper()).thenReturn(secondaryToPrimaryMapper); + stubInformerStore(); + } + + /** + * A successfully started {@link InformerEventSource} lists its cache to populate the + * primary-to-secondary index, and the mock client leaves the informer's store unstubbed. + */ + private void stubInformerStore() { + SharedIndexInformer informer = + clientMock + .resources(Deployment.class) + .inNamespace("ns1") + .withLabelSelector((String) null) + .withShardSelector(null) + .runnableInformer(0); + when(informer.getStore()).thenReturn(mock(Cache.class)); + } + + @Test + void releasesAlreadyAcquiredInformersWhenStartupFails() { + var eventSource = buildEventSource(); + + assertThatThrownBy(eventSource::start).isInstanceOf(RuntimeException.class); + + assertThat(eventSource.isRunning()).isFalse(); + assertThat(pool.numberOfInformersForResource(Deployment.class)) + .as("a failed start must not leave informers referenced in the pool") + .isZero(); + } + + @Test + void doesNotAcquireTwiceWhenStartIsRetriedAfterAFailure() { + var eventSource = buildEventSource(); + + assertThatThrownBy(eventSource::start).isInstanceOf(RuntimeException.class); + // a failed event source is started again, e.g. by a subsequent dynamic registration + eventSource.start(); + assertThat(eventSource.isRunning()).isTrue(); + assertThat(pool.numberOfInformersForResource(Deployment.class)).isEqualTo(NAMESPACES.size()); + + eventSource.stop(); + + assertThat(pool.numberOfInformersForResource(Deployment.class)) + .as("the retried start must not have acquired a second reference per informer") + .isZero(); + } + + private InformerEventSource buildEventSource() { + var configurationService = + ConfigurationService.newOverriddenConfigurationService( + new BaseConfigurationService(), + o -> o.withKubernetesClient(clientMock).withInformerPool(pool)); + var controllerConfiguration = mock(ControllerConfiguration.class); + when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); + when(controllerConfiguration.getName()).thenReturn("controller"); + + var eventSource = new InformerEventSource(configuration); + eventSource.setEventHandler(mock(EventHandler.class)); + eventSource.setControllerConfiguration(controllerConfiguration); + return eventSource; + } + + /** Fails the very first informer startup, so later attempts succeed. */ + private static class FailingOnceInformerPool extends DefaultInformerPool { + + private final AtomicBoolean failNextStart = new AtomicBoolean(true); + + @Override + public void start( + SharedIndexInformer informer, InformerClassifier classifier) { + if (failNextStart.compareAndSet(true, false)) { + throw new OperatorException("simulated informer startup failure"); + } + // not delegating on purpose: the pool's start is the seam under test, actually starting the + // mocked informer would add nothing + } + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPoolTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPoolTest.java new file mode 100644 index 0000000000..4ca4db22bd --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPoolTest.java @@ -0,0 +1,154 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer.pool; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * Unit tests for the reference-counting / sharing behavior of {@link DefaultInformerPool}: a single + * informer is created and shared per classifier, and it is stopped only when the last user releases + * it. + */ +class DefaultInformerPoolTest { + + private static final String CONTROLLER = "controller"; + private static final String ES_NAME = "event-source"; + + private final KubernetesClient client = MockKubernetesClient.client(TestCustomResource.class); + private final DefaultInformerPool pool = new DefaultInformerPool(); + + @BeforeEach + void setup() { + pool.setConfigurationService(new BaseConfigurationService()); + } + + @Test + void sharesSingleInformerForTheSameClassifier() { + var classifier = classifier("default"); + + var first = pool.getInformer(CONTROLLER, ES_NAME, classifier); + var second = pool.getInformer("other-controller", "other-es", classifier); + + assertThat(first).isSameAs(second); + assertThat(pool.size()).isEqualTo(1); + assertThat(pool.numberOfInformersForResource(TestCustomResource.class)).isEqualTo(1); + // the underlying informer must be created exactly once, not once per user + verify(client, times(1)).resources(TestCustomResource.class); + } + + @Test + void createsSeparateInformersForDifferentClassifiers() { + pool.getInformer(CONTROLLER, ES_NAME, classifier("ns1")); + pool.getInformer(CONTROLLER, ES_NAME, classifier("ns2")); + + assertThat(pool.size()).isEqualTo(2); + assertThat(pool.numberOfInformersForResource(TestCustomResource.class)).isEqualTo(2); + verify(client, times(2)).resources(TestCustomResource.class); + } + + @Test + void createsSeparateInformersForDifferentClientsWithTheSameApiServerUrl() { + var otherClient = MockKubernetesClient.client(TestCustomResource.class); + // the two clients are indistinguishable by URL, they are two different instances though and may + // well differ in credentials or TLS material, so they must not end up sharing an informer + assertThat(otherClient.getConfiguration().getMasterUrl()).isEqualTo(masterUrl()); + + pool.getInformer(CONTROLLER, ES_NAME, classifier("default")); + pool.getInformer("other-controller", "other-es", classifier(otherClient, "default")); + + assertThat(pool.size()).isEqualTo(2); + verify(client, times(1)).resources(TestCustomResource.class); + verify(otherClient, times(1)).resources(TestCustomResource.class); + } + + @Test + void sharesInformerWhenClassifiersDifferOnlyByListLimit() { + var withLimit100 = + new InformerClassifier<>( + client, null, null, "default", TestCustomResource.class, null, null, 100L, null); + var withLimit200 = + new InformerClassifier<>( + client, null, null, "default", TestCustomResource.class, null, null, 200L, null); + + var first = pool.getInformer(CONTROLLER, ES_NAME, withLimit100); + var second = pool.getInformer("other-controller", "other-es", withLimit200); + + assertThat(first).isSameAs(second); + assertThat(pool.size()).isEqualTo(1); + verify(client, times(1)).resources(TestCustomResource.class); + } + + @Test + void doesNotStopSharedInformerUntilLastRelease() { + var classifier = classifier("default"); + var informer = pool.getInformer(CONTROLLER, ES_NAME, classifier); + pool.getInformer("other-controller", "other-es", classifier); + + pool.releaseInformer(CONTROLLER, ES_NAME, classifier); + verify(informer, never()).stop(); + assertThat(pool.size()).isEqualTo(1); + + pool.releaseInformer("other-controller", "other-es", classifier); + verify(informer, times(1)).stop(); + assertThat(pool.size()).isZero(); + } + + @Test + void releaseReturnsInformerEvenWhileStillShared() { + var classifier = classifier("default"); + var informer = pool.getInformer(CONTROLLER, ES_NAME, classifier); + pool.getInformer("other-controller", "other-es", classifier); + + // the caller needs the (still-running) informer back so it can remove its own event handler + var released = pool.releaseInformer(CONTROLLER, ES_NAME, classifier); + + assertThat(released).containsSame(informer); + verify(informer, never()).stop(); + } + + @Test + void releaseOfUnknownClassifierReturnsEmptyAndDoesNotThrow() { + var released = pool.releaseInformer(CONTROLLER, ES_NAME, classifier("never-registered")); + + assertThat(released).isEmpty(); + assertThat(pool.size()).isZero(); + } + + private String masterUrl() { + return client.getConfiguration().getMasterUrl(); + } + + private InformerClassifier classifier(String namespace) { + return classifier(client, namespace); + } + + private InformerClassifier classifier( + KubernetesClient forClient, String namespace) { + return new InformerClassifier<>( + forClient, null, null, namespace, TestCustomResource.class, null, null, null, null); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifierTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifierTest.java new file mode 100644 index 0000000000..47f359a3f3 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifierTest.java @@ -0,0 +1,315 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer.pool; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.client.Config; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; +import io.javaoperatorsdk.operator.api.config.informer.FieldSelector; +import io.javaoperatorsdk.operator.processing.GroupVersionKind; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResourceOtherV1; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Unit tests for the identity semantics of {@link InformerClassifier}. The classifier is used as + * the key that decides whether two controllers share a single pooled informer, so its equality + * contract (in particular that {@code informerListLimit} is intentionally excluded and that the + * client is compared by identity) is load-bearing. + */ +class InformerClassifierTest { + + private static final KubernetesClient CLIENT = mock(KubernetesClient.class); + private static final String LABEL = "app=foo"; + private static final String SHARD = "shard-1"; + private static final String NAMESPACE = "default"; + private static final GroupVersionKind GVK = new GroupVersionKind("sample.io/v1", "Foo"); + private static final FieldSelector FIELD_SELECTOR = + new FieldSelector(new FieldSelector.Field("status.phase", "Running")); + private static final Long LIMIT = 100L; + private static final ItemStore ITEM_STORE = mock(ItemStore.class); + + private static InformerClassifier base() { + return new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + LIMIT, + ITEM_STORE); + } + + @Test + void classifiersWithIdenticalFieldsAreEqual() { + assertThat(base()).isEqualTo(base()); + assertThat(base()).hasSameHashCodeAs(base()); + } + + @Test + void informerListLimitIsExcludedFromEqualityAndHashCode() { + var withOtherLimit = + new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + 999L, + ITEM_STORE); + + assertThat(base()).isEqualTo(withOtherLimit); + assertThat(base()).hasSameHashCodeAs(withOtherLimit); + } + + @Test + void toStringContainsTheApiServerUrlDerivedFromTheClient() { + // the URL is not a component of its own, but classifiers are logged and the client alone does + // not tell which cluster it connects to + var config = mock(Config.class); + when(config.getMasterUrl()).thenReturn("https://localhost:8443/"); + var client = mock(KubernetesClient.class); + when(client.getConfiguration()).thenReturn(config); + + var classifier = + new InformerClassifier<>( + client, + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + LIMIT, + ITEM_STORE); + + assertThat(classifier.toString()) + .contains("https://localhost:8443/") + .contains(NAMESPACE) + .contains(LABEL) + .contains(TestCustomResource.class.getName()); + } + + @Test + void toStringDoesNotFailWithoutAClient() { + var classifier = + new InformerClassifier<>( + null, + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + LIMIT, + ITEM_STORE); + + assertThat(classifier.toString()).contains(NAMESPACE); + } + + @Test + void differsWhenClientDiffers() { + // two distinct clients may differ in credentials, impersonation or TLS material even when they + // report the same master URL, so they must never end up sharing an informer + assertThat(base()) + .isNotEqualTo( + new InformerClassifier<>( + mock(KubernetesClient.class), + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + LIMIT, + ITEM_STORE)); + } + + @Test + void differsWhenLabelSelectorDiffers() { + assertThat(base()) + .isNotEqualTo( + new InformerClassifier<>( + CLIENT, + "app=bar", + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + LIMIT, + ITEM_STORE)); + } + + @Test + void differsWhenShardSelectorDiffers() { + assertThat(base()) + .isNotEqualTo( + new InformerClassifier<>( + CLIENT, + LABEL, + "shard-2", + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + LIMIT, + ITEM_STORE)); + } + + @Test + void differsWhenNamespaceDiffers() { + assertThat(base()) + .isNotEqualTo( + new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + "other-ns", + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + LIMIT, + ITEM_STORE)); + } + + @Test + void differsWhenResourceClassDiffers() { + // item stores are null here because their generic type is tied to the resource class, which is + // exactly the field under test; this keeps the resource class the only difference. + var forTestResource = + new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + LIMIT, + null); + var forOtherResource = + new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + NAMESPACE, + TestCustomResourceOtherV1.class, + GVK, + FIELD_SELECTOR, + LIMIT, + null); + + assertThat(forTestResource).isNotEqualTo(forOtherResource); + } + + @Test + void differsWhenGroupVersionKindDiffers() { + assertThat(base()) + .isNotEqualTo( + new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + new GroupVersionKind("sample.io/v1", "Bar"), + FIELD_SELECTOR, + LIMIT, + ITEM_STORE)); + } + + @Test + void differsWhenFieldSelectorDiffers() { + assertThat(base()) + .isNotEqualTo( + new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + new FieldSelector(new FieldSelector.Field("status.phase", "Pending")), + LIMIT, + ITEM_STORE)); + } + + @Test + void differsWhenItemStoreDiffers() { + assertThat(base()) + .isNotEqualTo( + new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + LIMIT, + mock(ItemStore.class))); + } + + @Test + void differsOnlyByInformerListLimitIsTrueWhenOnlyLimitDiffers() { + var withOtherLimit = + new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + NAMESPACE, + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + 999L, + ITEM_STORE); + + assertThat(base().differsOnlyByInformerListLimit(withOtherLimit)).isTrue(); + } + + @Test + void differsOnlyByInformerListLimitIsFalseWhenFullyEqual() { + assertThat(base().differsOnlyByInformerListLimit(base())).isFalse(); + } + + @Test + void differsOnlyByInformerListLimitIsFalseWhenAnotherFieldDiffers() { + // different namespace AND different limit: not "only by limit" + var differentNamespaceAndLimit = + new InformerClassifier<>( + CLIENT, + LABEL, + SHARD, + "other-ns", + TestCustomResource.class, + GVK, + FIELD_SELECTOR, + 999L, + ITEM_STORE); + + assertThat(base().differsOnlyByInformerListLimit(differentNamespaceAndLimit)).isFalse(); + } +} diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPoolTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPoolTest.java new file mode 100644 index 0000000000..364103849e --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPoolTest.java @@ -0,0 +1,143 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer.pool; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +/** + * Unit tests for {@link NonSharingInformerPool}: unlike {@link DefaultInformerPool} it never shares + * informers. A distinct informer is created for every {@code controllerName}+{@code name}+{@code + * classifier} combination, and release is scoped to that same combination. + */ +class NonSharingInformerPoolTest { + + private static final String CONTROLLER = "controller"; + private static final String ES_NAME = "event-source"; + + private final KubernetesClient client = MockKubernetesClient.client(TestCustomResource.class); + private final NonSharingInformerPool pool = new NonSharingInformerPool(); + + @BeforeEach + void setUp() { + pool.setConfigurationService(new BaseConfigurationService()); + } + + @Test + void createsSeparateInformerForSameClassifierFromDifferentControllers() { + var classifier = classifier("default"); + + pool.getInformer(CONTROLLER, ES_NAME, classifier); + pool.getInformer("other-controller", ES_NAME, classifier); + + // no sharing: one informer created per user even for an identical classifier + assertThat(pool.size()).isEqualTo(2); + verify(client, times(2)).resources(TestCustomResource.class); + } + + @Test + void createsSeparateInformerForDifferentEventSourceNames() { + var classifier = classifier("default"); + + pool.getInformer(CONTROLLER, "event-source-1", classifier); + pool.getInformer(CONTROLLER, "event-source-2", classifier); + + assertThat(pool.size()).isEqualTo(2); + verify(client, times(2)).resources(TestCustomResource.class); + } + + @Test + void throwsWhenRequestingAnInformerForAnAlreadyRegisteredKey() { + var classifier = classifier("default"); + + pool.getInformer(CONTROLLER, ES_NAME, classifier); + + // requesting the same controller+event source+classifier combination again without releasing + // first would otherwise silently overwrite the map entry and leak the earlier informer + assertThatThrownBy(() -> pool.getInformer(CONTROLLER, ES_NAME, classifier)) + .isInstanceOf(OperatorException.class) + .hasMessageContaining(CONTROLLER) + .hasMessageContaining(ES_NAME) + .hasMessageContaining(classifier.toString()); + + // the earlier informer is left untouched, still registered exactly once + assertThat(pool.size()).isEqualTo(1); + verify(client, times(1)).resources(TestCustomResource.class); + } + + @Test + void allowsReRequestingAnInformerAfterItWasReleased() { + var classifier = classifier("default"); + pool.getInformer(CONTROLLER, ES_NAME, classifier); + pool.releaseInformer(CONTROLLER, ES_NAME, classifier); + + // must not throw: releasing frees up the key for reuse + pool.getInformer(CONTROLLER, ES_NAME, classifier); + + assertThat(pool.size()).isEqualTo(1); + verify(client, times(2)).resources(TestCustomResource.class); + } + + @Test + void releaseStopsAndRemovesTheInformer() { + var classifier = classifier("default"); + var informer = pool.getInformer(CONTROLLER, ES_NAME, classifier); + + var released = pool.releaseInformer(CONTROLLER, ES_NAME, classifier); + + assertThat(released).containsSame(informer); + verify(informer, times(1)).stop(); + assertThat(pool.size()).isZero(); + } + + @Test + void releaseIsScopedToControllerAndName() { + var classifier = classifier("default"); + var informer = pool.getInformer(CONTROLLER, ES_NAME, classifier); + + // same classifier but a different controller: nothing is released or stopped + var released = pool.releaseInformer("other-controller", ES_NAME, classifier); + + assertThat(released).isEmpty(); + verify(informer, never()).stop(); + assertThat(pool.size()).isEqualTo(1); + } + + @Test + void releaseOfUnknownInformerReturnsEmptyAndDoesNotThrow() { + var released = pool.releaseInformer(CONTROLLER, ES_NAME, classifier("never-registered")); + + assertThat(released).isEmpty(); + assertThat(pool.size()).isZero(); + } + + private InformerClassifier classifier(String namespace) { + return new InformerClassifier<>( + client, null, null, namespace, TestCustomResource.class, null, null, null, null); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/AbstractSharedInformerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/AbstractSharedInformerIT.java new file mode 100644 index 0000000000..e6da24b467 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/AbstractSharedInformerIT.java @@ -0,0 +1,107 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.basic; + +import java.util.function.Consumer; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.AbstractInformerPool; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Registers two controllers, each backed by its own primary custom resource, that both watch {@link + * ConfigMap} as a secondary resource using an identical {@code InformerEventSource} configuration. + * + *

The functional behavior (both controllers reconcile) is identical regardless of the informer + * pool strategy; only the number of underlying {@code ConfigMap} informers differs, which is why + * the expected count is left abstract. Concrete subclasses pick the pool strategy via {@link + * #configurationServiceOverrider()}. + */ +public abstract class AbstractSharedInformerIT { + + public static final String TEST_RESOURCE_1 = "test1"; + public static final String TEST_RESOURCE_2 = "test2"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withReconciler(new SharedInformerReconciler1()) + .withReconciler(new SharedInformerReconciler2()) + .withConfigurationService(configurationServiceOverrider()) + .build(); + + /** The informer pool strategy under test. */ + protected abstract Consumer configurationServiceOverrider(); + + /** + * Expected number of {@code ConfigMap} informers: {@code 1} when the two controllers share a + * single informer, {@code 2} when each controller gets its own. + */ + protected abstract long expectedConfigMapInformerCount(); + + @Test + void bothControllersReconcileWatchingConfigMap() { + extension.create(customResource1(TEST_RESOURCE_1)); + extension.create(customResource2(TEST_RESOURCE_2)); + + // both controllers reconcile, which guarantees their event sources (and thus their informers) + // have been started + await() + .untilAsserted( + () -> { + assertThat( + extension + .getReconcilerOfType(SharedInformerReconciler1.class) + .getNumberOfExecutions()) + .isPositive(); + assertThat( + extension + .getReconcilerOfType(SharedInformerReconciler2.class) + .getNumberOfExecutions()) + .isPositive(); + }); + + var pool = + (AbstractInformerPool) extension.getOperator().getConfigurationService().informerPool(); + + // the ConfigMap informer count depends on the pool strategy (shared vs. one-per-controller) + assertThat(pool.numberOfInformersForResource(ConfigMap.class)) + .isEqualTo(expectedConfigMapInformerCount()); + // the two distinct primary resources are always backed by their own informers + assertThat(pool.numberOfInformersForResource(SharedInformerCustomResource1.class)).isEqualTo(1); + assertThat(pool.numberOfInformersForResource(SharedInformerCustomResource2.class)).isEqualTo(1); + } + + SharedInformerCustomResource1 customResource1(String name) { + var res = new SharedInformerCustomResource1(); + res.setMetadata(new ObjectMetaBuilder().withName(name).build()); + return res; + } + + SharedInformerCustomResource2 customResource2(String name) { + var res = new SharedInformerCustomResource2(); + res.setMetadata(new ObjectMetaBuilder().withName(name).build()); + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java new file mode 100644 index 0000000000..bc356006ee --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java @@ -0,0 +1,39 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.basic; + +import java.util.function.Consumer; + +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.NonSharingInformerPool; + +/** + * Runs {@link AbstractSharedInformerIT} with the {@link NonSharingInformerPool}: informers are + * never shared, so each of the two controllers watching {@code ConfigMap} gets its own informer. + */ +public class NonSharingInformerPoolSharedInformerIT extends AbstractSharedInformerIT { + + @Override + protected Consumer configurationServiceOverrider() { + return overrider -> overrider.withInformerPool(new NonSharingInformerPool()); + } + + @Override + protected long expectedConfigMapInformerCount() { + // no sharing: one ConfigMap informer per controller + return 2; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource1.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource1.java new file mode 100644 index 0000000000..af73cb15ff --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource1.java @@ -0,0 +1,28 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.basic; + +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("si1") +public class SharedInformerCustomResource1 extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource2.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource2.java new file mode 100644 index 0000000000..e4dbf667c2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource2.java @@ -0,0 +1,28 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.basic; + +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("si2") +public class SharedInformerCustomResource2 extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.java new file mode 100644 index 0000000000..6e51bcb44a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.java @@ -0,0 +1,38 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.basic; + +import java.util.function.Consumer; + +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; + +/** + * Runs {@link AbstractSharedInformerIT} with the default (sharing) informer pool, so both + * controllers watching {@code ConfigMap} are backed by a single shared informer. + */ +public class SharedInformerIT extends AbstractSharedInformerIT { + + @Override + protected Consumer configurationServiceOverrider() { + // no override: use the default DefaultInformerPool + return overrider -> {}; + } + + @Override + protected long expectedConfigMapInformerCount() { + return 1; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler1.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler1.java new file mode 100644 index 0000000000..d526ff7713 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler1.java @@ -0,0 +1,63 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.basic; + +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +/** + * Watches {@link ConfigMap} as a secondary resource. Together with {@link + * SharedInformerReconciler2}, which watches the same secondary resource type with an identical + * configuration, this is used to verify that both controllers share a single underlying informer + * from the informer pool. + */ +@ControllerConfiguration +public class SharedInformerReconciler1 implements Reconciler { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public List> prepareEventSources( + EventSourceContext context) { + var config = + InformerEventSourceConfiguration.from(ConfigMap.class, SharedInformerCustomResource1.class) + .build(); + return List.of(new InformerEventSource<>(config, context)); + } + + @Override + public UpdateControl reconcile( + SharedInformerCustomResource1 resource, Context context) { + numberOfExecutions.incrementAndGet(); + resource.setStatus(new SharedInformerStatus()); + resource.getStatus().setReconciledBy(getClass().getSimpleName()); + return UpdateControl.patchStatus(resource); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler2.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler2.java new file mode 100644 index 0000000000..84f9718d52 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler2.java @@ -0,0 +1,62 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.basic; + +import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +/** + * Watches {@link ConfigMap} as a secondary resource with the same configuration as {@link + * SharedInformerReconciler1} so that both controllers share a single underlying informer from the + * informer pool. + */ +@ControllerConfiguration +public class SharedInformerReconciler2 implements Reconciler { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public List> prepareEventSources( + EventSourceContext context) { + var config = + InformerEventSourceConfiguration.from(ConfigMap.class, SharedInformerCustomResource2.class) + .build(); + return List.of(new InformerEventSource<>(config, context)); + } + + @Override + public UpdateControl reconcile( + SharedInformerCustomResource2 resource, Context context) { + numberOfExecutions.incrementAndGet(); + resource.setStatus(new SharedInformerStatus()); + resource.getStatus().setReconciledBy(getClass().getSimpleName()); + return UpdateControl.patchStatus(resource); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerStatus.java new file mode 100644 index 0000000000..446fee6e8b --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerStatus.java @@ -0,0 +1,29 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.basic; + +public class SharedInformerStatus { + + private String reconciledBy; + + public String getReconciledBy() { + return reconciledBy; + } + + public void setReconciledBy(String reconciledBy) { + this.reconciledBy = reconciledBy; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/AbstractDeregisterSharedInformerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/AbstractDeregisterSharedInformerIT.java new file mode 100644 index 0000000000..9840fbf91f --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/AbstractDeregisterSharedInformerIT.java @@ -0,0 +1,99 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.deregister; + +import java.util.function.Consumer; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.AbstractInformerPool; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Verifies the lifecycle of a dynamically registered event source in the informer pool: registering + * it creates a (single) informer for the watched resource, and de-registering it releases that + * informer so the pool no longer holds one for that resource type. + * + *

A single controller registers a single event source, so no informer sharing is involved: the + * behavior and assertions are identical for every pool strategy. Concrete subclasses only pick the + * strategy via {@link #configurationServiceOverrider()}. + */ +public abstract class AbstractDeregisterSharedInformerIT { + + private static final String PRIMARY_NAME = "primary1"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withAdditionalCustomResourceDefinition(DeregisterWatchedCustomResource.class) + .withReconciler(new DeregisterReconciler()) + .withConfigurationService(configurationServiceOverrider()) + .build(); + + /** The informer pool strategy under test. */ + protected abstract Consumer configurationServiceOverrider(); + + @Test + void deregisteringDynamicEventSourceRemovesInformerFromPool() { + var reconciler = extension.getReconcilerOfType(DeregisterReconciler.class); + var pool = + (AbstractInformerPool) extension.getOperator().getConfigurationService().informerPool(); + + // Create the primary with registration enabled: the reconciler dynamically registers the event + // source for the watched resource. + extension.create(primary(true)); + + // The dynamically registered event source establishes exactly one informer for the watched + // resource in the pool. + await() + .untilAsserted( + () -> { + assertThat(reconciler.getNumberOfExecutions()).isPositive(); + assertThat(pool.numberOfInformersForResource(DeregisterWatchedCustomResource.class)) + .isEqualTo(1); + }); + + var executionsBeforeDeregister = reconciler.getNumberOfExecutions(); + + // Flip the spec so the next reconciliation de-registers the event source. + var toUpdate = extension.get(DeregisterPrimaryCustomResource.class, PRIMARY_NAME); + toUpdate.getSpec().setRegisterEventSource(false); + extension.replace(toUpdate); + + // After the de-registration reconciliation runs, the informer is released from the pool. + await() + .untilAsserted( + () -> { + assertThat(reconciler.getNumberOfExecutions()) + .isGreaterThan(executionsBeforeDeregister); + assertThat(pool.numberOfInformersForResource(DeregisterWatchedCustomResource.class)) + .isZero(); + }); + } + + DeregisterPrimaryCustomResource primary(boolean registerEventSource) { + var res = new DeregisterPrimaryCustomResource(); + res.setMetadata(new ObjectMetaBuilder().withName(PRIMARY_NAME).build()); + res.setSpec(new DeregisterSpec().setRegisterEventSource(registerEventSource)); + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java new file mode 100644 index 0000000000..7e03081b1a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java @@ -0,0 +1,29 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.deregister; + +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; + +/** Primary resource of {@link DeregisterReconciler}. */ +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("dreg-p") +public class DeregisterPrimaryCustomResource extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterReconciler.java new file mode 100644 index 0000000000..680ce87077 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterReconciler.java @@ -0,0 +1,73 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.deregister; + +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; + +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +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.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +/** + * Dynamically registers an event source for {@link DeregisterWatchedCustomResource} while the + * primary's spec requests it, and dynamically de-registers it otherwise. Used to verify that + * de-registering a dynamically registered event source releases the underlying informer from the + * pool. + */ +@ControllerConfiguration +public class DeregisterReconciler implements Reconciler { + + public static final String WATCHED_EVENT_SOURCE_NAME = "deregister-watched-informer"; + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + DeregisterPrimaryCustomResource primary, Context context) { + numberOfExecutions.incrementAndGet(); + + if (primary.getSpec() != null && primary.getSpec().isRegisterEventSource()) { + context.eventSourceRetriever().dynamicallyRegisterEventSource(watchedEventSource(context)); + } else { + context.eventSourceRetriever().dynamicallyDeRegisterEventSource(WATCHED_EVENT_SOURCE_NAME); + } + + return UpdateControl.noUpdate(); + } + + private InformerEventSource + watchedEventSource(Context context) { + var config = + InformerEventSourceConfiguration.from( + DeregisterWatchedCustomResource.class, DeregisterPrimaryCustomResource.class) + .withName(WATCHED_EVENT_SOURCE_NAME) + .withSecondaryToPrimaryMapper( + (DeregisterWatchedCustomResource watched) -> + Set.of(new ResourceID("ignored", watched.getMetadata().getNamespace()))) + .build(); + return new InformerEventSource<>( + config, context.eventSourceRetriever().eventSourceContextForDynamicRegistration()); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSharedInformerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSharedInformerIT.java new file mode 100644 index 0000000000..8c9a209553 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSharedInformerIT.java @@ -0,0 +1,29 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.deregister; + +import java.util.function.Consumer; + +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; + +/** Runs {@link AbstractDeregisterSharedInformerIT} with the default (sharing) informer pool. */ +class DeregisterSharedInformerIT extends AbstractDeregisterSharedInformerIT { + + @Override + protected Consumer configurationServiceOverrider() { + return overrider -> {}; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSpec.java new file mode 100644 index 0000000000..c88b0cde9d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSpec.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.deregister; + +/** + * Spec of {@link DeregisterPrimaryCustomResource}; toggles the dynamic event source registration. + */ +public class DeregisterSpec { + + /** + * When {@code true} the reconciler dynamically registers the event source for {@link + * DeregisterWatchedCustomResource}; when {@code false} it de-registers it. + */ + private boolean registerEventSource = true; + + public boolean isRegisterEventSource() { + return registerEventSource; + } + + public DeregisterSpec setRegisterEventSource(boolean registerEventSource) { + this.registerEventSource = registerEventSource; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterWatchedCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterWatchedCustomResource.java new file mode 100644 index 0000000000..9593bbd146 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterWatchedCustomResource.java @@ -0,0 +1,32 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.deregister; + +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; + +/** + * The custom resource watched via a dynamically registered (and later de-registered) event source + * by {@link DeregisterReconciler}. It has no reconciler of its own. + */ +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("dreg-w") +public class DeregisterWatchedCustomResource extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/NonSharingInformerPoolDeregisterSharedInformerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/NonSharingInformerPoolDeregisterSharedInformerIT.java new file mode 100644 index 0000000000..eefc2e6c1b --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/NonSharingInformerPoolDeregisterSharedInformerIT.java @@ -0,0 +1,34 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.deregister; + +import java.util.function.Consumer; + +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.NonSharingInformerPool; + +/** + * Runs {@link AbstractDeregisterSharedInformerIT} with the {@link NonSharingInformerPool}. Since + * only one controller registers/de-registers a single event source, the register-then-release + * lifecycle (and thus the assertions) is identical to the sharing pool. + */ +class NonSharingInformerPoolDeregisterSharedInformerIT extends AbstractDeregisterSharedInformerIT { + + @Override + protected Consumer configurationServiceOverrider() { + return overrider -> overrider.withInformerPool(new NonSharingInformerPool()); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/AbstractDynamicSharedInformerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/AbstractDynamicSharedInformerIT.java new file mode 100644 index 0000000000..bb55e2c80d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/AbstractDynamicSharedInformerIT.java @@ -0,0 +1,140 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.dynamic; + +import java.time.Duration; +import java.util.function.Consumer; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; +import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.AbstractInformerPool; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +/** + * Two reconcilers watch the same "third" custom resource ({@link + * DynamicSharedInformerThirdCustomResource}) as a secondary resource, but register their informer + * event sources differently: + * + *

+ * + *

Regardless of the pool strategy, the dynamically registered event source must be triggered for + * the pre-existing third resource: on registration the framework replays the resources already in + * the running informer's cache to the newly added handler, which maps the third resource back to + * the dynamic reconciler's primary. + * + *

What differs by strategy is the number of underlying informers for the third resource: a + * sharing pool establishes a single informer used by both reconcilers, whereas a non-sharing pool + * creates one per reconciler. That expected count is left abstract; concrete subclasses pick the + * strategy via {@link #configurationServiceOverrider()}. + */ +public abstract class AbstractDynamicSharedInformerIT { + + private static final String THIRD_RESOURCE_NAME = "third1"; + + @RegisterExtension + LocallyRunOperatorExtension extension = + LocallyRunOperatorExtension.builder() + .withAdditionalCustomResourceDefinition(DynamicSharedInformerThirdCustomResource.class) + .withReconciler(new StaticSharedInformerReconciler()) + .withReconciler(new DynamicSharedInformerReconciler()) + .withConfigurationService(configurationServiceOverrider()) + .build(); + + /** The informer pool strategy under test. */ + protected abstract Consumer configurationServiceOverrider(); + + /** + * Expected number of informers for the third resource once both reconcilers watch it: {@code 1} + * when the informer is shared, {@code 2} when each reconciler gets its own. + */ + protected abstract long expectedThirdResourceInformerCount(); + + @Test + void dynamicallyRegisteredEventSourceReceivesInitialEvent() { + var staticReconciler = extension.getReconcilerOfType(StaticSharedInformerReconciler.class); + var dynamicReconciler = extension.getReconcilerOfType(DynamicSharedInformerReconciler.class); + + // The static reconciler's primary must exist so that third-resource events (which map to it) + // actually result in a reconciliation. + extension.create(primary1()); + // The third resource is created before the dynamic event source is registered, so it is a + // pre-existing resource from the perspective of the dynamically added handler. + extension.create(thirdResource()); + + // Sanity check that the informer machinery works at all: the static reconciler, whose handler + // was present from startup, is triggered by the (live) creation of the third resource. + await().untilAsserted(() -> assertThat(staticReconciler.getNumberOfExecutions()).isPositive()); + + // Creating the second primary triggers the dynamic reconciler, which registers its own event + // source for the third resource against an already-running informer. + extension.create(primary2()); + await().untilAsserted(() -> assertThat(dynamicReconciler.getNumberOfExecutions()).isPositive()); + + // (1) Informer count for the third resource, which depends on the pool strategy. + var pool = + (AbstractInformerPool) extension.getOperator().getConfigurationService().informerPool(); + await() + .untilAsserted( + () -> + assertThat( + pool.numberOfInformersForResource( + DynamicSharedInformerThirdCustomResource.class)) + .isEqualTo(expectedThirdResourceInformerCount())); + + // (2) The dynamically registered event source now watches the pre-existing third resource. On + // registration the framework replays the resources already in the running informer's cache to + // the newly added handler, which maps the third resource back to the dynamic reconciler's + // primary. This triggers a second reconciliation, in addition to the first one (the primary2 + // creation) that performed the registration. Without the replay the dynamic reconciler would + // only ever run once. + await() + .atMost(Duration.ofSeconds(15)) + .untilAsserted( + () -> assertThat(dynamicReconciler.getNumberOfExecutions()).isGreaterThanOrEqualTo(2)); + } + + DynamicSharedInformerPrimaryCustomResource1 primary1() { + var res = new DynamicSharedInformerPrimaryCustomResource1(); + res.setMetadata( + new ObjectMetaBuilder().withName(StaticSharedInformerReconciler.PRIMARY_NAME).build()); + return res; + } + + DynamicSharedInformerPrimaryCustomResource2 primary2() { + var res = new DynamicSharedInformerPrimaryCustomResource2(); + res.setMetadata( + new ObjectMetaBuilder().withName(DynamicSharedInformerReconciler.PRIMARY_NAME).build()); + return res; + } + + DynamicSharedInformerThirdCustomResource thirdResource() { + var res = new DynamicSharedInformerThirdCustomResource(); + res.setMetadata(new ObjectMetaBuilder().withName(THIRD_RESOURCE_NAME).build()); + return res; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.java new file mode 100644 index 0000000000..b39664a0c2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.java @@ -0,0 +1,37 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.dynamic; + +import java.util.function.Consumer; + +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; + +/** + * Runs {@link AbstractDynamicSharedInformerIT} with the default (sharing) informer pool: the static + * and dynamic reconcilers share a single informer for the third resource. + */ +class DynamicSharedInformerIT extends AbstractDynamicSharedInformerIT { + + @Override + protected Consumer configurationServiceOverrider() { + return overrider -> {}; + } + + @Override + protected long expectedThirdResourceInformerCount() { + return 1; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.java new file mode 100644 index 0000000000..7515f3b971 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.java @@ -0,0 +1,29 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.dynamic; + +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; + +/** Primary resource of {@link StaticSharedInformerReconciler}. */ +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("dsi1") +public class DynamicSharedInformerPrimaryCustomResource1 extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.java new file mode 100644 index 0000000000..55b16d3ab8 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.java @@ -0,0 +1,29 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.dynamic; + +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; + +/** Primary resource of {@link DynamicSharedInformerReconciler}. */ +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("dsi2") +public class DynamicSharedInformerPrimaryCustomResource2 extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerReconciler.java new file mode 100644 index 0000000000..1779a1b43e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerReconciler.java @@ -0,0 +1,81 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.dynamic; + +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; + +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +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.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +/** + * Watches the same {@link DynamicSharedInformerThirdCustomResource} ("third" custom resource) as + * {@link StaticSharedInformerReconciler}, but registers its informer event source + * dynamically from within {@link #reconcile} instead of statically at startup. Since the + * configuration matches the static reconciler's, the pool is expected to hand out the same, + * already-running informer, so the two reconcilers share a single informer for the third resource. + * + *

The event source is registered while the shared informer is already running and has the + * pre-existing third resource in its cache. Because the handler is added to an already-running + * informer, the framework replays the resources already in that informer's cache to this newly + * added handler on registration, so this reconciler is triggered for the pre-existing third + * resource. This is asserted by the integration test. + */ +@ControllerConfiguration +public class DynamicSharedInformerReconciler + implements Reconciler { + + public static final String PRIMARY_NAME = "dynamic-primary"; + public static final String THIRD_EVENT_SOURCE_NAME = "dynamic-third-informer"; + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public UpdateControl reconcile( + DynamicSharedInformerPrimaryCustomResource2 resource, + Context context) { + numberOfExecutions.incrementAndGet(); + context + .eventSourceRetriever() + .dynamicallyRegisterEventSource(thirdResourceEventSource(context)); + return UpdateControl.noUpdate(); + } + + private InformerEventSource< + DynamicSharedInformerThirdCustomResource, DynamicSharedInformerPrimaryCustomResource2> + thirdResourceEventSource(Context context) { + var config = + InformerEventSourceConfiguration.from( + DynamicSharedInformerThirdCustomResource.class, + DynamicSharedInformerPrimaryCustomResource2.class) + .withName(THIRD_EVENT_SOURCE_NAME) + .withSecondaryToPrimaryMapper( + (DynamicSharedInformerThirdCustomResource third) -> + Set.of(new ResourceID(PRIMARY_NAME, third.getMetadata().getNamespace()))) + .build(); + return new InformerEventSource<>( + config, context.eventSourceRetriever().eventSourceContextForDynamicRegistration()); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerThirdCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerThirdCustomResource.java new file mode 100644 index 0000000000..fb4ad62586 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerThirdCustomResource.java @@ -0,0 +1,34 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.dynamic; + +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; + +/** + * The "third" custom resource that is watched as a secondary resource by two different reconcilers. + * It has no reconciler of its own. One reconciler watches it via a statically registered event + * source, the other via a dynamically registered one; both are expected to share a single informer + * for this type from the informer pool. + */ +@Group("sample.javaoperatorsdk") +@Version("v1") +@ShortNames("dsi3") +public class DynamicSharedInformerThirdCustomResource extends CustomResource + implements Namespaced {} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/NonSharingInformerPoolDynamicSharedInformerIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/NonSharingInformerPoolDynamicSharedInformerIT.java new file mode 100644 index 0000000000..f06d8155e2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/NonSharingInformerPoolDynamicSharedInformerIT.java @@ -0,0 +1,41 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.dynamic; + +import java.util.function.Consumer; + +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.NonSharingInformerPool; + +/** + * Runs {@link AbstractDynamicSharedInformerIT} with the {@link NonSharingInformerPool}: the static + * and dynamic reconcilers each get their own informer for the third resource. The dynamic + * reconciler is still triggered for the pre-existing third resource, because its own (newly + * created) informer replays the cache to the handler once it syncs. + */ +class NonSharingInformerPoolDynamicSharedInformerIT extends AbstractDynamicSharedInformerIT { + + @Override + protected Consumer configurationServiceOverrider() { + return overrider -> overrider.withInformerPool(new NonSharingInformerPool()); + } + + @Override + protected long expectedThirdResourceInformerCount() { + // no sharing: one informer for the static reconciler and one for the dynamic reconciler + return 2; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/StaticSharedInformerReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/StaticSharedInformerReconciler.java new file mode 100644 index 0000000000..be7bce6ce6 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/StaticSharedInformerReconciler.java @@ -0,0 +1,73 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.baseapi.informerpool.dynamic; + +import java.util.List; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; + +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; + +/** + * Watches {@link DynamicSharedInformerThirdCustomResource} (the "third" custom resource) as a + * secondary resource using a statically registered informer event source. Because this + * event source is registered at startup, its handler is present on the underlying informer before + * any third resource exists, so this reconciler is triggered by third-resource events. It is the + * counterpart to {@link DynamicSharedInformerReconciler}, which watches the same third resource but + * registers its event source dynamically. + */ +@ControllerConfiguration +public class StaticSharedInformerReconciler + implements Reconciler { + + public static final String PRIMARY_NAME = "static-primary"; + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + + @Override + public List> prepareEventSources( + EventSourceContext context) { + var config = + InformerEventSourceConfiguration.from( + DynamicSharedInformerThirdCustomResource.class, + DynamicSharedInformerPrimaryCustomResource1.class) + .withSecondaryToPrimaryMapper( + (DynamicSharedInformerThirdCustomResource third) -> + Set.of(new ResourceID(PRIMARY_NAME, third.getMetadata().getNamespace()))) + .build(); + return List.of(new InformerEventSource<>(config, context)); + } + + @Override + public UpdateControl reconcile( + DynamicSharedInformerPrimaryCustomResource1 resource, + Context context) { + numberOfExecutions.incrementAndGet(); + return UpdateControl.noUpdate(); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java index a44b3e13f3..52cbd79f52 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java @@ -28,6 +28,7 @@ import io.javaoperatorsdk.operator.api.config.*; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.AbstractInformerPool; import static org.assertj.core.api.Assertions.assertThat; import static org.awaitility.Awaitility.await; @@ -127,6 +128,12 @@ public Set getKnownReconcilerNames() { public Version getVersion() { return null; } + + // only used here to obtain the resource cloner, so the pool is never accessed + @Override + public AbstractInformerPool informerPool() { + return null; + } }.getResourceCloner(); } } From 2952d9ec6e5a4b7ae3a0eb70e037fbb043984000 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 31 Jul 2026 15:58:19 +0200 Subject: [PATCH 2/3] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- .../event/source/informer/InformerManager.java | 18 +++++++++--------- .../informer/ManagedInformerEventSource.java | 2 +- .../InformerManagerConcurrentReleaseTest.java | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index edb8bf718a..3908bbcf09 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -51,14 +51,19 @@ class InformerManager> private final Map> sources = new ConcurrentHashMap<>(); private final C configuration; private final ResourceEventHandler eventHandler; + // the identity of the event source these informers are managed for, towards the pool and towards + // the index names on a shared informer. Deliberately the event source's own name rather than + // InformerConfiguration#getName, which is null unless the event source was explicitly named + private final String eventSourceName; private final Map>> indexers = new HashMap<>(); private ControllerConfiguration controllerConfiguration; private InformerPool informerPool; private KubernetesClient targetClient; - InformerManager(C configuration, ResourceEventHandler eventHandler) { + InformerManager(C configuration, ResourceEventHandler eventHandler, String eventSourceName) { this.configuration = configuration; this.eventHandler = eventHandler; + this.eventSourceName = eventSourceName; } void setControllerConfiguration(ControllerConfiguration controllerConfiguration) { @@ -139,17 +144,14 @@ private InformerWrapper getEventSourceForNamespace(String namespaceIdentifier final InformerWrapper source; InformerClassifier classifier = getClassifier(namespaceIdentifier); var informer = - informerPool.getInformer( - controllerConfiguration.getName(), - configuration.getInformerConfig().getName(), - classifier); + informerPool.getInformer(controllerConfiguration.getName(), eventSourceName, classifier); source = new InformerWrapper<>( informer, namespaceIdentifier, classifier, controllerConfiguration.getName(), - configuration.getInformerConfig().getName()); + eventSourceName); sources.put(namespaceIdentifier, source); source.addIndexers(indexers); source.addEventHandler(eventHandler); @@ -219,9 +221,7 @@ private void releaseSource(String namespaceIdentifier) { wrapper.removeIndexers(); informerPool .releaseInformer( - controllerConfiguration.getName(), - configuration.getInformerConfig().getName(), - wrapper.getClassifier()) + controllerConfiguration.getName(), eventSourceName, wrapper.getClassifier()) .ifPresent(i -> i.removeEventHandler(eventHandler)); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 5945936ee1..5aecf34eb0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -165,7 +165,7 @@ public synchronized void start() { return; } temporaryResourceCache = new TemporaryResourceCache<>(comparableResourceVersions, this); - this.cache = new InformerManager<>(configuration, this); + this.cache = new InformerManager<>(configuration, this, name()); cache.setControllerConfiguration(controllerConfiguration); cache.addIndexers(indexers); // A dynamically registered event source may join an already-running shared informer whose cache diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.java index a6906f85cf..e8801781db 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.java @@ -76,7 +76,7 @@ void setup() { void concurrentStopAndNamespaceChangeReleaseTheInformerOnlyOnce() throws Exception { var manager = new InformerManager>( - configuration, eventHandler); + configuration, eventHandler, "event-source"); manager.setControllerConfiguration(controllerConfiguration()); manager.start(); // a second controller shares the very same informer and never releases it, so the pool has to From 93dae2d0d2e2bd059885864418f115b98b529369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 31 Jul 2026 15:58:29 +0200 Subject: [PATCH 3/3] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- ...edInformerEventSourcePoolIdentityTest.java | 164 ++++++++++++++++++ 1 file changed, 164 insertions(+) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourcePoolIdentityTest.java diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourcePoolIdentityTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourcePoolIdentityTest.java new file mode 100644 index 0000000000..a2d0759fbc --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourcePoolIdentityTest.java @@ -0,0 +1,164 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.processing.event.source.informer; + +import java.util.List; +import java.util.Optional; +import java.util.Set; +import java.util.concurrent.CopyOnWriteArrayList; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.apps.Deployment; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.fabric8.kubernetes.client.informers.cache.Cache; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerEventSourceConfiguration; +import io.javaoperatorsdk.operator.processing.event.EventHandler; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.DefaultInformerPool; +import io.javaoperatorsdk.operator.processing.event.source.informer.pool.InformerClassifier; +import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * The name an event source is known by in the pool decides two things: which pool entry it shares + * (or, with a non-sharing pool, occupies), and the namespace its index names live in on a shared + * informer. It therefore has to identify the event source, which {@link + * InformerConfiguration#getName()} does not: that is {@code null} unless the event source was + * explicitly named, which would collapse every unnamed event source of a controller onto one + * identity. + */ +@SuppressWarnings({"rawtypes", "unchecked"}) +class ManagedInformerEventSourcePoolIdentityTest { + + private static final String NAMESPACE = "ns1"; + private static final String CONTROLLER = "controller"; + + private final KubernetesClient clientMock = MockKubernetesClient.client(Deployment.class); + private final RecordingInformerPool pool = new RecordingInformerPool(); + + @BeforeEach + void setup() { + SharedIndexInformer informer = + clientMock + .resources(Deployment.class) + .inNamespace(NAMESPACE) + .withLabelSelector((String) null) + .withShardSelector(null) + .runnableInformer(0); + when(informer.getStore()).thenReturn(mock(Cache.class)); + } + + @Test + void anUnnamedEventSourceIsIdentifiedByItsOwnGeneratedName() { + var eventSource = startedEventSource(); + + assertThat(eventSource.name()).isNotNull(); + assertThat(pool.acquiredNames) + .as("the pool has to see the event source's own name, not the unset configured one") + .containsExactly(eventSource.name()); + } + + @Test + void twoUnnamedEventSourcesOfOneControllerAreIdentifiedDistinctly() { + var first = startedEventSource(); + var second = startedEventSource(); + + // identical configuration, so both resolve the same classifier and share one informer: only the + // name keeps them apart, both as pool users and as owners of their index names + assertThat(pool.acquiredNames).doesNotHaveDuplicates(); + assertThat(first.name()).isNotEqualTo(second.name()); + } + + @Test + void releaseUsesTheSameNameAsTheAcquisition() { + var eventSource = startedEventSource(); + + eventSource.stop(); + + // an asymmetry here would leave the pool holding a reference forever + assertThat(pool.releasedNames).isEqualTo(pool.acquiredNames); + } + + private InformerEventSource startedEventSource() { + var configuration = mock(InformerEventSourceConfiguration.class); + var informerConfig = mock(InformerConfiguration.class); + when(informerConfig.getEffectiveNamespaces(any())).thenReturn(Set.of(NAMESPACE)); + when(informerConfig.getInformerListLimit()).thenReturn(null); + // the event source is not named, which is what makes the configured name null + when(informerConfig.getName()).thenReturn(null); + when(configuration.getInformerConfig()).thenReturn(informerConfig); + when(configuration.getResourceClass()).thenReturn(Deployment.class); + var secondaryToPrimaryMapper = mock(SecondaryToPrimaryMapper.class); + when(secondaryToPrimaryMapper.toPrimaryResourceIDs(any())) + .thenReturn(Set.of(new ResourceID("name", NAMESPACE))); + when(configuration.getSecondaryToPrimaryMapper()).thenReturn(secondaryToPrimaryMapper); + + var configurationService = + ConfigurationService.newOverriddenConfigurationService( + new BaseConfigurationService(), + o -> o.withKubernetesClient(clientMock).withInformerPool(pool)); + var controllerConfiguration = mock(ControllerConfiguration.class); + when(controllerConfiguration.getConfigurationService()).thenReturn(configurationService); + when(controllerConfiguration.getName()).thenReturn(CONTROLLER); + + var eventSource = new InformerEventSource(configuration); + eventSource.setEventHandler(mock(EventHandler.class)); + eventSource.setControllerConfiguration(controllerConfiguration); + eventSource.start(); + return eventSource; + } + + /** Records the identities the pool is asked about, and does not start the mocked informers. */ + private static class RecordingInformerPool extends DefaultInformerPool { + + private final List acquiredNames = new CopyOnWriteArrayList<>(); + private final List releasedNames = new CopyOnWriteArrayList<>(); + + @Override + public SharedIndexInformer getInformer( + String controllerName, String name, InformerClassifier classifier) { + acquiredNames.add(name); + return super.getInformer(controllerName, name, classifier); + } + + @Override + public Optional> releaseInformer( + String controllerName, String name, InformerClassifier classifier) { + releasedNames.add(name); + return super.releaseInformer(controllerName, name, classifier); + } + + @Override + public void start( + SharedIndexInformer informer, InformerClassifier classifier) { + // the informers here are mocks, starting them would add nothing + } + } +}