-
Notifications
You must be signed in to change notification settings - Fork 674
feat(api): add HTTP/2 connection keepalive to ClientTrafficPolicy #8213
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
Changes from all commits
e25323c
e8624ca
7f00be1
3ec7a3c
311a277
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,7 +98,7 @@ type ClientTrafficPolicySpec struct { | |
| // HTTP2 provides HTTP/2 configuration on the listener. | ||
| // | ||
| // +optional | ||
| HTTP2 *HTTP2Settings `json:"http2,omitempty"` | ||
| HTTP2 *HTTP2ClientSettings `json:"http2,omitempty"` | ||
| // HTTP3 provides HTTP/3 configuration on the listener. | ||
| // | ||
| // +optional | ||
|
|
@@ -440,6 +440,42 @@ const ( | |
| SchemeHeaderTransformMatchBackend SchemeHeaderTransform = "MatchBackend" | ||
| ) | ||
|
|
||
| // HTTP2ClientSettings provides HTTP/2 configuration for client connections to the listener. | ||
| type HTTP2ClientSettings struct { | ||
| // Embed shared HTTP/2 settings. | ||
| HTTP2Settings `json:",inline"` | ||
|
|
||
| // ConnectionKeepalive configures HTTP/2 connection keepalive using PING frames. | ||
| // +optional | ||
| ConnectionKeepalive *HTTP2ConnectionKeepalive `json:"connectionKeepalive,omitempty"` | ||
| } | ||
|
|
||
| // HTTP2ConnectionKeepalive configures HTTP/2 PING-based keepalive settings. | ||
| type HTTP2ConnectionKeepalive struct { | ||
| // Interval specifies how often to send HTTP/2 PING frames to keep the connection alive. | ||
| // If not set, PING frames will not be sent periodically. | ||
| // +optional | ||
| Interval *gwapiv1.Duration `json:"interval,omitempty"` | ||
|
|
||
| // Timeout specifies how long to wait for a PING response before considering the connection dead. | ||
| // If not set, a default timeout is used. | ||
| // +optional | ||
| Timeout *gwapiv1.Duration `json:"timeout,omitempty"` | ||
|
Comment on lines
+455
to
+463
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs validation that timeout < interval
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we validate timeout < interval with CEL so this can be checked eariler?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we document the default timeout value in the API docs? |
||
|
|
||
| // IntervalJitter specifies a random jitter percentage added to each interval. | ||
| // Defaults to 15% if not specified. | ||
| // +kubebuilder:validation:Minimum=0 | ||
| // +kubebuilder:validation:Maximum=100 | ||
| // +optional | ||
| IntervalJitter *uint32 `json:"intervalJitter,omitempty"` | ||
|
|
||
| // ConnectionIdleInterval specifies how long a connection must be idle before a PING is sent | ||
| // to check if the connection is still alive. | ||
| // If not set, idle connection checks are not performed. | ||
| // +optional | ||
| ConnectionIdleInterval *gwapiv1.Duration `json:"connectionIdleInterval,omitempty"` | ||
| } | ||
|
|
||
| func init() { | ||
| localSchemeBuilder.Register(&ClientTrafficPolicy{}, &ClientTrafficPolicyList{}) | ||
| } | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ package gatewayapi | |
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "time" | ||
|
|
||
| "k8s.io/utils/ptr" | ||
|
|
||
|
|
@@ -74,3 +75,52 @@ func buildIRHTTP2Settings(http2Settings *egv1a1.HTTP2Settings) (*ir.HTTP2Setting | |
|
|
||
| return http2, errs | ||
| } | ||
|
|
||
| func buildIRHTTP2ClientSettings(http2Settings *egv1a1.HTTP2ClientSettings) (*ir.HTTP2Settings, error) { | ||
| if http2Settings == nil { | ||
| return nil, nil | ||
| } | ||
|
|
||
| // Reuse shared builder for common fields | ||
| http2, errs := buildIRHTTP2Settings(&http2Settings.HTTP2Settings) | ||
| if http2 == nil { | ||
| http2 = &ir.HTTP2Settings{} | ||
| } | ||
|
|
||
| if http2Settings.ConnectionKeepalive != nil { | ||
| keepalive := &ir.HTTP2ConnectionKeepalive{} | ||
| if http2Settings.ConnectionKeepalive.Interval != nil { | ||
| d, err := time.ParseDuration(string(*http2Settings.ConnectionKeepalive.Interval)) | ||
| if err != nil { | ||
| errs = errors.Join(errs, fmt.Errorf("invalid ConnectionKeepalive.Interval: %w", err)) | ||
| } else { | ||
| keepalive.Interval = ptr.To(uint32(d.Seconds())) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The API allows ms, but the translation drops ms. Should we use Then envoy api does allows ms: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/protocol.proto#envoy-v3-api-msg-config-core-v3-keepalivesettings It's fine for TCP keepalive to use seconds only, since the API only allows seconds: |
||
| } | ||
| } | ||
| if http2Settings.ConnectionKeepalive.Timeout != nil { | ||
| d, err := time.ParseDuration(string(*http2Settings.ConnectionKeepalive.Timeout)) | ||
| if err != nil { | ||
| errs = errors.Join(errs, fmt.Errorf("invalid ConnectionKeepalive.Timeout: %w", err)) | ||
| } else { | ||
| keepalive.Timeout = ptr.To(uint32(d.Seconds())) | ||
| } | ||
| } | ||
| if keepalive.Interval != nil && keepalive.Timeout != nil && *keepalive.Timeout >= *keepalive.Interval { | ||
| errs = errors.Join(errs, fmt.Errorf("ConnectionKeepalive.Timeout must be less than Interval")) | ||
| } | ||
| if http2Settings.ConnectionKeepalive.IntervalJitter != nil { | ||
| keepalive.IntervalJitter = http2Settings.ConnectionKeepalive.IntervalJitter | ||
| } | ||
| if http2Settings.ConnectionKeepalive.ConnectionIdleInterval != nil { | ||
| d, err := time.ParseDuration(string(*http2Settings.ConnectionKeepalive.ConnectionIdleInterval)) | ||
| if err != nil { | ||
| errs = errors.Join(errs, fmt.Errorf("invalid ConnectionKeepalive.ConnectionIdleInterval: %w", err)) | ||
| } else { | ||
| keepalive.ConnectionIdleInterval = ptr.To(uint32(d.Seconds())) | ||
| } | ||
| } | ||
| http2.ConnectionKeepalive = keepalive | ||
| } | ||
|
|
||
| return http2, errs | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| clientTrafficPolicies: | ||
| - apiVersion: gateway.envoyproxy.io/v1alpha1 | ||
| kind: ClientTrafficPolicy | ||
| metadata: | ||
| namespace: envoy-gateway | ||
| name: target-gateway-1-section-http-1 | ||
| spec: | ||
| http2: | ||
| connectionKeepalive: | ||
| interval: 10s | ||
| timeout: 60s | ||
| targetRef: | ||
| group: gateway.networking.k8s.io | ||
| kind: Gateway | ||
| name: gateway-1 | ||
| sectionName: http-1 | ||
| gateways: | ||
| - apiVersion: gateway.networking.k8s.io/v1 | ||
| kind: Gateway | ||
| metadata: | ||
| namespace: envoy-gateway | ||
| name: gateway-1 | ||
| spec: | ||
| gatewayClassName: envoy-gateway-class | ||
| listeners: | ||
| - name: http-1 | ||
| protocol: HTTP | ||
| port: 80 | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: Same |
Uh oh!
There was an error while loading. Please reload this page.