From 1849f31c7ca5b1b183c1a3b07c9814ba5cb3da6f Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 24 Sep 2024 13:57:29 +0200 Subject: [PATCH 1/2] refactor: make shouldUseSSA work with types instead of instances + tests Signed-off-by: Chris Laprun --- .../api/config/ConfigurationService.java | 51 ++++--- .../KubernetesDependentResource.java | 3 +- .../config/BaseConfigurationServiceTest.java | 125 ++++++++++++++---- 3 files changed, 135 insertions(+), 44 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 a5c0cdc265..e5db6c5b60 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 @@ -24,7 +24,6 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; -import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceUpdaterMatcher; import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowFactory; @@ -362,29 +361,45 @@ default boolean ssaBasedCreateUpdateMatchForDependentResources() { } /** - * This is mostly useful as an integration point for downstream projects to be able to reuse the - * logic used to determine whether a given {@link KubernetesDependentResource} should use SSA or - * not. - * - * @param dependentResource the {@link KubernetesDependentResource} under consideration - * @return {@code true} if SSA should be used for - * @param the dependent resource type - * @param

the primary resource type - * @since 4.9.4 + * @deprecated Use {@link #shouldUseSSA(Class, Class)} instead */ + @Deprecated(forRemoval = true) default boolean shouldUseSSA( KubernetesDependentResource dependentResource) { - if (dependentResource instanceof ResourceUpdaterMatcher) { + return shouldUseSSA(dependentResource.getClass(), dependentResource.resourceType()); + } + + /** + * This is mostly useful as an integration point for downstream projects to be able to reuse the + * logic used to determine whether a given {@link KubernetesDependentResource} type should use SSA + * or not. + * + * @param dependentResourceType the {@link KubernetesDependentResource} type under consideration + * @param resourceType the resource type associated with the considered dependent resource type + * @return {@code true} if SSA should be used for specified dependent resource type, {@code false} + * otherwise + * @since 4.9.5 + */ + @SuppressWarnings("rawtypes") + default boolean shouldUseSSA(Class dependentResourceType, + Class resourceType) { + if (ResourceUpdaterMatcher.class.isAssignableFrom(dependentResourceType)) { return false; } - Optional useSSAConfig = dependentResource.configuration() - .flatMap(KubernetesDependentResourceConfig::useSSA); - // don't use SSA for certain resources by default, only if explicitly overriden - if (useSSAConfig.isEmpty() - && defaultNonSSAResources().contains(dependentResource.resourceType())) { - return false; + Boolean useSSAConfig = + Optional.ofNullable(dependentResourceType.getAnnotation(KubernetesDependent.class)) + .map(kd -> kd.useSSA().asBoolean()) + .orElse(null); + // don't use SSA for certain resources by default, only if explicitly overridden + if (useSSAConfig == null) { + if (defaultNonSSAResources().contains(resourceType)) { + return false; + } else { + return ssaBasedCreateUpdateMatchForDependentResources(); + } + } else { + return useSSAConfig; } - return useSSAConfig.orElse(ssaBasedCreateUpdateMatchForDependentResources()); } /** 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 264337c7d9..fad99d2489 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 @@ -193,7 +193,8 @@ protected void addMetadata(boolean forMatch, R actualResource, final R target, P protected boolean useSSA(Context

context) { if (useSSA == null) { - useSSA = context.getControllerConfiguration().getConfigurationService().shouldUseSSA(this); + useSSA = context.getControllerConfiguration().getConfigurationService() + .shouldUseSSA(getClass(), resourceType()); } return useSSA; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java index e107623347..91519a9c13 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java @@ -15,6 +15,7 @@ import io.fabric8.kubernetes.api.model.ConfigMap; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.Service; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.config.AnnotationConfigurable; import io.javaoperatorsdk.operator.api.config.BaseConfigurationService; @@ -31,6 +32,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResource; import io.javaoperatorsdk.operator.api.reconciler.dependent.ReconcileResult; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.BooleanWithUndefined; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; @@ -40,6 +42,7 @@ import io.javaoperatorsdk.operator.processing.retry.GradualRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import io.javaoperatorsdk.operator.processing.retry.RetryExecution; +import io.javaoperatorsdk.operator.sample.readonly.ConfigMapReader; import io.javaoperatorsdk.operator.sample.readonly.ReadOnlyDependent; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -106,7 +109,7 @@ void getDependentResources() { var maybeConfig = DependentResourceConfigurationResolver.configurationFor(dependentSpec, configuration); assertNotNull(maybeConfig); - assertTrue(maybeConfig instanceof KubernetesDependentResourceConfig); + assertInstanceOf(KubernetesDependentResourceConfig.class, maybeConfig); final var config = (KubernetesDependentResourceConfig) maybeConfig; // check that the DependentResource inherits the controller's configuration if applicable assertEquals(1, config.namespaces().size()); @@ -121,7 +124,7 @@ void getDependentResources() { maybeConfig = DependentResourceConfigurationResolver.configurationFor(dependentSpec, configuration); assertNotNull(maybeConfig); - assertTrue(maybeConfig instanceof KubernetesDependentResourceConfig); + assertInstanceOf(KubernetesDependentResourceConfig.class, maybeConfig); } @Test @@ -234,6 +237,50 @@ void configuringFromCustomAnnotationsShouldWork() { assertEquals(CustomConfigConverter.CONVERTER_PROVIDED_DEFAULT, getValue(config, 1)); } + @Test + void excludedResourceClassesShouldNotUseSSAByDefault() { + final var config = configFor(new SelectorReconciler()); + + // ReadOnlyDependent targets ConfigMap which is configured to not use SSA by default + final var kubernetesDependentResourceConfig = + extractDependentKubernetesResourceConfig(config, 1); + assertNotNull(kubernetesDependentResourceConfig); + assertTrue(kubernetesDependentResourceConfig.useSSA().isEmpty()); + assertFalse(configurationService.shouldUseSSA(ReadOnlyDependent.class, ConfigMap.class)); + } + + @Test + void excludedResourceClassesShouldUseSSAIfAnnotatedToDoSo() { + final var config = configFor(new SelectorReconciler()); + + // WithAnnotation dependent also targets ConfigMap but overrides the default with the annotation + final var kubernetesDependentResourceConfig = + extractDependentKubernetesResourceConfig(config, 0); + assertNotNull(kubernetesDependentResourceConfig); + assertFalse(kubernetesDependentResourceConfig.useSSA().isEmpty()); + assertTrue((Boolean) kubernetesDependentResourceConfig.useSSA().get()); + assertTrue(configurationService.shouldUseSSA(SelectorReconciler.WithAnnotation.class, + ConfigMap.class)); + } + + @Test + void dependentsShouldUseSSAByDefaultIfNotExcluded() { + final var config = configFor(new DefaultSSAForDependentsReconciler()); + + var kubernetesDependentResourceConfig = extractDependentKubernetesResourceConfig(config, 0); + assertNotNull(kubernetesDependentResourceConfig); + assertTrue(kubernetesDependentResourceConfig.useSSA().isEmpty()); + assertTrue(configurationService.shouldUseSSA( + DefaultSSAForDependentsReconciler.DefaultDependent.class, ConfigMapReader.class)); + + kubernetesDependentResourceConfig = extractDependentKubernetesResourceConfig(config, 1); + assertNotNull(kubernetesDependentResourceConfig); + assertTrue(kubernetesDependentResourceConfig.useSSA().isPresent()); + assertFalse((Boolean) kubernetesDependentResourceConfig.useSSA().get()); + assertFalse(configurationService + .shouldUseSSA(DefaultSSAForDependentsReconciler.NonSSADependent.class, Service.class)); + } + private static int getValue( io.javaoperatorsdk.operator.api.config.ControllerConfiguration configuration, int index) { return ((CustomConfig) DependentResourceConfigurationResolver @@ -247,32 +294,33 @@ private static int getValue( private static class MaxIntervalReconciler implements Reconciler { @Override - public UpdateControl reconcile(ConfigMap resource, Context context) - throws Exception { + public UpdateControl reconcile(ConfigMap resource, Context context) { return null; } } @ControllerConfiguration(namespaces = OneDepReconciler.CONFIGURED_NS, dependents = @Dependent(type = ReadOnlyDependent.class)) - private static class OneDepReconciler implements Reconciler { + private static class OneDepReconciler implements Reconciler { private static final String CONFIGURED_NS = "foo"; @Override - public UpdateControl reconcile(ConfigMap resource, Context context) { + public UpdateControl reconcile(ConfigMapReader resource, + Context context) { return null; } } @ControllerConfiguration( dependents = @Dependent(type = ReadOnlyDependent.class, name = NamedDepReconciler.NAME)) - private static class NamedDepReconciler implements Reconciler { + private static class NamedDepReconciler implements Reconciler { private static final String NAME = "foo"; @Override - public UpdateControl reconcile(ConfigMap resource, Context context) { + public UpdateControl reconcile(ConfigMapReader resource, + Context context) { return null; } } @@ -282,10 +330,11 @@ public UpdateControl reconcile(ConfigMap resource, Context @Dependent(type = ReadOnlyDependent.class), @Dependent(type = ReadOnlyDependent.class) }) - private static class DuplicatedDepReconciler implements Reconciler { + private static class DuplicatedDepReconciler implements Reconciler { @Override - public UpdateControl reconcile(ConfigMap resource, Context context) { + public UpdateControl reconcile(ConfigMapReader resource, + Context context) { return null; } } @@ -295,21 +344,23 @@ public UpdateControl reconcile(ConfigMap resource, Context @Dependent(type = ReadOnlyDependent.class, name = NamedDuplicatedDepReconciler.NAME), @Dependent(type = ReadOnlyDependent.class) }) - private static class NamedDuplicatedDepReconciler implements Reconciler { + private static class NamedDuplicatedDepReconciler implements Reconciler { private static final String NAME = "duplicated"; @Override - public UpdateControl reconcile(ConfigMap resource, Context context) { + public UpdateControl reconcile(ConfigMapReader resource, + Context context) { return null; } } @ControllerConfiguration - private static class NoDepReconciler implements Reconciler { + private static class NoDepReconciler implements Reconciler { @Override - public UpdateControl reconcile(ConfigMap resource, Context context) { + public UpdateControl reconcile(ConfigMapReader resource, + Context context) { return null; } } @@ -318,16 +369,17 @@ public UpdateControl reconcile(ConfigMap resource, Context @Dependent(type = SelectorReconciler.WithAnnotation.class), @Dependent(type = ReadOnlyDependent.class) }) - private static class SelectorReconciler implements Reconciler { + private static class SelectorReconciler implements Reconciler { @Override - public UpdateControl reconcile(ConfigMap resource, Context context) - throws Exception { + public UpdateControl reconcile(ConfigMapReader resource, + Context context) { return null; } - @KubernetesDependent - private static class WithAnnotation extends KubernetesDependentResource { + @KubernetesDependent(useSSA = BooleanWithUndefined.TRUE) + private static class WithAnnotation + extends KubernetesDependentResource { public WithAnnotation() { super(ConfigMap.class); @@ -343,6 +395,32 @@ public UpdateControl reconcile(ConfigMap resource, Context } } + @ControllerConfiguration(dependents = { + @Dependent(type = DefaultSSAForDependentsReconciler.DefaultDependent.class), + @Dependent(type = DefaultSSAForDependentsReconciler.NonSSADependent.class) + }) + private static class DefaultSSAForDependentsReconciler implements Reconciler { + + @Override + public UpdateControl reconcile(ConfigMap resource, Context context) { + return null; + } + + private static class DefaultDependent + extends KubernetesDependentResource { + public DefaultDependent() { + super(ConfigMapReader.class); + } + } + + @KubernetesDependent(useSSA = BooleanWithUndefined.FALSE) + private static class NonSSADependent extends KubernetesDependentResource { + public NonSSADependent() { + super(Service.class); + } + } + } + public static class TestRetry implements Retry, AnnotationConfigurable { private int value; @@ -377,8 +455,7 @@ public void initFrom(TestRetryConfiguration configuration) { private static class ConfigurableRateLimitAndRetryReconciler implements Reconciler { @Override - public UpdateControl reconcile(ConfigMap resource, Context context) - throws Exception { + public UpdateControl reconcile(ConfigMap resource, Context context) { return UpdateControl.noUpdate(); } } @@ -397,8 +474,7 @@ private static class CheckRetryingGraduallyConfiguration implements Reconciler reconcile(ConfigMap resource, Context context) - throws Exception { + public UpdateControl reconcile(ConfigMap resource, Context context) { return UpdateControl.noUpdate(); } } @@ -445,8 +521,7 @@ public UpdateControl reconcile(ConfigMap resource, Context private static class CustomAnnotationReconciler implements Reconciler { @Override - public UpdateControl reconcile(ConfigMap resource, Context context) - throws Exception { + public UpdateControl reconcile(ConfigMap resource, Context context) { return null; } } From ef5dc6e38819d3b99fe32aece30644520190598d Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Tue, 24 Sep 2024 16:30:30 +0200 Subject: [PATCH 2/2] fix: shouldUseSSA should also work with unmanaged configuration Signed-off-by: Chris Laprun --- .../api/config/ConfigurationService.java | 25 ++++++++++++------ .../KubernetesDependentResource.java | 2 +- .../operator/DependentSSAMatchingIT.java | 6 ++--- .../operator/DependentSSAMigrationIT.java | 20 +++++++------- .../config/BaseConfigurationServiceTest.java | 26 ++++++++++++++++--- ...e.java => DependentSSACustomResource.java} | 2 +- .../dependentssa/DependentSSAReconciler.java | 24 ++++++++++++----- .../dependentssa/SSAConfigMapDependent.java | 10 +++---- 8 files changed, 76 insertions(+), 39 deletions(-) rename operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/{DependnetSSACustomResource.java => DependentSSACustomResource.java} (92%) 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 e5db6c5b60..111ef03b1b 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 @@ -24,6 +24,7 @@ import io.javaoperatorsdk.operator.api.reconciler.dependent.DependentResourceFactory; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependent; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResource; +import io.javaoperatorsdk.operator.processing.dependent.kubernetes.KubernetesDependentResourceConfig; import io.javaoperatorsdk.operator.processing.dependent.kubernetes.ResourceUpdaterMatcher; import io.javaoperatorsdk.operator.processing.dependent.workflow.ManagedWorkflowFactory; @@ -361,12 +362,20 @@ default boolean ssaBasedCreateUpdateMatchForDependentResources() { } /** - * @deprecated Use {@link #shouldUseSSA(Class, Class)} instead + * This is mostly useful as an integration point for downstream projects to be able to reuse the + * logic used to determine whether a given {@link KubernetesDependentResource} should use SSA or + * not. + * + * @param dependentResource the {@link KubernetesDependentResource} under consideration + * @param the dependent resource type + * @param

the primary resource type + * @return {@code true} if SSA should be used for + * @since 4.9.4 */ - @Deprecated(forRemoval = true) default boolean shouldUseSSA( KubernetesDependentResource dependentResource) { - return shouldUseSSA(dependentResource.getClass(), dependentResource.resourceType()); + return shouldUseSSA(dependentResource.getClass(), dependentResource.resourceType(), + dependentResource.configuration().orElse(null)); } /** @@ -382,14 +391,14 @@ default boolean shouldUseSSA( */ @SuppressWarnings("rawtypes") default boolean shouldUseSSA(Class dependentResourceType, - Class resourceType) { + Class resourceType, + KubernetesDependentResourceConfig config) { if (ResourceUpdaterMatcher.class.isAssignableFrom(dependentResourceType)) { return false; } - Boolean useSSAConfig = - Optional.ofNullable(dependentResourceType.getAnnotation(KubernetesDependent.class)) - .map(kd -> kd.useSSA().asBoolean()) - .orElse(null); + Boolean useSSAConfig = Optional.ofNullable(config) + .flatMap(KubernetesDependentResourceConfig::useSSA) + .orElse(null); // don't use SSA for certain resources by default, only if explicitly overridden if (useSSAConfig == null) { if (defaultNonSSAResources().contains(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 fad99d2489..e7c7dbc0a7 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 @@ -194,7 +194,7 @@ protected void addMetadata(boolean forMatch, R actualResource, final R target, P protected boolean useSSA(Context

context) { if (useSSA == null) { useSSA = context.getControllerConfiguration().getConfigurationService() - .shouldUseSSA(getClass(), resourceType()); + .shouldUseSSA(getClass(), resourceType(), configuration().orElse(null)); } return useSSA; } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMatchingIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMatchingIT.java index 0249b44927..fe6dca887e 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMatchingIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMatchingIT.java @@ -12,9 +12,9 @@ import io.fabric8.kubernetes.client.dsl.base.PatchContext; import io.fabric8.kubernetes.client.dsl.base.PatchType; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSACustomResource; import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSAReconciler; import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSASpec; -import io.javaoperatorsdk.operator.sample.dependentssa.DependnetSSACustomResource; import io.javaoperatorsdk.operator.sample.dependentssa.SSAConfigMapDependent; import static org.assertj.core.api.Assertions.assertThat; @@ -86,8 +86,8 @@ void testMatchingAndUpdate() { }); } - public DependnetSSACustomResource testResource() { - DependnetSSACustomResource resource = new DependnetSSACustomResource(); + public DependentSSACustomResource testResource() { + DependentSSACustomResource resource = new DependentSSACustomResource(); resource.setMetadata(new ObjectMetaBuilder() .withName(TEST_RESOURCE_NAME) .build()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java index c4217bff25..06a32f9b23 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/DependentSSAMigrationIT.java @@ -12,9 +12,9 @@ import io.fabric8.kubernetes.client.KubernetesClientBuilder; import io.fabric8.kubernetes.client.utils.KubernetesResourceUtil; import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; +import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSACustomResource; import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSAReconciler; import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSASpec; -import io.javaoperatorsdk.operator.sample.dependentssa.DependnetSSACustomResource; import io.javaoperatorsdk.operator.sample.dependentssa.SSAConfigMapDependent; import static org.assertj.core.api.Assertions.assertThat; @@ -33,7 +33,7 @@ class DependentSSAMigrationIT { @BeforeEach void setup(TestInfo testInfo) { SSAConfigMapDependent.NUMBER_OF_UPDATES.set(0); - LocallyRunOperatorExtension.applyCrd(DependnetSSACustomResource.class, client); + LocallyRunOperatorExtension.applyCrd(DependentSSACustomResource.class, client); testInfo.getTestMethod().ifPresent(method -> { namespace = KubernetesResourceUtil.sanitizeName(method.getName()); cleanup(); @@ -53,7 +53,7 @@ void cleanup() { @Test void migratesFromLegacyToWorksAndBack() { var legacyOperator = createOperator(client, true, null); - DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator); + DependentSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator); var operator = createOperator(client, false, null); testResource = reconcileWithNewApproach(testResource, operator); @@ -66,7 +66,7 @@ void migratesFromLegacyToWorksAndBack() { @Test void usingDefaultFieldManagerDoesNotCreatesANewOneWithApplyOperation() { var legacyOperator = createOperator(client, true, null); - DependnetSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator); + DependentSSACustomResource testResource = reconcileWithLegacyOperator(legacyOperator); var operator = createOperator(client, false, FABRIC8_CLIENT_DEFAULT_FIELD_MANAGER); @@ -83,7 +83,7 @@ void usingDefaultFieldManagerDoesNotCreatesANewOneWithApplyOperation() { } private void reconcileAgainWithLegacy(Operator legacyOperator, - DependnetSSACustomResource testResource) { + DependentSSACustomResource testResource) { legacyOperator.start(); testResource.getSpec().setValue(INITIAL_VALUE); @@ -98,8 +98,8 @@ private void reconcileAgainWithLegacy(Operator legacyOperator, legacyOperator.stop(); } - private DependnetSSACustomResource reconcileWithNewApproach( - DependnetSSACustomResource testResource, Operator operator) { + private DependentSSACustomResource reconcileWithNewApproach( + DependentSSACustomResource testResource, Operator operator) { operator.start(); await().untilAsserted(() -> { @@ -124,7 +124,7 @@ private ConfigMap getDependentConfigMap() { return client.configMaps().inNamespace(namespace).withName(TEST_RESOURCE_NAME).get(); } - private DependnetSSACustomResource reconcileWithLegacyOperator(Operator legacyOperator) { + private DependentSSACustomResource reconcileWithLegacyOperator(Operator legacyOperator) { legacyOperator.start(); var testResource = client.resource(testResource()).create(); @@ -155,8 +155,8 @@ private Operator createOperator(KubernetesClient client, boolean legacyDependent return operator; } - public DependnetSSACustomResource testResource() { - DependnetSSACustomResource resource = new DependnetSSACustomResource(); + public DependentSSACustomResource testResource() { + DependentSSACustomResource resource = new DependentSSACustomResource(); resource.setMetadata(new ObjectMetaBuilder() .withNamespace(namespace) .withName(TEST_RESOURCE_NAME) diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java index 91519a9c13..0b89da68db 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/config/BaseConfigurationServiceTest.java @@ -42,6 +42,7 @@ import io.javaoperatorsdk.operator.processing.retry.GradualRetry; import io.javaoperatorsdk.operator.processing.retry.Retry; import io.javaoperatorsdk.operator.processing.retry.RetryExecution; +import io.javaoperatorsdk.operator.sample.dependentssa.DependentSSAReconciler; import io.javaoperatorsdk.operator.sample.readonly.ConfigMapReader; import io.javaoperatorsdk.operator.sample.readonly.ReadOnlyDependent; @@ -238,6 +239,7 @@ void configuringFromCustomAnnotationsShouldWork() { } @Test + @SuppressWarnings("unchecked") void excludedResourceClassesShouldNotUseSSAByDefault() { final var config = configFor(new SelectorReconciler()); @@ -246,10 +248,12 @@ void excludedResourceClassesShouldNotUseSSAByDefault() { extractDependentKubernetesResourceConfig(config, 1); assertNotNull(kubernetesDependentResourceConfig); assertTrue(kubernetesDependentResourceConfig.useSSA().isEmpty()); - assertFalse(configurationService.shouldUseSSA(ReadOnlyDependent.class, ConfigMap.class)); + assertFalse(configurationService.shouldUseSSA(ReadOnlyDependent.class, ConfigMap.class, + kubernetesDependentResourceConfig)); } @Test + @SuppressWarnings("unchecked") void excludedResourceClassesShouldUseSSAIfAnnotatedToDoSo() { final var config = configFor(new SelectorReconciler()); @@ -260,10 +264,11 @@ void excludedResourceClassesShouldUseSSAIfAnnotatedToDoSo() { assertFalse(kubernetesDependentResourceConfig.useSSA().isEmpty()); assertTrue((Boolean) kubernetesDependentResourceConfig.useSSA().get()); assertTrue(configurationService.shouldUseSSA(SelectorReconciler.WithAnnotation.class, - ConfigMap.class)); + ConfigMap.class, kubernetesDependentResourceConfig)); } @Test + @SuppressWarnings("unchecked") void dependentsShouldUseSSAByDefaultIfNotExcluded() { final var config = configFor(new DefaultSSAForDependentsReconciler()); @@ -271,14 +276,27 @@ void dependentsShouldUseSSAByDefaultIfNotExcluded() { assertNotNull(kubernetesDependentResourceConfig); assertTrue(kubernetesDependentResourceConfig.useSSA().isEmpty()); assertTrue(configurationService.shouldUseSSA( - DefaultSSAForDependentsReconciler.DefaultDependent.class, ConfigMapReader.class)); + DefaultSSAForDependentsReconciler.DefaultDependent.class, ConfigMapReader.class, + kubernetesDependentResourceConfig)); kubernetesDependentResourceConfig = extractDependentKubernetesResourceConfig(config, 1); assertNotNull(kubernetesDependentResourceConfig); assertTrue(kubernetesDependentResourceConfig.useSSA().isPresent()); assertFalse((Boolean) kubernetesDependentResourceConfig.useSSA().get()); assertFalse(configurationService - .shouldUseSSA(DefaultSSAForDependentsReconciler.NonSSADependent.class, Service.class)); + .shouldUseSSA(DefaultSSAForDependentsReconciler.NonSSADependent.class, Service.class, + kubernetesDependentResourceConfig)); + } + + @Test + void shouldUseSSAShouldAlsoWorkWithManualConfiguration() { + var reconciler = new DependentSSAReconciler(true); + assertEquals(reconciler.isUseSSA(), + configurationService.shouldUseSSA(reconciler.getSsaConfigMapDependent())); + + reconciler = new DependentSSAReconciler(false); + assertEquals(reconciler.isUseSSA(), + configurationService.shouldUseSSA(reconciler.getSsaConfigMapDependent())); } private static int getValue( diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependnetSSACustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSACustomResource.java similarity index 92% rename from operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependnetSSACustomResource.java rename to operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSACustomResource.java index 06834fe211..ca8ea4afc7 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependnetSSACustomResource.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSACustomResource.java @@ -9,7 +9,7 @@ @Group("sample.javaoperatorsdk") @Version("v1") @ShortNames("dssa") -public class DependnetSSACustomResource +public class DependentSSACustomResource extends CustomResource implements Namespaced { } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSAReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSAReconciler.java index f1c11dea6d..9b52dee28d 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSAReconciler.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/DependentSSAReconciler.java @@ -11,12 +11,13 @@ @ControllerConfiguration public class DependentSSAReconciler - implements Reconciler, TestExecutionInfoProvider, - EventSourceInitializer { + implements Reconciler, TestExecutionInfoProvider, + EventSourceInitializer { private final AtomicInteger numberOfExecutions = new AtomicInteger(0); - private SSAConfigMapDependent ssaConfigMapDependent = new SSAConfigMapDependent(); + private final SSAConfigMapDependent ssaConfigMapDependent = new SSAConfigMapDependent(); + private final boolean useSSA; public DependentSSAReconciler() { this(true); @@ -26,12 +27,21 @@ public DependentSSAReconciler(boolean useSSA) { ssaConfigMapDependent.configureWith(new KubernetesDependentResourceConfigBuilder() .withUseSSA(useSSA) .build()); + this.useSSA = useSSA; + } + + public boolean isUseSSA() { + return useSSA; + } + + public SSAConfigMapDependent getSsaConfigMapDependent() { + return ssaConfigMapDependent; } @Override - public UpdateControl reconcile( - DependnetSSACustomResource resource, - Context context) { + public UpdateControl reconcile( + DependentSSACustomResource resource, + Context context) { ssaConfigMapDependent.reconcile(resource, context); numberOfExecutions.addAndGet(1); @@ -44,7 +54,7 @@ public int getNumberOfExecutions() { @Override public Map prepareEventSources( - EventSourceContext context) { + EventSourceContext context) { return EventSourceInitializer.nameEventSourcesFromDependentResource(context, ssaConfigMapDependent); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/SSAConfigMapDependent.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/SSAConfigMapDependent.java index 806eccd717..dbeb9864e2 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/SSAConfigMapDependent.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/dependentssa/SSAConfigMapDependent.java @@ -10,7 +10,7 @@ import io.javaoperatorsdk.operator.processing.dependent.kubernetes.CRUDKubernetesDependentResource; public class SSAConfigMapDependent extends - CRUDKubernetesDependentResource { + CRUDKubernetesDependentResource { public static AtomicInteger NUMBER_OF_UPDATES = new AtomicInteger(0); @@ -21,8 +21,8 @@ public SSAConfigMapDependent() { } @Override - protected ConfigMap desired(DependnetSSACustomResource primary, - Context context) { + protected ConfigMap desired(DependentSSACustomResource primary, + Context context) { return new ConfigMapBuilder() .withMetadata(new ObjectMetaBuilder() .withName(primary.getMetadata().getName()) @@ -34,8 +34,8 @@ protected ConfigMap desired(DependnetSSACustomResource primary, @Override public ConfigMap update(ConfigMap actual, ConfigMap desired, - DependnetSSACustomResource primary, - Context context) { + DependentSSACustomResource primary, + Context context) { NUMBER_OF_UPDATES.incrementAndGet(); return super.update(actual, desired, primary, context); }