fix: dependent event handling with temp cache not cleared in some corner cases#1994
fix: dependent event handling with temp cache not cleared in some corner cases#1994
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
| // 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); |
There was a problem hiding this comment.
@shawkins this will always clear the temp cache if not propagated.
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
csviri
changed the title
csviri
changed the title
...io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| public R update(R actual, R target, P primary, Context<P> context) { | ||
| if (log.isDebugEnabled()) { |
There was a problem hiding this comment.
Why check if debug login is enabled here and not elsewhere?
There was a problem hiding this comment.
Since creating a new object in that case: ResourceID.fromResource(actual). Want to do that only if really needed
| 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. |
There was a problem hiding this comment.
I don't understand this comment
There was a problem hiding this comment.
removed that, just probably some note for myself. good catch :)
...n/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java
Outdated
Show resolved
Hide resolved
…tor/processing/dependent/kubernetes/KubernetesDependentResource.java Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com>
…tor/processing/event/source/informer/InformerEventSource.java Co-authored-by: Chris Laprun <metacosm@users.noreply.github.com>
|
jFYI – this change seem to cause issues with K8s <1.23. But that should be ok as even 1.23 is quite old by now and unsupported. |
|
I guess our tests do not cover this change properly because this was tested without errors on 1.23… |
|
1.23 fixes it. You need to be on <1.23, we discovered it with 1.22. |
|
@vmuzikar could you pls create a new issue and provide some additional information, ideally also logs, but instructions to reproduce would help too. thx! |
|
I can create an issue but does JOSDK really support this old K8s versions? :) |
|
We are not testing it now, there is now also a discussion, that if we should support older versions, or just the versions what also Kubernetes supports. What is the strategy of KeyCloak on this @vmuzikar ? :) |
Formally ,we're aligned with the OCP lifecycle. The oldest supported version now will be 4.11 which translates to K8s 1.24. |
|
Thank you @vmuzikar |
|
The issue is actually already reported here (which I missed before): #2033 |
3 participants
No description provided.