fix: derive the protocol of active health checks from the backend - #9806
fix: derive the protocol of active health checks from the backend#9806zhaohuabing wants to merge 1 commit into
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
34e59c0 to
fb840dd
Compare
zhaohuabing
changed the title
zhaohuabing
changed the title
fb840dd to
3f0c5e2
Compare
zhaohuabing
changed the title
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9806 +/- ##
==========================================
+ Coverage 76.34% 76.35% +0.01%
==========================================
Files 261 261
Lines 44440 44474 +34
==========================================
+ Hits 33927 33960 +33
- Misses 8269 8272 +3
+ Partials 2244 2242 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Envoy sends HTTP health check requests with a codec that is fixed when the health checker is created, and health check connections are created outside of the connection pool, so they don't inherit the ALPN protocols that the pool derives from the backend protocol settings. Envoy Gateway configures neither, so health checks are sent as HTTP/1.1 and fail against a backend that speaks HTTP/2 on its health check endpoint, marking all of its endpoints unhealthy. Derive both from the backend protocol: `codec_client_type` is set to HTTP2 for backends that use HTTP/2 and to HTTP1 otherwise, and for backends that use TLS, `tls_options.alpn_protocols` is pinned to the protocol matching the codec, so the protocol negotiated during the handshake is the one Envoy uses to send the request. The ALPN isn't pinned when the backend TLS settings ask for a protocol that the codec can't speak, such as `istio`, since overriding it would break the handshake. gRPC health checks are always sent over HTTP/2, so TLS backends get `h2`. Add `healthCheck.active.http.version` to select the version explicitly, for backends whose health check endpoint uses a different protocol than their application endpoint. Signed-off-by: Huabing (Robin) Zhao <zhaohuabing@gmail.com> Signed-off-by: Huabing (Robin) Zhao <huabing@tetrate.io>
3f0c5e2 to
c07f0ba
Compare
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c07f0ba46d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| upstreamHTTP2 := requiresHTTP2Options && !forceHTTP1UpstreamProtocol | ||
| cluster.HealthChecks, err = buildXdsHealthCheck(args.healthCheck.Active, args.routeHostname, args.healthCheckLog, args.settings, upstreamHTTP2) |
There was a problem hiding this comment.
When useClientProtocol is enabled on a route whose BackendRef indicates HTTP/2, requiresHTTP2Options remains true even though the protocol-options switch later gives useClientProtocol precedence. Consequently an HTTP/1 downstream request is forwarded upstream with HTTP/1 while the active health check is fixed to HTTP/2; an HTTP/1-only health endpoint then fails every probe and all endpoints are marked unhealthy. The Auto derivation needs to account for args.useClientProtocol rather than treating the BackendRef protocol as the effective fixed protocol.
Useful? React with 👍 / 👎.
| // | ||
| // +kubebuilder:default=Auto | ||
| // +optional | ||
| Version *HTTPHealthCheckVersion `json:"version,omitempty" yaml:"version,omitempty"` |
2 participants
Envoy Gateway doesn't configure the protocol used for active health checks, so Envoy falls back to its default and sends every health check request as HTTP/1.1. Health checks against a backend that speaks HTTP/2 on its health check endpoint fail, and all of its endpoints are marked unhealthy.
Envoy Gateway now derives the health check protocol from the backend protocol:
http_health_check.codec_client_typeis set toHTTP2for backends that use HTTP/2, and toHTTP1otherwise.tls_options.alpn_protocolsis set to the protocol matching the codec,h2orhttp/1.1, so that the protocol negotiated during the health check handshake is the one Envoy uses to send the request. It's left unset when the backend TLS settings ask for an ALPN protocol that the codec can't speak, such asistio, since overriding the ALPN would break the handshake.h2.The new
healthCheck.active.http.versionfield (Auto|HTTP1|HTTP2, defaults toAuto) overrides the derived version, for backends that serve a different protocol on their health check endpoint than on the data path.Note: since this PR introduces API changes, I'll raise a follow-up non-API change PR for v1.8/v1.9 only, and use the
Autodefault strategy.