Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[release/v1.1] cherry-pick for v1.1.4 #4897

Merged
merged 3 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 5 additions & 42 deletions internal/gatewayapi/backendtrafficpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
rt = t.buildRetry(policy)
}
if policy.Spec.Timeout != nil {
if to, err = t.buildTimeout(*policy, nil); err != nil {
if to, err = t.buildTimeout(*policy); err != nil {
err = perr.WithMessage(err, "Timeout")
errs = errors.Join(errs, err)
}
Expand Down Expand Up @@ -411,8 +411,7 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen

for _, http := range x.HTTP {
for _, r := range http.Routes {
// Some timeout setting originate from the route.
if localTo, err := t.buildTimeout(*policy, r.Traffic); err == nil {
if localTo, err := t.buildTimeout(*policy); err == nil {
to = localTo
}

Expand Down Expand Up @@ -497,7 +496,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
rt = t.buildRetry(policy)
}
if policy.Spec.Timeout != nil {
if ct, err = t.buildTimeout(*policy, nil); err != nil {
if ct, err = t.buildTimeout(*policy); err != nil {
err = perr.WithMessage(err, "Timeout")
errs = errors.Join(errs, err)
}
Expand Down Expand Up @@ -595,7 +594,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
// Update the Host field in HealthCheck, now that we have access to the Route Hostname.
r.Traffic.HealthCheck.SetHTTPHostIfAbsent(r.Hostname)

if ct, err = t.buildTimeout(*policy, r.Traffic); err == nil {
if ct, err = t.buildTimeout(*policy); err == nil {
r.Traffic.Timeout = ct
}

Expand Down Expand Up @@ -1066,12 +1065,8 @@ func (t *Translator) buildCircuitBreaker(policy *egv1a1.BackendTrafficPolicy) (*
return cb, nil
}

func (t *Translator) buildTimeout(policy egv1a1.BackendTrafficPolicy, traffic *ir.TrafficFeatures) (*ir.Timeout, error) {
func (t *Translator) buildTimeout(policy egv1a1.BackendTrafficPolicy) (*ir.Timeout, error) {
if policy.Spec.Timeout == nil {
if traffic != nil {
// Don't lose any existing timeout definitions.
return mergeTimeoutSettings(nil, traffic.Timeout), nil
}
return nil, nil
}
var (
Expand Down Expand Up @@ -1119,41 +1114,9 @@ func (t *Translator) buildTimeout(policy egv1a1.BackendTrafficPolicy, traffic *i
}
}

// http request timeout is translated during the gateway-api route resource translation
// merge route timeout setting with backendtrafficpolicy timeout settings.
// Merging is done after the clustersettings definitions are translated so that
// clustersettings will override previous settings.
if traffic != nil {
to = mergeTimeoutSettings(to, traffic.Timeout)
}
return to, errs
}

// merge secondary into main if both are not nil, otherwise return the
// one that is not nil. If both are nil, returns nil
func mergeTimeoutSettings(main, secondary *ir.Timeout) *ir.Timeout {
switch {
case main == nil && secondary == nil:
return nil
case main == nil:
return secondary.DeepCopy()
case secondary == nil:
return main
default: // Neither main nor secondary are nil here
if secondary.HTTP != nil {
setIfNil(&main.HTTP, &ir.HTTPTimeout{})
setIfNil(&main.HTTP.RequestTimeout, secondary.HTTP.RequestTimeout)
setIfNil(&main.HTTP.ConnectionIdleTimeout, secondary.HTTP.ConnectionIdleTimeout)
setIfNil(&main.HTTP.MaxConnectionDuration, secondary.HTTP.MaxConnectionDuration)
}
if secondary.TCP != nil {
setIfNil(&main.TCP, &ir.TCPTimeout{})
setIfNil(&main.TCP.ConnectTimeout, secondary.TCP.ConnectTimeout)
}
return main
}
}

func int64ToUint32(in int64) (uint32, bool) {
if in >= 0 && in <= math.MaxUint32 {
return uint32(in), true
Expand Down
7 changes: 0 additions & 7 deletions internal/gatewayapi/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -580,10 +580,3 @@ func getPolicyTargetRefs[T client.Object](policy egv1a1.PolicyTargetReferences,

return ret
}

// Sets *target to value if and only if *target is nil
func setIfNil[T any](target **T, value *T) {
if *target == nil {
*target = value
}
}
24 changes: 4 additions & 20 deletions internal/gatewayapi/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,14 +257,13 @@

func processRouteTimeout(irRoute *ir.HTTPRoute, rule gwapiv1.HTTPRouteRule) {
if rule.Timeouts != nil {
rto := &ir.Timeout{}

if rule.Timeouts.Request != nil {
d, err := time.ParseDuration(string(*rule.Timeouts.Request))
if err != nil {
d, _ = time.ParseDuration(HTTPRequestTimeout)
}
setRequestTimeout(rto, metav1.Duration{Duration: d})
irRoute.Timeout = ptr.To(metav1.Duration{Duration: d})
}

// Also set the IR Route Timeout to the backend request timeout
Expand All @@ -274,23 +273,8 @@
if err != nil {
d, _ = time.ParseDuration(HTTPRequestTimeout)
}
setRequestTimeout(rto, metav1.Duration{Duration: d})
irRoute.Timeout = ptr.To(metav1.Duration{Duration: d})
}

irRoute.Traffic = &ir.TrafficFeatures{
Timeout: rto,
}
}
}

func setRequestTimeout(irTimeout *ir.Timeout, d metav1.Duration) {
switch {
case irTimeout.HTTP == nil:
irTimeout.HTTP = &ir.HTTPTimeout{
RequestTimeout: ptr.To(d),
}
default:
irTimeout.HTTP.RequestTimeout = ptr.To(d)
}
}

Expand Down Expand Up @@ -692,11 +676,11 @@
Mirrors: routeRoute.Mirrors,
ExtensionRefs: routeRoute.ExtensionRefs,
IsHTTP2: routeRoute.IsHTTP2,
Timeout: routeRoute.Timeout,
}
if routeRoute.Traffic != nil {
hostRoute.Traffic = &ir.TrafficFeatures{
Timeout: routeRoute.Traffic.Timeout,
Retry: routeRoute.Traffic.Retry,
Retry: routeRoute.Traffic.Retry,

Check warning on line 683 in internal/gatewayapi/route.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/route.go#L683

Added line #L683 was not covered by tests
}
}
perHostRoutes = append(perHostRoutes, hostRoute)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,27 @@ httpRoutes:
port: 8080
timeouts:
request: 130s
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
namespace: default
name: httproute-2
spec:
hostnames:
- gateway.envoyproxy.io
parentRefs:
- namespace: envoy-gateway
name: gateway-1
sectionName: http
rules:
- matches:
- path:
value: "/"
backendRefs:
- name: service-1
port: 8080
timeouts:
request: 130s
backendTrafficPolicies:
- apiVersion: gateway.envoyproxy.io/v1alpha1
kind: BackendTrafficPolicy
Expand All @@ -47,4 +68,20 @@ backendTrafficPolicies:
kind: HTTPRoute
name: httproute-1
useClientProtocol: true

- apiVersion: gateway.envoyproxy.io/v1alpha1
kind: BackendTrafficPolicy
metadata:
namespace: envoy-gateway
name: policy-for-gateway
spec:
targetRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
useClientProtocol: true
loadBalancer:
type: ConsistentHash
consistentHash:
type: Cookie
cookie:
name: "test"
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,44 @@ backendTrafficPolicies:
status: "True"
type: Accepted
controllerName: gateway.envoyproxy.io/gatewayclass-controller
- apiVersion: gateway.envoyproxy.io/v1alpha1
kind: BackendTrafficPolicy
metadata:
creationTimestamp: null
name: policy-for-gateway
namespace: envoy-gateway
spec:
loadBalancer:
consistentHash:
cookie:
name: test
type: Cookie
type: ConsistentHash
targetRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
useClientProtocol: true
status:
ancestors:
- ancestorRef:
group: gateway.networking.k8s.io
kind: Gateway
name: gateway-1
namespace: envoy-gateway
conditions:
- lastTransitionTime: null
message: Policy has been accepted.
reason: Accepted
status: "True"
type: Accepted
- lastTransitionTime: null
message: 'This policy is being overridden by other backendTrafficPolicies
for these routes: [default/httproute-1]'
reason: Overridden
status: "True"
type: Overridden
controllerName: gateway.envoyproxy.io/gatewayclass-controller
gateways:
- apiVersion: gateway.networking.k8s.io/v1
kind: Gateway
Expand All @@ -44,7 +82,7 @@ gateways:
protocol: HTTP
status:
listeners:
- attachedRoutes: 1
- attachedRoutes: 2
conditions:
- lastTransitionTime: null
message: Sending translated listener configuration to the data plane
Expand Down Expand Up @@ -108,6 +146,46 @@ httpRoutes:
name: gateway-1
namespace: envoy-gateway
sectionName: http
- apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
creationTimestamp: null
name: httproute-2
namespace: default
spec:
hostnames:
- gateway.envoyproxy.io
parentRefs:
- name: gateway-1
namespace: envoy-gateway
sectionName: http
rules:
- backendRefs:
- name: service-1
port: 8080
matches:
- path:
value: /
timeouts:
request: 130s
status:
parents:
- conditions:
- lastTransitionTime: null
message: Route is accepted
reason: Accepted
status: "True"
type: Accepted
- lastTransitionTime: null
message: Resolved all the Object references for the Route
reason: ResolvedRefs
status: "True"
type: ResolvedRefs
controllerName: gateway.envoyproxy.io/gatewayclass-controller
parentRef:
name: gateway-1
namespace: envoy-gateway
sectionName: http
infraIR:
envoy-gateway/gateway-1:
proxy:
Expand Down Expand Up @@ -165,8 +243,33 @@ xdsIR:
distinct: false
name: ""
prefix: /
timeout: 2m10s
traffic: {}
useClientProtocol: true
- destination:
name: httproute/default/httproute-2/rule/0
settings:
- addressType: IP
endpoints:
- host: 7.7.7.7
port: 8080
protocol: HTTP
weight: 1
hostname: gateway.envoyproxy.io
isHTTP2: false
metadata:
kind: HTTPRoute
name: httproute-2
namespace: default
name: httproute/default/httproute-2/rule/0/match/0/gateway_envoyproxy_io
pathMatch:
distinct: false
name: ""
prefix: /
timeout: 2m10s
traffic:
timeout:
http:
requestTimeout: 2m10s
loadBalancer:
consistentHash:
cookie:
name: test
useClientProtocol: true
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,4 @@ xdsIR:
distinct: false
name: ""
prefix: /
traffic:
timeout:
http:
requestTimeout: 1s
timeout: 1s
Original file line number Diff line number Diff line change
Expand Up @@ -332,10 +332,10 @@ xdsIR:
distinct: false
name: ""
prefix: /
timeout: 1s
traffic:
timeout:
http:
maxConnectionDuration: 22s
requestTimeout: 1s
tcp:
connectTimeout: 20s
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,4 @@ xdsIR:
distinct: false
name: ""
prefix: /
traffic:
timeout:
http:
requestTimeout: 1s
timeout: 1s
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,4 @@ xdsIR:
distinct: false
name: ""
prefix: /
traffic:
timeout:
http:
requestTimeout: 5s
timeout: 5s
2 changes: 2 additions & 0 deletions internal/ir/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,8 @@ type HTTPRoute struct {
UseClientProtocol *bool `json:"useClientProtocol,omitempty" yaml:"useClientProtocol,omitempty"`
// Metadata is used to enrich envoy route metadata with user and provider-specific information
Metadata *ResourceMetadata `json:"metadata,omitempty" yaml:"metadata,omitempty"`
// Timeout is the time until which entire response is received from the upstream.
Timeout *metav1.Duration `json:"timeout,omitempty" yaml:"timeout,omitempty"`
}

// TrafficFeatures holds the information associated with the Backend Traffic Policy.
Expand Down
Loading
Loading