From a2fc3f3dc96a10c9043d511f1fca6d02ece6985f Mon Sep 17 00:00:00 2001 From: phlax Date: Fri, 17 Apr 2026 13:11:59 +0100 Subject: [PATCH 1/8] test: Fix ws integration test flake (#44508) Signed-off-by: Ryan Northey --- test/integration/BUILD | 6 +----- test/integration/websocket_integration_test.cc | 4 ++-- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/test/integration/BUILD b/test/integration/BUILD index a80971ea12131..ac23c0009b34f 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -1665,12 +1665,8 @@ envoy_cc_test_library( envoy_cc_test( name = "websocket_integration_test", - size = "large", srcs = ["websocket_integration_test.cc"], - rbe_pool = "2core", - tags = [ - "cpu:3", - ], + rbe_pool = "linux_x64_small", deps = [ ":http_protocol_integration_lib", ":websocket_integration_test_lib", diff --git a/test/integration/websocket_integration_test.cc b/test/integration/websocket_integration_test.cc index c303ea2a23c98..0477ffc5104a0 100644 --- a/test/integration/websocket_integration_test.cc +++ b/test/integration/websocket_integration_test.cc @@ -922,7 +922,7 @@ TEST_P(WebsocketIntegrationTest, WebSocketUpgradeRouteTimeoutWithRetries) { auto* route_config = hcm.mutable_route_config(); auto* virtual_host = route_config->mutable_virtual_hosts(0); auto* route = virtual_host->mutable_routes(0)->mutable_route(); - route->mutable_timeout()->set_nanos(200 * 1000 * 1000); // 200ms route timeout + route->mutable_timeout()->set_seconds(2); // 2s route timeout }); initialize(); @@ -936,7 +936,7 @@ TEST_P(WebsocketIntegrationTest, WebSocketUpgradeRouteTimeoutWithRetries) { ASSERT_TRUE(fake_upstreams_[0]->waitForHttpConnection(*dispatcher_, fake_upstream_connection_)); ASSERT_TRUE(fake_upstream_connection_->waitForNewStream(*dispatcher_, upstream_request_)); ASSERT_TRUE(upstream_request_->waitForHeadersComplete()); - upstream_request_->encodeHeaders(upgradeFailedResponseHeaders(), false); + upstream_request_->encodeHeaders(upgradeFailedResponseHeaders(), true); // Wait for the first request to be reset or disconnected ASSERT_TRUE(waitForUpstreamDisconnectOrReset()); From e91417ff5893d8b0d70603a27b1562e628f0f20c Mon Sep 17 00:00:00 2001 From: phlax Date: Fri, 17 Apr 2026 18:53:30 +0100 Subject: [PATCH 2/8] test/ws: Follow up fixes from #44508 (#44518) Signed-off-by: Ryan Northey --- test/integration/BUILD | 1 + test/integration/websocket_integration_test.cc | 10 +++++----- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/test/integration/BUILD b/test/integration/BUILD index ac23c0009b34f..0877507311245 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -1667,6 +1667,7 @@ envoy_cc_test( name = "websocket_integration_test", srcs = ["websocket_integration_test.cc"], rbe_pool = "linux_x64_small", + shard_count = 4, deps = [ ":http_protocol_integration_lib", ":websocket_integration_test_lib", diff --git a/test/integration/websocket_integration_test.cc b/test/integration/websocket_integration_test.cc index 0477ffc5104a0..146e376718dde 100644 --- a/test/integration/websocket_integration_test.cc +++ b/test/integration/websocket_integration_test.cc @@ -262,8 +262,8 @@ TEST_P(WebsocketIntegrationTest, WebSocketConnectionIdleTimeout) { auto* route_config = hcm.mutable_route_config(); auto* virtual_host = route_config->mutable_virtual_hosts(0); auto* route = virtual_host->mutable_routes(0)->mutable_route(); - route->mutable_idle_timeout()->set_seconds(0); - route->mutable_idle_timeout()->set_nanos(200 * 1000 * 1000); + route->mutable_idle_timeout()->set_seconds(1); + route->mutable_idle_timeout()->set_nanos(0); }); initialize(); @@ -850,8 +850,8 @@ TEST_P(WebsocketIntegrationTest, WebSocketUpgradePerTryTimeout) { auto* route_config = hcm.mutable_route_config(); auto* virtual_host = route_config->mutable_virtual_hosts(0); auto* route = virtual_host->mutable_routes(0)->mutable_route(); - route->mutable_retry_policy()->mutable_per_try_timeout()->set_nanos( - 200 * 1000 * 1000); // 200ms per-try timeout + route->mutable_retry_policy()->mutable_per_try_timeout()->set_seconds( + 2); // 2s per-try timeout (safe under msan/tsan) }); initialize(); @@ -886,7 +886,7 @@ TEST_P(WebsocketIntegrationTest, WebSocketUpgradeRouteTimeout) { auto* route_config = hcm.mutable_route_config(); auto* virtual_host = route_config->mutable_virtual_hosts(0); auto* route = virtual_host->mutable_routes(0)->mutable_route(); - route->mutable_timeout()->set_nanos(200 * 1000 * 1000); // 200ms route timeout + route->mutable_timeout()->set_seconds(2); // 2s route timeout (safe under msan/tsan) }); initialize(); From 79504fee9dc027b26c25d1f50cabb5779c96d4ad Mon Sep 17 00:00:00 2001 From: phlax Date: Mon, 20 Apr 2026 15:25:10 +0100 Subject: [PATCH 3/8] test/ws: Increase sharding further (#44529) as this can still flake in msan Signed-off-by: Ryan Northey --- test/integration/BUILD | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/BUILD b/test/integration/BUILD index 0877507311245..333803c5f3024 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -1667,7 +1667,7 @@ envoy_cc_test( name = "websocket_integration_test", srcs = ["websocket_integration_test.cc"], rbe_pool = "linux_x64_small", - shard_count = 4, + shard_count = 8, deps = [ ":http_protocol_integration_lib", ":websocket_integration_test_lib", From 165ff9c982bee8cdf59b28659fb6e9be4aad1790 Mon Sep 17 00:00:00 2001 From: phlax Date: Wed, 22 Apr 2026 15:07:08 +0100 Subject: [PATCH 4/8] test/idle_timeout: Fix flake (#44584) Scale RequestTimeoutMs by TIMEOUT_FACTOR to match IdleTimeoutMs. The hard-coded 200ms races test setup on loaded/sanitizer runs, causing the request timer to fire before the test body executes. fix #26979 Signed-off-by: Ryan Northey --- test/integration/idle_timeout_integration_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/idle_timeout_integration_test.cc b/test/integration/idle_timeout_integration_test.cc index f2c56c3cde4f6..25cc5bed5829c 100644 --- a/test/integration/idle_timeout_integration_test.cc +++ b/test/integration/idle_timeout_integration_test.cc @@ -117,7 +117,7 @@ class IdleTimeoutIntegrationTest : public HttpProtocolIntegrationTest { } static constexpr uint64_t IdleTimeoutMs = 300 * TIMEOUT_FACTOR; - static constexpr uint64_t RequestTimeoutMs = 200; + static constexpr uint64_t RequestTimeoutMs = 200 * TIMEOUT_FACTOR; bool enable_global_idle_timeout_{false}; bool enable_per_stream_idle_timeout_{false}; bool enable_request_timeout_{false}; From af0818eee717f6cb651dbbbb9c76556b1bb5cd6e Mon Sep 17 00:00:00 2001 From: phlax Date: Fri, 17 Apr 2026 22:10:30 +0100 Subject: [PATCH 5/8] quic: Remove flaky ASSERT in `ShouldCreateOutgoingBidirectionalStream` (#44500) QuicSpdyClientSession::ShouldCreateOutgoingBidirectionalStream() can legitimately return false, but we return true unconditionally to avoid a nullptr deref in QuicHttpClientConnectionImpl::newStream. The existing ASSERT on the parent's return value was therefore incorrect and triggered flakes (e.g. in buffer_accounting_integration_test on MSAN). Drop the ASSERT and update the comment. Fixes #41526 --------- Signed-off-by: Ryan Northey --- source/common/quic/envoy_quic_client_session.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/source/common/quic/envoy_quic_client_session.h b/source/common/quic/envoy_quic_client_session.h index 07aec4450a62c..f53a62c4b7fd1 100644 --- a/source/common/quic/envoy_quic_client_session.h +++ b/source/common/quic/envoy_quic_client_session.h @@ -117,9 +117,10 @@ class EnvoyQuicClientSession : public QuicFilterManagerConnectionImpl, quic::QuicSpdyStream* CreateIncomingStream(quic::PendingStream* pending) override; std::unique_ptr CreateQuicCryptoStream() override; bool ShouldCreateOutgoingBidirectionalStream() override { - ASSERT(quic::QuicSpdyClientSession::ShouldCreateOutgoingBidirectionalStream()); - // Prefer creating an "invalid" stream outside of current stream bounds to - // crashing when dereferencing a nullptr in QuicHttpClientConnectionImpl::newStream + // quic::QuicSpdyClientSession::ShouldCreateOutgoingBidirectionalStream() + // might return false, but we want to create the stream anyway + // because otherwise we crash dereferencing a nullptr, so we + // don't even ask it, and just return true. return true; } // QuicFilterManagerConnectionImpl From 2fd63ee1e367a2c6aef2d32a5bc60063739ec2bd Mon Sep 17 00:00:00 2001 From: phlax Date: Fri, 17 Apr 2026 21:06:54 +0100 Subject: [PATCH 6/8] test: Attempt to fix flake in grpc transcoder test (#44510) by separating the tests Hopefully fix: #44427 Signed-off-by: Ryan Northey --- .../grpc_json_transcoder_integration_test.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc index 5a9990bbce34f..d13393560390c 100644 --- a/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc +++ b/test/extensions/filters/http/grpc_json_transcoder/grpc_json_transcoder_integration_test.cc @@ -1299,7 +1299,7 @@ TEST_P(GrpcJsonTranscoderIntegrationTest, UTF8) { false); } -TEST_P(GrpcJsonTranscoderIntegrationTest, DisableRequestValidation) { +TEST_P(GrpcJsonTranscoderIntegrationTest, DisableRequestValidationGrpcContentType) { HttpIntegrationTest::initialize(); // Transcoding does not occur from a request with the gRPC content type. @@ -1315,7 +1315,10 @@ TEST_P(GrpcJsonTranscoderIntegrationTest, DisableRequestValidation) { Http::TestResponseHeaderMapImpl{ {":status", "200"}, {"grpc-status", "5"}, {"grpc-message", "Shelf 9999 Not Found"}}, "", true, false, R"({ "theme" : "Children")"); +} +TEST_P(GrpcJsonTranscoderIntegrationTest, DisableRequestValidationUnknownPath) { + HttpIntegrationTest::initialize(); // Transcoding does not occur when unknown path is called. // HTTP Request to is passed directly to gRPC backend. // gRPC response is passed directly to HTTP client. @@ -1328,7 +1331,10 @@ TEST_P(GrpcJsonTranscoderIntegrationTest, DisableRequestValidation) { Http::TestResponseHeaderMapImpl{ {":status", "200"}, {"grpc-status", "5"}, {"grpc-message", "Shelf 9999 Not Found"}}, "", true, false, R"({ "theme" : "Children")"); +} +TEST_P(GrpcJsonTranscoderIntegrationTest, DisableRequestValidationUnknownQueryParam) { + HttpIntegrationTest::initialize(); // Transcoding does not occur when unknown query param is included. // HTTP Request to is passed directly to gRPC backend. // gRPC response is passed directly to HTTP client. From 761782291c00e3869b7aa53d0d05d61b2562f672 Mon Sep 17 00:00:00 2001 From: "Adi (Suissa) Peleg" Date: Fri, 30 Jan 2026 10:17:42 -0500 Subject: [PATCH 7/8] lrs: proper shutdown of the LRS stream (#43083) Signed-off-by: Adi Suissa-Peleg Signed-off-by: Ryan Northey --- changelogs/current.yaml | 3 +++ source/common/upstream/cluster_manager_impl.h | 1 + source/common/upstream/load_stats_reporter.cc | 11 +++++++++++ source/common/upstream/load_stats_reporter.h | 1 + test/common/upstream/load_stats_reporter_test.cc | 13 ++++++++++++- 5 files changed, 28 insertions(+), 1 deletion(-) diff --git a/changelogs/current.yaml b/changelogs/current.yaml index 222fd3633355d..82bc8356a6917 100644 --- a/changelogs/current.yaml +++ b/changelogs/current.yaml @@ -11,6 +11,9 @@ bug_fixes: - area: http2 change: | Apply nghttp2 CVE-2026-27135 patch. +- area: load_report + change: | + Fixed an issue upon load-report shutdown race with ADS stream. Introduced proper cleanup of the gRPC stream. removed_config_or_runtime: # *Normally occurs at the end of the* :ref:`deprecation period ` diff --git a/source/common/upstream/cluster_manager_impl.h b/source/common/upstream/cluster_manager_impl.h index fd4adb4d330e5..da43a4b0be6f4 100644 --- a/source/common/upstream/cluster_manager_impl.h +++ b/source/common/upstream/cluster_manager_impl.h @@ -312,6 +312,7 @@ class ClusterManagerImpl : public ClusterManager, // Make sure we destroy all potential outgoing connections before this returns. cds_api_.reset(); xds_manager_.shutdown(); + load_stats_reporter_.reset(); active_clusters_.clear(); warming_clusters_.clear(); updateClusterCounts(); diff --git a/source/common/upstream/load_stats_reporter.cc b/source/common/upstream/load_stats_reporter.cc index f5379ab1a5ca0..e5e041ec763ad 100644 --- a/source/common/upstream/load_stats_reporter.cc +++ b/source/common/upstream/load_stats_reporter.cc @@ -39,6 +39,17 @@ LoadStatsReporter::LoadStatsReporter(const LocalInfo::LocalInfo& local_info, establishNewStream(); } +LoadStatsReporter::~LoadStatsReporter() { + // Disable the timer. + ENVOY_LOG_MISC(info, "Destroying LoadStatsReporter"); + retry_timer_->disableTimer(); + response_timer_->disableTimer(); + if (stream_ != nullptr) { + stream_->resetStream(); + stream_ = nullptr; + } +} + void LoadStatsReporter::setRetryTimer() { ENVOY_LOG(info, "Load reporter stats stream/connection will retry in {} ms.", RETRY_DELAY_MS); retry_timer_->enableTimer(std::chrono::milliseconds(RETRY_DELAY_MS)); diff --git a/source/common/upstream/load_stats_reporter.h b/source/common/upstream/load_stats_reporter.h index 85d39d7bd8368..3682698a14437 100644 --- a/source/common/upstream/load_stats_reporter.h +++ b/source/common/upstream/load_stats_reporter.h @@ -36,6 +36,7 @@ class LoadStatsReporter LoadStatsReporter(const LocalInfo::LocalInfo& local_info, ClusterManager& cluster_manager, Stats::Scope& scope, Grpc::RawAsyncClientSharedPtr&& async_client, Event::Dispatcher& dispatcher); + virtual ~LoadStatsReporter(); // Grpc::AsyncStreamCallbacks void onCreateInitialMetadata(Http::RequestHeaderMap& metadata) override; diff --git a/test/common/upstream/load_stats_reporter_test.cc b/test/common/upstream/load_stats_reporter_test.cc index f959e426fd8df..0a7b7046e27c4 100644 --- a/test/common/upstream/load_stats_reporter_test.cc +++ b/test/common/upstream/load_stats_reporter_test.cc @@ -37,6 +37,17 @@ class LoadStatsReporterTest : public testing::Test { : retry_timer_(new Event::MockTimer()), response_timer_(new Event::MockTimer()), async_client_(new Grpc::MockAsyncClient()) {} + void TearDown() override { + if (load_stats_reporter_ != nullptr) { + // Validate that LoadStatsReporter correctly shuts down by disabling + // timers and resetting the stream. + EXPECT_CALL(*retry_timer_, disableTimer()); + EXPECT_CALL(*response_timer_, disableTimer()); + EXPECT_CALL(async_stream_, resetStream()); + load_stats_reporter_.reset(); + } + } + void createLoadStatsReporter() { InSequence s; EXPECT_CALL(dispatcher_, createTimer_(_)).WillOnce(Invoke([this](Event::TimerCb timer_cb) { @@ -97,7 +108,6 @@ class LoadStatsReporterTest : public testing::Test { NiceMock cm_; Event::MockDispatcher dispatcher_; Stats::IsolatedStoreImpl stats_store_; - std::unique_ptr load_stats_reporter_; Event::MockTimer* retry_timer_; Event::TimerCb retry_timer_cb_; Event::MockTimer* response_timer_; @@ -105,6 +115,7 @@ class LoadStatsReporterTest : public testing::Test { Grpc::MockAsyncStream async_stream_; Grpc::MockAsyncClient* async_client_; NiceMock local_info_; + std::unique_ptr load_stats_reporter_; }; // Validate that stream creation results in a timer based retry. From 4fe73b90dfec10776485bd769f630077ce6bdf9c Mon Sep 17 00:00:00 2001 From: botengyao Date: Mon, 2 Mar 2026 16:37:24 -0500 Subject: [PATCH 8/8] dns test: improve tests by teardown the server and upstream (#43711) Signed-off-by: Ryan Northey --- .../filters/udp/dns_filter/dns_filter_integration_test.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/test/extensions/filters/udp/dns_filter/dns_filter_integration_test.cc b/test/extensions/filters/udp/dns_filter/dns_filter_integration_test.cc index d413533514a5d..cfa4b23944e22 100644 --- a/test/extensions/filters/udp/dns_filter/dns_filter_integration_test.cc +++ b/test/extensions/filters/udp/dns_filter/dns_filter_integration_test.cc @@ -89,6 +89,13 @@ class DnsFilterIntegrationTest : public testing::TestWithParam