Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bp/1.25] test: fix flaky test DownstreamProtocolIntegrationTest.HandleDownstreamSocketFail (#27546) #28013

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion test/integration/buffer_accounting_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ class Http2BufferWatermarksTest
}

Http2BufferWatermarksTest()
: HttpIntegrationTest(
: SocketInterfaceSwap(Network::Socket::Type::Stream),
HttpIntegrationTest(
std::get<0>(GetParam()).downstream_protocol, std::get<0>(GetParam()).version,
ConfigHelper::httpProxyConfig(
/*downstream_is_quic=*/std::get<0>(GetParam()).downstream_protocol ==
Expand Down
10 changes: 9 additions & 1 deletion test/integration/filters/test_socket_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,12 @@ class TestIoSocketHandle : public Test::IoSocketHandlePlatformImpl {
TestIoSocketHandle(WriteOverrideProc write_override_proc, os_fd_t fd = INVALID_SOCKET,
bool socket_v6only = false, absl::optional<int> domain = absl::nullopt)
: Test::IoSocketHandlePlatformImpl(fd, socket_v6only, domain),
write_override_(write_override_proc) {}
write_override_(write_override_proc) {
int type;
socklen_t length = sizeof(int);
EXPECT_EQ(0, getOption(SOL_SOCKET, SO_TYPE, &type, &length).return_value_);
socket_type_ = type == SOCK_STREAM ? Socket::Type::Stream : Socket::Type::Datagram;
}

void initializeFileEvent(Event::Dispatcher& dispatcher, Event::FileReadyCb cb,
Event::FileTriggerType trigger, uint32_t events) override {
Expand Down Expand Up @@ -59,6 +64,8 @@ class TestIoSocketHandle : public Test::IoSocketHandlePlatformImpl {
return Test::IoSocketHandlePlatformImpl::peerAddress();
}

Socket::Type getSocketType() const { return socket_type_; }

private:
IoHandlePtr accept(struct sockaddr* addr, socklen_t* addrlen) override;
Api::IoCallUint64Result writev(const Buffer::RawSlice* slices, uint64_t num_slice) override;
Expand All @@ -72,6 +79,7 @@ class TestIoSocketHandle : public Test::IoSocketHandlePlatformImpl {
const WriteOverrideProc write_override_;
absl::Mutex mutex_;
Event::Dispatcher* dispatcher_ ABSL_GUARDED_BY(mutex_) = nullptr;
Socket::Type socket_type_;
};

/**
Expand Down
4 changes: 3 additions & 1 deletion test/integration/http2_flood_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,9 @@ class Http2FloodMitigationTest
public testing::TestWithParam<std::tuple<Network::Address::IpVersion, bool, bool>>,
public Http2RawFrameIntegrationTest {
public:
Http2FloodMitigationTest() : Http2RawFrameIntegrationTest(std::get<0>(GetParam())) {
Http2FloodMitigationTest()
: SocketInterfaceSwap(Network::Socket::Type::Stream),
Http2RawFrameIntegrationTest(std::get<0>(GetParam())) {
// This test tracks the number of buffers created, and the tag extraction check uses some
// buffers, so disable it in this test.
skip_tag_extraction_rule_check_ = true;
Expand Down
8 changes: 7 additions & 1 deletion test/integration/multiplexed_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2166,7 +2166,13 @@ TEST_P(MultiplexedIntegrationTest, Reset101SwitchProtocolResponse) {
// Ordering of inheritance is important here, SocketInterfaceSwap must be
// destroyed after HttpProtocolIntegrationTest.
class SocketSwappableMultiplexedIntegrationTest : public SocketInterfaceSwap,
public HttpProtocolIntegrationTest {};
public HttpProtocolIntegrationTest {
public:
SocketSwappableMultiplexedIntegrationTest()
: SocketInterfaceSwap(GetParam().downstream_protocol == Http::CodecType::HTTP3
? Network::Socket::Type::Datagram
: Network::Socket::Type::Stream) {}
};

INSTANTIATE_TEST_SUITE_P(IpVersions, SocketSwappableMultiplexedIntegrationTest,
testing::ValuesIn(HttpProtocolIntegrationTest::getProtocolTestParams(
Expand Down
8 changes: 6 additions & 2 deletions test/integration/protocol_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3981,7 +3981,9 @@ TEST_P(DownstreamProtocolIntegrationTest, HandleDownstreamSocketFail) {
NoUdpGso reject_gso_;
TestThreadsafeSingletonInjector<Api::OsSysCallsImpl> os_calls{&reject_gso_};
ASSERT(!Api::OsSysCallsSingleton::get().supportsUdpGso());
SocketInterfaceSwap socket_swap;
SocketInterfaceSwap socket_swap(downstreamProtocol() == Http::CodecType::HTTP3
? Network::Socket::Type::Datagram
: Network::Socket::Type::Stream);

initialize();
codec_client_ = makeHttpConnection(lookupPort("http"));
Expand Down Expand Up @@ -4015,7 +4017,9 @@ TEST_P(DownstreamProtocolIntegrationTest, HandleDownstreamSocketFail) {
}

TEST_P(ProtocolIntegrationTest, HandleUpstreamSocketFail) {
SocketInterfaceSwap socket_swap;
SocketInterfaceSwap socket_swap(upstreamProtocol() == Http::CodecType::HTTP3
? Network::Socket::Type::Datagram
: Network::Socket::Type::Stream);

useAccessLog("%RESPONSE_CODE_DETAILS%");
initialize();
Expand Down
4 changes: 3 additions & 1 deletion test/integration/shadow_policy_integration_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ namespace {
class ShadowPolicyIntegrationTest : public testing::TestWithParam<Network::Address::IpVersion>,
public HttpIntegrationTest {
public:
ShadowPolicyIntegrationTest() : HttpIntegrationTest(Http::CodecType::HTTP2, GetParam()) {
ShadowPolicyIntegrationTest()
: HttpIntegrationTest(Http::CodecType::HTTP2, GetParam()),
SocketInterfaceSwap(Network::Socket::Type::Stream) {
setUpstreamProtocol(Http::CodecType::HTTP2);
autonomous_upstream_ = true;
setUpstreamCount(2);
Expand Down
3 changes: 2 additions & 1 deletion test/integration/socket_interface_swap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ namespace Envoy {

void preserveIoError(Api::IoError*) {}

SocketInterfaceSwap::SocketInterfaceSwap() {
SocketInterfaceSwap::SocketInterfaceSwap(Network::Socket::Type socket_type)
: write_matcher_(std::make_shared<IoHandleMatcher>(socket_type)) {
Envoy::Network::SocketInterfaceSingleton::clear();
test_socket_interface_loader_ = std::make_unique<Envoy::Network::SocketInterfaceLoader>(
std::make_unique<Envoy::Network::TestSocketInterface>(
Expand Down
12 changes: 8 additions & 4 deletions test/integration/socket_interface_swap.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@ class SocketInterfaceSwap {
// Object of this class hold the state determining the IoHandle which
// should return the supplied return from the `writev` or `sendmsg` calls.
struct IoHandleMatcher {
explicit IoHandleMatcher(Network::Socket::Type type) : socket_type_(type) {}

Network::IoSocketError* returnOverride(Envoy::Network::TestIoSocketHandle* io_handle) {
absl::MutexLock lock(&mutex_);
if (error_ && (io_handle->localAddress()->ip()->port() == src_port_ ||
(dst_port_ && io_handle->peerAddress()->ip()->port() == dst_port_))) {
if (socket_type_ == io_handle->getSocketType() && error_ &&
(io_handle->localAddress()->ip()->port() == src_port_ ||
(dst_port_ && io_handle->peerAddress()->ip()->port() == dst_port_))) {
ASSERT(matched_iohandle_ == nullptr || matched_iohandle_ == io_handle,
"Matched multiple io_handles, expected at most one to match.");
matched_iohandle_ = io_handle;
Expand Down Expand Up @@ -59,9 +62,10 @@ class SocketInterfaceSwap {
uint32_t dst_port_ ABSL_GUARDED_BY(mutex_) = 0;
Network::IoSocketError* error_ ABSL_GUARDED_BY(mutex_) = nullptr;
Network::TestIoSocketHandle* matched_iohandle_{};
Network::Socket::Type socket_type_;
};

SocketInterfaceSwap();
explicit SocketInterfaceSwap(Network::Socket::Type socket_type);

~SocketInterfaceSwap() {
test_socket_interface_loader_.reset();
Expand All @@ -70,7 +74,7 @@ class SocketInterfaceSwap {

Envoy::Network::SocketInterface* const previous_socket_interface_{
Envoy::Network::SocketInterfaceSingleton::getExisting()};
std::shared_ptr<IoHandleMatcher> write_matcher_{std::make_shared<IoHandleMatcher>()};
std::shared_ptr<IoHandleMatcher> write_matcher_;
std::unique_ptr<Envoy::Network::SocketInterfaceLoader> test_socket_interface_loader_;
};

Expand Down
Loading