From 25643e904343d7921cfad88aa88f371597856190 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 14 Jan 2021 16:54:11 +0100 Subject: [PATCH 1/7] fix!: remove registerControllerForAllNamespaces to use controller config The intent here is that controllers should be registered according to their configuration. This will become even more relevant when externalized configuration is in place (see #237). This also means using ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER in the namespaces field of the Controller annotation where appropriate since that's what ControllerConfiguration uses to determine if a controller should watch all namespaces. This still needs to be unified across the code base (see #302). --- .../java/io/javaoperatorsdk/operator/Operator.java | 12 +----------- .../operator/sample/MySQLSchemaOperator.java | 2 +- .../operator/sample/SchemaController.java | 3 ++- .../io/javaoperatorsdk/operator/sample/Config.java | 2 +- .../operator/sample/TomcatController.java | 3 +++ .../operator/sample/TomcatOperator.java | 4 ++-- .../operator/sample/WebappController.java | 3 ++- .../operator/sample/WebServerController.java | 3 +++ .../operator/sample/WebServerOperator.java | 2 +- 9 files changed, 16 insertions(+), 18 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 b857ddb710..0f49acab5f 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 @@ -44,17 +44,7 @@ public void register(ResourceController controller registerController(controller, configuration.watchAllNamespaces(), retry, targetNamespaces); } } - - public void registerControllerForAllNamespaces( - ResourceController controller, Retry retry) throws OperatorException { - registerController(controller, true, retry); - } - - public void registerControllerForAllNamespaces( - ResourceController controller) throws OperatorException { - registerController(controller, true, null); - } - + public void registerController( ResourceController controller, Retry retry, String... targetNamespaces) throws OperatorException { diff --git a/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java b/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java index dd6aacfb64..d465965e21 100644 --- a/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java +++ b/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java @@ -24,7 +24,7 @@ public static void main(String[] args) throws IOException { Config config = new ConfigBuilder().withNamespace(null).build(); KubernetesClient client = new DefaultKubernetesClient(config); Operator operator = new Operator(client, DefaultConfigurationService.instance()); - operator.registerControllerForAllNamespaces(new SchemaController(client)); + operator.register(new SchemaController(client)); new FtBasic(new TkFork(new FkRegex("/health", "ALL GOOD!")), 8080).start(Exit.NEVER); } diff --git a/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaController.java b/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaController.java index 4367333ed4..c99480ad26 100644 --- a/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaController.java +++ b/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaController.java @@ -10,6 +10,7 @@ import io.javaoperatorsdk.operator.api.DeleteControl; import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.UpdateControl; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import java.sql.Connection; import java.sql.DriverManager; import java.sql.PreparedStatement; @@ -21,7 +22,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@Controller +@Controller(namespaces = ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER) public class SchemaController implements ResourceController { static final String USERNAME_FORMAT = "%s-user"; static final String SECRET_FORMAT = "%s-secret"; diff --git a/samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java b/samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java index 239fb64816..509b83bcd9 100644 --- a/samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java +++ b/samples/spring-boot-plain/src/main/java/io/javaoperatorsdk/operator/sample/Config.java @@ -26,7 +26,7 @@ public CustomServiceController customServiceController(KubernetesClient client) @Bean public Operator operator(KubernetesClient client, List controllers) { Operator operator = new Operator(client, DefaultConfigurationService.instance()); - controllers.forEach(c -> operator.registerControllerForAllNamespaces(c)); + controllers.forEach(operator::register); return operator; } } diff --git a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatController.java b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatController.java index c2326e0b53..0c20d8c774 100644 --- a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatController.java +++ b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatController.java @@ -10,9 +10,11 @@ import io.fabric8.kubernetes.client.dsl.ServiceResource; import io.fabric8.kubernetes.client.utils.Serialization; import io.javaoperatorsdk.operator.api.Context; +import io.javaoperatorsdk.operator.api.Controller; import io.javaoperatorsdk.operator.api.DeleteControl; import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.UpdateControl; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; import java.io.IOException; @@ -23,6 +25,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@Controller(namespaces = ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER) public class TomcatController implements ResourceController { private final Logger log = LoggerFactory.getLogger(getClass()); diff --git a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java index e2bfff6d54..7012a7d77c 100644 --- a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java +++ b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatOperator.java @@ -25,9 +25,9 @@ public static void main(String[] args) throws IOException { Operator operator = new Operator(client, DefaultConfigurationService.instance()); TomcatController tomcatController = new TomcatController(client); - operator.registerControllerForAllNamespaces(tomcatController); + operator.register(tomcatController); - operator.registerControllerForAllNamespaces(new WebappController(client)); + operator.register(new WebappController(client)); new FtBasic(new TkFork(new FkRegex("/health", "ALL GOOD.")), 8080).start(Exit.NEVER); } diff --git a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/WebappController.java b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/WebappController.java index 80c6a647ce..7d99a8f915 100644 --- a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/WebappController.java +++ b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/WebappController.java @@ -4,6 +4,7 @@ import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.*; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import java.io.ByteArrayOutputStream; import java.util.List; import java.util.Objects; @@ -12,7 +13,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@Controller +@Controller(namespaces = ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER) public class WebappController implements ResourceController { private KubernetesClient kubernetesClient; diff --git a/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerController.java b/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerController.java index be5bbe4495..a53419be25 100644 --- a/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerController.java +++ b/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerController.java @@ -12,9 +12,11 @@ import io.fabric8.kubernetes.client.dsl.ServiceResource; import io.fabric8.kubernetes.client.utils.Serialization; import io.javaoperatorsdk.operator.api.Context; +import io.javaoperatorsdk.operator.api.Controller; import io.javaoperatorsdk.operator.api.DeleteControl; import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.UpdateControl; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import java.io.IOException; import java.io.InputStream; import java.util.HashMap; @@ -23,6 +25,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +@Controller(namespaces = ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER) public class WebServerController implements ResourceController { private final Logger log = LoggerFactory.getLogger(getClass()); diff --git a/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerOperator.java b/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerOperator.java index db06f88e6a..dab33397b8 100644 --- a/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerOperator.java +++ b/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerOperator.java @@ -24,7 +24,7 @@ public static void main(String[] args) throws IOException { Config config = new ConfigBuilder().withNamespace(null).build(); KubernetesClient client = new DefaultKubernetesClient(config); Operator operator = new Operator(client, DefaultConfigurationService.instance()); - operator.registerControllerForAllNamespaces(new WebServerController(client)); + operator.register(new WebServerController(client)); new FtBasic(new TkFork(new FkRegex("/health", "ALL GOOD!")), 8080).start(Exit.NEVER); } From 25256c7b8a9ef7b60a39a912f54ad2a8ce87d8cb Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 14 Jan 2021 17:07:32 +0100 Subject: [PATCH 2/7] fix!: remove registerController to use controller config instead --- .../java/io/javaoperatorsdk/operator/Operator.java | 13 ++++++------- .../operator/sample/PureJavaApplicationRunner.java | 2 +- 2 files changed, 7 insertions(+), 8 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 0f49acab5f..a1b1595ef6 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 @@ -44,18 +44,17 @@ public void register(ResourceController controller registerController(controller, configuration.watchAllNamespaces(), retry, targetNamespaces); } } - - public void registerController( + + // currently only used by IntegrationTestSupport but this should also disappear as by default the + // controllers should register into the current namespace the client is connected to which should + // work as expected for tests, in particular if we want to start testing using random namespace + // names + void registerController( ResourceController controller, Retry retry, String... targetNamespaces) throws OperatorException { registerController(controller, false, retry, targetNamespaces); } - public void registerController( - ResourceController controller, String... targetNamespaces) throws OperatorException { - registerController(controller, false, null, targetNamespaces); - } - @SuppressWarnings("rawtypes") private void registerController( ResourceController controller, diff --git a/samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java b/samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java index e2ab96617b..24d2957ef7 100644 --- a/samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java +++ b/samples/pure-java/src/main/java/io/javaoperatorsdk/operator/sample/PureJavaApplicationRunner.java @@ -10,6 +10,6 @@ public class PureJavaApplicationRunner { public static void main(String[] args) { KubernetesClient client = new DefaultKubernetesClient(); Operator operator = new Operator(client, DefaultConfigurationService.instance()); - operator.registerController(new CustomServiceController(client)); + operator.register(new CustomServiceController(client)); } } From 18bf0c81838f5ed632e11a537eb1b106d3ed058c Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 19 Jan 2021 21:52:09 +0100 Subject: [PATCH 3/7] refactor: introduce AbstractControllerConfiguration --- .../AbstractControllerConfiguration.java | 80 +++++++++++++++++++ .../QuarkusControllerConfiguration.java | 78 ++++-------------- 2 files changed, 96 insertions(+), 62 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java new file mode 100644 index 0000000000..014a57f94c --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java @@ -0,0 +1,80 @@ +package io.javaoperatorsdk.operator.api.config; + +import io.fabric8.kubernetes.client.CustomResource; +import java.util.Collections; +import java.util.Set; + +public abstract class AbstractControllerConfiguration + implements ControllerConfiguration { + + private final String associatedControllerClassName; + private final String name; + private final String crdName; + private final String finalizer; + private final boolean generationAware; + private final Set namespaces; + private final boolean watchAllNamespaces; + private final RetryConfiguration retryConfiguration; + + public AbstractControllerConfiguration( + String associatedControllerClassName, + String name, + String crdName, + String finalizer, + boolean generationAware, + Set namespaces, + RetryConfiguration retryConfiguration) { + this.associatedControllerClassName = associatedControllerClassName; + this.name = name; + this.crdName = crdName; + this.finalizer = finalizer; + this.generationAware = generationAware; + this.namespaces = + namespaces != null ? Collections.unmodifiableSet(namespaces) : Collections.emptySet(); + this.watchAllNamespaces = this.namespaces.contains(WATCH_ALL_NAMESPACES_MARKER); + this.retryConfiguration = + retryConfiguration == null + ? ControllerConfiguration.super.getRetryConfiguration() + : retryConfiguration; + } + + @Override + public String getName() { + return name; + } + + @Override + public String getCRDName() { + return crdName; + } + + @Override + public String getFinalizer() { + return finalizer; + } + + @Override + public boolean isGenerationAware() { + return generationAware; + } + + @Override + public String getAssociatedControllerClassName() { + return associatedControllerClassName; + } + + @Override + public Set getNamespaces() { + return namespaces; + } + + @Override + public boolean watchAllNamespaces() { + return watchAllNamespaces; + } + + @Override + public RetryConfiguration getRetryConfiguration() { + return retryConfiguration; + } +} diff --git a/operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/QuarkusControllerConfiguration.java b/operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/QuarkusControllerConfiguration.java index 0c0a90b6ca..b5dc8fb881 100644 --- a/operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/QuarkusControllerConfiguration.java +++ b/operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/QuarkusControllerConfiguration.java @@ -1,23 +1,17 @@ package io.javaoperatorsdk.quarkus.extension; import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.AbstractControllerConfiguration; import io.javaoperatorsdk.operator.api.config.RetryConfiguration; import io.quarkus.runtime.annotations.RecordableConstructor; import java.util.Collections; import java.util.Set; public class QuarkusControllerConfiguration - implements ControllerConfiguration { - private final String associatedControllerClassName; - private final String name; - private final String crdName; - private final String finalizer; - private final boolean generationAware; - private final Set namespaces; + extends AbstractControllerConfiguration { + private final String crClass; - private final boolean watchAllNamespaces; - private final RetryConfiguration retryConfiguration; + private Class clazz; @RecordableConstructor public QuarkusControllerConfiguration( @@ -29,18 +23,15 @@ public QuarkusControllerConfiguration( Set namespaces, String crClass, RetryConfiguration retryConfiguration) { - this.associatedControllerClassName = associatedControllerClassName; - this.name = name; - this.crdName = crdName; - this.finalizer = finalizer; - this.generationAware = generationAware; - this.namespaces = namespaces; + super( + associatedControllerClassName, + name, + crdName, + finalizer, + generationAware, + namespaces, + retryConfiguration); this.crClass = crClass; - this.watchAllNamespaces = this.namespaces.contains(WATCH_ALL_NAMESPACES_MARKER); - this.retryConfiguration = - retryConfiguration == null - ? ControllerConfiguration.super.getRetryConfiguration() - : retryConfiguration; } public static Set asSet(String[] namespaces) { @@ -59,29 +50,12 @@ public String getCrClass() { return crClass; } - @Override - public String getName() { - return name; - } - - @Override - public String getCRDName() { - return crdName; - } - - @Override - public String getFinalizer() { - return finalizer; - } - - @Override - public boolean isGenerationAware() { - return generationAware; - } - @Override public Class getCustomResourceClass() { - return (Class) loadClass(crClass); + if (clazz == null) { + clazz = (Class) loadClass(crClass); + } + return clazz; } private Class loadClass(String className) { @@ -91,24 +65,4 @@ private Class loadClass(String className) { throw new IllegalArgumentException("Couldn't find class " + className); } } - - @Override - public String getAssociatedControllerClassName() { - return associatedControllerClassName; - } - - @Override - public Set getNamespaces() { - return namespaces; - } - - @Override - public boolean watchAllNamespaces() { - return watchAllNamespaces; - } - - @Override - public RetryConfiguration getRetryConfiguration() { - return retryConfiguration; - } } From 4971655421131671f236cb6343c6c98d3c96a84c Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 19 Jan 2021 21:51:05 +0100 Subject: [PATCH 4/7] refactor: Retry now extends RetryConfiguration --- .../operator/api/config/RetryConfiguration.java | 1 - .../operator/processing/retry/GenericRetry.java | 5 ----- .../io/javaoperatorsdk/operator/processing/retry/Retry.java | 4 +++- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java index 8029f01b56..dee54c3e8d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/RetryConfiguration.java @@ -6,7 +6,6 @@ public interface RetryConfiguration { int DEFAULT_MAX_ATTEMPTS = 5; long DEFAULT_INITIAL_INTERVAL = 2000L; - double DEFAULT_MULTIPLIER = 1.5D; default int getMaxAttempts() { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java index 7b188239b1..83fa9bb769 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/GenericRetry.java @@ -3,11 +3,6 @@ import io.javaoperatorsdk.operator.api.config.RetryConfiguration; public class GenericRetry implements Retry { - - public static final int DEFAULT_MAX_ATTEMPTS = 5; - public static final long DEFAULT_INITIAL_INTERVAL = 2000L; - public static final double DEFAULT_MULTIPLIER = 1.5D; - private int maxAttempts = DEFAULT_MAX_ATTEMPTS; private long initialInterval = DEFAULT_INITIAL_INTERVAL; private double intervalMultiplier = DEFAULT_MULTIPLIER; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java index dd7a34722a..b7911d1a74 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/retry/Retry.java @@ -1,6 +1,8 @@ package io.javaoperatorsdk.operator.processing.retry; -public interface Retry { +import io.javaoperatorsdk.operator.api.config.RetryConfiguration; + +public interface Retry extends RetryConfiguration { RetryExecution initExecution(); } From bb6beeb1b51b5db561837af7aec6a5c8955d23ca Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 19 Jan 2021 21:56:32 +0100 Subject: [PATCH 5/7] feat: add ControllerConfigurationOverrider to fluently override config --- .../ControllerConfigurationOverrider.java | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java 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 new file mode 100644 index 0000000000..be8c82af02 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -0,0 +1,80 @@ +package io.javaoperatorsdk.operator.api.config; + +import io.fabric8.kubernetes.client.CustomResource; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + +public class ControllerConfigurationOverrider { + + private String finalizer; + private boolean generationAware; + private Set namespaces; + private RetryConfiguration retry; + private final ControllerConfiguration original; + + private ControllerConfigurationOverrider(ControllerConfiguration original) { + finalizer = original.getFinalizer(); + generationAware = original.isGenerationAware(); + namespaces = new HashSet<>(original.getNamespaces()); + retry = original.getRetryConfiguration(); + this.original = original; + } + + public ControllerConfigurationOverrider withFinalizer(String finalizer) { + this.finalizer = finalizer; + return this; + } + + public ControllerConfigurationOverrider withGenerationAware(boolean generationAware) { + this.generationAware = generationAware; + return this; + } + + public ControllerConfigurationOverrider withCurrentNamespace() { + namespaces.clear(); + return this; + } + + public ControllerConfigurationOverrider addingNamespaces(String... namespaces) { + this.namespaces.addAll(List.of(namespaces)); + return this; + } + + public ControllerConfigurationOverrider removingNamespaces(String... namespaces) { + this.namespaces.removeAll(List.of(namespaces)); + return this; + } + + public ControllerConfigurationOverrider settingNamespace(String namespace) { + this.namespaces.clear(); + this.namespaces.add(namespace); + return this; + } + + public ControllerConfigurationOverrider withRetry(RetryConfiguration retry) { + this.retry = retry; + return this; + } + + public ControllerConfiguration build() { + return new AbstractControllerConfiguration( + original.getAssociatedControllerClassName(), + original.getName(), + original.getCRDName(), + finalizer, + generationAware, + namespaces, + retry) { + @Override + public Class getCustomResourceClass() { + return original.getCustomResourceClass(); + } + }; + } + + public static ControllerConfigurationOverrider override( + ControllerConfiguration original) { + return new ControllerConfigurationOverrider<>(original); + } +} From b2d2dd92baaf059e9481f7cc5cd5465c3c288de3 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 19 Jan 2021 21:57:49 +0100 Subject: [PATCH 6/7] feat!: allow config override when registering a controller --- .../io/javaoperatorsdk/operator/Operator.java | 24 +++++++++---------- .../operator/IntegrationTestSupport.java | 23 +++++++++--------- 2 files changed, 24 insertions(+), 23 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 a1b1595ef6..b1e8f25af5 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 @@ -5,6 +5,7 @@ import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.CustomResourceCache; import io.javaoperatorsdk.operator.processing.DefaultEventHandler; import io.javaoperatorsdk.operator.processing.EventDispatcher; @@ -30,8 +31,14 @@ public Operator(KubernetesClient k8sClient, ConfigurationService configurationSe public void register(ResourceController controller) throws OperatorException { - final var configuration = configurationService.getConfigurationFor(controller); - if (configuration == null) { + register(controller, null); + } + + public void register( + ResourceController controller, ControllerConfiguration configuration) + throws OperatorException { + final var existing = configurationService.getConfigurationFor(controller); + if (existing == null) { log.warn( "Skipping registration of {} controller named {} because its configuration cannot be found.\n" + "Known controllers are: {}", @@ -39,22 +46,15 @@ public void register(ResourceController controller ControllerUtils.getNameFor(controller), configurationService.getKnownControllerNames()); } else { + if (configuration == null) { + configuration = existing; + } final var retry = GenericRetry.fromConfiguration(configuration.getRetryConfiguration()); final var targetNamespaces = configuration.getNamespaces().toArray(new String[] {}); registerController(controller, configuration.watchAllNamespaces(), retry, targetNamespaces); } } - // currently only used by IntegrationTestSupport but this should also disappear as by default the - // controllers should register into the current namespace the client is connected to which should - // work as expected for tests, in particular if we want to start testing using random namespace - // names - void registerController( - ResourceController controller, Retry retry, String... targetNamespaces) - throws OperatorException { - registerController(controller, false, retry, targetNamespaces); - } - @SuppressWarnings("rawtypes") private void registerController( ResourceController controller, diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/IntegrationTestSupport.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/IntegrationTestSupport.java index 128a332e91..9805380e22 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/IntegrationTestSupport.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/IntegrationTestSupport.java @@ -12,9 +12,9 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.fabric8.kubernetes.client.dsl.base.CustomResourceDefinitionContext; import io.fabric8.kubernetes.client.utils.Serialization; import io.javaoperatorsdk.operator.api.ResourceController; +import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; import io.javaoperatorsdk.operator.config.runtime.DefaultConfigurationService; import io.javaoperatorsdk.operator.processing.retry.Retry; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; @@ -46,8 +46,7 @@ public void initialize( KubernetesClient k8sClient, ResourceController controller, String crdPath, Retry retry) { log.info("Initializing integration test in namespace {}", TEST_NAMESPACE); this.k8sClient = k8sClient; - CustomResourceDefinition crd = loadCRDAndApplyToCluster(crdPath); - CustomResourceDefinitionContext crdContext = CustomResourceDefinitionContext.fromCrd(crd); + loadCRDAndApplyToCluster(crdPath); this.controller = controller; final var configurationService = DefaultConfigurationService.instance(); @@ -56,16 +55,18 @@ public void initialize( final var customResourceClass = config.getCustomResourceClass(); this.crOperations = k8sClient.customResources(customResourceClass); - if (k8sClient.namespaces().withName(TEST_NAMESPACE).get() == null) { - k8sClient - .namespaces() - .create( - new NamespaceBuilder() - .withMetadata(new ObjectMetaBuilder().withName(TEST_NAMESPACE).build()) - .build()); + final var namespaces = k8sClient.namespaces(); + if (namespaces.withName(TEST_NAMESPACE).get() == null) { + namespaces.create( + new NamespaceBuilder().withNewMetadata().withName(TEST_NAMESPACE).endMetadata().build()); } operator = new Operator(k8sClient, configurationService); - operator.registerController(controller, retry, TEST_NAMESPACE); + final var overriddenConfig = + ControllerConfigurationOverrider.override(config).settingNamespace(TEST_NAMESPACE); + if (retry != null) { + overriddenConfig.withRetry(retry); + } + operator.register(controller, overriddenConfig.build()); log.info("Operator is running with {}", controller.getClass().getCanonicalName()); } From 456ea3c5877a79f01a47c00928af08d88ce169bd Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 20 Jan 2021 12:07:00 +0100 Subject: [PATCH 7/7] fix!: watch all namespaces by default if no namespaces are specified --- .../java/io/javaoperatorsdk/operator/api/Controller.java | 6 +----- .../api/config/AbstractControllerConfiguration.java | 2 +- .../operator/api/config/ControllerConfiguration.java | 2 +- .../quarkus/extension/ExternalControllerConfiguration.java | 6 ++---- .../javaoperatorsdk/operator/sample/SchemaController.java | 3 --- .../javaoperatorsdk/operator/sample/TomcatController.java | 3 --- .../javaoperatorsdk/operator/sample/WebappController.java | 7 ++++--- .../operator/sample/WebServerController.java | 3 --- 8 files changed, 9 insertions(+), 23 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java index d3ca3dd05b..fa882154b1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/Controller.java @@ -28,11 +28,7 @@ /** * Specified which namespaces this Controller monitors for custom resources events. If no - * namespace is specified then the controller will monitor the namespace it is deployed in (or the - * namespace to which the Kubernetes client is connected to). To specify that the controller needs - * to monitor all namespaces, add {@link - * io.javaoperatorsdk.operator.api.config.ControllerConfiguration#WATCH_ALL_NAMESPACES_MARKER} to - * this field. + * namespace is specified then the controller will monitor all namespaces by default. * * @return the list of namespaces this controller monitors */ diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java index 014a57f94c..85b71d18c3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractControllerConfiguration.java @@ -31,7 +31,7 @@ public AbstractControllerConfiguration( this.generationAware = generationAware; this.namespaces = namespaces != null ? Collections.unmodifiableSet(namespaces) : Collections.emptySet(); - this.watchAllNamespaces = this.namespaces.contains(WATCH_ALL_NAMESPACES_MARKER); + this.watchAllNamespaces = this.namespaces.isEmpty(); this.retryConfiguration = retryConfiguration == null ? ControllerConfiguration.super.getRetryConfiguration() diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index f6bde0df82..e04faf7450 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -25,7 +25,7 @@ default Set getNamespaces() { } default boolean watchAllNamespaces() { - return getNamespaces().contains(WATCH_ALL_NAMESPACES_MARKER); + return getNamespaces().isEmpty(); } default RetryConfiguration getRetryConfiguration() { diff --git a/operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/ExternalControllerConfiguration.java b/operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/ExternalControllerConfiguration.java index b1de70f962..561499f0d3 100644 --- a/operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/ExternalControllerConfiguration.java +++ b/operator-framework-quarkus-extension/runtime/src/main/java/io/javaoperatorsdk/quarkus/extension/ExternalControllerConfiguration.java @@ -1,6 +1,5 @@ package io.javaoperatorsdk.quarkus.extension; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.quarkus.runtime.annotations.ConfigGroup; import io.quarkus.runtime.annotations.ConfigItem; import java.util.List; @@ -10,9 +9,8 @@ public class ExternalControllerConfiguration { /** - * An optional list of comma-separated namespace names the controller should watch. If the list - * contains {@link ControllerConfiguration#WATCH_ALL_NAMESPACES_MARKER} then the controller will - * watch all namespaces. + * An optional list of comma-separated namespace names the controller should watch. If this + * property is left empty then the controller will watch all namespaces. */ @ConfigItem public Optional> namespaces; diff --git a/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaController.java b/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaController.java index c99480ad26..26688ff520 100644 --- a/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaController.java +++ b/samples/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/SchemaController.java @@ -6,11 +6,9 @@ import io.fabric8.kubernetes.api.model.SecretBuilder; import io.fabric8.kubernetes.client.KubernetesClient; import io.javaoperatorsdk.operator.api.Context; -import io.javaoperatorsdk.operator.api.Controller; import io.javaoperatorsdk.operator.api.DeleteControl; import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.UpdateControl; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import java.sql.Connection; import java.sql.DriverManager; import java.sql.PreparedStatement; @@ -22,7 +20,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@Controller(namespaces = ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER) public class SchemaController implements ResourceController { static final String USERNAME_FORMAT = "%s-user"; static final String SECRET_FORMAT = "%s-secret"; diff --git a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatController.java b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatController.java index 0c20d8c774..c2326e0b53 100644 --- a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatController.java +++ b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/TomcatController.java @@ -10,11 +10,9 @@ import io.fabric8.kubernetes.client.dsl.ServiceResource; import io.fabric8.kubernetes.client.utils.Serialization; import io.javaoperatorsdk.operator.api.Context; -import io.javaoperatorsdk.operator.api.Controller; import io.javaoperatorsdk.operator.api.DeleteControl; import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.UpdateControl; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; import io.javaoperatorsdk.operator.processing.event.internal.CustomResourceEvent; import java.io.IOException; @@ -25,7 +23,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@Controller(namespaces = ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER) public class TomcatController implements ResourceController { private final Logger log = LoggerFactory.getLogger(getClass()); diff --git a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/WebappController.java b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/WebappController.java index 7d99a8f915..de32c66fca 100644 --- a/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/WebappController.java +++ b/samples/tomcat/src/main/java/io/javaoperatorsdk/operator/sample/WebappController.java @@ -3,8 +3,10 @@ import io.fabric8.kubernetes.api.model.Pod; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.api.*; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.Context; +import io.javaoperatorsdk.operator.api.DeleteControl; +import io.javaoperatorsdk.operator.api.ResourceController; +import io.javaoperatorsdk.operator.api.UpdateControl; import java.io.ByteArrayOutputStream; import java.util.List; import java.util.Objects; @@ -13,7 +15,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@Controller(namespaces = ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER) public class WebappController implements ResourceController { private KubernetesClient kubernetesClient; diff --git a/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerController.java b/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerController.java index a53419be25..be5bbe4495 100644 --- a/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerController.java +++ b/samples/webserver/src/main/java/io/javaoperatorsdk/operator/sample/WebServerController.java @@ -12,11 +12,9 @@ import io.fabric8.kubernetes.client.dsl.ServiceResource; import io.fabric8.kubernetes.client.utils.Serialization; import io.javaoperatorsdk.operator.api.Context; -import io.javaoperatorsdk.operator.api.Controller; import io.javaoperatorsdk.operator.api.DeleteControl; import io.javaoperatorsdk.operator.api.ResourceController; import io.javaoperatorsdk.operator.api.UpdateControl; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import java.io.IOException; import java.io.InputStream; import java.util.HashMap; @@ -25,7 +23,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; -@Controller(namespaces = ControllerConfiguration.WATCH_ALL_NAMESPACES_MARKER) public class WebServerController implements ResourceController { private final Logger log = LoggerFactory.getLogger(getClass());