From ce6c12da5bac3162d56196ea0d749625e627d6f1 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 16 Mar 2022 14:49:14 +0100 Subject: [PATCH 01/10] feat: extract DependentResource instantiation to ConfigurationService --- .../api/config/ConfigurationService.java | 13 +++++++++ .../dependent/DependentResourceManager.java | 28 ++++++++----------- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 150af208bd..fe4ab986b0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator.api.config; +import java.lang.reflect.InvocationTargetException; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -7,8 +8,10 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.CustomResource; +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -19,6 +22,7 @@ public interface ConfigurationService { ObjectMapper OBJECT_MAPPER = new ObjectMapper(); Cloner DEFAULT_CLONER = new Cloner() { + @SuppressWarnings("unchecked") @Override public HasMetadata clone(HasMetadata object) { try { @@ -126,4 +130,13 @@ default boolean closeClientOnStop() { default ObjectMapper getObjectMapper() { return OBJECT_MAPPER; } + + default > T createFrom(DependentResourceSpec spec) { + try { + return spec.getDependentResourceClass().getConstructor().newInstance(); + } catch (InstantiationException | NoSuchMethodException | IllegalAccessException + | InvocationTargetException e) { + throw new IllegalStateException(e); + } + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java index 3dd3a2860e..23aa8cd47f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java @@ -1,6 +1,5 @@ package io.javaoperatorsdk.operator.processing.dependent; -import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.List; import java.util.stream.Collectors; @@ -10,6 +9,7 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.Context; @@ -85,25 +85,19 @@ private void initContextIfNeeded(P resource, Context context) { private DependentResource createAndConfigureFrom(DependentResourceSpec dependentResourceSpec, KubernetesClient client) { - try { - DependentResource dependentResource = - (DependentResource) dependentResourceSpec.getDependentResourceClass() - .getConstructor().newInstance(); + DependentResource dependentResource = + ConfigurationServiceProvider.instance().createFrom(dependentResourceSpec); - if (dependentResource instanceof KubernetesClientAware) { - ((KubernetesClientAware) dependentResource).setKubernetesClient(client); - } + if (dependentResource instanceof KubernetesClientAware) { + ((KubernetesClientAware) dependentResource).setKubernetesClient(client); + } - if (dependentResource instanceof DependentResourceConfigurator) { - final var configurator = (DependentResourceConfigurator) dependentResource; - dependentResourceSpec.getDependentResourceConfiguration() - .ifPresent(configurator::configureWith); - } - return dependentResource; - } catch (InstantiationException | NoSuchMethodException | IllegalAccessException - | InvocationTargetException e) { - throw new IllegalStateException(e); + if (dependentResource instanceof DependentResourceConfigurator) { + final var configurator = (DependentResourceConfigurator) dependentResource; + dependentResourceSpec.getDependentResourceConfiguration() + .ifPresent(configurator::configureWith); } + return dependentResource; } public List getDependents() { From ad3e696febba7e782f87e43f406218667b85adef Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 16 Mar 2022 14:50:55 +0100 Subject: [PATCH 02/10] feat: make resourceType part of DependentResource interface --- .../dependent/AbstractDependentResource.java | 9 ++++++++ .../dependent/DependentResource.java | 2 ++ .../AbstractCachingDependentResource.java | 4 +++- .../KubernetesDependentResource.java | 3 ++- .../WebPageReconcilerDependentResources.java | 22 ++++++++++++++----- 5 files changed, 33 insertions(+), 7 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java index 0a750d836c..3d98020727 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java @@ -4,6 +4,7 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.api.config.Utils; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -125,6 +126,7 @@ private RecentOperationCacheFiller eventSourceAsRecentOperationCacheFiller() return (RecentOperationCacheFiller) ((EventSourceProvider

) this).getEventSource(); } + @SuppressWarnings("unchecked") // this cannot be done in constructor since event source might be initialized later protected boolean isFilteringEventSource() { if (this instanceof EventSourceProvider) { @@ -135,6 +137,7 @@ protected boolean isFilteringEventSource() { } } + @SuppressWarnings("unchecked") // this cannot be done in constructor since event source might be initialized later protected boolean isRecentOperationCacheFiller() { if (this instanceof EventSourceProvider) { @@ -171,4 +174,10 @@ protected boolean isUpdatable(P primary, Context

context) { protected boolean isDeletable(P primary, Context

context) { return deletable; } + + @SuppressWarnings("unchecked") + @Override + public Class resourceType() { + return (Class) Utils.getFirstTypeArgumentFromExtendedClass(getClass()); + } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java index 377e4aea22..7f01802975 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java @@ -11,4 +11,6 @@ public interface DependentResource { default void cleanup(P primary, Context

context) {} Optional getResource(P primaryResource); + + Class resourceType(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractCachingDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractCachingDependentResource.java index 86f2c23d54..656ac1aed4 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractCachingDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractCachingDependentResource.java @@ -23,7 +23,9 @@ public EventSource getEventSource() { return eventSource; } - protected Class resourceType() { + @SuppressWarnings("unchecked") + @Override + public Class resourceType() { return (Class) Utils.getFirstTypeArgumentFromExtendedClass(getClass()); } 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 7ceadaf13c..fc1244984a 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 @@ -140,8 +140,9 @@ public KubernetesDependentResource setInformerEventSource( return this; } + @Override @SuppressWarnings("unchecked") - protected Class resourceType() { + public Class resourceType() { return (Class) Utils.getFirstTypeArgumentFromExtendedClass(getClass()); } diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconcilerDependentResources.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconcilerDependentResources.java index d9efec3b19..7eedb2dcfb 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconcilerDependentResources.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconcilerDependentResources.java @@ -1,15 +1,27 @@ package io.javaoperatorsdk.operator.sample; -import java.util.*; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import io.fabric8.kubernetes.api.model.*; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.api.model.ConfigMapVolumeSourceBuilder; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.api.model.Service; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusHandler; +import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CrudKubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; @@ -116,7 +128,7 @@ protected Deployment desired(WebPage webPage, Context context) { } @Override - protected Class resourceType() { + public Class resourceType() { return Deployment.class; } }; @@ -139,7 +151,7 @@ protected Service desired(WebPage webPage, Context context) { } @Override - protected Class resourceType() { + public Class resourceType() { return Service.class; } }; From 179d62792472d7feedd5728e4b1fe7eaeed6ed41 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 16 Mar 2022 22:04:35 +0100 Subject: [PATCH 03/10] chore: test getFirstTypeArgumentFromInterface --- .../operator/api/config/UtilsTest.java | 35 ++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java index 8895922e82..0a24d4f26d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/UtilsTest.java @@ -1,14 +1,21 @@ package io.javaoperatorsdk.operator.api.config; +import java.util.Optional; + import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.apps.Deployment; import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; class UtilsTest { @@ -75,12 +82,32 @@ void getsFirstTypeArgumentFromExtendedClass() { assertThat(res).isEqualTo(Deployment.class); } - public static class TestKubernetesDependentResource - extends KubernetesDependentResource { + @Test + void getsFirstTypeArgumentFromInterface() { + assertThat(Utils.getFirstTypeArgumentFromInterface(TestDependentResource.class)) + .isEqualTo(Deployment.class); + } + + public static class TestDependentResource + implements DependentResource { @Override - protected Deployment desired(TestCustomResource primary, Context context) { + public ReconcileResult reconcile(TestCustomResource primary, + Context context) { return null; } + + @Override + public Optional getResource(TestCustomResource primaryResource) { + return Optional.empty(); + } + } + + public static class TestKubernetesDependentResource + extends KubernetesDependentResource { + + public TestKubernetesDependentResource() { + super(Deployment.class); + } } } From 55a903a665885a916edf3a89bddb9958598a6d31 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 16 Mar 2022 22:10:03 +0100 Subject: [PATCH 04/10] feat: add resource type to DependentResourceSpec --- .../config/AnnotationControllerConfiguration.java | 12 +++++++++--- .../operator/api/config/ConfigurationService.java | 2 +- .../operator/api/config/ControllerConfiguration.java | 2 +- .../api/config/ControllerConfigurationOverrider.java | 8 +++++--- .../api/config/DefaultControllerConfiguration.java | 7 +++---- .../api/config/dependent/DependentResourceSpec.java | 12 ++++++++++-- 6 files changed, 29 insertions(+), 14 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 72d34e16a8..3242f6fd69 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -2,6 +2,7 @@ import java.time.Duration; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -26,7 +27,7 @@ public class AnnotationControllerConfiguration protected final Reconciler reconciler; private final ControllerConfiguration annotation; - private List> specs; + private List> specs; public AnnotationControllerConfiguration(Reconciler reconciler) { this.reconciler = reconciler; @@ -135,7 +136,7 @@ public static T valueOrDefault( @SuppressWarnings({"rawtypes", "unchecked"}) @Override - public List> getDependentResources() { + public List> getDependentResources() { if (specs == null) { final var dependents = valueOrDefault(annotation, ControllerConfiguration::dependents, new Dependent[] {}); @@ -147,6 +148,11 @@ public static T valueOrDefault( specs = new ArrayList<>(dependents.length); for (Dependent dependent : dependents) { final Class dependentType = dependent.type(); + final Class resourceType = + Arrays.asList(dependentType.getInterfaces()).contains(DependentResource.class) + ? Utils.getFirstTypeArgumentFromInterface(dependentType) + : Utils.getFirstTypeArgumentFromExtendedClass(dependentType); + if (KubernetesDependentResource.class.isAssignableFrom(dependentType)) { final var kubeDependent = dependentType.getAnnotation(KubernetesDependent.class); final var namespaces = @@ -164,7 +170,7 @@ public static T valueOrDefault( config = new KubernetesDependentResourceConfig(addOwnerReference, namespaces, labelSelector); } - specs.add(new DependentResourceSpec(dependentType, config)); + specs.add(new DependentResourceSpec(dependentType, resourceType, config)); } } return specs; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index fe4ab986b0..d6e374e1f8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -131,7 +131,7 @@ default ObjectMapper getObjectMapper() { return OBJECT_MAPPER; } - default > T createFrom(DependentResourceSpec spec) { + default > T createFrom(DependentResourceSpec spec) { try { return spec.getDependentResourceClass().getConstructor().newInstance(); } catch (InstantiationException | NoSuchMethodException | IllegalAccessException diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index 03485412b6..39b4eea599 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -51,7 +51,7 @@ default ResourceEventFilter getEventFilter() { return ResourceEventFilters.passthrough(); } - default List> getDependentResources() { + default List> getDependentResources() { return Collections.emptyList(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index 87297b8d49..cfcf6a7787 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -22,7 +22,7 @@ public class ControllerConfigurationOverrider { private ResourceEventFilter customResourcePredicate; private final ControllerConfiguration original; private Duration reconciliationMaxInterval; - private final List> dependentResourceSpecs; + private final List> dependentResourceSpecs; private ControllerConfigurationOverrider(ControllerConfiguration original) { finalizer = original.getFinalizer(); @@ -91,6 +91,7 @@ public ControllerConfigurationOverrider withReconciliationMaxInterval( public void replaceDependentResourceConfig( Class> dependentResourceClass, + Class resourceType, Object dependentResourceConfig) { var currentConfig = @@ -101,7 +102,8 @@ public void replaceDependentResourceConfig( } dependentResourceSpecs.remove(currentConfig.get()); dependentResourceSpecs - .add(new DependentResourceSpec(dependentResourceClass, dependentResourceConfig)); + .add(new DependentResourceSpec(dependentResourceClass, resourceType, + dependentResourceConfig)); } public void addNewDependentResourceConfig(DependentResourceSpec dependentResourceSpec) { @@ -115,7 +117,7 @@ public void addNewDependentResourceConfig(DependentResourceSpec dependentResourc dependentResourceSpecs.add(dependentResourceSpec); } - private Optional> findConfigForDependentResourceClass( + private Optional> findConfigForDependentResourceClass( Class dependentResourceClass) { return dependentResourceSpecs.stream() .filter(dc -> dc.getDependentResourceClass().equals(dependentResourceClass)) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java index 4545c9885e..cbd039f8ee 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java @@ -10,7 +10,6 @@ import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; -@SuppressWarnings("rawtypes") public class DefaultControllerConfiguration extends DefaultResourceConfiguration implements ControllerConfiguration { @@ -22,7 +21,7 @@ public class DefaultControllerConfiguration private final boolean generationAware; private final RetryConfiguration retryConfiguration; private final ResourceEventFilter resourceEventFilter; - private final List> dependents; + private final List> dependents; private final Duration reconciliationMaxInterval; // NOSONAR constructor is meant to provide all information @@ -38,7 +37,7 @@ public DefaultControllerConfiguration( ResourceEventFilter resourceEventFilter, Class resourceClass, Duration reconciliationMaxInterval, - List> dependents) { + List> dependents) { super(labelSelector, resourceClass, namespaces); this.associatedControllerClassName = associatedControllerClassName; this.name = name; @@ -91,7 +90,7 @@ public ResourceEventFilter getEventFilter() { } @Override - public List> getDependentResources() { + public List> getDependentResources() { return dependents; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java index 0684c04bef..f90f85f6b6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java @@ -4,15 +4,19 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -public class DependentResourceSpec, C> { +public class DependentResourceSpec, R, C> { private final Class dependentResourceClass; private final C dependentResourceConfig; - public DependentResourceSpec(Class dependentResourceClass, C dependentResourceConfig) { + private final Class resourceType; + + public DependentResourceSpec(Class dependentResourceClass, Class resourceType, + C dependentResourceConfig) { this.dependentResourceClass = dependentResourceClass; this.dependentResourceConfig = dependentResourceConfig; + this.resourceType = resourceType; } public Class getDependentResourceClass() { @@ -22,4 +26,8 @@ public Class getDependentResourceClass() { public Optional getDependentResourceConfiguration() { return Optional.ofNullable(dependentResourceConfig); } + + public Class getResourceType() { + return resourceType; + } } From 1aa554b705a09585c339172efeb5211163ec3be1 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 16 Mar 2022 22:11:52 +0100 Subject: [PATCH 05/10] feat: add AbstractPollingDependentResource --- .../AbstractPollingDependentResource.java | 27 +++++++++++++++++++ .../PerResourcePollingDependentResource.java | 27 +++++-------------- .../external/PollingDependentResource.java | 20 +++++--------- 3 files changed, 39 insertions(+), 35 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractPollingDependentResource.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractPollingDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractPollingDependentResource.java new file mode 100644 index 0000000000..8c91dea15d --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractPollingDependentResource.java @@ -0,0 +1,27 @@ +package io.javaoperatorsdk.operator.processing.dependent.external; + +import io.fabric8.kubernetes.api.model.HasMetadata; + +public abstract class AbstractPollingDependentResource + extends AbstractCachingDependentResource { + + public static final int DEFAULT_POLLING_PERIOD = 5000; + private long pollingPeriod; + + protected AbstractPollingDependentResource(Class resourceType) { + this(resourceType, DEFAULT_POLLING_PERIOD); + } + + public AbstractPollingDependentResource(Class resourceType, long pollingPeriod) { + super(resourceType); + this.pollingPeriod = pollingPeriod; + } + + public void setPollingPeriod(long pollingPeriod) { + this.pollingPeriod = pollingPeriod; + } + + public long getPollingPeriod() { + return pollingPeriod; + } +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/PerResourcePollingDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/PerResourcePollingDependentResource.java index c5f9e2c888..11b18832a7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/PerResourcePollingDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/PerResourcePollingDependentResource.java @@ -5,36 +5,21 @@ import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.polling.PerResourcePollingEventSource; -import static io.javaoperatorsdk.operator.processing.dependent.external.PollingDependentResource.DEFAULT_POLLING_PERIOD; - public abstract class PerResourcePollingDependentResource - extends AbstractCachingDependentResource + extends AbstractPollingDependentResource implements PerResourcePollingEventSource.ResourceFetcher { - - protected long pollingPeriod; - - public PerResourcePollingDependentResource() { - this(DEFAULT_POLLING_PERIOD); + public PerResourcePollingDependentResource(Class resourceType) { + super(resourceType); } - public PerResourcePollingDependentResource(long pollingPeriod) { - this.pollingPeriod = pollingPeriod; + public PerResourcePollingDependentResource(Class resourceType, long pollingPeriod) { + super(resourceType, pollingPeriod); } @Override public EventSource initEventSource(EventSourceContext

context) { eventSource = new PerResourcePollingEventSource<>(this, context.getPrimaryCache(), - pollingPeriod, resourceType()); + getPollingPeriod(), resourceType()); return eventSource; } - - public PerResourcePollingDependentResource setPollingPeriod(long pollingPeriod) { - this.pollingPeriod = pollingPeriod; - return this; - } - - public long getPollingPeriod() { - return pollingPeriod; - } - } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/PollingDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/PollingDependentResource.java index 18e0c66b60..58723a6f11 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/PollingDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/PollingDependentResource.java @@ -10,27 +10,19 @@ import io.javaoperatorsdk.operator.processing.event.source.polling.PollingEventSource; public abstract class PollingDependentResource - extends AbstractCachingDependentResource implements Supplier> { + extends AbstractPollingDependentResource implements Supplier> { - public static final int DEFAULT_POLLING_PERIOD = 5000; - protected long pollingPeriod; - - public PollingDependentResource() { - this(DEFAULT_POLLING_PERIOD); + public PollingDependentResource(Class resourceType) { + super(resourceType); } - public PollingDependentResource(long pollingPeriod) { - this.pollingPeriod = pollingPeriod; + public PollingDependentResource(Class resourceType, long pollingPeriod) { + super(resourceType, pollingPeriod); } @Override public EventSource initEventSource(EventSourceContext

context) { - eventSource = new PollingEventSource<>(this, pollingPeriod, resourceType()); + eventSource = new PollingEventSource<>(this, getPollingPeriod(), resourceType()); return eventSource; } - - public PollingDependentResource setPollingPeriod(long pollingPeriod) { - this.pollingPeriod = pollingPeriod; - return this; - } } From 110929711339526000f2524fe7aa8c8904146eac Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Wed, 16 Mar 2022 22:15:27 +0100 Subject: [PATCH 06/10] feat: add ResourceTypeAware interface DependentResources that implement this interface will get their resource type "injected" via a requirement to implement a single-arg public constructor. --- .../api/config/ConfigurationService.java | 12 ++++++++++- .../dependent/AbstractDependentResource.java | 7 ------- .../dependent/DependentResource.java | 2 -- .../dependent/ResourceTypeAware.java | 6 ++++++ .../AbstractCachingDependentResource.java | 13 ++++++++---- .../CrudKubernetesDependentResource.java | 8 +++++-- .../KubernetesDependentResource.java | 15 ++++++------- .../GenericKubernetesResourceMatcherTest.java | 2 +- .../sample/readonly/ReadOnlyDependent.java | 4 ++++ .../StandaloneDependentTestReconciler.java | 13 +++++++++++- .../operator/sample/MySQLSchemaOperator.java | 2 ++ .../dependent/SchemaDependentResource.java | 7 ++++++- .../dependent/SecretDependentResource.java | 4 ++++ .../sample/MySQLSchemaOperatorE2E.java | 2 ++ .../sample/DeploymentDependentResource.java | 4 ++++ .../sample/ServiceDependentResource.java | 4 ++++ .../WebPageReconcilerDependentResources.java | 21 +++++++------------ 17 files changed, 86 insertions(+), 40 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ResourceTypeAware.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index d6e374e1f8..3fc8041f95 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -12,6 +12,7 @@ import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.ResourceTypeAware; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -132,10 +133,19 @@ default ObjectMapper getObjectMapper() { } default > T createFrom(DependentResourceSpec spec) { + final var dependentResourceClass = spec.getDependentResourceClass(); + final var resourceTypeAware = ResourceTypeAware.class.isAssignableFrom(dependentResourceClass); try { - return spec.getDependentResourceClass().getConstructor().newInstance(); + return resourceTypeAware + ? dependentResourceClass.getConstructor(Class.class).newInstance(spec.getResourceType()) + : dependentResourceClass.getConstructor().newInstance(); } catch (InstantiationException | NoSuchMethodException | IllegalAccessException | InvocationTargetException e) { + if (resourceTypeAware) { + throw new IllegalStateException(dependentResourceClass.getCanonicalName() + + " must provide a public constructor taking the resource type class as single parameter when implementing " + + ResourceTypeAware.class.getCanonicalName(), e); + } throw new IllegalStateException(e); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java index 3d98020727..799355185c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/AbstractDependentResource.java @@ -4,7 +4,6 @@ import org.slf4j.LoggerFactory; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.config.Utils; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.processing.event.ResourceID; @@ -174,10 +173,4 @@ protected boolean isUpdatable(P primary, Context

context) { protected boolean isDeletable(P primary, Context

context) { return deletable; } - - @SuppressWarnings("unchecked") - @Override - public Class resourceType() { - return (Class) Utils.getFirstTypeArgumentFromExtendedClass(getClass()); - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java index 7f01802975..377e4aea22 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResource.java @@ -11,6 +11,4 @@ public interface DependentResource { default void cleanup(P primary, Context

context) {} Optional getResource(P primaryResource); - - Class resourceType(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ResourceTypeAware.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ResourceTypeAware.java new file mode 100644 index 0000000000..e83d39aebd --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/ResourceTypeAware.java @@ -0,0 +1,6 @@ +package io.javaoperatorsdk.operator.api.reconciler.dependent; + +public interface ResourceTypeAware { + + Class resourceType(); +} diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractCachingDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractCachingDependentResource.java index 656ac1aed4..04b49bf0bf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractCachingDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/external/AbstractCachingDependentResource.java @@ -3,16 +3,22 @@ import java.util.Optional; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.api.config.Utils; import io.javaoperatorsdk.operator.api.reconciler.dependent.AbstractDependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; +import io.javaoperatorsdk.operator.api.reconciler.dependent.ResourceTypeAware; import io.javaoperatorsdk.operator.processing.event.ExternalResourceCachingEventSource; import io.javaoperatorsdk.operator.processing.event.source.EventSource; public abstract class AbstractCachingDependentResource - extends AbstractDependentResource implements EventSourceProvider

{ + extends AbstractDependentResource + implements EventSourceProvider

, ResourceTypeAware { protected ExternalResourceCachingEventSource eventSource; + private final Class resourceType; + + protected AbstractCachingDependentResource(Class resourceType) { + this.resourceType = resourceType; + } public Optional fetchResource(P primaryResource) { return eventSource.getAssociated(primaryResource); @@ -23,10 +29,9 @@ public EventSource getEventSource() { return eventSource; } - @SuppressWarnings("unchecked") @Override public Class resourceType() { - return (Class) Utils.getFirstTypeArgumentFromExtendedClass(getClass()); + return resourceType; } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CrudKubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CrudKubernetesDependentResource.java index 88e9bdc7ae..b363e81b51 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CrudKubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/CrudKubernetesDependentResource.java @@ -12,6 +12,10 @@ * @param

Primary Resource */ public abstract class CrudKubernetesDependentResource - extends - KubernetesDependentResource implements Creator, Updater, Deleter

{ + extends KubernetesDependentResource + implements Creator, Updater, Deleter

{ + + public CrudKubernetesDependentResource(Class resourceType) { + super(resourceType); + } } 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 fc1244984a..9439d74865 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 @@ -11,7 +11,6 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.dsl.NonNamespaceOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.javaoperatorsdk.operator.api.config.Utils; import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; @@ -21,6 +20,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.KubernetesClientAware; import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher; import io.javaoperatorsdk.operator.api.reconciler.dependent.Matcher.Result; +import io.javaoperatorsdk.operator.api.reconciler.dependent.ResourceTypeAware; import io.javaoperatorsdk.operator.api.reconciler.dependent.ResourceUpdatePreProcessor; import io.javaoperatorsdk.operator.processing.event.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.EventSource; @@ -31,7 +31,7 @@ public abstract class KubernetesDependentResource extends AbstractDependentResource - implements KubernetesClientAware, EventSourceProvider

, + implements KubernetesClientAware, EventSourceProvider

, ResourceTypeAware, DependentResourceConfigurator { private static final Logger log = LoggerFactory.getLogger(KubernetesDependentResource.class); @@ -41,15 +41,17 @@ public abstract class KubernetesDependentResource matcher; private final ResourceUpdatePreProcessor processor; + private final Class resourceType; @SuppressWarnings("unchecked") - public KubernetesDependentResource() { + public KubernetesDependentResource(Class resourceType) { + this.resourceType = resourceType; matcher = this instanceof Matcher ? (Matcher) this - : GenericKubernetesResourceMatcher.matcherFor(resourceType(), this); + : GenericKubernetesResourceMatcher.matcherFor(resourceType, this); processor = this instanceof ResourceUpdatePreProcessor ? (ResourceUpdatePreProcessor) this - : GenericResourceUpdatePreProcessor.processorFor(resourceType()); + : GenericResourceUpdatePreProcessor.processorFor(resourceType); } @Override @@ -141,9 +143,8 @@ public KubernetesDependentResource setInformerEventSource( } @Override - @SuppressWarnings("unchecked") public Class resourceType() { - return (Class) Utils.getFirstTypeArgumentFromExtendedClass(getClass()); + return resourceType; } @Override diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java index 1eb7ea1bb4..bad10d551d 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcherTest.java @@ -32,7 +32,7 @@ void checksIfDesiredValuesAreTheSame() { var actual = createDeployment(); final var desired = createDeployment(); final var matcher = GenericKubernetesResourceMatcher.matcherFor(Deployment.class, - new KubernetesDependentResource<>() { + new KubernetesDependentResource<>(Deployment.class) { @Override protected Deployment desired(HasMetadata primary, Context context) { final var currentCase = Optional.ofNullable(primary) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/readonly/ReadOnlyDependent.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/readonly/ReadOnlyDependent.java index 0285d47b83..15fb02a0b6 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/readonly/ReadOnlyDependent.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/readonly/ReadOnlyDependent.java @@ -4,4 +4,8 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; public class ReadOnlyDependent extends KubernetesDependentResource { + + public ReadOnlyDependent() { + super(ConfigMap.class); + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java index 4a236e6e00..e740032d6e 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/standalonedependent/StandaloneDependentTestReconciler.java @@ -7,7 +7,14 @@ import io.fabric8.kubernetes.client.KubernetesClient; import io.fabric8.kubernetes.client.KubernetesClientException; import io.javaoperatorsdk.operator.ReconcilerUtils; -import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.api.reconciler.Context; +import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusHandler; +import io.javaoperatorsdk.operator.api.reconciler.ErrorStatusUpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceContext; +import io.javaoperatorsdk.operator.api.reconciler.EventSourceInitializer; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.api.reconciler.dependent.Creator; import io.javaoperatorsdk.operator.api.reconciler.dependent.Updater; import io.javaoperatorsdk.operator.junit.KubernetesClientAware; @@ -86,6 +93,10 @@ private static class DeploymentDependentResource extends implements Creator, Updater { + public DeploymentDependentResource() { + super(Deployment.class); + } + @Override protected Deployment desired(StandaloneDependentTestCustomResource primary, Context context) { diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java index 0a5703a676..aaab3c2d52 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java @@ -17,6 +17,7 @@ import io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics; import io.javaoperatorsdk.operator.sample.dependent.ResourcePollerConfig; import io.javaoperatorsdk.operator.sample.dependent.SchemaDependentResource; +import io.javaoperatorsdk.operator.sample.schema.Schema; import io.micrometer.core.instrument.logging.LoggingMeterRegistry; public class MySQLSchemaOperator { @@ -37,6 +38,7 @@ public static void main(String[] args) throws IOException { operator.register(schemaReconciler, configOverrider -> configOverrider.replaceDependentResourceConfig( SchemaDependentResource.class, + Schema.class, new ResourcePollerConfig(300, MySQLDbConfig.loadFromEnvironmentVars()))); operator.installShutdownHook(); operator.start(); diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SchemaDependentResource.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SchemaDependentResource.java index f5a5e9ea18..273cd086a1 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SchemaDependentResource.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SchemaDependentResource.java @@ -16,7 +16,8 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceConfigurator; import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; import io.javaoperatorsdk.operator.processing.dependent.external.PerResourcePollingDependentResource; -import io.javaoperatorsdk.operator.sample.*; +import io.javaoperatorsdk.operator.sample.MySQLDbConfig; +import io.javaoperatorsdk.operator.sample.MySQLSchema; import io.javaoperatorsdk.operator.sample.schema.Schema; import io.javaoperatorsdk.operator.sample.schema.SchemaService; @@ -35,6 +36,10 @@ public class SchemaDependentResource private MySQLDbConfig dbConfig; + public SchemaDependentResource() { + super(Schema.class); + } + @Override public void configureWith(ResourcePollerConfig config) { this.dbConfig = config.getMySQLDbConfig(); diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java index 15fff4d65e..60c4c4d2b0 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java @@ -22,6 +22,10 @@ public class SecretDependentResource extends KubernetesDependentResource { c.replaceDependentResourceConfig( SchemaDependentResource.class, + Schema.class, new ResourcePollerConfig( 700, new MySQLDbConfig("127.0.0.1", "3306", "root", "password"))); }) diff --git a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java index b408a07ac9..75632f3e69 100644 --- a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java +++ b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/DeploymentDependentResource.java @@ -14,6 +14,10 @@ public class DeploymentDependentResource extends KubernetesDependentResource implements Creator, Updater { + public DeploymentDependentResource() { + super(Deployment.class); + } + private static String tomcatImage(Tomcat tomcat) { return "tomcat:" + tomcat.getSpec().getVersion(); } diff --git a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java index 3b618906ab..c52553851d 100644 --- a/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java +++ b/sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/ServiceDependentResource.java @@ -12,6 +12,10 @@ public class ServiceDependentResource extends KubernetesDependentResource implements Creator, Updater { + public ServiceDependentResource() { + super(Service.class); + } + @Override protected Service desired(Tomcat tomcat, Context context) { final ObjectMeta tomcatMetadata = tomcat.getMetadata(); diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconcilerDependentResources.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconcilerDependentResources.java index 7eedb2dcfb..90f747d158 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconcilerDependentResources.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageReconcilerDependentResources.java @@ -100,7 +100,7 @@ private void createDependentResources(KubernetesClient client) { .setLabelSelector(DEPENDENT_RESOURCE_LABEL_SELECTOR)); this.deploymentDR = - new CrudKubernetesDependentResource<>() { + new CrudKubernetesDependentResource<>(Deployment.class) { @Override protected Deployment desired(WebPage webPage, Context context) { @@ -126,18 +126,13 @@ protected Deployment desired(WebPage webPage, Context context) { new ConfigMapVolumeSourceBuilder().withName(configMapName(webPage)).build()); return deployment; } - - @Override - public Class resourceType() { - return Deployment.class; - } }; deploymentDR.setKubernetesClient(client); deploymentDR.configureWith(new KubernetesDependentResourceConfig() .setLabelSelector(DEPENDENT_RESOURCE_LABEL_SELECTOR)); this.serviceDR = - new CrudKubernetesDependentResource<>() { + new CrudKubernetesDependentResource<>(Service.class) { @Override protected Service desired(WebPage webPage, Context context) { @@ -149,11 +144,6 @@ protected Service desired(WebPage webPage, Context context) { service.getSpec().setSelector(labels); return service; } - - @Override - public Class resourceType() { - return Service.class; - } }; serviceDR.setKubernetesClient(client); serviceDR.configureWith(new KubernetesDependentResourceConfig() @@ -174,8 +164,11 @@ public static String serviceName(WebPage webPage) { private class ConfigMapDependentResource extends CrudKubernetesDependentResource - implements - PrimaryToSecondaryMapper { + implements PrimaryToSecondaryMapper { + + public ConfigMapDependentResource() { + super(ConfigMap.class); + } @Override protected ConfigMap desired(WebPage webPage, Context context) { From 5ef4529829b078b24c2ce72a0e90dd6c36f32c9c Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 17 Mar 2022 10:50:03 +0100 Subject: [PATCH 07/10] revert: unneeded injection of resource type via constructor --- .../operator/api/config/ConfigurationService.java | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index 3fc8041f95..da26c4783b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -12,7 +12,6 @@ import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -import io.javaoperatorsdk.operator.api.reconciler.dependent.ResourceTypeAware; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -133,20 +132,12 @@ default ObjectMapper getObjectMapper() { } default > T createFrom(DependentResourceSpec spec) { - final var dependentResourceClass = spec.getDependentResourceClass(); - final var resourceTypeAware = ResourceTypeAware.class.isAssignableFrom(dependentResourceClass); try { - return resourceTypeAware - ? dependentResourceClass.getConstructor(Class.class).newInstance(spec.getResourceType()) - : dependentResourceClass.getConstructor().newInstance(); + return spec.getDependentResourceClass().getConstructor().newInstance(); } catch (InstantiationException | NoSuchMethodException | IllegalAccessException | InvocationTargetException e) { - if (resourceTypeAware) { - throw new IllegalStateException(dependentResourceClass.getCanonicalName() - + " must provide a public constructor taking the resource type class as single parameter when implementing " - + ResourceTypeAware.class.getCanonicalName(), e); - } - throw new IllegalStateException(e); + throw new IllegalArgumentException("Cannot instantiate DependentResource " + + spec.getDependentResourceClass().getCanonicalName(), e); } } } From caabe147761af57ca62a190130683dcd2cbd9be7 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 17 Mar 2022 10:51:11 +0100 Subject: [PATCH 08/10] Revert "feat: add resource type to DependentResourceSpec" This reverts commit 55a903a665885a916edf3a89bddb9958598a6d31. --- .../config/AnnotationControllerConfiguration.java | 12 +++--------- .../operator/api/config/ConfigurationService.java | 2 +- .../operator/api/config/ControllerConfiguration.java | 2 +- .../api/config/ControllerConfigurationOverrider.java | 8 +++----- .../api/config/DefaultControllerConfiguration.java | 7 ++++--- .../api/config/dependent/DependentResourceSpec.java | 12 ++---------- .../operator/sample/MySQLSchemaOperator.java | 2 -- .../operator/sample/MySQLSchemaOperatorE2E.java | 2 -- 8 files changed, 14 insertions(+), 33 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java index 3242f6fd69..72d34e16a8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AnnotationControllerConfiguration.java @@ -2,7 +2,6 @@ import java.time.Duration; import java.util.ArrayList; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Optional; @@ -27,7 +26,7 @@ public class AnnotationControllerConfiguration protected final Reconciler reconciler; private final ControllerConfiguration annotation; - private List> specs; + private List> specs; public AnnotationControllerConfiguration(Reconciler reconciler) { this.reconciler = reconciler; @@ -136,7 +135,7 @@ public static T valueOrDefault( @SuppressWarnings({"rawtypes", "unchecked"}) @Override - public List> getDependentResources() { + public List> getDependentResources() { if (specs == null) { final var dependents = valueOrDefault(annotation, ControllerConfiguration::dependents, new Dependent[] {}); @@ -148,11 +147,6 @@ public static T valueOrDefault( specs = new ArrayList<>(dependents.length); for (Dependent dependent : dependents) { final Class dependentType = dependent.type(); - final Class resourceType = - Arrays.asList(dependentType.getInterfaces()).contains(DependentResource.class) - ? Utils.getFirstTypeArgumentFromInterface(dependentType) - : Utils.getFirstTypeArgumentFromExtendedClass(dependentType); - if (KubernetesDependentResource.class.isAssignableFrom(dependentType)) { final var kubeDependent = dependentType.getAnnotation(KubernetesDependent.class); final var namespaces = @@ -170,7 +164,7 @@ public static T valueOrDefault( config = new KubernetesDependentResourceConfig(addOwnerReference, namespaces, labelSelector); } - specs.add(new DependentResourceSpec(dependentType, resourceType, config)); + specs.add(new DependentResourceSpec(dependentType, config)); } } return specs; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index da26c4783b..a4fad2d0e3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -131,7 +131,7 @@ default ObjectMapper getObjectMapper() { return OBJECT_MAPPER; } - default > T createFrom(DependentResourceSpec spec) { + default > T createFrom(DependentResourceSpec spec) { try { return spec.getDependentResourceClass().getConstructor().newInstance(); } catch (InstantiationException | NoSuchMethodException | IllegalAccessException diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java index 39b4eea599..03485412b6 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfiguration.java @@ -51,7 +51,7 @@ default ResourceEventFilter getEventFilter() { return ResourceEventFilters.passthrough(); } - default List> getDependentResources() { + default List> getDependentResources() { return Collections.emptyList(); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java index cfcf6a7787..87297b8d49 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ControllerConfigurationOverrider.java @@ -22,7 +22,7 @@ public class ControllerConfigurationOverrider { private ResourceEventFilter customResourcePredicate; private final ControllerConfiguration original; private Duration reconciliationMaxInterval; - private final List> dependentResourceSpecs; + private final List> dependentResourceSpecs; private ControllerConfigurationOverrider(ControllerConfiguration original) { finalizer = original.getFinalizer(); @@ -91,7 +91,6 @@ public ControllerConfigurationOverrider withReconciliationMaxInterval( public void replaceDependentResourceConfig( Class> dependentResourceClass, - Class resourceType, Object dependentResourceConfig) { var currentConfig = @@ -102,8 +101,7 @@ public void replaceDependentResourceConfig( } dependentResourceSpecs.remove(currentConfig.get()); dependentResourceSpecs - .add(new DependentResourceSpec(dependentResourceClass, resourceType, - dependentResourceConfig)); + .add(new DependentResourceSpec(dependentResourceClass, dependentResourceConfig)); } public void addNewDependentResourceConfig(DependentResourceSpec dependentResourceSpec) { @@ -117,7 +115,7 @@ public void addNewDependentResourceConfig(DependentResourceSpec dependentResourc dependentResourceSpecs.add(dependentResourceSpec); } - private Optional> findConfigForDependentResourceClass( + private Optional> findConfigForDependentResourceClass( Class dependentResourceClass) { return dependentResourceSpecs.stream() .filter(dc -> dc.getDependentResourceClass().equals(dependentResourceClass)) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java index cbd039f8ee..4545c9885e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultControllerConfiguration.java @@ -10,6 +10,7 @@ import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.processing.event.source.controller.ResourceEventFilter; +@SuppressWarnings("rawtypes") public class DefaultControllerConfiguration extends DefaultResourceConfiguration implements ControllerConfiguration { @@ -21,7 +22,7 @@ public class DefaultControllerConfiguration private final boolean generationAware; private final RetryConfiguration retryConfiguration; private final ResourceEventFilter resourceEventFilter; - private final List> dependents; + private final List> dependents; private final Duration reconciliationMaxInterval; // NOSONAR constructor is meant to provide all information @@ -37,7 +38,7 @@ public DefaultControllerConfiguration( ResourceEventFilter resourceEventFilter, Class resourceClass, Duration reconciliationMaxInterval, - List> dependents) { + List> dependents) { super(labelSelector, resourceClass, namespaces); this.associatedControllerClassName = associatedControllerClassName; this.name = name; @@ -90,7 +91,7 @@ public ResourceEventFilter getEventFilter() { } @Override - public List> getDependentResources() { + public List> getDependentResources() { return dependents; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java index f90f85f6b6..0684c04bef 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/dependent/DependentResourceSpec.java @@ -4,19 +4,15 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -public class DependentResourceSpec, R, C> { +public class DependentResourceSpec, C> { private final Class dependentResourceClass; private final C dependentResourceConfig; - private final Class resourceType; - - public DependentResourceSpec(Class dependentResourceClass, Class resourceType, - C dependentResourceConfig) { + public DependentResourceSpec(Class dependentResourceClass, C dependentResourceConfig) { this.dependentResourceClass = dependentResourceClass; this.dependentResourceConfig = dependentResourceConfig; - this.resourceType = resourceType; } public Class getDependentResourceClass() { @@ -26,8 +22,4 @@ public Class getDependentResourceClass() { public Optional getDependentResourceConfiguration() { return Optional.ofNullable(dependentResourceConfig); } - - public Class getResourceType() { - return resourceType; - } } diff --git a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java index aaab3c2d52..0a5703a676 100644 --- a/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java +++ b/sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperator.java @@ -17,7 +17,6 @@ import io.javaoperatorsdk.operator.monitoring.micrometer.MicrometerMetrics; import io.javaoperatorsdk.operator.sample.dependent.ResourcePollerConfig; import io.javaoperatorsdk.operator.sample.dependent.SchemaDependentResource; -import io.javaoperatorsdk.operator.sample.schema.Schema; import io.micrometer.core.instrument.logging.LoggingMeterRegistry; public class MySQLSchemaOperator { @@ -38,7 +37,6 @@ public static void main(String[] args) throws IOException { operator.register(schemaReconciler, configOverrider -> configOverrider.replaceDependentResourceConfig( SchemaDependentResource.class, - Schema.class, new ResourcePollerConfig(300, MySQLDbConfig.loadFromEnvironmentVars()))); operator.installShutdownHook(); operator.start(); diff --git a/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java b/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java index e690dc772d..b8e8b0566a 100644 --- a/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java +++ b/sample-operators/mysql-schema/src/test/java/io/javaoperatorsdk/operator/sample/MySQLSchemaOperatorE2E.java @@ -22,7 +22,6 @@ import io.javaoperatorsdk.operator.junit.OperatorExtension; import io.javaoperatorsdk.operator.sample.dependent.ResourcePollerConfig; import io.javaoperatorsdk.operator.sample.dependent.SchemaDependentResource; -import io.javaoperatorsdk.operator.sample.schema.Schema; import static java.util.concurrent.TimeUnit.MINUTES; import static org.awaitility.Awaitility.await; @@ -69,7 +68,6 @@ boolean isLocal() { c -> { c.replaceDependentResourceConfig( SchemaDependentResource.class, - Schema.class, new ResourcePollerConfig( 700, new MySQLDbConfig("127.0.0.1", "3306", "root", "password"))); }) From 0b80e088e47358b0993be79b84913107e09462cd Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 18 Mar 2022 12:59:36 +0100 Subject: [PATCH 09/10] refactor: create DependentResources in Controller --- .../operator/processing/Controller.java | 58 ++++++++-- .../dependent/DependentResourceManager.java | 106 ------------------ 2 files changed, 49 insertions(+), 115 deletions(-) delete mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java 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 5fc1b9ede7..fbf6d4d64b 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 @@ -2,6 +2,7 @@ import java.util.LinkedList; import java.util.List; +import java.util.stream.Collectors; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -18,9 +19,11 @@ import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +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.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; @@ -28,11 +31,13 @@ import io.javaoperatorsdk.operator.api.reconciler.Reconciler; import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; -import io.javaoperatorsdk.operator.processing.dependent.DependentResourceManager; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceConfigurator; +import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; +import io.javaoperatorsdk.operator.api.reconciler.dependent.KubernetesClientAware; import io.javaoperatorsdk.operator.processing.event.EventSourceManager; import io.javaoperatorsdk.operator.processing.event.source.EventSource; -@SuppressWarnings({"unchecked"}) +@SuppressWarnings({"unchecked", "rawtypes"}) @Ignore public class Controller

implements Reconciler

, LifecycleAware, EventSourceInitializer

{ @@ -43,7 +48,8 @@ public class Controller

implements Reconciler

, private final ControllerConfiguration

configuration; private final KubernetesClient kubernetesClient; private final EventSourceManager

eventSourceManager; - private final DependentResourceManager

dependents; + private final List dependents; + private final boolean contextInitializer; public Controller(Reconciler

reconciler, ControllerConfiguration

configuration, @@ -51,14 +57,43 @@ public Controller(Reconciler

reconciler, this.reconciler = reconciler; this.configuration = configuration; this.kubernetesClient = kubernetesClient; + contextInitializer = reconciler instanceof ContextInitializer; eventSourceManager = new EventSourceManager<>(this); - dependents = new DependentResourceManager<>(this); + + dependents = configuration.getDependentResources().stream() + .map(drs -> createAndConfigureFrom(drs, kubernetesClient)) + .collect(Collectors.toList()); + } + + @SuppressWarnings("rawtypes") + private DependentResource createAndConfigureFrom(DependentResourceSpec dependentResourceSpec, + KubernetesClient client) { + DependentResource dependentResource = + ConfigurationServiceProvider.instance().createFrom(dependentResourceSpec); + + if (dependentResource instanceof KubernetesClientAware) { + ((KubernetesClientAware) dependentResource).setKubernetesClient(client); + } + + if (dependentResource instanceof DependentResourceConfigurator) { + final var configurator = (DependentResourceConfigurator) dependentResource; + dependentResourceSpec.getDependentResourceConfiguration() + .ifPresent(configurator::configureWith); + } + return dependentResource; + } + + private void initContextIfNeeded(P resource, Context

context) { + if (contextInitializer) { + ((ContextInitializer

) reconciler).initContext(resource, context); + } } @Override public DeleteControl cleanup(P resource, Context

context) { - dependents.cleanup(resource, context); + initContextIfNeeded(resource, context); + dependents.forEach(dependent -> dependent.cleanup(resource, context)); try { return metrics().timeControllerExecution( @@ -90,7 +125,8 @@ public DeleteControl execute() { @Override public UpdateControl

reconcile(P resource, Context

context) throws Exception { - dependents.reconcile(resource, context); + initContextIfNeeded(resource, context); + dependents.forEach(dependent -> dependent.reconcile(resource, context)); return metrics().timeControllerExecution( new ControllerExecution<>() { @@ -131,8 +167,12 @@ private Metrics metrics() { @Override public List prepareEventSources(EventSourceContext

context) { - final var dependentSources = dependents.prepareEventSources(context); - List sources = new LinkedList<>(dependentSources); + List sources = new LinkedList<>(); + dependents.stream() + .filter(dependentResource -> dependentResource instanceof EventSourceProvider) + .map(EventSourceProvider.class::cast) + .map(provider -> provider.initEventSource(context)) + .forEach(sources::add); // add manually defined event sources if (reconciler instanceof EventSourceInitializer) { @@ -267,6 +307,6 @@ public void stop() { @SuppressWarnings("rawtypes") public List getDependents() { - return dependents.getDependents(); + return dependents; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java deleted file mode 100644 index 23aa8cd47f..0000000000 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/DependentResourceManager.java +++ /dev/null @@ -1,106 +0,0 @@ -package io.javaoperatorsdk.operator.processing.dependent; - -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Collectors; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.KubernetesClient; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; -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.dependent.DependentResource; -import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceConfigurator; -import io.javaoperatorsdk.operator.api.reconciler.dependent.EventSourceProvider; -import io.javaoperatorsdk.operator.api.reconciler.dependent.KubernetesClientAware; -import io.javaoperatorsdk.operator.processing.Controller; -import io.javaoperatorsdk.operator.processing.event.source.EventSource; - -@SuppressWarnings({"rawtypes", "unchecked"}) -@Ignore -public class DependentResourceManager

- implements EventSourceInitializer

, Reconciler

{ - - private static final Logger log = LoggerFactory.getLogger(DependentResourceManager.class); - - private final Reconciler

reconciler; - private final ControllerConfiguration

controllerConfiguration; - private List dependents; - - public DependentResourceManager(Controller

controller) { - this.reconciler = controller.getReconciler(); - this.controllerConfiguration = controller.getConfiguration(); - } - - @Override - public List prepareEventSources(EventSourceContext

context) { - final var dependentResources = controllerConfiguration.getDependentResources(); - final var sources = new ArrayList(dependentResources.size()); - dependents = - dependentResources.stream() - .map( - drc -> { - final var dependentResource = createAndConfigureFrom(drc, context.getClient()); - if (dependentResource instanceof EventSourceProvider) { - EventSourceProvider provider = (EventSourceProvider) dependentResource; - sources.add(provider.initEventSource(context)); - } - return dependentResource; - }) - .collect(Collectors.toList()); - return sources; - } - - @Override - public UpdateControl

reconcile(P resource, Context

context) { - initContextIfNeeded(resource, context); - dependents.forEach(dependent -> dependent.reconcile(resource, context)); - return UpdateControl.noUpdate(); - } - - @Override - public DeleteControl cleanup(P resource, Context

context) { - initContextIfNeeded(resource, context); - dependents.forEach(dependent -> dependent.cleanup(resource, context)); - return Reconciler.super.cleanup(resource, context); - } - - private void initContextIfNeeded(P resource, Context context) { - if (reconciler instanceof ContextInitializer) { - final var initializer = (ContextInitializer

) reconciler; - initializer.initContext(resource, context); - } - } - - private DependentResource createAndConfigureFrom(DependentResourceSpec dependentResourceSpec, - KubernetesClient client) { - DependentResource dependentResource = - ConfigurationServiceProvider.instance().createFrom(dependentResourceSpec); - - if (dependentResource instanceof KubernetesClientAware) { - ((KubernetesClientAware) dependentResource).setKubernetesClient(client); - } - - if (dependentResource instanceof DependentResourceConfigurator) { - final var configurator = (DependentResourceConfigurator) dependentResource; - dependentResourceSpec.getDependentResourceConfiguration() - .ifPresent(configurator::configureWith); - } - return dependentResource; - } - - public List getDependents() { - return dependents; - } -} From 36138c7b72066b501bbe8a8ab9c3d06fc4d21454 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Fri, 18 Mar 2022 13:05:33 +0100 Subject: [PATCH 10/10] refactor: introduce DependentResourceFactory --- .../api/config/ConfigurationService.java | 14 +++----------- .../dependent/DependentResourceFactory.java | 19 +++++++++++++++++++ .../operator/processing/Controller.java | 9 ++++----- 3 files changed, 26 insertions(+), 16 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResourceFactory.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java index a4fad2d0e3..f13ae118f8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java @@ -1,6 +1,5 @@ package io.javaoperatorsdk.operator.api.config; -import java.lang.reflect.InvocationTargetException; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -8,10 +7,9 @@ import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.Config; import io.fabric8.kubernetes.client.CustomResource; -import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.monitoring.Metrics; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; -import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; +import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.ObjectMapper; @@ -131,13 +129,7 @@ default ObjectMapper getObjectMapper() { return OBJECT_MAPPER; } - default > T createFrom(DependentResourceSpec spec) { - try { - return spec.getDependentResourceClass().getConstructor().newInstance(); - } catch (InstantiationException | NoSuchMethodException | IllegalAccessException - | InvocationTargetException e) { - throw new IllegalArgumentException("Cannot instantiate DependentResource " - + spec.getDependentResourceClass().getCanonicalName(), e); - } + default DependentResourceFactory dependentResourceFactory() { + return new DependentResourceFactory() {}; } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResourceFactory.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResourceFactory.java new file mode 100644 index 0000000000..ffd0ba496f --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/DependentResourceFactory.java @@ -0,0 +1,19 @@ +package io.javaoperatorsdk.operator.api.reconciler.dependent; + +import java.lang.reflect.InvocationTargetException; + +import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; + +public interface DependentResourceFactory { + + default > T createFrom(DependentResourceSpec spec) { + try { + return spec.getDependentResourceClass().getConstructor().newInstance(); + } catch (InstantiationException | NoSuchMethodException | IllegalAccessException + | InvocationTargetException e) { + throw new IllegalArgumentException("Cannot instantiate DependentResource " + + spec.getDependentResourceClass().getCanonicalName(), e); + } + } + +} 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 fbf6d4d64b..de71d54171 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 @@ -67,10 +67,10 @@ public Controller(Reconciler

reconciler, } @SuppressWarnings("rawtypes") - private DependentResource createAndConfigureFrom(DependentResourceSpec dependentResourceSpec, + private DependentResource createAndConfigureFrom(DependentResourceSpec spec, KubernetesClient client) { - DependentResource dependentResource = - ConfigurationServiceProvider.instance().createFrom(dependentResourceSpec); + final var dependentResource = + ConfigurationServiceProvider.instance().dependentResourceFactory().createFrom(spec); if (dependentResource instanceof KubernetesClientAware) { ((KubernetesClientAware) dependentResource).setKubernetesClient(client); @@ -78,8 +78,7 @@ private DependentResource createAndConfigureFrom(DependentResourceSpec dependent if (dependentResource instanceof DependentResourceConfigurator) { final var configurator = (DependentResourceConfigurator) dependentResource; - dependentResourceSpec.getDependentResourceConfiguration() - .ifPresent(configurator::configureWith); + spec.getDependentResourceConfiguration().ifPresent(configurator::configureWith); } return dependentResource; }