From ea90f9cce22e1d1b28c1b51b9daafd9c2cb3113c Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 4 Aug 2023 11:39:34 +0200 Subject: [PATCH 1/6] improvemetns: dependent event handling --- .../dependent/kubernetes/KubernetesDependentResource.java | 4 ++++ .../processing/event/source/informer/InformerEventSource.java | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 9a2603d54c..3477712ae8 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -128,6 +128,10 @@ protected R handleUpdate(R actual, R desired, P primary, Context

context) { @SuppressWarnings("unused") public R create(R target, P primary, Context

context) { + if (useSSA(context)) { + // setting resource version for SSA so only created if not exists yet + target.getMetadata().setResourceVersion("0"); + } final var resource = prepare(target, primary, "Creating"); return useSSA(context) ? resource diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index fdd8312670..af3a93460c 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -51,7 +51,7 @@ *

*
*

- * 2. Additional API is provided that is ment to be used with the combination of the previous one, + * 2. Additional API is provided that is meant to be used with the combination of the previous one, * and the goal is to filter out events that are the results of updates and creates made by the * controller itself. For example if in reconciler a ConfigMaps is created, there should be an * Informer in place to handle change events of that ConfigMap, but since it has bean created (or From 7e32737ff83637c6a3fe9186c2a2a689fca80da5 Mon Sep 17 00:00:00 2001 From: csviri Date: Fri, 4 Aug 2023 16:11:32 +0200 Subject: [PATCH 2/6] fix, logging improvements --- .../KubernetesDependentResource.java | 14 +++++-- .../source/informer/InformerEventSource.java | 37 +++++++++++++------ 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 3477712ae8..6f46d66e1b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -130,7 +130,7 @@ protected R handleUpdate(R actual, R desired, P primary, Context

context) { public R create(R target, P primary, Context

context) { if (useSSA(context)) { // setting resource version for SSA so only created if not exists yet - target.getMetadata().setResourceVersion("0"); + target.getMetadata().setResourceVersion("1"); } final var resource = prepare(target, primary, "Creating"); return useSSA(context) @@ -142,15 +142,23 @@ public R create(R target, P primary, Context

context) { } public R update(R actual, R target, P primary, Context

context) { + if (log.isDebugEnabled()) { + log.debug("Updating actual resource: {} version: {}", ResourceID.fromResource(actual), + actual.getMetadata().getResourceVersion()); + } + R updatedResource; if (useSSA(context)) { target.getMetadata().setResourceVersion(actual.getMetadata().getResourceVersion()); - return prepare(target, primary, "Updating") + updatedResource = prepare(target, primary, "Updating") .fieldManager(context.getControllerConfiguration().fieldManager()) .forceConflicts().serverSideApply(); } else { var updatedActual = updaterMatcher.updateResource(actual, target, context); - return prepare(updatedActual, primary, "Updating").replace(); + updatedResource = prepare(updatedActual, primary, "Updating").replace(); } + log.debug("Resource version after update: {}", + updatedResource.getMetadata().getResourceVersion()); + return updatedResource; } public Result match(R actualResource, P primary, Context

context) { diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index af3a93460c..f8aa4ee08a 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -113,9 +113,9 @@ public InformerEventSource(InformerConfiguration configuration, KubernetesCli @Override public void onAdd(R newResource) { if (log.isDebugEnabled()) { - log.debug("On add event received for resource id: {} type: {}", + log.debug("On add event received for resource id: {} type: {} version: {}", ResourceID.fromResource(newResource), - resourceType().getSimpleName()); + resourceType().getSimpleName(), newResource.getMetadata().getResourceVersion()); } primaryToSecondaryIndex.onAddOrUpdate(newResource); onAddOrUpdate(Operation.ADD, newResource, null, @@ -125,9 +125,12 @@ public void onAdd(R newResource) { @Override public void onUpdate(R oldObject, R newObject) { if (log.isDebugEnabled()) { - log.debug("On update event received for resource id: {} type: {}", + log.debug( + "On update event received for resource id: {} type: {} version: {} old version: {} ", ResourceID.fromResource(newObject), - resourceType().getSimpleName()); + resourceType().getSimpleName(), + newObject.getMetadata().getResourceVersion(), + oldObject.getMetadata().getResourceVersion()); } primaryToSecondaryIndex.onAddOrUpdate(newObject); onAddOrUpdate(Operation.UPDATE, newObject, oldObject, @@ -277,22 +280,32 @@ private void handleRecentResourceOperationAndStopEventRecording(Operation operat R newResource, R oldResource) { ResourceID resourceID = ResourceID.fromResource(newResource); try { + // what if the prev event delayed, thus that even did not clear the temp cache. if (!eventRecorder.containsEventWithResourceVersion( resourceID, newResource.getMetadata().getResourceVersion())) { log.debug( "Did not found event in buffer with target version and resource id: {}", resourceID); temporaryResourceCache.unconditionallyCacheResource(newResource); - } else if (eventRecorder.containsEventWithVersionButItsNotLastOne( - resourceID, newResource.getMetadata().getResourceVersion())) { - R lastEvent = eventRecorder.getLastEvent(resourceID); - log.debug( - "Found events in event buffer but the target event is not last for id: {}. Propagating event.", - resourceID); - if (eventAcceptedByFilter(operation, newResource, oldResource)) { - propagateEvent(lastEvent); + } else { + // if the resource is not added to the temp cache, it is cleared, since + // the cache is cleared by subsequent events after updates, but if those did not received + // the temp cache is still filled at this point with an old resource + log.debug("Cleaning temporal cache for resource id: {}", resourceID); + temporaryResourceCache.removeResourceFromCache(newResource); + if (eventRecorder.containsEventWithVersionButItsNotLastOne( + resourceID, newResource.getMetadata().getResourceVersion())) { + R lastEvent = eventRecorder.getLastEvent(resourceID); + + log.debug( + "Found events in event buffer but the target event is not last for id: {}. Propagating event.", + resourceID); + if (eventAcceptedByFilter(operation, newResource, oldResource)) { + propagateEvent(lastEvent); + } } } } finally { + log.debug("Stopping event recording for: {}", resourceID); eventRecorder.stopEventRecording(resourceID); } } From 5707737a362efa7faf621f76a5b96cbc408a4e38 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 7 Aug 2023 10:19:50 +0200 Subject: [PATCH 3/6] typo --- .../processing/event/source/informer/InformerEventSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index f8aa4ee08a..2bcc2dfe6e 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -288,7 +288,7 @@ private void handleRecentResourceOperationAndStopEventRecording(Operation operat temporaryResourceCache.unconditionallyCacheResource(newResource); } else { // if the resource is not added to the temp cache, it is cleared, since - // the cache is cleared by subsequent events after updates, but if those did not received + // the cache is cleared by subsequent events after updates, but if those did not receive // the temp cache is still filled at this point with an old resource log.debug("Cleaning temporal cache for resource id: {}", resourceID); temporaryResourceCache.removeResourceFromCache(newResource); From 398c47745c8433675f4ac158991da4920646f484 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 7 Aug 2023 10:46:39 +0200 Subject: [PATCH 4/6] Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java Co-authored-by: Chris Laprun --- .../dependent/kubernetes/KubernetesDependentResource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java index 6f46d66e1b..d03d96dee3 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java @@ -129,7 +129,7 @@ protected R handleUpdate(R actual, R desired, P primary, Context

context) { @SuppressWarnings("unused") public R create(R target, P primary, Context

context) { if (useSSA(context)) { - // setting resource version for SSA so only created if not exists yet + // setting resource version for SSA so only created if it doesn't exist already target.getMetadata().setResourceVersion("1"); } final var resource = prepare(target, primary, "Creating"); From fd085d825411fe21cc9d69ea76514e646bd23991 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Mon, 7 Aug 2023 10:48:45 +0200 Subject: [PATCH 5/6] Update operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java Co-authored-by: Chris Laprun --- .../processing/event/source/informer/InformerEventSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index 2bcc2dfe6e..caa948b96f 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -290,7 +290,7 @@ private void handleRecentResourceOperationAndStopEventRecording(Operation operat // if the resource is not added to the temp cache, it is cleared, since // the cache is cleared by subsequent events after updates, but if those did not receive // the temp cache is still filled at this point with an old resource - log.debug("Cleaning temporal cache for resource id: {}", resourceID); + log.debug("Cleaning temporary cache for resource id: {}", resourceID); temporaryResourceCache.removeResourceFromCache(newResource); if (eventRecorder.containsEventWithVersionButItsNotLastOne( resourceID, newResource.getMetadata().getResourceVersion())) { From bce444e66b9c46a98524fe648576450da7f57013 Mon Sep 17 00:00:00 2001 From: csviri Date: Mon, 7 Aug 2023 10:57:52 +0200 Subject: [PATCH 6/6] removed leftover comment --- .../processing/event/source/informer/InformerEventSource.java | 1 - 1 file changed, 1 deletion(-) diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java index caa948b96f..8cca524464 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java @@ -280,7 +280,6 @@ private void handleRecentResourceOperationAndStopEventRecording(Operation operat R newResource, R oldResource) { ResourceID resourceID = ResourceID.fromResource(newResource); try { - // what if the prev event delayed, thus that even did not clear the temp cache. if (!eventRecorder.containsEventWithResourceVersion( resourceID, newResource.getMetadata().getResourceVersion())) { log.debug(