From c126aef57d3ed31f9afe4545ad6ccc1ca3be774d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Wed, 23 Nov 2022 12:40:37 +0100 Subject: [PATCH 01/23] feat: runtime info for health probes (#1594) --- .../processing/event/EventSourceManager.java | 1 + .../source/informer/InformerWrapper.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 6a6aae471a..7ebde8e5bf 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -6,6 +6,7 @@ import java.util.Objects; import java.util.Set; import java.util.function.Function; +import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java index 8809022e95..dc4f11283a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java @@ -180,4 +180,24 @@ public boolean isRunning() { public String getTargetNamespace() { return namespaceIdentifier; } + + @Override + public boolean hasSynced() { + return informer.hasSynced(); + } + + @Override + public boolean isWatching() { + return informer.isWatching(); + } + + @Override + public boolean isRunning() { + return informer.isRunning(); + } + + @Override + public String getTargetNamespace() { + return namespaceIdentifier; + } } From 52b38f90c162f955ddbd730c97100286f597ab86 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 23 Nov 2022 16:47:42 +0100 Subject: [PATCH 02/23] feat: tranforming object in store --- .../informer/InformerConfiguration.java | 21 +++++- .../source/informer/InformerManager.java | 4 +- .../source/informer/InformerWrapper.java | 1 - .../informer/ObjectTransformingItemStore.java | 69 +++++++++++++++++++ 4 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index 52f71501a9..3ae0452c21 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 @@ -5,6 +5,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.config.DefaultResourceConfiguration; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.api.config.Utils; @@ -29,6 +30,7 @@ class DefaultInformerConfiguration extends private final SecondaryToPrimaryMapper secondaryToPrimaryMapper; private final boolean followControllerNamespaceChanges; private final OnDeleteFilter onDeleteFilter; + private final ItemStore itemStore; protected DefaultInformerConfiguration(String labelSelector, Class resourceClass, @@ -38,7 +40,8 @@ protected DefaultInformerConfiguration(String labelSelector, OnAddFilter onAddFilter, OnUpdateFilter onUpdateFilter, OnDeleteFilter onDeleteFilter, - GenericFilter genericFilter) { + GenericFilter genericFilter, + ItemStore itemStore) { super(labelSelector, resourceClass, onAddFilter, onUpdateFilter, genericFilter, namespaces); this.followControllerNamespaceChanges = followControllerNamespaceChanges; @@ -47,6 +50,7 @@ protected DefaultInformerConfiguration(String labelSelector, Objects.requireNonNullElse(secondaryToPrimaryMapper, Mappers.fromOwnerReference()); this.onDeleteFilter = onDeleteFilter; + this.itemStore = itemStore; } @Override @@ -67,6 +71,11 @@ public Optional> onDeleteFilter() { public

PrimaryToSecondaryMapper

getPrimaryToSecondaryMapper() { return (PrimaryToSecondaryMapper

) primaryToSecondaryMapper; } + + @Override + public ItemStore itemStore() { + return this.itemStore; + } } /** @@ -89,6 +98,8 @@ public

PrimaryToSecondaryMapper

getPrimaryToSecondary

PrimaryToSecondaryMapper

getPrimaryToSecondaryMapper(); + ItemStore itemStore(); + @SuppressWarnings("unused") class InformerConfigurationBuilder { @@ -102,6 +113,7 @@ class InformerConfigurationBuilder { private OnDeleteFilter onDeleteFilter; private GenericFilter genericFilter; private boolean inheritControllerNamespacesOnChange = false; + private ItemStore itemStore; private InformerConfigurationBuilder(Class resourceClass) { this.resourceClass = resourceClass; @@ -202,12 +214,17 @@ public InformerConfigurationBuilder withGenericFilter(GenericFilter generi return this; } + public InformerConfigurationBuilder withOnDeleteFilter(ItemStore itemStore) { + this.itemStore = itemStore; + return this; + } + public InformerConfiguration build() { return new DefaultInformerConfiguration<>(labelSelector, resourceClass, primaryToSecondaryMapper, secondaryToPrimaryMapper, namespaces, inheritControllerNamespacesOnChange, onAddFilter, onUpdateFilter, - onDeleteFilter, genericFilter); + onDeleteFilter, genericFilter, itemStore); } } 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 d51301c385..b19341773f 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 @@ -109,8 +109,10 @@ private InformerWrapper createEventSourceForNamespace(String namespace) { private InformerWrapper createEventSource( FilterWatchListDeletable, Resource> filteredBySelectorClient, ResourceEventHandler eventHandler, String namespaceIdentifier) { + var informer = filteredBySelectorClient.runnableInformer(0); + // todo add custom item store var source = - new InformerWrapper<>(filteredBySelectorClient.runnableInformer(0), namespaceIdentifier); + new InformerWrapper<>(informer, namespaceIdentifier); source.addEventHandler(eventHandler); sources.put(namespaceIdentifier, source); return source; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java index dc4f11283a..e870ceaa5e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java @@ -39,7 +39,6 @@ public InformerWrapper(SharedIndexInformer informer, String namespaceIdentifi this.informer = informer; this.namespaceIdentifier = namespaceIdentifier; this.cache = (Cache) informer.getStore(); - } @Override diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java new file mode 100644 index 0000000000..e26ac5bad5 --- /dev/null +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java @@ -0,0 +1,69 @@ +package io.javaoperatorsdk.operator.processing.event.source.informer; + +import java.util.concurrent.ConcurrentHashMap; +import java.util.function.Function; +import java.util.stream.Stream; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.Cache; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; + +public class ObjectTransformingItemStore implements ItemStore { + + private Function keyFunction; + private Function transformationFunction; + private ConcurrentHashMap store = new ConcurrentHashMap<>(); + + public ObjectTransformingItemStore(Function transformationFunction) { + this(Cache::metaNamespaceKeyFunc, transformationFunction); + } + + public ObjectTransformingItemStore(Function keyFunction, + Function transformationFunction) { + this.keyFunction = keyFunction; + this.transformationFunction = transformationFunction; + } + + @Override + public String getKey(R obj) { + return keyFunction.apply(obj); + } + + @Override + public R put(String key, R obj) { + var transformed = transformationFunction.apply(obj); + // resource must be always stored. + transformed.getMetadata().setResourceVersion(obj.getMetadata().getResourceVersion()); + return store.put(key, transformed); + } + + @Override + public R remove(String key) { + return store.remove(key); + } + + @Override + public Stream keySet() { + return store.keySet().stream(); + } + + @Override + public Stream values() { + return store.values().stream(); + } + + @Override + public R get(String key) { + return store.get(key); + } + + @Override + public int size() { + return store.size(); + } + + @Override + public boolean isFullState() { + return false; + } +} From f43fa22b85353d4c7572c21651048837afb55a92 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 24 Nov 2022 13:36:27 +0100 Subject: [PATCH 03/23] wip --- .../AnnotationControllerConfiguration.java | 9 +++++++++ .../config/ControllerConfigurationOverrider.java | 10 +++++++++- .../config/DefaultControllerConfiguration.java | 8 ++++++-- .../api/config/DefaultResourceConfiguration.java | 16 ++++++++++++++-- .../api/config/ResourceConfiguration.java | 5 +++++ .../config/informer/InformerConfiguration.java | 9 ++++----- .../api/reconciler/ControllerConfiguration.java | 3 +++ .../operator/ControllerManagerTest.java | 2 +- .../event/source/ResourceEventFilterTest.java | 2 +- .../ControllerResourceEventSourceTest.java | 2 +- 10 files changed, 53 insertions(+), 13 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 43c61319ac..6614f69b9d 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 @@ -11,6 +11,7 @@ import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; @@ -83,6 +84,14 @@ public Set getNamespaces() { DEFAULT_NAMESPACES_SET.toArray(String[]::new))); } + @Override + @SuppressWarnings("unchecked") + public Optional> itemStore() { + return Optional.ofNullable( + Utils.instantiate(annotation.itemStore(), ItemStore.class, + Utils.contextFor(this, null, null))); + } + @Override @SuppressWarnings("unchecked") public Class

getResourceClass() { 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 c36aa51d2e..004b127e1b 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 @@ -9,6 +9,7 @@ import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; @@ -38,6 +39,7 @@ public class ControllerConfigurationOverrider { private OnUpdateFilter onUpdateFilter; private GenericFilter genericFilter; private RateLimiter rateLimiter; + private ItemStore itemStore; private ControllerConfigurationOverrider(ControllerConfiguration original) { finalizer = original.getFinalizerName(); @@ -56,6 +58,7 @@ private ControllerConfigurationOverrider(ControllerConfiguration original) { dependentResources.forEach(drs -> namedDependentResourceSpecs.put(drs.getName(), drs)); this.original = original; this.rateLimiter = original.getRateLimiter(); + this.itemStore = original.itemStore().orElse(null); } public ControllerConfigurationOverrider withFinalizer(String finalizer) { @@ -158,6 +161,11 @@ public ControllerConfigurationOverrider withGenericFilter(GenericFilter ge return this; } + public ControllerConfigurationOverrider withItemStore(ItemStore itemStore) { + this.itemStore = itemStore; + return this; + } + @SuppressWarnings("unchecked") public ControllerConfigurationOverrider replacingNamedDependentResourceConfig(String name, Object dependentResourceConfig) { @@ -208,7 +216,7 @@ public ControllerConfiguration build() { onUpdateFilter, genericFilter, rateLimiter, - newDependentSpecs); + newDependentSpecs, itemStore); } public static ControllerConfigurationOverrider override( 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 3f4d952133..1dba841350 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 @@ -7,6 +7,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; @@ -49,8 +50,10 @@ public DefaultControllerConfiguration( OnUpdateFilter onUpdateFilter, GenericFilter genericFilter, RateLimiter rateLimiter, - List dependents) { - super(labelSelector, resourceClass, onAddFilter, onUpdateFilter, genericFilter, namespaces); + List dependents, + ItemStore itemStore) { + super(labelSelector, resourceClass, onAddFilter, onUpdateFilter, genericFilter, namespaces, + itemStore); this.associatedControllerClassName = associatedControllerClassName; this.name = name; this.crdName = crdName; @@ -116,4 +119,5 @@ public Optional maxReconciliationInterval() { public RateLimiter getRateLimiter() { return rateLimiter; } + } 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 9bc6ce5dba..d78b2f6ef3 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 @@ -4,6 +4,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; 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; @@ -19,18 +20,23 @@ public class DefaultResourceConfiguration private final OnAddFilter onAddFilter; private final OnUpdateFilter onUpdateFilter; private final GenericFilter genericFilter; + private final ItemStore itemStore; public DefaultResourceConfiguration(String labelSelector, Class resourceClass, OnAddFilter onAddFilter, OnUpdateFilter onUpdateFilter, GenericFilter genericFilter, String... namespaces) { this(labelSelector, resourceClass, onAddFilter, onUpdateFilter, genericFilter, namespaces == null || namespaces.length == 0 ? DEFAULT_NAMESPACES_SET - : Set.of(namespaces)); + : Set.of(namespaces), + null); } public DefaultResourceConfiguration(String labelSelector, Class resourceClass, OnAddFilter onAddFilter, - OnUpdateFilter onUpdateFilter, GenericFilter genericFilter, Set namespaces) { + OnUpdateFilter onUpdateFilter, + GenericFilter genericFilter, + Set namespaces, + ItemStore itemStore) { this.labelSelector = labelSelector; this.resourceClass = resourceClass; this.onAddFilter = onAddFilter; @@ -39,6 +45,7 @@ public DefaultResourceConfiguration(String labelSelector, Class resourceClass this.namespaces = namespaces == null || namespaces.isEmpty() ? DEFAULT_NAMESPACES_SET : namespaces; + this.itemStore = itemStore; } @Override @@ -56,6 +63,11 @@ public Set getNamespaces() { return namespaces; } + @Override + public Optional> itemStore() { + return Optional.ofNullable(this.itemStore); + } + @Override public Class getResourceClass() { return resourceClass; 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 90e18f3e52..1c306cf690 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 @@ -5,6 +5,7 @@ import java.util.Set; import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.reconciler.Constants; @@ -108,4 +109,8 @@ default Set getEffectiveNamespaces() { } return targetNamespaces; } + + default Optional> itemStore() { + 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 3ae0452c21..eb9b9fd39e 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 @@ -42,7 +42,8 @@ protected DefaultInformerConfiguration(String labelSelector, OnDeleteFilter onDeleteFilter, GenericFilter genericFilter, ItemStore itemStore) { - super(labelSelector, resourceClass, onAddFilter, onUpdateFilter, genericFilter, namespaces); + super(labelSelector, resourceClass, onAddFilter, onUpdateFilter, genericFilter, namespaces, + itemStore); this.followControllerNamespaceChanges = followControllerNamespaceChanges; this.primaryToSecondaryMapper = primaryToSecondaryMapper; @@ -73,8 +74,8 @@ public

PrimaryToSecondaryMapper

getPrimaryToSecondary } @Override - public ItemStore itemStore() { - return this.itemStore; + public Optional> itemStore() { + return Optional.ofNullable(this.itemStore); } } @@ -98,8 +99,6 @@ public ItemStore itemStore() {

PrimaryToSecondaryMapper

getPrimaryToSecondaryMapper(); - ItemStore itemStore(); - @SuppressWarnings("unused") class InformerConfigurationBuilder { 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 ec76adf89d..a4a4a78797 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 @@ -6,6 +6,7 @@ import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; @@ -118,4 +119,6 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation * accessible no-arg constructor. */ Class rateLimiter() default LinearRateLimiter.class; + + Class itemStore() default ItemStore.class; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java index d788f61e4a..94af637b38 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ControllerManagerTest.java @@ -58,7 +58,7 @@ private static class TestControllerConfiguration public TestControllerConfiguration(Reconciler controller, Class crClass) { super(null, getControllerName(controller), CustomResource.getCRDName(crClass), null, false, null, null, null, null, crClass, - null, null, null, null, null, null); + null, null, null, null, null, null, null); this.controller = controller; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java index 7cc5a20781..55ab40c173 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/ResourceEventFilterTest.java @@ -145,7 +145,7 @@ public ControllerConfig(String finalizer, boolean generationAware, eventFilter, customResourceClass, null, - null, null, null, null, null); + null, null, null, null, null, null); } } 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 00980743e0..d30a69d694 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 @@ -188,7 +188,7 @@ public TestConfiguration(boolean generationAware, OnAddFilter Date: Thu, 24 Nov 2022 16:23:33 +0100 Subject: [PATCH 04/23] controller limit cache size --- .../event/ReconciliationDispatcher.java | 30 ++++++++++++------- .../source/informer/InformerManager.java | 2 +- .../event/ReconciliationDispatcherTest.java | 15 ++++++---- .../webpage/src/main/resources/log4j2.xml | 2 +- 4 files changed, 31 insertions(+), 18 deletions(-) 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 015a2f5c31..5afd4489d9 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 @@ -10,6 +10,8 @@ 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; @@ -82,7 +84,7 @@ private PostExecutionControl

handleDispatch(ExecutionScope

executionScope) Context

context = new DefaultContext<>(executionScope.getRetryInfo(), controller, originalResource); if (markedForDeletion) { - return handleCleanup(resourceForExecution, context); + return handleCleanup(originalResource, resourceForExecution, context); } else { return handleReconcile(executionScope, resourceForExecution, originalResource, context); } @@ -109,7 +111,8 @@ private PostExecutionControl

handleReconcile( * finalizer add. This will make sure that the resources are not created before there is a * finalizer. */ - var updatedResource = updateCustomResourceWithFinalizer(originalResource); + var updatedResource = + updateCustomResourceWithFinalizer(resourceForExecution, originalResource); return PostExecutionControl.onlyFinalizerAdded(updatedResource); } else { try { @@ -276,7 +279,8 @@ private void updatePostExecutionControlWithReschedule( } - private PostExecutionControl

handleCleanup(P resource, Context

context) { + private PostExecutionControl

handleCleanup(P originalResource, P resource, + Context

context) { log.debug( "Executing delete for resource: {} with version: {}", getName(resource), @@ -289,7 +293,7 @@ private PostExecutionControl

handleCleanup(P resource, Context

context) { // cleanup is finished, nothing left to done final var finalizerName = configuration().getFinalizerName(); if (deleteControl.isRemoveFinalizer() && resource.hasFinalizer(finalizerName)) { - P customResource = removeFinalizer(resource, finalizerName); + P customResource = removeFinalizer(resource, originalResource, finalizerName); return PostExecutionControl.customResourceFinalizerRemoved(customResource); } } @@ -304,11 +308,12 @@ private PostExecutionControl

handleCleanup(P resource, Context

context) { return postExecutionControl; } - private P updateCustomResourceWithFinalizer(P resource) { + private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalResource) { log.debug( - "Adding finalizer for resource: {} version: {}", getUID(resource), getVersion(resource)); - resource.addFinalizer(configuration().getFinalizerName()); - return customResourceFacade.updateResource(resource); + "Adding finalizer for resource: {} version: {}", getUID(originalResource), + getVersion(originalResource)); + resourceForExecution.addFinalizer(configuration().getFinalizerName()); + return customResourceFacade.patchLockResource(resourceForExecution, originalResource); } private P updateCustomResource(P resource) { @@ -321,7 +326,7 @@ ControllerConfiguration

configuration() { return controller.getConfiguration(); } - public P removeFinalizer(P resource, String finalizer) { + public P removeFinalizer(P resource, P originalResource, String finalizer) { if (log.isDebugEnabled()) { log.debug("Removing finalizer on resource: {}", ResourceID.fromResource(resource)); } @@ -332,7 +337,7 @@ public P removeFinalizer(P resource, String finalizer) { if (!removed) { return resource; } - return customResourceFacade.updateResource(resource); + return customResourceFacade.patchLockResource(originalResource, resource); } catch (KubernetesClientException e) { log.trace("Exception during finalizer removal for resource: {}", resource); retryIndex++; @@ -370,12 +375,15 @@ public R getResource(String namespace, String name) { } } + public R patchLockResource(R resource, R originalResource) { + return resource(resource).patch(PatchContext.of(PatchType.JSON), originalResource); + } + public R updateResource(R resource) { log.debug( "Trying to replace resource {}, version: {}", getName(resource), resource.getMetadata().getResourceVersion()); - return resource(resource).lockResourceVersion(resource.getMetadata().getResourceVersion()) .replace(); } 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 b19341773f..fbdedb0b89 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 @@ -110,7 +110,7 @@ private InformerWrapper createEventSource( FilterWatchListDeletable, Resource> filteredBySelectorClient, ResourceEventHandler eventHandler, String namespaceIdentifier) { var informer = filteredBySelectorClient.runnableInformer(0); - // todo add custom item store + configuration.itemStore().ifPresent(informer::itemStore); var source = new InformerWrapper<>(informer, namespaceIdentifier); source.addEventHandler(eventHandler); 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 892fffcdbb..a3550af1e0 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 @@ -2,10 +2,19 @@ import java.time.Duration; import java.util.ArrayList; +import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; - +import java.util.function.UnaryOperator; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; +import io.fabric8.kubernetes.client.dsl.base.PatchContext; +import io.fabric8.kubernetes.client.dsl.base.PatchType; +import io.javaoperatorsdk.operator.api.config.*; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -22,10 +31,6 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.TestUtils; -import io.javaoperatorsdk.operator.api.config.Cloner; -import io.javaoperatorsdk.operator.api.config.ConfigurationServiceProvider; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.MockControllerConfiguration; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; diff --git a/sample-operators/webpage/src/main/resources/log4j2.xml b/sample-operators/webpage/src/main/resources/log4j2.xml index 5b794e7de3..3e92919d3b 100644 --- a/sample-operators/webpage/src/main/resources/log4j2.xml +++ b/sample-operators/webpage/src/main/resources/log4j2.xml @@ -2,7 +2,7 @@ - + From 436a5a5324ab363bc15e66c9c252e9c424064fd1 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 25 Nov 2022 10:52:23 +0100 Subject: [PATCH 05/23] wip --- .../event/ReconciliationDispatcher.java | 3 +- .../event/ReconciliationDispatcherTest.java | 35 +++++----- .../operator/ResourcePatchLockingIT.java | 65 +++++++++++++++++++ 3 files changed, 82 insertions(+), 21 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java 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 5afd4489d9..db8eaaa579 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 @@ -313,6 +313,7 @@ private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalRe "Adding finalizer for resource: {} version: {}", getUID(originalResource), getVersion(originalResource)); resourceForExecution.addFinalizer(configuration().getFinalizerName()); + // todo try repeatedly locally on error return customResourceFacade.patchLockResource(resourceForExecution, originalResource); } @@ -376,7 +377,7 @@ public R getResource(String namespace, String name) { } public R patchLockResource(R resource, R originalResource) { - return resource(resource).patch(PatchContext.of(PatchType.JSON), originalResource); + return resource(originalResource).patch(PatchContext.of(PatchType.JSON_MERGE), resource); } public R updateResource(R 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 a3550af1e0..7b55f53fb4 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 @@ -2,19 +2,10 @@ import java.time.Duration; import java.util.ArrayList; -import java.util.Map; import java.util.Optional; -import java.util.Set; import java.util.concurrent.TimeUnit; import java.util.function.BiFunction; -import java.util.function.UnaryOperator; -import io.fabric8.kubernetes.api.model.ConfigMap; -import io.fabric8.kubernetes.api.model.ConfigMapBuilder; -import io.fabric8.kubernetes.client.KubernetesClientBuilder; -import io.fabric8.kubernetes.client.dsl.base.PatchContext; -import io.fabric8.kubernetes.client.dsl.base.PatchType; -import io.javaoperatorsdk.operator.api.config.*; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; @@ -31,6 +22,7 @@ import io.javaoperatorsdk.operator.MockKubernetesClient; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.TestUtils; +import io.javaoperatorsdk.operator.api.config.*; import io.javaoperatorsdk.operator.api.reconciler.Cleaner; import io.javaoperatorsdk.operator.api.reconciler.Context; import io.javaoperatorsdk.operator.api.reconciler.DeleteControl; @@ -142,8 +134,9 @@ void addFinalizerOnNewResource() { verify(reconciler, never()) .reconcile(ArgumentMatchers.eq(testCustomResource), any()); verify(customResourceFacade, times(1)) - .updateResource( - argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER))); + .patchLockResource( + argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)), + any()); assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)).isTrue(); } @@ -223,7 +216,8 @@ void removesDefaultFinalizerOnDeleteIfSet() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertThat(postExecControl.isFinalizerRemoved()).isTrue(); - verify(customResourceFacade, times(1)).updateResource(testCustomResource); + verify(customResourceFacade, times(1)).patchLockResource(testCustomResource, + testCustomResource); } @Test @@ -232,7 +226,7 @@ void retriesFinalizerRemovalWithFreshResource() { markForDeletion(testCustomResource); var resourceWithFinalizer = TestUtils.testCustomResource(); resourceWithFinalizer.addFinalizer(DEFAULT_FINALIZER); - when(customResourceFacade.updateResource(testCustomResource)) + when(customResourceFacade.patchLockResource(testCustomResource, testCustomResource)) .thenThrow(new KubernetesClientException(null, 409, null)) .thenReturn(testCustomResource); when(customResourceFacade.getResource(any(), any())).thenReturn(resourceWithFinalizer); @@ -241,7 +235,7 @@ void retriesFinalizerRemovalWithFreshResource() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertThat(postExecControl.isFinalizerRemoved()).isTrue(); - verify(customResourceFacade, times(2)).updateResource(any()); + verify(customResourceFacade, times(2)).patchLockResource(any(), any()); verify(customResourceFacade, times(1)).getResource(any(), any()); } @@ -249,7 +243,7 @@ void retriesFinalizerRemovalWithFreshResource() { void throwsExceptionIfFinalizerRemovalRetryExceeded() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); markForDeletion(testCustomResource); - when(customResourceFacade.updateResource(any())) + when(customResourceFacade.patchLockResource(any(), any())) .thenThrow(new KubernetesClientException(null, 409, null)); when(customResourceFacade.getResource(any(), any())) .thenAnswer((Answer) invocationOnMock -> createResourceWithFinalizer()); @@ -261,7 +255,8 @@ void throwsExceptionIfFinalizerRemovalRetryExceeded() { assertThat(postExecControl.getRuntimeException()).isPresent(); assertThat(postExecControl.getRuntimeException().get()) .isInstanceOf(OperatorException.class); - verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY)).updateResource(any()); + verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY)).patchLockResource(any(), + any()); verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY - 1)).getResource(any(), any()); } @@ -270,7 +265,7 @@ void throwsExceptionIfFinalizerRemovalRetryExceeded() { void throwsExceptionIfFinalizerRemovalClientExceptionIsNotConflict() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); markForDeletion(testCustomResource); - when(customResourceFacade.updateResource(any())) + when(customResourceFacade.patchLockResource(any(), any())) .thenThrow(new KubernetesClientException(null, 400, null)); var res = @@ -278,7 +273,7 @@ void throwsExceptionIfFinalizerRemovalClientExceptionIsNotConflict() { assertThat(res.getRuntimeException()).isPresent(); assertThat(res.getRuntimeException().get()).isInstanceOf(KubernetesClientException.class); - verify(customResourceFacade, times(1)).updateResource(any()); + verify(customResourceFacade, times(1)).patchLockResource(any(), any()); verify(customResourceFacade, never()).getResource(any(), any()); } @@ -342,13 +337,13 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() { removeFinalizers(testCustomResource); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); - when(customResourceFacade.updateResource(any())).thenReturn(testCustomResource); + when(customResourceFacade.patchLockResource(any(), any())).thenReturn(testCustomResource); var postExecControl = reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertEquals(1, testCustomResource.getMetadata().getFinalizers().size()); - verify(customResourceFacade, times(1)).updateResource(any()); + verify(customResourceFacade, times(1)).patchLockResource(any(), any()); assertThat(postExecControl.updateIsStatusPatch()).isFalse(); assertThat(postExecControl.getUpdatedCustomResource()).isPresent(); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java new file mode 100644 index 0000000000..c2dd76ce42 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java @@ -0,0 +1,65 @@ +package io.javaoperatorsdk.operator; + +import java.util.Map; +import java.util.Set; + +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; +import io.fabric8.kubernetes.client.KubernetesClientBuilder; +import io.fabric8.kubernetes.client.dsl.base.PatchContext; +import io.fabric8.kubernetes.client.dsl.base.PatchType; +import io.javaoperatorsdk.operator.api.config.ConfigurationService; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.api.config.Version; +import io.javaoperatorsdk.operator.api.reconciler.Reconciler; + +// todo delete +class ResourcePatchLockingIT { + + @Disabled + @Test + void patchWithOptimisticLocking() { + var client = new KubernetesClientBuilder().build(); + var cm = new ConfigMap(); + cm.setMetadata(new ObjectMetaBuilder() + .withName("testpatch1") + .withNamespace("default") + .build()); + cm.setData(Map.of("key1", "val1")); + var created = client.configMaps().resource(cm).createOrReplace(); + + var modified = clone(created); + modified.getMetadata().setResourceVersion("1234567"); + modified.setData(Map.of("key2", "val2")); + var patched = client.configMaps().resource(created).patch(PatchContext.of(PatchType.JSON_MERGE), + modified); + System.out.println(patched); + } + + + private ConfigMap clone(ConfigMap cm) { + return new ConfigurationService() { + @Override + public ControllerConfiguration getConfigurationFor( + Reconciler reconciler) { + return null; + } + + @Override + public Set getKnownReconcilerNames() { + return null; + } + + @Override + public Version getVersion() { + return null; + } + }.getResourceCloner().clone(cm); + } + + +} From 795ed862a97566f49507286916e0bb06977d36bc Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 25 Nov 2022 12:55:30 +0100 Subject: [PATCH 06/23] patch with optimistick locking --- .../event/ReconciliationDispatcher.java | 22 +++++++++++-------- .../informer/ObjectTransformingItemStore.java | 5 +++-- 2 files changed, 16 insertions(+), 11 deletions(-) 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 db8eaaa579..7c8aba0dae 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 @@ -1,5 +1,7 @@ package io.javaoperatorsdk.operator.processing.event; +import java.util.function.Function; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -293,7 +295,8 @@ 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 = removeFinalizer(resource, originalResource, finalizerName); + P customResource = conflictRetryingPatch(resource, originalResource, + r -> r.removeFinalizer(finalizerName)); return PostExecutionControl.customResourceFinalizerRemoved(customResource); } } @@ -312,9 +315,9 @@ private P updateCustomResourceWithFinalizer(P resourceForExecution, P originalRe log.debug( "Adding finalizer for resource: {} version: {}", getUID(originalResource), getVersion(originalResource)); - resourceForExecution.addFinalizer(configuration().getFinalizerName()); - // todo try repeatedly locally on error - return customResourceFacade.patchLockResource(resourceForExecution, originalResource); + + return conflictRetryingPatch(resourceForExecution, originalResource, + r -> r.addFinalizer(configuration().getFinalizerName())); } private P updateCustomResource(P resource) { @@ -327,20 +330,21 @@ ControllerConfiguration

configuration() { return controller.getConfiguration(); } - public P removeFinalizer(P resource, P originalResource, String finalizer) { + public P conflictRetryingPatch(P resource, P originalResource, + Function modificationFunction) { if (log.isDebugEnabled()) { log.debug("Removing finalizer on resource: {}", ResourceID.fromResource(resource)); } int retryIndex = 0; while (true) { try { - var removed = resource.removeFinalizer(finalizer); - if (!removed) { + var modified = modificationFunction.apply(resource); + if (Boolean.FALSE.equals(modified)) { return resource; } return customResourceFacade.patchLockResource(originalResource, resource); } catch (KubernetesClientException e) { - log.trace("Exception during finalizer removal for resource: {}", resource); + log.trace("Exception during patch for resource: {}", resource); retryIndex++; // only retry on conflict (HTTP 409), otherwise fail if (e.getCode() != 409) { @@ -349,7 +353,7 @@ public P removeFinalizer(P resource, P originalResource, String finalizer) { if (retryIndex >= MAX_FINALIZER_REMOVAL_RETRY) { throw new OperatorException( "Exceeded maximum (" + MAX_FINALIZER_REMOVAL_RETRY - + ") retry attempts to remove finalizer '" + finalizer + "' for resource " + + ") retry attempts to patch resource: " + ResourceID.fromResource(resource)); } resource = customResourceFacade.getResource(resource.getMetadata().getNamespace(), diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java index e26ac5bad5..a678a5069a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java @@ -31,9 +31,10 @@ public String getKey(R obj) { @Override public R put(String key, R obj) { + var originalResourceVersion = obj.getMetadata().getResourceVersion(); var transformed = transformationFunction.apply(obj); - // resource must be always stored. - transformed.getMetadata().setResourceVersion(obj.getMetadata().getResourceVersion()); + // resourceVersion must be always stored. + transformed.getMetadata().setResourceVersion(originalResourceVersion); return store.put(key, transformed); } From c0f6a7ac6c07dfb560fdd778d2da68ffd698cdb5 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 25 Nov 2022 13:23:33 +0100 Subject: [PATCH 07/23] unit test --- .../event/ReconciliationDispatcherTest.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) 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 7b55f53fb4..8aab4ce28e 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 @@ -646,6 +646,24 @@ void canSkipSchedulingMaxDelayIf() { assertThat(control.getReScheduleDelay()).isNotPresent(); } + @Test + void retriesAddingFinalizer() { + removeFinalizers(testCustomResource); + reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); + when(customResourceFacade.patchLockResource(any(), any())) + .thenThrow(new KubernetesClientException(null, 409, null)) + .thenReturn(testCustomResource); + when(customResourceFacade.getResource(any(), any())) + .then((Answer) invocationOnMock -> { + testCustomResource.getFinalizers().clear(); + return testCustomResource; + }); + + reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); + + verify(customResourceFacade, times(2)).patchLockResource(any(), any()); + } + private ObservedGenCustomResource createObservedGenCustomResource() { ObservedGenCustomResource observedGenCustomResource = new ObservedGenCustomResource(); observedGenCustomResource.setMetadata(new ObjectMeta()); From 02730432b695a55829fc66d6c6c976f9f6a16d66 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 28 Nov 2022 10:20:09 +0100 Subject: [PATCH 08/23] using ssa --- .../event/ReconciliationDispatcher.java | 7 ++--- .../event/ReconciliationDispatcherTest.java | 26 ++++++++++--------- .../operator/ResourcePatchLockingIT.java | 19 +++++++++++--- 3 files changed, 33 insertions(+), 19 deletions(-) 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 7c8aba0dae..216b4baef7 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 @@ -342,7 +342,7 @@ public P conflictRetryingPatch(P resource, P originalResource, if (Boolean.FALSE.equals(modified)) { return resource; } - return customResourceFacade.patchLockResource(originalResource, resource); + return customResourceFacade.serverSideApplyLockResource(resource, originalResource); } catch (KubernetesClientException e) { log.trace("Exception during patch for resource: {}", resource); retryIndex++; @@ -380,8 +380,9 @@ public R getResource(String namespace, String name) { } } - public R patchLockResource(R resource, R originalResource) { - return resource(originalResource).patch(PatchContext.of(PatchType.JSON_MERGE), resource); + public R serverSideApplyLockResource(R resource, R originalResource) { + return resource(originalResource).patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY), + resource); } public R updateResource(R 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 8aab4ce28e..bdda20f08b 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 @@ -134,7 +134,7 @@ void addFinalizerOnNewResource() { verify(reconciler, never()) .reconcile(ArgumentMatchers.eq(testCustomResource), any()); verify(customResourceFacade, times(1)) - .patchLockResource( + .serverSideApplyLockResource( argThat(testCustomResource -> testCustomResource.hasFinalizer(DEFAULT_FINALIZER)), any()); assertThat(testCustomResource.hasFinalizer(DEFAULT_FINALIZER)).isTrue(); @@ -216,7 +216,7 @@ void removesDefaultFinalizerOnDeleteIfSet() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertThat(postExecControl.isFinalizerRemoved()).isTrue(); - verify(customResourceFacade, times(1)).patchLockResource(testCustomResource, + verify(customResourceFacade, times(1)).serverSideApplyLockResource(testCustomResource, testCustomResource); } @@ -226,7 +226,7 @@ void retriesFinalizerRemovalWithFreshResource() { markForDeletion(testCustomResource); var resourceWithFinalizer = TestUtils.testCustomResource(); resourceWithFinalizer.addFinalizer(DEFAULT_FINALIZER); - when(customResourceFacade.patchLockResource(testCustomResource, testCustomResource)) + when(customResourceFacade.serverSideApplyLockResource(testCustomResource, testCustomResource)) .thenThrow(new KubernetesClientException(null, 409, null)) .thenReturn(testCustomResource); when(customResourceFacade.getResource(any(), any())).thenReturn(resourceWithFinalizer); @@ -235,7 +235,7 @@ void retriesFinalizerRemovalWithFreshResource() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertThat(postExecControl.isFinalizerRemoved()).isTrue(); - verify(customResourceFacade, times(2)).patchLockResource(any(), any()); + verify(customResourceFacade, times(2)).serverSideApplyLockResource(any(), any()); verify(customResourceFacade, times(1)).getResource(any(), any()); } @@ -243,7 +243,7 @@ void retriesFinalizerRemovalWithFreshResource() { void throwsExceptionIfFinalizerRemovalRetryExceeded() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); markForDeletion(testCustomResource); - when(customResourceFacade.patchLockResource(any(), any())) + when(customResourceFacade.serverSideApplyLockResource(any(), any())) .thenThrow(new KubernetesClientException(null, 409, null)); when(customResourceFacade.getResource(any(), any())) .thenAnswer((Answer) invocationOnMock -> createResourceWithFinalizer()); @@ -255,7 +255,8 @@ void throwsExceptionIfFinalizerRemovalRetryExceeded() { assertThat(postExecControl.getRuntimeException()).isPresent(); assertThat(postExecControl.getRuntimeException().get()) .isInstanceOf(OperatorException.class); - verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY)).patchLockResource(any(), + verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY)).serverSideApplyLockResource( + any(), any()); verify(customResourceFacade, times(MAX_FINALIZER_REMOVAL_RETRY - 1)).getResource(any(), any()); @@ -265,7 +266,7 @@ void throwsExceptionIfFinalizerRemovalRetryExceeded() { void throwsExceptionIfFinalizerRemovalClientExceptionIsNotConflict() { testCustomResource.addFinalizer(DEFAULT_FINALIZER); markForDeletion(testCustomResource); - when(customResourceFacade.patchLockResource(any(), any())) + when(customResourceFacade.serverSideApplyLockResource(any(), any())) .thenThrow(new KubernetesClientException(null, 400, null)); var res = @@ -273,7 +274,7 @@ void throwsExceptionIfFinalizerRemovalClientExceptionIsNotConflict() { assertThat(res.getRuntimeException()).isPresent(); assertThat(res.getRuntimeException().get()).isInstanceOf(KubernetesClientException.class); - verify(customResourceFacade, times(1)).patchLockResource(any(), any()); + verify(customResourceFacade, times(1)).serverSideApplyLockResource(any(), any()); verify(customResourceFacade, never()).getResource(any(), any()); } @@ -337,13 +338,14 @@ void doesNotUpdateTheResourceIfNoUpdateUpdateControlIfFinalizerSet() { void addsFinalizerIfNotMarkedForDeletionAndEmptyCustomResourceReturned() { removeFinalizers(testCustomResource); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); - when(customResourceFacade.patchLockResource(any(), any())).thenReturn(testCustomResource); + when(customResourceFacade.serverSideApplyLockResource(any(), any())) + .thenReturn(testCustomResource); var postExecControl = reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); assertEquals(1, testCustomResource.getMetadata().getFinalizers().size()); - verify(customResourceFacade, times(1)).patchLockResource(any(), any()); + verify(customResourceFacade, times(1)).serverSideApplyLockResource(any(), any()); assertThat(postExecControl.updateIsStatusPatch()).isFalse(); assertThat(postExecControl.getUpdatedCustomResource()).isPresent(); } @@ -650,7 +652,7 @@ void canSkipSchedulingMaxDelayIf() { void retriesAddingFinalizer() { removeFinalizers(testCustomResource); reconciler.reconcile = (r, c) -> UpdateControl.noUpdate(); - when(customResourceFacade.patchLockResource(any(), any())) + when(customResourceFacade.serverSideApplyLockResource(any(), any())) .thenThrow(new KubernetesClientException(null, 409, null)) .thenReturn(testCustomResource); when(customResourceFacade.getResource(any(), any())) @@ -661,7 +663,7 @@ void retriesAddingFinalizer() { reconciliationDispatcher.handleExecution(executionScopeWithCREvent(testCustomResource)); - verify(customResourceFacade, times(2)).patchLockResource(any(), any()); + verify(customResourceFacade, times(2)).serverSideApplyLockResource(any(), any()); } private ObservedGenCustomResource createObservedGenCustomResource() { diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java index c2dd76ce42..d73a7191c3 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java @@ -17,6 +17,8 @@ import io.javaoperatorsdk.operator.api.config.Version; import io.javaoperatorsdk.operator.api.reconciler.Reconciler; +import static org.assertj.core.api.Assertions.assertThat; + // todo delete class ResourcePatchLockingIT { @@ -33,10 +35,19 @@ void patchWithOptimisticLocking() { var created = client.configMaps().resource(cm).createOrReplace(); var modified = clone(created); - modified.getMetadata().setResourceVersion("1234567"); - modified.setData(Map.of("key2", "val2")); - var patched = client.configMaps().resource(created).patch(PatchContext.of(PatchType.JSON_MERGE), - modified); + // modified.getMetadata().setResourceVersion("1234567"); + modified.addFinalizer("finalizer.com/finalizer"); + var patched = + client.configMaps().resource(created).patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY), + modified); + + var noFin = clone(patched); + noFin.removeFinalizer("finalizer.com/finalizer"); + patched = + client.configMaps().resource(patched).patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY), + noFin); + + assertThat(patched.getMetadata().getFinalizers()).isEmpty(); System.out.println(patched); } From a9a0a055d8cd00bf3073d85e72644596540f7c51 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 28 Nov 2022 12:32:16 +0100 Subject: [PATCH 09/23] wip --- .../informer/InformerConfiguration.java | 2 +- .../event/ReconciliationDispatcher.java | 5 +- .../operator/CachePruneIT.java | 4 + .../cacheprune/CachePruneCustomResource.java | 15 ++++ .../cacheprune/CachePruneReconciler.java | 82 +++++++++++++++++++ .../sample/cacheprune/CachePruneStatus.java | 15 ++++ 6 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneCustomResource.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneReconciler.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneStatus.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java index eb9b9fd39e..11f4a3e95b 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 @@ -213,7 +213,7 @@ public InformerConfigurationBuilder withGenericFilter(GenericFilter generi return this; } - public InformerConfigurationBuilder withOnDeleteFilter(ItemStore itemStore) { + public InformerConfigurationBuilder withItemStore(ItemStore itemStore) { this.itemStore = itemStore; return this; } 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 216b4baef7..e2e2856625 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 @@ -381,7 +381,10 @@ public R getResource(String namespace, String name) { } public R serverSideApplyLockResource(R resource, R originalResource) { - return resource(originalResource).patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY), + var patchContext = PatchContext.of(PatchType.SERVER_SIDE_APPLY); + // todo verify + patchContext.setForce(true); + return resource(originalResource).patch(patchContext, resource); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java new file mode 100644 index 0000000000..ba4592aa2d --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java @@ -0,0 +1,4 @@ +package io.javaoperatorsdk.operator; + +public class CachePruneIT { +} 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 new file mode 100644 index 0000000000..bf44809324 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneCustomResource.java @@ -0,0 +1,15 @@ +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 new file mode 100644 index 0000000000..634cc686da --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneReconciler.java @@ -0,0 +1,82 @@ +package io.javaoperatorsdk.operator.sample.cacheprune; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.config.informer.InformerConfiguration; +import io.javaoperatorsdk.operator.api.reconciler.*; +import io.javaoperatorsdk.operator.junit.KubernetesClientAware; +import io.javaoperatorsdk.operator.processing.event.ResourceID; +import io.javaoperatorsdk.operator.processing.event.source.EventSource; +import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; +import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; + +@ControllerConfiguration +public class CachePruneReconciler + implements Reconciler, EventSourceInitializer, + Cleaner, KubernetesClientAware { + + private final AtomicInteger numberOfExecutions = new AtomicInteger(0); + private KubernetesClient client; + + @Override + public UpdateControl reconcile( + CachePruneCustomResource resource, + Context context) { + numberOfExecutions.addAndGet(1); + + + + + return UpdateControl.noUpdate(); + } + + + public int getNumberOfExecutions() { + return numberOfExecutions.get(); + } + + @Override + public Map prepareEventSources( + EventSourceContext context) { + + InformerEventSource configMapEventSource = + new InformerEventSource(InformerConfiguration.from(ConfigMap.class,context).build(), + context); + return EventSourceInitializer.nameEventSources(configMapEventSource); + } + + ConfigMap configMap(String name, CachePruneCustomResource resource) { + ConfigMap configMap = new ConfigMap(); + configMap.setMetadata(new ObjectMeta()); + configMap.getMetadata().setName(name); + configMap.getMetadata().setNamespace(resource.getMetadata().getNamespace()); + configMap.setData(new HashMap<>()); + configMap.getData().put(name, name); + HashMap labels = new HashMap<>(); + labels.put("multisecondary", "true"); + 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 null; + } +} 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 new file mode 100644 index 0000000000..f361d9a06e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneStatus.java @@ -0,0 +1,15 @@ +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; + } +} From 04b5c1a5591326c8741627824963cf82bce7e940 Mon Sep 17 00:00:00 2001 From: csviri Date: Tue, 29 Nov 2022 10:02:01 +0100 Subject: [PATCH 10/23] wip --- .../informer/ObjectTransformingItemStore.java | 7 +- .../operator/CachePruneIT.java | 66 ++++++++++++++++++- .../cacheprune/CachePruneCustomResource.java | 2 +- .../cacheprune/CachePruneReconciler.java | 61 ++++++++++------- .../sample/cacheprune/CachePruneSpec.java | 15 +++++ .../sample/cacheprune/CachePruneStatus.java | 16 ++--- .../cacheprune/LabelRemovingItemStore.java | 14 ++++ 7 files changed, 147 insertions(+), 34 deletions(-) create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneSpec.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingItemStore.java diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java index a678a5069a..dcf296db7b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java @@ -31,9 +31,14 @@ public String getKey(R obj) { @Override public R put(String key, R obj) { + var originalName = obj.getMetadata().getName(); + var originalNamespace = obj.getMetadata().getNamespace(); var originalResourceVersion = obj.getMetadata().getResourceVersion(); + var transformed = transformationFunction.apply(obj); - // resourceVersion must be always stored. + + transformed.getMetadata().setName(originalName); + transformed.getMetadata().setNamespace(originalNamespace); transformed.getMetadata().setResourceVersion(originalResourceVersion); return store.put(key, transformed); } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java index ba4592aa2d..4396a1b4a7 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java @@ -1,4 +1,68 @@ package io.javaoperatorsdk.operator; -public class CachePruneIT { +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 { + // todo temp cache item store? + + 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(() -> { + 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, DEFAULT_DATA); + assertThat(configMap.getMetadata().getLabels()).isNotEmpty(); + }); + + res.getSpec().setData(UPDATED_DATA); + var updated = operator.replace(res); + + await().untilAsserted(() -> { + var actual = operator.get(CachePruneCustomResource.class, TEST_RESOURCE_NAME); + var configMap = operator.get(ConfigMap.class, TEST_RESOURCE_NAME); + assertThat(actual.getStatus().getCreated()).isTrue(); + assertThat(actual.getMetadata().getLabels()).isNotEmpty(); + assertThat(configMap.getData()).containsEntry(DATA_KEY, UPDATED_DATA); + 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 index bf44809324..60431588fd 100644 --- 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 @@ -10,6 +10,6 @@ @Version("v1") @ShortNames("cpr") public class CachePruneCustomResource - extends CustomResource + 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 index 634cc686da..d5d406d25f 100644 --- 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 @@ -1,26 +1,29 @@ package io.javaoperatorsdk.operator.sample.cacheprune; +import java.util.HashMap; +import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + 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.ResourceID; import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; -import io.javaoperatorsdk.operator.support.TestExecutionInfoProvider; - -import java.util.HashMap; -import java.util.Map; -import java.util.Set; -import java.util.concurrent.atomic.AtomicInteger; @ControllerConfiguration +// @ControllerConfiguration(itemStore = LabelRemovingItemStore.class) public class CachePruneReconciler - implements Reconciler, EventSourceInitializer, - Cleaner, KubernetesClientAware { + implements Reconciler, + EventSourceInitializer, + Cleaner, KubernetesClientAware { + public static final String DATA_KEY = "data"; private final AtomicInteger numberOfExecutions = new AtomicInteger(0); private KubernetesClient client; @@ -30,10 +33,21 @@ public UpdateControl reconcile( Context context) { numberOfExecutions.addAndGet(1); - - - - return UpdateControl.noUpdate(); + var configMap = context.getSecondaryResource(ConfigMap.class); + configMap.ifPresentOrElse(cm -> { + 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 res = client.configMaps().resource(cm) + .patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY), cloned); + System.out.println(res); + } + }, () -> client.configMaps().resource(configMap(resource)).create()); + + resource.setStatus(new CachePruneStatus()); + resource.getStatus().setCreated(true); + return UpdateControl.patchStatus(resource); } @@ -44,22 +58,22 @@ public int getNumberOfExecutions() { @Override public Map prepareEventSources( EventSourceContext context) { - InformerEventSource configMapEventSource = - new InformerEventSource(InformerConfiguration.from(ConfigMap.class,context).build(), - context); + new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context) + // .withItemStore(new LabelRemovingItemStore<>()) + .build(), + context); return EventSourceInitializer.nameEventSources(configMapEventSource); } - ConfigMap configMap(String name, CachePruneCustomResource resource) { + ConfigMap configMap(CachePruneCustomResource resource) { ConfigMap configMap = new ConfigMap(); configMap.setMetadata(new ObjectMeta()); - configMap.getMetadata().setName(name); + configMap.getMetadata().setName(resource.getMetadata().getName()); configMap.getMetadata().setNamespace(resource.getMetadata().getNamespace()); - configMap.setData(new HashMap<>()); - configMap.getData().put(name, name); + configMap.setData(Map.of(DATA_KEY, resource.getSpec().getData())); HashMap labels = new HashMap<>(); - labels.put("multisecondary", "true"); + labels.put("mylabel", "val"); configMap.getMetadata().setLabels(labels); configMap.addOwnerReference(resource); return configMap; @@ -76,7 +90,8 @@ public void setKubernetesClient(KubernetesClient kubernetesClient) { } @Override - public DeleteControl cleanup(CachePruneCustomResource resource, Context context) { - return null; + 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 new file mode 100644 index 0000000000..2d58a70d3a --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/CachePruneSpec.java @@ -0,0 +1,15 @@ +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 index f361d9a06e..a074c0e011 100644 --- 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 @@ -2,14 +2,14 @@ public class CachePruneStatus { - private Boolean created; + private Boolean created; - public Boolean getCreated() { - return created; - } + public Boolean getCreated() { + return created; + } - public CachePruneStatus setCreated(Boolean created) { - this.created = created; - return this; - } + public CachePruneStatus setCreated(Boolean created) { + this.created = created; + return this; + } } diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingItemStore.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingItemStore.java new file mode 100644 index 0000000000..3f63c2866e --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingItemStore.java @@ -0,0 +1,14 @@ +package io.javaoperatorsdk.operator.sample.cacheprune; + +import io.fabric8.kubernetes.api.model.HasMetadata; +import io.javaoperatorsdk.operator.processing.event.source.informer.ObjectTransformingItemStore; + +public class LabelRemovingItemStore extends ObjectTransformingItemStore { + + public LabelRemovingItemStore() { + super(r -> { + r.getMetadata().setLabels(null); + return r; + }); + } +} From 7e736c3dde4efcde3e7cf3e6e8dadb520a1bc43b Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 11:13:08 +0100 Subject: [PATCH 11/23] wip --- .../event/ReconciliationDispatcher.java | 1 - .../operator/CachePruneIT.java | 10 +++++ .../cacheprune/CachePruneReconciler.java | 40 +++++++++++++++---- 3 files changed, 42 insertions(+), 9 deletions(-) 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 e2e2856625..4a7ac68082 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 @@ -382,7 +382,6 @@ public R getResource(String namespace, String name) { public R serverSideApplyLockResource(R resource, R originalResource) { var patchContext = PatchContext.of(PatchType.SERVER_SIDE_APPLY); - // todo verify patchContext.setForce(true); return resource(originalResource).patch(patchContext, resource); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java index 4396a1b4a7..2ef4096229 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java @@ -49,9 +49,19 @@ void pruningRelatedBehavior() { var configMap = operator.get(ConfigMap.class, TEST_RESOURCE_NAME); assertThat(actual.getStatus().getCreated()).isTrue(); assertThat(actual.getMetadata().getLabels()).isNotEmpty(); + assertThat(actual.getMetadata().getFinalizers()).isNotEmpty(); assertThat(configMap.getData()).containsEntry(DATA_KEY, UPDATED_DATA); assertThat(configMap.getMetadata().getLabels()).isNotEmpty(); }); + + operator.delete(updated); + + await().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(); + }); } CachePruneCustomResource testResource() { 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 index d5d406d25f..487b4d648c 100644 --- 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 @@ -4,11 +4,14 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +import org.assertj.core.util.Lists; + import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.FieldsV1Builder; +import io.fabric8.kubernetes.api.model.ManagedFieldsEntryBuilder; 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.*; @@ -16,14 +19,15 @@ import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; -@ControllerConfiguration -// @ControllerConfiguration(itemStore = LabelRemovingItemStore.class) +@ControllerConfiguration(itemStore = LabelRemovingItemStore.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 final AtomicInteger numberOfExecutions = new AtomicInteger(0); private KubernetesClient client; @@ -39,11 +43,15 @@ public UpdateControl reconcile( .equals(resource.getSpec().getData())) { var cloned = ConfigurationServiceProvider.instance().getResourceCloner().clone(cm); cloned.getData().put(DATA_KEY, resource.getSpec().getData()); - var res = client.configMaps().resource(cm) - .patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY), cloned); - System.out.println(res); + var patchContext = patchContextWithFieldManager(FIELD_MANAGER); + // setting new field manager since we don't control label anymore + patchContext.setForce(true); + patchContext.setFieldManager(FIELD_MANAGER); + client.configMaps().resource(cm) + .patch(patchContext, cloned); } - }, () -> client.configMaps().resource(configMap(resource)).create()); + }, () -> client.configMaps().resource(configMap(resource)) + .patch(patchContextWithFieldManager(SECONDARY_CREATE_FIELD_MANAGER))); resource.setStatus(new CachePruneStatus()); resource.getStatus().setCreated(true); @@ -55,12 +63,18 @@ public int getNumberOfExecutions() { return numberOfExecutions.get(); } + private PatchContext patchContextWithFieldManager(String fieldManager) { + PatchContext patchContext = new PatchContext(); + patchContext.setFieldManager(fieldManager); + return patchContext; + } + @Override public Map prepareEventSources( EventSourceContext context) { InformerEventSource configMapEventSource = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context) - // .withItemStore(new LabelRemovingItemStore<>()) + .withItemStore(new LabelRemovingItemStore<>()) .build(), context); return EventSourceInitializer.nameEventSources(configMapEventSource); @@ -71,6 +85,16 @@ ConfigMap configMap(CachePruneCustomResource resource) { configMap.setMetadata(new ObjectMeta()); configMap.getMetadata().setName(resource.getMetadata().getName()); configMap.getMetadata().setNamespace(resource.getMetadata().getNamespace()); + configMap.getMetadata().setManagedFields(Lists.list(new ManagedFieldsEntryBuilder() + .withApiVersion("v1") + .withFieldsType("FieldsV1") + .withManager("fabric8") + .withOperation("Apply") + .withTime("2022-11-30T09:27:00.000Z") + .withFieldsV1(new FieldsV1Builder() + .withAdditionalProperties(Map.of("f:data", Map.of("f:data", Map.of()))) + .build()) + .build())); configMap.setData(Map.of(DATA_KEY, resource.getSpec().getData())); HashMap labels = new HashMap<>(); labels.put("mylabel", "val"); From 3a815600f8c315cf1709324567dda078e9923ffe Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 13:17:06 +0100 Subject: [PATCH 12/23] wip --- .../AnnotationControllerConfiguration.java | 6 +-- .../ControllerConfigurationOverrider.java | 12 ++--- .../DefaultControllerConfiguration.java | 6 +-- .../config/DefaultResourceConfiguration.java | 12 ++--- .../api/config/ResourceConfiguration.java | 4 +- .../informer/InformerConfiguration.java | 22 ++++---- .../reconciler/ControllerConfiguration.java | 4 +- .../source/informer/InformerManager.java | 3 +- .../informer/ManagedInformerEventSource.java | 4 +- .../informer/TemporaryResourceCache.java | 9 +++- ...mStore.java => TransformingItemStore.java} | 11 ++-- .../informer/TemporaryResourceCacheTest.java | 2 +- .../informer/TransformingItemStoreTest.java | 52 +++++++++++++++++++ .../cacheprune/CachePruneReconciler.java | 4 +- .../cacheprune/LabelRemovingItemStore.java | 14 ----- .../LabelRemovingPruneFunction.java | 13 +++++ 16 files changed, 120 insertions(+), 58 deletions(-) rename operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/{ObjectTransformingItemStore.java => TransformingItemStore.java} (82%) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStoreTest.java delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingItemStore.java create mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingPruneFunction.java 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 6614f69b9d..b996eeead9 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 @@ -8,10 +8,10 @@ import java.util.Optional; import java.util.Set; import java.util.function.Function; +import java.util.function.UnaryOperator; import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; @@ -86,9 +86,9 @@ public Set getNamespaces() { @Override @SuppressWarnings("unchecked") - public Optional> itemStore() { + public Optional> cachePruneFunction() { return Optional.ofNullable( - Utils.instantiate(annotation.itemStore(), ItemStore.class, + Utils.instantiate(annotation.cachePruneFunction(), UnaryOperator.class, Utils.contextFor(this, null, null))); } 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 004b127e1b..f78a74ad17 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,10 +6,10 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.UnaryOperator; import java.util.stream.Collectors; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.api.reconciler.dependent.managed.DependentResourceConfigurator; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; @@ -39,7 +39,7 @@ public class ControllerConfigurationOverrider { private OnUpdateFilter onUpdateFilter; private GenericFilter genericFilter; private RateLimiter rateLimiter; - private ItemStore itemStore; + private UnaryOperator cachePruneFunction; private ControllerConfigurationOverrider(ControllerConfiguration original) { finalizer = original.getFinalizerName(); @@ -58,7 +58,7 @@ private ControllerConfigurationOverrider(ControllerConfiguration original) { dependentResources.forEach(drs -> namedDependentResourceSpecs.put(drs.getName(), drs)); this.original = original; this.rateLimiter = original.getRateLimiter(); - this.itemStore = original.itemStore().orElse(null); + this.cachePruneFunction = original.cachePruneFunction().orElse(null); } public ControllerConfigurationOverrider withFinalizer(String finalizer) { @@ -161,8 +161,8 @@ public ControllerConfigurationOverrider withGenericFilter(GenericFilter ge return this; } - public ControllerConfigurationOverrider withItemStore(ItemStore itemStore) { - this.itemStore = itemStore; + public ControllerConfigurationOverrider withItemStore(UnaryOperator cachePruneFunction) { + this.cachePruneFunction = cachePruneFunction; return this; } @@ -216,7 +216,7 @@ public ControllerConfiguration build() { onUpdateFilter, genericFilter, rateLimiter, - newDependentSpecs, itemStore); + newDependentSpecs, cachePruneFunction); } public static ControllerConfigurationOverrider override( 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 1dba841350..f6277c4139 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 @@ -5,9 +5,9 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.UnaryOperator; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.config.dependent.DependentResourceSpec; import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; @@ -51,9 +51,9 @@ public DefaultControllerConfiguration( GenericFilter genericFilter, RateLimiter rateLimiter, List dependents, - ItemStore itemStore) { + UnaryOperator cachePruneFunction) { super(labelSelector, resourceClass, onAddFilter, onUpdateFilter, genericFilter, namespaces, - itemStore); + cachePruneFunction); this.associatedControllerClassName = associatedControllerClassName; this.name = name; this.crdName = crdName; 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 d78b2f6ef3..8e42284a25 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,9 +2,9 @@ import java.util.Optional; import java.util.Set; +import java.util.function.UnaryOperator; import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.client.informers.cache.ItemStore; 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; @@ -20,7 +20,7 @@ public class DefaultResourceConfiguration private final OnAddFilter onAddFilter; private final OnUpdateFilter onUpdateFilter; private final GenericFilter genericFilter; - private final ItemStore itemStore; + private final UnaryOperator cachePruneFunction; public DefaultResourceConfiguration(String labelSelector, Class resourceClass, OnAddFilter onAddFilter, @@ -36,7 +36,7 @@ public DefaultResourceConfiguration(String labelSelector, Class resourceClass OnUpdateFilter onUpdateFilter, GenericFilter genericFilter, Set namespaces, - ItemStore itemStore) { + UnaryOperator cachePruneFunction) { this.labelSelector = labelSelector; this.resourceClass = resourceClass; this.onAddFilter = onAddFilter; @@ -45,7 +45,7 @@ public DefaultResourceConfiguration(String labelSelector, Class resourceClass this.namespaces = namespaces == null || namespaces.isEmpty() ? DEFAULT_NAMESPACES_SET : namespaces; - this.itemStore = itemStore; + this.cachePruneFunction = cachePruneFunction; } @Override @@ -64,8 +64,8 @@ public Set getNamespaces() { } @Override - public Optional> itemStore() { - return Optional.ofNullable(this.itemStore); + public Optional> cachePruneFunction() { + return Optional.ofNullable(this.cachePruneFunction); } @Override 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 1c306cf690..d90ad100bc 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 @@ -3,9 +3,9 @@ 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.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.OperatorException; import io.javaoperatorsdk.operator.ReconcilerUtils; import io.javaoperatorsdk.operator.api.reconciler.Constants; @@ -110,7 +110,7 @@ default Set getEffectiveNamespaces() { return targetNamespaces; } - default Optional> itemStore() { + 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 11f4a3e95b..5fa6823225 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,9 +3,9 @@ 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.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.config.DefaultResourceConfiguration; import io.javaoperatorsdk.operator.api.config.ResourceConfiguration; import io.javaoperatorsdk.operator.api.config.Utils; @@ -30,7 +30,7 @@ class DefaultInformerConfiguration extends private final SecondaryToPrimaryMapper secondaryToPrimaryMapper; private final boolean followControllerNamespaceChanges; private final OnDeleteFilter onDeleteFilter; - private final ItemStore itemStore; + private final UnaryOperator cachePruneFunction; protected DefaultInformerConfiguration(String labelSelector, Class resourceClass, @@ -41,9 +41,9 @@ protected DefaultInformerConfiguration(String labelSelector, OnUpdateFilter onUpdateFilter, OnDeleteFilter onDeleteFilter, GenericFilter genericFilter, - ItemStore itemStore) { + UnaryOperator cachePruneFunction) { super(labelSelector, resourceClass, onAddFilter, onUpdateFilter, genericFilter, namespaces, - itemStore); + cachePruneFunction); this.followControllerNamespaceChanges = followControllerNamespaceChanges; this.primaryToSecondaryMapper = primaryToSecondaryMapper; @@ -51,7 +51,7 @@ protected DefaultInformerConfiguration(String labelSelector, Objects.requireNonNullElse(secondaryToPrimaryMapper, Mappers.fromOwnerReference()); this.onDeleteFilter = onDeleteFilter; - this.itemStore = itemStore; + this.cachePruneFunction = cachePruneFunction; } @Override @@ -74,8 +74,8 @@ public

PrimaryToSecondaryMapper

getPrimaryToSecondary } @Override - public Optional> itemStore() { - return Optional.ofNullable(this.itemStore); + public Optional> cachePruneFunction() { + return Optional.ofNullable(this.cachePruneFunction); } } @@ -112,7 +112,7 @@ class InformerConfigurationBuilder { private OnDeleteFilter onDeleteFilter; private GenericFilter genericFilter; private boolean inheritControllerNamespacesOnChange = false; - private ItemStore itemStore; + private UnaryOperator cachePruneFunction; private InformerConfigurationBuilder(Class resourceClass) { this.resourceClass = resourceClass; @@ -213,8 +213,8 @@ public InformerConfigurationBuilder withGenericFilter(GenericFilter generi return this; } - public InformerConfigurationBuilder withItemStore(ItemStore itemStore) { - this.itemStore = itemStore; + public InformerConfigurationBuilder withCachePruneFunction(UnaryOperator itemStore) { + this.cachePruneFunction = itemStore; return this; } @@ -223,7 +223,7 @@ public InformerConfiguration build() { primaryToSecondaryMapper, secondaryToPrimaryMapper, namespaces, inheritControllerNamespacesOnChange, onAddFilter, onUpdateFilter, - onDeleteFilter, genericFilter, itemStore); + onDeleteFilter, genericFilter, cachePruneFunction); } } 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 a4a4a78797..06e618e261 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,8 +5,8 @@ import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; +import java.util.function.UnaryOperator; -import io.fabric8.kubernetes.client.informers.cache.ItemStore; import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; import io.javaoperatorsdk.operator.processing.event.rate.LinearRateLimiter; import io.javaoperatorsdk.operator.processing.event.rate.RateLimiter; @@ -120,5 +120,5 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation */ Class rateLimiter() default LinearRateLimiter.class; - Class itemStore() default ItemStore.class; + Class cachePruneFunction() default UnaryOperator.class; } 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 fbdedb0b89..94788e6706 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 @@ -110,7 +110,8 @@ private InformerWrapper createEventSource( FilterWatchListDeletable, Resource> filteredBySelectorClient, ResourceEventHandler eventHandler, String namespaceIdentifier) { var informer = filteredBySelectorClient.runnableInformer(0); - configuration.itemStore().ifPresent(informer::itemStore); + 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 43173a28f7..4286335644 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 @@ -36,13 +36,15 @@ public abstract class ManagedInformerEventSource temporaryResourceCache = new TemporaryResourceCache<>(this); + protected TemporaryResourceCache temporaryResourceCache; protected InformerManager cache = new InformerManager<>(); protected C configuration; protected ManagedInformerEventSource( MixedOperation, Resource> client, C configuration) { super(configuration.getResourceClass()); + temporaryResourceCache = new TemporaryResourceCache<>(this, + configuration.cachePruneFunction().orElse(null)); 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 c0c041f3fb..8ae1a842cb 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,6 +3,7 @@ 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; @@ -34,11 +35,15 @@ 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) { + // todo prune + public TemporaryResourceCache(ManagedInformerEventSource managedInformerEventSource, + UnaryOperator cachePruneFunction) { this.managedInformerEventSource = managedInformerEventSource; + this.cachePruneFunction = cachePruneFunction; } public synchronized void removeResourceFromCache(T resource) { @@ -78,6 +83,8 @@ public synchronized void putUpdatedResource(T newResource, String previousResour } } + + public synchronized Optional getResourceFromCache(ResourceID resourceID) { return Optional.ofNullable(cache.get(resourceID)); } diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStore.java similarity index 82% rename from operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java rename to operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStore.java index dcf296db7b..60fdd32005 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ObjectTransformingItemStore.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStore.java @@ -2,24 +2,25 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; +import java.util.function.UnaryOperator; import java.util.stream.Stream; import io.fabric8.kubernetes.api.model.HasMetadata; import io.fabric8.kubernetes.client.informers.cache.Cache; import io.fabric8.kubernetes.client.informers.cache.ItemStore; -public class ObjectTransformingItemStore implements ItemStore { +public class TransformingItemStore implements ItemStore { private Function keyFunction; - private Function transformationFunction; + private UnaryOperator transformationFunction; private ConcurrentHashMap store = new ConcurrentHashMap<>(); - public ObjectTransformingItemStore(Function transformationFunction) { + public TransformingItemStore(UnaryOperator transformationFunction) { this(Cache::metaNamespaceKeyFunc, transformationFunction); } - public ObjectTransformingItemStore(Function keyFunction, - Function transformationFunction) { + public TransformingItemStore(Function keyFunction, + UnaryOperator transformationFunction) { this.keyFunction = keyFunction; this.transformationFunction = transformationFunction; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java index 2af66a4abe..0defd9efdc 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java @@ -18,7 +18,7 @@ class TemporaryResourceCacheTest { public static final String RESOURCE_VERSION = "1"; private InformerEventSource informerEventSource = mock(InformerEventSource.class); private TemporaryResourceCache temporaryResourceCache = - new TemporaryResourceCache<>(informerEventSource); + new TemporaryResourceCache<>(informerEventSource, null); @Test diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStoreTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStoreTest.java new file mode 100644 index 0000000000..9576e97a0e --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStoreTest.java @@ -0,0 +1,52 @@ +package io.javaoperatorsdk.operator.processing.event.source.informer; + +import java.util.Map; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; + +import static io.fabric8.kubernetes.client.informers.cache.Cache.metaNamespaceKeyFunc; +import static org.assertj.core.api.Assertions.assertThat; + +class TransformingItemStoreTest { + + @Test + void cachedObjectTransformed() { + TransformingItemStore transformingItemStore = new TransformingItemStore<>(r -> { + r.getMetadata().setLabels(null); + return r; + }); + + var cm = configMap(); + cm.getMetadata().setLabels(Map.of("k", "v")); + transformingItemStore.put(metaNamespaceKeyFunc(cm), cm); + + assertThat(transformingItemStore.get(metaNamespaceKeyFunc(cm)).getMetadata().getLabels()) + .isNull(); + } + + @Test + void preservesSelectedAttributes() { + TransformingItemStore transformingItemStore = new TransformingItemStore<>(r -> { + r.getMetadata().setName(null); + return r; + }); + var cm = configMap(); + transformingItemStore.put(metaNamespaceKeyFunc(cm), cm); + + assertThat(transformingItemStore.get(metaNamespaceKeyFunc(cm)).getMetadata().getName()) + .isNotNull(); + } + + ConfigMap configMap() { + var cm = new ConfigMap(); + cm.setMetadata(new ObjectMetaBuilder() + .withName("test1") + .withNamespace("default").withResourceVersion("1") + .build()); + return cm; + } + +} 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 index 487b4d648c..d421b1c901 100644 --- 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 @@ -19,7 +19,7 @@ import io.javaoperatorsdk.operator.processing.event.source.EventSource; import io.javaoperatorsdk.operator.processing.event.source.informer.InformerEventSource; -@ControllerConfiguration(itemStore = LabelRemovingItemStore.class) +@ControllerConfiguration(cachePruneFunction = LabelRemovingPruneFunction.class) public class CachePruneReconciler implements Reconciler, EventSourceInitializer, @@ -74,7 +74,7 @@ public Map prepareEventSources( EventSourceContext context) { InformerEventSource configMapEventSource = new InformerEventSource<>(InformerConfiguration.from(ConfigMap.class, context) - .withItemStore(new LabelRemovingItemStore<>()) + .withCachePruneFunction(new LabelRemovingPruneFunction<>()) .build(), context); return EventSourceInitializer.nameEventSources(configMapEventSource); diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingItemStore.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingItemStore.java deleted file mode 100644 index 3f63c2866e..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingItemStore.java +++ /dev/null @@ -1,14 +0,0 @@ -package io.javaoperatorsdk.operator.sample.cacheprune; - -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.javaoperatorsdk.operator.processing.event.source.informer.ObjectTransformingItemStore; - -public class LabelRemovingItemStore extends ObjectTransformingItemStore { - - public LabelRemovingItemStore() { - super(r -> { - r.getMetadata().setLabels(null); - return r; - }); - } -} 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 new file mode 100644 index 0000000000..a495803628 --- /dev/null +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/sample/cacheprune/LabelRemovingPruneFunction.java @@ -0,0 +1,13 @@ +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; + } +} From 570ef9db9b6d0d3d173751aa48ee43a82be3e3c3 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 13:42:10 +0100 Subject: [PATCH 13/23] tests --- .../informer/TemporaryResourceCache.java | 14 ++++++---- .../informer/TemporaryResourceCacheTest.java | 26 ++++++++++++++++++- 2 files changed, 34 insertions(+), 6 deletions(-) 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 8ae1a842cb..0af2ec0b2f 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 @@ -39,7 +39,6 @@ public class TemporaryResourceCache { private final Map cache = new ConcurrentHashMap<>(); private final ManagedInformerEventSource managedInformerEventSource; - // todo prune public TemporaryResourceCache(ManagedInformerEventSource managedInformerEventSource, UnaryOperator cachePruneFunction) { this.managedInformerEventSource = managedInformerEventSource; @@ -51,14 +50,14 @@ public synchronized void removeResourceFromCache(T resource) { } public synchronized void unconditionallyCacheResource(T newResource) { - cache.put(ResourceID.fromResource(newResource), newResource); + putToCache(newResource, null); } public synchronized void putAddedResource(T newResource) { ResourceID resourceID = ResourceID.fromResource(newResource); if (managedInformerEventSource.get(resourceID).isEmpty()) { log.debug("Putting resource to cache with ID: {}", resourceID); - cache.put(resourceID, newResource); + putToCache(newResource, resourceID); } else { log.debug("Won't put resource into cache found already informer cache: {}", resourceID); } @@ -75,7 +74,7 @@ public synchronized void putUpdatedResource(T newResource, String previousResour if (informerCacheResource.get().getMetadata().getResourceVersion() .equals(previousResourceVersion)) { log.debug("Putting resource to temporal cache with id: {}", resourceId); - cache.put(resourceId, newResource); + putToCache(newResource, resourceId); } else { // if something is in cache it's surely obsolete now log.debug("Trying to remove an obsolete resource from cache for id: {}", resourceId); @@ -83,7 +82,12 @@ 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); + } public synchronized Optional getResourceFromCache(ResourceID resourceID) { return Optional.ofNullable(cache.get(resourceID)); diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java index 0defd9efdc..9a51b95f32 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java @@ -1,7 +1,9 @@ package io.javaoperatorsdk.operator.processing.event.source.informer; +import java.util.Map; import java.util.Optional; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.ConfigMap; @@ -79,6 +81,26 @@ 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()); @@ -90,7 +112,9 @@ private ConfigMap propagateTestResourceToCache() { ConfigMap testResource() { ConfigMap configMap = new ConfigMap(); - configMap.setMetadata(new ObjectMeta()); + configMap.setMetadata(new ObjectMetaBuilder() + .withLabels(Map.of("k","v")) + .build()); configMap.getMetadata().setName("test"); configMap.getMetadata().setNamespace("default"); configMap.getMetadata().setResourceVersion(RESOURCE_VERSION); From e47f445315ff7c29186736d7a2a91247e7c4ed2b Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 13:43:33 +0100 Subject: [PATCH 14/23] format --- .../informer/TemporaryResourceCacheTest.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java index 9a51b95f32..2e77a557ea 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TemporaryResourceCacheTest.java @@ -3,11 +3,10 @@ import java.util.Map; import java.util.Optional; -import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import org.junit.jupiter.api.Test; import io.fabric8.kubernetes.api.model.ConfigMap; -import io.fabric8.kubernetes.api.model.ObjectMeta; +import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; import io.javaoperatorsdk.operator.processing.event.ResourceID; import static org.assertj.core.api.Assertions.assertThat; @@ -84,7 +83,10 @@ void removesResourceFromCache() { @Test void objectIsTransformedBeforePutIntoCache() { temporaryResourceCache = - new TemporaryResourceCache<>(informerEventSource, r->{r.getMetadata().setLabels(null);return r;}); + new TemporaryResourceCache<>(informerEventSource, r -> { + r.getMetadata().setLabels(null); + return r; + }); temporaryResourceCache.putAddedResource(testResource()); assertLabelsIsEmpty(temporaryResourceCache); @@ -98,7 +100,7 @@ void objectIsTransformedBeforePutIntoCache() { private void assertLabelsIsEmpty(TemporaryResourceCache temporaryResourceCache) { assertThat(temporaryResourceCache.getResourceFromCache(ResourceID.fromResource(testResource())) - .orElseThrow().getMetadata().getLabels()).isNull(); + .orElseThrow().getMetadata().getLabels()).isNull(); } private ConfigMap propagateTestResourceToCache() { @@ -113,8 +115,8 @@ private ConfigMap propagateTestResourceToCache() { ConfigMap testResource() { ConfigMap configMap = new ConfigMap(); configMap.setMetadata(new ObjectMetaBuilder() - .withLabels(Map.of("k","v")) - .build()); + .withLabels(Map.of("k", "v")) + .build()); configMap.getMetadata().setName("test"); configMap.getMetadata().setNamespace("default"); configMap.getMetadata().setResourceVersion(RESOURCE_VERSION); From ad83a5de6b3fb7b6bdb46878582397c8fcb7674e Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 13:48:40 +0100 Subject: [PATCH 15/23] rebase next --- .../processing/event/EventSourceManager.java | 1 - .../source/informer/InformerWrapper.java | 20 ------------------- 2 files changed, 21 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java index 7ebde8e5bf..6a6aae471a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java @@ -6,7 +6,6 @@ import java.util.Objects; import java.util.Set; import java.util.function.Function; -import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java index e870ceaa5e..17dc5cc969 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java @@ -179,24 +179,4 @@ public boolean isRunning() { public String getTargetNamespace() { return namespaceIdentifier; } - - @Override - public boolean hasSynced() { - return informer.hasSynced(); - } - - @Override - public boolean isWatching() { - return informer.isWatching(); - } - - @Override - public boolean isRunning() { - return informer.isRunning(); - } - - @Override - public String getTargetNamespace() { - return namespaceIdentifier; - } } From fdb38d6edf1a2f142997c3d5bdd859179b826702 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 14:07:23 +0100 Subject: [PATCH 16/23] cleanup --- .../operator/CachePruneIT.java | 30 ++++---- .../operator/ResourcePatchLockingIT.java | 76 ------------------- .../cacheprune/CachePruneReconciler.java | 34 +++------ 3 files changed, 24 insertions(+), 116 deletions(-) delete mode 100644 operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java index 2ef4096229..8bf8a64816 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java @@ -17,11 +17,11 @@ import static org.awaitility.Awaitility.await; class CachePruneIT { - // todo temp cache item store? 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() @@ -31,27 +31,14 @@ class CachePruneIT { void pruningRelatedBehavior() { var res = operator.create(testResource()); await().untilAsserted(() -> { - 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, DEFAULT_DATA); - assertThat(configMap.getMetadata().getLabels()).isNotEmpty(); + assertState(DEFAULT_DATA); }); res.getSpec().setData(UPDATED_DATA); var updated = operator.replace(res); await().untilAsserted(() -> { - var actual = operator.get(CachePruneCustomResource.class, TEST_RESOURCE_NAME); - var configMap = operator.get(ConfigMap.class, TEST_RESOURCE_NAME); - assertThat(actual.getStatus().getCreated()).isTrue(); - assertThat(actual.getMetadata().getLabels()).isNotEmpty(); - assertThat(actual.getMetadata().getFinalizers()).isNotEmpty(); - assertThat(configMap.getData()).containsEntry(DATA_KEY, UPDATED_DATA); - assertThat(configMap.getMetadata().getLabels()).isNotEmpty(); + assertState(UPDATED_DATA); }); operator.delete(updated); @@ -64,6 +51,17 @@ void pruningRelatedBehavior() { }); } + 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() diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java deleted file mode 100644 index d73a7191c3..0000000000 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/ResourcePatchLockingIT.java +++ /dev/null @@ -1,76 +0,0 @@ -package io.javaoperatorsdk.operator; - -import java.util.Map; -import java.util.Set; - -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Test; - -import io.fabric8.kubernetes.api.model.ConfigMap; -import io.fabric8.kubernetes.api.model.HasMetadata; -import io.fabric8.kubernetes.api.model.ObjectMetaBuilder; -import io.fabric8.kubernetes.client.KubernetesClientBuilder; -import io.fabric8.kubernetes.client.dsl.base.PatchContext; -import io.fabric8.kubernetes.client.dsl.base.PatchType; -import io.javaoperatorsdk.operator.api.config.ConfigurationService; -import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; -import io.javaoperatorsdk.operator.api.config.Version; -import io.javaoperatorsdk.operator.api.reconciler.Reconciler; - -import static org.assertj.core.api.Assertions.assertThat; - -// todo delete -class ResourcePatchLockingIT { - - @Disabled - @Test - void patchWithOptimisticLocking() { - var client = new KubernetesClientBuilder().build(); - var cm = new ConfigMap(); - cm.setMetadata(new ObjectMetaBuilder() - .withName("testpatch1") - .withNamespace("default") - .build()); - cm.setData(Map.of("key1", "val1")); - var created = client.configMaps().resource(cm).createOrReplace(); - - var modified = clone(created); - // modified.getMetadata().setResourceVersion("1234567"); - modified.addFinalizer("finalizer.com/finalizer"); - var patched = - client.configMaps().resource(created).patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY), - modified); - - var noFin = clone(patched); - noFin.removeFinalizer("finalizer.com/finalizer"); - patched = - client.configMaps().resource(patched).patch(PatchContext.of(PatchType.SERVER_SIDE_APPLY), - noFin); - - assertThat(patched.getMetadata().getFinalizers()).isEmpty(); - System.out.println(patched); - } - - - private ConfigMap clone(ConfigMap cm) { - return new ConfigurationService() { - @Override - public ControllerConfiguration getConfigurationFor( - Reconciler reconciler) { - return null; - } - - @Override - public Set getKnownReconcilerNames() { - return null; - } - - @Override - public Version getVersion() { - return null; - } - }.getResourceCloner().clone(cm); - } - - -} 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 index d421b1c901..491b591402 100644 --- 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 @@ -2,16 +2,12 @@ import java.util.HashMap; import java.util.Map; -import java.util.concurrent.atomic.AtomicInteger; - -import org.assertj.core.util.Lists; import io.fabric8.kubernetes.api.model.ConfigMap; -import io.fabric8.kubernetes.api.model.FieldsV1Builder; -import io.fabric8.kubernetes.api.model.ManagedFieldsEntryBuilder; 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.*; @@ -28,23 +24,26 @@ public class CachePruneReconciler public static final String DATA_KEY = "data"; public static final String FIELD_MANAGER = "controller"; public static final String SECONDARY_CREATE_FIELD_MANAGER = "creator"; - private final AtomicInteger numberOfExecutions = new AtomicInteger(0); private KubernetesClient client; @Override public UpdateControl reconcile( CachePruneCustomResource resource, Context context) { - numberOfExecutions.addAndGet(1); - var configMap = context.getSecondaryResource(ConfigMap.class); configMap.ifPresentOrElse(cm -> { + if (cm.getMetadata().getLabels() != null) { + 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 + // 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) @@ -58,13 +57,10 @@ public UpdateControl reconcile( return UpdateControl.patchStatus(resource); } - - public int getNumberOfExecutions() { - return numberOfExecutions.get(); - } - private PatchContext patchContextWithFieldManager(String fieldManager) { PatchContext patchContext = new PatchContext(); + // using server side apply + patchContext.setPatchType(PatchType.SERVER_SIDE_APPLY); patchContext.setFieldManager(fieldManager); return patchContext; } @@ -85,16 +81,6 @@ ConfigMap configMap(CachePruneCustomResource resource) { configMap.setMetadata(new ObjectMeta()); configMap.getMetadata().setName(resource.getMetadata().getName()); configMap.getMetadata().setNamespace(resource.getMetadata().getNamespace()); - configMap.getMetadata().setManagedFields(Lists.list(new ManagedFieldsEntryBuilder() - .withApiVersion("v1") - .withFieldsType("FieldsV1") - .withManager("fabric8") - .withOperation("Apply") - .withTime("2022-11-30T09:27:00.000Z") - .withFieldsV1(new FieldsV1Builder() - .withAdditionalProperties(Map.of("f:data", Map.of("f:data", Map.of()))) - .build()) - .build())); configMap.setData(Map.of(DATA_KEY, resource.getSpec().getData())); HashMap labels = new HashMap<>(); labels.put("mylabel", "val"); From 7d30c1e5b7ed21db14205fad8277056dff61124d Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 14:30:53 +0100 Subject: [PATCH 17/23] test fix --- docs/documentation/features.md | 2 +- .../operator/api/reconciler/ControllerConfiguration.java | 8 ++++++++ .../event/source/informer/TransformingItemStoreTest.java | 7 +++++++ .../operator/sample/cacheprune/CachePruneReconciler.java | 2 +- 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index b4cbd1f2fd..50b9fd7411 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -711,7 +711,7 @@ 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) -## Monitoring with Micrometer +## Optimization of Caches ## Automatic Generation of CRDs 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 06e618e261..d0d68c62e2 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 @@ -120,5 +120,13 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation */ Class rateLimiter() default LinearRateLimiter.class; + /** + * 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. + * + * @return function to remove parts of the resource. + */ Class cachePruneFunction() default UnaryOperator.class; } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStoreTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStoreTest.java index 9576e97a0e..3bebc79094 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStoreTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/TransformingItemStoreTest.java @@ -31,6 +31,8 @@ void cachedObjectTransformed() { void preservesSelectedAttributes() { TransformingItemStore transformingItemStore = new TransformingItemStore<>(r -> { r.getMetadata().setName(null); + r.getMetadata().setNamespace(null); + r.getMetadata().setResourceVersion(null); return r; }); var cm = configMap(); @@ -38,6 +40,11 @@ void preservesSelectedAttributes() { assertThat(transformingItemStore.get(metaNamespaceKeyFunc(cm)).getMetadata().getName()) .isNotNull(); + assertThat(transformingItemStore.get(metaNamespaceKeyFunc(cm)).getMetadata().getNamespace()) + .isNotNull(); + assertThat( + transformingItemStore.get(metaNamespaceKeyFunc(cm)).getMetadata().getResourceVersion()) + .isNotNull(); } ConfigMap configMap() { 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 index 491b591402..236b205c2c 100644 --- 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 @@ -32,7 +32,7 @@ public UpdateControl reconcile( Context context) { var configMap = context.getSecondaryResource(ConfigMap.class); configMap.ifPresentOrElse(cm -> { - if (cm.getMetadata().getLabels() != null) { + if (!cm.getMetadata().getLabels().isEmpty()) { throw new AssertionError("Labels should be null"); } if (!cm.getData().get(DATA_KEY) From c84178d3b9bd8b77f129fba2975113345f091b8b Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 14:52:08 +0100 Subject: [PATCH 18/23] test exp timeout --- docs/documentation/features.md | 2 ++ .../operator/api/config/ResourceConfiguration.java | 4 ++++ .../operator/api/reconciler/ControllerConfiguration.java | 3 +++ .../test/java/io/javaoperatorsdk/operator/CachePruneIT.java | 3 ++- 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 50b9fd7411..3741c5f636 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -713,6 +713,8 @@ See also an example implementation in the ## Optimization of Caches +In case of large clusters with huge amount of resources the memory consumption of an operator + ## 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/ResourceConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ResourceConfiguration.java index d90ad100bc..6a6574182a 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 @@ -9,6 +9,7 @@ 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; @@ -110,6 +111,9 @@ 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/reconciler/ControllerConfiguration.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ControllerConfiguration.java index d0d68c62e2..2d303a1d0d 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 @@ -125,6 +125,9 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation * 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. */ diff --git a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java index 8bf8a64816..9bf57ab372 100644 --- a/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java +++ b/operator-framework/src/test/java/io/javaoperatorsdk/operator/CachePruneIT.java @@ -1,5 +1,6 @@ package io.javaoperatorsdk.operator; +import java.time.Duration; import java.util.Map; import org.junit.jupiter.api.Test; @@ -43,7 +44,7 @@ void pruningRelatedBehavior() { operator.delete(updated); - await().untilAsserted(() -> { + 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(); From 88f76f67acaaa5c79b2d1b0a3ccc8cc23e5e5553 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 15:15:59 +0100 Subject: [PATCH 19/23] docs --- docs/documentation/features.md | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 3741c5f636..0505bca778 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -713,7 +713,17 @@ See also an example implementation in the ## Optimization of Caches -In case of large clusters with huge amount of resources the memory consumption of an operator +In case of large clusters with huge amount of resources the memory consumption of an operator. +In order to reduce the memory consumption both primary and secondary resources in cache can be pruned, thus only +partial objects will remain in memory. + +This has some implications regarding how those objects needs to be managed within a reconciler. Since from this point +reconciler will work only from partial object, all the updates on the pruned resources needs to be done by a PATCH +operations, thus just to send only the required changes. + +To see how to use, and how to handle related caveats regarding patches what utilizes +[server side apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) 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 reconciler. ## Automatic Generation of CRDs From d804850ad848c189de882dfd3edbc62fc239eba3 Mon Sep 17 00:00:00 2001 From: csviri Date: Wed, 30 Nov 2022 15:19:17 +0100 Subject: [PATCH 20/23] docs --- docs/documentation/features.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 0505bca778..334e15ab97 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -725,6 +725,8 @@ To see how to use, and how to handle related caveats regarding patches what util [server side apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) 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 reconciler. +Dependent Resources does not work with pruned caches for now. + ## Automatic Generation of CRDs Note that this feature is provided by the From a7c0d91426e2b5c6a2ad0cc6168a318ccf886d84 Mon Sep 17 00:00:00 2001 From: csviri Date: Thu, 1 Dec 2022 15:44:18 +0100 Subject: [PATCH 21/23] naming fixes --- .../api/config/ControllerConfigurationOverrider.java | 3 ++- .../operator/api/config/informer/InformerConfiguration.java | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) 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 f78a74ad17..1f5494050e 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 @@ -161,7 +161,8 @@ public ControllerConfigurationOverrider withGenericFilter(GenericFilter ge return this; } - public ControllerConfigurationOverrider withItemStore(UnaryOperator cachePruneFunction) { + public ControllerConfigurationOverrider withCachePruneFunction( + UnaryOperator cachePruneFunction) { this.cachePruneFunction = cachePruneFunction; return this; } 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 5fa6823225..03e37863df 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 @@ -213,8 +213,9 @@ public InformerConfigurationBuilder withGenericFilter(GenericFilter generi return this; } - public InformerConfigurationBuilder withCachePruneFunction(UnaryOperator itemStore) { - this.cachePruneFunction = itemStore; + public InformerConfigurationBuilder withCachePruneFunction( + UnaryOperator cachePruneFunction) { + this.cachePruneFunction = cachePruneFunction; return this; } From 72e4eee90af8e02dd4ff9a2872e1df2ed94d3286 Mon Sep 17 00:00:00 2001 From: Chris Laprun Date: Thu, 1 Dec 2022 14:23:58 +0100 Subject: [PATCH 22/23] fix: improve wording --- docs/documentation/features.md | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 334e15ab97..29aa54f93d 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -713,19 +713,25 @@ See also an example implementation in the ## Optimization of Caches -In case of large clusters with huge amount of resources the memory consumption of an operator. -In order to reduce the memory consumption both primary and secondary resources in cache can be pruned, thus only -partial objects will remain in memory. - -This has some implications regarding how those objects needs to be managed within a reconciler. Since from this point -reconciler will work only from partial object, all the updates on the pruned resources needs to be done by a PATCH -operations, thus just to send only the required changes. - -To see how to use, and how to handle related caveats regarding patches what utilizes -[server side apply](https://kubernetes.io/docs/reference/using-api/server-side-apply/) 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 reconciler. - -Dependent Resources does not work with pruned caches for now. +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 From 7b95b14114fe539451e675393383a964558f3a24 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 9 Dec 2022 12:37:36 +0100 Subject: [PATCH 23/23] docs update --- docs/documentation/features.md | 2 ++ .../operator/api/reconciler/ControllerConfiguration.java | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/docs/documentation/features.md b/docs/documentation/features.md index 29aa54f93d..d8e1cfa642 100644 --- a/docs/documentation/features.md +++ b/docs/documentation/features.md @@ -713,6 +713,8 @@ See also an example implementation in the ## 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 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 2d303a1d0d..df3efa7d40 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 @@ -121,14 +121,22 @@ MaxReconciliationInterval maxReconciliationInterval() default @MaxReconciliation 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;