HTTPRoute timeout

[SRodi]

What type of PR is this?

HTTPRoute Timeouts

• update HTTPRoute interfaces according to latest proposal in GEP-1742
• add Duration type as per GEP-2257
• include Request and BackendRequest in HTTPRouteTimeouts
• add validation for HTTPRouteTimeouts
• add unit tests in httproute_test.go
• relates to GEP-1742: HTTPRoute Timeouts API #1997

Relates to:

• https://gateway-api.sigs.k8s.io/geps/gep-1742/
• https://gateway-api.sigs.k8s.io/geps/gep-2257/
• GEP: Timeouts #1742
• GEP-1742: HTTPRoute Timeouts API #1997

What this PR does / why we need it:
POC for HTTPRout timeout proposal

Which issue(s) this PR fixes:

Relates to #1742

Does this PR introduce a user-facing change?:

New fields have been added to configure timeouts on `HTTPRoute` rules:

- `rules.timeouts.request`: to specify a timeout for responding to client HTTP requests
- `rules.timeouts.backendRequest`: to specify a timeout for individual requests from the gateway to a backend service

See [GEP-1472](https://gateway-api.sigs.k8s.io/geps/gep-1742/) for more information.

[k8s-ci-robot]

Welcome @SRodi!

It looks like this is your first PR to kubernetes-sigs/gateway-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/gateway-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @SRodi. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[frankbu]

@SRodi Can you also add a check for timeouts in the validator, e.g., check the value is >= 1ms?

gateway-api/apis/v1beta1/validation/httproute.go

Line 54 in 7b8f168

func ValidateHTTPRouteSpec(spec *gatewayv1b1.HTTPRouteSpec, path *field.Path) field.ErrorList {

[kflynn]

I had a couple of small questions – many thanks for doing this, @SRodi! 🙂

[frankbu]

Do we also need type HTTPRouteTimeouts = v1beta1.HTTPRouteTimeouts in apis/v1alpha2/httproute_types.go?

[robscott]

Let's leave it out. We're moving away from v1alpha2 types (they're no longer served in v0.8.0 CRDs) and I'd expect most/all implementations to have moved away from them before we hit v1.0.0. I haven't had time to figure out if we should leave v1alpha2 type definitions in place for one more release or drop them in v1.0, but in either case, I think I'd rather avoid adding anything new to them.

[frankbu]

Looks like the cel tests are failing?

I'm wondering why pull-gateway-api-verify says "Job succedded" even though several tests have failed?

^^^ @robscott any idea what's wrong here?

[robscott]

@frankbu good catch, my best guess would be that #2362 may have introduced a regression here. Will put a hold on this until we can figure it out.

/hold

[frankbu]

Looks like the cel tests are failing?

Does config/crd/standard/gateway.networking.k8s.io_httproutes.yaml need to be regenerated to include the timeouts as well?

[SRodi]

@frankbu @robscott as mentioned here #2013 (comment) the CEL tests are failing on standard channel but passing on experimental. What should I do about that ?

[robscott]

@frankbu @robscott as mentioned here #2013 (comment) the CEL tests are failing on standard channel but passing on experimental. What should I do about that ?

@SRodi you want to add the tests to this file instead: https://github.com/kubernetes-sigs/gateway-api/blob/main/pkg/test/cel/httproute_experimental_test.go. The test script actually loops through the channels, and installs CRDs and runs tests specifically for that channel (using build tags). Here's the actual test command that's being run: https://github.com/kubernetes-sigs/gateway-api/blob/main/hack/verify-crds-kind.sh#L63.

Does config/crd/standard/gateway.networking.k8s.io_httproutes.yaml need to be regenerated to include the timeouts as well?

Nope, experimental features are intentionally excluded from standard channel CRDs, so that part of this PR is working as intended.

[SRodi]

All tests are passing after moving them to httproute_experimental_test.go - thanks for clarifying this @robscott

Is there anything else I should add to this PR?

[youngnick]

This PR is just on hold while I check the changes I introduced in #2362 - once I've either fixed any problems or confirmed it's not actually a problem, we can proceed. Until then, please sit tight.

[robscott]

Fixes in #2362 were great, looks like we can remove the hold on this one.

/hold cancel
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adleong, frankbu, robscott, SRodi

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:

• OWNERS [robscott]

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

[youngnick]

/lgtm

Nice work @SRodi