feat(timeouts): support HTTPRoute timeouts.request and timeouts.backendRequest - #81
Open
KealanAU wants to merge 11 commits into
Open
feat(timeouts): support HTTPRoute timeouts.request and timeouts.backendRequest#81KealanAU wants to merge 11 commits into
KealanAU wants to merge 11 commits into
Conversation
Per-route backend timeouts plumbed from HTTPRoute through routing.json and ghost.json to the ghost VMOD, which bridges the value to bereq via the X-Ghost-Timeout header. Varnish backends are pooled by address:port and so cannot carry per-route timeouts themselves. vcl_backend_fetch turns the header into bereq.first_byte_timeout and between_bytes_timeout; the postamble vcl_backend_error uses its presence to report 504 instead of Varnish's default 503. The flip sits in the postamble so user VCL keeps precedence. Gateway API's "0s" (disable) is emitted as absent, inheriting varnishd's global defaults — Varnish cannot uncap a fetch. Enables SupportHTTPRouteBackendTimeout in the conformance suite.
request is applied as an alias for backendRequest: Varnish has no total-request timeout, so the clock necessarily starts at the backend fetch. When both fields are set the tighter value wins — normally backendRequest, since the spec requires it to be no larger than request, but a spec-violating route must not end up with the looser bound. Enables SupportHTTPRouteRequestTimeout in the conformance suite.
The longer name was wider than every field in five ghost.Route literals, forcing gofmt to realign them all — 138 diff lines to add five. TimeoutMs fits the existing columns. The backend_timeout_ms JSON tag is unchanged, so routing.json, ghost.json and the ghost VMOD are untouched. Also inline the single-caller minNonZeroMs helper and drop rationale duplicated between the code comments and the reference doc.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #81 +/- ##
==========================================
- Coverage 71.99% 71.94% -0.05%
==========================================
Files 41 41
Lines 6716 6740 +24
==========================================
+ Hits 4835 4849 +14
- Misses 1539 1550 +11
+ Partials 342 341 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bereq.connect_timeout now moves with first_byte_timeout and between_bytes_timeout, so an unreachable pod fails inside the route's budget instead of after varnishd's 3.5s global. Gateway API scopes backendRequest to after request headers are sent, but the bound users want is their own wall-clock wait. Drops the "connect time is not bounded" caveat from the reference docs. Also adds coverage for timeouts on backendless routes.
KealanAU
marked this pull request as ready for review
August 14, 2026 07:45
The timeout is per-rule, so setting it in each of the five ghost.Route literals meant gofmt realigned every field in all five — 183 changed lines to carry one value. Stamp the rule's routes at the bottom of the loop instead; a sixth append can no longer forget the field. Drops TestCollectHTTPRouteBackends_TimeoutOnBackendlessRoutes, which only guarded against exactly that omission, and the VTC client c_spoof, which asserted the unset in the test file's own VCL rather than the preamble's — that check now lives in TestGenerate_RouteTimeoutSetsAllFetchTimeouts.
deploy/01-operator.yaml set no GATEWAY_IMAGE, so the operator fell back to its built-in ghcr.io/varnish/gateway-chaperone:latest. Being a :latest tag, the node pulled it from the registry and ignored the locally built image kind-load had just installed — make deploy, kind-deploy and test-conformance-kind all ran the published data plane, so no ghost or VCL change could be verified against them. The value matches the operator's own default, so nothing changes for kubectl-apply users; deploy-update rewrites the tag for local runs.
backend_timeout_ms deserializes into a u32, but the GEP-2257 pattern allows up to four 5-digit components, so a CRD-valid duration like 2000h exceeds it. serde would fail the whole ghost.json parse rather than just that route, freezing routing updates for every route on the gateway. durationMs now saturates at math.MaxUint32 (~49.7 days). Clamping at the single conversion point covers both request and backendRequest.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #24.
HTTPRoute
timeouts.requestandtimeouts.backendRequestare now honoured: a route whose backend takes too long returns 504.Can't stack PRs from forks, so the GATEWAY_IMAGE fix I hit while building and testing is folded in below.
Timeouts
One integer travels from the HTTPRoute rule to three varnishd parameters.
routeBackendTimeoutMstakes the tighter ofrequestandbackendRequest, in ms. Unset,0s, and unparseable all become 0, which serialises as absent, so the route keeps the varnishd globals.routing.jsoncarries it asbackend_timeout_ms, and it joins the merge key inmergeRoutesByMatchCriteria— otherwise two routes that match identically but time out differently collapse into one group.X-Ghost-Timeout: <n>ms. Backends are pooled by address:port, so the backend object cannot hold a per-route value.vcl_backend_fetchturns the header intoconnect_timeout,first_byte_timeout, andbetween_bytes_timeout. Eachstd.duration()falls back to that parameter's default, not 0s.vcl_backend_errorfinds the header still set and rewrites the status to 504. Noreturn(deliver), so a user's own handler still wins.Two judgment calls worth checking:
requestbecomes an alias forbackendRequest. The spec bounds the whole request lifecycle; this bounds the backend fetch. Conformance passes because both tests use a slow backend, where the definitions agree.connect_timeoutis set alongside the two fetch timeouts, though it is arguably out of scope. It makes an unreachable pod fail in 0.5s instead of 3.5s, at the cost of a tight timeout having to cover the TCP and TLS handshake.Other edges:
vcl_backend_errorhas no failure reason to branch on.backend_timeout_msis a u32 and GEP-2257 permits2000h, sodurationMssaturates atmath.MaxUint32. Otherwise, one absurd route fails the wholeghost.jsonparse.docs/reference/httproute-timeouts.md.GATEWAY_IMAGE fix
deploy/01-operator.yamlnever set it, so the operator defaulted togateway-chaperone:latestand the node pulled from ghcr instead of the imagekind loadinstalled.make deploy,make kind-deploy, andmake test-conformance-kindwere all testing the published data plane.kubectl apply -f deploy/is unchanged.deploy-updaterewrites the tag for local runs.Testing
make test-go,make test-ghost, and theHTTPRouteTimeoutBackendRequestandHTTPRouteTimeoutRequestconformance tests pass. I re-ran conformance on a fresh kind cluster with the image fix; without it both tests fail.By hand on kind against
echo-basic:/backend-timeout?delay=1s/backend-timeout/disable-backend-timeout?delay=1s0sdisables/request-timeout?delay=1srequestis honored/disable-request-timeout?delay=1s0sdisables onrequesttoo/generous-timeout?delay=1s/both-timeouts?delay=1sbackend_timeout_msis 500, not 10000/no-timeout?delay=1s/unreachableconnect_timeoutapplies (192.0.2.1, SYN dropped)/unreachable-no-timeoutThe one-stamp refactor dropped
TestCollectHTTPRouteBackends_TimeoutOnBackendlessRoutes(it asserted all five route literals) and thec_spoofVTC client (it tested VCL in the test file, not the preamble).TestGenerate_RouteTimeoutSetsAllFetchTimeoutscovers both.The slow server in
test_backend_timeout.vtcnever responds, so it shows the 503→504 flip but not that the value reaches bereq. The table above shows that.Disclaimer: researched and partially built with Claude Opus 5 and GLM 5.2. Tested and reviewed by hand.