Skip to content

Commit f936a51

Browse files
authored
fix: dependent event handling with temp cache not cleared in some corner cases (#1994)
1 parent 077bbc2 commit f936a51

File tree

2 files changed

+39
-15
lines changed

2 files changed

+39
-15
lines changed

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java

+14-2
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,10 @@ protected R handleUpdate(R actual, R desired, P primary, Context<P> context) {
128128

129129
@SuppressWarnings("unused")
130130
public R create(R target, P primary, Context<P> context) {
131+
if (useSSA(context)) {
132+
// setting resource version for SSA so only created if it doesn't exist already
133+
target.getMetadata().setResourceVersion("1");
134+
}
131135
final var resource = prepare(target, primary, "Creating");
132136
return useSSA(context)
133137
? resource
@@ -138,15 +142,23 @@ public R create(R target, P primary, Context<P> context) {
138142
}
139143

140144
public R update(R actual, R target, P primary, Context<P> context) {
145+
if (log.isDebugEnabled()) {
146+
log.debug("Updating actual resource: {} version: {}", ResourceID.fromResource(actual),
147+
actual.getMetadata().getResourceVersion());
148+
}
149+
R updatedResource;
141150
if (useSSA(context)) {
142151
target.getMetadata().setResourceVersion(actual.getMetadata().getResourceVersion());
143-
return prepare(target, primary, "Updating")
152+
updatedResource = prepare(target, primary, "Updating")
144153
.fieldManager(context.getControllerConfiguration().fieldManager())
145154
.forceConflicts().serverSideApply();
146155
} else {
147156
var updatedActual = updaterMatcher.updateResource(actual, target, context);
148-
return prepare(updatedActual, primary, "Updating").replace();
157+
updatedResource = prepare(updatedActual, primary, "Updating").replace();
149158
}
159+
log.debug("Resource version after update: {}",
160+
updatedResource.getMetadata().getResourceVersion());
161+
return updatedResource;
150162
}
151163

152164
public Result<R> match(R actualResource, P primary, Context<P> context) {

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java

+25-13
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
* </p>
5252
* <br>
5353
* <p>
54-
* 2. Additional API is provided that is ment to be used with the combination of the previous one,
54+
* 2. Additional API is provided that is meant to be used with the combination of the previous one,
5555
* and the goal is to filter out events that are the results of updates and creates made by the
5656
* controller itself. For example if in reconciler a ConfigMaps is created, there should be an
5757
* Informer in place to handle change events of that ConfigMap, but since it has bean created (or
@@ -113,9 +113,9 @@ public InformerEventSource(InformerConfiguration<R> configuration, KubernetesCli
113113
@Override
114114
public void onAdd(R newResource) {
115115
if (log.isDebugEnabled()) {
116-
log.debug("On add event received for resource id: {} type: {}",
116+
log.debug("On add event received for resource id: {} type: {} version: {}",
117117
ResourceID.fromResource(newResource),
118-
resourceType().getSimpleName());
118+
resourceType().getSimpleName(), newResource.getMetadata().getResourceVersion());
119119
}
120120
primaryToSecondaryIndex.onAddOrUpdate(newResource);
121121
onAddOrUpdate(Operation.ADD, newResource, null,
@@ -125,9 +125,12 @@ public void onAdd(R newResource) {
125125
@Override
126126
public void onUpdate(R oldObject, R newObject) {
127127
if (log.isDebugEnabled()) {
128-
log.debug("On update event received for resource id: {} type: {}",
128+
log.debug(
129+
"On update event received for resource id: {} type: {} version: {} old version: {} ",
129130
ResourceID.fromResource(newObject),
130-
resourceType().getSimpleName());
131+
resourceType().getSimpleName(),
132+
newObject.getMetadata().getResourceVersion(),
133+
oldObject.getMetadata().getResourceVersion());
131134
}
132135
primaryToSecondaryIndex.onAddOrUpdate(newObject);
133136
onAddOrUpdate(Operation.UPDATE, newObject, oldObject,
@@ -282,17 +285,26 @@ private void handleRecentResourceOperationAndStopEventRecording(Operation operat
282285
log.debug(
283286
"Did not found event in buffer with target version and resource id: {}", resourceID);
284287
temporaryResourceCache.unconditionallyCacheResource(newResource);
285-
} else if (eventRecorder.containsEventWithVersionButItsNotLastOne(
286-
resourceID, newResource.getMetadata().getResourceVersion())) {
287-
R lastEvent = eventRecorder.getLastEvent(resourceID);
288-
log.debug(
289-
"Found events in event buffer but the target event is not last for id: {}. Propagating event.",
290-
resourceID);
291-
if (eventAcceptedByFilter(operation, newResource, oldResource)) {
292-
propagateEvent(lastEvent);
288+
} else {
289+
// if the resource is not added to the temp cache, it is cleared, since
290+
// the cache is cleared by subsequent events after updates, but if those did not receive
291+
// the temp cache is still filled at this point with an old resource
292+
log.debug("Cleaning temporary cache for resource id: {}", resourceID);
293+
temporaryResourceCache.removeResourceFromCache(newResource);
294+
if (eventRecorder.containsEventWithVersionButItsNotLastOne(
295+
resourceID, newResource.getMetadata().getResourceVersion())) {
296+
R lastEvent = eventRecorder.getLastEvent(resourceID);
297+
298+
log.debug(
299+
"Found events in event buffer but the target event is not last for id: {}. Propagating event.",
300+
resourceID);
301+
if (eventAcceptedByFilter(operation, newResource, oldResource)) {
302+
propagateEvent(lastEvent);
303+
}
293304
}
294305
}
295306
} finally {
307+
log.debug("Stopping event recording for: {}", resourceID);
296308
eventRecorder.stopEventRecording(resourceID);
297309
}
298310
}

0 commit comments

Comments
 (0)