From f2d561f824eae81c4112a8d940ffe5842a2fc592 Mon Sep 17 00:00:00 2001 From: "radoslaw.chrzanowski" Date: Wed, 16 Mar 2022 13:41:04 +0100 Subject: [PATCH] cr changes --- docs/configuration.md | 14 +++++++------- .../envoycontrol/groups/NodeMetadata.kt | 4 ++-- .../resource/clusters/EnvoyClustersFactory.kt | 14 ++++++++------ .../envoycontrol/groups/NodeMetadataTest.kt | 16 +++++++++------- .../envoycontrol/groups/TestNodeFactory.kt | 6 ++++-- .../ClusterCircuitBreakerDefaultSettingsTest.kt | 2 +- 6 files changed, 31 insertions(+), 25 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 665debf61..2735e4e4a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -179,13 +179,13 @@ Where `` is one of the following: * `default` - default retry policy, applied for every request that doesn't match more specific selector ### Outgoing traffic -Property | Description | Default value --------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | --------- -**envoy-control.envoy.snapshot.egress.common-http.retryPolicy.numberOfRetries** | Number of retries | 1 -**envoy-control.envoy.snapshot.egress.common-http.retryPolicy.hostSelectionRetryMaxAttempts** | The maximum number of times host selection will be reattempted before request being routed to last selected host | 3 -**envoy-control.envoy.snapshot.egress.common-http.retryPolicy.retryHostPredicate** | Specifies a collection of RetryHostPredicates that will be consulted when selecting a host for retries | a list with one entry "envoy.retry_host_predicates.previous_hosts" -**envoy-control.envoy.snapshot.egress.common-http.retryPolicy.retryBackOff.baseInterval** | Specifies parameters that control exponential retry back off base interval | 25ms -**envoy-control.envoy.snapshot.egress.common-http.retryPolicy.retryBackOff.maxInterval** | Specifies parameters that control exponential retry back off max interval | 10 times base interval +Property | Description | Default value +-------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | --------- +**envoy-control.envoy.snapshot.egress.retryPolicy.numberOfRetries** | Number of retries | 1 +**envoy-control.envoy.snapshot.egress.retryPolicy.hostSelectionRetryMaxAttempts** | The maximum number of times host selection will be reattempted before request being routed to last selected host | 3 +**envoy-control.envoy.snapshot.egress.retryPolicy.retryHostPredicate** | Specifies a collection of RetryHostPredicates that will be consulted when selecting a host for retries | a list with one entry "envoy.retry_host_predicates.previous_hosts" +**envoy-control.envoy.snapshot.egress.retryPolicy.retryBackOff.baseInterval** | Specifies parameters that control exponential retry back off base interval | 25ms +**envoy-control.envoy.snapshot.egress.retryPolicy.retryBackOff.maxInterval** | Specifies parameters that control exponential retry back off max interval | 10 times base interval ## Metrics Property | Description | Default value diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt index 457bdf004..c2fd3621d 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt @@ -79,7 +79,7 @@ fun Value?.toOutgoing(properties: SnapshotProperties): Outgoing { val allServiceDependenciesIdentifier = properties.outgoingPermissions.allServicesDependencies.identifier val rawDependencies = this?.field("dependencies")?.list().orEmpty().map(::toRawDependency) val allServicesDependencies = toAllServiceDependencies(rawDependencies, allServiceDependenciesIdentifier) - val defaultSettingsFromProperties = createDefaultOutgoingProperties(properties.egress) + val defaultSettingsFromProperties = createDefaultDependencySettingFromEgressProperties(properties.egress) val allServicesDefaultSettings = allServicesDependencies?.value.toSettings(defaultSettingsFromProperties) val services = rawDependencies.filter { it.service != null && it.service != allServiceDependenciesIdentifier } .map { @@ -103,7 +103,7 @@ fun Value?.toOutgoing(properties: SnapshotProperties): Outgoing { ) } -private fun createDefaultOutgoingProperties(egress: EgressProperties) : DependencySettings { +private fun createDefaultDependencySettingFromEgressProperties(egress: EgressProperties) : DependencySettings { return DependencySettings( handleInternalRedirect = egress.handleInternalRedirect, timeoutPolicy = egress.commonHttp.let { diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt index fa6f5cf89..b5660fe0e 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt @@ -222,7 +222,7 @@ class EnvoyClustersFactory( ?: defaultDependencySettings?.timeoutPolicy?.connectionIdleTimeout ?: cluster.commonHttpProtocolOptions.idleTimeout Cluster.newBuilder(cluster) - .setCircuitBreakers(createCircuitBreakers(dependencySettings)) + .setCircuitBreakers(createCircuitBreakers(dependencySettings, defaultDependencySettings)) .setCommonHttpProtocolOptions( HttpProtocolOptions.newBuilder().setIdleTimeout(idleTimeoutPolicy) ).build() @@ -237,11 +237,12 @@ class EnvoyClustersFactory( ?: defaultDependencySettings?.circuitBreakers?.defaultThreshold val highThreshold = dependencySettings.circuitBreakers.highThreshold ?: defaultDependencySettings?.circuitBreakers?.highThreshold - val thresholds = listOf( + val thresholds = listOfNotNull( defaultThreshold?.toThreshold(RoutingPriority.DEFAULT), highThreshold?.toThreshold(RoutingPriority.HIGH) - ).filterNotNull() - return CircuitBreakers.newBuilder().addAllThresholds(thresholds) + ) + return CircuitBreakers.newBuilder() + .addAllThresholds(thresholds) .build() } @@ -265,7 +266,7 @@ class EnvoyClustersFactory( private fun Int.toValue() = this.let { UInt32Value.of(this) } - fun shouldAddDynamicForwardProxyCluster(group: Group) = + private fun shouldAddDynamicForwardProxyCluster(group: Group) = group.proxySettings.outgoing.getDomainPatternDependencies().isNotEmpty() private fun enableTlsForGroup(group: Group): Boolean { @@ -384,7 +385,8 @@ class EnvoyClustersFactory( domainDependency.settings.timeoutPolicy.connectionIdleTimeout?.let { clusterBuilder.setCommonHttpProtocolOptions(HttpProtocolOptions.newBuilder().setIdleTimeout(it)) } - clusterBuilder.setCircuitBreakers(createCircuitBreakers(domainDependency.settings)) + + clusterBuilder.circuitBreakers = createCircuitBreakers(domainDependency.settings) return clusterBuilder.build() } diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt index e445fbe84..4712ca2a9 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt @@ -955,7 +955,8 @@ class NodeMetadataTest { // given val proto = outgoingDependenciesProto { withService( - "service-1", circuitBreakers = OutgoingDependenciesProtoScope.CircuitBreakers( + "service-1", + circuitBreakers = OutgoingDependenciesProtoScope.CircuitBreakers( defaultThreshold = OutgoingDependenciesProtoScope.CircuitBreaker( maxRetries = 1, maxPendingRequests = 2, @@ -991,12 +992,12 @@ class NodeMetadataTest { val defaultCircuitBreaker = snapshotProperties().egress.commonHttp.circuitBreakers.defaultThreshold.toCircuitBreaker() val highCircuitBreaker = snapshotProperties().egress.commonHttp.circuitBreakers.highThreshold.toCircuitBreaker() outgoing.getServiceDependencies().assertServiceDependency("service-1") - .hasDefaultCircuitBreaker(expectedCircuitBreaker1) - .hasHighCircuitBreaker(highCircuitBreaker) + .hasDefaultThresholdCircuitBreaker(expectedCircuitBreaker1) + .hasHighThresholdCircuitBreaker(highCircuitBreaker) outgoing.getServiceDependencies().assertServiceDependency("service-2") - .hasDefaultCircuitBreaker(defaultCircuitBreaker) - .hasHighCircuitBreaker(highCircuitBreaker) + .hasDefaultThresholdCircuitBreaker(defaultCircuitBreaker) + .hasHighThresholdCircuitBreaker(highCircuitBreaker) } @Test @@ -1069,13 +1070,14 @@ class NodeMetadataTest { return this } - fun ObjectAssert.hasDefaultCircuitBreaker( + fun ObjectAssert.hasDefaultThresholdCircuitBreaker( circuitBreaker: CircuitBreaker ): ObjectAssert { this.extracting { it.circuitBreakers.defaultThreshold }.isEqualTo(circuitBreaker) return this } - fun ObjectAssert.hasHighCircuitBreaker( + + fun ObjectAssert.hasHighThresholdCircuitBreaker( circuitBreaker: CircuitBreaker ): ObjectAssert { this.extracting { it.circuitBreakers.highThreshold }.isEqualTo(circuitBreaker) diff --git a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt index 576b4c7dc..60b256c5a 100644 --- a/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt +++ b/envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt @@ -79,7 +79,8 @@ fun ProxySettings.with( defaultServiceSettings: DependencySettings = DependencySettings( circuitBreakers = CircuitBreakers( defaultThreshold = CircuitBreaker( - RoutingPriority.DEFAULT, maxRequests = 1024, + RoutingPriority.DEFAULT, + maxRequests = 1024, maxPendingRequests = 1024, maxConnections = 1024, maxRetries = 3, @@ -88,7 +89,8 @@ fun ProxySettings.with( retryBudget = RetryBudget(20.0, 3) ), highThreshold = CircuitBreaker( - RoutingPriority.HIGH, maxRequests = 1024, + RoutingPriority.HIGH, + maxRequests = 1024, maxPendingRequests = 1024, maxConnections = 1024, maxRetries = 3, diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt index 74f827bce..a96968d7b 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt @@ -86,7 +86,7 @@ node: } @Test - fun `should enable set default circuit breaker threstholds setting`() { + fun `should set default circuit breaker thresholds setting`() { // given consul.server.operations.registerService(name = "echo", extension = service) untilAsserted {