Skip to content

health_check: gracefully handle upstream connection creation failure - #46503

Open
bpalermo wants to merge 2 commits into
envoyproxy:mainfrom
bpalermo:bug/health-check-null-connection
Open

health_check: gracefully handle upstream connection creation failure#46503
bpalermo wants to merge 2 commits into
envoyproxy:mainfrom
bpalermo:bug/health-check-null-connection

Conversation

@bpalermo

@bpalermo bpalermo commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Commit Message

health_check: gracefully handle upstream connection creation failure

Additional Description

Follow-up to #45975, which made HostImplBase::createConnection return a null connection
(instead of crashing) when the upstream connection cannot be created — for example when binding
to a configured Linux network namespace (SocketAddress.network_namespace_filepath) that became
unavailable at runtime. The connection pools were taught to surface that as a pool failure, but
the active health checkers create connections directly via createHealthCheckConnection and
still dereference the result unconditionally, so they crash in the same scenario.

This change makes the TCP, HTTP, gRPC, and Thrift active health checkers treat a null connection
as a NETWORK health-check failure instead of dereferencing it. The TCP and gRPC onTimeout
handlers also tolerate a null client, because the interval base re-arms the timeout timer after a
failed interval. Thrift's Client::start now returns whether the connection was created so the
session can fail gracefully.

The redis proxy upstream client has the same direct-call pattern; making it graceful requires
threading a connection-creation failure through the redis connection pool and is left as a
follow-up.

Risk Level

Low — converts a crash into the existing graceful network-failure path.

Testing

Unit tests: a TCP health checker test covering a null connection and the re-armed timeout firing
against a session with no client (test/common/upstream/health_checker_impl_test.cc); Thrift
mock updated for the new Client::start return value.

Docs Changes

N/A

Release Notes

Included (changelogs/current/bug_fixes/sockets__health_check_network_namespace.rst).

Platform Specific Features

N/A

The TCP, HTTP, gRPC, and Thrift active health checkers create an upstream
connection each interval and dereferenced it unconditionally. When the
connection cannot be created -- for example when binding to a configured
Linux network namespace that became unavailable at runtime, in which case
the host connection factory returns a null connection -- this crashed.

Each health checker now treats a null connection as a network failure
(`handleFailure(NETWORK)`) instead of dereferencing it. The TCP and gRPC
`onTimeout` handlers also tolerate a null client, since the interval base
re-arms the timeout timer after a failed interval. Thrift's `Client::start`
now returns whether the connection was created so the session can fail
gracefully.

This builds on envoyproxy#45975, which made HostImplBase::createConnection return a
null connection instead of crashing in this scenario.

Signed-off-by: Bruno Palermo <b@palermo.dev>
Add unit tests for the null-connection handling in the HTTP, gRPC, and
Thrift health checkers (the TCP path was already covered) to satisfy
per-directory coverage thresholds.

Signed-off-by: Bruno Palermo <b@palermo.dev>
@bpalermo

bpalermo commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

@yanavlasov could you take a look? This is the follow-up to #45975 (which you reviewed): that PR made HostImplBase::createConnection return a null connection instead of crashing and taught the connection pools to handle it, but the active health checkers (TCP, HTTP, gRPC, Thrift) create connections directly and still dereference the result unconditionally. This applies the same graceful handling there, turning the null connection into a NETWORK health-check failure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant