Support conformance test HTTPRouteTimeoutBackendRequest

[randmonkey]

Is there an existing issue for this?

• I have searched the existing issues

Problem Statement

Support extended conformance test HTTPRouteTimeoutBackendRequest added in gateway API 1.0.0-rc1.

Proposed Solution

No response

Additional information

blocked by #4879

Acceptance Criteria

• pass conformance test HTTPRouteTimeoutBackendRequest

[tao12345666333]

As per GEP-1742 https://gateway-api.sigs.k8s.io/geps/gep-1742/#timeout-values the HTTPRouteTimeoutBackendRequest refers to the timeout for a single request from the gateway to a backend.

In the NGINX ecosystem, we have three parameters that can control this flow.

• proxy_connect_timeout
• proxy_send_timeout
• proxy_read_timeout

In Kong Gateway, we also have three parameters in the Service object. https://docs.konghq.com/gateway/latest/admin-api/#service-object

connect_timeout	The timeout in milliseconds for establishing a connection to the upstream server. Default: 60000.
write_timeout	The timeout in milliseconds between two successive write operations for transmitting a request to the upstream server. Default: 60000.
read_timeout	The timeout in milliseconds between two successive read operations for transmitting a request to the upstream server. Default: 60000.

This means that timeouts.backendRequest should equal the sum of connect_timeout, read_timeout, and write_time.

I think we should discuss assigning this value to the three parameters or setting it as a specific parameter.

[pmalek]

After reading through the GEP-1742 and specifically the description of the timeouts.backendRequest:

    // BackendRequest specifies a timeout for an individual request from the gateway
    // to a backend. This covers the time from when the request first starts being
    // sent from the gateway to when the full response has been received from the backend.
    //
    // An entire client HTTP transaction with a gateway, covered by the Request timeout,
    // may result in more than one call from the gateway to the destination backend,
    // for example, if automatic retries are supported.
    //
    // Because the Request timeout encompasses the BackendRequest timeout, the value of
    // BackendRequest must be <= the value of Request timeout.
    //
    // Support: Extended
    //
    // +optional
    BackendRequest *Duration `json:"backendRequest,omitempty"`

I'm afraid that we might not be able to implement this given that read and write timeouts are related to individual operations within the whole request while the connect timeout only specifies the timeout for establishing the connection with the upstream.

[tao12345666333]

Yes.

Our team has already discussed a lot and also made records in the upstream project's issue. We will collaborate with the upstream to explore feasible solutions.
ref: kubernetes-sigs/gateway-api#1742 (comment)

capability level 1: the user setting read / write / connect timeouts by route annotations (done today)
capability level 2: the user setting the timeouts.backendRequest in a route that KIC models in an imperfect way using connect/read/write timeouts (possible with Jintao's change)
capability level 3: the user setting the timeouts.backendRequest in a route that KIC models in a perfect way (requires a plugin or a new feature in Kong Gateway)
capability level 4: the GEP being extended so that read/write/connect timeouts are first class gateway api concepts (requires GEP/GWAPI CRD change but possible with Kong Gateway)
quote by @mflendrich

I will implement according to the second part, when the user specifies timeouts.backendRequest, set ConnectTimeout, ReadTimeout and WriteTimeout in Service to the same value, and clearly describe in the document what impact the user will receive.

• Modify the code to make timeouts.backendRequest effective.
• Add documentation to clearly describe this behavior.

[tao12345666333]

I have another question regarding the #3060 that introduces a CombinedRoutes feature to reduce configurations.

kubernetes-ingress-controller/internal/dataplane/translator/subtranslator/httproute.go

Lines 71 to 80 in ec64c88

	func (i *httpRouteTranslationIndex) translate() []*KongServiceTranslation {
	rulesGroupedByBackendRed := groupRulesByBackendRefs(i.rulesMeta)
	translations := make([]*KongServiceTranslation, 0, len(rulesGroupedByBackendRed))
	for _, rulesByBackends := range rulesGroupedByBackendRed {
	// each backend refs group is a separate Kong service, not eligible for consolidation
	kongServiceTranslation := i.translateToKongService(rulesByBackends)
	i.translateToKongServiceRoutes(kongServiceTranslation, rulesByBackends)
	translations = append(translations, kongServiceTranslation)
	}

For example, we have the following configuration:

apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: timeout-example
spec:
  ...
  rules:
  - backendRefs:
    - name: some-service
      port: 8080
    timeouts:
      backendRequest: 5s
  - backendRefs:
    - name: some-service
      port: 8080
    timeouts:
      backendRequest: 2s

Timeout and backendRefs are at the same level. Using the current implementation logic, there will be a situation where the timeout is overwritten.
In our actual situation, I believe that because the timeout is still experimental and our implementation differs from the specification in some ways, I expect to implement this function at a lower cost.
Therefore, I think a feasible method is not to make too many modifications to the current implementation logic.
Even if there is an overwrite situation, only keep the last one. At the same time, inform users of this behavior.

EDIT: Update the example to make it simpler and more specific.