Skip to content

Commit

Permalink
cr changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@Ferdudas97
Ferdudas97 committed Mar 21, 2022
1 parent ac7e60c commit f2d561f
Show file tree
Hide file tree
Showing 6 changed files with 31 additions and 25 deletions.
14 changes: 7 additions & 7 deletions docs/configuration.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,13 +179,13 @@ Where `<selector>` 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
Expand Down
4 changes: 2 additions & 2 deletions ...trol-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down
14 changes: 8 additions & 6 deletions .../allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
}

Expand All @@ -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 {
Expand Down Expand Up @@ -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()
}
Expand Down
16 changes: 9 additions & 7 deletions ...-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1069,13 +1070,14 @@ class NodeMetadataTest {
return this
}

fun ObjectAssert<DependencySettings>.hasDefaultCircuitBreaker(
fun ObjectAssert<DependencySettings>.hasDefaultThresholdCircuitBreaker(
circuitBreaker: CircuitBreaker
): ObjectAssert<DependencySettings> {
this.extracting { it.circuitBreakers.defaultThreshold }.isEqualTo(circuitBreaker)
return this
}
fun ObjectAssert<DependencySettings>.hasHighCircuitBreaker(

fun ObjectAssert<DependencySettings>.hasHighThresholdCircuitBreaker(
circuitBreaker: CircuitBreaker
): ObjectAssert<DependencySettings> {
this.extracting { it.circuitBreakers.highThreshold }.isEqualTo(circuitBreaker)
Expand Down
6 changes: 4 additions & 2 deletions ...l-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion ...tlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down

0 comments on commit f2d561f

Please sign in to comment.