diff --git a/docs/documentation/features.md b/docs/documentation/features.md index d8e1cfa642..afc010ec06 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -711,30 +711,6 @@ setting, where this flag usually needs to be set to false, in order to control t See also an example implementation in the [WebPage sample](https://github.com/java-operator-sdk/java-operator-sdk/blob/3e2e7c4c834ef1c409d636156b988125744ca911/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageOperator.java#L38-L43) -## Optimization of Caches - -** Cache pruning is an experimental feature. Might a subject of change or even removal in the future. ** - -Operators using informers will initially cache the data for all known resources when starting up -so that access to resources can be performed quickly. Consequently, the memory required for the -operator to run and startup time will both increase quite dramatically when dealing with large -clusters with numerous resources. - -It is thus possible to configure the operator to cache only pruned versions of the resources to -alleviate the memory usage of the primary and secondary caches. This setup, however, has -implications on how reconcilers deal with resources since they will only work with partial -objects. As a consequence, resources need to be updated using PATCH operations only, sending -only required changes. - -To see how to use, and how to handle related caveats regarding how to deal with pruned objects -that leverage -[server side apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) patches, -please check the provided -[integration test](https://github.com/java-operator-sdk/java-operator-sdk/blob/c688524e64205690ba15587e7ed96a64dc231430/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java) -and associates reconciler. - -Pruned caches are currently not supported with the Dependent Resources feature. - ## Automatic Generation of CRDs Note that this feature is provided by the diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java index db91ee22af..2e8895ad87 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/BaseConfigurationService.java @@ -7,7 +7,6 @@ import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.function.Function; -import java.util.function.UnaryOperator; import java.util.stream.Collectors; import org.slf4j.Logger; @@ -144,8 +143,6 @@ protected

ControllerConfiguration

configFor(Reconcile Utils.contextFor(name, null, null)), Utils.instantiate(annotation.genericFilter(), GenericFilter.class, Utils.contextFor(name, null, null)), - Utils.instantiate(annotation.cachePruneFunction(), UnaryOperator.class, - Utils.contextFor(name, null, null)), Set.of(valueOrDefault(annotation, io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration::namespaces, DEFAULT_NAMESPACES_SET.toArray(String[]::new))), 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 789de127f5..4a3efbc4a6 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 @@ -6,7 +6,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.function.UnaryOperator; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; @@ -36,7 +35,6 @@ public class ControllerConfigurationOverrider { private OnUpdateFilter onUpdateFilter; private GenericFilter genericFilter; private RateLimiter rateLimiter; - private UnaryOperator cachePruneFunction; private Map configurations; private ControllerConfigurationOverrider(ControllerConfiguration original) { @@ -52,7 +50,6 @@ private ControllerConfigurationOverrider(ControllerConfiguration original) { this.genericFilter = original.genericFilter().orElse(null); this.original = original; this.rateLimiter = original.getRateLimiter(); - this.cachePruneFunction = original.cachePruneFunction().orElse(null); } public ControllerConfigurationOverrider withFinalizer(String finalizer) { @@ -155,12 +152,6 @@ public ControllerConfigurationOverrider withGenericFilter(GenericFilter ge return this; } - public ControllerConfigurationOverrider withCachePruneFunction( - UnaryOperator cachePruneFunction) { - this.cachePruneFunction = cachePruneFunction; - return this; - } - public ControllerConfigurationOverrider replacingNamedDependentResourceConfig(String name, Object dependentResourceConfig) { @@ -182,7 +173,7 @@ public ControllerConfiguration build() { final var overridden = new ResolvedControllerConfiguration<>( original.getResourceClass(), original.getName(), generationAware, original.getAssociatedReconcilerClassName(), retry, rateLimiter, - reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter, cachePruneFunction, + reconciliationMaxInterval, onAddFilter, onUpdateFilter, genericFilter, original.getDependentResources(), namespaces, finalizer, labelSelector, configurations); overridden.setEventFilter(customResourcePredicate); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java index bb9f365d58..b85bd76e9a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/DefaultResourceConfiguration.java @@ -2,7 +2,6 @@ import java.util.Optional; import java.util.Set; -import java.util.function.UnaryOperator; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.ReconcilerUtils; @@ -20,12 +19,10 @@ public class DefaultResourceConfiguration private final GenericFilter genericFilter; private final String labelSelector; private final Set namespaces; - private final UnaryOperator cachePruneFunction; protected DefaultResourceConfiguration(Class resourceClass, Set namespaces, String labelSelector, OnAddFilter onAddFilter, - OnUpdateFilter onUpdateFilter, GenericFilter genericFilter, - UnaryOperator cachePruneFunction) { + OnUpdateFilter onUpdateFilter, GenericFilter genericFilter) { this.resourceClass = resourceClass; this.resourceTypeName = ReconcilerUtils.getResourceTypeName(resourceClass); this.onAddFilter = onAddFilter; @@ -34,7 +31,6 @@ protected DefaultResourceConfiguration(Class resourceClass, this.namespaces = ResourceConfiguration.ensureValidNamespaces(namespaces); this.labelSelector = ResourceConfiguration.ensureValidLabelSelector(labelSelector); - this.cachePruneFunction = cachePruneFunction; } @Override @@ -52,11 +48,6 @@ public Set getNamespaces() { return namespaces; } - @Override - public Optional> cachePruneFunction() { - return Optional.ofNullable(this.cachePruneFunction); - } - @Override public Class getResourceClass() { return resourceClass; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java index 0566e1a328..6818c75c4a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResolvedControllerConfiguration.java @@ -1,13 +1,8 @@ package io.javaoperatorsdk.operator.api.config; import java.time.Duration; -import java.util.Collections; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; +import java.util.*; import java.util.concurrent.TimeUnit; -import java.util.function.UnaryOperator; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceConfigurationProvider; @@ -43,7 +38,7 @@ public ResolvedControllerConfiguration(Class

resourceClass, ControllerConfigu other.getAssociatedReconcilerClassName(), other.getRetry(), other.getRateLimiter(), other.maxReconciliationInterval().orElse(null), other.onAddFilter().orElse(null), other.onUpdateFilter().orElse(null), - other.genericFilter().orElse(null), other.cachePruneFunction().orElse(null), + other.genericFilter().orElse(null), other.getDependentResources(), other.getNamespaces(), other.getFinalizerName(), other.getLabelSelector(), Collections.emptyMap()); } @@ -69,12 +64,12 @@ public ResolvedControllerConfiguration(Class

resourceClass, String name, boolean generationAware, String associatedReconcilerClassName, Retry retry, RateLimiter rateLimiter, Duration maxReconciliationInterval, OnAddFilter

onAddFilter, OnUpdateFilter

onUpdateFilter, - GenericFilter

genericFilter, UnaryOperator

cachePruneFunction, + GenericFilter

genericFilter, List dependentResources, Set namespaces, String finalizer, String labelSelector, Map configurations) { this(resourceClass, name, generationAware, associatedReconcilerClassName, retry, rateLimiter, - maxReconciliationInterval, onAddFilter, onUpdateFilter, genericFilter, cachePruneFunction, + maxReconciliationInterval, onAddFilter, onUpdateFilter, genericFilter, namespaces, finalizer, labelSelector, configurations); setDependentResources(dependentResources); } @@ -83,11 +78,9 @@ protected ResolvedControllerConfiguration(Class

resourceClass, String name, boolean generationAware, String associatedReconcilerClassName, Retry retry, RateLimiter rateLimiter, Duration maxReconciliationInterval, OnAddFilter

onAddFilter, OnUpdateFilter

onUpdateFilter, GenericFilter

genericFilter, - UnaryOperator

cachePruneFunction, Set namespaces, String finalizer, String labelSelector, Map configurations) { - super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter, - cachePruneFunction); + super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter); this.name = ControllerConfiguration.ensureValidName(name, associatedReconcilerClassName); this.generationAware = generationAware; this.associatedReconcilerClassName = associatedReconcilerClassName; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java index 85f3bc3b36..81b7fdf4a7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java @@ -4,13 +4,11 @@ import java.util.Collections; import java.util.Optional; import java.util.Set; -import java.util.function.UnaryOperator; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.reconciler.Constants; -import io.javaoperatorsdk.operator.api.reconciler.ControllerConfiguration; import io.javaoperatorsdk.operator.processing.event.source.filter.GenericFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnAddFilter; import io.javaoperatorsdk.operator.processing.event.source.filter.OnUpdateFilter; @@ -124,11 +122,4 @@ default Set getEffectiveNamespaces() { } return targetNamespaces; } - - /** - * See {@link ControllerConfiguration#cachePruneFunction()} for details. - */ - default Optional> cachePruneFunction() { - return Optional.empty(); - } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index e26597dde5..c6ee847cb0 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java @@ -3,7 +3,6 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; -import java.util.function.UnaryOperator; import io.fabric8.kubernetes.api.model.HasMetadata; import io.javaoperatorsdk.operator.api.config.DefaultResourceConfiguration; @@ -39,10 +38,8 @@ protected DefaultInformerConfiguration(String labelSelector, OnAddFilter onAddFilter, OnUpdateFilter onUpdateFilter, OnDeleteFilter onDeleteFilter, - GenericFilter genericFilter, - UnaryOperator cachePruneFunction) { - super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter, - cachePruneFunction); + GenericFilter genericFilter) { + super(resourceClass, namespaces, labelSelector, onAddFilter, onUpdateFilter, genericFilter); this.followControllerNamespaceChanges = followControllerNamespaceChanges; this.primaryToSecondaryMapper = primaryToSecondaryMapper; @@ -106,7 +103,6 @@ class InformerConfigurationBuilder { private OnDeleteFilter onDeleteFilter; private GenericFilter genericFilter; private boolean inheritControllerNamespacesOnChange = false; - private UnaryOperator cachePruneFunction; private InformerConfigurationBuilder(Class resourceClass) { this.resourceClass = resourceClass; @@ -207,18 +203,12 @@ public InformerConfigurationBuilder withGenericFilter(GenericFilter generi return this; } - public InformerConfigurationBuilder withCachePruneFunction( - UnaryOperator cachePruneFunction) { - this.cachePruneFunction = cachePruneFunction; - return this; - } - public InformerConfiguration build() { return new DefaultInformerConfiguration<>(labelSelector, resourceClass, primaryToSecondaryMapper, secondaryToPrimaryMapper, namespaces, inheritControllerNamespacesOnChange, onAddFilter, onUpdateFilter, - onDeleteFilter, genericFilter, cachePruneFunction); + onDeleteFilter, genericFilter); } } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java index df3efa7d40..ec76adf89d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java @@ -5,7 +5,6 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import java.util.function.UnaryOperator; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; @@ -119,25 +118,4 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation * accessible no-arg constructor. */ Class rateLimiter() default LinearRateLimiter.class; - - /** - *

- * This is an experimental feature, might be a subject of change and even removal in the - * future. - *

- *

- * In order to optimize cache, thus set null on some attributes, this function can be set. Note - * that this has subtle implications how updates on the resources should be handled. Notably only - * patching of the resource can be used from that point, since update would remove not cached - * parts of the resource. - *

- *

- * Note that this feature does not work with Dependent Resources. - *

- * - * - * - * @return function to remove parts of the resource. - */ - Class cachePruneFunction() default UnaryOperator.class; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java index 4a7ac68082..3018ed30f1 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcher.java @@ -12,8 +12,6 @@ import io.fabric8.kubernetes.client.KubernetesClientException; import io.fabric8.kubernetes.client.dsl.MixedOperation; import io.fabric8.kubernetes.client.dsl.Resource; -import io.fabric8.kubernetes.client.dsl.base.PatchContext; -import io.fabric8.kubernetes.client.dsl.base.PatchType; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.api.ObservedGenerationAware; import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; @@ -36,7 +34,7 @@ */ class ReconciliationDispatcher

{ - public static final int MAX_FINALIZER_REMOVAL_RETRY = 10; + public static final int MAX_UPDATE_RETRY = 10; private static final Logger log = LoggerFactory.getLogger(ReconciliationDispatcher.class); @@ -295,8 +293,7 @@ private PostExecutionControl

handleCleanup(P originalResource, P resource, // cleanup is finished, nothing left to done final var finalizerName = configuration().getFinalizerName(); if (deleteControl.isRemoveFinalizer() && resource.hasFinalizer(finalizerName)) { - P customResource = conflictRetryingPatch(resource, originalResource, - r -> r.removeFinalizer(finalizerName)); + P customResource = conflictRetryingUpdate(resource, r -> r.removeFinalizer(finalizerName)); return PostExecutionControl.customResourceFinalizerRemoved(customResource); } } @@ -315,8 +312,7 @@ private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalRe log.debug( "Adding finalizer for resource: {} version: {}", getUID(originalResource), getVersion(originalResource)); - - return conflictRetryingPatch(resourceForExecution, originalResource, + return conflictRetryingUpdate(resourceForExecution, r -> r.addFinalizer(configuration().getFinalizerName())); } @@ -330,8 +326,7 @@ ControllerConfiguration

configuration() { return controller.getConfiguration(); } - public P conflictRetryingPatch(P resource, P originalResource, - Function modificationFunction) { + public P conflictRetryingUpdate(P resource, Function modificationFunction) { if (log.isDebugEnabled()) { log.debug("Removing finalizer on resource: {}", ResourceID.fromResource(resource)); } @@ -342,7 +337,7 @@ public P conflictRetryingPatch(P resource, P originalResource, if (Boolean.FALSE.equals(modified)) { return resource; } - return customResourceFacade.serverSideApplyLockResource(resource, originalResource); + return customResourceFacade.updateResource(resource); } catch (KubernetesClientException e) { log.trace("Exception during patch for resource: {}", resource); retryIndex++; @@ -350,9 +345,9 @@ public P conflictRetryingPatch(P resource, P originalResource, if (e.getCode() != 409) { throw e; } - if (retryIndex >= MAX_FINALIZER_REMOVAL_RETRY) { + if (retryIndex >= MAX_UPDATE_RETRY) { throw new OperatorException( - "Exceeded maximum (" + MAX_FINALIZER_REMOVAL_RETRY + "Exceeded maximum (" + MAX_UPDATE_RETRY + ") retry attempts to patch resource: " + ResourceID.fromResource(resource)); } @@ -380,13 +375,6 @@ public R getResource(String namespace, String name) { } } - public R serverSideApplyLockResource(R resource, R originalResource) { - var patchContext = PatchContext.of(PatchType.SERVER_SIDE_APPLY); - patchContext.setForce(true); - return resource(originalResource).patch(patchContext, - resource); - } - public R updateResource(R resource) { log.debug( "Trying to replace resource {}, version: {}", diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java index 34f89dd68b..c6636a4a5d 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java @@ -113,8 +113,6 @@ private InformerWrapper createEventSource( FilterWatchListDeletable, Resource> filteredBySelectorClient, ResourceEventHandler eventHandler, String namespaceIdentifier) { var informer = filteredBySelectorClient.runnableInformer(0); - configuration.cachePruneFunction() - .ifPresent(f -> informer.itemStore(new TransformingItemStore<>(f))); var source = new InformerWrapper<>(informer, namespaceIdentifier); source.addEventHandler(eventHandler); diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java index 4286335644..3e7400b90f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java @@ -43,8 +43,7 @@ public abstract class ManagedInformerEventSource, Resource> client, C configuration) { super(configuration.getResourceClass()); - temporaryResourceCache = new TemporaryResourceCache<>(this, - configuration.cachePruneFunction().orElse(null)); + temporaryResourceCache = new TemporaryResourceCache<>(this); manager().initSources(client, configuration, this); this.configuration = configuration; } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java index 0af2ec0b2f..8c3d56c008 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCache.java @@ -3,7 +3,6 @@ import java.util.Map; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; -import java.util.function.UnaryOperator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,14 +34,11 @@ public class TemporaryResourceCache { private static final Logger log = LoggerFactory.getLogger(TemporaryResourceCache.class); - private UnaryOperator cachePruneFunction; private final Map cache = new ConcurrentHashMap<>(); private final ManagedInformerEventSource managedInformerEventSource; - public TemporaryResourceCache(ManagedInformerEventSource managedInformerEventSource, - UnaryOperator cachePruneFunction) { + public TemporaryResourceCache(ManagedInformerEventSource managedInformerEventSource) { this.managedInformerEventSource = managedInformerEventSource; - this.cachePruneFunction = cachePruneFunction; } public synchronized void removeResourceFromCache(T resource) { @@ -83,9 +79,6 @@ public synchronized void putUpdatedResource(T newResource, String previousResour } private void putToCache(T resource, ResourceID resourceID) { - if (cachePruneFunction != null) { - resource = cachePruneFunction.apply(resource); - } cache.put(resourceID == null ? ResourceID.fromResource(resource) : resourceID, resource); } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java index ef7a27677b..c154e8a910 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java @@ -38,7 +38,7 @@ import io.javaoperatorsdk.operator.sample.simple.TestCustomResource; import static io.javaoperatorsdk.operator.TestUtils.markForDeletion; -import static io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.MAX_FINALIZER_REMOVAL_RETRY; +import static io.javaoperatorsdk.operator.processing.event.ReconciliationDispatcher.MAX_UPDATE_RETRY; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -134,9 +134,8 @@ void addFinalizerOnNewResource() { verify(reconciler, never()) .reconcile(ArgumentMatchers.eq(testCustomResource), any()); verify(customResourceFacade, times(1)) - .serverSideApplyLockResource( - argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)), - any()); + .updateResource( + argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER))); assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)).isTrue(); } @@ -216,8 +215,7 @@ void removesDefaultFinalizerOnDeleteIfSet() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertThat(postExecControl.isFinalizerRemoved()).isTrue(); - verify(customResourceFacade, times(1)).serverSideApplyLockResource(testCustomResource, - testCustomResource); + verify(customResourceFacade, times(1)).updateResource(testCustomResource); } @Test @@ -226,7 +224,7 @@ void retriesFinalizerRemovalWithFreshResource() { markForDeletion(testCustomResource); var resourceWithFinalizer = TestUtils.testCustomResource(); resourceWithFinalizer.addFinalizer(DEFAULT_FINALIZER); - when(customResourceFacade.serverSideApplyLockResource(testCustomResource, testCustomResource)) + when(customResourceFacade.updateResource(testCustomResource)) .thenThrow(new KubernetesClientException(null, 409, null)) .thenReturn(testCustomResource); when(customResourceFacade.getResource(any(), any())).thenReturn(resourceWithFinalizer); @@ -235,7 +233,7 @@ void retriesFinalizerRemovalWithFreshResource() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertThat(postExecControl.isFinalizerRemoved()).isTrue(); - verify(customResourceFacade, times(2)).serverSideApplyLockResource(any(), any()); + verify(customResourceFacade, times(2)).updateResource(any()); verify(customResourceFacade, times(1)).getResource(any(), any()); } @@ -243,7 +241,7 @@ void retriesFinalizerRemovalWithFreshResource() { void throwsExceptionIfFinalizerRemovalRetryExceeded() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); markForDeletion(testCustomResource); - when(customResourceFacade.serverSideApplyLockResource(any(), any())) + when(customResourceFacade.updateResource(any())) .thenThrow(new KubernetesClientException(null, 409, null)); when(customResourceFacade.getResource(any(), any())) .thenAnswer((Answer) invocationOnMock -> createResourceWithFinalizer()); @@ -255,10 +253,8 @@ void throwsExceptionIfFinalizerRemovalRetryExceeded() { assertThat(postExecControl.getRuntimeException()).isPresent(); assertThat(postExecControl.getRuntimeException().get()) .isInstanceOf(OperatorException.class); - verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY)).serverSideApplyLockResource( - any(), - any()); - verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY - 1)).getResource(any(), + verify(customResourceFacade, times(MAX_UPDATE_RETRY)).updateResource(any()); + verify(customResourceFacade, times(MAX_UPDATE_RETRY - 1)).getResource(any(), any()); } @@ -266,7 +262,7 @@ void throwsExceptionIfFinalizerRemovalRetryExceeded() { void throwsExceptionIfFinalizerRemovalClientExceptionIsNotConflict() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); markForDeletion(testCustomResource); - when(customResourceFacade.serverSideApplyLockResource(any(), any())) + when(customResourceFacade.updateResource(any())) .thenThrow(new KubernetesClientException(null, 400, null)); var res = @@ -274,7 +270,7 @@ void throwsExceptionIfFinalizerRemovalClientExceptionIsNotConflict() { assertThat(res.getRuntimeException()).isPresent(); assertThat(res.getRuntimeException().get()).isInstanceOf(KubernetesClientException.class); - verify(customResourceFacade, times(1)).serverSideApplyLockResource(any(), any()); + verify(customResourceFacade, times(1)).updateResource(any()); verify(customResourceFacade, never()).getResource(any(), any()); } @@ -338,14 +334,14 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() { removeFinalizers(testCustomResource); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); - when(customResourceFacade.serverSideApplyLockResource(any(), any())) + when(customResourceFacade.updateResource(any())) .thenReturn(testCustomResource); var postExecControl = reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertEquals(1, testCustomResource.getMetadata().getFinalizers().size()); - verify(customResourceFacade, times(1)).serverSideApplyLockResource(any(), any()); + verify(customResourceFacade, times(1)).updateResource(any()); assertThat(postExecControl.updateIsStatusPatch()).isFalse(); assertThat(postExecControl.getUpdatedCustomResource()).isPresent(); } @@ -645,7 +641,7 @@ void canSkipSchedulingMaxDelayIf() { void retriesAddingFinalizer() { removeFinalizers(testCustomResource); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); - when(customResourceFacade.serverSideApplyLockResource(any(), any())) + when(customResourceFacade.updateResource(any())) .thenThrow(new KubernetesClientException(null, 409, null)) .thenReturn(testCustomResource); when(customResourceFacade.getResource(any(), any())) @@ -656,7 +652,7 @@ void retriesAddingFinalizer() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, times(2)).serverSideApplyLockResource(any(), any()); + verify(customResourceFacade, times(2)).updateResource(any()); } private ObservedGenCustomResource createObservedGenCustomResource() { diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java index 5a171195e1..dc8cf8698b 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerResourceEventSourceTest.java @@ -195,7 +195,6 @@ public TestConfiguration(boolean generationAware, OnAddFilter informerEventSource = mock(InformerEventSource.class); private TemporaryResourceCache temporaryResourceCache = - new TemporaryResourceCache<>(informerEventSource, null); - + new TemporaryResourceCache<>(informerEventSource); @Test void updateAddsTheResourceIntoCacheIfTheInformerHasThePreviousResourceVersion() { @@ -80,29 +79,6 @@ void removesResourceFromCache() { .isNotPresent(); } - @Test - void objectIsTransformedBeforePutIntoCache() { - temporaryResourceCache = - new TemporaryResourceCache<>(informerEventSource, r -> { - r.getMetadata().setLabels(null); - return r; - }); - - temporaryResourceCache.putAddedResource(testResource()); - assertLabelsIsEmpty(temporaryResourceCache); - - temporaryResourceCache.unconditionallyCacheResource(testResource()); - assertLabelsIsEmpty(temporaryResourceCache); - - temporaryResourceCache.unconditionallyCacheResource(testResource()); - assertLabelsIsEmpty(temporaryResourceCache); - } - - private void assertLabelsIsEmpty(TemporaryResourceCache temporaryResourceCache) { - assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource())) - .orElseThrow().getMetadata().getLabels()).isNull(); - } - private ConfigMap propagateTestResourceToCache() { var testResource = testResource(); when(informerEventSource.get(any())).thenReturn(Optional.empty()); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java deleted file mode 100644 index 9bf57ab372..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java +++ /dev/null @@ -1,77 +0,0 @@ -package io.javaoperatorsdk.operator; - -import java.time.Duration; -import java.util.Map; - -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.RegisterExtension; - -import io.fabric8.kubernetes.api.model.ConfigMap; -import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.javaoperatorsdk.operator.junit.LocallyRunOperatorExtension; -import io.javaoperatorsdk.operator.sample.cacheprune.CachePruneCustomResource; -import io.javaoperatorsdk.operator.sample.cacheprune.CachePruneReconciler; -import io.javaoperatorsdk.operator.sample.cacheprune.CachePruneSpec; - -import static io.javaoperatorsdk.operator.sample.cacheprune.CachePruneReconciler.DATA_KEY; -import static org.assertj.core.api.Assertions.assertThat; -import static org.awaitility.Awaitility.await; - -class CachePruneIT { - - public static final String DEFAULT_DATA = "default_data"; - public static final String TEST_RESOURCE_NAME = "test1"; - public static final String UPDATED_DATA = "updated_data"; - - @RegisterExtension - LocallyRunOperatorExtension operator = - LocallyRunOperatorExtension.builder() - .withReconciler(new CachePruneReconciler()).build(); - - @Test - void pruningRelatedBehavior() { - var res = operator.create(testResource()); - await().untilAsserted(() -> { - assertState(DEFAULT_DATA); - }); - - res.getSpec().setData(UPDATED_DATA); - var updated = operator.replace(res); - - await().untilAsserted(() -> { - assertState(UPDATED_DATA); - }); - - operator.delete(updated); - - await().atMost(Duration.ofSeconds(30)).untilAsserted(() -> { - var actual = operator.get(CachePruneCustomResource.class, TEST_RESOURCE_NAME); - var configMap = operator.get(ConfigMap.class, TEST_RESOURCE_NAME); - assertThat(configMap).isNull(); - assertThat(actual).isNull(); - }); - } - - void assertState(String expectedData) { - var actual = operator.get(CachePruneCustomResource.class, TEST_RESOURCE_NAME); - assertThat(actual.getMetadata()).isNotNull(); - assertThat(actual.getMetadata().getFinalizers()).isNotEmpty(); - assertThat(actual.getStatus().getCreated()).isTrue(); - assertThat(actual.getMetadata().getLabels()).isNotEmpty(); - var configMap = operator.get(ConfigMap.class, TEST_RESOURCE_NAME); - assertThat(configMap.getData()).containsEntry(DATA_KEY, expectedData); - assertThat(configMap.getMetadata().getLabels()).isNotEmpty(); - } - - CachePruneCustomResource testResource() { - var res = new CachePruneCustomResource(); - res.setMetadata(new ObjectMetaBuilder() - .withName(TEST_RESOURCE_NAME) - .withLabels(Map.of("sampleLabel", "val")) - .build()); - res.setSpec(new CachePruneSpec()); - res.getSpec().setData(DEFAULT_DATA); - return res; - } - -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneCustomResource.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneCustomResource.java deleted file mode 100644 index 60431588fd..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneCustomResource.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.javaoperatorsdk.operator.sample.cacheprune; - -import io.fabric8.kubernetes.api.model.Namespaced; -import io.fabric8.kubernetes.client.CustomResource; -import io.fabric8.kubernetes.model.annotation.Group; -import io.fabric8.kubernetes.model.annotation.ShortNames; -import io.fabric8.kubernetes.model.annotation.Version; - -@Group("sample.javaoperatorsdk") -@Version("v1") -@ShortNames("cpr") -public class CachePruneCustomResource - extends CustomResource - implements Namespaced { -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneReconciler.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneReconciler.java deleted file mode 100644 index 236b205c2c..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneReconciler.java +++ /dev/null @@ -1,107 +0,0 @@ -package io.javaoperatorsdk.operator.sample.cacheprune; - -import java.util.HashMap; -import java.util.Map; - -import io.fabric8.kubernetes.api.model.ConfigMap; -import io.fabric8.kubernetes.api.model.ObjectMeta; -import io.fabric8.kubernetes.client.KubernetesClient; -import io.fabric8.kubernetes.client.dsl.base.PatchContext; -import io.fabric8.kubernetes.client.dsl.base.PatchType; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; -import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; -import io.javaoperatorsdk.operator.api.reconciler.*; -import io.javaoperatorsdk.operator.junit.KubernetesClientAware; -import io.javaoperatorsdk.operator.processing.event.source.EventSource; -import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; - -@ControllerConfiguration(cachePruneFunction = LabelRemovingPruneFunction.class) -public class CachePruneReconciler - implements Reconciler, - EventSourceInitializer, - Cleaner, KubernetesClientAware { - - public static final String DATA_KEY = "data"; - public static final String FIELD_MANAGER = "controller"; - public static final String SECONDARY_CREATE_FIELD_MANAGER = "creator"; - private KubernetesClient client; - - @Override - public UpdateControl reconcile( - CachePruneCustomResource resource, - Context context) { - var configMap = context.getSecondaryResource(ConfigMap.class); - configMap.ifPresentOrElse(cm -> { - if (!cm.getMetadata().getLabels().isEmpty()) { - throw new AssertionError("Labels should be null"); - } - if (!cm.getData().get(DATA_KEY) - .equals(resource.getSpec().getData())) { - var cloned = ConfigurationServiceProvider.instance().getResourceCloner().clone(cm); - cloned.getData().put(DATA_KEY, resource.getSpec().getData()); - var patchContext = patchContextWithFieldManager(FIELD_MANAGER); - // setting new field manager since we don't control label anymore: - // since not the whole object is present in cache SSA would remove labels if the controller - // is not the manager. - // Note that JSON Merge Patch (or others would also work here, without this "hack". - patchContext.setForce(true); - patchContext.setFieldManager(FIELD_MANAGER); - client.configMaps().resource(cm) - .patch(patchContext, cloned); - } - }, () -> client.configMaps().resource(configMap(resource)) - .patch(patchContextWithFieldManager(SECONDARY_CREATE_FIELD_MANAGER))); - - resource.setStatus(new CachePruneStatus()); - resource.getStatus().setCreated(true); - return UpdateControl.patchStatus(resource); - } - - private PatchContext patchContextWithFieldManager(String fieldManager) { - PatchContext patchContext = new PatchContext(); - // using server side apply - patchContext.setPatchType(PatchType.SERVER_SIDE_APPLY); - patchContext.setFieldManager(fieldManager); - return patchContext; - } - - @Override - public Map prepareEventSources( - EventSourceContext context) { - InformerEventSource configMapEventSource = - new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context) - .withCachePruneFunction(new LabelRemovingPruneFunction<>()) - .build(), - context); - return EventSourceInitializer.nameEventSources(configMapEventSource); - } - - ConfigMap configMap(CachePruneCustomResource resource) { - ConfigMap configMap = new ConfigMap(); - configMap.setMetadata(new ObjectMeta()); - configMap.getMetadata().setName(resource.getMetadata().getName()); - configMap.getMetadata().setNamespace(resource.getMetadata().getNamespace()); - configMap.setData(Map.of(DATA_KEY, resource.getSpec().getData())); - HashMap labels = new HashMap<>(); - labels.put("mylabel", "val"); - configMap.getMetadata().setLabels(labels); - configMap.addOwnerReference(resource); - return configMap; - } - - @Override - public KubernetesClient getKubernetesClient() { - return client; - } - - @Override - public void setKubernetesClient(KubernetesClient kubernetesClient) { - this.client = kubernetesClient; - } - - @Override - public DeleteControl cleanup(CachePruneCustomResource resource, - Context context) { - return DeleteControl.defaultDelete(); - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneSpec.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneSpec.java deleted file mode 100644 index 2d58a70d3a..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneSpec.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.javaoperatorsdk.operator.sample.cacheprune; - -public class CachePruneSpec { - - private String data; - - public String getData() { - return data; - } - - public CachePruneSpec setData(String data) { - this.data = data; - return this; - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneStatus.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneStatus.java deleted file mode 100644 index a074c0e011..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneStatus.java +++ /dev/null @@ -1,15 +0,0 @@ -package io.javaoperatorsdk.operator.sample.cacheprune; - -public class CachePruneStatus { - - private Boolean created; - - public Boolean getCreated() { - return created; - } - - public CachePruneStatus setCreated(Boolean created) { - this.created = created; - return this; - } -} diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingPruneFunction.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingPruneFunction.java deleted file mode 100644 index a495803628..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingPruneFunction.java +++ /dev/null @@ -1,13 +0,0 @@ -package io.javaoperatorsdk.operator.sample.cacheprune; - -import java.util.function.UnaryOperator; - -import io.fabric8.kubernetes.api.model.HasMetadata; - -public class LabelRemovingPruneFunction implements UnaryOperator { - @Override - public R apply(R r) { - r.getMetadata().setLabels(null); - return r; - } -} diff --git a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java index b2e0963ef2..fa41e2ff13 100644 --- a/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java +++ b/sample-operators/webpage/src/main/java/io/javaoperatorsdk/operator/sample/WebPageManagedDependentsReconciler.java @@ -1,12 +1,7 @@ package io.javaoperatorsdk.operator.sample; import io.fabric8.kubernetes.api.model.ConfigMap; -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.Reconciler; -import io.javaoperatorsdk.operator.api.reconciler.UpdateControl; +import io.javaoperatorsdk.operator.api.reconciler.*; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import static io.javaoperatorsdk.operator.sample.Utils.createStatus; @@ -25,7 +20,7 @@ reconcilePrecondition = ExposedIngressCondition.class) }) public class WebPageManagedDependentsReconciler - implements Reconciler, ErrorStatusHandler { + implements Reconciler, ErrorStatusHandler, Cleaner { public static final String SELECTOR = "managed"; @@ -45,4 +40,9 @@ public UpdateControl reconcile(WebPage webPage, Context contex webPage.setStatus(createStatus(name)); return UpdateControl.patchStatus(webPage); } + + @Override + public DeleteControl cleanup(WebPage resource, Context context) { + return DeleteControl.defaultDelete(); + } }