cr changes

allegro · Mar 16, 2022 · c3bca8a · c3bca8a
1 parent ac7e60c
commit c3bca8a
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 16 deletions.
diff --git a/...trol-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadata.kt b/...trol-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/.../allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt b/.../allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt
@@ -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 {

diff --git a/...-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataTest.kt b/...-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<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)

diff --git a/...l-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/TestNodeFactory.kt b/...l-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/...tlin/pl/allegro/tech/servicemesh/envoycontrol/ClusterCircuitBreakerDefaultSettingsTest.kt b/...tlin/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 {