KEP 1645: fix ServiceExport conditions

[MrFreezeex]

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

• Issue link: https://github.com/kubernetes/enhancements/issues/1645

• Other comments:

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

ServiceImport definitions are introduced in: https://github.com/kubernetes-sigs/mcs-api/pull/112

[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: https://github.com/kubernetes-sigs/mcs-api/pull/112 to showcase what it would look like

[tpantelis]

/lgtm

[lauralorenz]

Triage notes:

• Updates per a prior plan triage note in the mcs-api repo have been done as of https://github.com/kubernetes-sigs/mcs-api/pull/112#issuecomment-3161460719
• Ready for review again

[tpantelis]

/lgtm

[MrFreezeex]

(fixed a little typo because I realized I confused the status on the Condition field in this text :sweat_smile:)

[tpantelis]

/lgtm

[skitt]

/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MrFreezeex, skitt, tpantelis

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

The pull request process is described here

Needs approval from an approver in each of these files:

• ~~keps/sig-multicluster/OWNERS~~ [skitt]

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

[MrFreezeex]

/hold cancel