Expose "UpToDate" condition on all resources, not just during uptest
- Dominant language
- Go
- Stars
- 481
- Forks
- 131
- Avg merge
- 2d 1h
- Merged PRs (30d)
- 11
Description
### What problem are you facing?
Sometimes managed resources are stuck in a constant reconciliation loop. I've seen this due to several different causes:
* An error in the `spec.forProvider` that the cloud service corrects, but the terraform provider sees as a drift
* For example, omitting the `Version` in an aws IAM policy document json string
* An attribute which, due to the way the external cloud provider works, can only be applied if the resource is in a particular state
* For example, in an `Application.kinesisanalyticsv2.aws.upbound.io`, the `spec.forProvider.applicationConfiguration.runConfiguration.applicationRestoreConfiguration` can only be applied if the application is in a running state, and if `spec.startApplication` is false, it won't get into that state until someone starts it manually.
The only real indication of these problems is an unusually high number of `UpdatedExternalResource` events. It would be really helpful if upjet exposed a condition as to whether the latest reconciliation loop found a diff or not. This is basically what the current "Test" condition does, although that's only applied if the resource has an uptest-specific annotation on it.
### How could Upjet help solve your problem?
Rather than looking for the `upjet.upbound.io/test=true` annotation, exposing some version of this condition for all resources would be a useful way to improve visibility into whether a resource is actually in the desired state. I think we'd want a better name than `Test`, though, and a way to make the condition set to false when a diff is observed.
What about something like
```
Type: UpToDate
Reason: DiffDetected
Status: False
```
and
```
Type: UpToDate
Reason: NoDiffObserved
Status: True
```
I think eventually it would be great for upjet to apply some sort of exponential backoff when it's been observing the same diff for a long time, to reduce the number of ultimately-futile provider api calls, but that sounds like a much more complex feature to implement another time.
I read through the comments on https://github.com/crossplane/upjet/pull/23, where this was introduced, but one question I was still left wondering about was why this was originally limited to test resources only.
I would be willing to contribute this feature, if the maintainers think it's a good idea.
Contributor guide
Assessment
This issue has not been assessed yet.