Skip to content

fix: invoke ConfigurationServiceOverrider consumer only once in Operator constructor#3353

Open
Dennis-Mircea wants to merge 1 commit into
operator-framework:mainfrom
Dennis-Mircea:fix/operator-init-config-service-duplicate-overrider-invocation
Open

fix: invoke ConfigurationServiceOverrider consumer only once in Operator constructor#3353
Dennis-Mircea wants to merge 1 commit into
operator-framework:mainfrom
Dennis-Mircea:fix/operator-init-config-service-duplicate-overrider-invocation

Conversation

@Dennis-Mircea
Copy link
Copy Markdown
Contributor

@Dennis-Mircea Dennis-Mircea commented May 15, 2026

Summary

Operator.initConfigurationService invokes the caller-supplied Consumer<ConfigurationServiceOverrider> twice when the Operator(Consumer<ConfigurationServiceOverrider>) constructor is used. Any side-effecting code inside that consumer (e.g. logging) is therefore emitted twice on startup.

Observed in downstream operators such as Apache Flink Kubernetes Operator, which configures the operator via new Operator(this::overrideOperatorConfigs) and ends up logging each startup line twice:

2026-05-11 14:52:16,723 o.a.f.k.o.FlinkOperator [INFO] Configuring operator with 50 reconciliation threads.
2026-05-11 14:52:16,723 o.a.f.k.o.FlinkOperator [INFO] Operator leader election is disabled.
2026-05-11 14:52:16,724 o.a.f.k.o.FlinkOperator [INFO] Configuring operator with 50 reconciliation threads.
2026-05-11 14:52:16,724 o.a.f.k.o.FlinkOperator [INFO] Operator leader election is disabled.

Root cause

Today's initConfigurationService does a two-pass build when client == null:

// Pass 1: build a throwaway service just to extract the client
if (client == null) {
  var configurationService = ConfigurationService.newOverriddenConfigurationService(overrider);
  client = configurationService.getKubernetesClient();
}

// ... wrap the overrider to pin that exact client ...

// Pass 2: build the real service
return ConfigurationService.newOverriddenConfigurationService(overrider);

Each newOverriddenConfigurationService(overrider) call invokes the user's overrider once, so it runs twice in the client == null path. The Operator(KubernetesClient) path is unaffected and it skips the first pass.

The second pass exists to "pin" the resolved KubernetesClient back onto the configuration service via withKubernetesClient(...). That pinning is redundant: AbstractConfigurationService#getKubernetesClient() already memoizes the client lazily on first read, so any consumer asking the service for its client gets the same instance whether it was set explicitly or constructed lazily.

Fix

Collapse to a single pass:

protected ConfigurationService initConfigurationService(
    KubernetesClient client, Consumer<ConfigurationServiceOverrider> overrider) {
  if (client != null) {
      Consumer<ConfigurationServiceOverrider> bindClient =
          o -> o.withKubernetesClient(client);
      overrider = overrider == null ? bindClient : overrider.andThen(bindClient);
    }
  return ConfigurationService.newOverriddenConfigurationService(overrider);
}
  • When the caller passes an explicit KubernetesClient, the overrider is augmented to pin it (same behavior as before).
  • When no client is passed, the configuration service falls back to its already-documented lazy client construction (AbstractConfigurationService.java:177 - // lazy init to avoid needing initializing a client when not needed).

Compatibility

  • Public API: unchanged - same method signature, same return type, same protected visibility for subclass overrides (OperatorIT.OperatorExtension still overrides it; verified).
  • Observable behavior:
    • User-supplied overriders are now invoked exactly once instead of twice. Any code that relied on being invoked twice (logging counts, increment-style side effects) would change, but invoking a configuration overrider twice was never a documented contract and is a clear bug.
    • When no KubernetesClient is passed, the client is constructed lazily on first getKubernetesClient() call instead of eagerly during initConfigurationService. This matches the already-documented intent of the lazy initialization in AbstractConfigurationService.

Test plan

  • ./mvnw -pl operator-framework-core -am compile - succeeds, no new warnings.
  • ./mvnw -pl operator-framework-core -am spotless:check - passes.
  • ./mvnw -pl operator-framework-core -am test -Dtest='ConfigurationServiceOverriderTest,LeaderElectionManagerTest,ControllerTest,ReconciliationDispatcherTest,EventProcessorTest' - 77/77 pass.

Copilot AI review requested due to automatic review settings May 15, 2026 11:25
@openshift-ci openshift-ci Bot requested review from csviri and metacosm May 15, 2026 11:25
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Refactors Operator#initConfigurationService to compose the provided KubernetesClient into the configuration overrider only when a client is explicitly supplied, simplifying initialization.

Changes:

  • Augments the overrider to bind a provided KubernetesClient (when non-null).
  • Removes the previous two-step creation of a ConfigurationService used only to obtain a client instance.
  • Simplifies the control flow around client binding.

Comment thread operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java Outdated
@Dennis-Mircea Dennis-Mircea force-pushed the fix/operator-init-config-service-duplicate-overrider-invocation branch 2 times, most recently from 8ec8b10 to 1ff1043 Compare May 15, 2026 12:23
@Dennis-Mircea
Copy link
Copy Markdown
Contributor Author

Please re-run the workflow. It was previously failing the Spotless check because I committed the GitHub Copilot suggestion as-is, assuming it was already formatted correctly.

Copy link
Copy Markdown
Collaborator

@csviri csviri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

thank you @Dennis-Mircea !

…tor constructor

Signed-off-by: Dennis-Mircea Ciupitu <dennis.mircea.ciupitu@gmail.com>
@csviri csviri force-pushed the fix/operator-init-config-service-duplicate-overrider-invocation branch from 1ff1043 to b3b5fd2 Compare May 15, 2026 13:12
@csviri csviri requested a review from xstefank May 15, 2026 13:15
@csviri
Copy link
Copy Markdown
Collaborator

csviri commented May 15, 2026

@metacosm please take a look. thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants