rzo1 opened a new pull request, #153:
URL: https://github.com/apache/openjpa/pull/153
`KubernetesTCPRemoteCommitProviderTest.addresses` fails intermittently on CI
with:
```
java.lang.AssertionError: expected:<2> but was:<0>
at
org.apache.openjpa.event.kubernetes.KubernetesTCPRemoteCommitProviderTest.addresses(KubernetesTCPRemoteCommitProviderTest.java:166)
```
### Root cause
`KubernetesTCPRemoteCommitProvider.fetchDynamicAddresses()` closes the
client it obtains:
```java
try (KubernetesClient client = kubernetesClient()) { ... }
```
In production that is fine, since `kubernetesClient()` builds a new client
on every call. The test, however, overrides it to return `server.getClient()` —
the *shared* client of the mock server — so the first fetch closes it and every
later fetch fails and returns an empty list.
That matters because `DynamicTCPRemoteCommitProvider.endConfiguration()`
runs the updater once synchronously **and** then schedules it with an initial
delay of `0`:
```java
updater.run();
timer.scheduleAtFixedRate(updater, 0, _cacheDurationMillis);
```
So a second run always fires immediately on the timer thread, gets an empty
address list, and the removal step drops both addresses again. Whether the
assertion reads `_addresses` before or after that run is a pure race — green on
a fast machine, flaky on CI.
Reproducible deterministically by inserting a `Thread.sleep(500)` before the
assertion: it then fails with `expected:<2> but was:<0>` on every run.
### Change
- hand out a fresh client per invocation
(`server.getKubernetesMockServer().createClient()`), exactly as the production
implementation does
- drop the misleading `mock will drop all IPs if test will run too long`
comment — the mock never dropped anything, the closed client did
- refresh every 100 ms and assert the addresses again after several refresh
cycles, so the defect is now caught deterministically instead of racily
### Verification
- `mvn -pl openjpa-kubernetes test` -> passes, 4 consecutive runs
- reverting only the client line makes it fail on every run:
`addresses:175->assertAddresses:180 expected:<2> but was:<0>`
### Note
Production code is untouched. The redundant immediate second run in
`DynamicTCPRemoteCommitProvider.endConfiguration()` (initial delay `0` right
after a synchronous `run()`) is harmless once the fetches no longer fail, but
changing the initial delay to `_cacheDurationMillis` would be a sensible
follow-up.
No JIRA key on the commit yet — happy to reword the subject if you want this
filed under a specific issue.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
