charith-elastic
commented
4 years ago There are also code paths that update the object (e.g. add annotations for controller version, remote clusters, UUID) as a side effect. They don't cause the reconciliation loop to short-circuit and that results in conflict errors if any of the following reconciliation functions have modifications to apply to the object as well. This is not ideal for several reasons:
- Tracking down where the modification happens is not easy
- A single reconciliation run can trigger several requeues before all the work is complete. As there are several expensive operations in the reconciliation run, this results in increased resource usage and slowness.
- It might be possible to trigger a "reconciliation storm" under certain circumstances (intentionally or unintentionally)
The internal reconciliation loop has non-obvious side effects (events being queued into the state from code deep down in the tree) and wraps errors -- which makes it difficult to identify some error conditions that should be reported as events to the user (eg. certificate secret is corrupt). We should review the code here and ensure that errors and reportable events are consistently handled.
Related: #1478 #1717