add UDPRoute GEP

[zac-nixon]

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

[k8s-ci-robot]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[rikatz]

I can't remember the full discussion of it, and I guess this also happened during Kubecon/EGADS, but I guess we need a different parentRef semantic for TCP and UDP routes, where we enforce it must be attached to a named listener, so you know which port will be routing to it.

@rostislavbobo @youngnick I need some help here, as I don't know what was the conclusion of TCP and UDPRoute

[zac-nixon]

I'd be interesting to hear why. A common use-case for Layer 4 load balancers is to listen on a range of ports and forward to the same backend.

[rikatz]

well, you still need a well defined port for it, right? So it means you need each port defined on the listener today, being named and then you need to select one by one which you want to attach to the same backends.

Makes me think that we also do not quite support port ranges on listeners today, but that's another story

[zac-nixon]

The use-case that we're making hard is that suppose I expose my application on a range of ports, 80-100, I would need to parent ref each individual port. What would be the reason to make this use-case harder?

[rikatz]

right, true. But I still have the feeling I am missing a reason why we do not do this, so I will defer to @youngnick because he may remember better than me!

Anyway, given this is provisional and eventually experimental, it is not a hard block but probably a "Must decide" situation for me

[rikatz]

anyway, should we consider a support for a port range on a listener and attachment to that listener directly? I am still thinking that for the range case you may need:

• one listener per port on your range
• then one parentRef for each port

even if we lower the second bar, the first one still is necessary and may be problematic, wdyt?

[youngnick]

As it stands today, each Listener supports only one port. We have discussed port range semantics in the past, but nothing moved forward.

I think it's probably fine to say, for now:

UDPRoutes will attach to:

• a single listener with a matching name if sectionName is supplied in the parentRef
• a single listener with a matching port if port is supplied in the parentRef, and the implementation supports the GatewayParentRefPort feature (I think that's the right feature name)
• All Listeners on the Gateway with protocol set to UDP if no sectionName or port is specified.

That handles the multiple ports case for now, until we decide if and how we would add port range support to Listener. (I expect there to be more appetite for that once UDPRoute and TCPRoute move forward). That would make port range on Listener semantic sugar for having that many Listener entries, one for each distinct port. I'd also recommend that port ranges must be contiguous.

But again, we should wait and handle the simpler, single-port case first, and then come back and figure out how to handle the more complex case once we have that done.

[zac-nixon]

I like the semantics you laid out, @youngnick as that matches the correct behavior for listener attachment.

[rikatz]

I would like to have the conformance tests added on a more descriptive way. I know this is a draft for now, so leaving it as a comment for once we decide to move on.

As an example: https://github.com/kubernetes-sigs/gateway-api/blob/main/geps/gep-1713/index.md?plain=1#L989

[zac-nixon]

Yeah, I will add more detail on the conformance tests once we're happy with the UDPRoute requirements. For example, you mentioned removing weighted routing which will be important to finalize before writing conformance tests.

[zac-nixon]

Added more detailed conformance tests.

[rikatz]

/cc
/assign

[youngnick]

/cc

[cnvergence]

we might update it to gateway.networking.k8s.io/v1
opened #4722

[zac-nixon]

Is the GEP for the current state or for the desired state?

[cnvergence]

I would say both, for the factual state

[zac-nixon]

I don't know how to represent both states. Are you suggesting duplicating the YAMLs?

[cnvergence]

It is really a nit comment, the gateway resource has been GA since the Gateway API v1.0 release and should use gateway.networking.k8s.io/v1. I have found that the examples in the conformance were outdated, so we have updated them.
Thanks 🙂

[rikatz]

we must keep the GEP on the current state. We move the APIs versioning to the right state once the API is promoted.

That said:

• Any reference to a Gateway/GatewayClass must use apiVersion v1 (the latest GA)
• Any reference to a UDPRoute must use the current apiVersion (v1alpha2)

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: zac-nixon
Once this PR has been reviewed and has the lgtm label, please ask for approval from rikatz. For more information see the Code Review Process.

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

Details

Needs approval from an approver in each of these files:

• geps/OWNERS

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

[rikatz]

maybe add some example on how you envision the "port range" case mentioned above

[youngnick]

This can probably hold until the discussion of how we handle port ranges on the Listener. I should note here that Service does not have port range support that I can see, so we'll need to manage that somehow as well.

[zac-nixon]

Will hold off on port range

[zac-nixon]

I suppose that the user would need to manually specify the ports in the Gateway, but we can save them the headache of duplicating the parent refs :) ideally -- it would be nice to support a range concept in the Gateway.

[rikatz]

not for now, but between experimental and standard, we may need some better descriptive tests. What means an invalid backendRef for a UDPRoute?

What means multiple backends? What means multiple rules?

No worries for now, but please take a look into TLSRoute Gep and ListenerSet for reference

[zac-nixon]

Done!

[youngnick]

This caveat is fine for Provisional, but for Standard, I think we'll probably need an Extended Feature for this, along with a separate Extended Feature for Client IP preservation. Folks who don't need those things don't care if they have them or not, but folks that do need them, really need them, and need to be able to be explicit about it.

[youngnick]

This LGTM for Provisional, but I think it would benefit from a TODO list for Experimental. From what I can see, that includes:

• Declaring the invalid behaviors, conflict resolution, and so on. Effectively, we need to define the Core conformance tests we want to see here.
• Declaring optional behaviors, along with conformance tests there. As it stands, I can see at least two Extended features, Client IP preservation, and UDP pseudo-session management support (basically, light sticky sessions based on the traffic's Layer 3 4-tuple of source IP, source port, destination IP, destination port. This usually includes some config of the timeout settings for these pseudo-connections as well.)
• Making sure we record that Port Range support for UDPRoute requires Port Range support for Listener, and for us to figure out how to handle that on the backend, given Service also does not have Port Range support.

I think once we record that, this is good to merge.

[rikatz]

I "think" this is extended, given you may want to support Gateway but not UDP, but Nick knows those definitions much much better!

[rikatz]

https://gateway-api.sigs.k8s.io/reference/spec/#tcproute is probably the right link

[rikatz]

overall this lgtm, thanks @zac-nixon

I will have some sync with Nick on it soon and we will bring back some more feedback.