From b8b9e2fe2e32c7cfeabc2be50318605e4024ffd1 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 29 Apr 2022 13:17:16 +0200 Subject: [PATCH 01/24] dynamic namespace config --- .../io/javaoperatorsdk/operator/Operator.java | 12 +++-- .../operator/api/RegisteredController.java | 16 ++++++ .../api/config/ResourceConfiguration.java | 1 - .../informer/InformerConfiguration.java | 2 +- .../operator/processing/Controller.java | 36 +++++++------ .../ControllerResourceEventSource.java | 1 - .../source/informer/InformerManager.java | 51 ++++++++++++++++--- .../informer/ManagedInformerEventSource.java | 5 ++ .../config/ControllerConfigurationTest.java | 7 +-- 9 files changed, 93 insertions(+), 38 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java 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 9765ede762..7182283a8d 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 @@ -13,6 +13,7 @@ import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.Version; +import io.javaoperatorsdk.operator.api.RegisteredController; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; @@ -129,11 +130,11 @@ public void stop() throws OperatorException { * @param the {@code CustomResource} type associated with the reconciler * @throws OperatorException if a problem occurred during the registration process */ - public void register(Reconciler reconciler) + public RegisteredController register(Reconciler reconciler) throws OperatorException { final var controllerConfiguration = ConfigurationServiceProvider.instance().getConfigurationFor(reconciler); - register(reconciler, controllerConfiguration); + return register(reconciler, controllerConfiguration); } /** @@ -148,7 +149,7 @@ public void register(Reconciler 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, + public RegisteredController register(Reconciler reconciler, ControllerConfiguration configuration) throws OperatorException { @@ -173,6 +174,7 @@ public void register(Reconciler reconciler, configuration.getName(), configuration.getResourceClass(), watchedNS); + return new RegisteredController(controller); } /** @@ -182,13 +184,13 @@ 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, + public RegisteredController register(Reconciler reconciler, Consumer> configOverrider) { final var controllerConfiguration = ConfigurationServiceProvider.instance().getConfigurationFor(reconciler); var configToOverride = ControllerConfigurationOverrider.override(controllerConfiguration); configOverrider.accept(configToOverride); - register(reconciler, configToOverride.build()); + return register(reconciler, configToOverride.build()); } static class ControllerManager implements LifecycleAware { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java new file mode 100644 index 0000000000..fbdc1ad10d --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java @@ -0,0 +1,16 @@ +package io.javaoperatorsdk.operator.api; + +import io.javaoperatorsdk.operator.processing.Controller; + +public class RegisteredController { + + private Controller controller; + + public RegisteredController(Controller controller) { + this.controller = controller; + } + + public void changeNamespaces(String... namespaces) { + controller.changeNamespaces(namespaces); + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java index fe85476735..58a41070d2 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java @@ -64,7 +64,6 @@ static void failIfNotValid(Set namespaces) { return; } } - throw new IllegalArgumentException( "Must specify namespaces. To watch all namespaces, use only '" + Constants.WATCH_ALL_NAMESPACES diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index a2105cac11..82a0037a6b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -45,6 +45,7 @@ class InformerConfigurationBuilder { private Set namespaces; private String labelSelector; private final Class resourceClass; + private boolean followNamespaceChanges = false; private InformerConfigurationBuilder(Class resourceClass) { this.resourceClass = resourceClass; @@ -66,7 +67,6 @@ public InformerConfigurationBuilder withNamespaces(Set namespaces) { return this; } - public InformerConfigurationBuilder withLabelSelector(String labelSelector) { this.labelSelector = labelSelector; return this; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 63b3c85a9c..1eff64e9ef 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -307,29 +307,14 @@ public void start() throws OperatorException { // fail early if we're missing the current namespace information failOnMissingCurrentNS(); - try { // check that the custom resource is known by the cluster if configured that way - final CustomResourceDefinition crd; // todo: check proper CRD spec version based on config - if (ConfigurationServiceProvider.instance().checkCRDAndValidateLocalModel() - && CustomResource.class.isAssignableFrom(resClass)) { - crd = kubernetesClient.apiextensions().v1().customResourceDefinitions().withName(crdName) - .get(); - if (crd == null) { - throwMissingCRDException(crdName, specVersion, controllerName); - } - - // Apply validations that are not handled by fabric8 - CustomResourceUtils.assertCustomResource(resClass, crd); - } - + validateCRDWithLocalModelIfRequired(resClass, controllerName, crdName, specVersion); final var context = new EventSourceContext<>( eventSourceManager.getControllerResourceEventSource(), configuration, kubernetesClient); initAndRegisterEventSources(context); - eventSourceManager.start(); - log.info("'{}' controller started, pending event sources initialization", controllerName); } catch (MissingCRDException e) { stop(); @@ -337,6 +322,25 @@ public void start() throws OperatorException { } } + private void validateCRDWithLocalModelIfRequired(Class

resClass, String controllerName, + String crdName, String specVersion) { + final CustomResourceDefinition crd; + if (ConfigurationServiceProvider.instance().checkCRDAndValidateLocalModel() + && CustomResource.class.isAssignableFrom(resClass)) { + crd = kubernetesClient.apiextensions().v1().customResourceDefinitions().withName(crdName) + .get(); + if (crd == null) { + throwMissingCRDException(crdName, specVersion, controllerName); + } + // Apply validations that are not handled by fabric8 + CustomResourceUtils.assertCustomResource(resClass, crd); + } + } + + public void changeNamespaces(String... namespaces) { + + } + private void throwMissingCRDException(String crdName, String specVersion, String controllerName) { throw new MissingCRDException( crdName, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java index 0c0f0c6b8e..233cecb644 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSource.java @@ -24,7 +24,6 @@ public class ControllerResourceEventSource extends ManagedInformerEventSource> implements ResourceEventHandler { - public static final String ANY_NAMESPACE_MAP_KEY = "anyNamespace"; private static final Logger log = LoggerFactory.getLogger(ControllerResourceEventSource.class); private final Controller controller; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 514caa2a2e..8b5c442886 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -29,11 +29,15 @@ public class InformerManager> implements LifecycleAware, IndexerResourceCache, UpdatableCache { - private static final String ANY_NAMESPACE_MAP_KEY = "anyNamespace"; + private static final String ALL_NAMESPACE_MAP_KEY = "allNamespace"; private static final Logger log = LoggerFactory.getLogger(InformerManager.class); private final Map> sources = new ConcurrentHashMap<>(); private Cloner cloner; + private C configuration; + private MixedOperation, Resource> client; + private ResourceEventHandler eventHandler; + private Map>> indexers; @Override public void start() throws OperatorException { @@ -43,6 +47,9 @@ public void start() throws OperatorException { void initSources(MixedOperation, Resource> client, C configuration, ResourceEventHandler eventHandler) { cloner = ConfigurationServiceProvider.instance().getResourceCloner(); + this.configuration = configuration; + this.client = client; + this.eventHandler = eventHandler; final var targetNamespaces = configuration.getEffectiveNamespaces(); final var labelSelector = configuration.getLabelSelector(); @@ -51,7 +58,7 @@ void initSources(MixedOperation, Resource> clien final var filteredBySelectorClient = client.inAnyNamespace().withLabelSelector(labelSelector); final var source = - createEventSource(filteredBySelectorClient, eventHandler, ANY_NAMESPACE_MAP_KEY); + createEventSource(filteredBySelectorClient, eventHandler, ALL_NAMESPACE_MAP_KEY); log.debug("Registered {} -> {} for any namespace", this, source); } else { targetNamespaces.forEach( @@ -65,6 +72,33 @@ void initSources(MixedOperation, Resource> clien } } + public void changeNamespaces(Set namespaces) { + if (ResourceConfiguration.allNamespacesWatched(namespaces)) { + throw new OperatorException("This feature is only supported for "); + } + + var sourcesToRemove = sources.keySet().stream() + .filter(k -> !namespaces.contains(k)).collect(Collectors.toSet()); + log.debug("Stopping to watch namespaces: {} for {}", sourcesToRemove, this); + sourcesToRemove.forEach(k -> { + sources.remove(k).stop(); + }); + + namespaces.forEach(ns -> { + if (!sources.containsKey(ns)) { + final var source = + createEventSource( + client.inNamespace(ns).withLabelSelector(configuration.getLabelSelector()), + eventHandler, ns); + source.addIndexers(this.indexers); + source.start(); + log.debug("Registered New {} -> {} for namespace: {}", this, source, + ns); + } + }); + } + + private InformerWrapper createEventSource( FilterWatchListDeletable> filteredBySelectorClient, @@ -98,7 +132,7 @@ public Stream list(Predicate predicate) { @Override public Stream list(String namespace, Predicate predicate) { if (isWatchingAllNamespaces()) { - return getSource(ANY_NAMESPACE_MAP_KEY) + return getSource(ALL_NAMESPACE_MAP_KEY) .map(source -> source.list(namespace, predicate)) .orElse(Stream.empty()); } else { @@ -110,7 +144,7 @@ public Stream list(String namespace, Predicate predicate) { @Override public Optional get(ResourceID resourceID) { - return getSource(resourceID.getNamespace().orElse(ANY_NAMESPACE_MAP_KEY)) + return getSource(resourceID.getNamespace().orElse(ALL_NAMESPACE_MAP_KEY)) .flatMap(source -> source.get(resourceID)) .map(cloner::clone); } @@ -121,24 +155,24 @@ public Stream keys() { } private boolean isWatchingAllNamespaces() { - return sources.containsKey(ANY_NAMESPACE_MAP_KEY); + return sources.containsKey(ALL_NAMESPACE_MAP_KEY); } private Optional> getSource(String namespace) { - namespace = isWatchingAllNamespaces() || namespace == null ? ANY_NAMESPACE_MAP_KEY : namespace; + namespace = isWatchingAllNamespaces() || namespace == null ? ALL_NAMESPACE_MAP_KEY : namespace; return Optional.ofNullable(sources.get(namespace)); } @Override public T remove(ResourceID key) { - return getSource(key.getNamespace().orElse(ANY_NAMESPACE_MAP_KEY)) + return getSource(key.getNamespace().orElse(ALL_NAMESPACE_MAP_KEY)) .map(c -> c.remove(key)) .orElse(null); } @Override public void put(ResourceID key, T resource) { - getSource(key.getNamespace().orElse(ANY_NAMESPACE_MAP_KEY)) + getSource(key.getNamespace().orElse(ALL_NAMESPACE_MAP_KEY)) .ifPresentOrElse(c -> c.put(key, resource), () -> log.warn( "Cannot put resource in the cache. No related cache found: {}. Resource: {}", @@ -147,6 +181,7 @@ public void put(ResourceID key, T resource) { @Override public void addIndexers(Map>> indexers) { + this.indexers.putAll(indexers); sources.values().forEach(s -> s.addIndexers(indexers)); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 95aba48ad7..0e32fc26d5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -3,6 +3,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.function.Function; import java.util.function.Predicate; import java.util.stream.Stream; @@ -60,6 +61,10 @@ protected InformerManager manager() { return (InformerManager) cache; } + public void changeNamespaces(Set namespaces) { + manager().changeNamespaces(namespaces); + } + @Override public void start() { manager().start(); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java index 05aa637f1f..a2eeacf82b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java @@ -10,12 +10,7 @@ class ControllerConfigurationTest { @Test void getCustomResourceClass() { - final ControllerConfiguration conf = new ControllerConfiguration<>() { - @Override - public String getAssociatedReconcilerClassName() { - return null; - } - }; + final ControllerConfiguration conf = () -> null; assertEquals(TestCustomResource.class, conf.getResourceClass()); } } From 97332bf1dfd552c34ecca64a2c514cf4047bc6f6 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 29 Apr 2022 14:31:20 +0200 Subject: [PATCH 02/24] progress --- .../informer/InformerConfiguration.java | 27 +++++++++++-------- .../kubernetes/KubernetesDependent.java | 6 ++--- .../source/informer/InformerManager.java | 2 +- .../config/ControllerConfigurationTest.java | 7 ++++- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index 82a0037a6b..dbe0fd0ef0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -7,6 +7,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.DefaultResourceConfiguration; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper; import io.javaoperatorsdk.operator.processing.event.source.informer.Mappers; @@ -18,17 +19,22 @@ class DefaultInformerConfiguration extends DefaultResourceConfiguration implements InformerConfiguration { private final SecondaryToPrimaryMapper secondaryToPrimaryMapper; + private final Boolean inheritControllerNamespaces; protected DefaultInformerConfiguration(String labelSelector, Class resourceClass, SecondaryToPrimaryMapper secondaryToPrimaryMapper, - Set namespaces) { + Set namespaces, Boolean inheritControllerNamespaces) { super(labelSelector, resourceClass, namespaces); + this.inheritControllerNamespaces = inheritControllerNamespaces; this.secondaryToPrimaryMapper = Objects.requireNonNullElse(secondaryToPrimaryMapper, Mappers.fromOwnerReference()); } + public Boolean isInheritControllerNamespaces() { + return inheritControllerNamespaces; + } public SecondaryToPrimaryMapper getSecondaryToPrimaryMapper() { return secondaryToPrimaryMapper; @@ -45,7 +51,7 @@ class InformerConfigurationBuilder { private Set namespaces; private String labelSelector; private final Class resourceClass; - private boolean followNamespaceChanges = false; + private boolean inheritControllerNamespaces = false; private InformerConfigurationBuilder(Class resourceClass) { this.resourceClass = resourceClass; @@ -72,10 +78,17 @@ public InformerConfigurationBuilder withLabelSelector(String labelSelector) { return this; } + public

InformerConfigurationBuilder inheritControllerNamespaces( + EventSourceContext

context) { + namespaces = context.getControllerConfiguration().getEffectiveNamespaces(); + this.inheritControllerNamespaces = true; + return this; + } + public InformerConfiguration build() { return new DefaultInformerConfiguration<>(labelSelector, resourceClass, secondaryToPrimaryMapper, - namespaces); + namespaces, inheritControllerNamespaces); } } @@ -84,12 +97,4 @@ static InformerConfigurationBuilder from( return new InformerConfigurationBuilder<>(resourceClass); } - - static InformerConfigurationBuilder from( - InformerConfiguration configuration) { - return new InformerConfigurationBuilder(configuration.getResourceClass()) - .withNamespaces(configuration.getNamespaces()) - .withLabelSelector(configuration.getLabelSelector()) - .withSecondaryToPrimaryMapper(configuration.getSecondaryToPrimaryMapper()); - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java index 8ce04d752e..bda5dcb1dd 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java @@ -10,9 +10,9 @@ @Retention(RetentionPolicy.RUNTIME) @Target({ElementType.TYPE}) public @interface KubernetesDependent { - String SAME_AS_PARENT = "JOSDK_SAME_AS_PARENT"; + String SAME_AS_CONTROLLER = "JOSDK_SAME_AS_CONTROLLER"; - String[] DEFAULT_NAMESPACES = {SAME_AS_PARENT}; + String[] DEFAULT_NAMESPACES = {SAME_AS_CONTROLLER}; /** * Specified which namespaces this Controller monitors for custom resources events. If no @@ -21,7 +21,7 @@ * * @return the list of namespaces this controller monitors */ - String[] namespaces() default {SAME_AS_PARENT}; + String[] namespaces() default {SAME_AS_CONTROLLER}; /** * Optional label selector used to identify the set of custom resources the controller will acc diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 8b5c442886..f6b0e35451 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -37,7 +37,7 @@ public class InformerManager, Resource> client; private ResourceEventHandler eventHandler; - private Map>> indexers; + private Map>> indexers = new HashMap<>(); @Override public void start() throws OperatorException { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java index a2eeacf82b..05aa637f1f 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationTest.java @@ -10,7 +10,12 @@ class ControllerConfigurationTest { @Test void getCustomResourceClass() { - final ControllerConfiguration conf = () -> null; + final ControllerConfiguration conf = new ControllerConfiguration<>() { + @Override + public String getAssociatedReconcilerClassName() { + return null; + } + }; assertEquals(TestCustomResource.class, conf.getResourceClass()); } } From a6a0b4456d15a66f752d3585048ed2e2b2e9c289 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 29 Apr 2022 15:44:19 +0200 Subject: [PATCH 03/24] DR event source update --- .../operator/api/RegisteredController.java | 9 +++++++- .../informer/InformerConfiguration.java | 21 +++++++++++++------ .../operator/processing/Controller.java | 18 ++++++++++------ .../KubernetesDependentResource.java | 21 ++++++++++++------- .../processing/event/EventSourceManager.java | 2 +- 5 files changed, 49 insertions(+), 22 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java index fbdc1ad10d..0764386234 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java @@ -1,5 +1,7 @@ package io.javaoperatorsdk.operator.api; +import java.util.Set; + import io.javaoperatorsdk.operator.processing.Controller; public class RegisteredController { @@ -10,7 +12,12 @@ public RegisteredController(Controller controller) { this.controller = controller; } - public void changeNamespaces(String... namespaces) { + + public void changeNamespaces(Set namespaces) { controller.changeNamespaces(namespaces); } + + public void changeNamespaces(String... namespaces) { + changeNamespaces(Set.of(namespaces)); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index dbe0fd0ef0..a3afb47995 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -19,7 +19,7 @@ class DefaultInformerConfiguration extends DefaultResourceConfiguration implements InformerConfiguration { private final SecondaryToPrimaryMapper secondaryToPrimaryMapper; - private final Boolean inheritControllerNamespaces; + private final boolean inheritControllerNamespaces; protected DefaultInformerConfiguration(String labelSelector, Class resourceClass, @@ -32,7 +32,7 @@ protected DefaultInformerConfiguration(String labelSelector, Mappers.fromOwnerReference()); } - public Boolean isInheritControllerNamespaces() { + public Boolean isInheritControllerNamespacesOnChange() { return inheritControllerNamespaces; } @@ -42,6 +42,8 @@ public SecondaryToPrimaryMapper getSecondaryToPrimaryMapper() { } + Boolean isInheritControllerNamespacesOnChange(); + SecondaryToPrimaryMapper getSecondaryToPrimaryMapper(); @SuppressWarnings("unused") @@ -51,7 +53,7 @@ class InformerConfigurationBuilder { private Set namespaces; private String labelSelector; private final Class resourceClass; - private boolean inheritControllerNamespaces = false; + private boolean inheritControllerNamespacesOnChange = false; private InformerConfigurationBuilder(Class resourceClass) { this.resourceClass = resourceClass; @@ -78,17 +80,24 @@ public InformerConfigurationBuilder withLabelSelector(String labelSelector) { return this; } - public

InformerConfigurationBuilder inheritControllerNamespaces( + public

InformerConfigurationBuilder setAndInheritControllerNamespaces( + Set namespaces) { + this.namespaces = namespaces; + this.inheritControllerNamespacesOnChange = true; + return this; + } + + public

InformerConfigurationBuilder setAndInheritControllerNamespaces( EventSourceContext

context) { namespaces = context.getControllerConfiguration().getEffectiveNamespaces(); - this.inheritControllerNamespaces = true; + this.inheritControllerNamespacesOnChange = true; return this; } public InformerConfiguration build() { return new DefaultInformerConfiguration<>(labelSelector, resourceClass, secondaryToPrimaryMapper, - namespaces, inheritControllerNamespaces); + namespaces, inheritControllerNamespacesOnChange); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 1eff64e9ef..e22b17f2bf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -1,9 +1,6 @@ package io.javaoperatorsdk.operator.processing; -import java.util.ArrayList; -import java.util.LinkedHashMap; -import java.util.Map; -import java.util.Optional; +import java.util.*; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -41,6 +38,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceException; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; @SuppressWarnings({"unchecked", "rawtypes"}) @Ignore @@ -337,8 +335,16 @@ private void validateCRDWithLocalModelIfRequired(Class

resClass, String contr } } - public void changeNamespaces(String... namespaces) { - + public void changeNamespaces(Set namespaces) { + eventSourceManager.getRegisteredEventSources().forEach(es -> { + if (es instanceof InformerEventSource) { + InformerEventSource ies = (InformerEventSource) es; + if (ies.getConfiguration().isInheritControllerNamespacesOnChange()) { + ies.changeNamespaces(namespaces); + } + } + }); + eventSourceManager.getControllerResourceEventSource().changeNamespaces(namespaces); } private void throwMissingCRDException(String crdName, String specVersion, String controllerName) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index ca0b94b230..3f44839004 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -49,21 +49,26 @@ public KubernetesDependentResource(Class resourceType) { @Override public void configureWith(KubernetesDependentResourceConfig config) { - configureWith(config.labelSelector(), config.namespaces()); + configureWith(config.labelSelector(), config.namespaces(), !config.wereNamespacesConfigured()); } @SuppressWarnings("unchecked") - private void configureWith(String labelSelector, Set namespaces) { + private void configureWith(String labelSelector, Set namespaces, + boolean inheritNamespacesOnChange) { final SecondaryToPrimaryMapper primaryResourcesRetriever = (this instanceof SecondaryToPrimaryMapper) ? (SecondaryToPrimaryMapper) this : Mappers.fromOwnerReference(); - InformerConfiguration ic = + InformerConfiguration.InformerConfigurationBuilder ic = InformerConfiguration.from(resourceType()) .withLabelSelector(labelSelector) - .withNamespaces(namespaces) - .withSecondaryToPrimaryMapper(primaryResourcesRetriever) - .build(); - configureWith(new InformerEventSource<>(ic, client)); + .withSecondaryToPrimaryMapper(primaryResourcesRetriever); + if (inheritNamespacesOnChange) { + ic.setAndInheritControllerNamespaces(namespaces); + } else { + ic.withNamespaces(namespaces); + } + + configureWith(new InformerEventSource<>(ic.build(), client)); } /** @@ -139,7 +144,7 @@ protected NonNamespaceOperation, Resource> prepa @Override protected InformerEventSource createEventSource(EventSourceContext

context) { - configureWith(null, context.getControllerConfiguration().getNamespaces()); + configureWith(null, context.getControllerConfiguration().getNamespaces(), true); log.warn("Using default configuration for " + resourceType().getSimpleName() + " KubernetesDependentResource, call configureWith to provide configuration"); return eventSource(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 1ccc0ad2d9..95f1777de8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -162,7 +162,7 @@ EventHandler getEventHandler() { return eventProcessor; } - Set getRegisteredEventSources() { + public Set getRegisteredEventSources() { return eventSources.flatMappedSources() .map(NamedEventSource::original) .collect(Collectors.toCollection(LinkedHashSet::new)); From 96703be8dab32ba970b5343ba0a4c24dd23327dd Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 29 Apr 2022 15:54:30 +0200 Subject: [PATCH 04/24] improvements --- .../api/config/informer/InformerConfiguration.java | 4 ++-- .../event/source/informer/InformerEventSource.java | 8 ++++++++ .../processing/event/source/informer/InformerManager.java | 4 +--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index a3afb47995..cfcf5508a5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -32,7 +32,7 @@ protected DefaultInformerConfiguration(String labelSelector, Mappers.fromOwnerReference()); } - public Boolean isInheritControllerNamespacesOnChange() { + public boolean isInheritControllerNamespacesOnChange() { return inheritControllerNamespaces; } @@ -42,7 +42,7 @@ public SecondaryToPrimaryMapper getSecondaryToPrimaryMapper() { } - Boolean isInheritControllerNamespacesOnChange(); + boolean isInheritControllerNamespacesOnChange(); SecondaryToPrimaryMapper getSecondaryToPrimaryMapper(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 5d1d12875f..0bbbeb00c5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -266,4 +266,12 @@ public synchronized void cleanupOnCreateOrUpdateEventFiltering(ResourceID resour eventRecorder.stopEventRecording(resourceID); } + @Override + public void changeNamespaces(Set namespaces) { + if (!configuration.isInheritControllerNamespacesOnChange()) { + throw new IllegalStateException( + "InformerEventSource is not configured to change inherit namespaces."); + } + super.changeNamespaces(namespaces); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index f6b0e35451..4a5ef4378c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -80,9 +80,7 @@ public void changeNamespaces(Set namespaces) { var sourcesToRemove = sources.keySet().stream() .filter(k -> !namespaces.contains(k)).collect(Collectors.toSet()); log.debug("Stopping to watch namespaces: {} for {}", sourcesToRemove, this); - sourcesToRemove.forEach(k -> { - sources.remove(k).stop(); - }); + sourcesToRemove.forEach(k -> sources.remove(k).stop()); namespaces.forEach(ns -> { if (!sources.containsKey(ns)) { From 91f9699b532441ea8bf6126c5d9fcde95f131f95 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 2 May 2022 09:58:49 +0200 Subject: [PATCH 05/24] fix: impl improvements --- .../operator/processing/Controller.java | 11 +-- .../processing/event/EventSourceManager.java | 72 +++++++++---------- 2 files changed, 37 insertions(+), 46 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index e22b17f2bf..2dd18dc342 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -38,7 +38,6 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.KubernetesClientAware; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceException; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; -import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; @SuppressWarnings({"unchecked", "rawtypes"}) @Ignore @@ -336,15 +335,7 @@ private void validateCRDWithLocalModelIfRequired(Class

resClass, String contr } public void changeNamespaces(Set namespaces) { - eventSourceManager.getRegisteredEventSources().forEach(es -> { - if (es instanceof InformerEventSource) { - InformerEventSource ies = (InformerEventSource) es; - if (ies.getConfiguration().isInheritControllerNamespacesOnChange()) { - ies.changeNamespaces(namespaces); - } - } - }); - eventSourceManager.getControllerResourceEventSource().changeNamespaces(namespaces); + eventSourceManager.changeNamespaces(namespaces); } private void throwMissingCRDException(String crdName, String specVersion, String controllerName) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 95f1777de8..90c6f9f6e8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -4,7 +4,6 @@ import java.util.List; import java.util.Objects; import java.util.Set; -import java.util.concurrent.locks.ReentrantLock; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -21,13 +20,13 @@ import io.javaoperatorsdk.operator.processing.event.source.ResourceEventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; import io.javaoperatorsdk.operator.processing.event.source.timer.TimerEventSource; public class EventSourceManager implements LifecycleAware { private static final Logger log = LoggerFactory.getLogger(EventSourceManager.class); - private final ReentrantLock lock = new ReentrantLock(); private final EventSources eventSources = new EventSources<>(); private final EventProcessor eventProcessor; private final Controller controller; @@ -61,24 +60,19 @@ public EventSourceManager(Controller controller) { * {@link ControllerResourceEventSource} , which is started first. */ @Override - public void start() { - lock.lock(); - try { - for (var eventSource : eventSources) { - try { - logEventSourceEvent(eventSource, "Starting"); - eventSource.start(); - logEventSourceEvent(eventSource, "Started"); - } catch (MissingCRDException e) { - throw e; // leave untouched - } catch (Exception e) { - throw new OperatorException("Couldn't start source " + eventSource.name(), e); - } + public synchronized void start() { + for (var eventSource : eventSources) { + try { + logEventSourceEvent(eventSource, "Starting"); + eventSource.start(); + logEventSourceEvent(eventSource, "Started"); + } catch (MissingCRDException e) { + throw e; // leave untouched + } catch (Exception e) { + throw new OperatorException("Couldn't start source " + eventSource.name(), e); } - eventProcessor.start(); - } finally { - lock.unlock(); } + eventProcessor.start(); } @SuppressWarnings("rawtypes") @@ -95,22 +89,17 @@ private void logEventSourceEvent(NamedEventSource eventSource, String event) { } @Override - public void stop() { - lock.lock(); - try { - for (var eventSource : eventSources) { - try { - logEventSourceEvent(eventSource, "Stopping"); - eventSource.stop(); - logEventSourceEvent(eventSource, "Stopped"); - } catch (Exception e) { - log.warn("Error closing {} -> {}", eventSource.name(), e); - } + public synchronized void stop() { + for (var eventSource : eventSources) { + try { + logEventSourceEvent(eventSource, "Stopping"); + eventSource.stop(); + logEventSourceEvent(eventSource, "Stopped"); + } catch (Exception e) { + log.warn("Error closing {} -> {}", eventSource.name(), e); } - eventSources.clear(); - } finally { - lock.unlock(); } + eventSources.clear(); eventProcessor.stop(); } @@ -118,10 +107,9 @@ public final void registerEventSource(EventSource eventSource) throws OperatorEx registerEventSource(null, eventSource); } - public final void registerEventSource(String name, EventSource eventSource) + public final synchronized void registerEventSource(String name, EventSource eventSource) throws OperatorException { Objects.requireNonNull(eventSource, "EventSource must not be null"); - lock.lock(); try { if (name == null || name.isBlank()) { name = EventSourceInitializer.generateNameFor(eventSource); @@ -133,8 +121,6 @@ public final void registerEventSource(String name, EventSource eventSource) } catch (Exception e) { throw new OperatorException("Couldn't register event source: " + name + " for " + controller.getConfiguration().getName() + " controller`", e); - } finally { - lock.unlock(); } } @@ -158,6 +144,20 @@ public void broadcastOnResourceEvent(ResourceAction action, R resource, R oldRes } } + public void changeNamespaces(Set namespaces) { + eventProcessor.stop(); + getRegisteredEventSources().forEach(es -> { + if (es instanceof InformerEventSource) { + InformerEventSource ies = (InformerEventSource) es; + if (ies.getConfiguration().isInheritControllerNamespacesOnChange()) { + ies.changeNamespaces(namespaces); + } + } + }); + getControllerResourceEventSource().changeNamespaces(namespaces); + eventProcessor.start(); + } + EventHandler getEventHandler() { return eventProcessor; } From 5aeaccd01f1ece3799e2f7c9c2b093b9b6aeba47 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 2 May 2022 10:05:04 +0200 Subject: [PATCH 06/24] improvements --- .../operator/processing/event/EventSourceManager.java | 1 + 1 file changed, 1 insertion(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 90c6f9f6e8..506daa3024 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -144,6 +144,7 @@ public void broadcastOnResourceEvent(ResourceAction action, R resource, R oldRes } } + @SuppressWarnings({"unchecked", "rawtypes"}) public void changeNamespaces(Set namespaces) { eventProcessor.stop(); getRegisteredEventSources().forEach(es -> { From facfadf17340cb2bfc06a2c90f284fd952969f1a Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 2 May 2022 10:36:25 +0200 Subject: [PATCH 07/24] formatting --- .../io/javaoperatorsdk/operator/api/RegisteredController.java | 1 - 1 file changed, 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java index 0764386234..84622a8662 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java @@ -12,7 +12,6 @@ public RegisteredController(Controller controller) { this.controller = controller; } - public void changeNamespaces(Set namespaces) { controller.changeNamespaces(namespaces); } From a8df89bf7f72bcc6190f53eb183355c828d8cc38 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 3 May 2022 08:51:04 +0200 Subject: [PATCH 08/24] move and refactor of RegisteredController --- .../io/javaoperatorsdk/operator/Operator.java | 3 +-- .../operator/RegisteredController.java | 11 ++++++++++ .../operator/api/RegisteredController.java | 22 ------------------- .../operator/processing/Controller.java | 12 +++++----- 4 files changed, 19 insertions(+), 29 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java 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 7182283a8d..21b1216ced 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 @@ -13,7 +13,6 @@ import io.fabric8.kubernetes.client.DefaultKubernetesClient; import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.Version; -import io.javaoperatorsdk.operator.api.RegisteredController; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceOverrider; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; @@ -174,7 +173,7 @@ public RegisteredController register(Reconciler recon configuration.getName(), configuration.getResourceClass(), watchedNS); - return new RegisteredController(controller); + return controller; } /** diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java new file mode 100644 index 0000000000..f95d96bc8e --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java @@ -0,0 +1,11 @@ +package io.javaoperatorsdk.operator; + +import java.util.Set; + +public interface RegisteredController { + + void changeNamespaces(Set namespaces); + + void changeNamespaces(String... namespaces); + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java deleted file mode 100644 index 84622a8662..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/RegisteredController.java +++ /dev/null @@ -1,22 +0,0 @@ -package io.javaoperatorsdk.operator.api; - -import java.util.Set; - -import io.javaoperatorsdk.operator.processing.Controller; - -public class RegisteredController { - - private Controller controller; - - public RegisteredController(Controller controller) { - this.controller = controller; - } - - public void changeNamespaces(Set namespaces) { - controller.changeNamespaces(namespaces); - } - - public void changeNamespaces(String... namespaces) { - changeNamespaces(Set.of(namespaces)); - } -} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 2dd18dc342..fb22af53aa 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -13,10 +13,7 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.javaoperatorsdk.operator.AggregatedOperatorException; -import io.javaoperatorsdk.operator.CustomResourceUtils; -import io.javaoperatorsdk.operator.MissingCRDException; -import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.*; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; @@ -42,7 +39,7 @@ @SuppressWarnings({"unchecked", "rawtypes"}) @Ignore public class Controller

- implements Reconciler

, Cleaner

, LifecycleAware { + implements Reconciler

, Cleaner

, LifecycleAware, RegisteredController { private static final Logger log = LoggerFactory.getLogger(Controller.class); @@ -338,6 +335,11 @@ public void changeNamespaces(Set namespaces) { eventSourceManager.changeNamespaces(namespaces); } + @Override + public void changeNamespaces(String... namespaces) { + changeNamespaces(Set.of(namespaces)); + } + private void throwMissingCRDException(String crdName, String specVersion, String controllerName) { throw new MissingCRDException( crdName, From 55b8fc7d3c21d0b6baa6feb56a670e213b19cad8 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 3 May 2022 12:34:38 +0200 Subject: [PATCH 09/24] add integration test --- .../informer/InformerConfiguration.java | 12 +- .../KubernetesDependentResource.java | 2 +- .../operator/junit/OperatorExtension.java | 13 +- .../operator/ChangeNamespaceIT.java | 112 ++++++++++++++++++ .../ChangeNamespaceTestCustomResource.java | 22 ++++ ...angeNamespaceTestCustomResourceStatus.java | 16 +++ .../ChangeNamespaceTestReconciler.java | 77 ++++++++++++ 7 files changed, 246 insertions(+), 8 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/ChangeNamespaceIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResourceStatus.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index cfcf5508a5..72fed27ee5 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -19,21 +19,21 @@ class DefaultInformerConfiguration extends DefaultResourceConfiguration implements InformerConfiguration { private final SecondaryToPrimaryMapper secondaryToPrimaryMapper; - private final boolean inheritControllerNamespaces; + private final boolean followControllerNamespaceChanges; protected DefaultInformerConfiguration(String labelSelector, Class resourceClass, SecondaryToPrimaryMapper secondaryToPrimaryMapper, - Set namespaces, Boolean inheritControllerNamespaces) { + Set namespaces, Boolean followControllerNamespaceChanges) { super(labelSelector, resourceClass, namespaces); - this.inheritControllerNamespaces = inheritControllerNamespaces; + this.followControllerNamespaceChanges = followControllerNamespaceChanges; this.secondaryToPrimaryMapper = Objects.requireNonNullElse(secondaryToPrimaryMapper, Mappers.fromOwnerReference()); } public boolean isInheritControllerNamespacesOnChange() { - return inheritControllerNamespaces; + return followControllerNamespaceChanges; } public SecondaryToPrimaryMapper getSecondaryToPrimaryMapper() { @@ -80,14 +80,14 @@ public InformerConfigurationBuilder withLabelSelector(String labelSelector) { return this; } - public

InformerConfigurationBuilder setAndInheritControllerNamespaces( + public

InformerConfigurationBuilder setAndFollowControllerNamespaceChanges( Set namespaces) { this.namespaces = namespaces; this.inheritControllerNamespacesOnChange = true; return this; } - public

InformerConfigurationBuilder setAndInheritControllerNamespaces( + public

InformerConfigurationBuilder setAndFollowControllerNamespaceChanges( EventSourceContext

context) { namespaces = context.getControllerConfiguration().getEffectiveNamespaces(); this.inheritControllerNamespacesOnChange = true; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 3f44839004..7d242d23f7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -63,7 +63,7 @@ private void configureWith(String labelSelector, Set namespaces, .withLabelSelector(labelSelector) .withSecondaryToPrimaryMapper(primaryResourcesRetriever); if (inheritNamespacesOnChange) { - ic.setAndInheritControllerNamespaces(namespaces); + ic.setAndFollowControllerNamespaceChanges(namespaces); } else { ic.withNamespaces(namespaces); } diff --git a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java index d95e9466a7..5f72a840ef 100644 --- a/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java +++ b/operator-framework-junit5/src/main/java/io/javaoperatorsdk/operator/junit/OperatorExtension.java @@ -3,7 +3,9 @@ import java.io.InputStream; import java.time.Duration; import java.util.ArrayList; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -15,6 +17,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.LocalPortForward; import io.javaoperatorsdk.operator.Operator; +import io.javaoperatorsdk.operator.RegisteredController; import io.javaoperatorsdk.operator.api.config.ConfigurationService; import io.javaoperatorsdk.operator.api.config.ControllerConfigurationOverrider; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; @@ -32,6 +35,7 @@ public class OperatorExtension extends AbstractOperatorExtension { private final List reconcilers; private List portForwards; private List localPortForwards; + private Map registeredControllers; private OperatorExtension( ConfigurationService configurationService, @@ -53,6 +57,7 @@ private OperatorExtension( this.portForwards = portForwards; this.localPortForwards = new ArrayList<>(portForwards.size()); this.operator = new Operator(getKubernetesClient(), this.configurationService); + this.registeredControllers = new HashMap<>(); } /** @@ -85,6 +90,11 @@ public T getReconcilerOfType(Class type) { () -> new IllegalArgumentException("Unable to find a reconciler of type: " + type)); } + public RegisteredController getRegisteredControllerForReconcile( + Class type) { + return registeredControllers.get(getReconcilerOfType(type)); + } + @SuppressWarnings("unchecked") protected void before(ExtensionContext context) { super.before(context); @@ -133,7 +143,8 @@ protected void before(ExtensionContext context) { ((KubernetesClientAware) ref.reconciler).setKubernetesClient(kubernetesClient); } - this.operator.register(ref.reconciler, oconfig.build()); + var registeredController = this.operator.register(ref.reconciler, oconfig.build()); + registeredControllers.put(ref.reconciler, registeredController); } LOGGER.debug("Starting the operator locally"); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ChangeNamespaceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ChangeNamespaceIT.java new file mode 100644 index 0000000000..bec51df3f2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ChangeNamespaceIT.java @@ -0,0 +1,112 @@ +package io.javaoperatorsdk.operator; + +import java.time.Duration; +import java.util.Map; +import java.util.Set; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.Namespace; +import io.fabric8.kubernetes.api.model.NamespaceBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.junit.OperatorExtension; +import io.javaoperatorsdk.operator.sample.changenamespace.ChangeNamespaceTestCustomResource; +import io.javaoperatorsdk.operator.sample.changenamespace.ChangeNamespaceTestReconciler; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.awaitility.Awaitility.await; + +class ChangeNamespaceIT { + + public static final String TEST_RESOURCE_NAME_1 = "test1"; + public static final String TEST_RESOURCE_NAME_2 = "test2"; + public static final String TEST_RESOURCE_NAME_3 = "test3"; + public static final String ADDITIONAL_TEST_NAMESPACE = "additional-test-namespace"; + @RegisterExtension + OperatorExtension operator = + OperatorExtension.builder().withReconciler(new ChangeNamespaceTestReconciler()).build(); + + @Test + void addNewAndRemoveOldNamespaceTest() { + try { + operator.create(ChangeNamespaceTestCustomResource.class, + customResource(TEST_RESOURCE_NAME_1)); + + await().pollDelay(Duration.ofMillis(100)).untilAsserted(() -> assertThat( + operator.get(ChangeNamespaceTestCustomResource.class, TEST_RESOURCE_NAME_1) + .getStatus().getNumberOfStatusUpdates()).isEqualTo(2)); + + client().namespaces().create(additionalTestNamespace()); + createResourceInTestNamespace(); + + await().pollDelay(Duration.ofMillis(200)).untilAsserted( + () -> assertThat(client().resources(ChangeNamespaceTestCustomResource.class) + .inNamespace(ADDITIONAL_TEST_NAMESPACE) + .withName(TEST_RESOURCE_NAME_2).get().getStatus().getNumberOfStatusUpdates()) + .isEqualTo(0)); + + // adding additional namespace + RegisteredController registeredController = + operator.getRegisteredControllerForReconcile(ChangeNamespaceTestReconciler.class); + registeredController + .changeNamespaces(Set.of(operator.getNamespace(), ADDITIONAL_TEST_NAMESPACE)); + + await().untilAsserted( + () -> assertThat(client().resources(ChangeNamespaceTestCustomResource.class) + .inNamespace(ADDITIONAL_TEST_NAMESPACE) + .withName(TEST_RESOURCE_NAME_2).get().getStatus().getNumberOfStatusUpdates()) + .withFailMessage("Informers don't watch the new namespace") + .isEqualTo(2)); + + // removing a namespace + registeredController.changeNamespaces(Set.of(ADDITIONAL_TEST_NAMESPACE)); + + operator.create(ChangeNamespaceTestCustomResource.class, + customResource(TEST_RESOURCE_NAME_3)); + await().pollDelay(Duration.ofMillis(200)) + .untilAsserted(() -> assertThat( + operator.get(ChangeNamespaceTestCustomResource.class, TEST_RESOURCE_NAME_3) + .getStatus().getNumberOfStatusUpdates()).isZero()); + + + ConfigMap firstMap = operator.get(ConfigMap.class, TEST_RESOURCE_NAME_1); + firstMap.setData(Map.of("data", "newdata")); + operator.replace(ConfigMap.class, firstMap); + + assertThat(operator.get(ChangeNamespaceTestCustomResource.class, TEST_RESOURCE_NAME_1) + .getStatus().getNumberOfStatusUpdates()) + .withFailMessage("ConfigMap change induced a reconcile") + .isEqualTo(2); + + } finally { + client().namespaces().delete(additionalTestNamespace()); + } + } + + private void createResourceInTestNamespace() { + var res = customResource(TEST_RESOURCE_NAME_2); + client().resources(ChangeNamespaceTestCustomResource.class) + .inNamespace(ADDITIONAL_TEST_NAMESPACE) + .create(res); + } + + private KubernetesClient client() { + return operator.getKubernetesClient(); + } + + private Namespace additionalTestNamespace() { + return new NamespaceBuilder().withMetadata(new ObjectMetaBuilder() + .withName(ADDITIONAL_TEST_NAMESPACE) + .build()).build(); + } + + private ChangeNamespaceTestCustomResource customResource(String name) { + ChangeNamespaceTestCustomResource customResource = new ChangeNamespaceTestCustomResource(); + customResource.setMetadata( + new ObjectMetaBuilder().withName(name).build()); + return customResource; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java new file mode 100644 index 0000000000..2c39872f93 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java @@ -0,0 +1,22 @@ +package io.javaoperatorsdk.operator.sample.changenamespace; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Kind; +import io.fabric8.kubernetes.model.annotation.ShortNames; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.javaoperatorsdk") +@Version("v1") +@Kind("MaxIntervalTestCustomResource") +@ShortNames("mit") +public class ChangeNamespaceTestCustomResource + extends CustomResource + implements Namespaced { + + @Override + protected ChangeNamespaceTestCustomResourceStatus initStatus() { + return new ChangeNamespaceTestCustomResourceStatus(); + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResourceStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResourceStatus.java new file mode 100644 index 0000000000..a3858cda05 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResourceStatus.java @@ -0,0 +1,16 @@ +package io.javaoperatorsdk.operator.sample.changenamespace; + +public class ChangeNamespaceTestCustomResourceStatus { + + private int numberOfStatusUpdates = 0; + + public int getNumberOfStatusUpdates() { + return numberOfStatusUpdates; + } + + public ChangeNamespaceTestCustomResourceStatus setNumberOfStatusUpdates( + int numberOfStatusUpdates) { + this.numberOfStatusUpdates = numberOfStatusUpdates; + return this; + } +} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java new file mode 100644 index 0000000000..a9318e7bf2 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java @@ -0,0 +1,77 @@ +package io.javaoperatorsdk.operator.sample.changenamespace; + +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.junit.KubernetesClientAware; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; +import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; + +@ControllerConfiguration +public class ChangeNamespaceTestReconciler + implements Reconciler, TestExecutionInfoProvider, + EventSourceInitializer, KubernetesClientAware { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + private KubernetesClient client; + + @Override + public Map prepareEventSources( + EventSourceContext context) { + + InformerEventSource configMapES = + new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class) + .setAndFollowControllerNamespaceChanges(context) + .build(), context); + + return EventSourceInitializer.nameEventSources(configMapES); + } + + @Override + public UpdateControl reconcile( + ChangeNamespaceTestCustomResource primary, + Context context) { + + var actualConfigMap = context.getSecondaryResource(ConfigMap.class); + if (actualConfigMap.isEmpty()) { + client.configMaps().inNamespace(primary.getMetadata().getNamespace()) + .create(configMap(primary)); + } + + numberOfExecutions.addAndGet(1); + + var statusUpdates = primary.getStatus().getNumberOfStatusUpdates(); + primary.getStatus().setNumberOfStatusUpdates(statusUpdates + 1); + return UpdateControl.updateStatus(primary); + } + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + + private ConfigMap configMap(ChangeNamespaceTestCustomResource primary) { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new ObjectMetaBuilder().withName(primary.getMetadata().getName()) + .withNamespace(primary.getMetadata().getNamespace()) + .build()); + configMap.setData(Map.of("data", primary.getMetadata().getName())); + configMap.addOwnerReference(primary); + return configMap; + } + + @Override + public KubernetesClient getKubernetesClient() { + return client; + } + + @Override + public void setKubernetesClient(KubernetesClient kubernetesClient) { + this.client = kubernetesClient; + } +} From 58d2b0b7a70375551a1d1ce83c6dcb49daf3d663 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 3 May 2022 14:10:11 +0200 Subject: [PATCH 10/24] unit test for ES manager --- .../processing/event/EventSourceManager.java | 12 +++++- .../operator/MockKubernetesClient.java | 3 ++ .../event/EventSourceManagerTest.java | 40 +++++++++++++++---- .../operator/ChangeNamespaceIT.java | 2 +- 4 files changed, 47 insertions(+), 10 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 506daa3024..df38ad9848 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -27,17 +27,27 @@ public class EventSourceManager implements LifecycleAware private static final Logger log = LoggerFactory.getLogger(EventSourceManager.class); - private final EventSources eventSources = new EventSources<>(); + private final EventSources eventSources; private final EventProcessor eventProcessor; private final Controller controller; EventSourceManager(EventProcessor eventProcessor) { + this(eventProcessor, new EventSources<>()); + } + + EventSourceManager(EventProcessor eventProcessor, EventSources eventSources) { this.eventProcessor = eventProcessor; + this.eventSources = new EventSources<>(); controller = null; registerEventSource(eventSources.retryEventSource()); } public EventSourceManager(Controller controller) { + this(controller, new EventSources<>()); + } + + EventSourceManager(Controller controller, EventSources eventSources) { + this.eventSources = eventSources; this.controller = controller; // controller event source needs to be available before we create the event processor final var controllerEventSource = eventSources.initControllerEventSource(controller); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java index 0c2db0f6b1..b282e11fe5 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java @@ -11,6 +11,7 @@ import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.informers.SharedIndexInformer; +import io.fabric8.kubernetes.client.informers.cache.Indexer; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyLong; @@ -36,6 +37,8 @@ public static KubernetesClient client(Class clazz) { when(resources.inAnyNamespace()).thenReturn(inAnyNamespace); when(inAnyNamespace.withLabelSelector(nullable(String.class))).thenReturn(filterable); SharedIndexInformer informer = mock(SharedIndexInformer.class); + Indexer mockIndexer = mock(Indexer.class); + when(informer.getIndexer()).thenReturn(mockIndexer); when(filterable.runnableInformer(anyLong())).thenReturn(informer); when(client.resources(clazz)).thenReturn(resources); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index f09605d095..a776a8771e 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -8,11 +8,13 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.event.source.CachingEventSource; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; import io.javaoperatorsdk.operator.processing.event.source.timer.TimerEventSource; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; @@ -21,17 +23,13 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.mockito.Mockito.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; @SuppressWarnings({"rawtypes", "unchecked"}) class EventSourceManagerTest { - private final EventProcessor eventHandler = mock(EventProcessor.class); - private final EventSourceManager eventSourceManager = new EventSourceManager(eventHandler); + private final EventProcessor eventProcessor = mock(EventProcessor.class); + private final EventSourceManager eventSourceManager = new EventSourceManager(eventProcessor); @Test public void registersEventSource() { @@ -42,7 +40,7 @@ public void registersEventSource() { Set registeredSources = eventSourceManager.getRegisteredEventSources(); assertThat(registeredSources).contains(eventSource); - verify(eventSource, times(1)).setEventHandler(eq(eventSourceManager.getEventHandler())); + verify(eventSource, times(1)).setEventHandler(eventSourceManager.getEventHandler()); } @Test @@ -139,6 +137,32 @@ void retrievingAnEventSourceWhenMultipleAreRegisteredForATypeShouldRequireAQuali eventSource); } + @Test + void changesNamespacesOnControllerAndInformerEventSources() { + String newNamespaces = "new-namespace"; + + final var configuration = MockControllerConfiguration.forResource(HasMetadata.class); + final Controller controller = new Controller(mock(Reconciler.class), configuration, + MockKubernetesClient.client(HasMetadata.class)); + + EventSources eventSources = spy(new EventSources()); + var controllerResourceEventSourceMock = mock(ControllerResourceEventSource.class); + doReturn(controllerResourceEventSourceMock).when(eventSources).controllerResourceEventSource(); + var manager = new EventSourceManager(controller, eventSources); + + InformerConfiguration informerConfigurationMock = mock(InformerConfiguration.class); + when(informerConfigurationMock.isInheritControllerNamespacesOnChange()).thenReturn(true); + InformerEventSource informerEventSource = mock(InformerEventSource.class); + when(informerEventSource.resourceType()).thenReturn(TestCustomResource.class); + when(informerEventSource.getConfiguration()).thenReturn(informerConfigurationMock); + manager.registerEventSource("ies", informerEventSource); + + manager.changeNamespaces(Set.of(newNamespaces)); + + verify(informerEventSource, times(1)).changeNamespaces(Set.of(newNamespaces)); + verify(controllerResourceEventSourceMock, times(1)).changeNamespaces(Set.of(newNamespaces)); + } + private EventSourceManager initManager() { final var configuration = MockControllerConfiguration.forResource(HasMetadata.class); final Controller controller = new Controller(mock(Reconciler.class), configuration, diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ChangeNamespaceIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ChangeNamespaceIT.java index bec51df3f2..56802ca8a3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ChangeNamespaceIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ChangeNamespaceIT.java @@ -46,7 +46,7 @@ void addNewAndRemoveOldNamespaceTest() { () -> assertThat(client().resources(ChangeNamespaceTestCustomResource.class) .inNamespace(ADDITIONAL_TEST_NAMESPACE) .withName(TEST_RESOURCE_NAME_2).get().getStatus().getNumberOfStatusUpdates()) - .isEqualTo(0)); + .isZero()); // adding additional namespace RegisteredController registeredController = From 6cee3c544e257f2dfe0700b69eaaecc5424a947d Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 3 May 2022 14:23:40 +0200 Subject: [PATCH 11/24] unit test --- .../operator/processing/event/EventProcessor.java | 4 ---- .../operator/processing/event/EventSourceManager.java | 11 ----------- .../processing/event/EventSourceManagerTest.java | 2 +- 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java index 9cdb73b5aa..a7ec8c8494 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventProcessor.java @@ -98,10 +98,6 @@ private EventProcessor( this.eventSourceManager = eventSourceManager; } - EventMarker getEventMarker() { - return eventMarker; - } - @Override public void handleEvent(Event event) { lock.lock(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index df38ad9848..7c343131ae 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -31,17 +31,6 @@ public class EventSourceManager implements LifecycleAware private final EventProcessor eventProcessor; private final Controller controller; - EventSourceManager(EventProcessor eventProcessor) { - this(eventProcessor, new EventSources<>()); - } - - EventSourceManager(EventProcessor eventProcessor, EventSources eventSources) { - this.eventProcessor = eventProcessor; - this.eventSources = new EventSources<>(); - controller = null; - registerEventSource(eventSources.retryEventSource()); - } - public EventSourceManager(Controller controller) { this(controller, new EventSources<>()); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index a776a8771e..d8c618f6d6 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -29,7 +29,7 @@ class EventSourceManagerTest { private final EventProcessor eventProcessor = mock(EventProcessor.class); - private final EventSourceManager eventSourceManager = new EventSourceManager(eventProcessor); + private final EventSourceManager eventSourceManager = initManager(); @Test public void registersEventSource() { From 6f2e55f3b701726e09629b8d0799223a2796665c Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 3 May 2022 15:29:04 +0200 Subject: [PATCH 12/24] docs --- docs/documentation/features.md | 60 ++++++++++++++++++- .../operator/RegisteredController.java | 8 +++ .../informer/InformerConfiguration.java | 10 +++- .../processing/event/EventSourceManager.java | 2 +- .../source/informer/InformerEventSource.java | 2 +- .../source/informer/InformerManager.java | 2 +- .../event/EventSourceManagerTest.java | 3 +- 7 files changed, 79 insertions(+), 8 deletions(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 1b9c534f46..0a6bdf33c5 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -458,9 +458,67 @@ following attributes are available in most parts of reconciliation logic and dur For more information about MDC see this [link](https://www.baeldung.com/mdc-in-log4j-2-logback). +## Dynamically Adjusting Target Namespaces + +A controller can be configured to watch a set of namespaces (not only a single namespace or the whole cluster). +The framework supports to dynamically change the list of these namespaces while the operator is running. +When a reconciler is registered the +[`RegisteredController`](https://github.com/java-operator-sdk/java-operator-sdk/blob/ec37025a15046d8f409c77616110024bf32c3416/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java#L5-L5) +is returned, which provides the related methods. These namespaces are meant to be changed when the operator is already +running. + +In a real life scenario usually the list of the target namespaces configured in `ConfigMap` or other input, +this part however is out of the scope of the framework. So in case you want to use a `ConfigMap` and react to change +of it, registering an Informer and calling the `RegisteredController` is up to the developer to implement. + +```java + + public static void main(String[] args) throws IOException { + KubernetesClient client = new DefaultKubernetesClient(); + Operator operator = new Operator(client); + RegisteredController registeredController = operator.register(new WebPageReconciler(client)); + operator.installShutdownHook(); + operator.start(); + + // call registeredController further while operator is running + } + +``` + +If a target namespaces change for a controller, it might be desirable to change the target namespaces of registered +`InformerEventSource`-s. In order to express this, the InformerEventSource needs to be configured to +`followControllerNamespaceChanges`, this the related method in `InformerConfiguration` should return `true`: + +```java + +@ControllerConfiguration +public class MyReconciler + implements Reconciler, EventSourceInitializer{ + + @Override + public Map prepareEventSources( + EventSourceContext context) { + + InformerEventSource configMapES = + new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class) + .setAndFollowControllerNamespaceChanges(context) + .build(), context); + + return EventSourceInitializer.nameEventSources(configMapES); + } + +} +``` + +As seen in the above code snippet, the informer will have the initial namespaces inherited from controller, but +also will adjust the target namespaces if it changes for the controller. + +See also the [integration test](https://github.com/java-operator-sdk/java-operator-sdk/blob/ec37025a15046d8f409c77616110024bf32c3416/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java) +for this feature. + ## Monitoring with Micrometer -## Automatic generation of CRDs +## Automatic Generation of CRDs Note that this is feature of [Fabric8 Kubernetes Client](https://github.com/fabric8io/kubernetes-client) not the JOSDK. But it's worth to mention here. diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java index f95d96bc8e..183b2e07d9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java @@ -4,6 +4,14 @@ public interface RegisteredController { + /** + * If the controller and possibly registered + * {@link io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource} + * watches a set of namespaces this set can be adjusted dynamically, this when the operator is + * running. + * + * @param namespaces target namespaces to watch + */ void changeNamespaces(Set namespaces); void changeNamespaces(String... namespaces); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index 72fed27ee5..3ff5165874 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -32,7 +32,7 @@ protected DefaultInformerConfiguration(String labelSelector, Mappers.fromOwnerReference()); } - public boolean isInheritControllerNamespacesOnChange() { + public boolean followControllerNamespaceChanges() { return followControllerNamespaceChanges; } @@ -42,7 +42,13 @@ public SecondaryToPrimaryMapper getSecondaryToPrimaryMapper() { } - boolean isInheritControllerNamespacesOnChange(); + /** + * Used in case the watched namespaces are changed dynamically, thus when operator is running (See + * {@link io.javaoperatorsdk.operator.RegisteredController}). If true, changing the target + * namespaces of a controller would result to change target namespaces for the + * InformerEventSource. + */ + boolean followControllerNamespaceChanges(); SecondaryToPrimaryMapper getSecondaryToPrimaryMapper(); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 7c343131ae..c9d38b2768 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -149,7 +149,7 @@ public void changeNamespaces(Set namespaces) { getRegisteredEventSources().forEach(es -> { if (es instanceof InformerEventSource) { InformerEventSource ies = (InformerEventSource) es; - if (ies.getConfiguration().isInheritControllerNamespacesOnChange()) { + if (ies.getConfiguration().followControllerNamespaceChanges()) { ies.changeNamespaces(namespaces); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 0bbbeb00c5..ab26c089c9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -268,7 +268,7 @@ public synchronized void cleanupOnCreateOrUpdateEventFiltering(ResourceID resour @Override public void changeNamespaces(Set namespaces) { - if (!configuration.isInheritControllerNamespacesOnChange()) { + if (!configuration.followControllerNamespaceChanges()) { throw new IllegalStateException( "InformerEventSource is not configured to change inherit namespaces."); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 4a5ef4378c..02a0e1d66f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -37,7 +37,7 @@ public class InformerManager, Resource> client; private ResourceEventHandler eventHandler; - private Map>> indexers = new HashMap<>(); + private final Map>> indexers = new HashMap<>(); @Override public void start() throws OperatorException { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index d8c618f6d6..a8880a51d5 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -28,7 +28,6 @@ @SuppressWarnings({"rawtypes", "unchecked"}) class EventSourceManagerTest { - private final EventProcessor eventProcessor = mock(EventProcessor.class); private final EventSourceManager eventSourceManager = initManager(); @Test @@ -151,7 +150,7 @@ void changesNamespacesOnControllerAndInformerEventSources() { var manager = new EventSourceManager(controller, eventSources); InformerConfiguration informerConfigurationMock = mock(InformerConfiguration.class); - when(informerConfigurationMock.isInheritControllerNamespacesOnChange()).thenReturn(true); + when(informerConfigurationMock.followControllerNamespaceChanges()).thenReturn(true); InformerEventSource informerEventSource = mock(InformerEventSource.class); when(informerEventSource.resourceType()).thenReturn(TestCustomResource.class); when(informerEventSource.getConfiguration()).thenReturn(informerConfigurationMock); From 59c3191588dab5b5e2cbcf850afb8a5435803150 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 3 May 2022 15:31:58 +0200 Subject: [PATCH 13/24] docs update --- docs/documentation/features.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 0a6bdf33c5..483899a050 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -458,7 +458,7 @@ following attributes are available in most parts of reconciliation logic and dur For more information about MDC see this [link](https://www.baeldung.com/mdc-in-log4j-2-logback). -## Dynamically Adjusting Target Namespaces +## Dynamically Changing Target Namespaces A controller can be configured to watch a set of namespaces (not only a single namespace or the whole cluster). The framework supports to dynamically change the list of these namespaces while the operator is running. From 00ec65d4451c564750f31ce06ee09108dad8246a Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 3 May 2022 15:40:55 +0200 Subject: [PATCH 14/24] additional input validation --- .../operator/processing/Controller.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index fb22af53aa..9174f5b298 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -19,15 +19,7 @@ import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.monitoring.Metrics.ControllerExecution; -import io.javaoperatorsdk.operator.api.reconciler.Cleaner; -import io.javaoperatorsdk.operator.api.reconciler.Context; -import io.javaoperatorsdk.operator.api.reconciler.ContextInitializer; -import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; -import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; -import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; -import io.javaoperatorsdk.operator.api.reconciler.Ignore; -import io.javaoperatorsdk.operator.api.reconciler.Reconciler; -import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.api.reconciler.dependent.Deleter; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; @@ -36,6 +28,8 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.ManagedDependentResourceException; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; +import static io.javaoperatorsdk.operator.api.reconciler.Constants.WATCH_CURRENT_NAMESPACE; + @SuppressWarnings({"unchecked", "rawtypes"}) @Ignore public class Controller

@@ -332,6 +326,10 @@ private void validateCRDWithLocalModelIfRequired(Class

resClass, String contr } public void changeNamespaces(Set namespaces) { + if (namespaces.contains(Constants.WATCH_ALL_NAMESPACES) + || namespaces.contains(WATCH_CURRENT_NAMESPACE)) { + throw new OperatorException("Unexpected value in target namespaces: " + namespaces); + } eventSourceManager.changeNamespaces(namespaces); } From 0ae3aabad94b53c32505aa2b6b394733c0098890 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 3 May 2022 17:04:49 +0200 Subject: [PATCH 15/24] refactor: follow existing pattern to make method names more discoverable --- .../api/config/ResourceConfiguration.java | 4 +- .../informer/InformerConfiguration.java | 61 ++++++++++++++----- .../KubernetesDependentResource.java | 24 +++----- .../ChangeNamespaceTestReconciler.java | 2 +- 4 files changed, 57 insertions(+), 34 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java index 58a41070d2..833063af93 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java @@ -10,8 +10,8 @@ public interface ResourceConfiguration { - Set DEFAULT_NAMESPACES = Set.of(Constants.WATCH_ALL_NAMESPACES); - Set CURRENT_NAMESPACE_ONLY = Set.of(Constants.WATCH_CURRENT_NAMESPACE); + Set DEFAULT_NAMESPACES = Collections.singleton(Constants.WATCH_ALL_NAMESPACES); + Set CURRENT_NAMESPACE_ONLY = Collections.singleton(Constants.WATCH_CURRENT_NAMESPACE); default String getResourceTypeName() { return ReconcilerUtils.getResourceTypeName(getResourceClass()); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index 3ff5165874..f5b07470db 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -1,6 +1,5 @@ package io.javaoperatorsdk.operator.api.config.informer; -import java.util.Collections; import java.util.Objects; import java.util.Set; @@ -11,7 +10,6 @@ import io.javaoperatorsdk.operator.processing.event.source.SecondaryToPrimaryMapper; import io.javaoperatorsdk.operator.processing.event.source.informer.Mappers; -@SuppressWarnings("rawtypes") public interface InformerConfiguration extends ResourceConfiguration { @@ -72,34 +70,67 @@ public InformerConfigurationBuilder withSecondaryToPrimaryMapper( } public InformerConfigurationBuilder withNamespaces(String... namespaces) { - this.namespaces = namespaces != null ? Set.of(namespaces) : Collections.emptySet(); - return this; + return withNamespaces( + namespaces != null ? Set.of(namespaces) : ResourceConfiguration.DEFAULT_NAMESPACES); } public InformerConfigurationBuilder withNamespaces(Set namespaces) { - this.namespaces = namespaces != null ? namespaces : Collections.emptySet(); - return this; - } - - public InformerConfigurationBuilder withLabelSelector(String labelSelector) { - this.labelSelector = labelSelector; - return this; + return withNamespaces(namespaces, false); } - public

InformerConfigurationBuilder setAndFollowControllerNamespaceChanges( - Set namespaces) { - this.namespaces = namespaces; + /** + * Sets the initial set of namespaces to watch (typically extracted from the parent + * {@link io.javaoperatorsdk.operator.processing.Controller}'s configuration), specifying + * whether changes made to the parent controller configured namespaces should be tracked or not. + * + * @param namespaces the initial set of namespaces to watch + * @param followChanges {@code true} to follow the changes made to the parent controller + * namespaces, {@code false} otherwise + * @return the builder instance so that calls can be chained fluently + */ + public InformerConfigurationBuilder withNamespaces(Set namespaces, + boolean followChanges) { + this.namespaces = namespaces != null ? namespaces : ResourceConfiguration.DEFAULT_NAMESPACES; this.inheritControllerNamespacesOnChange = true; return this; } - public

InformerConfigurationBuilder setAndFollowControllerNamespaceChanges( + /** + * Configures the informer to watch and track the same namespaces as the parent + * {@link io.javaoperatorsdk.operator.processing.Controller}, meaning that the informer will be + * restarted to watch the new namespaces if the parent controller's namespace configuration + * changes. + * + * @param context {@link EventSourceContext} from which the parent + * {@link io.javaoperatorsdk.operator.processing.Controller}'s configuration is retrieved + * @param

the primary resource type associated with the parent controller + * @return the builder instance so that calls can be chained fluently + */ + public

InformerConfigurationBuilder withNamespacesInheritedFromController( EventSourceContext

context) { namespaces = context.getControllerConfiguration().getEffectiveNamespaces(); this.inheritControllerNamespacesOnChange = true; return this; } + /** + * Whether or not the associated informer should track changes made to the parent + * {@link io.javaoperatorsdk.operator.processing.Controller}'s namespaces configuration. + * + * @param followChanges {@code true} to reconfigure the associated informer when the parent + * controller's namespaces are reconfigured, {@code false} otherwise + * @return the builder instance so that calls can be chained fluently + */ + public InformerConfigurationBuilder followNamespaceChanges(boolean followChanges) { + this.inheritControllerNamespacesOnChange = followChanges; + return this; + } + + public InformerConfigurationBuilder withLabelSelector(String labelSelector) { + this.labelSelector = labelSelector; + return this; + } + public InformerConfiguration build() { return new DefaultInformerConfiguration<>(labelSelector, resourceClass, secondaryToPrimaryMapper, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 7d242d23f7..4196eb0e7e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -58,17 +58,13 @@ private void configureWith(String labelSelector, Set namespaces, final SecondaryToPrimaryMapper primaryResourcesRetriever = (this instanceof SecondaryToPrimaryMapper) ? (SecondaryToPrimaryMapper) this : Mappers.fromOwnerReference(); - InformerConfiguration.InformerConfigurationBuilder ic = - InformerConfiguration.from(resourceType()) - .withLabelSelector(labelSelector) - .withSecondaryToPrimaryMapper(primaryResourcesRetriever); - if (inheritNamespacesOnChange) { - ic.setAndFollowControllerNamespaceChanges(namespaces); - } else { - ic.withNamespaces(namespaces); - } + var ic = InformerConfiguration.from(resourceType()) + .withLabelSelector(labelSelector) + .withSecondaryToPrimaryMapper(primaryResourcesRetriever) + .withNamespaces(namespaces, inheritNamespacesOnChange) + .build(); - configureWith(new InformerEventSource<>(ic.build(), client)); + configureWith(new InformerEventSource<>(ic, client)); } /** @@ -83,11 +79,9 @@ public void configureWith(InformerEventSource informerEventSource) { protected R handleCreate(R desired, P primary, Context

context) { ResourceID resourceID = ResourceID.fromResource(desired); - R created = null; try { prepareEventFiltering(desired, resourceID); - created = super.handleCreate(desired, primary, context); - return created; + return super.handleCreate(desired, primary, context); } catch (RuntimeException e) { cleanupAfterEventFiltering(resourceID); throw e; @@ -96,11 +90,9 @@ protected R handleCreate(R desired, P primary, Context

context) { protected R handleUpdate(R actual, R desired, P primary, Context

context) { ResourceID resourceID = ResourceID.fromResource(desired); - R updated = null; try { prepareEventFiltering(desired, resourceID); - updated = super.handleUpdate(actual, desired, primary, context); - return updated; + return super.handleUpdate(actual, desired, primary, context); } catch (RuntimeException e) { cleanupAfterEventFiltering(resourceID); throw e; diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java index a9318e7bf2..8b2cf9dc1f 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java @@ -27,7 +27,7 @@ public Map prepareEventSources( InformerEventSource configMapES = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class) - .setAndFollowControllerNamespaceChanges(context) + .withNamespacesInheritedFromController(context) .build(), context); return EventSourceInitializer.nameEventSources(configMapES); From 3022ac4c320184ac94781720d54394d93fe4b64e Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 3 May 2022 17:05:21 +0200 Subject: [PATCH 16/24] fix: remove incorrect and unneeded configuration --- .../changenamespace/ChangeNamespaceTestCustomResource.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java index 2c39872f93..abf65a2ea2 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java @@ -9,8 +9,6 @@ @Group("sample.javaoperatorsdk") @Version("v1") -@Kind("MaxIntervalTestCustomResource") -@ShortNames("mit") public class ChangeNamespaceTestCustomResource extends CustomResource implements Namespaced { From a38c354eccf46189f18dd9b7ebaac95424d97ec8 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 3 May 2022 17:05:41 +0200 Subject: [PATCH 17/24] docs: clarify --- docs/documentation/features.md | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 483899a050..67b692f25a 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -460,16 +460,19 @@ For more information about MDC see this [link](https://www.baeldung.com/mdc-in-l ## Dynamically Changing Target Namespaces -A controller can be configured to watch a set of namespaces (not only a single namespace or the whole cluster). -The framework supports to dynamically change the list of these namespaces while the operator is running. -When a reconciler is registered the -[`RegisteredController`](https://github.com/java-operator-sdk/java-operator-sdk/blob/ec37025a15046d8f409c77616110024bf32c3416/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java#L5-L5) -is returned, which provides the related methods. These namespaces are meant to be changed when the operator is already -running. - -In a real life scenario usually the list of the target namespaces configured in `ConfigMap` or other input, -this part however is out of the scope of the framework. So in case you want to use a `ConfigMap` and react to change -of it, registering an Informer and calling the `RegisteredController` is up to the developer to implement. +A controller can be configured to watch a specific set of namespaces in addition of the +namespace in which it is currently deployed or the whole cluster. The framework supports +dynamically changing the list of these namespaces while the operator is running. +When a reconciler is registered, an instance of +[`RegisteredController`](https://github.com/java-operator-sdk/java-operator-sdk/blob/ec37025a15046d8f409c77616110024bf32c3416/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java#L5) +is returned, providing access to the methods allowing users to change watched namespaces as the +operator is running. + +A typical scenario would probably involve extracting the list of target namespaces from a +`ConfigMap` or some other input but this part is out of the scope of the framework since this is +use-case specific. For example, reacting to changes to a `ConfigMap` would probably involve +registering an associated `Informer` and then calling the `changeNamespaces` method on +`RegisteredController`. ```java @@ -485,9 +488,10 @@ of it, registering an Informer and calling the `RegisteredController` is up to t ``` -If a target namespaces change for a controller, it might be desirable to change the target namespaces of registered -`InformerEventSource`-s. In order to express this, the InformerEventSource needs to be configured to -`followControllerNamespaceChanges`, this the related method in `InformerConfiguration` should return `true`: +If watched namespaces change for a controller, it might be desirable to propagate these changes to +`InformerEventSources` associated with the controller. In order to express this, +`InformerEventSource` implementations interested in following such changes need to be +configured appropriately so that the `followControllerNamespaceChanges` method returns `true`: ```java @@ -501,7 +505,7 @@ public class MyReconciler InformerEventSource configMapES = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class) - .setAndFollowControllerNamespaceChanges(context) + .withNamespacesInheritedFromController(context) .build(), context); return EventSourceInitializer.nameEventSources(configMapES); From 99aa13b818c6a6d3222f5e3d17788eedd2b8c644 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 3 May 2022 17:11:31 +0200 Subject: [PATCH 18/24] fix: typo --- .../event/source/informer/InformerManager.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 02a0e1d66f..85200c89ee 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -29,7 +29,7 @@ public class InformerManager> implements LifecycleAware, IndexerResourceCache, UpdatableCache { - private static final String ALL_NAMESPACE_MAP_KEY = "allNamespace"; + private static final String ALL_NAMESPACES_MAP_KEY = "allNamespaces"; private static final Logger log = LoggerFactory.getLogger(InformerManager.class); private final Map> sources = new ConcurrentHashMap<>(); @@ -58,7 +58,7 @@ void initSources(MixedOperation, Resource> clien final var filteredBySelectorClient = client.inAnyNamespace().withLabelSelector(labelSelector); final var source = - createEventSource(filteredBySelectorClient, eventHandler, ALL_NAMESPACE_MAP_KEY); + createEventSource(filteredBySelectorClient, eventHandler, ALL_NAMESPACES_MAP_KEY); log.debug("Registered {} -> {} for any namespace", this, source); } else { targetNamespaces.forEach( @@ -130,7 +130,7 @@ public Stream list(Predicate predicate) { @Override public Stream list(String namespace, Predicate predicate) { if (isWatchingAllNamespaces()) { - return getSource(ALL_NAMESPACE_MAP_KEY) + return getSource(ALL_NAMESPACES_MAP_KEY) .map(source -> source.list(namespace, predicate)) .orElse(Stream.empty()); } else { @@ -142,7 +142,7 @@ public Stream list(String namespace, Predicate predicate) { @Override public Optional get(ResourceID resourceID) { - return getSource(resourceID.getNamespace().orElse(ALL_NAMESPACE_MAP_KEY)) + return getSource(resourceID.getNamespace().orElse(ALL_NAMESPACES_MAP_KEY)) .flatMap(source -> source.get(resourceID)) .map(cloner::clone); } @@ -153,24 +153,24 @@ public Stream keys() { } private boolean isWatchingAllNamespaces() { - return sources.containsKey(ALL_NAMESPACE_MAP_KEY); + return sources.containsKey(ALL_NAMESPACES_MAP_KEY); } private Optional> getSource(String namespace) { - namespace = isWatchingAllNamespaces() || namespace == null ? ALL_NAMESPACE_MAP_KEY : namespace; + namespace = isWatchingAllNamespaces() || namespace == null ? ALL_NAMESPACES_MAP_KEY : namespace; return Optional.ofNullable(sources.get(namespace)); } @Override public T remove(ResourceID key) { - return getSource(key.getNamespace().orElse(ALL_NAMESPACE_MAP_KEY)) + return getSource(key.getNamespace().orElse(ALL_NAMESPACES_MAP_KEY)) .map(c -> c.remove(key)) .orElse(null); } @Override public void put(ResourceID key, T resource) { - getSource(key.getNamespace().orElse(ALL_NAMESPACE_MAP_KEY)) + getSource(key.getNamespace().orElse(ALL_NAMESPACES_MAP_KEY)) .ifPresentOrElse(c -> c.put(key, resource), () -> log.warn( "Cannot put resource in the cache. No related cache found: {}. Resource: {}", From 4370755a9d343d34369dbe73b8321fde245929e3 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 3 May 2022 17:21:10 +0200 Subject: [PATCH 19/24] fix: tweaked logging message --- .../processing/event/source/informer/InformerManager.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 85200c89ee..dbed5d5fd9 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -79,7 +79,7 @@ public void changeNamespaces(Set namespaces) { var sourcesToRemove = sources.keySet().stream() .filter(k -> !namespaces.contains(k)).collect(Collectors.toSet()); - log.debug("Stopping to watch namespaces: {} for {}", sourcesToRemove, this); + log.debug("Stopped informer {} for namespaces: {}", this, sourcesToRemove); sourcesToRemove.forEach(k -> sources.remove(k).stop()); namespaces.forEach(ns -> { @@ -90,7 +90,7 @@ public void changeNamespaces(Set namespaces) { eventHandler, ns); source.addIndexers(this.indexers); source.start(); - log.debug("Registered New {} -> {} for namespace: {}", this, source, + log.debug("Registered new {} -> {} for namespace: {}", this, source, ns); } }); From 534274f06b46f7de8b0248b273aeab74c1e8c7ae Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 3 May 2022 18:47:53 +0200 Subject: [PATCH 20/24] fix: format --- .../changenamespace/ChangeNamespaceTestCustomResource.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java index abf65a2ea2..2b3b2295cd 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestCustomResource.java @@ -3,8 +3,6 @@ import io.fabric8.kubernetes.api.model.Namespaced; import io.fabric8.kubernetes.client.CustomResource; import io.fabric8.kubernetes.model.annotation.Group; -import io.fabric8.kubernetes.model.annotation.Kind; -import io.fabric8.kubernetes.model.annotation.ShortNames; import io.fabric8.kubernetes.model.annotation.Version; @Group("sample.javaoperatorsdk") From 9b950375b61f843a2ccf705293266d8b11f56c5e Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 3 May 2022 18:48:35 +0200 Subject: [PATCH 21/24] feat: add NamespaceChangeable interface --- .../operator/RegisteredController.java | 17 ++--------- .../api/config/NamespaceChangeable.java | 26 +++++++++++++++++ .../operator/processing/Controller.java | 5 ---- .../processing/event/EventSourceManager.java | 19 +++++-------- .../processing/event/EventSources.java | 6 ++++ .../source/informer/InformerEventSource.java | 8 ++---- .../informer/ManagedInformerEventSource.java | 9 ++++-- .../event/EventSourceManagerTest.java | 2 ++ .../processing/event/EventSourcesTest.java | 28 +++++++++++++++++++ 9 files changed, 80 insertions(+), 40 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/NamespaceChangeable.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java index 183b2e07d9..f1fa1f37c4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/RegisteredController.java @@ -1,19 +1,6 @@ package io.javaoperatorsdk.operator; -import java.util.Set; - -public interface RegisteredController { - - /** - * If the controller and possibly registered - * {@link io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource} - * watches a set of namespaces this set can be adjusted dynamically, this when the operator is - * running. - * - * @param namespaces target namespaces to watch - */ - void changeNamespaces(Set namespaces); - - void changeNamespaces(String... namespaces); +import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; +public interface RegisteredController extends NamespaceChangeable { } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/NamespaceChangeable.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/NamespaceChangeable.java new file mode 100644 index 0000000000..d03f59a8d0 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/NamespaceChangeable.java @@ -0,0 +1,26 @@ +package io.javaoperatorsdk.operator.api.config; + +import java.util.Set; + +public interface NamespaceChangeable { + + /** + * If the controller and possibly registered + * {@link io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource} + * watches a set of namespaces this set can be adjusted dynamically, this when the operator is + * running. + * + * @param namespaces target namespaces to watch + */ + void changeNamespaces(Set namespaces); + + default void changeNamespaces(String... namespaces) { + changeNamespaces( + namespaces != null ? Set.of(namespaces) : ResourceConfiguration.DEFAULT_NAMESPACES); + } + + default boolean allowsNamespaceChanges() { + return true; + } + +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java index 9174f5b298..7723d19007 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java @@ -333,11 +333,6 @@ public void changeNamespaces(Set namespaces) { eventSourceManager.changeNamespaces(namespaces); } - @Override - public void changeNamespaces(String... namespaces) { - changeNamespaces(Set.of(namespaces)); - } - private void throwMissingCRDException(String crdName, String specVersion, String controllerName) { throw new MissingCRDException( crdName, diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index c9d38b2768..c1fd07438b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -12,6 +12,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.MissingCRDException; import io.javaoperatorsdk.operator.OperatorException; +import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.LifecycleAware; @@ -20,7 +21,6 @@ import io.javaoperatorsdk.operator.processing.event.source.ResourceEventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerResourceEventSource; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceAction; -import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; import io.javaoperatorsdk.operator.processing.event.source.timer.TimerEventSource; public class EventSourceManager implements LifecycleAware { @@ -54,7 +54,7 @@ public EventSourceManager(Controller controller) { * caches propagated - although for non k8s related event sources this behavior might be different * (see * {@link io.javaoperatorsdk.operator.processing.event.source.polling.PerResourcePollingEventSource}). - * + *

* Now the event sources are also started sequentially, mainly because others might depend on * {@link ControllerResourceEventSource} , which is started first. */ @@ -143,18 +143,13 @@ public void broadcastOnResourceEvent(ResourceAction action, R resource, R oldRes } } - @SuppressWarnings({"unchecked", "rawtypes"}) public void changeNamespaces(Set namespaces) { eventProcessor.stop(); - getRegisteredEventSources().forEach(es -> { - if (es instanceof InformerEventSource) { - InformerEventSource ies = (InformerEventSource) es; - if (ies.getConfiguration().followControllerNamespaceChanges()) { - ies.changeNamespaces(namespaces); - } - } - }); - getControllerResourceEventSource().changeNamespaces(namespaces); + eventSources.allEventSources() + .filter(es -> es instanceof NamespaceChangeable) + .map(NamespaceChangeable.class::cast) + .filter(NamespaceChangeable::allowsNamespaceChanges) + .forEach(ies -> ies.changeNamespaces(namespaces)); eventProcessor.start(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java index 1d2d343831..674648bb0f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSources.java @@ -45,6 +45,12 @@ Stream flatMappedSources() { .map(esEntry -> new NamedEventSource(esEntry.getValue(), esEntry.getKey()))); } + Stream allEventSources() { + return Stream.concat( + Stream.of(retryEventSource(), controllerResourceEventSource()).filter(Objects::nonNull), + sources.values().stream().flatMap(c -> c.values().stream())); + } + public void clear() { sources.clear(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index ab26c089c9..47fe4abcf3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -267,11 +267,7 @@ public synchronized void cleanupOnCreateOrUpdateEventFiltering(ResourceID resour } @Override - public void changeNamespaces(Set namespaces) { - if (!configuration.followControllerNamespaceChanges()) { - throw new IllegalStateException( - "InformerEventSource is not configured to change inherit namespaces."); - } - super.changeNamespaces(namespaces); + public boolean allowsNamespaceChanges() { + return getConfiguration().followControllerNamespaceChanges(); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 0e32fc26d5..98e445032c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -16,6 +16,7 @@ import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; import io.fabric8.kubernetes.client.informers.ResourceEventHandler; +import io.javaoperatorsdk.operator.api.config.NamespaceChangeable; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.api.reconciler.dependent.RecentOperationCacheFiller; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -25,7 +26,8 @@ public abstract class ManagedInformerEventSource> extends CachingEventSource - implements ResourceEventHandler, IndexerResourceCache, RecentOperationCacheFiller { + implements ResourceEventHandler, IndexerResourceCache, RecentOperationCacheFiller, + NamespaceChangeable { private static final Logger log = LoggerFactory.getLogger(ManagedInformerEventSource.class); @@ -61,8 +63,11 @@ protected InformerManager manager() { return (InformerManager) cache; } + @Override public void changeNamespaces(Set namespaces) { - manager().changeNamespaces(namespaces); + if (allowsNamespaceChanges()) { + manager().changeNamespaces(namespaces); + } } @Override diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java index a8880a51d5..ecf9fae52d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java @@ -147,6 +147,7 @@ void changesNamespacesOnControllerAndInformerEventSources() { EventSources eventSources = spy(new EventSources()); var controllerResourceEventSourceMock = mock(ControllerResourceEventSource.class); doReturn(controllerResourceEventSourceMock).when(eventSources).controllerResourceEventSource(); + when(controllerResourceEventSourceMock.allowsNamespaceChanges()).thenCallRealMethod(); var manager = new EventSourceManager(controller, eventSources); InformerConfiguration informerConfigurationMock = mock(InformerConfiguration.class); @@ -154,6 +155,7 @@ void changesNamespacesOnControllerAndInformerEventSources() { InformerEventSource informerEventSource = mock(InformerEventSource.class); when(informerEventSource.resourceType()).thenReturn(TestCustomResource.class); when(informerEventSource.getConfiguration()).thenReturn(informerConfigurationMock); + when(informerEventSource.allowsNamespaceChanges()).thenCallRealMethod(); manager.registerEventSource("ies", informerEventSource); manager.changeNamespaces(Set.of(newNamespaces)); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java index bc5e4551c7..3bc223653f 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourcesTest.java @@ -1,12 +1,21 @@ package io.javaoperatorsdk.operator.processing.event; +import java.util.Set; +import java.util.stream.Collectors; + import org.junit.jupiter.api.Test; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.MockKubernetesClient; +import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.processing.Controller; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import static org.junit.jupiter.api.Assertions.*; import static org.mockito.Mockito.mock; +@SuppressWarnings({"unchecked", "rawtypes"}) class EventSourcesTest { EventSources eventSources = new EventSources(); @@ -19,4 +28,23 @@ void cannotAddTwoEventSourcesWithSameName() { }); } + @Test + void allEventSourcesShouldReturnAll() { + // initial state doesn't have ControllerResourceEventSource + assertEquals(Set.of(eventSources.retryEventSource()), eventSources.allEventSources().collect( + Collectors.toSet())); + final var configuration = MockControllerConfiguration.forResource(HasMetadata.class); + final var controller = new Controller(mock(Reconciler.class), configuration, + MockKubernetesClient.client(HasMetadata.class)); + eventSources.initControllerEventSource(controller); + assertEquals( + Set.of(eventSources.retryEventSource(), eventSources.controllerResourceEventSource()), + eventSources.allEventSources().collect(Collectors.toSet())); + final var source = mock(EventSource.class); + eventSources.add("foo", source); + assertEquals(Set.of(eventSources.retryEventSource(), + eventSources.controllerResourceEventSource(), source), + eventSources.allEventSources().collect(Collectors.toSet())); + } + } From 8b4bfa52f176a9d4b2a4ee46640b27198b8d0333 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 4 May 2022 10:10:53 +0200 Subject: [PATCH 22/24] remove check, minor improvement --- .../operator/api/config/informer/InformerConfiguration.java | 2 +- .../processing/event/source/informer/InformerManager.java | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index f5b07470db..bc1f62cafd 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -22,7 +22,7 @@ class DefaultInformerConfiguration extends protected DefaultInformerConfiguration(String labelSelector, Class resourceClass, SecondaryToPrimaryMapper secondaryToPrimaryMapper, - Set namespaces, Boolean followControllerNamespaceChanges) { + Set namespaces, boolean followControllerNamespaceChanges) { super(labelSelector, resourceClass, namespaces); this.followControllerNamespaceChanges = followControllerNamespaceChanges; this.secondaryToPrimaryMapper = diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index dbed5d5fd9..da4631a24f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -73,10 +73,6 @@ void initSources(MixedOperation, Resource> clien } public void changeNamespaces(Set namespaces) { - if (ResourceConfiguration.allNamespacesWatched(namespaces)) { - throw new OperatorException("This feature is only supported for "); - } - var sourcesToRemove = sources.keySet().stream() .filter(k -> !namespaces.contains(k)).collect(Collectors.toSet()); log.debug("Stopped informer {} for namespaces: {}", this, sourcesToRemove); From abc4da246c83d4ceb28501c27f6eb34fac8d58f3 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 4 May 2022 10:19:31 +0200 Subject: [PATCH 23/24] samples update, smell fix --- .../operator/processing/event/EventSourceManager.java | 5 +++-- .../io/javaoperatorsdk/operator/sample/WebappReconciler.java | 1 + .../javaoperatorsdk/operator/sample/WebPageReconciler.java | 4 ++++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index c1fd07438b..3990c8c300 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -145,8 +145,9 @@ public void broadcastOnResourceEvent(ResourceAction action, R resource, R oldRes public void changeNamespaces(Set namespaces) { eventProcessor.stop(); - eventSources.allEventSources() - .filter(es -> es instanceof NamespaceChangeable) + eventSources + .allEventSources() + .filter(NamespaceChangeable.class::isInstance) .map(NamespaceChangeable.class::cast) .filter(NamespaceChangeable::allowsNamespaceChanges) .forEach(ies -> ies.changeNamespaces(namespaces)); diff --git a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/WebappReconciler.java b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/WebappReconciler.java index ffd530713c..d5a5ca62ef 100644 --- a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/WebappReconciler.java +++ b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/WebappReconciler.java @@ -60,6 +60,7 @@ public Map prepareEventSources(EventSourceContext c InformerConfiguration configuration = InformerConfiguration.from(Tomcat.class) + .withNamespacesInheritedFromController(context) .withSecondaryToPrimaryMapper(webappsMatchingTomcatName) .build(); return EventSourceInitializer diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java index 7ff94edc4b..897598c32f 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java @@ -52,18 +52,22 @@ public WebPageReconciler(KubernetesClient kubernetesClient) { public Map prepareEventSources(EventSourceContext context) { var configMapEventSource = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class) + .withNamespacesInheritedFromController(context) .withLabelSelector(LOW_LEVEL_LABEL_KEY) .build(), context); var deploymentEventSource = new InformerEventSource<>(InformerConfiguration.from(Deployment.class) + .withNamespacesInheritedFromController(context) .withLabelSelector(LOW_LEVEL_LABEL_KEY) .build(), context); var serviceEventSource = new InformerEventSource<>(InformerConfiguration.from(Service.class) + .withNamespacesInheritedFromController(context) .withLabelSelector(LOW_LEVEL_LABEL_KEY) .build(), context); var ingressEventSource = new InformerEventSource<>(InformerConfiguration.from(Ingress.class) + .withNamespacesInheritedFromController(context) .withLabelSelector(LOW_LEVEL_LABEL_KEY) .build(), context); return EventSourceInitializer.nameEventSources(configMapEventSource, deploymentEventSource, From 4aed6fe53e35025403f1329f9a8bfbdea92bbf46 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 4 May 2022 15:45:21 +0200 Subject: [PATCH 24/24] added from method --- .../config/informer/InformerConfiguration.java | 15 +++++++++++++++ .../ChangeNamespaceTestReconciler.java | 3 +-- .../operator/sample/WebappReconciler.java | 3 +-- .../operator/sample/WebPageReconciler.java | 12 ++++-------- 4 files changed, 21 insertions(+), 12 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index bc1f62cafd..b3388f969a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -143,4 +143,19 @@ static InformerConfigurationBuilder from( return new InformerConfigurationBuilder<>(resourceClass); } + /** + * Creates a configuration builder that inherits namespaces from the controller and follows + * namespaces changes. + * + * @param resourceClass secondary resource class + * @param eventSourceContext of the initializer + * @return builder + * @param secondary resource type + */ + static InformerConfigurationBuilder from( + Class resourceClass, EventSourceContext eventSourceContext) { + return new InformerConfigurationBuilder<>(resourceClass) + .withNamespacesInheritedFromController(eventSourceContext); + } + } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java index 8b2cf9dc1f..eb8dff9e7f 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/changenamespace/ChangeNamespaceTestReconciler.java @@ -26,8 +26,7 @@ public Map prepareEventSources( EventSourceContext context) { InformerEventSource configMapES = - new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class) - .withNamespacesInheritedFromController(context) + new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context) .build(), context); return EventSourceInitializer.nameEventSources(configMapES); diff --git a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/WebappReconciler.java b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/WebappReconciler.java index d5a5ca62ef..49325def03 100644 --- a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/WebappReconciler.java +++ b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/WebappReconciler.java @@ -59,8 +59,7 @@ public Map prepareEventSources(EventSourceContext c .collect(Collectors.toSet()); InformerConfiguration configuration = - InformerConfiguration.from(Tomcat.class) - .withNamespacesInheritedFromController(context) + InformerConfiguration.from(Tomcat.class, context) .withSecondaryToPrimaryMapper(webappsMatchingTomcatName) .build(); return EventSourceInitializer diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java index 897598c32f..453fd1671f 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconciler.java @@ -51,23 +51,19 @@ public WebPageReconciler(KubernetesClient kubernetesClient) { @Override public Map prepareEventSources(EventSourceContext context) { var configMapEventSource = - new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class) - .withNamespacesInheritedFromController(context) + new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context) .withLabelSelector(LOW_LEVEL_LABEL_KEY) .build(), context); var deploymentEventSource = - new InformerEventSource<>(InformerConfiguration.from(Deployment.class) - .withNamespacesInheritedFromController(context) + new InformerEventSource<>(InformerConfiguration.from(Deployment.class, context) .withLabelSelector(LOW_LEVEL_LABEL_KEY) .build(), context); var serviceEventSource = - new InformerEventSource<>(InformerConfiguration.from(Service.class) - .withNamespacesInheritedFromController(context) + new InformerEventSource<>(InformerConfiguration.from(Service.class, context) .withLabelSelector(LOW_LEVEL_LABEL_KEY) .build(), context); var ingressEventSource = - new InformerEventSource<>(InformerConfiguration.from(Ingress.class) - .withNamespacesInheritedFromController(context) + new InformerEventSource<>(InformerConfiguration.from(Ingress.class, context) .withLabelSelector(LOW_LEVEL_LABEL_KEY) .build(), context); return EventSourceInitializer.nameEventSources(configMapEventSource, deploymentEventSource,