Added possibility for configuring priorities per service (#436)

* allegro-internal/flex-roadmap#814 Added service-specific zone priorities
allegro · Oct 24, 2024 · dfbf7bf · dfbf7bf
1 parent cd0f215
commit dfbf7bf
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 Lists all changes with user impact.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
+## [0.22.4]
+- Added possibility for configuring priorities per service
+
 ## [0.22.3]
 ### Changed
 - Changed names of some metrics

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,6 +157,7 @@ class LoadBalancingProperties {
     var policy = Cluster.LbPolicy.LEAST_REQUEST
     var useKeysSubsetFallbackPolicy = true
     var priorities = LoadBalancingPriorityProperties()
+    var servicePriorities: Map<String, LoadBalancingPriorityProperties> = mapOf()
 }
 
 class CanaryProperties {

diff --git a/...llegro/tech/servicemesh/envoycontrol/snapshot/resource/endpoints/EnvoyEndpointsFactory.kt b/...llegro/tech/servicemesh/envoycontrol/snapshot/resource/endpoints/EnvoyEndpointsFactory.kt
@@ -188,7 +188,7 @@ class EnvoyEndpointsFactory(
                 ?.map {
                     createLbEndpoint(it, serviceInstances.serviceName, locality)
                 } ?: emptyList())
-            .setPriority(toEnvoyPriority(zone, locality))
+            .setPriority(toEnvoyPriority(zone, locality, serviceInstances))
             .build()
     }
 
@@ -286,8 +286,14 @@ class EnvoyEndpointsFactory(
             false -> this
         }
 
-    private fun toEnvoyPriority(zone: String, locality: Locality): Int {
-        val zonePriorities = properties.loadBalancing.priorities.zonePriorities
+    private fun toEnvoyPriority(zone: String, locality: Locality, serviceInstances: ServiceInstances?): Int {
+        var zonePriorities = properties.loadBalancing.priorities.zonePriorities
+        serviceInstances?.let {
+            if (properties.loadBalancing.servicePriorities.containsKey(serviceInstances.serviceName)) {
+                zonePriorities =
+                    properties.loadBalancing.servicePriorities[serviceInstances.serviceName]!!.zonePriorities
+            }
+        }
         return when (zonePriorities.isNotEmpty()) {
             true -> zonePriorities[currentZone]?.get(zone) ?: toEnvoyPriority(locality)
             false -> toEnvoyPriority(locality)

diff --git a/...ro/tech/servicemesh/envoycontrol/snapshot/resource/endpoints/EnvoyEndpointsFactoryTest.kt b/...ro/tech/servicemesh/envoycontrol/snapshot/resource/endpoints/EnvoyEndpointsFactoryTest.kt
@@ -406,6 +406,40 @@ internal class EnvoyEndpointsFactoryTest {
             .anySatisfy { it.hasZoneWithPriority("DC3", 2) }
     }
 
+    @Test
+    fun `should create load assignment with service zone priorities`() {
+        val envoyEndpointsFactory = EnvoyEndpointsFactory(
+            snapshotPropertiesWithPriorities
+                (
+                mapOf("DC2" to mapOf("DC1" to 1, "DC2" to 1, "DC3" to 1)),
+                mapOf("DC2" to mapOf("DC1" to 2, "DC2" to 2, "DC3" to 2)),
+                serviceName
+            ),
+            currentZone = "DC2"
+        )
+        val loadAssignments = envoyEndpointsFactory.createLoadAssignment(setOf(serviceName), multiClusterStateDC2Local)
+        loadAssignments.assertHasLoadAssignment(
+            mapOf("DC1" to 2, "DC2" to 2, "DC3" to 2)
+        )
+    }
+
+    @Test
+    fun `should create load assignment with global zone priorities when no service config found`() {
+        val envoyEndpointsFactory = EnvoyEndpointsFactory(
+            snapshotPropertiesWithPriorities
+                (
+                mapOf("DC2" to mapOf("DC1" to 1, "DC2" to 1, "DC3" to 1)),
+                mapOf("DC2" to mapOf("DC1" to 2, "DC2" to 2, "DC3" to 2)),
+                "another-service"
+            ),
+            currentZone = "DC2"
+        )
+        val loadAssignments = envoyEndpointsFactory.createLoadAssignment(setOf(serviceName), multiClusterStateDC2Local)
+        loadAssignments.assertHasLoadAssignment(
+            mapOf("DC1" to 1, "DC2" to 1, "DC3" to 1)
+        )
+    }
+
     private fun List<ClusterLoadAssignment>.assertHasLoadAssignment(map: Map<String, Int>) {
         assertThat(this)
             .isNotEmpty()
@@ -453,13 +487,22 @@ internal class EnvoyEndpointsFactoryTest {
         assertThat(this.locality.zone).isEqualTo(zone)
     }
 
-    private fun snapshotPropertiesWithPriorities(priorities: Map<String, Map<String, Int>>) =
+    private fun snapshotPropertiesWithPriorities(
+        priorities: Map<String, Map<String, Int>>,
+        servicePriorities: Map<String, Map<String, Int>> = mapOf(),
+        serviceName: String? = null
+    ) =
         SnapshotProperties().apply {
             loadBalancing = LoadBalancingProperties()
                 .apply {
                     this.priorities = LoadBalancingPriorityProperties().apply {
                         zonePriorities = priorities
                     }
+                    serviceName?.let {
+                        this.servicePriorities = mapOf(serviceName to LoadBalancingPriorityProperties().apply {
+                            zonePriorities = servicePriorities
+                        })
+                    }
                 }
         }