Improve handling of updating resources together with statuses

[goldmann]

In case the workflow you implement requires to update the resource itself and the status sub-resource at the same time it cannot be handled in a single pass of the createOrUpdateResource() method.

To be able to handle it currently you need to:

1. Disable generationAwareEventProcessing:

   @Controller(crdName = "some.awesome.crd.com", generationAwareEventProcessing = false)

2. Handle update to the resource and finish with return UpdateControl.updateCustomResource(res). A new event will be published and another pass of the createOrUpdateResource() will be triggered.

3. Handle update to the status sub-resource and finish with return UpdateControl.updateStatusSubResource(res)

Ideally, we should just update the resource itself and the status sub-resource in a single UpdateControl.updateCustomResource(res) call. In other words - if there are status updates, these should be applied as well on the updated resource automatically.

[csviri]

I see there 2 ways of support this:

1. Sending instant events. Thus for example the TimerEventSource would have an addition, to send an event immediately. This will ensure that after a an update (either status sub-resource or CR) there will be a new controller execution for sure. This is a quick win, quite easy to implement.

2. Support this in update control with an explicit API (this way the generation processing can be still turned on). Where we will update both the custom resource and the status sub-resource. Note that these are 2 separate calls to kubernetes API, both includes optimistic locking. Thus in theory it can happen that after first call there is an update happening from a different parti, so the second call will fail on conflict. (Bot CR update and Status updates increases resource version in a CR). The best way to cover this is with retry, since the 3. party update is not necessary updates generation either (so there is not necessary an event from that). So in any case a retry config is advised.

I think we should do both.

@metacosm @psycho-ir @adam-sandor what is your opinion?

[csviri]

@metacosm currently I don't see how we could avoid doing optimistic locking on status subresource updates? fabric8 client seems to be doing this all the time. I think there should be an api to avoid that. (Hope K8S supports that).
Note that this is completely fine since only the controller should update status by definition.

See current code:https://github.com/java-operator-sdk/java-operator-sdk/blob/cfbbac3ab15ead086a1e8f0287c629c292f64e09/operator-framework/src/main/java/io/javaoperatorsdk/operator/processing/EventDispatcher.java#L183-L189

[s-soroosh]

The best way to cover this is with retry

What I think could be tricky here is that technically the status itself could be updated with a newer event. so retrying may override the new status with the old one.

[metacosm]

I see there 2 ways of support this:

1. Sending instant events. Thus for example the TimerEventSource would have an addition, to send an event immediately. This will ensure that after a an update (either status sub-resource or CR) there will be a new controller execution for sure. This is a quick win, quite easy to implement.

Re-triggering the method for simply a status update doesn't appear very friendly to me, especially because the createOrUpdateResource method might be costly.

2. Support this in update control with an explicit API (this way the generation processing can be still turned on). Where we will update both the custom resource and the status sub-resource. Note that these are 2 separate calls to kubernetes API, both includes optimistic locking. Thus in theory it can happen that after first call there is an update happening from a different parti, so the second call will fail on conflict. (Bot CR update and Status updates increases resource version in a CR). The best way to cover this is with retry, since the 3. party update is not necessary updates generation either (so there is not necessary an event from that). So in any case a retry config is advised.

If you're only updating the status, the generation shouldn't be updated so there shouldn't be a conflict on the status update call.

[s-soroosh]

If you're only updating the status, the generation shouldn't be updated so there shouldn't be a conflict on the status update call.

@metacosm
But the resourceVersion will be updated, right?

[metacosm]

If you're only updating the status, the generation shouldn't be updated so there shouldn't be a conflict on the status update call.

@metacosm
But the resourceVersion will be updated, right?

I would say no because you're not changing the spec (if you only update the status sub-resource, that is), hence not changing the resource itself but I have to double-check.

[metacosm]

In the go framework I previously implemented for operators, updating the status was a separate operation from the create/update process, where each resource associated with the main resource being reconciled was supposed to compute its own status based on the new state of the cluster after the create/update.

[csviri]

@metacosm the resource version is update if you update the status sub-resource.

Its always a separate method, since its a different API call for K8S.

[csviri]

The best way to cover this is with retry

What I think could be tricky here is that technically the status itself could be updated with a newer event. so retrying may override the new status with the old one.

This should not be the case, since the controller should update the status only. But this is more like a convention, its not enforced. On the other hand this is why it a sub-resource. So you don't override it, when you change the resource. (End the controller eventually updates it after execution of a resource change)

[metacosm]

@metacosm the resource version is update if you update the status sub-resource.

Are you sure? It doesn't make sense to me that the resource version would be updated when you update a sub-resource status. You're not changing the resource, just its status…

[csviri]

@metacosm the resource version is update if you update the status sub-resource.

Are you sure? It doesn't make sense to me that the resource version would be updated when you update a sub-resource status. You're not changing the resource, just its status…

Yes, it, does not make sense, but I remember double checking this some time ago.

[s-soroosh]

The best way to cover this is with retry

What I think could be tricky here is that technically the status itself could be updated with a newer event. so retrying may override the new status with the old one.

This should not be the case, since the controller should update the status only. But this is more like a convention, its not enforced. On the other hand this is why it a sub-resource. So you don't override it, when you change the resource. (End the controller eventually updates it after execution of a resource change)

OK, so if we guide the user to update the status subresource via the controller API, then it makes sense to retry on conflicts.