From fe40cc3d31876cc7cdd11514f5ccbe9b9573a58d Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 5 Apr 2021 14:29:20 -0400 Subject: [PATCH 1/4] quic: turning up http2 upstream test for http3 Signed-off-by: Alyssa Wilk --- .../common/quic/envoy_quic_client_stream.cc | 1 - test/integration/BUILD | 3 +- .../http2_upstream_integration_test.cc | 47 +++++++++++++++---- .../http2_upstream_integration_test.h | 13 ++--- 4 files changed, 44 insertions(+), 20 deletions(-) diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index b9aae50c0c249..9c399449e79c6 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -235,7 +235,6 @@ void EnvoyQuicClientStream::OnTrailingHeadersComplete(bool fin, size_t frame_len void EnvoyQuicClientStream::maybeDecodeTrailers() { if (sequencer()->IsClosed() && !FinishedReadingTrailers()) { - ASSERT(!received_trailers().empty()); // Only decode trailers after finishing decoding body. end_stream_decoded_ = true; response_decoder_->decodeTrailers( diff --git a/test/integration/BUILD b/test/integration/BUILD index 40639457921e7..ed7dfc17e9f52 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -480,8 +480,9 @@ envoy_cc_test( "http2_upstream_integration_test.cc", "http2_upstream_integration_test.h", ], + shard_count = 2, deps = [ - ":http_integration_lib", + ":http_protocol_integration_lib", "//source/common/http:header_map_lib", "//source/extensions/access_loggers/grpc:http_config", "//source/extensions/filters/http/buffer:config", diff --git a/test/integration/http2_upstream_integration_test.cc b/test/integration/http2_upstream_integration_test.cc index 2eb725d5283be..1331242dc1f07 100644 --- a/test/integration/http2_upstream_integration_test.cc +++ b/test/integration/http2_upstream_integration_test.cc @@ -15,9 +15,17 @@ namespace Envoy { -INSTANTIATE_TEST_SUITE_P(IpVersions, Http2UpstreamIntegrationTest, - testing::ValuesIn(TestEnvironment::getIpVersionsForTest()), - TestUtility::ipTestParamsToString); +// TODO(#14829) categorize or fix all failures. +#define EXCLUDE_UPSTREAM_HTTP3 \ + if (upstreamProtocol() == FakeHttpConnection::Type::HTTP3) { \ + return; \ + } + +INSTANTIATE_TEST_SUITE_P(Protocols, Http2UpstreamIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( + {Http::CodecClient::Type::HTTP2}, + {FakeHttpConnection::Type::HTTP2, FakeHttpConnection::Type::HTTP3})), + HttpProtocolIntegrationTest::protocolTestParamsToString); TEST_P(Http2UpstreamIntegrationTest, RouterRequestAndResponseWithBodyNoBuffer) { testRouterRequestAndResponseWithBody(1024, 512, false); @@ -32,10 +40,12 @@ TEST_P(Http2UpstreamIntegrationTest, RouterHeaderOnlyRequestAndResponseNoBuffer) } TEST_P(Http2UpstreamIntegrationTest, RouterUpstreamDisconnectBeforeRequestcomplete) { + EXCLUDE_UPSTREAM_HTTP3; // Close loop. testRouterUpstreamDisconnectBeforeRequestComplete(); } TEST_P(Http2UpstreamIntegrationTest, RouterUpstreamDisconnectBeforeResponseComplete) { + EXCLUDE_UPSTREAM_HTTP3; // Close loop. testRouterUpstreamDisconnectBeforeResponseComplete(); } @@ -51,11 +61,20 @@ TEST_P(Http2UpstreamIntegrationTest, RouterUpstreamResponseBeforeRequestComplete testRouterUpstreamResponseBeforeRequestComplete(); } -TEST_P(Http2UpstreamIntegrationTest, Retry) { testRetry(); } +TEST_P(Http2UpstreamIntegrationTest, Retry) { + EXCLUDE_UPSTREAM_HTTP3; // CHECK failed: max_plaintext_size_ (=18) >= PacketSize() (=20) + testRetry(); +} -TEST_P(Http2UpstreamIntegrationTest, GrpcRetry) { testGrpcRetry(); } +TEST_P(Http2UpstreamIntegrationTest, GrpcRetry) { + EXCLUDE_UPSTREAM_HTTP3; // CHECK failed: max_plaintext_size_ (=18) >= PacketSize() (=20) + testGrpcRetry(); +} -TEST_P(Http2UpstreamIntegrationTest, Trailers) { testTrailers(1024, 2048, true, true); } +TEST_P(Http2UpstreamIntegrationTest, Trailers) { + EXCLUDE_UPSTREAM_HTTP3; // CHECK failed: max_plaintext_size_ (=18) >= PacketSize() (=20) + testTrailers(1024, 2048, true, true); +} TEST_P(Http2UpstreamIntegrationTest, TestSchemeAndXFP) { autonomous_upstream_ = true; @@ -226,6 +245,7 @@ TEST_P(Http2UpstreamIntegrationTest, SimultaneousRequest) { } TEST_P(Http2UpstreamIntegrationTest, LargeSimultaneousRequestWithBufferLimits) { + EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. simultaneousRequest(1024 * 20, 1024 * 14 + 2, 1024 * 10 + 5, 1024 * 16); } @@ -236,6 +256,7 @@ TEST_P(Http2UpstreamIntegrationTest, SimultaneousRequestAlpn) { } TEST_P(Http2UpstreamIntegrationTest, LargeSimultaneousRequestWithBufferLimitsAlpn) { + EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. use_alpn_ = true; config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. simultaneousRequest(1024 * 20, 1024 * 14 + 2, 1024 * 10 + 5, 1024 * 16); @@ -290,21 +311,25 @@ TEST_P(Http2UpstreamIntegrationTest, ManySimultaneousRequest) { } TEST_P(Http2UpstreamIntegrationTest, ManyLargeSimultaneousRequestWithBufferLimits) { + EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. manySimultaneousRequests(1024 * 20, 1024 * 20); } TEST_P(Http2UpstreamIntegrationTest, ManyLargeSimultaneousRequestWithRandomBackup) { - config_helper_.addFilter(R"EOF( - name: random-pause-filter + EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. + config_helper_.addFilter( + fmt::format(R"EOF( + name: pause-filter{} typed_config: - "@type": type.googleapis.com/google.protobuf.Empty - )EOF"); + "@type": type.googleapis.com/google.protobuf.Empty)EOF", + downstreamProtocol() == Http::CodecClient::Type::HTTP3 ? "-for-quic" : "")); manySimultaneousRequests(1024 * 20, 1024 * 20); } TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) { + EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. const uint32_t num_requests = 20; std::vector encoders; @@ -423,6 +448,8 @@ name: router // Tests the default limit for the number of response headers is 100. Results in a stream reset if // exceeds. TEST_P(Http2UpstreamIntegrationTest, TestManyResponseHeadersRejected) { + EXCLUDE_UPSTREAM_HTTP3; // no 503. + // Default limit for response headers is 100. initialize(); codec_client_ = makeHttpConnection(lookupPort("http")); diff --git a/test/integration/http2_upstream_integration_test.h b/test/integration/http2_upstream_integration_test.h index 754c6359907d8..7f308de1b82cf 100644 --- a/test/integration/http2_upstream_integration_test.h +++ b/test/integration/http2_upstream_integration_test.h @@ -1,22 +1,19 @@ #pragma once -#include "test/integration/http_integration.h" +#include "test/integration/http_protocol_integration.h" #include "gtest/gtest.h" namespace Envoy { -class Http2UpstreamIntegrationTest : public testing::TestWithParam, - public HttpIntegrationTest { +class Http2UpstreamIntegrationTest : public HttpProtocolIntegrationTest { public: - Http2UpstreamIntegrationTest() - : HttpIntegrationTest(Http::CodecClient::Type::HTTP2, GetParam()) {} + Http2UpstreamIntegrationTest() {} void SetUp() override { - setDownstreamProtocol(Http::CodecClient::Type::HTTP2); - setUpstreamProtocol(FakeHttpConnection::Type::HTTP2); + HttpProtocolIntegrationTest::SetUp(); upstream_tls_ = true; - config_helper_.configureUpstreamTls(use_alpn_); + config_helper_.configureUpstreamTls(use_alpn_, upstreamProtocol() == FakeHttpConnection::Type::HTTP3); } void initialize() override { HttpIntegrationTest::initialize(); } From 8571c8641a7bf393d0a085390651875b121df7d9 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 5 Apr 2021 15:01:56 -0400 Subject: [PATCH 2/4] http3: turning up upstream integration test Signed-off-by: Alyssa Wilk --- test/integration/BUILD | 6 +++--- ...ion_test.cc => multiplexed_upstream_integration_test.cc} | 2 +- ...ation_test.h => multiplexed_upstream_integration_test.h} | 3 ++- 3 files changed, 6 insertions(+), 5 deletions(-) rename test/integration/{http2_upstream_integration_test.cc => multiplexed_upstream_integration_test.cc} (99%) rename test/integration/{http2_upstream_integration_test.h => multiplexed_upstream_integration_test.h} (83%) diff --git a/test/integration/BUILD b/test/integration/BUILD index ed7dfc17e9f52..55f87c736b6fe 100644 --- a/test/integration/BUILD +++ b/test/integration/BUILD @@ -475,10 +475,10 @@ envoy_cc_test( ) envoy_cc_test( - name = "http2_upstream_integration_test", + name = "multiplexed_upstream_integration_test", srcs = [ - "http2_upstream_integration_test.cc", - "http2_upstream_integration_test.h", + "multiplexed_upstream_integration_test.cc", + "multiplexed_upstream_integration_test.h", ], shard_count = 2, deps = [ diff --git a/test/integration/http2_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc similarity index 99% rename from test/integration/http2_upstream_integration_test.cc rename to test/integration/multiplexed_upstream_integration_test.cc index 1331242dc1f07..33d6d8d76ece3 100644 --- a/test/integration/http2_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -1,4 +1,4 @@ -#include "test/integration/http2_upstream_integration_test.h" +#include "test/integration/multiplexed_upstream_integration_test.h" #include diff --git a/test/integration/http2_upstream_integration_test.h b/test/integration/multiplexed_upstream_integration_test.h similarity index 83% rename from test/integration/http2_upstream_integration_test.h rename to test/integration/multiplexed_upstream_integration_test.h index 7f308de1b82cf..12dafa128ea09 100644 --- a/test/integration/http2_upstream_integration_test.h +++ b/test/integration/multiplexed_upstream_integration_test.h @@ -13,7 +13,8 @@ class Http2UpstreamIntegrationTest : public HttpProtocolIntegrationTest { HttpProtocolIntegrationTest::SetUp(); upstream_tls_ = true; - config_helper_.configureUpstreamTls(use_alpn_, upstreamProtocol() == FakeHttpConnection::Type::HTTP3); + config_helper_.configureUpstreamTls(use_alpn_, + upstreamProtocol() == FakeHttpConnection::Type::HTTP3); } void initialize() override { HttpIntegrationTest::initialize(); } From 8f16a61dfdb826e14071ffe9b42354ee14df2de5 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 5 Apr 2021 16:26:46 -0400 Subject: [PATCH 3/4] more tests! Signed-off-by: Alyssa Wilk --- source/common/quic/envoy_quic_client_stream.cc | 2 -- test/integration/multiplexed_upstream_integration_test.cc | 8 +++----- test/integration/protocol_integration_test.cc | 1 - 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/source/common/quic/envoy_quic_client_stream.cc b/source/common/quic/envoy_quic_client_stream.cc index 9c399449e79c6..2e535ecf8a13e 100644 --- a/source/common/quic/envoy_quic_client_stream.cc +++ b/source/common/quic/envoy_quic_client_stream.cc @@ -110,8 +110,6 @@ void EnvoyQuicClientStream::resetStream(Http::StreamResetReason reason) { } void EnvoyQuicClientStream::switchStreamBlockState(bool should_block) { - ASSERT(FinishedReadingHeaders(), - "Upper stream buffer limit is reached before response body is delivered."); if (should_block) { sequencer()->SetBlockedUntilFlush(); } else { diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc index 33d6d8d76ece3..e365485d0b1e2 100644 --- a/test/integration/multiplexed_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -245,7 +245,6 @@ TEST_P(Http2UpstreamIntegrationTest, SimultaneousRequest) { } TEST_P(Http2UpstreamIntegrationTest, LargeSimultaneousRequestWithBufferLimits) { - EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. simultaneousRequest(1024 * 20, 1024 * 14 + 2, 1024 * 10 + 5, 1024 * 16); } @@ -256,7 +255,6 @@ TEST_P(Http2UpstreamIntegrationTest, SimultaneousRequestAlpn) { } TEST_P(Http2UpstreamIntegrationTest, LargeSimultaneousRequestWithBufferLimitsAlpn) { - EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. use_alpn_ = true; config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. simultaneousRequest(1024 * 20, 1024 * 14 + 2, 1024 * 10 + 5, 1024 * 16); @@ -311,13 +309,13 @@ TEST_P(Http2UpstreamIntegrationTest, ManySimultaneousRequest) { } TEST_P(Http2UpstreamIntegrationTest, ManyLargeSimultaneousRequestWithBufferLimits) { - EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. + EXCLUDE_UPSTREAM_HTTP3; // quic_stream_sequencer.cc:235 CHECK failed: !blocked_. config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. manySimultaneousRequests(1024 * 20, 1024 * 20); } TEST_P(Http2UpstreamIntegrationTest, ManyLargeSimultaneousRequestWithRandomBackup) { - EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. + EXCLUDE_UPSTREAM_HTTP3; // fails: no 200s. config_helper_.addFilter( fmt::format(R"EOF( name: pause-filter{} @@ -329,7 +327,7 @@ TEST_P(Http2UpstreamIntegrationTest, ManyLargeSimultaneousRequestWithRandomBacku } TEST_P(Http2UpstreamIntegrationTest, UpstreamConnectionCloseWithManyStreams) { - EXCLUDE_UPSTREAM_HTTP3; // Upper stream buffer limit is reached before response body is delivered. + EXCLUDE_UPSTREAM_HTTP3; // Close loop. config_helper_.setBufferLimits(1024, 1024); // Set buffer limits upstream and downstream. const uint32_t num_requests = 20; std::vector encoders; diff --git a/test/integration/protocol_integration_test.cc b/test/integration/protocol_integration_test.cc index 1e5687ae79f96..1b6a9769baf9e 100644 --- a/test/integration/protocol_integration_test.cc +++ b/test/integration/protocol_integration_test.cc @@ -1081,7 +1081,6 @@ TEST_P(ProtocolIntegrationTest, EnvoyProxyingLateMultiple1xx) { TEST_P(ProtocolIntegrationTest, TwoRequests) { testTwoRequests(); } TEST_P(ProtocolIntegrationTest, TwoRequestsWithForcedBackup) { - // hits assert "Upper stream buffer limit is reached before response body is delivered." EXCLUDE_UPSTREAM_HTTP3; testTwoRequests(true); } From dd9311555f3c4c55cf56500812263cae7c262e11 Mon Sep 17 00:00:00 2001 From: Alyssa Wilk Date: Mon, 5 Apr 2021 18:28:30 -0400 Subject: [PATCH 4/4] ci Signed-off-by: Alyssa Wilk --- .../multiplexed_upstream_integration_test.cc | 11 +++++++++-- .../multiplexed_upstream_integration_test.h | 2 -- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/test/integration/multiplexed_upstream_integration_test.cc b/test/integration/multiplexed_upstream_integration_test.cc index e365485d0b1e2..c10d7be9f56ba 100644 --- a/test/integration/multiplexed_upstream_integration_test.cc +++ b/test/integration/multiplexed_upstream_integration_test.cc @@ -23,10 +23,17 @@ namespace Envoy { INSTANTIATE_TEST_SUITE_P(Protocols, Http2UpstreamIntegrationTest, testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( - {Http::CodecClient::Type::HTTP2}, - {FakeHttpConnection::Type::HTTP2, FakeHttpConnection::Type::HTTP3})), + {Http::CodecClient::Type::HTTP2}, {FakeHttpConnection::Type::HTTP2})), HttpProtocolIntegrationTest::protocolTestParamsToString); +// TODO(alyssawilk) move #defines into getProtocolTestParams in a follow-up +#ifdef ENVOY_ENABLE_QUIC +INSTANTIATE_TEST_SUITE_P(ProtocolsWithQuic, Http2UpstreamIntegrationTest, + testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams( + {Http::CodecClient::Type::HTTP2}, {FakeHttpConnection::Type::HTTP3})), + HttpProtocolIntegrationTest::protocolTestParamsToString); +#endif + TEST_P(Http2UpstreamIntegrationTest, RouterRequestAndResponseWithBodyNoBuffer) { testRouterRequestAndResponseWithBody(1024, 512, false); } diff --git a/test/integration/multiplexed_upstream_integration_test.h b/test/integration/multiplexed_upstream_integration_test.h index 12dafa128ea09..09d51ecf5fcf8 100644 --- a/test/integration/multiplexed_upstream_integration_test.h +++ b/test/integration/multiplexed_upstream_integration_test.h @@ -7,8 +7,6 @@ namespace Envoy { class Http2UpstreamIntegrationTest : public HttpProtocolIntegrationTest { public: - Http2UpstreamIntegrationTest() {} - void SetUp() override { HttpProtocolIntegrationTest::SetUp();