Skip to content

fix: do not output warning when resolving a configuration #2892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: do not output warning when resolving a configuration #2892

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b75ff27
fix: do not output warning when resolving a configuration
metacosm Aug 8, 2025
b4186cd
fix: remove test code that shouldn't have been included
metacosm Aug 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions ...k-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@

import static io.javaoperatorsdk.operator.api.config.ControllerConfiguration.CONTROLLER_NAME_AS_FIELD_MANAGER;

/**
* A default {@link ConfigurationService} implementation, resolving {@link Reconciler}s
* configuration when it has already been resolved before. If this behavior is not adequate, please
* use {@link AbstractConfigurationService} instead as a base for your {@code ConfigurationService}
* implementation.
*/
public class BaseConfigurationService extends AbstractConfigurationService {

private static final String LOGGER_NAME = "Default ConfigurationService implementation";
Expand Down Expand Up @@ -149,10 +155,12 @@ private static void configureFromAnnotatedReconciler(

@Override
protected void logMissingReconcilerWarning(String reconcilerKey, String reconcilersNameMessage) {
logger.warn(
"Configuration for reconciler '{}' was not found. {}",
reconcilerKey,
reconcilersNameMessage);
if (!createIfNeeded()) {
Copy link
Collaborator

@csviri csviri Aug 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

what is the probelm we are solving this method evetually?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean in quarkus operator sdk

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right now, the issue is preserving the possibility to log (or do something else, if needed) if the configuration doesn't exist in the abstract class, without logging when the configuration is created in BaseConfigurationService. Removing the logging and createIfNeeded methods would be API breaking changes so the goal is to have an implementation that doesn't break the existing API and doesn't log a warning when a configuration is created on-demand.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you, approved

logger.warn(
"Configuration for reconciler '{}' was not found. {}",
reconcilerKey,
reconcilersNameMessage);
}
}

@SuppressWarnings("unused")
Expand Down Expand Up @@ -318,6 +326,13 @@ private <P extends HasMetadata> ResolvedControllerConfiguration<P> controllerCon
informerConfig);
}

/**
* @deprecated This method was meant to allow subclasses to prevent automatic creation of the
* configuration when not found. This functionality is now removed, if you want to be able to
* prevent automated, on-demand creation of a reconciler's configuration, please use the
* {@link AbstractConfigurationService} implementation instead as base for your extension.
*/
@Deprecated(forRemoval = true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Somewhat strange to introduce a new usage of the method and deprecate it at the same time. How will this be handled when this method is removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The current implementation uses the method but the documentation explains what should happen instead. It's only deprecated to avoid API breakage in case implementations use it but this method should really be removed, because, as far as I'm aware, there is no implementation that doesn't currently return true and the creation scenario should only be confined to this particular implementation (or its subclasses) anyway.

protected boolean createIfNeeded() {
return true;
}
Expand Down