gateway-api icon indicating copy to clipboard operation
gateway-api copied to clipboard

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

Open mikemorris opened this issue 2 years ago • 9 comments

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)
  • Possibly add an Accepted GatewayConditionType, with Accepted, InvalidParameters and/or Invalid and Pending GatewayConditionReason to disambiguate the status of the object being accepted by the Gateway Controller versus being ready to route traffic.
  • 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 https://github.com/kubernetes-sigs/gateway-api/issues/1110 for more detail
  • Add InvalidParameters ListenerConditionReason, prefer over more generic Invalid when applicable?
  • Add Accepted RouteConditionReason, deprecate Admitted (for parity with GatewayClassConditionType)
  • Possibly add Ready RouteConditionType, although this is known to be difficult to implement accurately.
  • Add Pending and InvalidParameters RouteConditionReason, prefer over generic Refused when applicable?
    • Possibly add Invalid RouteConditionReason, deprecate Refused (for parity with ListenerConditionReason)

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 https://github.com/kubernetes-sigs/gateway-api/issues/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.)

mikemorris avatar Apr 13 '22 19:04 mikemorris

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

robscott avatar Apr 18 '22 22:04 robscott

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

mehabhalodiya avatar Apr 20 '22 16:04 mehabhalodiya

@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?

shaneutt avatar May 04 '22 19:05 shaneutt

@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.

mehabhalodiya avatar May 04 '22 19:05 mehabhalodiya

/assign @shaneutt

shaneutt avatar May 04 '22 19:05 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.

shaneutt avatar May 09 '22 18:05 shaneutt

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

nathancoleman avatar May 10 '22 15:05 nathancoleman

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.

youngnick avatar May 19 '22 09:05 youngnick

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

k8s-triage-robot avatar Aug 17 '22 10:08 k8s-triage-robot

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

/assign

youngnick avatar Sep 15 '22 04:09 youngnick

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

  • https://github.com/kubernetes-sigs/gateway-api/pull/1446
  • https://github.com/kubernetes-sigs/gateway-api/pull/1447
  • https://github.com/kubernetes-sigs/gateway-api/issues/1449
  • https://github.com/kubernetes-sigs/gateway-api/issues/1156

mikemorris avatar Oct 10 '22 21:10 mikemorris