crossplane / crossplane/upjet

Expose "UpToDate" condition on all resources, not just during uptest

Open
#327 0 comments 0 reactions 0 assignees View on GitHub
enhancement v2
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.