Informer pools

[csviri]

Summary

Introduces an informer pool so that informers backing InformerEventSources (and the internal controller event source) can be shared across controllers instead of each controller/event source always creating its own SharedIndexInformer. This reduces the number of watch connections opened against the API server and memory overhead in operators where multiple controllers (or multiple dynamically-registered event sources) watch the same secondary resource type (e.g. ConfigMap, Secret).

This is marked @Experimental: the pooling behavior itself is production ready, but the configuration API may still evolve in a non-backwards-compatible way.

What's new

InformerPool abstraction (processing/event/source/informer/pool)

• InformerPool — interface for getting/releasing informers, keyed by an InformerClassifier, plus reference counting via numberOfInformersForResource.
• InformerClassifier — identifies "equivalent" informer requests: API server URL, label/field/shard selectors, namespace, resource type (or GVK for generic resources), and item store. informerListLimit and indexers are intentionally excluded from identity (a warning is logged if an existing informer is reused with a different list limit; indexers can be added independently to a shared informer as long as names don't collide).
• AbstractInformerPool — shared logic for building the underlying SharedIndexInformer from a classifier (selectors, field selectors, item store, list limit, exception/stopped handlers) and starting it with the configured cacheSyncTimeout.
• DefaultInformerPool (new default) — reference-counted, sharing pool: creates an informer on first request for a given classifier and reuses it for subsequent requests; only stops it once the last user releases it.
• NonSharingInformerPool — opt-out pool that creates a dedicated informer per event source, preserving pre-pooling behavior.

Configuration

• ConfigurationService.informerPool() — new (non-default) method; implementations must return the same instance on every call so controllers using the same ConfigurationService share the pool. AbstractConfigurationService caches a DefaultInformerPool lazily (synchronized to avoid creating more than one instance under concurrent first access).
• ConfigurationServiceOverrider.withInformerPool(InformerPool) — lets users opt into AlwaysNewInformerPool (or a custom implementation) to disable/customize sharing.

Integration

• InformerManager and InformerWrapper were reworked to go through the pool instead of creating/starting/stopping the SharedIndexInformer directly:
  • InformerManager now builds an InformerClassifier per namespace and calls informerPool.getInformer(...) / releaseInformer(...) instead of constructing the informer from the Fabric8 client itself.
  • Starting/stopping is now delegated to the pool: getInformer only registers/reference-counts, InformerPool#start blocks (idempotently) until cache sync, and releaseInformer decrements the reference count, stopping the informer only when it drops to zero. The event handler is always removed from the returned informer, even when it stays running for other users.
  • InformerWrapper no longer owns lifecycle (start/stop removed); it now just exposes the informer/classifier and cache-facing behavior.
• ControllerEventSource and ManagedInformerEventSource updated to go through the same pooled path.
• Dynamic registration/deregistration of event sources correctly reuses/releases pooled informers, replaying existing cache state to newly added handlers when attaching to an already-running shared informer.

Docs

Added a new "Sharing Informers Between Controllers (Informer Pool)" section to docs/content/en/docs/documentation/eventing.md describing sharing semantics, what counts toward informer identity, and how to select/override the pooling strategy.

Tests

• Unit tests for NonSharingInformerPool, DefaultInformerPool, and InformerClassifier (equality/identity semantics).
• New integration tests under informerpool:
  • basic — two reconcilers sharing an informer for the same secondary resource type, parameterized to also run against AlwaysNewInformerPool for regression coverage.
  • deregister — verifies informer lifecycle when an event source is dynamically deregistered while shared.
  • dynamic — verifies dynamically-registered/static shared informers, including replay of existing cache state to a newly attached handler.
• Existing tests (ControllerTest, EventSourceManagerTest, ReconciliationDispatcherTest, ControllerEventSourceTest, InformerEventSourceTest, MockKubernetesClient, StandaloneDependentResourceIT, WorkflowMultipleActivationIT) updated for the new construction/wiring paths.

[Copilot]

Pull request overview

Introduces initial scaffolding for “event source pooling” (starting with informers) under processing.event.source.pool, likely to enable sharing/reuse of informers across components/controllers.

Changes:

• Added EventSourcePool interface and AbstractEventSourcePool base type.
• Added InformerClassifier record to key pooled informers by selector/namespace/resource type.
• Added initial (currently incomplete) InformerPool implementation and a minor whitespace cleanup in InformerManager.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/pool/InformerPool.java	Adds a new pool for SharedIndexInformer<?> instances (currently stubbed/incomplete).
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/pool/InformerClassifier.java	Adds a classifier record intended as the cache key for pooled informers.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/pool/EventSourcePool.java	Introduces a generic pool interface for event sources.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/pool/AbstractEventSourcePool.java	Adds a base class placeholder for pool implementations.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java	Removes an extraneous whitespace line in createEventSource.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (2)

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AlwaysNewInformerPool.java:45

• The containsKey(...) + put(...) sequence is not atomic on a ConcurrentHashMap. If two threads request the same (controllerName,name,classifier) at the same time, both can pass containsKey and one will overwrite the other, leaking an informer instance. Use putIfAbsent (or compute) to make registration atomic, and stop the newly created informer if it loses the race.

    var key = new ClassifierWithName(controllerName, name, classifier);
    if (informers.containsKey(key)) {
      throw new OperatorException(

operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java:136

• informerPool() returning null can lead to a hard-to-diagnose NullPointerException if this test (or the default methods it calls) ever ends up touching the pool. If the pool is intentionally unsupported here, it’s safer to throw an explicit UnsupportedOperationException.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (4)

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AlwaysNewInformerPool.java:58

• The containsKey(...) check followed by informers.put(...) is not atomic. Under concurrent calls, two threads can both pass the containsKey check, create two informers, and one put will overwrite the other entry (leaking the overwritten informer) before the exception path is hit. Use an atomic putIfAbsent/compute and, if you still throw on duplicates, ensure any newly created informer is stopped before throwing to avoid leaking watch threads/connections.

    var informer = createInformer(classifier, client);
    informers.put(key, informer);
    return informer;

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.java:140

• The thread name format string looks accidentally duplicated ("InformerInfo[informerInfo...") which makes debugging output harder to read/grep. Consider simplifying to a single, consistent prefix (e.g., "InformerInfo[] ").

      thread.setName(
          "InformerInfo[" + informer.getApiTypeClass().getSimpleName() + "] " + thread.getId());
      final var resourceName = informer.getApiTypeClass().getSimpleName();
      // idempotent: if the informer was already started (e.g. by the pool when it was
      // created/reused), this just returns the existing start future without restarting it
      var start = informer.start();

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java:109

• This toString/informerInfo label contains an extra "informerInfo" token ("InformerWrapper [informerInfo]") which looks like an unintentional duplication and makes logs less clear. Consider emitting just the type name (and optionally namespace) in the wrapper label.

  private String informerInfo() {
    return "InformerWrapper [informerInfo" + informer.getApiTypeClass().getSimpleName() + "]";
  }

operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java:136

• Returning null from ConfigurationService#informerPool() violates the new contract and will cause a NullPointerException if this helper ConfigurationService is ever used beyond the current narrow purpose. Prefer failing fast with an UnsupportedOperationException (or returning a real pool) so future refactors don't introduce a hard-to-diagnose NPE.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java:109

• informerInfo() currently returns "InformerWrapper [informerInfo]", which looks like a leftover debug label and makes toString()/logs confusing when diagnosing informer issues. It should format consistently without the stray "informerInfo" token.

  private String informerInfo() {
    return "InformerWrapper [informerInfo" + informer.getApiTypeClass().getSimpleName() + "]";
  }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AlwaysNewInformerPool.java:47

• getInformer() uses informers.containsKey(key) followed by informers.put(key, informer). With a ConcurrentHashMap this check-then-act sequence is not atomic: two concurrent callers can both observe the key as absent, create two informers, and one put will overwrite the other (leaking an informer) without throwing the intended OperatorException. Use putIfAbsent (or compute) to make the registration atomic and stop the newly-created informer on collision.

    var key = new ClassifierWithName(controllerName, name, classifier);
    if (informers.containsKey(key)) {
      throw new OperatorException(
          "Informer already registered for controller: "
              + controllerName

operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java:136

• This test-only ConfigurationService implementation returns null from informerPool(). Even if currently “never accessed”, it’s brittle and can turn into an unexpected NPE if the cloner implementation (or any other default method used here) starts consulting informerPool(). Since the test already has a running LocallyRunOperatorExtension, it can avoid the ad-hoc ConfigurationService entirely and reuse the operator’s real ConfigurationService/resource cloner instead.

[Copilot]

Pull request overview

Copilot reviewed 52 out of 52 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AlwaysNewInformerPool.java:47

• getInformer uses containsKey followed by put, which is not atomic even with ConcurrentHashMap. Under concurrent calls for the same (controllerName, event source name, classifier) key, this can still create (and potentially leak) multiple informers and bypass the intended "already registered" exception. Use putIfAbsent (or computeIfAbsent) so the check+insert is atomic, and stop the extra informer if another thread won the race.

    var key = new ClassifierWithName(controllerName, name, classifier);
    if (informers.containsKey(key)) {
      throw new OperatorException(
          "Informer already registered for controller: "
              + controllerName

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java:109

• informerInfo() currently returns "InformerWrapper [informerInfo" + ..., which looks like an accidental leftover token and makes toString() output confusing.

  private String informerInfo() {
    return "InformerWrapper [informerInfo" + informer.getApiTypeClass().getSimpleName() + "]";
  }

[csviri]

@SamBarker @k-wall I saw that you added explicit support in kroxy operator to share informers, in case you are interested and have the bandwidth PTAL if this makes sense for you; this should solve that problem out of the box. thank you!

[k-wall]

@SamBarker @k-wall I saw that you added explicit support in kroxy operator to share informers, in case you are interested and have the bandwidth PTAL if this makes sense for you; this should solve that problem out of the box. thank you!

Thank you for the heads up. We'll take a look and feed back.

[xstefank]

approving, I have only opinioned enhancements. Feel free to comment on each and we can either do it in this PR or as followups.

[Copilot]

Pull request overview

Copilot reviewed 59 out of 59 changed files in this pull request and generated no new comments.

Suppressed comments (4)

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java:43

• NonSharingInformerPool#getInformer uses containsKey followed by put, which is not atomic on a ConcurrentHashMap. Two concurrent calls for the same (controllerName, eventSourceName, classifier) can both pass the containsKey check and then overwrite each other, leaking the earlier informer (never released/stopped). Use putIfAbsent (or compute) to make registration atomic and stop the newly-created informer if a previous one was already present.

    var key = new ClassifierWithName(controllerName, name, classifier);
    if (informers.containsKey(key)) {
      throw new OperatorException(
          "Informer already registered for controller: "
              + controllerName

operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java:136

• This test ConfigurationService implementation returns null from informerPool(). With informer pooling now part of normal wiring, accidental access will fail with an NPE (and the comment here may become untrue as the code evolves). Return a non-null pool instance wired with this ConfigurationService instead, even if the test currently doesn't use it.
  operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java:174
• getTargetClient() is meant to resolve and cache the client once, but it is not synchronized/volatile. When changeNamespaces(...) starts multiple namespaces in parallel, multiple threads can observe targetClient == null and fetch different ConfigurationService#getKubernetesClient() instances (the default implementation creates a new client per call), producing inconsistent classifiers within the same manager and preventing sharing. Make the accessor synchronized (or otherwise thread-safe) to ensure only one client instance is ever chosen.

  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.

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java:157

• The isAssignableFrom check in versionedFullResourceName() is reversed. As written, it returns false for subclasses of GenericKubernetesResource, so generic resources may be reported with the wrong type name (affecting logs / errors). The usual pattern is GenericKubernetesResource.class.isAssignableFrom(apiTypeClass).

  private String versionedFullResourceName() {
    final var apiTypeClass = informer.getApiTypeClass();
    if (apiTypeClass.isAssignableFrom(GenericKubernetesResource.class)) {
      return GenericKubernetesResource.class.getSimpleName();
    }

[Copilot]

Pull request overview

Copilot reviewed 59 out of 59 changed files in this pull request and generated 1 comment.

Suppressed comments (4)

operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java:136

• ConfigurationService#informerPool() returns null here. Even if this helper currently only calls getResourceCloner(), returning null violates the new ConfigurationService contract and will cause a NullPointerException if this ConfigurationService instance is ever used by code paths that resolve/start informers (e.g. future refactors of this IT). Returning a cached, properly-initialized pool makes the test more robust.
  operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java:65
• The warning logged when a release misses the pool only includes the classifier. In this pool the lookup key also includes controllerName and name, so omitting them makes the message much harder to act on when diagnosing mismatched release calls.

    } else {
      log.warn("Informer was not found for classifier: {}", classifier);
    }

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java:195

• PR description mentions ConfigurationServiceOverrider.withInformerPool(InformerPool), but the implemented API takes AbstractInformerPool. Please align the PR description (or the API) so users aren’t misled about which type they can pass and whether implementing the interface alone is sufficient.

  public ConfigurationServiceOverrider withInformerPool(AbstractInformerPool informerPool) {
    this.informerPool = informerPool;
    return this;

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.java:89

• When a release misses the pool, the warning currently logs only the classifier. Including controllerName and name would make diagnosing double-releases / mismatched keys much easier, especially now that informers can be shared across controllers.

    if (pooled == null) {
      log.warn("No informer found in the pool for classifier: {}", classifier);
      return Optional.empty();