Allow XRs and XRCs to record when they last published connection details #227
Conversation
This information is useful both for debugging XRs and XRCs, and for controllers to watch for updates to connection details (i.e. if those details were stored in an external system rather than in Secrets, which can themselves be watched). Signed-off-by: Nic Cope <negz@rk0n.org>
The claim controller in Crossplane core is the only place this machinery is still used, so it will be migrated into that controller. Signed-off-by: Nic Cope <negz@rk0n.org>
| v1alpha1.ConditionedStatus | ||
| ConnectionDetailsLastPublishedTimer |
There was a problem hiding this comment.
What do you think about having this as a Condition? I see that Equal check for condition doesn't include the last set time but maybe we can publish the time in the message. That could help keeping the schema simpler.
There was a problem hiding this comment.
I considered that - it would be nice but I decided not to because we need a timestamp of some kind (or at least a state change) and working that into a condition felt tricky from a UX perspective. Today we set lastTransitionTime on our conditions, but that represents the last time we transitioned between condition statuses so it's not a good fit. API conventions mention that we could support lastHeartbeatTime which is a better fit (it's described as the "last time we got an update on a given condition"), but still a little less explicit. i.e.
status:
connectionDetails:
lastPublishedTime: TCompared to:
status:
conditions:
- type: PublishedConnectionDetails
status: True
lastTransitionTime: T
lastHeartbeatTime: T+NIn the condition example it's not super clear to the user (IMO) whether the "last heartbeat time" is the time the connection details were last published, or just the last time we assessed whether we had published connection details.
Description of your changes
See crossplane/crossplane#1927 for details.
I have:
make reviewable testto ensure this PR is ready for review.How has this code been tested
See crossplane/crossplane#1927 for testing details.