Status updates occurring close together result in an exeception

[shawkins]

We have a custom resource with several operands. The current code will return UpdateControl.updateStatusSubResource on every call to ResourceController.createOrUpdateResource. When several operand updates are processed close together we sometimes see:

ERROR [io.jav.ope.pro.EventDispatcher] Error during event processing ExecutionScope
...
io.fabric8.kubernetes.client.KubernetesClientException: Failure executing: PUT
...
the object has been modified; please apply your changes to the latest version and try again

I need to run in debug more to check if the operator sdk is retrying this - if it is retrying, then can this be logged at debug. If it's not, then is this something that the operator or fabric8 should be handling?

[shawkins]

It does appear in debug that the retry handling is kicking in - the logic will look for any new events, and run the update again. So this does appear to be benign. @metacosm are you open to a change that would lower the amount of logging related to this exception?

[metacosm]

Yep.

[shawkins]

I've submitted a pr to change this to an info log without a stacktrace.

[metacosm]

Can we close this issue then? Or are there still things that need to be investigated?

[shawkins]

No, it seems that this is how it's expected to work. The only alternative/addition would be to add locking per resource being passed to createOrUpate such that you could serialize its processing - at least for the single operator case that would typically eliminate these conflicts.

[metacosm]

What do you think, @csviri?

[csviri]

Hi @metacosm and @shawkins
Not sure if I understand the situation correctly: We have already serialized processing in terms of operator for a custom resource. If I understand correctly what happens is that an other process(es) changes the CR frequently, that lead to an optimistick locking error.

By locking I guess you mean pessimistic locking. I'm not aware that kubernetes supports such thing. I think what can be done however is not do updates on status subresource using optimistic locking, since the controller is anyways the only process that should change status.
See:
https://github.com/java-operator-sdk/java-operator-sdk/blob/b4c6483ab3c4ddffcdf030681d03c4f3258556f8/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java#L203-L207

I think this needs to be changed in fabric8 client. @metacosm

[shawkins]

If I understand correctly what happens is that an other process(es) changes the CR frequently, that lead to an optimistick locking error.

The cr is not changing frequently. It does however have quite a few operands, which are event sources. My test to reproduce this will create the crs, sleep for a bit, then make an update to the crs that triggers the deletion of the operands. This is just a single operator, and a single process creating and then modify the spec of the crs.

From what I can now gather in the code and here, you do expect the updates against a single cr to effectively be serialized so that the cache update happens in time for the next batch of events. So the question becomes why in this scenario would there be a conflict? I see comments and checks in the caching logic https://github.com/java-operator-sdk/java-operator-sdk/blob/b4c6483ab3c4ddffcdf030681d03c4f3258556f8/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/DefaultEventHandler.java#L211 that it's expected for things to get out of sync - but presumably only when there are more actors.