KEP 1645: fix ServiceExport conditions

[MrFreezeex]

• One-line PR description: Fix examples and change required conditions types in ServiceExports

• Issue link: Multi-Cluster Services API #1645

• Other comments:

Followup fixes from https://github.com/kubernetes/enhancements/pull/5437/files#r2169361973

ServiceExport definitions are introduced in: kubernetes-sigs/mcs-api#112

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: MrFreezeex
Once this PR has been reviewed and has the lgtm label, please assign jeremyot for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• keps/sig-multicluster/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mikemorris]

I'd suggest just using a simple Ready reason for that condition type, and similarly for Valid. The more complex/interesting reasons are typically for standardizing the "something is wrong" cases such as {type: Valid, status: False} or {type: Conflicted, status: true}

There's substantial prior art on standardized reasons over in https://github.com/kubernetes-sigs/gateway-api/blob/main/apis/v1/gateway_types.go#L974-L1027 with explanation of positive (status: True is good, like for type: Ready) vs negative polarity condition types (status: False is good, such as for type: Conflicted paired with something like reason: NoConflicts) too.

[MrFreezeex]

I'd suggest just using a simple Ready reason for that condition type, and similarly for Valid. The more complex/interesting reasons are typically for standardizing the "something is wrong" cases such as {type: Valid, status: False} or {type: Conflicted, status: true}

There's substantial prior art on standardized reasons over in https://github.com/kubernetes-sigs/gateway-api/blob/main/apis/v1/gateway_types.go#L974-L1027 with explanation of positive (status: True is good, like for type: Ready) vs negative polarity condition types (status: False is good, such as for type: Conflicted paired with something like reason: NoConflicts) too.

Thanks for the link/suggestions! Hmm I guess then let's define those things in the MCS-API repo first and then fix up those example once done.

/hold

[mikemorris]

@MrFreezeex I think it's actually fine to show the intent/example in the KEP first if we add a caveat that this is just an example and we hope to standardize these condition types and reasons in the project eventually? I expect that may take a bit longer and don't want to block this.

[MrFreezeex]

@MrFreezeex I think it's actually fine to show the intent/example in the KEP first if we add a caveat that this is just an example and we hope to standardize these condition types and reasons in the project eventually? I expect that may take a bit longer and don't want to block this.

Well we would probably have around ~10 case to define so I don't think this should take that long if it takes too long we can unblock that but the KEP was broken for years on this matter it can stay broken for a few more week(s) probably IMO

[mikemorris]

Sounds good - conditions like this are extensible too so this shouldn't be a limiting factor. Implementations can add their own types or reasons (especially if they can have implementation-specific failure scenarios, and we add/promote these later if broadly applicable), but covering the core set of expected states/conflicts would be great.

(The message field is often impl-specific for adding more specific detail to standard reasons FWIW.)

[MrFreezeex]

I adapted this vs my current (non definitive so far ofc!) proposal here: kubernetes-sigs/mcs-api#112 to showcase what it would look like

[tpantelis]

/lgtm