Consider aligning Cluster API conditions to upstream conditions

[sbueringer]

A while back an upstream KEP was merged which tries to standardize conditions: https://github.com/kubernetes/enhancements/tree/master/keps/sig-api-machinery/1623-standardize-conditions
(corresponding implementation: kubernetes/kubernetes#90454)

The diff to our conditions is:

• upstream has an additional optional ObservedGeneration field
• Cluster API has an additional optional Severity field

I think it could be interesting to consider if we want to add the optional ObservedGeneration field to our Conditions.

godoc:

	// If set, this represents the .metadata.generation that the condition was set based upon.
	// For instance, if .metadata.generation is currently 12, but the .status.condition[x].observedGeneration is 9, the condition is out of date
	// with respect to the current state of the instance.
	// +optional
	ObservedGeneration int64 `json:"observedGeneration,omitempty" protobuf:"varint,3,opt,name=observedGeneration"`

Recently there was a thread about this in #sig-api-machinery https://kubernetes.slack.com/archives/C0EG7JC6T/p1666371350588619

/area api

[One or more /area label. See https://github.com/kubernetes-sigs/cluster-api/labels?q=area for the list of labels]

[sbueringer]

cc @fabriziopandini @dlipovetsky

[fabriziopandini]

/triage accepted
I have written https://docs.google.com/document/d/1hBQnWWa5d16FOslNhDwYVOhcMjLIul4tMeUgh4maI3w/edit some time ago with similar considerations, which is also linked by our meeting minutes in the open proposal section

[fabriziopandini]

/kind api-change

[k8s-triage-robot]

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

• Confirm that this issue is still relevant with /triage accepted (org members only)
• Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

[fabriziopandini]

/priority important-longterm

[fabriziopandini]

/triage accepted
/assign

It definetly makes sense.
I will take a look about starting getting rid of our limitation about only positive polarity + make a first assesment about other differences.
cc @theobarberbany

[fabriziopandini]

A few other differences worth to consider:

• Condition type names should make sense for humans; neither positive nor negative polarity can be recommended as a general rule (hopefully to be fixed with ✨ Add support negative polarity conditions #10550)
• Controllers should apply their conditions to a resource the first time they visit the resource, even if the status is Unknown (we don't do this now; also worth checking nuances in the API conventions)
• For state transitions which take a long period of time (e.g. more than 1 minute), it is reasonable to treat the transition itself as an observed state (we do this now, but worth checking nuances in the API conventions because this was not specified when we implemented CAPI conditions back in time)
• Use of the Reason field is required (we don't do this now when a condition have a positive state)

[sbueringer]

/close

This will be covered as part of #10897
(we are even going to use metav1.Condition directly)

[k8s-ci-robot]

@sbueringer: Closing this issue.

In response to this:

/close

This will be covered as part of #10897
(we are even going to use metav1.Condition directly)

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.