Add Support for Clients

[see-quick]

This PR adds support for Kafka clients inside our test container ecosystem. Specifically producer and consumer.

[mimaison]

Thanks for the PR, I left a few comments

[mimaison]

Not directly related to this PR but Utils.waitFor() should accept Duration arguments and convert those into the unit it needs internally. Otherwise calling it is really cumbersome. I find Duration.ofSeconds(1).toMillis() less readable than 1000.

[scholzj]

Can you elaborate on the value of this?

[mimaison]

API-wise I wonder if it's worth supporting all the flags from the console producer and consumer. Some of the flags enable special behavior but many of them are just proxy for setting a configuration.

Instead of all these configuration methods, maybe just supporting providing a Map<String, String> or a file would be enough. That would really reduce the scope of the API. Sure the implementation of most methods is trivial, but its code, and its associated cost, that you carry in the future.

Overall the proposed API will be helpful for testing the metrics reporter. It might be worth checking with other component owners (I'm thinking of strimzi-kafka-oauth in particular which should have similar testing requirements) whether this satisfy their needs.

[im-konge]

Instead of all these configuration methods, maybe just supporting providing a Map<String, String> or a file would be enough.

Aren't the official Kafka clients in Java using exactly this? What is then the benefit of supporting the Kafka scripts, which can change with each new version of the Kafka? I mean, you have there the test-clients repository, which you can use directly. In case that it's wanted, I can (or someone else) add some easier usage and not deploying the clients as Jobs in K8s. Having another API that will implement the same thing as we have on X different places doesn't look to me as a great idea.

Or do you think that the test-clients are not that great in terms of the configuration? We can enhance them. But I would not implement almost the same thing across multiple repositories.

Overall the proposed API will be helpful for testing the metrics reporter.

I'm not familiar with the use-case of this in the metrics reporter (sorry about that) so maybe this will be a dumb question, but you will use it for testing of some clients' related metrics?

[mimaison]

Let me start by explaining my requirements, so you can advise me on the best options.

The metrics-reporter project can be used with both Kafka clients and brokers. As we're adding more complex features and aiming at a production ready release, I've started adding integration tests to complement the unit testing and ensure everything works in a real environment. In addition metrics-reporter can be used without Strimzi, so it should have its own test suite.

So far I've added integration tests for brokers using test-container. This worked well and allowed me to have all the testing code in Java and I did not have to write any shell scripts, or dockerfiles. You can see TestBrokerMetricsIT is actually quite nice. The setup injects the metrics reporter JARs and then tests can just set the configurations they need and check the metrics.

Now I would like to do something similar for the client side. To do so I need to start a Kafka cluster, and clients. Unfortunately this is not something I can do today with test-container.

I believe this requirement is not unique to metrics-reporter. I think strimzi-kafka-oauth has similar requirements, but correct me if I'm wrong. Looking at the integration tests for strimzi-kafka-oauth, I see that it relies on a quite a lot of custom logic, shell script, docker-compose files to build test environments.

Ideally I'd like to avoid all of that and write the whole integration test suite of metrics-reporter in plain Java using testcontainers, and this project (which is a wrapper) so it's easier to maintain and extend.

[im-konge]

Now I would like to do something similar for the client side. To do so I need to start a Kafka cluster, and clients. Unfortunately this is not something I can do today with test-container.

@Frawless added some tests using the Kafka cluster with usage of test-containers to the test-clients repository and he was able to test the clients part without issues. I think that instead of implementing the wheel again, we can just enhance the test-clients -> as I wanted to move the builder classes from the operators repository there -> and you can use it directly in the repository, from the local machine -> and test the clients side of things. I mean, currently in the test-clients repository we are releasing just the images. But we can also add release of the Maven artifacts, so we can use them also as the "external clients" in the operators STs, but also in the ITs of other components as well (we support there OAuth, tracing, HTTP clients, we have our own implementation of Admin client).

I mean, I'm not against implementing this, if it is really needed and I just don't see the use-case. But what I see is that:

• there will be confusion in terms of what should be used for testing what (we have one clients in the test-containers, other in the test-clients repository, what is used where?)
• it will be maintainability burden, where when you will like to add something and it will be related to also the second client implementation, you will have to do it on both places (or you will have to change API because of the changes in the particular component - OAuth, Bridge ...)
• you will add redundancy to the repositories
• what if you will want to use the HTTP and send/receive the message? (that's for example the Bridge case) You will have to keep it consistent and fix the issues we are experiencing with the Bridge from time to time in our clients

These are just few things that came to my mind.

And with the test-clients images, I think it's not a big issue to use them directly in the particular repository with usage of the basic TestContainers (where you will add the image and other configuration), if it is really needed -> as we can see in the case of the Apicurio in our test-clients ITs, @Frawless added the TestContainer for that without big issues (but correct me if I'm wrong Jakub).

[scholzj]

@mimaison I think this is a very hacky way for using test containers. I wonder ... what prevents you from using the clients from the test directly? Don't they have the metrics reporter on the classpath? If not, I think having a dedicated system test suite for this that simply does this on its own might be better?

[mimaison]

My main concern with this approach is that the metrics reporter opens a port. Running the client in a container is way nicer than requiring the host that is running tests to have specific ports available. In unit tests we can use 0 as the port to dynamically pick a suitable port and use a getter exposed for tests to find the actual value. However when running the reporter in a client, if we use 0 we'd have to parse the client logs to find the actual value. In these tests, I consider the client to be part of the environment. The actual test, the code that retrieves and checks the metrics, runs directly in the test and not in a container.

You're right that this may look like system tests. All the target environments are so simple with either one broker, or one broker and one client (we could probably not even require the broker to check client metrics) that standing it all in containers in Java is attractive.

[see-quick]

Closing this, as we discovered that using test-clients images is a better way to solve that problem :).