From 85f1e2dbbe4a5927b78f0f4a7a7a9af5e7b272b4 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 16 Feb 2022 09:29:59 +0100 Subject: [PATCH 1/6] feat: nicer config override --- .../io/javaoperatorsdk/operator/Operator.java | 18 +++++++++++++++++- .../ControllerConfigurationOverrider.java | 5 +++++ .../operator/sample/MySQLSchemaOperator.java | 11 +++-------- 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index 5f71ec2747..d2ec46bfde 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -4,7 +4,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.function.Consumer; +import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -118,7 +120,7 @@ public void register(Reconciler reconciler) * * @param reconciler part of the reconciler to register * @param configuration the configuration with which we want to register the reconciler - * @param the {@code CustomResource} type associated with the reconciler + * @param the {@code HasMetadata} type associated with the reconciler * @throws OperatorException if a problem occurred during the registration process */ public void register(Reconciler reconciler, @@ -147,6 +149,20 @@ public void register(Reconciler reconciler, watchedNS); } + /** + * Method to register operator and facilitate configuration override. + * + * @param reconciler part of the reconciler to register + * @param configOverrider consumer to use to change config values + * @param the {@code HasMetadata} type associated with the reconciler + */ + public void register(Reconciler reconciler, Consumer configOverrider) { + final var controllerConfiguration = configurationService.getConfigurationFor(reconciler); + var configToOverride = ControllerConfigurationOverrider.override(controllerConfiguration); + configOverrider.accept(configToOverride); + register(reconciler, controllerConfiguration); + } + static class ControllerManager implements LifecycleAware { private final Map controllers = new HashMap<>(); private boolean started = false; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 141b58ded0..3ea63b425b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -88,6 +88,11 @@ public ControllerConfigurationOverrider withReconciliationMaxInterval( return this; } + public void replaceDependentResourceConfig(Class> dependentResourceClass, + Object dependentResourceConfig) { + replaceDependentResourceConfig(new DependentResourceSpec(dependentResourceClass,dependentResourceConfig)); + } + /** * If a {@link DependentResourceSpec} already exists with the same dependentResourceClass it will * be replaced. Otherwise, an exception is thrown; diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java index 4fddf8cadc..ae635e3da1 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java @@ -37,15 +37,10 @@ public static void main(String[] args) throws IOException { .build()); MySQLSchemaReconciler schemaReconciler = new MySQLSchemaReconciler(); - ControllerConfigurationOverrider configOverrider = - ControllerConfigurationOverrider - .override(new AnnotationControllerConfiguration(schemaReconciler)); - configOverrider.replaceDependentResourceConfig( - new DependentResourceSpec(SchemaDependentResource.class, - new ResourcePollerConfig(500, MySQLDbConfig.loadFromEnvironmentVars()))); - - operator.register(schemaReconciler, configOverrider.build()); + operator.register(schemaReconciler, configOverrider -> + configOverrider.replaceDependentResourceConfig(SchemaDependentResource.class, + new ResourcePollerConfig(500, MySQLDbConfig.loadFromEnvironmentVars()))); operator.installShutdownHook(); operator.start(); From 4e56c22826fe24a61cec2137d6201342b1103226 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 16 Feb 2022 09:36:41 +0100 Subject: [PATCH 2/6] fix: format and bug --- .../java/io/javaoperatorsdk/operator/Operator.java | 5 +++-- .../api/config/ControllerConfigurationOverrider.java | 8 +++++--- .../java/io/javaoperatorsdk/operator/OperatorTest.java | 2 +- .../operator/sample/MySQLSchemaOperator.java | 10 ++++------ 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index d2ec46bfde..fee76fbdf0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -6,7 +6,6 @@ import java.util.Map; import java.util.function.Consumer; -import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -16,6 +15,7 @@ import io.fabric8.kubernetes.client.Version; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; import io.javaoperatorsdk.operator.api.config.ExecutorServiceManager; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.Controller; @@ -156,7 +156,8 @@ public void register(Reconciler reconciler, * @param configOverrider consumer to use to change config values * @param the {@code HasMetadata} type associated with the reconciler */ - public void register(Reconciler reconciler, Consumer configOverrider) { + public void register(Reconciler reconciler, + Consumer configOverrider) { final var controllerConfiguration = configurationService.getConfigurationFor(reconciler); var configToOverride = ControllerConfigurationOverrider.override(controllerConfiguration); configOverrider.accept(configToOverride); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 3ea63b425b..7fae470bcb 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -88,9 +88,11 @@ public ControllerConfigurationOverrider withReconciliationMaxInterval( return this; } - public void replaceDependentResourceConfig(Class> dependentResourceClass, - Object dependentResourceConfig) { - replaceDependentResourceConfig(new DependentResourceSpec(dependentResourceClass,dependentResourceConfig)); + public void replaceDependentResourceConfig( + Class> dependentResourceClass, + Object dependentResourceConfig) { + replaceDependentResourceConfig( + new DependentResourceSpec(dependentResourceClass, dependentResourceConfig)); } /** diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java index f532756f58..99bd7e64b9 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/OperatorTest.java @@ -47,7 +47,7 @@ public void shouldRegisterReconcilerToController() { @Test @DisplayName("should throw `OperationException` when Configuration is null") public void shouldThrowOperatorExceptionWhenConfigurationIsNull() { - Assertions.assertThrows(OperatorException.class, () -> operator.register(fooReconciler, null)); + Assertions.assertThrows(OperatorException.class, () -> operator.register(fooReconciler)); } private static class FooCustomResource extends CustomResource { diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java index ae635e3da1..5a39408ec8 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java @@ -15,9 +15,6 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.Operator; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; -import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; -import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; -import io.javaoperatorsdk.operator.config.runtime.AnnotationControllerConfiguration; import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics; import io.micrometer.core.instrument.logging.LoggingMeterRegistry; @@ -38,9 +35,10 @@ public static void main(String[] args) throws IOException { MySQLSchemaReconciler schemaReconciler = new MySQLSchemaReconciler(); - operator.register(schemaReconciler, configOverrider -> - configOverrider.replaceDependentResourceConfig(SchemaDependentResource.class, - new ResourcePollerConfig(500, MySQLDbConfig.loadFromEnvironmentVars()))); + operator.register(schemaReconciler, + configOverrider -> configOverrider.replaceDependentResourceConfig( + SchemaDependentResource.class, + new ResourcePollerConfig(500, MySQLDbConfig.loadFromEnvironmentVars()))); operator.installShutdownHook(); operator.start(); From 4d99957c0c137cf17b0205360c170bfdfd8a0c58 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 16 Feb 2022 09:37:36 +0100 Subject: [PATCH 3/6] fix: bug --- .../src/main/java/io/javaoperatorsdk/operator/Operator.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java index fee76fbdf0..2b15ab5cda 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/Operator.java @@ -161,7 +161,7 @@ public void register(Reconciler reconciler, final var controllerConfiguration = configurationService.getConfigurationFor(reconciler); var configToOverride = ControllerConfigurationOverrider.override(controllerConfiguration); configOverrider.accept(configToOverride); - register(reconciler, controllerConfiguration); + register(reconciler, configToOverride.build()); } static class ControllerManager implements LifecycleAware { From 227906d63209d6484f0cf91d5093a0e01742c9c9 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 16 Feb 2022 14:26:50 +0100 Subject: [PATCH 4/6] fix: default config values --- .../operator/sample/SchemaDependentResource.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaDependentResource.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaDependentResource.java index 62ce1356a2..338a420b65 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaDependentResource.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaDependentResource.java @@ -19,7 +19,7 @@ public class SchemaDependentResource extends AbstractDependentResource { private MySQLDbConfig dbConfig; - private int pollPeriod; + private int pollPeriod = 500; @Override public void configureWith(ResourcePollerConfig config) { @@ -29,6 +29,9 @@ public void configureWith(ResourcePollerConfig config) { @Override public Optional eventSource(EventSourceContext context) { + if (dbConfig == null) { + dbConfig = MySQLDbConfig.loadFromEnvironmentVars(); + } return Optional.of(new PerResourcePollingEventSource<>( new SchemaPollingResourceSupplier(dbConfig), context.getPrimaryCache(), pollPeriod, Schema.class)); From 0c92bb199056d575efb589066e2b6105a21ab895 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 16 Feb 2022 14:36:40 +0100 Subject: [PATCH 5/6] fix: method rename --- .../processing/dependent/DependentResourceManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java index 6f3a98b4dd..b3f93387a3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java @@ -50,7 +50,7 @@ public List prepareEventSources(EventSourceContext

context) { .map( drc -> { final var dependentResource = - from(drc, context.getClient()); + createAndConfigureFrom(drc, context.getClient()); dependentResource .eventSource(context) .ifPresent(es -> sources.add((EventSource) es)); @@ -81,8 +81,8 @@ private void initContextIfNeeded(P resource, Context context) { } } - private DependentResource from(DependentResourceSpec dependentResourceSpec, - KubernetesClient client) { + private DependentResource createAndConfigureFrom(DependentResourceSpec dependentResourceSpec, + KubernetesClient client) { try { DependentResource dependentResource = (DependentResource) dependentResourceSpec.getDependentResourceClass() From 52baca7a11a6da3c24dd9be0e3767dea552cacff Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 16 Feb 2022 16:36:14 +0100 Subject: [PATCH 6/6] fix: removed confusing API --- .../ControllerConfigurationOverrider.java | 17 ++++------------- .../dependent/DependentResourceManager.java | 2 +- .../operator/sample/MySQLSchemaOperatorE2E.java | 8 +++----- 3 files changed, 8 insertions(+), 19 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 7fae470bcb..7fb919abb1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -91,25 +91,16 @@ public ControllerConfigurationOverrider withReconciliationMaxInterval( public void replaceDependentResourceConfig( Class> dependentResourceClass, Object dependentResourceConfig) { - replaceDependentResourceConfig( - new DependentResourceSpec(dependentResourceClass, dependentResourceConfig)); - } - /** - * If a {@link DependentResourceSpec} already exists with the same dependentResourceClass it will - * be replaced. Otherwise, an exception is thrown; - * - * @param dependentResourceSpec to add or replace - */ - public void replaceDependentResourceConfig(DependentResourceSpec dependentResourceSpec) { var currentConfig = - findConfigForDependentResourceClass(dependentResourceSpec.getDependentResourceClass()); + findConfigForDependentResourceClass(dependentResourceClass); if (currentConfig.isEmpty()) { throw new IllegalStateException("Cannot find DependentResource config for class: " - + dependentResourceSpec.getDependentResourceClass()); + + dependentResourceClass); } dependentResourceSpecs.remove(currentConfig.get()); - dependentResourceSpecs.add(dependentResourceSpec); + dependentResourceSpecs + .add(new DependentResourceSpec(dependentResourceClass, dependentResourceConfig)); } public void addNewDependentResourceConfig(DependentResourceSpec dependentResourceSpec) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java index b3f93387a3..102ffef088 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java @@ -82,7 +82,7 @@ private void initContextIfNeeded(P resource, Context context) { } private DependentResource createAndConfigureFrom(DependentResourceSpec dependentResourceSpec, - KubernetesClient client) { + KubernetesClient client) { try { DependentResource dependentResource = (DependentResource) dependentResourceSpec.getDependentResourceClass() diff --git a/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java b/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java index f433671c08..2ebece8aa2 100644 --- a/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java +++ b/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java @@ -17,7 +17,6 @@ import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.LocalPortForward; -import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.junit.AbstractOperatorExtension; import io.javaoperatorsdk.operator.junit.E2EOperatorExtension; @@ -67,10 +66,9 @@ boolean isLocal() { new MySQLSchemaReconciler(), c -> { c.replaceDependentResourceConfig( - new DependentResourceSpec( - SchemaDependentResource.class, - new ResourcePollerConfig( - 700, new MySQLDbConfig("127.0.0.1", "3306", "root", "password")))); + SchemaDependentResource.class, + new ResourcePollerConfig( + 700, new MySQLDbConfig("127.0.0.1", "3306", "root", "password"))); }) .withInfrastructure(infrastructure) .build()