discrepancies between Accepted, Ready, and Detached condition types and reasons

[mikemorris]

Several different Kubernetes meta/v1.Condition types are defined by the spec, and some use differing terminology to communicate apparently similar status.

What would you like to be added:

• Add Pending GatewayClassConditionReason, deprecate Waiting (for parity with ListenerConditionType)
  • Tracking in Add "Pending" reason and document usage for condition "Accepted", status: "Unknown" on all resources #1449
• Possibly add an Accepted GatewayConditionType, with Accepted, Invalid and Pending GatewayConditionReason to disambiguate the status of the object being accepted by the Gateway Controller versus being ready to route traffic.
  • GEP-1364 proposed replacing the current GatewayConditionScheduled with a new GatewayConditionAccepted
  • add GatewayConditionAccepted and GatewayReasonAccepted #1447
  • Invalid syntax should be caught by the webhook, so not proposing to add this currently.
• Add Attached or Accepted ListenerConditionType, deprecate Detached
  • Attached could still be a reasonable choice to clarify that this object isn't submitted to the controller individually, but rather evaluated as part of a Gateway object.
  • If choosing Accepted, add Accepted ListenerConditionReason, deprecate Attached
  • See "good" state for ListenerStatus feels logically inverted, difficult to understand #1110 for more detail
  • GEP-1364 proposed switching this to Accepted
  • add ListenerConditionAccepted, update docs from "Detached" #1446
• Add InvalidParameters ListenerConditionReason, prefer over more generic Invalid when applicable?
• Add Accepted RouteConditionReason, deprecate Admitted (for parity with GatewayClassConditionType)
  • Admitted was only in the v1alpha1 spec, and had already been removed in Another round of v1alpha2 cleanup #839
  • Documented Accepted properly in api: add RouteConditionReason to v1alpha2 #1114
• Possibly add Ready RouteConditionType, although this is known to be difficult to implement accurately.
  • Tracked in Indicate a new Route "Readiness" condition #1156
• Add Pending and Invalid or InvalidParameters RouteConditionReason, prefer over generic Refused when applicable?
  • Tracking Pending in Add "Pending" reason and document usage for condition "Accepted", status: "Unknown" on all resources #1449
  • Invalid syntax should be caught by the webhook, so not proposing to add this currently.

Why this is needed:

GatewayClassConditionType

"Accepted"

The possible false reasons indicate either that the controller is evaluating the GatewayClass still, or that it has been rejected and likely requires manual intervention to resolve. A Ready condition doesn't feel appropriate here as this isn't a concrete instance.

• true reasons
  • "Accepted"
• false reasons
  • "InvalidParameters"
  • "Waiting"

GatewayConditionType

"Ready"

This is pretty clearly intended to communicate different issues than the Accepted conditions, judging from the possible false values, the latter two of which seem to communicate issues that could be expected to be transient and eventually resolve on their own, or to look to a child object for additional detail.

• true reasons
  • "Ready"
• false reasons
  • “ListenersNotValid”
  • “ListenersNotReady”
  • “AddressNotAssigned”

ListenerConditionType

"Detached"

The logical inversion issues with this condition are separately described in #1110, but at a high level the possible true reasons seem to communicate issues that could be expected to require manual intervention to resolve.

• true reasons
  • “PortUnavailable”
  • “UnsupportedExtension”
  • “UnsupportedProtocol”
  • “UnsupportedAddress”
• false reasons
  • "Attached"

"Ready"

The possible false reasons for this field look quite similar to the false reasons for the Accepted GatewayClassConditionType, but use slightly different terminology. I'm not quite sure how useful this field is beyond being a cue to look to other conditions if the status is false.

• true reasons
  • "Ready"
• false reasons
  • “Invalid”
  • “Pending"

RouteConditionType

"Accepted"

The possible false reasons for this field are not currently defined in the v1alpha2 API spec docs but were previously defined in v1alpha1, although don't appear to offer any additional context beyond the status boolean.

• true reasons (only documented in v1alpha1)
  • "Admitted"
• false reasons (only documented in v1alpha1)
  • "Refused"

(If there's interest in enough of this changing, it could be substantial enough to warrant a GEP.)

[robscott]

We need to make sure this is compatible with Route inclusion status proposal

[mehabhalodiya]

I would like to work on this, if it's alright 🙂

[shaneutt]

@mehabhalodiya from what I've heard in Slack it sounds like you may not be taking this one on because you've got some other things you're working on? If that's the case I was gonna pick this one up, let me know what's up?

[mehabhalodiya]

@shaneutt I think that you already have some more context on what needs to be done; so that you can get it done a little quicker as compared to me!

Also @robscott and @youngnick would really like to get this sorted asap.

/assign @shaneutt

Thank you.

[shaneutt]

/assign @shaneutt

[shaneutt]

Turns out the ask here is somewhat large and after reviewing I do think a GEP might be warranted as suggested so that we can ensure consensus about the changes ahead of implementation. I'm not going to be able to work on this one after all as I have significant time constraints and several things competing for my time at this moment, so I will be un-assigning myself.

[carlisia]

Cross-posting here that I extracted the case for a "Ready" Route condition into a new issue.

[nathancoleman]

Thanks @carlisia ! I think that makes this issue much more manageable

[youngnick]

Moving this out of the v0.5.0 milestone so it doesn't block the release. We can add the reasons in a patch release afterward.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[youngnick]

I'm attempting to close this one as part of GEP-1364, so I'll take it.

/assign

[mikemorris]

GEP-1364 helped significantly clarify the expected behavior here, tracking followup work individually in:

• add ListenerConditionAccepted, update docs from "Detached" #1446
• add GatewayConditionAccepted and GatewayReasonAccepted #1447
• Add "Pending" reason and document usage for condition "Accepted", status: "Unknown" on all resources #1449
• Indicate a new Route "Readiness" condition #1156