Skip to content

Commit bce122b

Browse files
jrhee17jrhee17
authored
Fix flaky xDS tests (#6768)
## Fix flaky xDS integration tests ### Motivation Several xDS integration tests are flaky due to a few different race conditions: 1. On macOS, the JDK polls by `stat()`-ing every file in the watched directory. `SelfSignedCertificateExtension` creates files in `$TMPDIR`, which is shared system-wide. Other processes concurrently deleting temp files causes `NoSuchFileException` during `WatchService` registration, preventing the xDS snapshot from arriving: ``` Caused by: java.nio.file.NoSuchFileException: /var/folders/.../T/keyutil_localhost_9963317911493234305.key at sun.nio.fs.PollingWatchService$PollingWatchKey.<init>(PollingWatchService.java:242) ``` 2. **Istio bootstrap file not yet available** — Istio sidecars write bootstrap JSON files asynchronously. Tests that read the file immediately after pod startup can fail if the sidecar hasn't written it yet. 3. `SnapshotWatcher` is added to `XdsBootstrap` add build time to avoid missing error events. 4. A `symlinkRotation` test has been added to keep a realistic scenario of key material switching behavior ### Modifications - Added `XdsCertificateExtension` that wraps `SelfSignedCertificateExtension` and copies cert/key files into an isolated `xds-certs-*` temp directory. The `WatchService` now monitors a directory containing only our files, eliminating the race with `$TMPDIR`. - Migrated affected test classes to use `XdsCertificateExtension` and reordered field declarations so certificate extensions are initialized before servers that reference them. - Added `XdsTestUtil.awaitAndReadBootstrapJson()` for Istio integration tests to await the sidecar's bootstrap file before reading it. - Expanded `XdsResourceReader`'s type registry to also scan `com.github.udpa` and `com.github.xds` packages to support Istio environments, and made lenient parsing (ignore unknown fields) the default. - Excluded `controlplane:api` from `it/xds-client` dependencies to avoid classpath conflicts. - Enabled Gradle build cache for `downloadIstioctl` task. ### Result - xDS integration tests are no longer flaky.
1 parent c224ff4 commit bce122b

24 files changed

Lines changed: 440 additions & 218 deletions

‎it/xds-client/build.gradle‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ dependencies {
22
api project(':xds')
33
api libs.jackson.dataformat.yaml
44
api libs.protobuf.java.util
5-
api libs.controlplane.server
6-
api libs.controlplane.cache
5+
api(libs.controlplane.server) {
6+
exclude group: 'io.envoyproxy.controlplane', module: 'api'
7+
}
8+
api(libs.controlplane.cache) {
9+
exclude group: 'io.envoyproxy.controlplane', module: 'api'
10+
}
711
}

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/BootstrapSecretsTest.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,12 @@
3737
class BootstrapSecretsTest {
3838

3939
@RegisterExtension
40-
static final SelfSignedCertificateExtension certificate1 = new SelfSignedCertificateExtension();
40+
static final XdsCertificateExtension certificate1 =
41+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
42+
4143
@RegisterExtension
42-
static final SelfSignedCertificateExtension certificate2 = new SelfSignedCertificateExtension();
44+
static final XdsCertificateExtension certificate2 =
45+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
4346

4447
//language=YAML
4548
private static final String staticBootstrap =

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/CertificateValidationContextTest.java‎

Lines changed: 29 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import com.linecorp.armeria.testing.junit5.server.ServerExtension;
4040
import com.linecorp.armeria.xds.CertificateValidationContextSnapshot;
4141
import com.linecorp.armeria.xds.ListenerSnapshot;
42+
import com.linecorp.armeria.xds.SnapshotWatcher;
4243
import com.linecorp.armeria.xds.TransportSocketSnapshot;
4344
import com.linecorp.armeria.xds.XdsBootstrap;
4445

@@ -68,10 +69,11 @@ protected void configure(ServerBuilder sb) {
6869
};
6970

7071
@RegisterExtension
71-
static final SelfSignedCertificateExtension certificate1 = new SelfSignedCertificateExtension();
72-
72+
static final XdsCertificateExtension certificate1 =
73+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
7374
@RegisterExtension
74-
static final SelfSignedCertificateExtension certificate2 = new SelfSignedCertificateExtension();
75+
static final XdsCertificateExtension certificate2 =
76+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
7577

7678
//language=YAML
7779
private static final String sdsBootstrapYaml =
@@ -172,12 +174,14 @@ void invalidCaCertificateFile(@TempDir File tempDir) throws Exception {
172174
final Bootstrap bootstrap = XdsResourceReader.fromYaml(bootstrapStr, Bootstrap.class);
173175

174176
final AtomicReference<Throwable> errorRef = new AtomicReference<>();
175-
try (XdsBootstrap xdsBootstrap = XdsBootstrap.of(bootstrap)) {
176-
xdsBootstrap.listenerRoot("my-listener").addSnapshotWatcher((snapshot, t) -> {
177-
if (t != null) {
178-
errorRef.set(t);
179-
}
180-
});
177+
final SnapshotWatcher<Object> watcher = (snapshot, t) -> {
178+
if (t != null) {
179+
errorRef.set(t);
180+
}
181+
};
182+
try (XdsBootstrap xdsBootstrap =
183+
XdsBootstrap.builder(bootstrap).defaultSnapshotWatcher(watcher).build()) {
184+
xdsBootstrap.listenerRoot("my-listener");
181185

182186
await().untilAsserted(() -> assertThat(errorRef.get()).isNotNull());
183187
assertThat(errorRef.get()).isInstanceOf(CertificateException.class);
@@ -207,12 +211,14 @@ void invalidSpkiPinFailsSnapshot() throws Exception {
207211
final Bootstrap bootstrap = XdsResourceReader.fromYaml(bootstrapStr, Bootstrap.class);
208212

209213
final AtomicReference<Throwable> errorRef = new AtomicReference<>();
210-
try (XdsBootstrap xdsBootstrap = XdsBootstrap.of(bootstrap)) {
211-
xdsBootstrap.listenerRoot("my-listener").addSnapshotWatcher((snapshot, t) -> {
212-
if (t != null) {
213-
errorRef.set(t);
214-
}
215-
});
214+
final SnapshotWatcher<Object> watcher = (snapshot, t) -> {
215+
if (t != null) {
216+
errorRef.set(t);
217+
}
218+
};
219+
try (XdsBootstrap xdsBootstrap =
220+
XdsBootstrap.builder(bootstrap).defaultSnapshotWatcher(watcher).build()) {
221+
xdsBootstrap.listenerRoot("my-listener");
216222

217223
await().untilAsserted(() -> assertThat(errorRef.get()).isNotNull());
218224
assertThat(errorRef.get()).hasRootCauseInstanceOf(ValidationException.class);
@@ -242,12 +248,14 @@ void invalidCertHashPinFailsSnapshot() throws Exception {
242248
final Bootstrap bootstrap = XdsResourceReader.fromYaml(bootstrapStr, Bootstrap.class);
243249

244250
final AtomicReference<Throwable> errorRef = new AtomicReference<>();
245-
try (XdsBootstrap xdsBootstrap = XdsBootstrap.of(bootstrap)) {
246-
xdsBootstrap.listenerRoot("my-listener").addSnapshotWatcher((snapshot, t) -> {
247-
if (t != null) {
248-
errorRef.set(t);
249-
}
250-
});
251+
final SnapshotWatcher<Object> watcher = (snapshot, t) -> {
252+
if (t != null) {
253+
errorRef.set(t);
254+
}
255+
};
256+
try (XdsBootstrap xdsBootstrap =
257+
XdsBootstrap.builder(bootstrap).defaultSnapshotWatcher(watcher).build()) {
258+
xdsBootstrap.listenerRoot("my-listener");
251259

252260
await().untilAsserted(() -> assertThat(errorRef.get()).isNotNull());
253261
assertThat(errorRef.get()).hasRootCauseInstanceOf(ValidationException.class);

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/ControlPlaneTlsIntegrationTest.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,13 @@ class ControlPlaneTlsIntegrationTest {
6969

7070
@RegisterExtension
7171
@Order(0)
72-
static final SelfSignedCertificateExtension controlPlaneCert =
73-
new SelfSignedCertificateExtension("127.0.0.1");
72+
static final XdsCertificateExtension controlPlaneCert =
73+
new XdsCertificateExtension(new SelfSignedCertificateExtension("127.0.0.1"));
7474

7575
@RegisterExtension
7676
@Order(0)
77-
static final SelfSignedCertificateExtension clientCert =
78-
new SelfSignedCertificateExtension("client.example.com");
77+
static final XdsCertificateExtension clientCert =
78+
new XdsCertificateExtension(new SelfSignedCertificateExtension("client.example.com"));
7979

8080
@RegisterExtension
8181
@Order(1)

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/DataSourceTest.java‎

Lines changed: 153 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,17 @@
2121

2222
import java.io.File;
2323
import java.nio.file.Files;
24+
import java.nio.file.Path;
25+
import java.nio.file.Paths;
2426
import java.nio.file.StandardCopyOption;
2527
import java.time.Duration;
2628
import java.util.Base64;
2729
import java.util.concurrent.atomic.AtomicLong;
2830
import java.util.concurrent.atomic.AtomicReference;
2931

3032
import org.junit.jupiter.api.Test;
33+
import org.junit.jupiter.api.condition.DisabledOnOs;
34+
import org.junit.jupiter.api.condition.OS;
3135
import org.junit.jupiter.api.extension.RegisterExtension;
3236
import org.junit.jupiter.api.io.TempDir;
3337

@@ -68,10 +72,12 @@ protected void configure(ServerBuilder sb) {
6872
};
6973

7074
@RegisterExtension
71-
static final SelfSignedCertificateExtension certificate1 = new SelfSignedCertificateExtension();
75+
static final XdsCertificateExtension certificate1 =
76+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
7277

7378
@RegisterExtension
74-
static final SelfSignedCertificateExtension certificate2 = new SelfSignedCertificateExtension();
79+
static final XdsCertificateExtension certificate2 =
80+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
7581

7682
@Test
7783
void tlsCertificateWithPrivateKeyAndCertificateChain() throws Exception {
@@ -1032,4 +1038,149 @@ void emptyDataSource() throws Exception {
10321038
await().untilAsserted(() -> assertThat(errorRef.get()).isNotNull());
10331039
}
10341040
}
1041+
1042+
@Test
1043+
@DisabledOnOs(OS.WINDOWS) // Symbolic links require elevated privileges on Windows.
1044+
void symlinkRotation(@TempDir File tempDir) throws Exception {
1045+
final Path actualDir = tempDir.toPath().resolve("actual");
1046+
final File privateKeyFile = new File(actualDir.toFile(), "private_key.pem");
1047+
final File certificateFile = new File(actualDir.toFile(), "certificate.pem");
1048+
1049+
//language=YAML
1050+
final String tlsCertYaml =
1051+
"""
1052+
name: my-cert
1053+
tls_certificate:
1054+
watched_directory:
1055+
path: %s
1056+
private_key:
1057+
filename: %s
1058+
certificate_chain:
1059+
filename: %s
1060+
""".formatted(tempDir.getAbsolutePath(),
1061+
privateKeyFile.getAbsolutePath(),
1062+
certificateFile.getAbsolutePath());
1063+
final Secret secret = XdsResourceReader.fromYaml(tlsCertYaml, Secret.class);
1064+
version.incrementAndGet();
1065+
cache.setSnapshot(GROUP, Snapshot.create(ImmutableList.of(), ImmutableList.of(), ImmutableList.of(),
1066+
ImmutableList.of(), ImmutableList.of(secret),
1067+
version.toString()));
1068+
1069+
//language=YAML
1070+
final String bootstrapStr =
1071+
"""
1072+
dynamic_resources:
1073+
ads_config:
1074+
api_type: GRPC
1075+
grpc_services:
1076+
- envoy_grpc:
1077+
cluster_name: bootstrap-cluster
1078+
static_resources:
1079+
clusters:
1080+
- name: bootstrap-cluster
1081+
type: STATIC
1082+
load_assignment:
1083+
cluster_name: bootstrap-cluster
1084+
endpoints:
1085+
- lb_endpoints:
1086+
- endpoint:
1087+
address:
1088+
socket_address:
1089+
address: 127.0.0.1
1090+
port_value: %s
1091+
- name: my-cluster
1092+
type: STATIC
1093+
load_assignment:
1094+
cluster_name: my-cluster
1095+
endpoints:
1096+
- lb_endpoints:
1097+
- endpoint:
1098+
address:
1099+
socket_address:
1100+
address: 127.0.0.1
1101+
port_value: 8080
1102+
transport_socket:
1103+
name: envoy.transport_sockets.tls
1104+
typed_config:
1105+
"@type": type.googleapis.com/envoy.extensions.transport_sockets\
1106+
.tls.v3.UpstreamTlsContext
1107+
common_tls_context:
1108+
tls_certificate_sds_secret_configs:
1109+
- name: my-cert
1110+
sds_config:
1111+
ads: {}
1112+
listeners:
1113+
- name: my-listener
1114+
api_listener:
1115+
api_listener:
1116+
"@type": type.googleapis.com/envoy.extensions.filters.network.http_connection_manager\
1117+
.v3.HttpConnectionManager
1118+
stat_prefix: http
1119+
route_config:
1120+
name: local_route
1121+
virtual_hosts:
1122+
- name: local_service1
1123+
domains: [ "*" ]
1124+
routes:
1125+
- match:
1126+
prefix: /
1127+
route:
1128+
cluster: my-cluster
1129+
http_filters:
1130+
- name: envoy.filters.http.router
1131+
typed_config:
1132+
"@type": type.googleapis.com/envoy.extensions.filters.http.router.v3.Router
1133+
""".formatted(server.httpPort());
1134+
1135+
final Bootstrap bootstrap = XdsResourceReader.fromYaml(bootstrapStr, Bootstrap.class);
1136+
try (XdsBootstrap xdsBootstrap = XdsBootstrap.of(bootstrap)) {
1137+
final ListenerRoot listenerRoot = xdsBootstrap.listenerRoot("my-listener");
1138+
final AtomicReference<ListenerSnapshot> snapshotRef = new AtomicReference<>();
1139+
listenerRoot.addSnapshotWatcher((snapshot, t) -> {
1140+
if (snapshot != null) {
1141+
snapshotRef.set(snapshot);
1142+
}
1143+
});
1144+
1145+
// 'actual' symlink doesn't exist yet — no snapshot should arrive.
1146+
await().during(Duration.ofSeconds(2))
1147+
.untilAsserted(() -> assertThat(snapshotRef.get()).isNull());
1148+
1149+
// Write certificate1 to staging_v1, then atomically create the 'actual' symlink.
1150+
final Path stagingV1 = tempDir.toPath().resolve("staging_v1");
1151+
Files.createDirectory(stagingV1);
1152+
Files.copy(certificate1.privateKeyFile().toPath(), stagingV1.resolve("private_key.pem"));
1153+
Files.copy(certificate1.certificateFile().toPath(), stagingV1.resolve("certificate.pem"));
1154+
final Path tmpLink = tempDir.toPath().resolve("actual_tmp");
1155+
Files.createSymbolicLink(tmpLink, Paths.get("staging_v1"));
1156+
Files.move(tmpLink, actualDir, StandardCopyOption.ATOMIC_MOVE);
1157+
1158+
await().untilAsserted(() -> assertThat(snapshotRef.get()).isNotNull());
1159+
final ListenerSnapshot snapshot1 = snapshotRef.get();
1160+
final TlsCertificateSnapshot cert1 =
1161+
snapshot1.routeSnapshot().virtualHostSnapshots().get(0)
1162+
.routeEntries().get(0).clusterSnapshot().transportSocket().tlsCertificate();
1163+
assertThat(cert1).isNotNull();
1164+
assertThat(cert1.tlsKeyPair()).isEqualTo(certificate1.tlsKeyPair());
1165+
1166+
// Rotate to certificate2: write to staging_v2, then atomically swap the 'actual' symlink.
1167+
final Path stagingV2 = tempDir.toPath().resolve("staging_v2");
1168+
Files.createDirectory(stagingV2);
1169+
Files.copy(certificate2.privateKeyFile().toPath(), stagingV2.resolve("private_key.pem"));
1170+
Files.copy(certificate2.certificateFile().toPath(), stagingV2.resolve("certificate.pem"));
1171+
final Path tmpLink2 = tempDir.toPath().resolve("actual_tmp");
1172+
Files.createSymbolicLink(tmpLink2, Paths.get("staging_v2"));
1173+
Files.move(tmpLink2, actualDir, StandardCopyOption.ATOMIC_MOVE);
1174+
1175+
await().untilAsserted(() -> {
1176+
final ListenerSnapshot current = snapshotRef.get();
1177+
assertThat(current).isNotNull();
1178+
final TlsCertificateSnapshot cert =
1179+
current.routeSnapshot().virtualHostSnapshots().get(0)
1180+
.routeEntries().get(0).clusterSnapshot().transportSocket().tlsCertificate();
1181+
assertThat(cert).isNotNull();
1182+
assertThat(cert.tlsKeyPair()).isEqualTo(certificate2.tlsKeyPair());
1183+
});
1184+
}
1185+
}
10351186
}

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/DynamicSecretTest.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,17 @@ protected void configure(ServerBuilder sb) {
6666
};
6767

6868
@RegisterExtension
69-
static final SelfSignedCertificateExtension certificate1 = new SelfSignedCertificateExtension();
69+
static final XdsCertificateExtension certificate1 =
70+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
7071
@RegisterExtension
71-
static final SelfSignedCertificateExtension certificate2 = new SelfSignedCertificateExtension();
72+
static final XdsCertificateExtension certificate2 =
73+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
7274
@RegisterExtension
73-
static final SelfSignedCertificateExtension certificate3 = new SelfSignedCertificateExtension();
75+
static final XdsCertificateExtension certificate3 =
76+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
7477
@RegisterExtension
75-
static final SelfSignedCertificateExtension certificate4 = new SelfSignedCertificateExtension();
78+
static final XdsCertificateExtension certificate4 =
79+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
7680

7781
//language=YAML
7882
private static final String tlsCertYaml =

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/ErrorHandlingTest.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ protected void configure(ServerBuilder sb) {
8686
};
8787

8888
@RegisterExtension
89-
static final SelfSignedCertificateExtension certificate = new SelfSignedCertificateExtension();
89+
static final XdsCertificateExtension certificate =
90+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
9091

9192
//language=YAML
9293
private static final String listenerYaml =

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/PipeEndpointTest.java‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -71,13 +71,13 @@ class PipeEndpointTest {
7171

7272
@RegisterExtension
7373
@Order(0)
74-
static final SelfSignedCertificateExtension serverCert =
75-
new SelfSignedCertificateExtension("localhost");
74+
static final XdsCertificateExtension serverCert =
75+
new XdsCertificateExtension(new SelfSignedCertificateExtension("localhost"));
7676

7777
@RegisterExtension
7878
@Order(0)
79-
static final SelfSignedCertificateExtension clientCert =
80-
new SelfSignedCertificateExtension();
79+
static final XdsCertificateExtension clientCert =
80+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
8181

8282
@RegisterExtension
8383
@Order(1)
@@ -326,7 +326,7 @@ void sdsViaControlPlanePipe() throws Exception {
326326
}
327327
}
328328

329-
private static Secret tlsCertSecret(String name, SelfSignedCertificateExtension cert) {
329+
private static Secret tlsCertSecret(String name, XdsCertificateExtension cert) {
330330
final String yaml = """
331331
name: %s
332332
tls_certificate:
@@ -341,7 +341,7 @@ private static Secret tlsCertSecret(String name, SelfSignedCertificateExtension
341341
}
342342

343343
private static Secret validationContextSecret(String name,
344-
SelfSignedCertificateExtension cert)
344+
XdsCertificateExtension cert)
345345
throws Exception {
346346
final byte[] caBytes = Files.readAllBytes(cert.certificateFile().toPath());
347347
final String yaml = """

‎it/xds-client/src/test/java/com/linecorp/armeria/xds/it/ResourceNodeMetricTest.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,12 @@ protected void configure(ServerBuilder sb) {
8585
};
8686

8787
@RegisterExtension
88-
static final SelfSignedCertificateExtension certificate1 = new SelfSignedCertificateExtension();
88+
static final XdsCertificateExtension certificate1 =
89+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
8990

9091
@RegisterExtension
91-
static final SelfSignedCertificateExtension certificate2 = new SelfSignedCertificateExtension();
92+
static final XdsCertificateExtension certificate2 =
93+
new XdsCertificateExtension(new SelfSignedCertificateExtension());
9294

9395
//language=YAML
9496
private static final String bootstrapYaml =

0 commit comments

Comments
 (0)