removed service dependencies check for traffic splitting #292

allegro · Sep 25, 2023 · 74f7afb · 74f7afb
1 parent 83ebfc5
commit 74f7afb
Show file tree

Hide file tree

Showing 7 changed files with 24 additions and 50 deletions.
diff --git a/...src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt b/...src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt
@@ -224,20 +224,19 @@ class EnvoySnapshotFactory(
         globalSnapshot: GlobalSnapshot,
     ): RouteSpecification {
         val trafficSplitting = properties.loadBalancing.trafficSplitting
-        val weights = trafficSplitting.serviceByWeightsProperties[serviceName]
         val enabledForDependency = globalSnapshot.endpoints[clusterName]?.endpointsList
             ?.any { e -> trafficSplitting.zoneName == e.locality.zone }
             ?: false
-        return if (weights != null && enabledForDependency) {
+        return if (trafficSplitting.clusterWeights.secondary > 0 && enabledForDependency) {
             logger.debug(
-                "Building traffic splitting route spec, weights: $weights, " +
+                "Building traffic splitting route spec, weights: ${trafficSplitting.clusterWeights}, " +
                     "serviceName: $serviceName, clusterName: $clusterName, "
             )
             WeightRouteSpecification(
                 clusterName,
                 routeDomains,
                 settings,
-                weights
+                trafficSplitting.clusterWeights
             )
         } else {
             StandardRouteSpecification(
@@ -433,5 +432,5 @@ data class WeightRouteSpecification(
     override val clusterName: String,
     override val routeDomains: List<String>,
     override val settings: DependencySettings,
-    val clusterWeights: ZoneWeights,
+    val clusterWeights: ClusterWeights,
 ) : RouteSpecification()
diff --git a/...e/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt b/...e/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/SnapshotProperties.kt
@@ -157,12 +157,12 @@ class CanaryProperties {
 
 class TrafficSplittingProperties {
     var zoneName = ""
-    var serviceByWeightsProperties: Map<String, ZoneWeights> = mapOf()
+    var clusterWeights: ClusterWeights = ClusterWeights()
     var secondaryClusterPostfix = "secondary"
     var aggregateClusterPostfix = "aggregate"
 }
 
-class ZoneWeights {
+class ClusterWeights {
     var main = 100
     var secondary = 0
 }

diff --git a/.../allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt b/.../allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt
@@ -52,7 +52,6 @@ import pl.allegro.tech.servicemesh.envoycontrol.snapshot.GlobalSnapshot
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.OAuthProvider
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.Threshold
-import pl.allegro.tech.servicemesh.envoycontrol.snapshot.TrafficSplittingProperties
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.filters.SanUriMatcherFactory
 
 typealias EnvoyClusterConfig = io.envoyproxy.envoy.extensions.clusters.aggregate.v3.ClusterConfig
@@ -199,7 +198,6 @@ class EnvoyClustersFactory(
         val clustersForGroup = when (group) {
             is ServicesGroup -> dependencies.flatMap {
                 createClusters(
-                    group.serviceName,
                     it.value.settings,
                     clusters[it.key],
                     globalSnapshot.endpoints[it.key]
@@ -210,7 +208,6 @@ class EnvoyClustersFactory(
                 globalSnapshot.allServicesNames.flatMap {
                     val dependency = dependencies[it]
                     createClusters(
-                        group.serviceName,
                         getDependencySettings(dependency, group),
                         clusters[it],
                         globalSnapshot.endpoints[it]
@@ -248,7 +245,7 @@ class EnvoyClustersFactory(
             .build()
     }
 
-    private fun createSetOfClustersForGroup(
+    private fun createTrafficSplittingClustersForGroup(
         dependencySettings: DependencySettings,
         cluster: Cluster
     ): Collection<Cluster> {
@@ -267,35 +264,27 @@ class EnvoyClustersFactory(
     }
 
     private fun createClusters(
-        serviceName: String,
         dependencySettings: DependencySettings,
         cluster: Cluster?,
         clusterLoadAssignment: ClusterLoadAssignment?
     ): Collection<Cluster> {
         return cluster?.let {
-            if (enableTrafficSplitting(serviceName, clusterLoadAssignment)) {
-                createSetOfClustersForGroup(dependencySettings, cluster)
+            if (hasEndpointsInZone(clusterLoadAssignment)) {
+                createTrafficSplittingClustersForGroup(dependencySettings, cluster)
             } else {
                 listOf(createClusterForGroup(dependencySettings, cluster))
             }
         } ?: listOf()
     }
 
-    private fun enableTrafficSplitting(
-        serviceName: String,
+    private fun hasEndpointsInZone(
         clusterLoadAssignment: ClusterLoadAssignment?
     ): Boolean {
         val trafficSplitting = properties.loadBalancing.trafficSplitting
-        val trafficSplitEnabled = trafficSplitting.serviceByWeightsProperties.containsKey(serviceName)
-        return trafficSplitEnabled && hasEndpointsInZone(clusterLoadAssignment, trafficSplitting)
+        return clusterLoadAssignment?.endpointsList
+            ?.any { e -> trafficSplitting.zoneName == e.locality.zone } ?: false
     }
 
-    private fun hasEndpointsInZone(
-        clusterLoadAssignment: ClusterLoadAssignment?,
-        trafficSplitting: TrafficSplittingProperties
-    ) = clusterLoadAssignment?.endpointsList
-        ?.any { e -> trafficSplitting.zoneName == e.locality.zone } ?: false
-
     private fun shouldAddDynamicForwardProxyCluster(group: Group) =
         group.proxySettings.outgoing.getDomainPatternDependencies().isNotEmpty()
 

diff --git a/...core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt b/...core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoySnapshotFactoryTest.kt
@@ -57,9 +57,7 @@ class EnvoySnapshotFactoryTest {
     }
 
     private val snapshotPropertiesWithWeights = SnapshotProperties().also {
-        it.loadBalancing.trafficSplitting.serviceByWeightsProperties = mapOf(
-            DEFAULT_SERVICE_NAME to DEFAULT_CLUSTER_WEIGHTS
-        )
+        it.loadBalancing.trafficSplitting.clusterWeights = DEFAULT_CLUSTER_WEIGHTS
         it.loadBalancing.trafficSplitting.zoneName = TRAFFIC_SPLITTING_FORCE_TRAFFIC_ZONE
     }
 
@@ -269,9 +267,7 @@ class EnvoySnapshotFactoryTest {
         // given
         val defaultProperties = SnapshotProperties().also {
             it.dynamicListeners.enabled = false
-            it.loadBalancing.trafficSplitting.serviceByWeightsProperties = mapOf(
-                DEFAULT_SERVICE_NAME to DEFAULT_CLUSTER_WEIGHTS
-            )
+            it.loadBalancing.trafficSplitting.clusterWeights = DEFAULT_CLUSTER_WEIGHTS
             it.loadBalancing.trafficSplitting.zoneName = "not-matching-dc"
         }
         val envoySnapshotFactory = createSnapshotFactory(defaultProperties)

diff --git a/...egro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactoryTest.kt b/...egro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactoryTest.kt
@@ -12,7 +12,6 @@ import pl.allegro.tech.servicemesh.envoycontrol.utils.AGGREGATE_CLUSTER_NAME
 import pl.allegro.tech.servicemesh.envoycontrol.utils.CLUSTER_NAME1
 import pl.allegro.tech.servicemesh.envoycontrol.utils.CLUSTER_NAME2
 import pl.allegro.tech.servicemesh.envoycontrol.utils.DEFAULT_CLUSTER_WEIGHTS
-import pl.allegro.tech.servicemesh.envoycontrol.utils.DEFAULT_SERVICE_NAME
 import pl.allegro.tech.servicemesh.envoycontrol.utils.MAIN_CLUSTER_NAME
 import pl.allegro.tech.servicemesh.envoycontrol.utils.SECONDARY_CLUSTER_NAME
 import pl.allegro.tech.servicemesh.envoycontrol.utils.TRAFFIC_SPLITTING_FORCE_TRAFFIC_ZONE
@@ -28,9 +27,7 @@ internal class EnvoyClustersFactoryTest {
     companion object {
         private val factory = EnvoyClustersFactory(SnapshotProperties())
         private val snapshotPropertiesWithWeights = SnapshotProperties().apply {
-            loadBalancing.trafficSplitting.serviceByWeightsProperties = mapOf(
-                DEFAULT_SERVICE_NAME to DEFAULT_CLUSTER_WEIGHTS
-            )
+            loadBalancing.trafficSplitting.clusterWeights = DEFAULT_CLUSTER_WEIGHTS
             loadBalancing.trafficSplitting.zoneName = TRAFFIC_SPLITTING_FORCE_TRAFFIC_ZONE
         }
     }

diff --git a/...ro/tech/servicemesh/envoycontrol/snapshot/resource/endpoints/EnvoyEndpointsFactoryTest.kt b/...ro/tech/servicemesh/envoycontrol/snapshot/resource/endpoints/EnvoyEndpointsFactoryTest.kt
@@ -17,13 +17,13 @@ import pl.allegro.tech.servicemesh.envoycontrol.services.ServiceInstance
 import pl.allegro.tech.servicemesh.envoycontrol.services.ServiceInstances
 import pl.allegro.tech.servicemesh.envoycontrol.services.ServiceName
 import pl.allegro.tech.servicemesh.envoycontrol.services.ServicesState
+import pl.allegro.tech.servicemesh.envoycontrol.snapshot.ClusterWeights
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.LoadBalancingPriorityProperties
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.LoadBalancingProperties
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.RouteSpecification
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.SnapshotProperties
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.TrafficSplittingProperties
 import pl.allegro.tech.servicemesh.envoycontrol.snapshot.WeightRouteSpecification
-import pl.allegro.tech.servicemesh.envoycontrol.snapshot.ZoneWeights
 import pl.allegro.tech.servicemesh.envoycontrol.utils.zoneWeights
 import java.util.concurrent.ConcurrentHashMap
 import java.util.stream.Stream
@@ -379,9 +379,7 @@ internal class EnvoyEndpointsFactoryTest {
 
         val services = setOf(serviceName, serviceName2)
         val envoyEndpointsFactory = EnvoyEndpointsFactory(
-            snapshotPropertiesWithTrafficSplitting(
-                mapOf(serviceName to defaultWeights)
-            ),
+            snapshotPropertiesWithTrafficSplitting(),
             currentZone = defaultZone
         )
         val loadAssignments = envoyEndpointsFactory
@@ -407,9 +405,7 @@ internal class EnvoyEndpointsFactoryTest {
         )
         val services = setOf(serviceName, serviceName2)
         val envoyEndpointsFactory = EnvoyEndpointsFactory(
-            snapshotPropertiesWithTrafficSplitting(
-                mapOf(serviceName to defaultWeights)
-            ),
+            snapshotPropertiesWithTrafficSplitting(),
             currentZone = defaultZone
         )
         val loadAssignments = envoyEndpointsFactory.createLoadAssignment(
@@ -437,9 +433,7 @@ internal class EnvoyEndpointsFactoryTest {
         )
         val services = setOf(serviceName, serviceName2)
         val envoyEndpointsFactory = EnvoyEndpointsFactory(
-            snapshotPropertiesWithTrafficSplitting(
-                mapOf(serviceName to defaultWeights)
-            ),
+            snapshotPropertiesWithTrafficSplitting(),
             currentZone = defaultZone
         )
         val loadAssignments = envoyEndpointsFactory.createLoadAssignment(
@@ -465,7 +459,6 @@ internal class EnvoyEndpointsFactoryTest {
         val services = setOf(serviceName, serviceName2)
         val envoyEndpointsFactory = EnvoyEndpointsFactory(
             snapshotPropertiesWithTrafficSplitting(
-                mapOf(serviceName to defaultWeights),
                 zone = "DC2"
             ),
             currentZone = defaultZone
@@ -540,17 +533,17 @@ internal class EnvoyEndpointsFactoryTest {
         }
 
     private fun snapshotPropertiesWithTrafficSplitting(
-        serviceByWeights: Map<String, ZoneWeights>,
+        serviceByWeights: ClusterWeights = defaultWeights,
         zone: String = defaultZone
     ) =
         SnapshotProperties().apply {
             loadBalancing.trafficSplitting = TrafficSplittingProperties().apply {
                 zoneName = zone
-                serviceByWeightsProperties = serviceByWeights
+                clusterWeights = serviceByWeights
             }
         }
 
-    private fun String.toRouteSpecification(weights: ZoneWeights = defaultWeights): RouteSpecification {
+    private fun String.toRouteSpecification(weights: ClusterWeights = defaultWeights): RouteSpecification {
         return WeightRouteSpecification(this, listOf(), DependencySettings(), weights)
     }
 

diff --git a/...y-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/utils/TestData.kt b/...y-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/utils/TestData.kt
@@ -1,6 +1,6 @@
 package pl.allegro.tech.servicemesh.envoycontrol.utils
 
-import pl.allegro.tech.servicemesh.envoycontrol.snapshot.ZoneWeights
+import pl.allegro.tech.servicemesh.envoycontrol.snapshot.ClusterWeights
 
 const val INGRESS_HOST = "ingress-host"
 const val INGRESS_PORT = 3380
@@ -20,7 +20,7 @@ const val CURRENT_ZONE = "dc1"
 
 val DEFAULT_CLUSTER_WEIGHTS = zoneWeights(50, 50)
 
-fun zoneWeights(main: Int, secondary: Int) = ZoneWeights().also {
+fun zoneWeights(main: Int, secondary: Int) = ClusterWeights().also {
     it.main = main
     it.secondary = secondary
 }