Clarify language around listener compatibility

[rainest]

What would you like to be added:

Add language stating:

• That implementations MUST be able to serve all attached routes from every address listed under the Gateway's address status.
• That implementations MUST mark Gateways not Accepted if the requested listeners cannot all be provisioned on a single IP. The Conditions here are a bit unusual in that they're not all or nothing, and the spec permits accepting a Gateway while still marking it as having broken listeners
• Clarify listener compatibility language to indicate criteria implementations should use to determine whether listeners are compatible for them, in addition to the existing example that may be compatible for some.

Add a catch-all Gateway Accepted reason indicating that the Gateway is not accepted because the implementation or environment cannot support it.

Why this is needed:

The Gateway API supports freely adding any combination of listeners that satisfy general conflict avoidance rules. However, implementations may not necessarily be able to a listener set that has no conflicts.

Language around listener compatibility has some clarity issues, where it describes an example that may be compatible, but doesn't state reasons why it might not be compatible for a given implementation.

LoadBalancers that handle both TCP and UDP traffic may not be available:

• MixedProtocolLBService was not GA in Kubernetes <1.26)
• MixedProtocolLBService is not necessarily supported by cloud providers. For example EKS still has an open PR to support this and GKE has code that rejects mixed-protocol LoadBalancers

Implementations may be unable to support multiple protocols on the same port:

• NGINX-based implementations separate http and stream (L4) proxying and cannot serve the two on the same IP:Port combo. They cannot, therefore, support both HTTPS and TLS listeners on the same port (absent elaborate configuration to double-proxy through an initial stream layer to an http layer behind).
• I suspect, but do not know for sure, that cloud provider-integrated implementations may be unable to support mixed HTTPS and TLS, as they typically implement rich HTTP routing and filtering features in L7 load balancers that cannot process non-HTTP traffic.
• As a counterexample, Envoy explicitly does support mixed HTTPS and TLS.

Existing condition reasons do not distinguish between spec violations and implementation/environment incompatibility.

The additional Gateway reason distinguishes these cases from cases where the listeners are invalid. Listener configuration may be valid (compliant with the general anti-conflict rules) but not satisfiable due to implementation or environment limitations. On further review, the existing condition constants have language that makes them applicable to either, and we can get without additional and hope that implementations populate the message field well--we probably don't have an obvious use case for separating those reasons in kubectl filters, since you mostly just want to find listeners that have issues, and fewer Reasons makes that easier.

Meeting discussion indicated we may wish to add additional listener conditions as well, but I think the existing port and protocol not accepted conditions are probably sufficient. We should add an "implementation reasons" clause to the ListenerReasonUnsupportedProtocol comment, but I think implementation-chosen messages in combination with the new broader Gateway accepted reason should be expressive enough.

[rainest]

Although cloud provider-level limitations will block acceptance of some Gateways, it's not clear that controllers have any way of determining this. There appears to be no standard means of determining that a cloud provider cannot provision a LoadBalancer:

• Service status' LoadBalancerStatus only has room for positive information, to indicate an IP if the cloud provider has attached one. While absence of an IP may indicate a provisioning failure, it may also indicate that no load balancer provider is available or that it is still busy provisioning.
• GKE assigns a Condition, but it appears these standard error reasons were intentionally removed from upstream core. There is no obvious successor.

Although controllers cannot determine this issue reliably, this is probably of less concern because the Service will never receive an IP, so we'll never indicate that the Gateway is live (by adding IPs to its status) anyway.

[shaneutt]

/triage accepted

[rainest]

Went and read through the existing language more in depth and now think that we don't need additional Reasons. The existing ones do state they can be used for either protocol or implementation reasons, and that's maybe a murkier boundary than I initially thought anyway.

There are still clarity issues with the language around collapsing compatible Listeners:

• You don't (AFAIK) really collapse compatible Listeners as stated--they're still separate entities in config and status, and really need to be separate in status since that's where attached routes info resides. Truly collapsing them would presumably mean saying that a TLSRoute is attached to an HTTPS protocol Listener, which doesn't really make sense.
• We should clearly state that compatibility is at the discretion of the implementation, rather than a set of rules in the spec. Close reading does indicate that, but the prominence of the single example can make it seem like those are conditions that should generally be compatible
• We don't clearly state here how an implementation should determine if it can consider Listeners compatible. The single IP requirement is elsewhere, but easy to miss reading this section alone. Not sure if there are other good criteria to call out.
• The language around Hostname being a must condition for choosing the correct Listener gets a bit confusing given the mixture of L7 and L4 protocols. Can a UDP protocol Listener have a hostname? In the Listener struct, it can! and you could hypothetically direct traffic for it in some protocols. UDPRoute does not support hostnames, but we could hypothetically add some other route type that can attach to UDP listeners and does.

https://github.com/rainest/gateway-api/pull/1/files to stick similar notes inline.

[youngnick]

I agree with the goals of the issue, and with the conclusions you've got in the initial comment. But I think that the above comment is worth digging into a bit:

• You don't (AFAIK) really collapse compatible Listeners as stated--they're still separate entities in config and status, and really need to be separate in status since that's where attached routes info resides. Truly collapsing them would presumably mean saying that a TLSRoute is attached to an HTTPS protocol Listener, which doesn't really make sense.

I guess this is a "naming is hard" problem again - when we did this originally, we were all pretty clear that the rules were about "collapsing" Listeners from multiple Gateways, on the assumption that a single Gateway with incompatible Listeners would be rejected by the implementation with the Conditions and Reasons we describe. (You don't need to worry about creation time or object name in terms of Listener priority when everything is in the same Gateway, for example).

So yeah, "collapsing compatible Listeners" probably really should read "collapsing compatible Listeners between Gateways" instead.

• We should clearly state that compatibility is at the discretion of the implementation, rather than a set of rules in the spec. Close reading does indicate that, but the prominence of the single example can make it seem like those are conditions that should generally be compatible

Agreed. This should be clearly stated, as should the fact that it's okay for implementations not to support these combinations. We should also codify combinations like this with Extended conformance tests and feature names (as far as we can), so they'll end up in the final conformance report.

• We don't clearly state here how an implementation should determine if it can consider Listeners compatible. The single IP requirement is elsewhere, but easy to miss reading this section alone. Not sure if there are other good criteria to call out.

I think something like "here is a list of things that might make a set of Listeners incompatible, depending on the implementation. This list is not exhaustive".

• The language around Hostname being a must condition for choosing the correct Listener gets a bit confusing given the mixture of L7 and L4 protocols. Can a UDP protocol Listener have a hostname? In the Listener struct, it can! and you could hypothetically direct traffic for it in some protocols. UDPRoute does not support hostnames, but we could hypothetically add some other route type that can attach to UDP listeners and does.

Hmm, yeah, we probably need to update this wording - maybe something like "If the hostname field is relevant for the Route type and protocol, then hostname must be distinct and compatible between Listeners (or whatever the rules actually are)", with an inline table including known route types and whether hostname is relevant.

tl;dr it turns out writing specs is hard, and I really appreciate the close reading @rainest!