Skip to content
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ message DnsCacheConfig {
config.cluster.v3.Cluster.RefreshRate dns_failure_refresh_rate = 6;

// The config of circuit breakers for resolver. It provides a configurable threshold.
// If `envoy.reloadable_features.enable_dns_cache_circuit_breakers` is enabled,
// envoy will use dns cache circuit breakers with default settings even if this value is not set.
// Envoy will use dns cache circuit breakers with default settings even if this value is not set.
DnsCacheCircuitBreakers dns_cache_circuit_breaker = 7;

// [#next-major-version: Reconcile DNS options in a single message.]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,6 @@ host when forwarding. See the example below within the configured routes.
.. _dns_cache_circuit_breakers:

Dynamic forward proxy uses circuit breakers built in to the DNS cache with the configuration
of :ref:`DNS cache circuit breakers <envoy_v3_api_field_extensions.common.dynamic_forward_proxy.v3.DnsCacheConfig.dns_cache_circuit_breaker>`. By default, this behavior is enabled by the runtime feature `envoy.reloadable_features.enable_dns_cache_circuit_breakers`.
If this runtime feature is disabled, cluster circuit breakers will be used even when setting the configuration
of :ref:`DNS cache circuit breakers <envoy_v3_api_field_extensions.common.dynamic_forward_proxy.v3.DnsCacheConfig.dns_cache_circuit_breaker>`.

.. literalinclude:: _include/dns-cache-circuit-breaker.yaml
Expand Down
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Removed Config or Runtime
*Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

* access_logs: removed legacy unbounded access logs and runtime guard `envoy.reloadable_features.disallow_unbounded_access_logs`.
* dynamic_forward_proxy: removed `envoy.reloadable_features.enable_dns_cache_circuit_breakers` and legacy code path.
* http: removed legacy HTTP/1.1 error reporting path and runtime guard `envoy.reloadable_features.early_errors_via_hcm`.

New Features
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ constexpr const char* runtime_features[] = {
"envoy.reloadable_features.consume_all_retry_headers",
"envoy.reloadable_features.check_ocsp_policy",
"envoy.reloadable_features.disable_tls_inspector_injection",
"envoy.reloadable_features.enable_dns_cache_circuit_breakers",
"envoy.reloadable_features.fix_upgrade_response",
"envoy.reloadable_features.fix_wildcard_matching",
"envoy.reloadable_features.fixed_connection_close",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,9 @@ class DnsCache {

/**
* Check if a DNS request is allowed given resource limits.
* @param pending_request optional pending request resource limit. If no resource limit is
* provided the internal DNS cache limit is used.
* @return RAII handle for pending request circuit breaker if the request was allowed.
*/
virtual Upstream::ResourceAutoIncDecPtr
canCreateDnsRequest(ResourceLimitOptRef pending_request) PURE;
virtual Upstream::ResourceAutoIncDecPtr canCreateDnsRequest() PURE;
};

using DnsCacheSharedPtr = std::shared_ptr<DnsCache>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,10 @@ DnsCacheImpl::loadDnsCacheEntry(absl::string_view host, uint16_t default_port,
}
}

Upstream::ResourceAutoIncDecPtr
DnsCacheImpl::canCreateDnsRequest(ResourceLimitOptRef pending_requests) {
const auto has_pending_requests = pending_requests.has_value();
auto& current_pending_requests =
has_pending_requests ? pending_requests->get() : resource_manager_.pendingRequests();
Upstream::ResourceAutoIncDecPtr DnsCacheImpl::canCreateDnsRequest() {
auto& current_pending_requests = resource_manager_.pendingRequests();
if (!current_pending_requests.canCreate()) {
if (!has_pending_requests) {
stats_.dns_rq_pending_overflow_.inc();
}
stats_.dns_rq_pending_overflow_.inc();
return nullptr;
}
return std::make_unique<Upstream::ResourceAutoIncDec>(current_pending_requests);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ class DnsCacheImpl : public DnsCache, Logger::Loggable<Logger::Id::forward_proxy
AddUpdateCallbacksHandlePtr addUpdateCallbacks(UpdateCallbacks& callbacks) override;
void iterateHostMap(IterateHostMapCb cb) override;
absl::optional<const DnsHostInfoSharedPtr> getHost(absl::string_view host_name) override;
Upstream::ResourceAutoIncDecPtr
canCreateDnsRequest(ResourceLimitOptRef pending_requests) override;
Upstream::ResourceAutoIncDecPtr canCreateDnsRequest() override;

private:
struct LoadDnsCacheEntryHandleImpl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ envoy_cc_library(
hdrs = ["proxy_filter.h"],
deps = [
"//include/envoy/http:filter_interface",
"//source/common/runtime:runtime_features_lib",
"//source/extensions/clusters:well_known_names",
"//source/extensions/common/dynamic_forward_proxy:dns_cache_interface",
"//source/extensions/filters/http:well_known_names",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
#include "envoy/config/core/v3/base.pb.h"
#include "envoy/extensions/filters/http/dynamic_forward_proxy/v3/dynamic_forward_proxy.pb.h"

#include "common/runtime/runtime_features.h"

#include "extensions/clusters/well_known_names.h"
#include "extensions/common/dynamic_forward_proxy/dns_cache.h"
#include "extensions/filters/http/well_known_names.h"
Expand Down Expand Up @@ -71,19 +69,9 @@ Http::FilterHeadersStatus ProxyFilter::decodeHeaders(Http::RequestHeaderMap& hea
return Http::FilterHeadersStatus::Continue;
}

const bool should_use_dns_cache_circuit_breakers =
Runtime::runtimeFeatureEnabled("envoy.reloadable_features.enable_dns_cache_circuit_breakers");

circuit_breaker_ = config_->cache().canCreateDnsRequest(
!should_use_dns_cache_circuit_breakers
? absl::make_optional(std::reference_wrapper<ResourceLimit>(
cluster_info_->resourceManager(route_entry->priority()).pendingRequests()))
: absl::nullopt);
circuit_breaker_ = config_->cache().canCreateDnsRequest();

if (circuit_breaker_ == nullptr) {
if (!should_use_dns_cache_circuit_breakers) {
cluster_info_->stats().upstream_rq_pending_overflow_.inc();
}
ENVOY_STREAM_LOG(debug, "pending request overflow", *this->decoder_callbacks_);
this->decoder_callbacks_->sendLocalReply(
Http::Code::ServiceUnavailable, ResponseStrings::get().PendingRequestOverflow, nullptr,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ Network::FilterStatus ProxyFilter::onNewConnection() {
return Network::FilterStatus::Continue;
}

circuit_breaker_ = config_->cache().canCreateDnsRequest(absl::nullopt);
circuit_breaker_ = config_->cache().canCreateDnsRequest();

if (circuit_breaker_ == nullptr) {
ENVOY_CONN_LOG(debug, "pending request overflow", read_callbacks_->connection());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -662,29 +662,19 @@ TEST_F(DnsCacheImplTest, MaxHostOverflow) {
TEST_F(DnsCacheImplTest, CircuitBreakersNotInvoked) {
initialize();

auto raii_ptr = dns_cache_->canCreateDnsRequest(absl::nullopt);
auto raii_ptr = dns_cache_->canCreateDnsRequest();
EXPECT_NE(raii_ptr.get(), nullptr);
}

TEST_F(DnsCacheImplTest, DnsCacheCircuitBreakersOverflow) {
config_.mutable_dns_cache_circuit_breaker()->mutable_max_pending_requests()->set_value(0);
initialize();

auto raii_ptr = dns_cache_->canCreateDnsRequest(absl::nullopt);
auto raii_ptr = dns_cache_->canCreateDnsRequest();
EXPECT_EQ(raii_ptr.get(), nullptr);
EXPECT_EQ(1, TestUtility::findCounter(store_, "dns_cache.foo.dns_rq_pending_overflow")->value());
}

TEST_F(DnsCacheImplTest, ClustersCircuitBreakersOverflow) {
initialize();
NiceMock<Upstream::MockBasicResourceLimit> pending_requests_;

EXPECT_CALL(pending_requests_, canCreate()).WillOnce(Return(false));
auto raii_ptr = dns_cache_->canCreateDnsRequest(pending_requests_);
EXPECT_EQ(raii_ptr.get(), nullptr);
EXPECT_EQ(0, TestUtility::findCounter(store_, "dns_cache.foo.dns_rq_pending_overflow")->value());
}

TEST(DnsCacheImplOptionsTest, UseTcpForDnsLookupsOptionSet) {
NiceMock<Event::MockDispatcher> dispatcher;
std::shared_ptr<Network::MockDnsResolver> resolver{std::make_shared<Network::MockDnsResolver>()};
Expand Down
2 changes: 1 addition & 1 deletion test/extensions/common/dynamic_forward_proxy/mocks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ MockDnsCacheResourceManager::MockDnsCacheResourceManager() {
MockDnsCacheResourceManager::~MockDnsCacheResourceManager() = default;

MockDnsCache::MockDnsCache() {
ON_CALL(*this, canCreateDnsRequest_(_)).WillByDefault(Return(nullptr));
ON_CALL(*this, canCreateDnsRequest_()).WillByDefault(Return(nullptr));
}
MockDnsCache::~MockDnsCache() = default;

Expand Down
7 changes: 3 additions & 4 deletions test/extensions/common/dynamic_forward_proxy/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,8 @@ class MockDnsCache : public DnsCache {
MockLoadDnsCacheEntryResult result = loadDnsCacheEntry_(host, default_port, callbacks);
return {result.status_, LoadDnsCacheEntryHandlePtr{result.handle_}};
}
Upstream::ResourceAutoIncDecPtr
canCreateDnsRequest(ResourceLimitOptRef pending_requests) override {
Upstream::ResourceAutoIncDec* raii_ptr = canCreateDnsRequest_(pending_requests);
Upstream::ResourceAutoIncDecPtr canCreateDnsRequest() override {
Upstream::ResourceAutoIncDec* raii_ptr = canCreateDnsRequest_();
return std::unique_ptr<Upstream::ResourceAutoIncDec>(raii_ptr);
}
MOCK_METHOD(MockLoadDnsCacheEntryResult, loadDnsCacheEntry_,
Expand All @@ -58,7 +57,7 @@ class MockDnsCache : public DnsCache {

MOCK_METHOD((void), iterateHostMap, (IterateHostMapCb));
MOCK_METHOD((absl::optional<const DnsHostInfoSharedPtr>), getHost, (absl::string_view));
MOCK_METHOD(Upstream::ResourceAutoIncDec*, canCreateDnsRequest_, (ResourceLimitOptRef));
MOCK_METHOD(Upstream::ResourceAutoIncDec*, canCreateDnsRequest_, ());
};

class MockLoadDnsCacheEntryHandle : public DnsCache::LoadDnsCacheEntryHandle {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,6 @@ name: envoy.clusters.dynamic_forward_proxy
}
}

void disableDnsCacheCircuitBreakers() {
config_helper_.addRuntimeOverride("envoy.reloadable_features.enable_dns_cache_circuit_breakers",
"false");
}

bool upstream_tls_{};
std::string upstream_cert_name_{"upstreamlocalhost"};
CdsHelper cds_helper_;
Expand Down Expand Up @@ -142,30 +137,6 @@ TEST_P(ProxyFilterIntegrationTest, RequestWithBody) {
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.host_added")->value());
}

TEST_P(ProxyFilterIntegrationTest, RequestWithBodyWithClusterCircuitBreaker) {
disableDnsCacheCircuitBreakers();
setup();
codec_client_ = makeHttpConnection(lookupPort("http"));
const Http::TestRequestHeaderMapImpl request_headers{
{":method", "POST"},
{":path", "/test/long/url"},
{":scheme", "http"},
{":authority",
fmt::format("localhost:{}", fake_upstreams_[0]->localAddress()->ip()->port())}};

auto response =
sendRequestAndWaitForResponse(request_headers, 1024, default_response_headers_, 1024);
checkSimpleRequestSuccess(1024, 1024, response.get());
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.dns_query_attempt")->value());
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.host_added")->value());

// Now send another request. This should hit the DNS cache.
response = sendRequestAndWaitForResponse(request_headers, 512, default_response_headers_, 512);
checkSimpleRequestSuccess(512, 512, response.get());
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.dns_query_attempt")->value());
EXPECT_EQ(1, test_server_->counter("dns_cache.foo.host_added")->value());
}

// Verify that after we populate the cache and reload the cluster we reattach to the cache with
// its existing hosts.
TEST_P(ProxyFilterIntegrationTest, ReloadClusterAndAttachToCache) {
Expand Down Expand Up @@ -346,25 +317,5 @@ TEST_P(ProxyFilterIntegrationTest, DnsCacheCircuitBreakersInvoked) {
EXPECT_EQ("503", response->headers().Status()->value().getStringView());
}

TEST_P(ProxyFilterIntegrationTest, ClusterCircuitBreakersInvoked) {
disableDnsCacheCircuitBreakers();
setup(1024, 0);

codec_client_ = makeHttpConnection(lookupPort("http"));
const Http::TestRequestHeaderMapImpl request_headers{
{":method", "POST"},
{":path", "/test/long/url"},
{":scheme", "http"},
{":authority",
fmt::format("localhost:{}", fake_upstreams_[0]->localAddress()->ip()->port())}};

auto response = codec_client_->makeRequestWithBody(request_headers, 1024);
response->waitForEndStream();
EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.upstream_rq_pending_overflow")->value());

EXPECT_TRUE(response->complete());
EXPECT_EQ("503", response->headers().Status()->value().getStringView());
}

} // namespace
} // namespace Envoy
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ TEST_F(ProxyFilterTest, HttpDefaultPort) {

EXPECT_CALL(callbacks_, route());
EXPECT_CALL(cm_, getThreadLocalCluster(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_(_))
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_())
.WillOnce(Return(circuit_breakers_));
EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(false));
Extensions::Common::DynamicForwardProxy::MockLoadDnsCacheEntryHandle* handle =
Expand All @@ -112,7 +112,7 @@ TEST_F(ProxyFilterTest, HttpsDefaultPort) {

EXPECT_CALL(callbacks_, route());
EXPECT_CALL(cm_, getThreadLocalCluster(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_(_))
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_())
.WillOnce(Return(circuit_breakers_));
EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(true));
Extensions::Common::DynamicForwardProxy::MockLoadDnsCacheEntryHandle* handle =
Expand All @@ -134,7 +134,7 @@ TEST_F(ProxyFilterTest, CacheOverflow) {

EXPECT_CALL(callbacks_, route());
EXPECT_CALL(cm_, getThreadLocalCluster(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_(_))
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_())
.WillOnce(Return(circuit_breakers_));
EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(true));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, loadDnsCacheEntry_(Eq("foo"), 443, _))
Expand All @@ -151,17 +151,13 @@ TEST_F(ProxyFilterTest, CacheOverflow) {

// Circuit breaker overflow
TEST_F(ProxyFilterTest, CircuitBreakerOverflow) {
// Disable dns cache circuit breakers because which we expect to be used cluster circuit breakers.
TestScopedRuntime scoped_runtime;
Runtime::LoaderSingleton::getExisting()->mergeValues(
{{"envoy.reloadable_features.enable_dns_cache_circuit_breakers", "false"}});
Upstream::ResourceAutoIncDec* circuit_breakers_(
new Upstream::ResourceAutoIncDec(pending_requests_));
InSequence s;

EXPECT_CALL(callbacks_, route());
EXPECT_CALL(cm_, getThreadLocalCluster(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_(_))
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_())
.WillOnce(Return(circuit_breakers_));
EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(true));
Extensions::Common::DynamicForwardProxy::MockLoadDnsCacheEntryHandle* handle =
Expand All @@ -176,7 +172,7 @@ TEST_F(ProxyFilterTest, CircuitBreakerOverflow) {
filter2->setDecoderFilterCallbacks(callbacks_);
EXPECT_CALL(callbacks_, route());
EXPECT_CALL(cm_, getThreadLocalCluster(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_());
EXPECT_CALL(callbacks_, sendLocalReply(Http::Code::ServiceUnavailable,
Eq("Dynamic forward proxy pending request overflow"), _, _,
Eq("Dynamic forward proxy pending request overflow")));
Expand All @@ -185,8 +181,6 @@ TEST_F(ProxyFilterTest, CircuitBreakerOverflow) {
EXPECT_EQ(Http::FilterHeadersStatus::StopIteration,
filter2->decodeHeaders(request_headers_, false));

EXPECT_EQ(1,
cm_.thread_local_cluster_.cluster_.info_->stats_.upstream_rq_pending_overflow_.value());
filter2->onDestroy();
EXPECT_CALL(*handle, onDestroy());
filter_->onDestroy();
Expand All @@ -200,7 +194,7 @@ TEST_F(ProxyFilterTest, CircuitBreakerOverflowWithDnsCacheResourceManager) {

EXPECT_CALL(callbacks_, route());
EXPECT_CALL(cm_, getThreadLocalCluster(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_(_))
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_())
.WillOnce(Return(circuit_breakers_));
EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(true));
Extensions::Common::DynamicForwardProxy::MockLoadDnsCacheEntryHandle* handle =
Expand All @@ -215,7 +209,7 @@ TEST_F(ProxyFilterTest, CircuitBreakerOverflowWithDnsCacheResourceManager) {
filter2->setDecoderFilterCallbacks(callbacks_);
EXPECT_CALL(callbacks_, route());
EXPECT_CALL(cm_, getThreadLocalCluster(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_());
EXPECT_CALL(callbacks_, sendLocalReply(Http::Code::ServiceUnavailable,
Eq("Dynamic forward proxy pending request overflow"), _, _,
Eq("Dynamic forward proxy pending request overflow")));
Expand Down Expand Up @@ -284,7 +278,7 @@ TEST_F(ProxyFilterTest, HostRewrite) {

EXPECT_CALL(callbacks_, route());
EXPECT_CALL(cm_, getThreadLocalCluster(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_(_))
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_())
.WillOnce(Return(circuit_breakers_));
EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(false));
Extensions::Common::DynamicForwardProxy::MockLoadDnsCacheEntryHandle* handle =
Expand Down Expand Up @@ -312,7 +306,7 @@ TEST_F(ProxyFilterTest, HostRewriteViaHeader) {

EXPECT_CALL(callbacks_, route());
EXPECT_CALL(cm_, getThreadLocalCluster(_));
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_(_))
EXPECT_CALL(*dns_cache_manager_->dns_cache_, canCreateDnsRequest_())
.WillOnce(Return(circuit_breakers_));
EXPECT_CALL(*transport_socket_factory_, implementsSecureTransport()).WillOnce(Return(false));
Extensions::Common::DynamicForwardProxy::MockLoadDnsCacheEntryHandle* handle =
Expand Down
Loading