Skip to content

Support drop_overload using EDS cache while CDS update warming failure #42949

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.

Sign up for GitHub

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

Merged
yanavlasov merged 6 commits into from
Jan 16, 2026
Merged

Support drop_overload using EDS cache while CDS update warming failure #42949

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
da6088c
Support drop_overload using EDS cache while CDS update warming failure
yanjunxiang-google Jan 12, 2026
b491a5b
fix format
yanjunxiang-google Jan 12, 2026
acf9ce9
adding release notes
yanjunxiang-google Jan 14, 2026
954769d
merge upstream main
yanjunxiang-google Jan 14, 2026
fcb043f
fix format
yanjunxiang-google Jan 14, 2026
71f628f
Merge branch 'main' into eds_cache
yanavlasov Jan 16, 2026
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: 3 additions & 0 deletions changelogs/current.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ bug_fixes:
would not be properly closed when in draining state. This could occur when a response was sent before the request
was fully received, causing connections to remain open indefinitely. This behavior change can be temporarily reverted
by setting the runtime guard ``envoy.reloadable_features.http1_close_connection_on_zombie_stream_complete`` to ``false``.
- area: drop_overload
change: |
Fixed a bug that drop_overload failed to use cached EDS resources.
- area: http
change: |
Fixed upstream client to not close connection when idle timeout fires before the connection is established.
Expand Down
9 changes: 3 additions & 6 deletions source/extensions/clusters/eds/eds.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,12 +245,6 @@ EdsClusterImpl::onConfigUpdate(const std::vector<Config::DecodedResourceRef>& re
}
}

// Drop overload configuration parsing.
absl::Status status = parseDropOverloadConfig(cluster_load_assignment);
if (!status.ok()) {
return status;
}

// Pause LEDS messages until the EDS config is finished processing.
Config::ScopedResume maybe_resume_leds;
const auto type_url = Config::getTypeUrl<envoy::config::endpoint::v3::LbEndpoint>();
Expand All @@ -269,6 +263,9 @@ EdsClusterImpl::onConfigUpdate(const std::vector<Config::DecodedResourceRef>& re

void EdsClusterImpl::update(
const envoy::config::endpoint::v3::ClusterLoadAssignment& cluster_load_assignment) {
// Drop overload configuration parsing.
THROW_IF_NOT_OK(parseDropOverloadConfig(cluster_load_assignment));
Copy link
Contributor

@yurykats yurykats Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that since update() is void, it's not expected to be doing any error checking. Maybe just log that overload wasn't applied and move on?

Copy link
Contributor Author

@yanjunxiang-google yanjunxiang-google Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed offline, we want to reject the bad configuration by throwing an exception. This follows the same logic as other EDS updates.

Copy link
Contributor

@adisuissa adisuissa Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that throwing here should be discouraged.
It works (does not throw in a non-onConfigUpdate path) because the EDS resource is first validated before it is added to the cache, so the next call to this function when retrieving the cached EDS resource will not fail.

IMHO a better solution would be to refactor the function parseDropOverloadConfig so instead of both validating and updating the value, one would be able to validate and store the values separately. Then, the validation can be done in the onConfigUpdate() method, and the storing of the value can be done here in update().

Copy link
Contributor Author

@yanjunxiang-google yanjunxiang-google Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea! Let's have a follow up PR to do the refactoring.


// Compare the current set of LEDS localities (localities using LEDS) to the one received in the
// update. A LEDS locality can either be added, removed, or kept. If it is added we add a
// subscription to it, and if it is removed we delete the subscription.
Expand Down
11 changes: 11 additions & 0 deletions test/integration/ads_integration.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,17 @@ void AdsIntegrationTestBase::makeSingleRequest() {
cleanupUpstreamAndDownstream();
}

void AdsIntegrationTestBase::makeSingleRequestWithDropOverload() {
registerTestServerPorts({"http"});
BufferingStreamDecoderPtr response = IntegrationUtil::makeSingleRequest(
lookupPort("http"), "GET", "/cluster_0", "", downstream_protocol_, version_, "foo.com");
ASSERT_TRUE(response->complete());
EXPECT_EQ("503", response->headers().getStatusValue());
EXPECT_THAT(response->headers(), ContainsHeader("x-envoy-unconditional-drop-overload", "true"));

cleanupUpstreamAndDownstream();
}

void AdsIntegrationTestBase::initialize() { initializeAds(false); }

void AdsIntegrationTestBase::initializeAds(const bool rate_limiting) {
Expand Down
1 change: 1 addition & 0 deletions test/integration/ads_integration.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class AdsIntegrationTestBase : public Grpc::BaseGrpcClientIntegrationParamTest,
const std::string& cluster);

void makeSingleRequest();
void makeSingleRequestWithDropOverload();

void initialize() override;
void initializeAds(const bool rate_limiting);
Expand Down
50 changes: 50 additions & 0 deletions test/integration/ads_integration_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -811,6 +811,56 @@ TEST_P(AdsIntegrationTest, CdsKeepEdsAfterWarmingFailure) {
makeSingleRequest();
}

TEST_P(AdsIntegrationTest, CdsKeepEdsDropOverloadAfterWarmingFailure) {
// This test should be kept after the runtime guard is deprecated
config_helper_.addRuntimeOverride("envoy.restart_features.use_eds_cache_for_ads", "true");
initialize();
EXPECT_TRUE(compareDiscoveryRequest(Config::TestTypeUrl::get().Cluster, "", {}, {}, {}, true));
envoy::config::cluster::v3::Cluster cluster = buildCluster("cluster_0");
// Set a small EDS subscription expiration.
cluster.mutable_eds_cluster_config()
->mutable_eds_config()
->mutable_initial_fetch_timeout()
->set_nanos(100 * 1000 * 1000);
sendDiscoveryResponse<envoy::config::cluster::v3::Cluster>(Config::TestTypeUrl::get().Cluster,
{cluster}, {cluster}, {}, "1");

auto cluster_load_assignment = buildClusterLoadAssignment("cluster_0");
auto* policy = cluster_load_assignment.mutable_policy();
auto* drop_overload = policy->add_drop_overloads();
drop_overload->set_category("lb_drop_overload");
// Set drop_overload to drop everything.
drop_overload->mutable_drop_percentage()->set_numerator(100);
sendDiscoveryResponse<envoy::config::endpoint::v3::ClusterLoadAssignment>(
Config::TestTypeUrl::get().ClusterLoadAssignment, {cluster_load_assignment},
{cluster_load_assignment}, {}, "1");

sendDiscoveryResponse<envoy::config::listener::v3::Listener>(
Config::TestTypeUrl::get().Listener, {buildListener("listener_0", "route_config_0")},
{buildListener("listener_0", "route_config_0")}, {}, "1");

sendDiscoveryResponse<envoy::config::route::v3::RouteConfiguration>(
Config::TestTypeUrl::get().RouteConfiguration,
{buildRouteConfig("route_config_0", "cluster_0")},
{buildRouteConfig("route_config_0", "cluster_0")}, {}, "1");

test_server_->waitForCounterGe("listener_manager.listener_create_success", 1);
// Send a HTTP request and verify it is dropped with unconditional_drop_overload.
makeSingleRequestWithDropOverload();

// Update a cluster's field (connect_timeout) so the cluster in Envoy will be explicitly updated.
cluster.mutable_connect_timeout()->set_seconds(7);
sendDiscoveryResponse<envoy::config::cluster::v3::Cluster>(Config::TestTypeUrl::get().Cluster,
{cluster}, {cluster}, {}, "2");
// Avoid sending an EDS update, and wait for EDS update timeout (that results in
// a cluster update without resources).
test_server_->waitForCounterGe("cluster.cluster_0.init_fetch_timeout", 1);
// Envoy uses the cached resource.
EXPECT_EQ(1, test_server_->counter("cluster.cluster_0.assignment_use_cached")->value());
// Send a HTTP request again and verify it is dropped with unconditional_drop_overload.
makeSingleRequestWithDropOverload();
}

// Validate that an update to 2 Clusters that have the same ClusterLoadAssignment, and
// that don't receive updated ClusterLoadAssignment use the previous (cached) cluster
// load assignment.
Expand Down
Loading