test: fix flaky KubernetesTaskRunnerTest start tests

[FrankChen021]

Related to #20312 (item 1).

Description

KubernetesTaskRunnerTest.test_start_whenDeserializationExceptionThrown_isIgnored fails intermittently on master with:

Unexpected method calls:
  EasyMock for field KubernetesTaskRunnerTest.peonClient -> KubernetesPeonClient.deleteCompletedPeonJobsOlderThan(172800000 (long), MILLISECONDS)

Example: https://github.com/apache/druid/actions/runs/34431507380/job/102727804299 (failed all 4 surefire attempts).

KubernetesTaskRunner.start() schedules deleteCompletedPeonJobsOlderThan on a real ScheduledExecutorService with an initial delay of 1 ms. Whether the first cleanup runs before the test's verifyAll() depends on thread scheduling, so the strict mock sometimes sees a call it was not told to expect.

Changes

• In the three tests that call runner.start() on a locally created runner, expect deleteCompletedPeonJobsOlderThan(anyLong(), MILLISECONDS) any number of times via a small helper. Only this one call is relaxed; the rest of peonClient stays strictly verified.
• Stop each locally created runner in a finally block so the cleanup executor does not outlive the test.

Verified locally with mvn -pl extensions-core/kubernetes-overlord-extensions test -Dtest=KubernetesTaskRunnerTest.

---

Key changed/added classes in this PR

• KubernetesTaskRunnerTest

---

This PR has:

• been self-reviewed.
• added or updated unit tests.

Additional CI evidence

This same scheduled-cleanup race reappeared as a recovered flaky failure in PR #20330, unit-test job:

Unexpected method calls:
  KubernetesPeonClient.deleteCompletedPeonJobsOlderThan(172800000 (long), MILLISECONDS)

The corresponding PR #20329 unit run did not reach this assertion: its embedded tests failed earlier with NoClassDefFoundError: org/apache/curator/test/TestingServer after the curator-test dependency was removed.

[Copilot]

🟢 Approval recommended

The changes are narrowly scoped to tests and deterministically address the known race without weakening unrelated mock verification.

Pull request overview

This PR fixes intermittent flakiness in KubernetesTaskRunnerTest caused by KubernetesTaskRunner.start() scheduling background cleanup work that can race with strict EasyMock verification.

Changes:

• Added a small test helper to allow peonClient.deleteCompletedPeonJobsOlderThan(..., MILLISECONDS) to be invoked any number of times during start().
• Wrapped the three tests that construct and start() a local runner in try/finally to ensure runner.stop() is always called and background executors don’t outlive the test.

File summaries

File	Description
extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/KubernetesTaskRunnerTest.java	Makes start()-related tests deterministic by relaxing only the scheduled cleanup call and ensuring locally created runners are stopped reliably.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[FrankChen021]

The unit tests (25, C*) failure on this PR is unrelated to the change here (this PR only touches KubernetesTaskRunnerTest, which is not in the C* shard).

The failing test is CompactionTaskTest.testCompaction:

CompactionTaskTest.testCompaction -> CompactionTestBase.verifySegmentsCount:102
Segment count mismatch in sys.segments table ==> expected: <2> but was: <4>

This is the flake tracked in #20312 (item 7) and fixed by #20320: verifySegmentsCount waits for a single segment/metadataCache/sync/time event and then asserts the sys.segments count once, so a sync that was already in flight can still report the pre-compaction state (here the 4 original segments). #20320 keeps the Overlord assertion and polls the Broker-side count instead.

[FrankChen021]

🟢 Approval recommended

No actionable issues found in this review. The only changed file is extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/KubernetesTaskRunnerTest.java; I inspected it in full along with the KubernetesTaskRunner.start()/stop() implementation and the relevant test fixture setup. The new expectation is limited to the scheduled cleanup call, while the existing start expectations remain strict, and each locally created runner that is started is stopped in finally.

Changed file reviewed: extensions-core/kubernetes-overlord-extensions/src/test/java/org/apache/druid/k8s/overlord/KubernetesTaskRunnerTest.java.

Reviewed 1 of 1 changed files.

Validation: git diff --check passed for the prepared PR-head diff. No builds or tests were run, per the requested static-only scope.

---

This is an automated review by Codex GPT-5.6-Luna(max)