feat(api): add HTTP/2 connection keepalive to ClientTrafficPolicy#8213
feat(api): add HTTP/2 connection keepalive to ClientTrafficPolicy#8213rajatvig wants to merge 5 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Rajat Vig <rvig@etsy.com>
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Rajat Vig <rvig@etsy.com>
Signed-off-by: Rajat Vig <rvig@etsy.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8213 +/- ##
==========================================
+ Coverage 73.81% 73.83% +0.01%
==========================================
Files 241 241
Lines 36608 36651 +43
==========================================
+ Hits 27021 27060 +39
+ Misses 7681 7678 -3
- Partials 1906 1913 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| // 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"` |
There was a problem hiding this comment.
Needs validation that timeout < interval
There was a problem hiding this comment.
Could we validate timeout < interval with CEL so this can be checked eariler?
Signed-off-by: Rajat Vig <rvig@etsy.com>
Signed-off-by: Rajat Vig <rvig@etsy.com>
| if err != nil { | ||
| errs = errors.Join(errs, fmt.Errorf("invalid ConnectionKeepalive.Interval: %w", err)) | ||
| } else { | ||
| keepalive.Interval = ptr.To(uint32(d.Seconds())) |
There was a problem hiding this comment.
The API allows ms, but the translation drops ms. Should we use metav1.Duration here?
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:
https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/address.proto#config-core-v3-tcpkeepalive
| // 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"` |
There was a problem hiding this comment.
Could we document the default timeout value in the API docs?
|
Closing this for #8215 |
3 participants
What this PR does / why we need it
Adds HTTP/2 connection keepalive configuration to ClientTrafficPolicy.
Which issue(s) this PR fixes
Fixes #7744
Release Notes: Yes