Migrate ExecutorTest to TestContainers

[kyguy]

Summary

1. Why: This PR is the third iteration of the TestContainers migration following the initial migration Migrate CruiseControlMetricsReporterTest to TestContainers #2303 in effort to remove the Cruise Control tests dependence on internal Kafka APIs. As detailed in issue Migrate off internal Kafka APIs to Improve Compatibility and Maintainability Migrate off internal Kafka APIs to Improve Compatibility and Maintainability #2282, the existing Cruise Control tests rely on the Kafka server wrapper class, CCEmbeddedBroker, which uses several non-public Kafka APIs. These internal APIs lack stability and compatibility guarantees, making it increasingly difficult to upgrade Kafka versions. As Kafka evolves, this reliance introduces upgrade friction, version lock-in, and a growing maintenance burden.

2. What: This PR is the next step of the TestContainers migration following this Migrate CruiseControlMetricsReporterTest to TestContainers #2303. It refactors the ExecutorTest to replace its dependency on the CCEmbeddedBroker class with the TestContainers Kafka module instead. This helps future-proof Cruise Control by easing upgrades to newer Kafka versions, expanding Kafka versions compatibility, and reducing the ongoing maintenance overhead.

Categorization

• documentation
• bugfix
• new feature
• refactor
• security/CVE
• other

This PR partially resolves #2282.

[kyguy]

Would appreciate feedback or reviews from anyone available! No pressure of course, and thanks in advance!

CC @ppatierno @fvaleri @ShubhamRwt @tomncooper @mimaison @danielgospodinow

[danielgospodinow]

LGTM! Thanks for the change! I really love seeing this effort progress.

[fvaleri]

LGTM, but I wanted to ask what's the difference in execution time compared to the previous version.

[mimaison]

Thanks for the PR. I left a few questions

[kyguy]

but I wanted to ask what's the difference in execution time compared to the previous version.

@fvaleri The execution time will definitely be slower since containers are spun up for each test. Based on some local runs, it appears to take roughly twice as long to complete compared to the original implementation. I know this isn’t ideal, I’ll see if I can tune the current implementation to improve the execution speed.

[danielgospodinow]

Based on some local runs, it appears to take roughly twice as long to complete compared to the original implementation. I know this isn’t ideal

@kyguy, can you share some exact numbers?

But anyhow, If that's the price to pay to be independent of Kafka internal APIs, then it's worth it, IMO.

[fvaleri]

@kyguy a slower execution is expected. As @danielgospodinow I was wondering about exact numbers. If we can improve the execution speed that would be great, but I would do this after all tests are migrated.

[kyguy]

I was wondering about exact numbers.

@danielgospodinow @fvaleri I did 10 test runs of the ExecutorTest class locally for each implementation, recording the execution time for each run, here are the exact numbers:

Original implementation:

105s + 105s + 95s + 85s + 105s + 95s + 105s + 75s + 95s + 105s = 97 seconds

970s / 10 test runs = 97s avg -> 1m 37s

New implementation:

192s + 174s + 182s + 179s + 203s + 180s + 175s + 182s + 176s + 180s = 182 seconds

1823 / 10 test runs = 182s avg -> 3m 2s

Comparing the average execution times:

Percentage increase = ( (182s - 97s) / (97s) ) * 100 = ~87%

The execution time almost doubled.

[fvaleri]

@kyguy that's not too bad in exchange for better maintenance. Thanks!

[kyguy]

Hi @mimaison, could you have another pass when you get a chance?

[mimaison]

Thanks for the PR, LGTM

[kyguy]

CC @CCisGG if you don't have any objections, this PR is ready to merge!

[CCisGG]

Thanks @kyguy !