From 6f22a401f2568a5ae8f98b0eb23f36bc8d0c9c20 Mon Sep 17 00:00:00 2001 From: "nastassia.dailidava" Date: Wed, 13 Dec 2023 18:05:21 +0100 Subject: [PATCH] Fixed traffic splitting for endpoints with tag metadata --- .../snapshot/EnvoySnapshotFactory.kt | 6 +- .../trafficsplitting/TrafficSplitting.kt | 5 + .../WeightedClustersRoutingTest.kt | 15 --- .../WeightedClustersTagRoutingTest.kt | 119 ++++++++++++++++++ 4 files changed, 127 insertions(+), 18 deletions(-) create mode 100644 envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/WeightedClustersTagRoutingTest.kt diff --git a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt index ae160ef02..7cc48dffe 100644 --- a/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt +++ b/envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt @@ -225,9 +225,9 @@ class EnvoySnapshotFactory( ): 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 + val enabledForDependency = globalSnapshot.endpoints[clusterName]?.let { + endpointsFactory.filterEndpoints(it, settings.routingPolicy) + }?.endpointsList?.any { e -> trafficSplitting.zoneName == e.locality.zone && e.lbEndpointsCount > 0 } ?: false return if (weights != null && enabledForDependency) { logger.debug( "Building traffic splitting route spec, weights: $weights, " + diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/TrafficSplitting.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/TrafficSplitting.kt index a49c7f996..31094b6ea 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/TrafficSplitting.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/TrafficSplitting.kt @@ -28,6 +28,11 @@ fun CallStats.verifyCallsCountCloseTo(service: EchoServiceExtension, expectedCou return this } +fun CallStats.verifyCallsCountEquals(service: EchoServiceExtension, expectedCount: Int): CallStats { + Assertions.assertThat(this.hits(service)).isEqualTo(expectedCount) + return this +} + fun CallStats.verifyCallsCountGreaterThan(service: EchoServiceExtension, hits: Int): CallStats { Assertions.assertThat(this.hits(service)).isGreaterThan(hits) return this diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/WeightedClustersRoutingTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/WeightedClustersRoutingTest.kt index 564660fce..be3d28cee 100644 --- a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/WeightedClustersRoutingTest.kt +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/WeightedClustersRoutingTest.kt @@ -96,19 +96,4 @@ class WeightedClustersRoutingTest { .verifyCallsCountCloseTo(upstreamServiceDC1, 90) .verifyCallsCountGreaterThan(upstreamServiceDC2, 1) } - - @Test - fun `should route traffic according to weights with service tag`() { - consul.serverFirst.operations.registerServiceWithEnvoyOnEgress(echoEnvoyDC1, name = serviceName) - - consul.serverFirst.operations.registerService(upstreamServiceDC1, name = upstreamServiceName, tags = listOf("tag")) - echoEnvoyDC1.verifyIsReachable(upstreamServiceDC1, upstreamServiceName) - - consul.serverSecond.operations.registerService(upstreamServiceDC2, name = upstreamServiceName, tags = listOf("tag")) - echoEnvoyDC1.verifyIsReachable(upstreamServiceDC2, upstreamServiceName) - - echoEnvoyDC1.callUpstreamServiceRepeatedly(upstreamServiceDC1, upstreamServiceDC2, tag = "tag") - .verifyCallsCountCloseTo(upstreamServiceDC1, 90) - .verifyCallsCountGreaterThan(upstreamServiceDC2, 1) - } } diff --git a/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/WeightedClustersTagRoutingTest.kt b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/WeightedClustersTagRoutingTest.kt new file mode 100644 index 000000000..8fac3531d --- /dev/null +++ b/envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/trafficsplitting/WeightedClustersTagRoutingTest.kt @@ -0,0 +1,119 @@ +package pl.allegro.tech.servicemesh.envoycontrol.trafficsplitting + +import TrafficSplitting.serviceName +import TrafficSplitting.upstreamServiceName +import callUpstreamServiceRepeatedly +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.extension.RegisterExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.Xds +import pl.allegro.tech.servicemesh.envoycontrol.config.consul.ConsulMultiClusterExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlClusteredExtension +import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension +import verifyCallsCountCloseTo +import verifyCallsCountEquals +import verifyIsReachable +import java.time.Duration + +class WeightedClustersTagRoutingTest { + companion object { + private const val forceTrafficZone = "dc2" + + private val properties = mapOf( + "logging.level.pl.allegro.tech.servicemesh.envoycontrol.snapshot.EnvoySnapshotFactory" to "DEBUG", + "envoy-control.envoy.snapshot.stateSampleDuration" to Duration.ofSeconds(0), + "envoy-control.sync.enabled" to true, + "envoy-control.envoy.snapshot.loadBalancing.trafficSplitting.zoneName" to forceTrafficZone, + "envoy-control.envoy.snapshot.loadBalancing.trafficSplitting.serviceByWeightsProperties.$serviceName.main" to 50, + "envoy-control.envoy.snapshot.loadBalancing.trafficSplitting.serviceByWeightsProperties.$serviceName.secondary" to 50, + "envoy-control.envoy.snapshot.routing.service-tags.enabled" to true, + "envoy-control.envoy.snapshot.routing.service-tags.metadata-key" to "x-service-tag", + "envoy-control.envoy.snapshot.loadBalancing.priorities.zonePriorities" to mapOf( + "dc1" to mapOf( + "dc1" to 0, + "dc2" to 1 + ), + "dc2" to mapOf( + "dc1" to 1, + "dc2" to 0, + ), + ) + ) + + private val echo2Config = """ + node: + metadata: + proxy_settings: + outgoing: + routingPolicy: + serviceTagPreference: ["tag"] + dependencies: + - service: "service-1" + """.trimIndent() + + private val config = Xds.copy(configOverride = echo2Config, serviceName = "echo2") + + @JvmField + @RegisterExtension + val consul = ConsulMultiClusterExtension() + + @JvmField + @RegisterExtension + val envoyControl = + EnvoyControlClusteredExtension(consul.serverFirst, { properties }, listOf(consul)) + + @JvmField + @RegisterExtension + val envoyControl2 = + EnvoyControlClusteredExtension(consul.serverSecond, { properties }, listOf(consul)) + + @JvmField + @RegisterExtension + val echoServiceDC1 = EchoServiceExtension() + + @JvmField + @RegisterExtension + val upstreamServiceDC1 = EchoServiceExtension() + + @JvmField + @RegisterExtension + val upstreamServiceDC2 = EchoServiceExtension() + + @JvmField + @RegisterExtension + val echoEnvoyDC1 = EnvoyExtension(envoyControl, localService = echoServiceDC1, config) + @JvmField + @RegisterExtension + val echoEnvoyDC2 = EnvoyExtension(envoyControl2) + } + + @Test + fun `should route traffic according to weights with service tag`() { + consul.serverFirst.operations.registerServiceWithEnvoyOnEgress(echoEnvoyDC1, name = serviceName) + + consul.serverFirst.operations.registerService(upstreamServiceDC1, name = upstreamServiceName, tags = listOf("tag")) + echoEnvoyDC1.verifyIsReachable(upstreamServiceDC1, upstreamServiceName) + + consul.serverSecond.operations.registerService(upstreamServiceDC2, name = upstreamServiceName, tags = listOf("tag")) + echoEnvoyDC1.verifyIsReachable(upstreamServiceDC2, upstreamServiceName) + + echoEnvoyDC1.callUpstreamServiceRepeatedly(upstreamServiceDC1, upstreamServiceDC2, tag = "tag") + .verifyCallsCountCloseTo(upstreamServiceDC1, 50) + .verifyCallsCountCloseTo(upstreamServiceDC2, 50) + } + + @Test + fun `should not split traffic with service tag if there are no endpoints in zone`() { + consul.serverFirst.operations.registerServiceWithEnvoyOnEgress(echoEnvoyDC1, name = serviceName) + + consul.serverFirst.operations.registerService(upstreamServiceDC1, name = upstreamServiceName, tags = listOf("tag")) + echoEnvoyDC1.verifyIsReachable(upstreamServiceDC1, upstreamServiceName) + + consul.serverSecond.operations.registerService(upstreamServiceDC2, name = upstreamServiceName, tags = listOf()) + echoEnvoyDC2.verifyIsReachable(upstreamServiceDC2, upstreamServiceName) + + echoEnvoyDC1.callUpstreamServiceRepeatedly(upstreamServiceDC1, upstreamServiceDC2, tag = "tag") + .verifyCallsCountCloseTo(upstreamServiceDC1, 100) + .verifyCallsCountEquals(upstreamServiceDC2, 0) + } +}