Approach to exception handling in Dependent Resource reconcile()

[csviri]

There have been discussion if an exception should be throwed from DependentResource.reconcile() implementation in case of an error. Currently this exception is thrown from it. (And handled by the ErrorStatusHandler if implemented - otherwise just the pure retry functionality activated. )

This assures seamless integration of dependent resources (both managed and standalone mode). See in docs:
https://javaoperatorsdk.io/docs/features#setting-error-status-after-last-retry-attempt
https://javaoperatorsdk.io/docs/features#automatic-retries-on-error

So the idea behind this (global exception handling) that any exception happens during the reconciliation, there is little if nothing we can be done about it. So by default if an exception is thrown from the reconciler, a standard retry mechanism kicks in and we retry the whole reconciliation. With the ErrorStatusHandler we can basically set some status and/or prevent retry some known not recoverable errors.

An alternative approach coined by @metacosm could be that ReconcileResult
would contain the exception and a possible Erros status in it's enum.

In this issue we should discuss what would be the pros/cons of this appraoch. In what scenarios it would help and how.

[csviri]

My opinion is that as mentioned also before we should stick with the current model. Since as mentioned, I really cannot think of a scenario where this could help. However I'm open to discuss it.

On the cons side however:

• in standalone mode for example we will have to check each result for an error state, that would mean lot's of boilerplate code
• not sure how this would work with workflows ( see Workflows for Dependent Resource - Umbrella Issue #1097 ), so if there is a depends on relation, we should just go further an reconcile those, if an error happened on the depended resource.

But mainly I cannot think of any scenario that this approach is solving / can help.

[csviri]

On the side note: regarding the ReconcileResult TBH I would remove the whole enum. Basically since the reconciliation should be level based, realying on this information could lead to a bad pattern or confusion. Or at least not see how this actually should be used.

[metacosm]

On the side note: regarding the ReconcileResult TBH I would remove the whole enum. Basically since the reconciliation should be level based, realying on this information could lead to a bad pattern or confusion. Or at least not see how this actually should be used.

This information is only available within one reconciliation, though. Maybe we should rename the class differently but ReconcileResult just conveys to the main reconciler what happened with a dependent during the current reconciliation so it's not about keeping information from different reconciliations. To me, the confusion was added when we added the standalone mode and the reconcile method on DependentResource. If you name a method reconcile, then it's quite logical that it would return a ReconcileResult. 😄
It's also important for the workflow work that dependents can report their status back to the reconciler so that actions can be taken, not sure if the current form is enough for this, though…

[csviri]

It's also important for the workflow work that dependents can report their status back to the reconciler so that actions can be taken, not sure if the current form is enough for this, though…

So that is the main question, what action though. Originally it was Optional<Resource>, and basically this sits more with the level based approach, if the resource is there or not. If it is there we don't care if it was created or updated or non. What I mnean b basically no decision should be made based on that (just on the contents of the resource).

We should rather create a separate issue to discuss this part.

[csviri]

For now implemented this that the exception is thrown so the ErrorStatusHandler can handle it.
See: https://github.com/java-operator-sdk/java-operator-sdk/blob/cafe8ff3fc5dac7740d873e47bf4fba82a42de51/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java#L106-L106

All the reconcile information will be still available in the context:
https://github.com/java-operator-sdk/java-operator-sdk/blob/cafe8ff3fc5dac7740d873e47bf4fba82a42de51/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/dependent/managed/ManagedDependentResourceContext.java#L54-L56

In the status handler. We can reopen and discuss further in case.