Skip to content

Commit 9fd2ce3

Browse files
KSmigielskiKSmigielski
committed
fix tests
1 parent e8a3971 commit 9fd2ce3

23 files changed

+144
-294
lines changed

‎envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/server/callbacks/MetricsDiscoveryServerCallbacks.kt

+3-3
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class MetricsDiscoveryServerCallbacks(private val meterRegistry: MeterRegistry)
3636

3737
meterRegistry.gauge("grpc.all-connections", connections)
3838
connectionsByType.forEach { (type, typeConnections) ->
39-
meterRegistry.gauge("grpc.connections.${type.name.toLowerCase()}", typeConnections)
39+
meterRegistry.gauge("grpc.connections.${type.name.lowercase()}", typeConnections)
4040
}
4141
}
4242

@@ -51,15 +51,15 @@ class MetricsDiscoveryServerCallbacks(private val meterRegistry: MeterRegistry)
5151
}
5252

5353
override fun onV3StreamRequest(streamId: Long, request: V3DiscoveryRequest) {
54-
meterRegistry.counter("grpc.requests.${StreamType.fromTypeUrl(request.typeUrl).name.toLowerCase()}")
54+
meterRegistry.counter("grpc.requests.${StreamType.fromTypeUrl(request.typeUrl).name.lowercase()}")
5555
.increment()
5656
}
5757

5858
override fun onV3StreamDeltaRequest(
5959
streamId: Long,
6060
request: V3DeltaDiscoveryRequest
6161
) {
62-
meterRegistry.counter("grpc.requests.${StreamType.fromTypeUrl(request.typeUrl).name.toLowerCase()}.delta")
62+
meterRegistry.counter("grpc.requests.${StreamType.fromTypeUrl(request.typeUrl).name.lowercase()}.delta")
6363
.increment()
6464
}
6565

‎envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/EnvoySnapshotFactory.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,7 @@ class EnvoySnapshotFactory(
315315
routes.add(
316316
egressRoutesFactory.createEgressDomainRoutes(
317317
it.value,
318-
it.key.port.toString().toLowerCase()
318+
it.key.port.toString().lowercase()
319319
)
320320
)
321321
}

‎envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/clusters/EnvoyClustersFactory.kt

+1-1
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ class EnvoyClustersFactory(
464464
thresholdsBuilder.maxPendingRequests = UInt32Value.of(threshold.maxPendingRequests)
465465
thresholdsBuilder.maxRequests = UInt32Value.of(threshold.maxRequests)
466466
thresholdsBuilder.maxRetries = UInt32Value.of(threshold.maxRetries)
467-
when (threshold.priority.toUpperCase()) {
467+
when (threshold.priority.uppercase()) {
468468
"DEFAULT" -> thresholdsBuilder.priority = RoutingPriority.DEFAULT
469469
"HIGH" -> thresholdsBuilder.priority = RoutingPriority.HIGH
470470
else -> thresholdsBuilder.priority = RoutingPriority.UNRECOGNIZED

‎envoy-control-core/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/snapshot/resource/listeners/filters/HttpConnectionManagerFactory.kt

+8-32
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ package pl.allegro.tech.servicemesh.envoycontrol.snapshot.resource.listeners.fil
33
import com.google.protobuf.BoolValue
44
import com.google.protobuf.Duration
55
import com.google.protobuf.util.Durations
6-
import io.envoyproxy.envoy.config.core.v3.ApiConfigSource
6+
import io.envoyproxy.envoy.config.core.v3.AggregatedConfigSource
77
import io.envoyproxy.envoy.config.core.v3.ApiVersion
88
import io.envoyproxy.envoy.config.core.v3.ConfigSource
9-
import io.envoyproxy.envoy.config.core.v3.GrpcService
109
import io.envoyproxy.envoy.config.core.v3.Http1ProtocolOptions
1110
import io.envoyproxy.envoy.config.core.v3.HttpProtocolOptions
1211
import io.envoyproxy.envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager
@@ -34,7 +33,6 @@ class HttpConnectionManagerFactory(
3433
snapshotProperties.dynamicForwardProxy
3534
).filter
3635

37-
private val defaultApiConfigSourceV3: ApiConfigSource = apiConfigSource()
3836
private val accessLogFilter = AccessLogFilter(snapshotProperties)
3937

4038
@SuppressWarnings("LongParameterList")
@@ -93,7 +91,7 @@ class HttpConnectionManagerFactory(
9391
connectionManagerBuilder.addAccessLog(
9492
accessLogFilter.createFilter(
9593
listenersConfig.accessLogPath,
96-
direction.name.toLowerCase(),
94+
direction.name.lowercase(),
9795
listenersConfig.accessLogFilterSettings
9896
)
9997
)
@@ -106,19 +104,16 @@ class HttpConnectionManagerFactory(
106104
private fun setupRds(
107105
rdsInitialFetchTimeout: Duration,
108106
routeConfigName: String
109-
): Rds {
110-
val configSource = ConfigSource.newBuilder()
111-
.setInitialFetchTimeout(rdsInitialFetchTimeout)
112-
.setResourceApiVersion(ApiVersion.V3)
113-
.setApiConfigSource(defaultApiConfigSourceV3)
114-
115-
return Rds.newBuilder()
107+
): Rds = Rds.newBuilder()
116108
.setRouteConfigName(routeConfigName)
117109
.setConfigSource(
118-
configSource.build()
110+
ConfigSource.newBuilder()
111+
.setInitialFetchTimeout(rdsInitialFetchTimeout)
112+
.setResourceApiVersion(ApiVersion.V3)
113+
.setAds(AggregatedConfigSource.getDefaultInstance())
114+
.build()
119115
)
120116
.build()
121-
}
122117

123118
private fun ingressHttp1ProtocolOptions(serviceName: String): Http1ProtocolOptions? {
124119
return Http1ProtocolOptions.newBuilder()
@@ -127,25 +122,6 @@ class HttpConnectionManagerFactory(
127122
.build()
128123
}
129124

130-
private fun apiConfigSource(): ApiConfigSource {
131-
return ApiConfigSource.newBuilder()
132-
.setApiType(
133-
if (snapshotProperties.deltaXdsEnabled) {
134-
ApiConfigSource.ApiType.DELTA_GRPC
135-
} else {
136-
ApiConfigSource.ApiType.GRPC
137-
}
138-
)
139-
.setTransportApiVersion(ApiVersion.V3)
140-
.addGrpcServices(
141-
GrpcService.newBuilder()
142-
.setEnvoyGrpc(
143-
GrpcService.EnvoyGrpc.newBuilder()
144-
.setClusterName(snapshotProperties.xdsClusterName)
145-
)
146-
).build()
147-
}
148-
149125
private fun addHttpFilters(
150126
connectionManagerBuilder: HttpConnectionManager.Builder,
151127
filterFactories: List<HttpFilterFactory>,

‎envoy-control-core/src/test/kotlin/pl/allegro/tech/servicemesh/envoycontrol/groups/NodeMetadataValidatorTest.kt

+4-12
Original file line numberDiff line numberDiff line change
@@ -152,12 +152,8 @@ class NodeMetadataValidatorTest {
152152
// expects
153153
assertThatExceptionOfType(ServiceNameNotProvidedException::class.java)
154154
.isThrownBy { requireServiceNameValidator.onV3StreamRequest(streamId = 123, request = request) }
155-
.satisfies {
156-
assertThat(it.status.description).isEqualTo(
157-
"Service name has not been provided."
158-
)
159-
assertThat(it.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
160-
}
155+
.matches { it.status.description == "Service name has not been provided." }
156+
.matches { it.status.code == Status.Code.INVALID_ARGUMENT }
161157
}
162158

163159
@Test
@@ -187,12 +183,8 @@ class NodeMetadataValidatorTest {
187183
// then
188184
assertThatExceptionOfType(RateLimitIncorrectValidationException::class.java)
189185
.isThrownBy { validator.onV3StreamRequest(123, request = request) }
190-
.satisfies {
191-
assertThat(it.status.description).isEqualTo(
192-
"Rate limit value: 0/j is incorrect."
193-
)
194-
assertThat(it.status.code).isEqualTo(Status.Code.INVALID_ARGUMENT)
195-
}
186+
.matches { it.status.description == "Rate limit value: 0/j is incorrect." }
187+
.matches { it.status.code == Status.Code.INVALID_ARGUMENT }
196188
}
197189

198190
private fun createIncomingPermissions(

‎envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EndpointMetadataMergingTests.kt

+1-2
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,6 @@ open class EndpointMetadataMergingTests {
5555
protected open fun callEchoServiceRepeatedly(
5656
repeat: Int,
5757
tag: String,
58-
assertNoErrors: Boolean = true
5958
): CallStats {
6059
val stats = CallStats(listOf(service))
6160
envoy.egressOperations.callServiceRepeatedly(
@@ -64,7 +63,7 @@ open class EndpointMetadataMergingTests {
6463
minRepeat = repeat,
6564
maxRepeat = repeat,
6665
headers = mapOf("x-service-tag" to tag),
67-
assertNoErrors = assertNoErrors
66+
assertNoErrors = true
6867
)
6968
return stats
7069
}

‎envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/EnvoyControlHttp2Test.kt

+12-34
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ import pl.allegro.tech.servicemesh.envoycontrol.config.envoy.EnvoyExtension
1313
import pl.allegro.tech.servicemesh.envoycontrol.config.envoycontrol.EnvoyControlExtension
1414
import pl.allegro.tech.servicemesh.envoycontrol.config.service.EchoServiceExtension
1515

16-
class AdsEnvoyControlHttp2Test : EnvoyControlHttp2Test {
16+
class EnvoyControlHttp2Test {
17+
1718
companion object {
1819

1920
@JvmField
@@ -37,63 +38,40 @@ class AdsEnvoyControlHttp2Test : EnvoyControlHttp2Test {
3738
val secondEnvoy = EnvoyExtension(envoyControl, service, config = Ads)
3839
}
3940

40-
override fun consul() = consul
41-
42-
override fun envoyControl() = envoyControl
43-
44-
override fun service() = service
45-
46-
override fun envoy() = envoy
47-
48-
override fun secondEnvoy() = secondEnvoy
49-
}
50-
51-
interface EnvoyControlHttp2Test {
52-
53-
fun consul(): ConsulExtension
54-
55-
fun envoyControl(): EnvoyControlExtension
56-
57-
fun service(): EchoServiceExtension
58-
59-
fun envoy(): EnvoyExtension
60-
61-
fun secondEnvoy(): EnvoyExtension
62-
6341
@Test
6442
fun `should establish http2 connection between envoys`() {
6543
// given
66-
consul().server.operations.registerService(
44+
consul.server.operations.registerService(
6745
name = "proxy1",
68-
address = secondEnvoy().container.ipAddress(),
46+
address = secondEnvoy.container.ipAddress(),
6947
port = EnvoyContainer.INGRESS_LISTENER_CONTAINER_PORT,
7048
tags = listOf("envoy")
7149
)
7250

7351
untilAsserted {
7452
// when
75-
val response = envoy().egressOperations.callService("proxy1")
53+
val response = envoy.egressOperations.callService("proxy1")
7654
assertThat(response).isOk()
7755

7856
// then
79-
assertDidNotUseHttp1(envoy())
80-
assertUsedHttp2(envoy())
57+
assertDidNotUseHttp1(envoy)
58+
assertUsedHttp2(envoy)
8159
}
8260
}
8361

8462
@Test
8563
fun `should establish http1 connection between envoy and a service by default`() {
8664
// given
87-
consul().server.operations.registerService(service(), name = "echo")
65+
consul.server.operations.registerService(service, name = "echo")
8866

8967
untilAsserted {
9068
// when
91-
val response = envoy().egressOperations.callService("echo")
92-
assertThat(response).isOk().isFrom(service())
69+
val response = envoy.egressOperations.callService("echo")
70+
assertThat(response).isOk().isFrom(service)
9371

9472
// then
95-
assertUsedHttp1(envoy())
96-
assertDidNotUseHttp2(envoy())
73+
assertUsedHttp1(envoy)
74+
assertDidNotUseHttp2(envoy)
9775
}
9876
}
9977

‎envoy-control-tests/src/main/kotlin/pl/allegro/tech/servicemesh/envoycontrol/MetricsDiscoveryServerCallbacksTest.kt

+43-41
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package pl.allegro.tech.servicemesh.envoycontrol
22

3-
import io.micrometer.core.instrument.MeterRegistry
43
import org.assertj.core.api.Assertions.assertThat
54
import org.junit.jupiter.api.Test
65
import org.junit.jupiter.api.extension.RegisterExtension
@@ -19,6 +18,7 @@ import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscover
1918
import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscoveryServerCallbacks.StreamType.RDS
2019
import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscoveryServerCallbacks.StreamType.SDS
2120
import pl.allegro.tech.servicemesh.envoycontrol.server.callbacks.MetricsDiscoveryServerCallbacks.StreamType.UNKNOWN
21+
import java.time.Duration
2222

2323
class AdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbacksTest {
2424
companion object {
@@ -59,20 +59,20 @@ class AdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbacksTes
5959
)
6060

6161
override fun expectedGrpcRequestsCounterValues() = mapOf(
62-
CDS.name.toLowerCase() to isGreaterThanZero(),
63-
EDS.name.toLowerCase() to isGreaterThanZero(),
64-
LDS.name.toLowerCase() to isGreaterThanZero(),
65-
RDS.name.toLowerCase() to isGreaterThanZero(),
66-
SDS.name.toLowerCase() to isNull(),
67-
ADS.name.toLowerCase() to isNull(),
68-
UNKNOWN.name.toLowerCase() to isNull(),
69-
"${CDS.name.toLowerCase()}.delta" to isNull(),
70-
"${EDS.name.toLowerCase()}.delta" to isNull(),
71-
"${LDS.name.toLowerCase()}.delta" to isNull(),
72-
"${RDS.name.toLowerCase()}.delta" to isNull(),
73-
"${SDS.name.toLowerCase()}.delta" to isNull(),
74-
"${ADS.name.toLowerCase()}.delta" to isNull(),
75-
"${UNKNOWN.name.toLowerCase()}.delta" to isNull()
62+
CDS.name.lowercase() to isGreaterThanZero(),
63+
EDS.name.lowercase() to isGreaterThanZero(),
64+
LDS.name.lowercase() to isGreaterThanZero(),
65+
RDS.name.lowercase() to isGreaterThanZero(),
66+
SDS.name.lowercase() to isNull(),
67+
ADS.name.lowercase() to isNull(),
68+
UNKNOWN.name.lowercase() to isNull(),
69+
"${CDS.name.lowercase()}.delta" to isNull(),
70+
"${EDS.name.lowercase()}.delta" to isNull(),
71+
"${LDS.name.lowercase()}.delta" to isNull(),
72+
"${RDS.name.lowercase()}.delta" to isNull(),
73+
"${SDS.name.lowercase()}.delta" to isNull(),
74+
"${ADS.name.lowercase()}.delta" to isNull(),
75+
"${UNKNOWN.name.lowercase()}.delta" to isNull()
7676
)
7777
}
7878

@@ -105,7 +105,7 @@ class DeltaAdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbac
105105
override fun envoy() = envoy
106106

107107
override fun expectedGrpcConnectionsGaugeValues() = mapOf(
108-
CDS to 0,
108+
CDS to 1,
109109
EDS to 0,
110110
LDS to 0,
111111
RDS to 0,
@@ -115,20 +115,20 @@ class DeltaAdsMetricsDiscoveryServerCallbackTest : MetricsDiscoveryServerCallbac
115115
)
116116

117117
override fun expectedGrpcRequestsCounterValues() = mapOf(
118-
CDS.name.toLowerCase() to isNull(),
119-
EDS.name.toLowerCase() to isNull(),
120-
LDS.name.toLowerCase() to isNull(),
121-
RDS.name.toLowerCase() to isNull(),
122-
SDS.name.toLowerCase() to isNull(),
123-
ADS.name.toLowerCase() to isNull(),
124-
UNKNOWN.name.toLowerCase() to isNull(),
125-
"${CDS.name.toLowerCase()}.delta" to isGreaterThanZero(),
126-
"${EDS.name.toLowerCase()}.delta" to isGreaterThanZero(),
127-
"${LDS.name.toLowerCase()}.delta" to isGreaterThanZero(),
128-
"${RDS.name.toLowerCase()}.delta" to isGreaterThanZero(),
129-
"${SDS.name.toLowerCase()}.delta" to isNull(),
130-
"${ADS.name.toLowerCase()}.delta" to isNull(),
131-
"${UNKNOWN.name.toLowerCase()}.delta" to isNull()
118+
CDS.name.lowercase() to isNull(),
119+
EDS.name.lowercase() to isNull(),
120+
LDS.name.lowercase() to isNull(),
121+
RDS.name.lowercase() to isNull(),
122+
SDS.name.lowercase() to isNull(),
123+
ADS.name.lowercase() to isNull(),
124+
UNKNOWN.name.lowercase() to isNull(),
125+
"${CDS.name.lowercase()}.delta" to isGreaterThanZero(),
126+
"${EDS.name.lowercase()}.delta" to isGreaterThanZero(),
127+
"${LDS.name.lowercase()}.delta" to isGreaterThanZero(),
128+
"${RDS.name.lowercase()}.delta" to isGreaterThanZero(),
129+
"${SDS.name.lowercase()}.delta" to isNull(),
130+
"${ADS.name.lowercase()}.delta" to isNull(),
131+
"${UNKNOWN.name.lowercase()}.delta" to isNull()
132132
)
133133
}
134134

@@ -146,9 +146,7 @@ interface MetricsDiscoveryServerCallbacksTest {
146146

147147
fun expectedGrpcRequestsCounterValues(): Map<String, (Int?) -> Boolean>
148148

149-
fun MeterRegistry.counterValue(name: String) = this.find(name).counter()?.count()?.toInt()
150-
151-
fun isGreaterThanZero() = { x: Int? -> x!! > 0 }
149+
fun isGreaterThanZero() = { x: Int? -> x != null && x > 0 }
152150

153151
fun isNull() = { x: Int? -> x == null }
154152

@@ -159,11 +157,14 @@ interface MetricsDiscoveryServerCallbacksTest {
159157
consul().server.operations.registerService(service(), name = "echo")
160158

161159
// expect
162-
untilAsserted {
160+
untilAsserted(wait = Duration.ofSeconds(5)) {
163161
expectedGrpcConnectionsGaugeValues().forEach { (type, value) ->
164-
val metric = "grpc.connections.${type.name.toLowerCase()}"
165-
assertThat(meterRegistry.find(metric).gauge()).isNotNull
166-
assertThat(meterRegistry.get(metric).gauge().value().toInt()).isEqualTo(value)
162+
val metric = "grpc.connections.${type.name.lowercase()}"
163+
assertThat(meterRegistry.find(metric).gauge())
164+
.withFailMessage("Metric $metric should not be null")
165+
.isNotNull
166+
.withFailMessage("Value of metric $metric should be $value")
167+
.matches { it.value().toInt() == value }
167168
}
168169
}
169170
}
@@ -175,11 +176,12 @@ interface MetricsDiscoveryServerCallbacksTest {
175176
consul().server.operations.registerService(service(), name = "echo")
176177

177178
// expect
178-
untilAsserted {
179+
untilAsserted(wait = Duration.ofSeconds(5)) {
179180
expectedGrpcRequestsCounterValues().forEach { (type, condition) ->
180-
val counterValue = meterRegistry.counterValue("grpc.requests.$type")
181-
println("$type $counterValue")
182-
assertThat(counterValue).satisfies { condition(it) }
181+
val metric = "grpc.requests.$type"
182+
assertThat(meterRegistry.find(metric).counter()?.count()?.toInt())
183+
.withFailMessage("Metric $metric does not meet the condition")
184+
.matches(condition)
183185
}
184186
}
185187
}

0 commit comments

Comments
 (0)