From e5ce16d8056637549771dbfdd5ce8fc99b934248 Mon Sep 17 00:00:00 2001 From: Saadia El Fekak <74792703+SaadiaELF@users.noreply.github.com> Date: Tue, 22 Apr 2025 02:40:29 -0700 Subject: [PATCH 1/5] fix c++ issues --- .../aggregate/cluster_integration_test.cc | 123 +++++++++--------- 1 file changed, 59 insertions(+), 64 deletions(-) diff --git a/test/extensions/clusters/aggregate/cluster_integration_test.cc b/test/extensions/clusters/aggregate/cluster_integration_test.cc index b3931285cad7a..9815fcb9c5695 100644 --- a/test/extensions/clusters/aggregate/cluster_integration_test.cc +++ b/test/extensions/clusters/aggregate/cluster_integration_test.cc @@ -37,31 +37,70 @@ struct CircuitBreakerLimits { uint32_t max_retries = 3; uint32_t max_connection_pools = std::numeric_limits::max(); - CircuitBreakerLimits withMaxConnections(uint32_t max_connections) const { - CircuitBreakerLimits limits = *this; - limits.max_connections = max_connections; - return limits; + CircuitBreakerLimits& withMaxConnections(uint32_t max_connections) { + this->max_connections = max_connections; + return *this; } - CircuitBreakerLimits withMaxRequests(uint32_t max_requests) const { - CircuitBreakerLimits limits = *this; - limits.max_requests = max_requests; - return limits; + CircuitBreakerLimits& withMaxRequests(uint32_t max_requests) { + this->max_requests = max_requests; + return *this; } - CircuitBreakerLimits withMaxPendingRequests(uint32_t max_pending_requests) const { - CircuitBreakerLimits limits = *this; - limits.max_pending_requests = max_pending_requests; - return limits; + CircuitBreakerLimits& withMaxPendingRequests(uint32_t max_pending_requests) { + this->max_pending_requests = max_pending_requests; + return *this; } - CircuitBreakerLimits withMaxRetries(uint32_t max_retries) const { - CircuitBreakerLimits limits = *this; - limits.max_retries = max_retries; - return limits; + CircuitBreakerLimits& withMaxRetries(uint32_t max_retries) { + this->max_retries = max_retries; + return *this; } }; +void setCircuitBreakerLimits(envoy::config::cluster::v3::Cluster& cluster, + const CircuitBreakerLimits& limits) { + auto* cluster_circuit_breakers = cluster.mutable_circuit_breakers(); + + auto* cluster_circuit_breakers_threshold_default = cluster_circuit_breakers->add_thresholds(); + cluster_circuit_breakers_threshold_default->set_priority( + envoy::config::core::v3::RoutingPriority::DEFAULT); + + cluster_circuit_breakers_threshold_default->mutable_max_connections()->set_value( + limits.max_connections); + cluster_circuit_breakers_threshold_default->mutable_max_pending_requests()->set_value( + limits.max_pending_requests); + cluster_circuit_breakers_threshold_default->mutable_max_requests()->set_value( + limits.max_requests); + cluster_circuit_breakers_threshold_default->mutable_max_retries()->set_value(limits.max_retries); + cluster_circuit_breakers_threshold_default->mutable_max_connection_pools()->set_value( + limits.max_connection_pools); + cluster_circuit_breakers_threshold_default->set_track_remaining(true); +}; + +void setMaxConcurrentStreams(envoy::config::cluster::v3::Cluster& cluster, + uint32_t max_concurrent_streams) { + envoy::extensions::upstreams::http::v3::HttpProtocolOptions http_protocol_options; + http_protocol_options.mutable_explicit_http_config() + ->mutable_http2_protocol_options() + ->mutable_max_concurrent_streams() + ->set_value(max_concurrent_streams); + (*cluster.mutable_typed_extension_protocol_options()) + ["envoy.extensions.upstreams.http.v3.HttpProtocolOptions"] + .PackFrom(http_protocol_options); +}; + +void reduceAggregateClustersListToOneCluster( + envoy::config::cluster::v3::Cluster& aggregate_cluster) { + auto* aggregate_cluster_type = aggregate_cluster.mutable_cluster_type(); + auto* aggregate_cluster_typed_config = aggregate_cluster_type->mutable_typed_config(); + envoy::extensions::clusters::aggregate::v3::ClusterConfig new_aggregate_cluster_typed_config; + aggregate_cluster_typed_config->UnpackTo(&new_aggregate_cluster_typed_config); + new_aggregate_cluster_typed_config.clear_clusters(); + new_aggregate_cluster_typed_config.add_clusters("cluster_1"); + aggregate_cluster_typed_config->PackFrom(new_aggregate_cluster_typed_config); +}; + const std::string& config() { CONSTRUCT_ON_FIRST_USE(std::string, fmt::format(R"EOF( admin: @@ -211,50 +250,6 @@ class AggregateIntegrationTest xds_stream_->startGrpcStream(); } - void setCircuitBreakerLimits(envoy::config::cluster::v3::Cluster& cluster, - const CircuitBreakerLimits& limits) { - auto* cluster_circuit_breakers = cluster.mutable_circuit_breakers(); - - auto* cluster_circuit_breakers_threshold_default = cluster_circuit_breakers->add_thresholds(); - cluster_circuit_breakers_threshold_default->set_priority( - envoy::config::core::v3::RoutingPriority::DEFAULT); - - cluster_circuit_breakers_threshold_default->mutable_max_connections()->set_value( - limits.max_connections); - cluster_circuit_breakers_threshold_default->mutable_max_pending_requests()->set_value( - limits.max_pending_requests); - cluster_circuit_breakers_threshold_default->mutable_max_requests()->set_value( - limits.max_requests); - cluster_circuit_breakers_threshold_default->mutable_max_retries()->set_value( - limits.max_retries); - cluster_circuit_breakers_threshold_default->mutable_max_connection_pools()->set_value( - limits.max_connection_pools); - cluster_circuit_breakers_threshold_default->set_track_remaining(true); - } - - void setMaxConcurrentStreams(envoy::config::cluster::v3::Cluster& cluster, - uint32_t max_concurrent_streams) { - envoy::extensions::upstreams::http::v3::HttpProtocolOptions http_protocol_options; - http_protocol_options.mutable_explicit_http_config() - ->mutable_http2_protocol_options() - ->mutable_max_concurrent_streams() - ->set_value(max_concurrent_streams); - (*cluster.mutable_typed_extension_protocol_options()) - ["envoy.extensions.upstreams.http.v3.HttpProtocolOptions"] - .PackFrom(http_protocol_options); - } - - void - reduceAggregateClustersListToOneCluster(envoy::config::cluster::v3::Cluster& aggregate_cluster) { - auto* aggregate_cluster_type = aggregate_cluster.mutable_cluster_type(); - auto* aggregate_cluster_typed_config = aggregate_cluster_type->mutable_typed_config(); - envoy::extensions::clusters::aggregate::v3::ClusterConfig new_aggregate_cluster_typed_config; - aggregate_cluster_typed_config->UnpackTo(&new_aggregate_cluster_typed_config); - new_aggregate_cluster_typed_config.clear_clusters(); - new_aggregate_cluster_typed_config.add_clusters("cluster_1"); - aggregate_cluster_typed_config->PackFrom(new_aggregate_cluster_typed_config); - } - const bool deferred_cluster_creation_; envoy::config::cluster::v3::Cluster cluster1_; envoy::config::cluster::v3::Cluster cluster2_; @@ -388,7 +383,7 @@ TEST_P(AggregateIntegrationTest, PreviousPrioritiesRetryPredicate) { TEST_P(AggregateIntegrationTest, CircuitBreakerTestMaxConnections) { setDownstreamProtocol(Http::CodecType::HTTP2); - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* static_resources = bootstrap.mutable_static_resources(); auto* aggregate_cluster = static_resources->mutable_clusters(1); @@ -510,7 +505,7 @@ TEST_P(AggregateIntegrationTest, CircuitBreakerTestMaxConnections) { TEST_P(AggregateIntegrationTest, CircuitBreakerTestMaxRequests) { setDownstreamProtocol(Http::CodecType::HTTP2); - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* static_resources = bootstrap.mutable_static_resources(); auto* aggregate_cluster = static_resources->mutable_clusters(1); @@ -630,7 +625,7 @@ TEST_P(AggregateIntegrationTest, CircuitBreakerTestMaxRequests) { TEST_P(AggregateIntegrationTest, CircuitBreakerTestMaxPendingRequests) { setDownstreamProtocol(Http::CodecType::HTTP2); - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* static_resources = bootstrap.mutable_static_resources(); auto* aggregate_cluster = static_resources->mutable_clusters(1); @@ -771,7 +766,7 @@ TEST_P(AggregateIntegrationTest, CircuitBreakerTestMaxPendingRequests) { TEST_P(AggregateIntegrationTest, CircuitBreakerMaxRetriesTest) { setDownstreamProtocol(Http::CodecType::HTTP2); - config_helper_.addConfigModifier([this](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { + config_helper_.addConfigModifier([](envoy::config::bootstrap::v3::Bootstrap& bootstrap) { auto* static_resources = bootstrap.mutable_static_resources(); auto* listener = static_resources->mutable_listeners(0); auto* filter_chain = listener->mutable_filter_chains(0); From 586b6fb1a104e64e3eebaa71428de93c7b6909ac Mon Sep 17 00:00:00 2001 From: Saadia El Fekak <74792703+SaadiaELF@users.noreply.github.com> Date: Tue, 22 Apr 2025 05:44:27 -0700 Subject: [PATCH 2/5] update doc --- .../arch_overview/upstream/aggregate_cluster.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst b/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst index ea94d2493fb15..ed83c1e7d6a00 100644 --- a/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst +++ b/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst @@ -106,6 +106,22 @@ cannot locate that specific endpoint at the aggregate level. As a result, Statef aggregate clusters, because the final cluster choice is made without direct knowledge of the specific endpoint which doesn't exist at the top level. +Circuit Breaking +^^^^^^^^^^^^^^^^^ +:ref:`Circuit Breakers ` on the level of the aggregate cluster are not configurable. The only circuit breaker that is configured on the aggregate cluster's level is **max_retries**. + +This behavior is intentional as the underlying clusters are accessible through multiple paths (directly or via the aggregate cluster(s)), and configuring the circuit breakers at the level of the aggregate cluster will multiply the effective limit by the number of access paths. This would make the circuit breakers useless. + +Unlike other circuit breakers, the **max_retries** circuit breaker must be configured at the aggregate cluster level because when Envoy processes a retry request, it needs to determine whether the retry limit has been exceeded before it selects the underlying cluster. + +* For **max_connections**, **max_requests** and **max_pending_requests** when the configured limit is reached on the underlying cluster(s), only the underlying cluster(s)' circuit breaker opens. The aggregate cluster's circuit breaker remains closed at all times, regardless of whether the circuit breakers limits on the underlying cluster(s) are reached or not. + +* For **max_retries**: + + * when the requests through the aggregate cluster reach the aggregate cluster's `max_retries` limit, the aggregate cluster's circuit breaker opens and the underlying cluster(s)' circuit breakers remain closed. + + * when the requests going directly to the underlying cluster(s) reach the underlying cluster(s) `max_retries` limit. The underlying cluster(s)' circuit breaker opens. + Load Balancing Example ---------------------- From 864796dffdfa01ab4211484cbc6429484ab986be Mon Sep 17 00:00:00 2001 From: Saadia El Fekak <74792703+SaadiaELF@users.noreply.github.com> Date: Tue, 22 Apr 2025 09:01:14 -0700 Subject: [PATCH 3/5] fmt --- .../arch_overview/upstream/aggregate_cluster.rst | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst b/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst index ed83c1e7d6a00..9541db5a2a621 100644 --- a/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst +++ b/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst @@ -108,19 +108,16 @@ doesn't exist at the top level. Circuit Breaking ^^^^^^^^^^^^^^^^^ -:ref:`Circuit Breakers ` on the level of the aggregate cluster are not configurable. The only circuit breaker that is configured on the aggregate cluster's level is **max_retries**. +:ref:`Circuit Breakers ` on the level of the aggregate cluster are not configurable. The only circuit breaker that is configured on the aggregate cluster's level is **max_retries**. This behavior is intentional as the underlying clusters are accessible through multiple paths (directly or via the aggregate cluster(s)), and configuring the circuit breakers at the level of the aggregate cluster will multiply the effective limit by the number of access paths. This would make the circuit breakers useless. -Unlike other circuit breakers, the **max_retries** circuit breaker must be configured at the aggregate cluster level because when Envoy processes a retry request, it needs to determine whether the retry limit has been exceeded before it selects the underlying cluster. +Unlike other circuit breakers, the **max_retries** circuit breaker must be configured at the aggregate cluster level because when Envoy processes a retry request, it needs to determine whether the retry limit has been exceeded before it selects the underlying cluster. * For **max_connections**, **max_requests** and **max_pending_requests** when the configured limit is reached on the underlying cluster(s), only the underlying cluster(s)' circuit breaker opens. The aggregate cluster's circuit breaker remains closed at all times, regardless of whether the circuit breakers limits on the underlying cluster(s) are reached or not. - * For **max_retries**: - - * when the requests through the aggregate cluster reach the aggregate cluster's `max_retries` limit, the aggregate cluster's circuit breaker opens and the underlying cluster(s)' circuit breakers remain closed. - - * when the requests going directly to the underlying cluster(s) reach the underlying cluster(s) `max_retries` limit. The underlying cluster(s)' circuit breaker opens. + * when the requests through the aggregate cluster reach the aggregate cluster's `max_retries` limit, the aggregate cluster's circuit breaker opens and the underlying cluster(s)' circuit breakers remain closed. + * when the requests going directly to the underlying cluster(s) reach the underlying cluster(s) `max_retries` limit. The underlying cluster(s)' circuit breaker opens. Load Balancing Example ---------------------- From 0001efc1e83b808dbc910f1785487cc90e827a0e Mon Sep 17 00:00:00 2001 From: Saadia El Fekak <74792703+SaadiaELF@users.noreply.github.com> Date: Wed, 23 Apr 2025 01:34:38 -0700 Subject: [PATCH 4/5] update --- .../upstream/aggregate_cluster.rst | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst b/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst index 9541db5a2a621..a0c9a80c1bf8d 100644 --- a/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst +++ b/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst @@ -108,16 +108,24 @@ doesn't exist at the top level. Circuit Breaking ^^^^^^^^^^^^^^^^^ -:ref:`Circuit Breakers ` on the level of the aggregate cluster are not configurable. The only circuit breaker that is configured on the aggregate cluster's level is **max_retries**. +:ref:`Circuit Breakers ` on the level of the aggregate cluster are +not configurable. The only circuit breaker that is configured on the aggregate cluster's level is **max_retries**. -This behavior is intentional as the underlying clusters are accessible through multiple paths (directly or via the aggregate cluster(s)), and configuring the circuit breakers at the level of the aggregate cluster will multiply the effective limit by the number of access paths. This would make the circuit breakers useless. +This behavior is intentional as the underlying clusters are accessible through multiple paths (directly or via the +aggregate cluster(s)), and configuring the circuit breakers at the level of the aggregate cluster will multiply the +effective limit by the number of access paths. This would make the circuit breakers useless. -Unlike other circuit breakers, the **max_retries** circuit breaker must be configured at the aggregate cluster level because when Envoy processes a retry request, it needs to determine whether the retry limit has been exceeded before it selects the underlying cluster. +Unlike other circuit breakers, the **max_retries** circuit breaker must be configured at the aggregate cluster level +because when Envoy processes a retry request, it needs to determine whether the retry limit has been exceeded before it +selects the underlying cluster. -* For **max_connections**, **max_requests** and **max_pending_requests** when the configured limit is reached on the underlying cluster(s), only the underlying cluster(s)' circuit breaker opens. The aggregate cluster's circuit breaker remains closed at all times, regardless of whether the circuit breakers limits on the underlying cluster(s) are reached or not. +* For **max_connections**, **max_requests** and **max_pending_requests**: + when the configured limit is reached on the underlying cluster(s), only the underlying cluster(s)' circuit breaker opens. The aggregate cluster's circuit breaker remains closed at all times, regardless of whether the circuit breakers limits on the underlying cluster(s) are reached or not. * For **max_retries**: - * when the requests through the aggregate cluster reach the aggregate cluster's `max_retries` limit, the aggregate cluster's circuit breaker opens and the underlying cluster(s)' circuit breakers remain closed. - * when the requests going directly to the underlying cluster(s) reach the underlying cluster(s) `max_retries` limit. The underlying cluster(s)' circuit breaker opens. + when the requests through the aggregate cluster reach the aggregate cluster's `max_retries` + limit, the aggregate cluster's circuit breaker opens and the underlying cluster(s)' circuit breakers remain closed. + when the requests going directly to the underlying cluster(s) reach the underlying cluster(s) `max_retries`limit. The + underlying cluster(s)' circuit breaker opens. Load Balancing Example ---------------------- From ae6c349d25c26af38636d5dd87b474ca7628c184 Mon Sep 17 00:00:00 2001 From: Saadia El Fekak <74792703+SaadiaELF@users.noreply.github.com> Date: Wed, 23 Apr 2025 01:44:31 -0700 Subject: [PATCH 5/5] update --- .../intro/arch_overview/upstream/aggregate_cluster.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst b/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst index a0c9a80c1bf8d..9ca2a7cea2059 100644 --- a/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst +++ b/docs/root/intro/arch_overview/upstream/aggregate_cluster.rst @@ -121,11 +121,11 @@ selects the underlying cluster. * For **max_connections**, **max_requests** and **max_pending_requests**: when the configured limit is reached on the underlying cluster(s), only the underlying cluster(s)' circuit breaker opens. The aggregate cluster's circuit breaker remains closed at all times, regardless of whether the circuit breakers limits on the underlying cluster(s) are reached or not. + * For **max_retries**: - when the requests through the aggregate cluster reach the aggregate cluster's `max_retries` - limit, the aggregate cluster's circuit breaker opens and the underlying cluster(s)' circuit breakers remain closed. - when the requests going directly to the underlying cluster(s) reach the underlying cluster(s) `max_retries`limit. The - underlying cluster(s)' circuit breaker opens. + when the requests through the aggregate cluster reach the aggregate cluster's `max_retries` limit, the aggregate + cluster's circuit breaker opens and the underlying cluster(s)' circuit breakers remain closed. when the requests going directly to the underlying cluster(s) reach the underlying cluster(s) `max_retries` limit. The underlying + cluster(s)' circuit breaker opens. Load Balancing Example ----------------------