diff --git a/envoy/api/os_sys_calls.h b/envoy/api/os_sys_calls.h index 6b0eaf6c61cf7..84a5272874b52 100644 --- a/envoy/api/os_sys_calls.h +++ b/envoy/api/os_sys_calls.h @@ -1,6 +1,5 @@ #pragma once -#include #include #include @@ -301,14 +300,9 @@ class OsSysCalls { virtual void freeaddrinfo(addrinfo* res) PURE; /** - * @see man getrlimit + * @see Increase soft file descriptor limit to match hard limit. */ - virtual SysCallIntResult getrlimit(int resource, struct rlimit* rlim) PURE; - - /** - * @see man setrlimit - */ - virtual SysCallIntResult setrlimit(int resource, const struct rlimit* rlim) PURE; + virtual SysCallIntResult raiseFileLimits() PURE; }; using OsSysCallsPtr = std::unique_ptr; diff --git a/source/common/api/posix/os_sys_calls_impl.cc b/source/common/api/posix/os_sys_calls_impl.cc index f8631eee19f27..576aaf1cfa5bd 100644 --- a/source/common/api/posix/os_sys_calls_impl.cc +++ b/source/common/api/posix/os_sys_calls_impl.cc @@ -3,6 +3,7 @@ #include #include #include +#include #include #include @@ -463,14 +464,20 @@ SysCallIntResult OsSysCallsImpl::getaddrinfo(const char* node, const char* servi void OsSysCallsImpl::freeaddrinfo(addrinfo* res) { ::freeaddrinfo(res); } -SysCallIntResult OsSysCallsImpl::getrlimit(int resource, struct rlimit* rlim) { - const int rc = ::getrlimit(resource, rlim); - return {rc, errno}; -} - -SysCallIntResult OsSysCallsImpl::setrlimit(int resource, const struct rlimit* rlim) { - const int rc = ::setrlimit(resource, rlim); - return {rc, errno}; +SysCallIntResult OsSysCallsImpl::raiseFileLimits() { + struct rlimit rlim; + if (const int result = ::getrlimit(RLIMIT_NOFILE, &rlim); result != 0) { + return {result, errno}; + } + const auto old = rlim.rlim_cur; + if (old == rlim.rlim_max) { + return {0, 0}; + } + rlim.rlim_cur = rlim.rlim_max; + if (const int result = ::setrlimit(RLIMIT_NOFILE, &rlim); result != 0) { + return {result, errno}; + } + return {0, 0}; } } // namespace Api diff --git a/source/common/api/posix/os_sys_calls_impl.h b/source/common/api/posix/os_sys_calls_impl.h index 5ee3059e89a06..9f3d9a8125cea 100644 --- a/source/common/api/posix/os_sys_calls_impl.h +++ b/source/common/api/posix/os_sys_calls_impl.h @@ -70,8 +70,7 @@ class OsSysCallsImpl : public OsSysCalls { SysCallIntResult getaddrinfo(const char* node, const char* service, const addrinfo* hints, addrinfo** res) override; void freeaddrinfo(addrinfo* res) override; - SysCallIntResult getrlimit(int resource, struct rlimit* rlim) override; - SysCallIntResult setrlimit(int resource, const struct rlimit* rlim) override; + SysCallIntResult raiseFileLimits() override; }; using OsSysCallsSingleton = ThreadSafeSingleton; diff --git a/source/common/api/win32/os_sys_calls_impl.cc b/source/common/api/win32/os_sys_calls_impl.cc index 1e932c27bb284..f708399e71a36 100644 --- a/source/common/api/win32/os_sys_calls_impl.cc +++ b/source/common/api/win32/os_sys_calls_impl.cc @@ -487,14 +487,10 @@ SysCallIntResult OsSysCallsImpl::getaddrinfo(const char* node, const char* servi void OsSysCallsImpl::freeaddrinfo(addrinfo* res) { ::freeaddrinfo(res); } -SysCallIntResult OsSysCallsImpl::getrlimit(int resource, struct rlimit* rlim) { - // Windows does not support all resource limits. +SysCallIntResult OsSysCallsImpl::raiseFileLimits() { + // Windows does not support this functionality. return {0, 0}; } -SysCallIntResult OsSysCallsImpl::setrlimit(int resource, const struct rlimit* rlim) { - PANIC("not implemented"); -} - } // namespace Api } // namespace Envoy diff --git a/source/common/api/win32/os_sys_calls_impl.h b/source/common/api/win32/os_sys_calls_impl.h index 97d649c557499..a419e1ad82062 100644 --- a/source/common/api/win32/os_sys_calls_impl.h +++ b/source/common/api/win32/os_sys_calls_impl.h @@ -72,8 +72,7 @@ class OsSysCallsImpl : public OsSysCalls { SysCallIntResult getaddrinfo(const char* node, const char* service, const addrinfo* hints, addrinfo** res) override; void freeaddrinfo(addrinfo* res) override; - SysCallIntResult getrlimit(int resource, struct rlimit* rlim) override; - SysCallIntResult setrlimit(int resource, const struct rlimit* rlim) override; + SysCallIntResult raiseFileLimits() override; }; using OsSysCallsSingleton = ThreadSafeSingleton; diff --git a/source/server/server.cc b/source/server/server.cc index 0ab8331c4788f..0e85d1e8a6e83 100644 --- a/source/server/server.cc +++ b/source/server/server.cc @@ -401,24 +401,12 @@ void InstanceUtil::raiseFileLimits() { if (!Runtime::runtimeFeatureEnabled("envoy.restart_features.raise_file_limits")) { return; } - struct rlimit rlim; - if (const auto result = Api::OsSysCallsSingleton::get().getrlimit(RLIMIT_NOFILE, &rlim); + if (const auto result = Api::OsSysCallsSingleton::get().raiseFileLimits(); result.return_value_ != 0) { - ENVOY_LOG(warn, "Failed to read file descriptor limit, error {}.", errorDetails(result.errno_)); - return; - } - const auto old = rlim.rlim_cur; - if (old == rlim.rlim_max) { - return; - } - rlim.rlim_cur = rlim.rlim_max; - if (const auto result = Api::OsSysCallsSingleton::get().setrlimit(RLIMIT_NOFILE, &rlim); - result.return_value_ != 0) { - ENVOY_LOG(warn, "Failed to raise file descriptor limit to maximum, error {}.", + ENVOY_LOG(warn, "Failed to raise file descriptor limit, error {}.", errorDetails(result.errno_)); return; } - ENVOY_LOG(info, "Raised file descriptor limits from {} to {}.", old, rlim.rlim_max); } void InstanceBase::initialize(Network::Address::InstanceConstSharedPtr local_address, diff --git a/test/common/api/os_sys_calls_test.cc b/test/common/api/os_sys_calls_test.cc index 261029a08c77d..83eeb7fda5368 100644 --- a/test/common/api/os_sys_calls_test.cc +++ b/test/common/api/os_sys_calls_test.cc @@ -90,10 +90,9 @@ TEST(OsSyscallsTest, IoCtlInvalidFd) { EXPECT_NE(0, Api::OsSysCallsSingleton::get().ioctl(0, 0, nullptr, 0, nullptr, 0, nullptr).errno_); } -TEST(OsSyscallsTest, Setrlimit) { - // Not all environments support it, but it is safe to read limits. - struct rlimit rlim; - Api::OsSysCallsSingleton::get().getrlimit(RLIMIT_NOFILE, &rlim); +TEST(OsSyscallsTest, RaiseFileLimits) { + // Not all environments support it, but it is safe to call it. + Api::OsSysCallsSingleton::get().raiseFileLimits(); } } // namespace Envoy diff --git a/test/coverage.yaml b/test/coverage.yaml index 7cd528481880a..03e014ba03fd2 100644 --- a/test/coverage.yaml +++ b/test/coverage.yaml @@ -4,8 +4,8 @@ thresholds: directories: source/common: 96.4 - source/common/api: 95.3 # some syscalls require sandboxing - source/common/api/posix: 94.9 # setns requires Linux CAP_NET_ADMIN privileges + source/common/api: 94.9 # some syscalls require sandboxing + source/common/api/posix: 94.6 # setns requires Linux CAP_NET_ADMIN privileges, no tests for set/getrlimit syscalls errors source/common/crypto: 91.2 # Static singleton initialization and OpenSSL internal error paths not testable source/common/filesystem/posix: 96.5 # FileReadToEndNotReadable fails in some env; createPath can't test all failure branches. source/common/http: 96.5 diff --git a/test/server/server_test.cc b/test/server/server_test.cc index 08373dda362c8..ef5fcd12ce3ad 100644 --- a/test/server/server_test.cc +++ b/test/server/server_test.cc @@ -1,3 +1,6 @@ +#ifndef WIN32 +#include +#endif #include #include #include @@ -173,58 +176,29 @@ TEST(ServerInstanceUtil, flushImportModeUninitializedGauges) { InstanceUtil::flushMetricsToSinks(sinks, store, cm, time_system); } +#ifndef WIN32 TEST(ServerInstanceUtil, RaiseFileLimits) { - Api::MockOsSysCalls os_sys_calls_; - TestThreadsafeSingletonInjector os_calls{&os_sys_calls_}; - EXPECT_CALL(os_sys_calls_, getrlimit(RLIMIT_NOFILE, _)) - .WillOnce(Invoke([&](int, struct rlimit* rlim) { - rlim->rlim_cur = 512; - rlim->rlim_max = 1024; - return Api::SysCallIntResult{0, 0}; - })); - EXPECT_CALL(os_sys_calls_, setrlimit(RLIMIT_NOFILE, _)) - .WillOnce(Invoke([&](int, const struct rlimit* rlim) { - EXPECT_EQ(1024, rlim->rlim_cur); - EXPECT_EQ(1024, rlim->rlim_max); - return Api::SysCallIntResult{0, 0}; - })); + struct rlimit rlim; + EXPECT_EQ(::getrlimit(RLIMIT_NOFILE, &rlim), 0); + ASSERT_GT(rlim.rlim_max, 1); + // Set the soft limit lower than the hard limit. + rlim.rlim_cur = rlim.rlim_max / 2; InstanceUtil::raiseFileLimits(); + EXPECT_EQ(::getrlimit(RLIMIT_NOFILE, &rlim), 0); + EXPECT_EQ(rlim.rlim_cur, rlim.rlim_max); } TEST(ServerInstanceUtil, RaiseFileLimitsAlreadyMaxed) { - Api::MockOsSysCalls os_sys_calls_; - TestThreadsafeSingletonInjector os_calls{&os_sys_calls_}; - EXPECT_CALL(os_sys_calls_, getrlimit(RLIMIT_NOFILE, _)) - .WillOnce(Invoke([&](int, struct rlimit* rlim) { - rlim->rlim_cur = 1024; - rlim->rlim_max = 1024; - return Api::SysCallIntResult{0, 0}; - })); - InstanceUtil::raiseFileLimits(); -} - -TEST(ServerInstanceUtil, RaiseFileLimitsReadError) { - Api::MockOsSysCalls os_sys_calls_; - TestThreadsafeSingletonInjector os_calls{&os_sys_calls_}; - EXPECT_CALL(os_sys_calls_, getrlimit(RLIMIT_NOFILE, _)).WillOnce(Invoke([&](int, struct rlimit*) { - return Api::SysCallIntResult{-1, 0}; - })); - InstanceUtil::raiseFileLimits(); -} - -TEST(ServerInstanceUtil, RaiseFileLimitsWriteError) { - Api::MockOsSysCalls os_sys_calls_; - TestThreadsafeSingletonInjector os_calls{&os_sys_calls_}; - EXPECT_CALL(os_sys_calls_, getrlimit(RLIMIT_NOFILE, _)) - .WillOnce(Invoke([&](int, struct rlimit* rlim) { - rlim->rlim_cur = 512; - rlim->rlim_max = 1024; - return Api::SysCallIntResult{0, 0}; - })); - EXPECT_CALL(os_sys_calls_, setrlimit(RLIMIT_NOFILE, _)) - .WillOnce(Invoke([&](int, const struct rlimit*) { return Api::SysCallIntResult{-1, 0}; })); + struct rlimit rlim; + EXPECT_EQ(::getrlimit(RLIMIT_NOFILE, &rlim), 0); + rlim.rlim_cur = rlim.rlim_max; + EXPECT_EQ(::setrlimit(RLIMIT_NOFILE, &rlim), 0); + // Verify that limits remain unchanged when they are the same. InstanceUtil::raiseFileLimits(); + EXPECT_EQ(::getrlimit(RLIMIT_NOFILE, &rlim), 0); + EXPECT_EQ(rlim.rlim_cur, rlim.rlim_max); } +#endif class RunHelperTest : public testing::Test { public: